From 07da7918e25aa927c82f0c22fc36ce4f82a1c7fd Mon Sep 17 00:00:00 2001 From: Gao Sun Date: Thu, 20 Jun 2024 14:16:55 +0800 Subject: [PATCH] feat(core): init organization app role apis --- .../core/src/queries/organization/index.ts | 5 + .../role-application-relations.ts | 64 +++++++++ .../organization/role-user-relations.ts | 16 +-- .../index.application-role-relations.ts | 126 ++++++++++++++++++ .../index.applications.openapi.json | 75 +++++++++++ .../core/src/routes/organization/index.ts | 7 +- .../organization/index.user-role-relations.ts | 41 +++--- .../integration-tests/src/api/organization.ts | 28 +++- .../organization-application.test.ts | 61 +++++++++ .../organization/organization-user.test.ts | 46 ++++++- ...organization-role-application-relations.ts | 34 +++++ ...rganization_role_application_relations.sql | 16 +++ 12 files changed, 486 insertions(+), 33 deletions(-) create mode 100644 packages/core/src/queries/organization/role-application-relations.ts create mode 100644 packages/core/src/routes/organization/index.application-role-relations.ts create mode 100644 packages/schemas/alterations/next-1718807616-organization-role-application-relations.ts create mode 100644 packages/schemas/tables/organization_role_application_relations.sql diff --git a/packages/core/src/queries/organization/index.ts b/packages/core/src/queries/organization/index.ts index baf8f3806..489ce8a46 100644 --- a/packages/core/src/queries/organization/index.ts +++ b/packages/core/src/queries/organization/index.ts @@ -33,6 +33,7 @@ import SchemaQueries from '#src/utils/SchemaQueries.js'; import { conditionalSql, convertToIdentifiers } from '#src/utils/sql.js'; import { EmailDomainQueries } from './email-domains.js'; +import { RoleApplicationRelationQueries } from './role-application-relations.js'; import { RoleUserRelationQueries } from './role-user-relations.js'; import { UserRelationQueries } from './user-relations.js'; @@ -283,6 +284,7 @@ export default class OrganizationQueries extends SchemaQueries< ), /** Queries for organization - user relations. */ users: new UserRelationQueries(this.pool), + // TODO: Rename to `usersRoles` /** Queries for organization - organization role - user relations. */ rolesUsers: new RoleUserRelationQueries(this.pool), /** Queries for organization - application relations. */ @@ -292,6 +294,9 @@ export default class OrganizationQueries extends SchemaQueries< Organizations, Applications ), + // TODO: Rename to `appsRoles` + /** Queries for organization - organization role - application relations. */ + rolesApps: new RoleApplicationRelationQueries(this.pool), invitationsRoles: new TwoRelationsQueries( this.pool, OrganizationInvitationRoleRelations.table, diff --git a/packages/core/src/queries/organization/role-application-relations.ts b/packages/core/src/queries/organization/role-application-relations.ts new file mode 100644 index 000000000..58eda8fbe --- /dev/null +++ b/packages/core/src/queries/organization/role-application-relations.ts @@ -0,0 +1,64 @@ +import { + Organizations, + OrganizationRoles, + Applications, + OrganizationRoleApplicationRelations, +} from '@logto/schemas'; +import { type CommonQueryMethods, sql } from '@silverhand/slonik'; + +import RelationQueries from '#src/utils/RelationQueries.js'; +import { convertToIdentifiers } from '#src/utils/sql.js'; + +export class RoleApplicationRelationQueries extends RelationQueries< + [typeof Organizations, typeof OrganizationRoles, typeof Applications] +> { + constructor(pool: CommonQueryMethods) { + super( + pool, + OrganizationRoleApplicationRelations.table, + Organizations, + OrganizationRoles, + Applications + ); + } + + /** Replace the roles of an application in an organization. */ + async replace(organizationId: string, applicationId: string, roleIds: readonly string[]) { + const applications = convertToIdentifiers(Applications); + const relations = convertToIdentifiers(OrganizationRoleApplicationRelations); + + return this.pool.transaction(async (transaction) => { + // Lock application + await transaction.query(sql` + select 1 + from ${applications.table} + where ${applications.fields.id} = ${applicationId} + for update + `); + + // Delete old relations + await transaction.query(sql` + delete from ${relations.table} + where ${relations.fields.organizationId} = ${organizationId} + and ${relations.fields.applicationId} = ${applicationId} + `); + + // Insert new relations + if (roleIds.length === 0) { + return; + } + + await transaction.query(sql` + insert into ${relations.table} ( + ${relations.fields.organizationId}, + ${relations.fields.applicationId}, + ${relations.fields.organizationRoleId} + ) + values ${sql.join( + roleIds.map((roleId) => sql`(${organizationId}, ${applicationId}, ${roleId})`), + sql`, ` + )} + `); + }); + } +} diff --git a/packages/core/src/queries/organization/role-user-relations.ts b/packages/core/src/queries/organization/role-user-relations.ts index 5710bd110..c3430fae4 100644 --- a/packages/core/src/queries/organization/role-user-relations.ts +++ b/packages/core/src/queries/organization/role-user-relations.ts @@ -83,7 +83,7 @@ export class RoleUserRelationQueries extends RelationQueries< return this.pool.transaction(async (transaction) => { // Lock user await transaction.query(sql` - select id + select 1 from ${users.table} where ${users.fields.id} = ${userId} for update @@ -92,8 +92,8 @@ export class RoleUserRelationQueries extends RelationQueries< // Delete old relations await transaction.query(sql` delete from ${relations.table} - where ${relations.fields.userId} = ${userId} - and ${relations.fields.organizationId} = ${organizationId} + where ${relations.fields.organizationId} = ${organizationId} + and ${relations.fields.userId} = ${userId} `); // Insert new relations @@ -103,14 +103,14 @@ export class RoleUserRelationQueries extends RelationQueries< await transaction.query(sql` insert into ${relations.table} ( - ${relations.fields.userId}, ${relations.fields.organizationId}, + ${relations.fields.userId}, ${relations.fields.organizationRoleId} ) - values ${sql.join( - roleIds.map((roleId) => sql`(${userId}, ${organizationId}, ${roleId})`), - sql`, ` - )} + values ${sql.join( + roleIds.map((roleId) => sql`(${organizationId}, ${userId}, ${roleId})`), + sql`, ` + )} `); }); } diff --git a/packages/core/src/routes/organization/index.application-role-relations.ts b/packages/core/src/routes/organization/index.application-role-relations.ts new file mode 100644 index 000000000..6de2a9090 --- /dev/null +++ b/packages/core/src/routes/organization/index.application-role-relations.ts @@ -0,0 +1,126 @@ +import { OrganizationRoles } from '@logto/schemas'; +import type Router from 'koa-router'; +import { z } from 'zod'; + +import RequestError from '#src/errors/RequestError/index.js'; +import koaGuard from '#src/middleware/koa-guard.js'; +import { type WithHookContext } from '#src/middleware/koa-management-api-hooks.js'; +import koaPagination from '#src/middleware/koa-pagination.js'; +import type OrganizationQueries from '#src/queries/organization/index.js'; + +// Consider building a class to handle these relations. See `index.user-role-relations.ts` for more information. +export default function applicationRoleRelationRoutes( + router: Router, + organizations: OrganizationQueries +) { + const params = Object.freeze({ + id: z.string().min(1), + applicationId: z.string().min(1), + } as const); + const pathname = '/:id/applications/:applicationId/roles'; + + // The pathname of `.use()` will not match the end of the path, for example: + // `.use('/foo', ...)` will match both `/foo` and `/foo/bar`. + // See https://github.com/koajs/router/blob/02ad6eedf5ced6ec1eab2138380fd67c63e3f1d7/lib/router.js#L330-L333 + router.use(pathname, koaGuard({ params: z.object(params) }), async (ctx, next) => { + const { id, applicationId } = ctx.guard.params; + + // Ensure membership + if (!(await organizations.relations.apps.exists({ organizationId: id, applicationId }))) { + throw new RequestError({ code: 'organization.require_membership', status: 422 }); + } + + return next(); + }); + + router.get( + pathname, + koaPagination(), + koaGuard({ + params: z.object(params), + response: OrganizationRoles.guard.array(), + status: [200, 422], + }), + async (ctx, next) => { + const { id, applicationId } = ctx.guard.params; + + const [totalCount, entities] = await organizations.relations.rolesApps.getEntities( + OrganizationRoles, + { + organizationId: id, + applicationId, + } + ); + + ctx.pagination.totalCount = totalCount; + ctx.body = entities; + return next(); + } + ); + + router.post( + pathname, + koaGuard({ + params: z.object(params), + body: z.object({ + organizationRoleIds: z.string().min(1).array().nonempty(), + }), + status: [201, 422], + }), + async (ctx, next) => { + const { id, applicationId } = ctx.guard.params; + const { organizationRoleIds } = ctx.guard.body; + + await organizations.relations.rolesApps.insert( + ...organizationRoleIds.map((organizationRoleId) => ({ + organizationId: id, + applicationId, + organizationRoleId, + })) + ); + + ctx.status = 201; + return next(); + } + ); + + router.put( + pathname, + koaGuard({ + params: z.object(params), + body: z.object({ + organizationRoleIds: z.string().min(1).array().nonempty(), + }), + status: [204, 422], + }), + async (ctx, next) => { + const { id, applicationId } = ctx.guard.params; + const { organizationRoleIds } = ctx.guard.body; + + await organizations.relations.rolesApps.replace(id, applicationId, organizationRoleIds); + + ctx.status = 204; + return next(); + } + ); + + router.delete( + `${pathname}/:organizationRoleId`, + koaGuard({ + params: z.object({ ...params, organizationRoleId: z.string().min(1) }), + status: [204, 422, 404], + }), + async (ctx, next) => { + const { id, applicationId, organizationRoleId } = ctx.guard.params; + + await organizations.relations.rolesApps.delete({ + organizationId: id, + applicationId, + organizationRoleId, + }); + + ctx.status = 204; + return next(); + } + ); +} diff --git a/packages/core/src/routes/organization/index.applications.openapi.json b/packages/core/src/routes/organization/index.applications.openapi.json index 672e766b1..afa802114 100644 --- a/packages/core/src/routes/organization/index.applications.openapi.json +++ b/packages/core/src/routes/organization/index.applications.openapi.json @@ -77,6 +77,81 @@ } } } + }, + "/api/organizations/{id}/applications/{applicationId}/roles": { + "get": { + "summary": "Get organization application roles", + "description": "Get roles associated with the application in the organization.", + "responses": { + "200": { + "description": "A list of roles." + } + } + }, + "post": { + "summary": "Add organization application role", + "description": "Add a role to the application in the organization.", + "requestBody": { + "content": { + "application/json": { + "schema": { + "properties": { + "organizationRoleIds": { + "description": "The role ID to add." + } + } + } + } + } + }, + "responses": { + "201": { + "description": "The role was added successfully." + }, + "422": { + "description": "The role could not be added. Some of the roles may not exist." + } + } + }, + "put": { + "summary": "Replace organization application roles", + "description": "Replace all roles associated with the application in the organization with the given data.", + "requestBody": { + "content": { + "application/json": { + "schema": { + "properties": { + "organizationRoleIds": { + "description": "An array of role IDs to replace existing roles." + } + } + } + } + } + }, + "responses": { + "204": { + "description": "The roles were replaced successfully." + }, + "422": { + "description": "The roles could not be replaced. Some of the roles may not exist." + } + } + } + }, + "/api/organizations/{id}/applications/{applicationId}/roles/{organizationRoleId}": { + "delete": { + "summary": "Remove organization application role", + "description": "Remove a role from the application in the organization.", + "responses": { + "204": { + "description": "The role was removed from the application in the organization successfully." + }, + "422": { + "description": "The role could not be removed. The role may not exist." + } + } + } } } } diff --git a/packages/core/src/routes/organization/index.ts b/packages/core/src/routes/organization/index.ts index 96339b49d..47d2d24c1 100644 --- a/packages/core/src/routes/organization/index.ts +++ b/packages/core/src/routes/organization/index.ts @@ -17,6 +17,7 @@ import { parseSearchOptions } from '#src/utils/search.js'; import { type ManagementApiRouter, type RouterInitArgs } from '../types.js'; +import applicationRoleRelationRoutes from './index.application-role-relations.js'; import emailDomainRoutes from './index.jit.email-domains.js'; import userRoleRelationRoutes from './index.user-role-relations.js'; import organizationInvitationRoutes from './invitations.js'; @@ -113,6 +114,7 @@ export default function organizationRoutes( } ); + // MARK: Organization - user role relation routes router.post( '/:id/users/roles', koaGuard({ @@ -140,11 +142,14 @@ export default function organizationRoutes( userRoleRelationRoutes(router, organizations); - // MARK: Organization - application relation routes if (EnvSet.values.isDevFeaturesEnabled) { + // MARK: Organization - application relation routes router.addRelationRoutes(organizations.relations.apps, undefined, { hookEvent: 'Organization.Membership.Updated', }); + + // MARK: Organization - application role relation routes + applicationRoleRelationRoutes(router, organizations); } // MARK: Just-in-time provisioning diff --git a/packages/core/src/routes/organization/index.user-role-relations.ts b/packages/core/src/routes/organization/index.user-role-relations.ts index 1f7b4e105..2afa1989e 100644 --- a/packages/core/src/routes/organization/index.user-role-relations.ts +++ b/packages/core/src/routes/organization/index.user-role-relations.ts @@ -8,12 +8,13 @@ import { type WithHookContext } from '#src/middleware/koa-management-api-hooks.j import koaPagination from '#src/middleware/koa-pagination.js'; import type OrganizationQueries from '#src/queries/organization/index.js'; -// Manually add these routes since I don't want to over-engineer the `SchemaRouter` +// Manually add these routes since I don't want to over-engineer the `SchemaRouter`. +// Update: Now we also have "organization - organization role - application" relations. Consider +// extracting the common logic to a class once we have one more relation like this. export default function userRoleRelationRoutes( router: Router, organizations: OrganizationQueries ) { - // MARK: Organization - user - organization role relation routes const params = Object.freeze({ id: z.string().min(1), userId: z.string().min(1) } as const); const pathname = '/:id/users/:userId/roles'; @@ -57,24 +58,6 @@ export default function userRoleRelationRoutes( } ); - router.put( - pathname, - koaGuard({ - params: z.object(params), - body: z.object({ organizationRoleIds: z.string().min(1).array() }), - status: [204, 422], - }), - async (ctx, next) => { - const { id, userId } = ctx.guard.params; - const { organizationRoleIds } = ctx.guard.body; - - await organizations.relations.rolesUsers.replace(id, userId, organizationRoleIds); - - ctx.status = 204; - return next(); - } - ); - router.post( pathname, koaGuard({ @@ -99,6 +82,24 @@ export default function userRoleRelationRoutes( } ); + router.put( + pathname, + koaGuard({ + params: z.object(params), + body: z.object({ organizationRoleIds: z.string().min(1).array() }), + status: [204, 422], + }), + async (ctx, next) => { + const { id, userId } = ctx.guard.params; + const { organizationRoleIds } = ctx.guard.body; + + await organizations.relations.rolesUsers.replace(id, userId, organizationRoleIds); + + ctx.status = 204; + return next(); + } + ); + router.delete( `${pathname}/:roleId`, koaGuard({ diff --git a/packages/integration-tests/src/api/organization.ts b/packages/integration-tests/src/api/organization.ts index e5cb2e0f1..87baf87cb 100644 --- a/packages/integration-tests/src/api/organization.ts +++ b/packages/integration-tests/src/api/organization.ts @@ -1,5 +1,4 @@ import { - type Role, type Organization, type OrganizationWithRoles, type UserWithOrganizationRoles, @@ -7,6 +6,7 @@ import { type OrganizationScope, type CreateOrganization, type Application, + type OrganizationRole, } from '@logto/schemas'; import { authedAdminApi } from './api.js'; @@ -68,8 +68,10 @@ export class OrganizationApi extends ApiFactory { - return authedAdminApi.get(`${this.path}/${id}/users/${userId}/roles`).json(); + async getUserRoles(id: string, userId: string): Promise { + return authedAdminApi + .get(`${this.path}/${id}/users/${userId}/roles`) + .json(); } async deleteUserRole(id: string, userId: string, roleId: string): Promise { @@ -85,4 +87,24 @@ export class OrganizationApi extends ApiFactory(); } + + async addApplicationRoles( + id: string, + applicationId: string, + organizationRoleIds: string[] + ): Promise { + await authedAdminApi.post(`${this.path}/${id}/applications/${applicationId}/roles`, { + json: { organizationRoleIds }, + }); + } + + async getApplicationRoles(id: string, applicationId: string): Promise { + return authedAdminApi + .get(`${this.path}/${id}/applications/${applicationId}/roles`) + .json(); + } + + async deleteApplicationRole(id: string, applicationId: string, roleId: string): Promise { + await authedAdminApi.delete(`${this.path}/${id}/applications/${applicationId}/roles/${roleId}`); + } } diff --git a/packages/integration-tests/src/tests/api/organization/organization-application.test.ts b/packages/integration-tests/src/tests/api/organization/organization-application.test.ts index 0edecff25..c231e1f91 100644 --- a/packages/integration-tests/src/tests/api/organization/organization-application.test.ts +++ b/packages/integration-tests/src/tests/api/organization/organization-application.test.ts @@ -85,4 +85,65 @@ devFeatureTest.describe('organization application APIs', () => { expect(response instanceof HTTPError && response.response.status).toBe(422); }); }); + + describe('organization - application - organization role relations', () => { + const organizationApi = new OrganizationApiTest(); + const applications: Application[] = []; + const createApplication = async (...args: Parameters) => { + const created = await createApplicationApi(...args); + // eslint-disable-next-line @silverhand/fp/no-mutating-methods + applications.push(created); + return created; + }; + + afterEach(async () => { + await Promise.all([ + organizationApi.cleanUp(), + // eslint-disable-next-line @typescript-eslint/no-empty-function + ...applications.map(async ({ id }) => deleteApplication(id).catch(() => {})), + ]); + }); + + it('should fail when try to add application to an organization role that does not exist', async () => { + const organization = await organizationApi.create({ name: 'test' }); + const application = await createApplication( + generateTestName(), + ApplicationType.MachineToMachine + ); + + const response = await organizationApi + .addApplicationRoles(organization.id, '0', [application.id]) + .catch((error: unknown) => error); + assert(response instanceof HTTPError); + expect(response.response.status).toBe(422); + }); + + it('should be able to add and delete organization application role', async () => { + const organization = await organizationApi.create({ name: 'test' }); + const role = await organizationApi.roleApi.create({ name: `test-${generateTestName()}` }); + const application = await createApplication( + generateTestName(), + ApplicationType.MachineToMachine + ); + + await organizationApi.applications.add(organization.id, [application.id]); + await organizationApi.addApplicationRoles(organization.id, application.id, [role.id]); + expect( + await organizationApi.getApplicationRoles(organization.id, application.id) + ).toContainEqual(role); + + await organizationApi.deleteApplicationRole(organization.id, application.id, role.id); + expect( + await organizationApi.getApplicationRoles(organization.id, application.id) + ).not.toContainEqual(role); + }); + + it('should fail when try to delete application role from an organization that does not exist', async () => { + const response = await organizationApi + .deleteApplicationRole('0', '0', '0') + .catch((error: unknown) => error); + assert(response instanceof HTTPError); + expect(response.response.status).toBe(422); + }); + }); }); diff --git a/packages/integration-tests/src/tests/api/organization/organization-user.test.ts b/packages/integration-tests/src/tests/api/organization/organization-user.test.ts index ffaf95989..3154f487d 100644 --- a/packages/integration-tests/src/tests/api/organization/organization-user.test.ts +++ b/packages/integration-tests/src/tests/api/organization/organization-user.test.ts @@ -124,7 +124,7 @@ describe('organization user APIs', () => { }); }); - describe('organization - user - organization role relation routes', () => { + describe('organization - user - organization role relations', () => { const organizationApi = new OrganizationApiTest(); const { roleApi } = organizationApi; const userApi = new UserApiTest(); @@ -246,6 +246,50 @@ describe('organization user APIs', () => { .catch((error: unknown) => error); expect(response instanceof HTTPError && response.response.status).toBe(422); // Require membership }); + + it('should fail when try to add or delete role to a user that does not exist', async () => { + const organization = await organizationApi.create({ name: 'test' }); + const response = await organizationApi + .addUserRoles(organization.id, '0', ['0']) + .catch((error: unknown) => error); + assert(response instanceof HTTPError); + expect(response.response.status).toBe(422); + expect(await response.response.json()).toMatchObject( + expect.objectContaining({ code: 'organization.require_membership' }) + ); + + const response2 = await organizationApi + .deleteUserRole(organization.id, '0', '0') + .catch((error: unknown) => error); + assert(response2 instanceof HTTPError); + expect(response2.response.status).toBe(422); + expect(await response2.response.json()).toMatchObject( + expect.objectContaining({ code: 'organization.require_membership' }) + ); + }); + + it('should fail when try to add or delete role that does not exist', async () => { + const organization = await organizationApi.create({ name: 'test' }); + const user = await userApi.create({ username: generateTestName() }); + await organizationApi.addUsers(organization.id, [user.id]); + const response = await organizationApi + .addUserRoles(organization.id, user.id, ['0']) + .catch((error: unknown) => error); + assert(response instanceof HTTPError); + expect(response.response.status).toBe(422); + expect(await response.response.json()).toMatchObject( + expect.objectContaining({ code: 'entity.relation_foreign_key_not_found' }) + ); + + const response2 = await organizationApi + .deleteUserRole(organization.id, user.id, '0') + .catch((error: unknown) => error); + assert(response2 instanceof HTTPError); + expect(response2.response.status).toBe(404); + expect(await response2.response.json()).toMatchObject( + expect.objectContaining({ code: 'entity.not_found' }) + ); + }); }); describe('organization - user - organization role - organization scopes relation', () => { diff --git a/packages/schemas/alterations/next-1718807616-organization-role-application-relations.ts b/packages/schemas/alterations/next-1718807616-organization-role-application-relations.ts new file mode 100644 index 000000000..fcb7b6730 --- /dev/null +++ b/packages/schemas/alterations/next-1718807616-organization-role-application-relations.ts @@ -0,0 +1,34 @@ +import { sql } from '@silverhand/slonik'; + +import type { AlterationScript } from '../lib/types/alteration.js'; + +import { applyTableRls, dropTableRls } from './utils/1704934999-tables.js'; + +const alteration: AlterationScript = { + up: async (pool) => { + await pool.query(sql` + create table organization_role_application_relations ( + tenant_id varchar(21) not null + references tenants (id) on update cascade on delete cascade, + organization_id varchar(21) not null, + organization_role_id varchar(21) not null + references organization_roles (id) on update cascade on delete cascade, + application_id varchar(21) not null, + primary key (tenant_id, organization_id, organization_role_id, application_id), + /** Application's roles in an organization should be synchronized with the application's membership in the organization. */ + foreign key (tenant_id, organization_id, application_id) + references organization_application_relations (tenant_id, organization_id, application_id) + on update cascade on delete cascade + ); + `); + await applyTableRls(pool, 'organization_role_application_relations'); + }, + down: async (pool) => { + await dropTableRls(pool, 'organization_role_application_relations'); + await pool.query(sql` + drop table organization_role_application_relations; + `); + }, +}; + +export default alteration; diff --git a/packages/schemas/tables/organization_role_application_relations.sql b/packages/schemas/tables/organization_role_application_relations.sql new file mode 100644 index 000000000..05f27a52a --- /dev/null +++ b/packages/schemas/tables/organization_role_application_relations.sql @@ -0,0 +1,16 @@ +/* init_order = 3 */ + +/** The relations between organizations, organization roles, and applications. A relation means that an application has a role in an organization. */ +create table organization_role_application_relations ( + tenant_id varchar(21) not null + references tenants (id) on update cascade on delete cascade, + organization_id varchar(21) not null, + organization_role_id varchar(21) not null + references organization_roles (id) on update cascade on delete cascade, + application_id varchar(21) not null, + primary key (tenant_id, organization_id, organization_role_id, application_id), + /** Application's roles in an organization should be synchronized with the application's membership in the organization. */ + foreign key (tenant_id, organization_id, application_id) + references organization_application_relations (tenant_id, organization_id, application_id) + on update cascade on delete cascade +);