chore: Updates based on PR feedback

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)
         })
       })

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": [

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<void> {
     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);'
     )

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" })

packages/api-key/src/services/api-key-module-service.ts

@@ -89,6 +89,7 @@ export default class ApiKeyModuleService<TEntity extends ApiKey = ApiKey>
       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<TEntity extends ApiKey = ApiKey>
     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<TEntity extends ApiKey = ApiKey>
       sharedContext
     )
 
-    return this.baseRepository_.serialize<ApiKeyTypes.ApiKeyDTO[]>(
+    return await this.baseRepository_.serialize<ApiKeyTypes.ApiKeyDTO[]>(
       apiKeys.map(omitToken),
       {
         populate: true,
@@ -215,17 +216,15 @@ export default class ApiKeyModuleService<TEntity extends ApiKey = ApiKey>
     config?: FindConfig<ApiKeyTypes.ApiKeyDTO>,
     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<ApiKeyTypes.ApiKeyDTO[]>(
-        withoutToken,
+        apiKeys.map(omitToken),
         {
           populate: true,
         }
@@ -267,7 +266,7 @@ export default class ApiKeyModuleService<TEntity extends ApiKey = ApiKey>
     data: ApiKeyTypes.RevokeApiKeyDTO[],
     @MedusaContext() sharedContext: Context = {}
   ): Promise<TEntity[]> {
-    await this.validateRevokeApiKeys(data)
+    await this.validateRevokeApiKeys_(data)
 
     const updateRequest = data.map((k) => ({
       id: k.id,
@@ -292,7 +291,7 @@ export default class ApiKeyModuleService<TEntity extends ApiKey = ApiKey>
     return Promise.resolve(false)
   }
 
-  protected async validateCreateApiKeys(
+  protected async validateCreateApiKeys_(
     data: ApiKeyTypes.CreateApiKeyDTO[],
     sharedContext: Context = {}
   ): Promise<void> {
@@ -327,7 +326,7 @@ export default class ApiKeyModuleService<TEntity extends ApiKey = ApiKey>
     }
   }
 
-  protected async validateRevokeApiKeys(
+  protected async validateRevokeApiKeys_(
     data: ApiKeyTypes.RevokeApiKeyDTO[],
     sharedContext: Context = {}
   ): Promise<void> {
@@ -335,6 +334,13 @@ export default class ApiKeyModuleService<TEntity extends ApiKey = ApiKey>
       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<TEntity extends ApiKey = ApiKey>
 }
 
 // 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<ApiKey, "salt"> & { salt?: string }
+): Omit<ApiKey, "salt"> => {
   key.token = key.type === ApiKeyType.SECRET ? "" : key.token
-  key.salt = ""
+  delete key.salt
   return key
 }