diff --git a/packages/api-key/integration-tests/__tests__/api-key-module-service.spec.ts b/packages/api-key/integration-tests/__tests__/api-key-module-service.spec.ts index c2d02d7706..0b1e179601 100644 --- a/packages/api-key/integration-tests/__tests__/api-key-module-service.spec.ts +++ b/packages/api-key/integration-tests/__tests__/api-key-module-service.spec.ts @@ -53,7 +53,7 @@ moduleIntegrationTestRunner({ expect.objectContaining({ title: "Test API Key", type: ApiKeyType.PUBLISHABLE, - salt: "", + salt: undefined, created_by: "test", last_used_at: null, revoked_by: null, @@ -75,7 +75,7 @@ moduleIntegrationTestRunner({ expect.objectContaining({ title: "Secret key", type: ApiKeyType.SECRET, - salt: "44de31ebcf085fa423fc584aa8540670", + salt: undefined, created_by: "test", last_used_at: null, revoked_by: null, @@ -136,6 +136,16 @@ moduleIntegrationTestRunner({ ) }) + it("should do nothing if the revokal list is empty", async function () { + const firstApiKey = await service.create(createSecretKeyFixture) + let revokedKeys = await service.revoke([]) + expect(revokedKeys).toHaveLength(0) + + const apiKey = await service.retrieve(firstApiKey.id) + expect(apiKey.revoked_at).toBeFalsy() + expect(apiKey.revoked_by).toBeFalsy() + }) + it("should not allow revoking an already revoked API key", async function () { const firstApiKey = await service.create(createSecretKeyFixture) await service.revoke({ @@ -180,7 +190,6 @@ moduleIntegrationTestRunner({ // These should not be returned on an update createdApiKey.token = "" - createdApiKey.salt = "" expect(createdApiKey).toEqual(updatedApiKey) }) }) diff --git a/packages/api-key/src/migrations/.snapshot-medusa-api-key.json b/packages/api-key/src/migrations/.snapshot-medusa-api-key.json index 779d7b0f38..5f7a4ea5a3 100644 --- a/packages/api-key/src/migrations/.snapshot-medusa-api-key.json +++ b/packages/api-key/src/migrations/.snapshot-medusa-api-key.json @@ -113,6 +113,16 @@ "name": "api_key", "schema": "public", "indexes": [ + { + "keyName": "IDX_api_key_token_unique", + "columnNames": [ + "token" + ], + "composite": false, + "primary": false, + "unique": false, + "expression": "CREATE UNIQUE INDEX IF NOT EXISTS \"IDX_api_key_token_unique\" ON \"api_key\" (token)" + }, { "keyName": "IDX_api_key_type", "columnNames": [ diff --git a/packages/api-key/src/migrations/InitialSetup20240220155605.ts b/packages/api-key/src/migrations/InitialSetup20240221144943.ts similarity index 77% rename from packages/api-key/src/migrations/InitialSetup20240220155605.ts rename to packages/api-key/src/migrations/InitialSetup20240221144943.ts index 2b3798caaf..512b3d16ed 100644 --- a/packages/api-key/src/migrations/InitialSetup20240220155605.ts +++ b/packages/api-key/src/migrations/InitialSetup20240221144943.ts @@ -1,10 +1,13 @@ import { Migration } from "@mikro-orm/migrations" -export class InitialSetup20240220155605 extends Migration { +export class InitialSetup20240221144943 extends Migration { async up(): Promise { this.addSql( 'create table if not exists "api_key" ("id" text not null, "token" text not null, "salt" text not null, "redacted" text not null, "title" text not null, "type" text not null, "last_used_at" timestamptz null, "created_by" text not null, "created_at" timestamptz not null default now(), "revoked_by" text null, "revoked_at" timestamptz null, constraint "api_key_pkey" primary key ("id"));' ) + this.addSql( + 'CREATE UNIQUE INDEX IF NOT EXISTS "IDX_api_key_token_unique" ON "api_key" (token);' + ) this.addSql( 'CREATE INDEX IF NOT EXISTS "IDX_api_key_type" ON "api_key" (type);' ) diff --git a/packages/api-key/src/models/api-key.ts b/packages/api-key/src/models/api-key.ts index 239028a07a..28099fcaf9 100644 --- a/packages/api-key/src/models/api-key.ts +++ b/packages/api-key/src/models/api-key.ts @@ -17,12 +17,19 @@ const TypeIndex = createPsqlIndexStatementHelper({ columns: "type", }) +const TokenIndex = createPsqlIndexStatementHelper({ + tableName: "api_key", + columns: "token", + unique: true, +}) + @Entity() export default class ApiKey { @PrimaryKey({ columnType: "text" }) id: string @Property({ columnType: "text" }) + @TokenIndex.MikroORMIndex() token: string @Property({ columnType: "text" }) diff --git a/packages/api-key/src/services/api-key-module-service.ts b/packages/api-key/src/services/api-key-module-service.ts index 55d3f2020a..03a12ca059 100644 --- a/packages/api-key/src/services/api-key-module-service.ts +++ b/packages/api-key/src/services/api-key-module-service.ts @@ -89,6 +89,7 @@ export default class ApiKeyModuleService token: generatedTokens.find((t) => t.hashedToken === key.token)?.rawToken ?? key.token, + salt: undefined, })) return Array.isArray(data) ? responseWithRawToken : responseWithRawToken[0] @@ -99,7 +100,7 @@ export default class ApiKeyModuleService data: ApiKeyTypes.CreateApiKeyDTO[], @MedusaContext() sharedContext: Context = {} ): Promise<[TEntity[], TokenDTO[]]> { - await this.validateCreateApiKeys(data, sharedContext) + await this.validateCreateApiKeys_(data, sharedContext) const normalizedInput: CreateApiKeyDTO[] = [] const generatedTokens: TokenDTO[] = [] @@ -201,7 +202,7 @@ export default class ApiKeyModuleService sharedContext ) - return this.baseRepository_.serialize( + return await this.baseRepository_.serialize( apiKeys.map(omitToken), { populate: true, @@ -215,17 +216,15 @@ export default class ApiKeyModuleService config?: FindConfig, sharedContext?: Context ): Promise<[ApiKeyTypes.ApiKeyDTO[], number]> { - const result = await this.apiKeyService_.listAndCount( + const [apiKeys, count] = await this.apiKeyService_.listAndCount( filters, config, sharedContext ) - const withoutToken = result[0].map(omitToken) - const count = result[1] return [ await this.baseRepository_.serialize( - withoutToken, + apiKeys.map(omitToken), { populate: true, } @@ -267,7 +266,7 @@ export default class ApiKeyModuleService data: ApiKeyTypes.RevokeApiKeyDTO[], @MedusaContext() sharedContext: Context = {} ): Promise { - await this.validateRevokeApiKeys(data) + await this.validateRevokeApiKeys_(data) const updateRequest = data.map((k) => ({ id: k.id, @@ -292,7 +291,7 @@ export default class ApiKeyModuleService return Promise.resolve(false) } - protected async validateCreateApiKeys( + protected async validateCreateApiKeys_( data: ApiKeyTypes.CreateApiKeyDTO[], sharedContext: Context = {} ): Promise { @@ -327,7 +326,7 @@ export default class ApiKeyModuleService } } - protected async validateRevokeApiKeys( + protected async validateRevokeApiKeys_( data: ApiKeyTypes.RevokeApiKeyDTO[], sharedContext: Context = {} ): Promise { @@ -335,6 +334,13 @@ export default class ApiKeyModuleService return } + if (data.some((k) => !k.id)) { + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `You must provide an api key id field when revoking a key.` + ) + } + if (data.some((k) => !k.revoked_by)) { throw new MedusaError( MedusaError.Types.INVALID_DATA, @@ -387,9 +393,12 @@ export default class ApiKeyModuleService } // We are mutating the object here as what microORM relies on non-enumerable fields for serialization, among other things. -const omitToken = (key: ApiKey): ApiKey => { +const omitToken = ( + // We have to make salt optional before deleting it (and we do want it required in the DB) + key: Omit & { salt?: string } +): Omit => { key.token = key.type === ApiKeyType.SECRET ? "" : key.token - key.salt = "" + delete key.salt return key }