fix(core): fix create org role API

Author: darcyYe

packages/core/src/routes/organization-role/index.openapi.json

@@ -46,7 +46,7 @@
             "description": "The organization role was created successfully."
           },
           "422": {
-            "description": "The organization role name is already in use."
+            "description": "The organization role name is already in use, or at least one of the IDs (organizationScopeIds or resourceScopeIds) provided is invalid. For example, the organization scope ID or resource scope ID does not exist."
           }
         }
       }

packages/core/src/routes/organization-role/index.ts

@@ -6,9 +6,10 @@ import {
   type OrganizationRoleKeys,
 } from '@logto/schemas';
 import { generateStandardId } from '@logto/shared';
-import { condArray } from '@silverhand/essentials';
+import { condArray, tryThat } from '@silverhand/essentials';
 import { z } from 'zod';
 
+import RequestError from '#src/errors/RequestError/index.js';
 import { buildManagementApiContext } from '#src/libraries/hook/utils.js';
 import koaGuard from '#src/middleware/koa-guard.js';
 import koaPagination from '#src/middleware/koa-pagination.js';
@@ -93,6 +94,28 @@ export default function organizationRoleRoutes<T extends ManagementApiRouter>(
         resourceScopeIds: z.array(z.string()).default([]),
       });
 
+  /** Helper function to handle scope insertion with error handling */
+  const insertScopesWithErrorHandling = async (
+    roleId: string,
+    insertFunction: () => Promise<unknown>,
+    errorCode:
+      | 'organization.roles.invalid_scope_ids'
+      | 'organization.roles.invalid_resource_scope_ids'
+  ) => {
+    await tryThat(
+      async () => insertFunction(),
+      (error) => {
+        void roles.deleteById(roleId);
+
+        throw new RequestError({
+          code: errorCode,
+          status: 422,
+          details: error instanceof Error ? error.message : String(error),
+        });
+      }
+    );
+  };
+
   router.post(
     '/',
     koaGuard({
@@ -104,23 +127,29 @@ export default function organizationRoleRoutes<T extends ManagementApiRouter>(
       const { organizationScopeIds, resourceScopeIds, ...data } = ctx.guard.body;
       const role = await roles.insert({ id: generateStandardId(), ...data });
 
-      if (organizationScopeIds.length > 0) {
-        await rolesScopes.insert(
-          ...organizationScopeIds.map((id) => ({
-            organizationRoleId: role.id,
-            organizationScopeId: id,
-          }))
-        );
-      }
+      await insertScopesWithErrorHandling(
+        role.id,
+        async () =>
+          rolesScopes.insert(
+            ...organizationScopeIds.map((id) => ({
+              organizationRoleId: role.id,
+              organizationScopeId: id,
+            }))
+          ),
+        'organization.roles.invalid_scope_ids'
+      );
 
-      if (resourceScopeIds.length > 0) {
-        await rolesResourceScopes.insert(
-          ...resourceScopeIds.map((id) => ({
-            organizationRoleId: role.id,
-            scopeId: id,
-          }))
-        );
-      }
+      await insertScopesWithErrorHandling(
+        role.id,
+        async () =>
+          rolesResourceScopes.insert(
+            ...resourceScopeIds.map((id) => ({
+              organizationRoleId: role.id,
+              scopeId: id,
+            }))
+          ),
+        'organization.roles.invalid_resource_scope_ids'
+      );
 
       ctx.body = role;
       ctx.status = 201;

packages/integration-tests/src/tests/api/organization/organization-role.test.ts

@@ -4,8 +4,11 @@ import { generateStandardId } from '@logto/shared';
 import { isKeyInObject, pick } from '@silverhand/essentials';
 import { HTTPError } from 'ky';
 
+import { createResource } from '#src/api/index.js';
+import { createScope } from '#src/api/scope.js';
 import { OrganizationRoleApiTest, OrganizationScopeApiTest } from '#src/helpers/organization.js';
 import { ScopeApiTest } from '#src/helpers/resource.js';
+import { generateScopeName } from '#src/utils.js';
 
 const randomId = () => generateStandardId(4);
 
@@ -179,6 +182,85 @@ describe('organization role APIs', () => {
       const response = await roleApi.delete('0').catch((error: unknown) => error);
       expect(response instanceof HTTPError && response.response.status).toBe(404);
     });
+
+    it('should fail when creating a role with invalid organization scope IDs', async () => {
+      const invalidScopeId = generateStandardId();
+      const response = await roleApi
+        .create({
+          name: 'test' + randomId(),
+          organizationScopeIds: [invalidScopeId],
+        })
+        .catch((error: unknown) => error);
+
+      assert(response instanceof HTTPError);
+      const body: unknown = await response.response.json();
+      expect(response.response.status).toBe(422);
+      expect(body).toMatchObject(
+        expect.objectContaining({
+          code: 'organization.roles.invalid_scope_ids',
+        })
+      );
+
+      const roles = await roleApi.getList();
+      expect(roles).toHaveLength(0);
+    });
+
+    it('should fail when creating a role with invalid resource scope IDs', async () => {
+      const invalidScopeId = generateStandardId();
+      const response = await roleApi
+        .create({
+          name: 'test' + randomId(),
+          resourceScopeIds: [invalidScopeId],
+        })
+        .catch((error: unknown) => error);
+
+      assert(response instanceof HTTPError);
+      const body: unknown = await response.response.json();
+      expect(response.response.status).toBe(422);
+      expect(body).toMatchObject(
+        expect.objectContaining({
+          code: 'organization.roles.invalid_resource_scope_ids',
+        })
+      );
+
+      const roles = await roleApi.getList();
+      expect(roles).toHaveLength(0);
+    });
+
+    it('should successfully create a role with scope IDs are provided', async () => {
+      const resource = await createResource();
+      const scopeName = generateScopeName();
+      const createdScope = await createScope(resource.id, scopeName);
+
+      const [scope1, scope2] = await Promise.all([
+        scopeApi.create({ name: 'test' + randomId() }),
+        scopeApi.create({ name: 'test' + randomId() }),
+      ]);
+      const createdRole = await roleApi.create({
+        name: 'test' + randomId(),
+        organizationScopeIds: [scope1.id, scope2.id],
+        resourceScopeIds: [createdScope.id],
+      });
+
+      expect(createdRole).toHaveProperty('id');
+      expect(createdRole).toHaveProperty('name');
+
+      const scopes = await roleApi.getScopes(createdRole.id);
+      expect(scopes).toContainEqual(
+        expect.objectContaining({
+          name: scope1.name,
+        })
+      );
+      expect(scopes).toContainEqual(
+        expect.objectContaining({
+          name: scope2.name,
+        })
+      );
+
+      const { resourceScopes } = await roleApi.get(createdRole.id);
+      expect(resourceScopes.length).toBe(1);
+      expect(resourceScopes[0]).toHaveProperty('name', scopeName);
+    });
   });
 
   describe('organization role - scope relations', () => {

packages/phrases/src/locales/en/errors/organization.ts

@@ -1,5 +1,9 @@
 const organizations = {
   require_membership: 'The user must be a member of the organization to proceed.',
+  roles: {
+    invalid_scope_ids: 'The organization scope IDs are not valid.',
+    invalid_resource_scope_ids: 'The resource scope IDs are not valid.',
+  },
 };
 
 export default Object.freeze(organizations);