feat(api-key,js-sdk,dashboard): allow deleting api keys only once its revoked (#9118)

what:

- module only deletes api keys once its revoked
- disables ui elements


https://github.com/user-attachments/assets/437821ae-497e-4b59-b02c-4a6ff36e6a30

RESOLVES CC-106
RESOLVES CC-105
RESOLVES CC-104

integration-tests/http/__tests__/api-key/admin/api-key.spec.ts

@@ -1,4 +1,4 @@
-import { ApiKeyType, ContainerRegistrationKeys } from "@medusajs/utils"
+import { ApiKeyType } from "@medusajs/utils"
 import { medusaIntegrationTestRunner } from "medusa-test-utils"
 import {
   adminHeaders,
@@ -372,9 +372,7 @@ medusaIntegrationTestRunner({
       it("should detach sales channels from a publishable API key on delete", async () => {
         const salesChannelRes = await api.post(
           `/admin/sales-channels`,
-          {
+          { name: "Test Sales Channel" },
-            name: "Test Sales Channel",
-          },
           adminHeaders
         )
 
@@ -393,9 +391,7 @@ medusaIntegrationTestRunner({
 
         const keyWithChannelsRes = await api.post(
           `/admin/api-keys/${api_key.id}/sales-channels`,
-          {
+          { add: [sales_channel.id] },
-            add: [sales_channel.id],
-          },
           adminHeaders
         )
 
@@ -415,6 +411,21 @@ medusaIntegrationTestRunner({
           }),
         ])
 
+        const revoked = await api.post(
+          `/admin/api-keys/${api_key.id}/revoke`,
+          {},
+          adminHeaders
+        )
+
+        expect(revoked.status).toEqual(200)
+        expect(revoked.data.api_key).toEqual(
+          expect.objectContaining({
+            id: api_key.id,
+            revoked_by: expect.stringMatching(/^user_*/),
+          })
+        )
+        expect(revoked.data.api_key.revoked_at).toBeTruthy()
+
         await api.delete(`/admin/api-keys/${api_key.id}`, adminHeaders)
 
         const deletedApiKeys = await api.get(

integration-tests/http/__tests__/api-key/admin/publishable-key.spec.ts

@@ -129,6 +129,12 @@ medusaIntegrationTestRunner({
 
       describe("DELETE /admin/api-keys/:id", () => {
         it("delete a publishable key", async () => {
+          await api.post(
+            `/admin/api-keys/${pubKey1.id}/revoke`,
+            {},
+            adminHeaders
+          )
+
           const response1 = await api.delete(
             `/admin/api-keys/${pubKey1.id}`,
             adminHeaders

packages/admin/dashboard/src/routes/api-key-management/api-key-management-detail/components/api-key-general-section/api-key-general-section.tsx

@@ -12,7 +12,10 @@ import {
 } from "@medusajs/ui"
 import { useTranslation } from "react-i18next"
 import { useNavigate } from "react-router-dom"
-import { ActionMenu } from "../../../../../components/common/action-menu"
+import {
+  Action,
+  ActionMenu,
+} from "../../../../../components/common/action-menu"
 import { Skeleton } from "../../../../../components/common/skeleton"
 import { UserLink } from "../../../../../components/common/user-link"
 import {
@@ -97,11 +100,12 @@ export const ApiKeyGeneralSection = ({ apiKey }: ApiKeyGeneralSectionProps) => {
     })
   }
 
-  const dangerousActions = [
+  const dangerousActions: Action[] = [
     {
       icon: <Trash />,
       label: t("actions.delete"),
       onClick: handleDelete,
+      disabled: !apiKey.revoked_at,
     },
   ]
 
@@ -110,6 +114,7 @@ export const ApiKeyGeneralSection = ({ apiKey }: ApiKeyGeneralSectionProps) => {
       icon: <XCircle />,
       label: t("apiKeyManagement.actions.revoke"),
       onClick: handleRevoke,
+      disabled: !!apiKey.revoked_at,
     })
   }
 

packages/admin/dashboard/src/routes/api-key-management/api-key-management-list/components/api-key-management-list-table/api-key-row-actions.tsx

@@ -107,11 +107,13 @@ export const ApiKeyRowActions = ({
               icon: <XCircle />,
               label: t("apiKeyManagement.actions.revoke"),
               onClick: handleRevoke,
+              disabled: !!apiKey.revoked_at,
             },
             {
               icon: <Trash />,
               label: t("actions.delete"),
               onClick: handleDelete,
+              disabled: !apiKey.revoked_at,
             },
           ],
         },

packages/core/js-sdk/src/admin/api-key.ts

@@ -38,9 +38,9 @@ export class ApiKey {
 
   async revoke(id: string, headers?: ClientHeaders) {
     return await this.client.fetch<HttpTypes.AdminApiKeyResponse>(
-      `/admin/api-keys/${id}`,
+      `/admin/api-keys/${id}/revoke`,
       {
-        method: "DELETE",
+        method: "POST",
         headers,
       }
     )

packages/modules/api-key/integration-tests/__tests__/api-key-module-service.spec.ts

@@ -261,6 +261,12 @@ moduleIntegrationTestRunner<IApiKeyModuleService>({
             createPublishableKeyFixture,
             createSecretKeyFixture,
           ])
+
+          await service.revoke(
+            { id: [createdApiKeys[0].id, createdApiKeys[1].id] },
+            { revoked_by: "test_user" }
+          )
+
           await service.deleteApiKeys([
             createdApiKeys[0].id,
             createdApiKeys[1].id,
@@ -269,6 +275,25 @@ moduleIntegrationTestRunner<IApiKeyModuleService>({
           const apiKeysInDatabase = await service.listApiKeys()
           expect(apiKeysInDatabase).toHaveLength(0)
         })
+
+        it("should throw when trying to delete unrevoked api keys", async function () {
+          const createdApiKeys = await service.createApiKeys([
+            createPublishableKeyFixture,
+            createSecretKeyFixture,
+          ])
+
+          const error = await service
+            .deleteApiKeys([createdApiKeys[0].id, createdApiKeys[1].id])
+            .catch((e) => e)
+
+          expect(error.type).toEqual("not_allowed")
+          expect(error.message).toContain(
+            `Cannot delete api keys that are not revoked - `
+          )
+
+          const apiKeysInDatabase = await service.listApiKeys()
+          expect(apiKeysInDatabase).toHaveLength(2)
+        })
       })
 
       describe("authenticating with API keys", () => {

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

@@ -1,5 +1,3 @@
-import crypto from "crypto"
-import util from "util"
 import {
   ApiKeyTypes,
   Context,
@@ -11,6 +9,18 @@ import {
   ModuleJoinerConfig,
   ModulesSdkTypes,
 } from "@medusajs/types"
+import {
+  ApiKeyType,
+  InjectManager,
+  InjectTransactionManager,
+  isObject,
+  isPresent,
+  isString,
+  MedusaContext,
+  MedusaError,
+  MedusaService,
+  promiseAll,
+} from "@medusajs/utils"
 import { ApiKey } from "@models"
 import {
   CreateApiKeyDTO,
@@ -18,17 +28,8 @@ import {
   TokenDTO,
   UpdateApiKeyInput,
 } from "@types"
-import {
+import crypto from "crypto"
-  ApiKeyType,
+import util from "util"
-  InjectManager,
-  InjectTransactionManager,
-  isObject,
-  isString,
-  MedusaContext,
-  MedusaError,
-  MedusaService,
-  promiseAll,
-} from "@medusajs/utils"
 import { joinerConfig } from "../joiner-config"
 
 const scrypt = util.promisify(crypto.scrypt)
@@ -61,6 +62,40 @@ export class ApiKeyModuleService
     return joinerConfig
   }
 
+  @InjectTransactionManager()
+  // @ts-expect-error
+  async deleteApiKeys(
+    ids: string | string[],
+    @MedusaContext() sharedContext: Context = {}
+  ) {
+    const apiKeyIds = Array.isArray(ids) ? ids : [ids]
+
+    const unrevokedApiKeys = (
+      await this.apiKeyService_.list(
+        {
+          id: ids,
+          $or: [
+            { revoked_at: { $eq: null } },
+            { revoked_at: { $gt: new Date() } },
+          ],
+        },
+        { take: null, select: ["id"] },
+        sharedContext
+      )
+    ).map((apiKey) => apiKey.id)
+
+    if (isPresent(unrevokedApiKeys)) {
+      throw new MedusaError(
+        MedusaError.Types.NOT_ALLOWED,
+        `Cannot delete api keys that are not revoked - ${unrevokedApiKeys.join(
+          ", "
+        )}`
+      )
+    }
+
+    return await super.deleteApiKeys(apiKeyIds, sharedContext)
+  }
+
   //@ts-expect-error
   createApiKeys(
     data: ApiKeyTypes.CreateApiKeyDTO[],