From b25bca3aa2e2b20a11376a3ba941bc486edef6d0 Mon Sep 17 00:00:00 2001 From: Gao Sun Date: Fri, 14 Jun 2024 22:44:03 +0800 Subject: [PATCH] refactor: add organization jit role api tests --- .../src/middleware/koa-pagination.test.ts | 2 +- .../core/src/middleware/koa-pagination.ts | 47 +++-- .../organization/index.jit.roles.openapi.json | 12 +- .../core/src/routes/organization/index.ts | 2 +- .../core/src/routes/sso-connector/index.ts | 5 +- packages/core/src/utils/SchemaRouter.ts | 15 +- .../src/api/organization-jit.ts | 55 ++++++ .../integration-tests/src/api/organization.ts | 36 +--- .../api/admin-user.organization-jit.test.ts | 2 +- .../api/hook/hook.trigger.custom.data.test.ts | 4 +- .../api/interaction/organization-jit.test.ts | 2 +- .../organization-email-domains.test.ts | 80 -------- .../api/organization/organization-jit.test.ts | 179 ++++++++++++++++++ 13 files changed, 300 insertions(+), 141 deletions(-) create mode 100644 packages/integration-tests/src/api/organization-jit.ts delete mode 100644 packages/integration-tests/src/tests/api/organization/organization-email-domains.test.ts create mode 100644 packages/integration-tests/src/tests/api/organization/organization-jit.test.ts diff --git a/packages/core/src/middleware/koa-pagination.test.ts b/packages/core/src/middleware/koa-pagination.test.ts index 3f7da06ad22..dc553366336 100644 --- a/packages/core/src/middleware/koa-pagination.test.ts +++ b/packages/core/src/middleware/koa-pagination.test.ts @@ -16,7 +16,7 @@ const appendHeader = jest.fn((key: string, value: string) => { } }); -const createContext = (query: Record): WithPaginationContext => { +const createContext = (query: Record): WithPaginationContext => { const baseContext = createMockContext(); const context = { ...baseContext, diff --git a/packages/core/src/middleware/koa-pagination.ts b/packages/core/src/middleware/koa-pagination.ts index 15bc66b15cf..a22e2f46445 100644 --- a/packages/core/src/middleware/koa-pagination.ts +++ b/packages/core/src/middleware/koa-pagination.ts @@ -9,37 +9,58 @@ type Pagination = { offset: number; limit: number; totalCount?: number; - disabled?: boolean; + disabled?: false; }; -export type WithPaginationContext = ContextT & { - pagination: Pagination; +type DisabledPagination = { + offset: undefined; + limit: undefined; + totalCount: undefined; + disabled: true; }; -type PaginationConfig = { +export type WithPaginationContext = ContextT & { + pagination: IsOptional extends true ? Pagination | DisabledPagination : Pagination; +}; + +type PaginationConfig = { defaultPageSize?: number; maxPageSize?: number; - isOptional?: boolean; + isOptional?: IsOptional; }; export const isPaginationMiddleware = ( function_: Type -): function_ is WithPaginationContext => function_.name === 'paginationMiddleware'; +): function_ is WithPaginationContext => function_.name === 'paginationMiddleware'; export const fallbackDefaultPageSize = 20; export const pageNumberKey = 'page'; export const pageSizeKey = 'page_size'; -export default function koaPagination({ +function koaPagination( + config?: PaginationConfig +): MiddlewareType, ResponseBodyT>; +function koaPagination( + config: PaginationConfig +): MiddlewareType, ResponseBodyT>; +function koaPagination( + config?: PaginationConfig +): MiddlewareType, ResponseBodyT>; +function koaPagination({ defaultPageSize = fallbackDefaultPageSize, maxPageSize = 100, - isOptional = false, -}: PaginationConfig = {}): MiddlewareType, ResponseBodyT> { + isOptional, +}: PaginationConfig = {}): MiddlewareType< + StateT, + WithPaginationContext, + ResponseBodyT +> { // Name this anonymous function for the utility function `isPaginationMiddleware` to identify it const paginationMiddleware: MiddlewareType< StateT, - WithPaginationContext, + WithPaginationContext, ResponseBodyT + // eslint-disable-next-line complexity -- maybe refactor me > = async (ctx, next) => { try { const { @@ -56,7 +77,9 @@ export default function koaPagination({ ? number().positive().max(maxPageSize).parse(Number(rawPageSize)) : defaultPageSize; - ctx.pagination = { offset: (pageNumber - 1) * pageSize, limit: pageSize, disabled }; + ctx.pagination = disabled + ? { disabled, totalCount: undefined, offset: undefined, limit: undefined } + : { disabled, totalCount: undefined, offset: (pageNumber - 1) * pageSize, limit: pageSize }; } catch { throw new RequestError({ code: 'guard.invalid_pagination', status: 400 }); } @@ -94,3 +117,5 @@ export default function koaPagination({ return paginationMiddleware; } + +export default koaPagination; diff --git a/packages/core/src/routes/organization/index.jit.roles.openapi.json b/packages/core/src/routes/organization/index.jit.roles.openapi.json index bbd4333dc05..cbd2dfaae65 100644 --- a/packages/core/src/routes/organization/index.jit.roles.openapi.json +++ b/packages/core/src/routes/organization/index.jit.roles.openapi.json @@ -16,15 +16,15 @@ } }, "post": { - "summary": "Add organization JIT role", - "description": "Add a new organization role that will be assigned to users during just-in-time provisioning.", + "summary": "Add organization JIT roles", + "description": "Add new organization roles that will be assigned to users during just-in-time provisioning.", "requestBody": { "content": { "application/json": { "schema": { "properties": { - "organizationRoleId": { - "description": "The organization role ID to add." + "organizationRoleIds": { + "description": "The organization role IDs to add." } } } @@ -33,10 +33,10 @@ }, "responses": { "201": { - "description": "The organization role was added successfully." + "description": "The organization roles were added successfully." }, "422": { - "description": "The organization role is already in use." + "description": "The organization roles could not be added. Some of the organization roles may not exist." } } }, diff --git a/packages/core/src/routes/organization/index.ts b/packages/core/src/routes/organization/index.ts index c7b37a0f09a..c34560ed458 100644 --- a/packages/core/src/routes/organization/index.ts +++ b/packages/core/src/routes/organization/index.ts @@ -141,7 +141,7 @@ export default function organizationRoutes( // MARK: Just-in-time provisioning emailDomainRoutes(router, organizations); - router.addRelationRoutes(organizations.jit.roles, 'jit/roles'); + router.addRelationRoutes(organizations.jit.roles, 'jit/roles', { isPaginationOptional: true }); // MARK: Mount sub-routes organizationRoleRoutes(...args); diff --git a/packages/core/src/routes/sso-connector/index.ts b/packages/core/src/routes/sso-connector/index.ts index 7a94439a16e..db95ed9026d 100644 --- a/packages/core/src/routes/sso-connector/index.ts +++ b/packages/core/src/routes/sso-connector/index.ts @@ -143,7 +143,10 @@ export default function singleSignOnConnectorsRoutes >, pathname = tableToPathname(relationQueries.schemas[1].table), - { disabled, hookEvent }: Partial = {} + { disabled, hookEvent, isPaginationOptional }: Partial = {} ) { const relationSchema = relationQueries.schemas[1]; const relationSchemaId = camelCaseSchemaId(relationSchema); @@ -205,7 +210,7 @@ export default class SchemaRouter< if (!disabled?.get) { this.get( `/:id/${pathname}`, - koaPagination(), + koaPagination({ isOptional: isPaginationOptional }), koaGuard({ params: z.object({ id: z.string().min(1) }), response: relationSchema.guard.array(), @@ -220,10 +225,12 @@ export default class SchemaRouter< const [totalCount, entities] = await relationQueries.getEntities( relationSchema, { [columns.schemaId]: id }, - ctx.pagination + ctx.pagination.disabled ? undefined : ctx.pagination ); - ctx.pagination.totalCount = totalCount; + if (!ctx.pagination.disabled) { + ctx.pagination.totalCount = totalCount; + } ctx.body = entities; return next(); } diff --git a/packages/integration-tests/src/api/organization-jit.ts b/packages/integration-tests/src/api/organization-jit.ts new file mode 100644 index 00000000000..6d2e9fdfef7 --- /dev/null +++ b/packages/integration-tests/src/api/organization-jit.ts @@ -0,0 +1,55 @@ +import { type OrganizationRole, type OrganizationJitEmailDomain } from '@logto/schemas'; + +import { authedAdminApi } from './api.js'; + +export class OrganizationJitApi { + constructor(public path: string) {} + + async getEmailDomains( + id: string, + page?: number, + pageSize?: number + ): Promise { + const searchParams = new URLSearchParams(); + + if (page) { + searchParams.append('page', String(page)); + } + + if (pageSize) { + searchParams.append('page_size', String(pageSize)); + } + + return authedAdminApi + .get(`${this.path}/${id}/jit/email-domains`, { searchParams }) + .json(); + } + + async addEmailDomain(id: string, emailDomain: string): Promise { + await authedAdminApi.post(`${this.path}/${id}/jit/email-domains`, { json: { emailDomain } }); + } + + async deleteEmailDomain(id: string, emailDomain: string): Promise { + await authedAdminApi.delete(`${this.path}/${id}/jit/email-domains/${emailDomain}`); + } + + async replaceEmailDomains(id: string, emailDomains: string[]): Promise { + await authedAdminApi.put(`${this.path}/${id}/jit/email-domains`, { json: { emailDomains } }); + } + + async getRoles(id: string): Promise { + return authedAdminApi.get(`${this.path}/${id}/jit/roles`).json(); + } + + async addRole(id: string, organizationRoleIds: string[]): Promise { + await authedAdminApi.post(`${this.path}/${id}/jit/roles`, { json: { organizationRoleIds } }); + } + + async deleteRole(id: string, organizationRoleId: string): Promise { + await authedAdminApi.delete(`${this.path}/${id}/jit/roles/${organizationRoleId}`); + } + + async replaceRoles(id: string, organizationRoleIds: string[]): Promise { + await authedAdminApi.put(`${this.path}/${id}/jit/roles`, { json: { organizationRoleIds } }); + } +} diff --git a/packages/integration-tests/src/api/organization.ts b/packages/integration-tests/src/api/organization.ts index 49551f5fc64..98bcac3b643 100644 --- a/packages/integration-tests/src/api/organization.ts +++ b/packages/integration-tests/src/api/organization.ts @@ -5,12 +5,12 @@ import { type UserWithOrganizationRoles, type OrganizationWithFeatured, type OrganizationScope, - type OrganizationJitEmailDomain, type CreateOrganization, } from '@logto/schemas'; import { authedAdminApi } from './api.js'; import { ApiFactory } from './factory.js'; +import { OrganizationJitApi } from './organization-jit.js'; type Query = { q?: string; @@ -19,6 +19,8 @@ type Query = { }; export class OrganizationApi extends ApiFactory> { + jit = new OrganizationJitApi(this.path); + constructor() { super('organizations'); } @@ -77,36 +79,4 @@ export class OrganizationApi extends ApiFactory(); } - - async getEmailDomains( - id: string, - page?: number, - pageSize?: number - ): Promise { - const searchParams = new URLSearchParams(); - - if (page) { - searchParams.append('page', String(page)); - } - - if (pageSize) { - searchParams.append('page_size', String(pageSize)); - } - - return authedAdminApi - .get(`${this.path}/${id}/jit/email-domains`, { searchParams }) - .json(); - } - - async addEmailDomain(id: string, emailDomain: string): Promise { - await authedAdminApi.post(`${this.path}/${id}/jit/email-domains`, { json: { emailDomain } }); - } - - async deleteEmailDomain(id: string, emailDomain: string): Promise { - await authedAdminApi.delete(`${this.path}/${id}/jit/email-domains/${emailDomain}`); - } - - async replaceEmailDomains(id: string, emailDomains: string[]): Promise { - await authedAdminApi.put(`${this.path}/${id}/jit/email-domains`, { json: { emailDomains } }); - } } diff --git a/packages/integration-tests/src/tests/api/admin-user.organization-jit.test.ts b/packages/integration-tests/src/tests/api/admin-user.organization-jit.test.ts index dbbcf84eabe..bdb6f489eed 100644 --- a/packages/integration-tests/src/tests/api/admin-user.organization-jit.test.ts +++ b/packages/integration-tests/src/tests/api/admin-user.organization-jit.test.ts @@ -19,7 +19,7 @@ describe('organization just-in-time provisioning', () => { const emailDomain = 'foo.com'; await Promise.all( organizations.map(async (organization) => - organizationApi.addEmailDomain(organization.id, emailDomain) + organizationApi.jit.addEmailDomain(organization.id, emailDomain) ) ); diff --git a/packages/integration-tests/src/tests/api/hook/hook.trigger.custom.data.test.ts b/packages/integration-tests/src/tests/api/hook/hook.trigger.custom.data.test.ts index b64c83aba06..6c0a5fa348f 100644 --- a/packages/integration-tests/src/tests/api/hook/hook.trigger.custom.data.test.ts +++ b/packages/integration-tests/src/tests/api/hook/hook.trigger.custom.data.test.ts @@ -123,7 +123,7 @@ describe('manual data hook tests', () => { it('should trigger `Organization.Membership.Updated` event when user is provisioned by Management API', async () => { const organization = await organizationApi.create({ name: 'foo' }); const domain = 'example.com'; - await organizationApi.addEmailDomain(organization.id, domain); + await organizationApi.jit.addEmailDomain(organization.id, domain); await userApi.create({ primaryEmail: `${randomString()}@${domain}` }); await assertOrganizationMembershipUpdated(organization.id); @@ -142,7 +142,7 @@ describe('manual data hook tests', () => { const organization = await organizationApi.create({ name: 'foo' }); const domain = 'example.com'; - await organizationApi.addEmailDomain(organization.id, domain); + await organizationApi.jit.addEmailDomain(organization.id, domain); await registerWithEmail(`${randomString()}@${domain}`); await assertOrganizationMembershipUpdated(organization.id); diff --git a/packages/integration-tests/src/tests/api/interaction/organization-jit.test.ts b/packages/integration-tests/src/tests/api/interaction/organization-jit.test.ts index cbedfd327a1..af9eb5f2442 100644 --- a/packages/integration-tests/src/tests/api/interaction/organization-jit.test.ts +++ b/packages/integration-tests/src/tests/api/interaction/organization-jit.test.ts @@ -48,7 +48,7 @@ describe('organization just-in-time provisioning', () => { const emailDomain = 'foo.com'; await Promise.all( organizations.map(async (organization) => - organizationApi.addEmailDomain(organization.id, emailDomain) + organizationApi.jit.addEmailDomain(organization.id, emailDomain) ) ); diff --git a/packages/integration-tests/src/tests/api/organization/organization-email-domains.test.ts b/packages/integration-tests/src/tests/api/organization/organization-email-domains.test.ts deleted file mode 100644 index ba467181a77..00000000000 --- a/packages/integration-tests/src/tests/api/organization/organization-email-domains.test.ts +++ /dev/null @@ -1,80 +0,0 @@ -import { generateStandardId } from '@logto/shared'; - -import { OrganizationApiTest } from '#src/helpers/organization.js'; - -const randomId = () => generateStandardId(6); - -describe('organization email domains', () => { - const organizationApi = new OrganizationApiTest(); - - afterEach(async () => { - await organizationApi.cleanUp(); - }); - - it('should add and delete email domains', async () => { - const organization = await organizationApi.create({ name: 'foo' }); - const emailDomain = `${randomId()}.com`; - - await organizationApi.addEmailDomain(organization.id, emailDomain); - await expect(organizationApi.getEmailDomains(organization.id)).resolves.toMatchObject([ - { emailDomain }, - ]); - - await organizationApi.deleteEmailDomain(organization.id, emailDomain); - await expect(organizationApi.getEmailDomains(organization.id)).resolves.toEqual([]); - }); - - it('should have default pagination', async () => { - const organization = await organizationApi.create({ name: 'foo' }); - - const emailDomains = Array.from({ length: 30 }, () => `${randomId()}.com`); - - await organizationApi.replaceEmailDomains(organization.id, emailDomains); - - const emailDomainsPage1 = await organizationApi.getEmailDomains(organization.id); - const emailDomainsPage2 = await organizationApi.getEmailDomains(organization.id, 2); - - expect(emailDomainsPage1).toHaveLength(20); - expect(emailDomainsPage2).toHaveLength(10); - expect(emailDomainsPage1.concat(emailDomainsPage2)).toEqual( - expect.arrayContaining( - emailDomains.map((emailDomain) => expect.objectContaining({ emailDomain })) - ) - ); - }); - - it('should return 404 when deleting a non-existent email domain', async () => { - const organization = await organizationApi.create({ name: 'foo' }); - const emailDomain = `${randomId()}.com`; - - await expect( - organizationApi.deleteEmailDomain(organization.id, emailDomain) - ).rejects.toMatchInlineSnapshot('[HTTPError: Request failed with status code 404 Not Found]'); - }); - - it('should return 400 when adding an email domain that already exists', async () => { - const organization = await organizationApi.create({ name: 'foo' }); - const emailDomain = `${randomId()}.com`; - - await organizationApi.addEmailDomain(organization.id, emailDomain); - await expect( - organizationApi.addEmailDomain(organization.id, emailDomain) - ).rejects.toMatchInlineSnapshot( - '[HTTPError: Request failed with status code 422 Unprocessable Entity]' - ); - }); - - it('should be able to replace email domains', async () => { - const organization = await organizationApi.create({ name: 'foo' }); - await organizationApi.addEmailDomain(organization.id, `${randomId()}.com`); - - const emailDomains = [`${randomId()}.com`, `${randomId()}.com`]; - - await organizationApi.replaceEmailDomains(organization.id, emailDomains); - await expect(organizationApi.getEmailDomains(organization.id)).resolves.toEqual( - expect.arrayContaining( - emailDomains.map((emailDomain) => expect.objectContaining({ emailDomain })) - ) - ); - }); -}); diff --git a/packages/integration-tests/src/tests/api/organization/organization-jit.test.ts b/packages/integration-tests/src/tests/api/organization/organization-jit.test.ts new file mode 100644 index 00000000000..04687c10d67 --- /dev/null +++ b/packages/integration-tests/src/tests/api/organization/organization-jit.test.ts @@ -0,0 +1,179 @@ +import { generateStandardId } from '@logto/shared'; + +import { OrganizationApiTest } from '#src/helpers/organization.js'; +import { randomString } from '#src/utils.js'; + +const randomId = () => generateStandardId(6); + +describe('organization just-in-time provisioning', () => { + const organizationApi = new OrganizationApiTest(); + + afterEach(async () => { + await organizationApi.cleanUp(); + }); + + describe('email domains', () => { + it('should add and delete email domains', async () => { + const organization = await organizationApi.create({ name: 'foo' }); + const emailDomain = `${randomId()}.com`; + + await organizationApi.jit.addEmailDomain(organization.id, emailDomain); + await expect(organizationApi.jit.getEmailDomains(organization.id)).resolves.toMatchObject([ + { emailDomain }, + ]); + + await organizationApi.jit.deleteEmailDomain(organization.id, emailDomain); + await expect(organizationApi.jit.getEmailDomains(organization.id)).resolves.toEqual([]); + }); + + it('should have default pagination', async () => { + const organization = await organizationApi.create({ name: 'foo' }); + + const emailDomains = Array.from({ length: 30 }, () => `${randomId()}.com`); + + await organizationApi.jit.replaceEmailDomains(organization.id, emailDomains); + + const emailDomainsPage1 = await organizationApi.jit.getEmailDomains(organization.id); + const emailDomainsPage2 = await organizationApi.jit.getEmailDomains(organization.id, 2); + + expect(emailDomainsPage1).toHaveLength(20); + expect(emailDomainsPage2).toHaveLength(10); + expect(emailDomainsPage1.concat(emailDomainsPage2)).toEqual( + expect.arrayContaining( + emailDomains.map((emailDomain) => expect.objectContaining({ emailDomain })) + ) + ); + }); + + it('should return 404 when deleting a non-existent email domain', async () => { + const organization = await organizationApi.create({ name: 'foo' }); + const emailDomain = `${randomId()}.com`; + + await expect( + organizationApi.jit.deleteEmailDomain(organization.id, emailDomain) + ).rejects.toMatchInlineSnapshot('[HTTPError: Request failed with status code 404 Not Found]'); + }); + + it('should return 422 when adding an email domain that already exists', async () => { + const organization = await organizationApi.create({ name: 'foo' }); + const emailDomain = `${randomId()}.com`; + + await organizationApi.jit.addEmailDomain(organization.id, emailDomain); + await expect( + organizationApi.jit.addEmailDomain(organization.id, emailDomain) + ).rejects.toMatchInlineSnapshot( + '[HTTPError: Request failed with status code 422 Unprocessable Entity]' + ); + }); + + it('should be able to replace email domains', async () => { + const organization = await organizationApi.create({ name: 'foo' }); + await organizationApi.jit.addEmailDomain(organization.id, `${randomId()}.com`); + + const emailDomains = [`${randomId()}.com`, `${randomId()}.com`]; + + await organizationApi.jit.replaceEmailDomains(organization.id, emailDomains); + await expect(organizationApi.jit.getEmailDomains(organization.id)).resolves.toEqual( + expect.arrayContaining( + emailDomains.map((emailDomain) => expect.objectContaining({ emailDomain })) + ) + ); + }); + }); + + describe('organization roles', () => { + it('should add and delete organization roles', async () => { + const organization = await organizationApi.create({ name: `jit-role:${randomString()}` }); + const { id: organizationRoleId } = await organizationApi.roleApi.create({ + name: `jit-role:${randomString()}`, + }); + + await organizationApi.jit.addRole(organization.id, [organizationRoleId]); + await expect(organizationApi.jit.getRoles(organization.id)).resolves.toMatchObject([ + { id: organizationRoleId }, + ]); + + await organizationApi.jit.deleteRole(organization.id, organizationRoleId); + await expect(organizationApi.jit.getRoles(organization.id)).resolves.toEqual([]); + }); + + it('should have no pagination', async () => { + const organization = await organizationApi.create({ name: `jit-role:${randomString()}` }); + const organizationRoles = await Promise.all( + Array.from({ length: 30 }, async () => + organizationApi.roleApi.create({ + name: `jit-role:${randomString()}`, + }) + ) + ); + + await organizationApi.jit.replaceRoles( + organization.id, + organizationRoles.map(({ id }) => id) + ); + + await expect(organizationApi.jit.getRoles(organization.id)).resolves.toEqual( + expect.arrayContaining(organizationRoles.map(({ id }) => expect.objectContaining({ id }))) + ); + }); + + it('should return 404 when deleting a non-existent organization role', async () => { + const organization = await organizationApi.create({ name: `jit-role:${randomString()}` }); + const organizationRoleId = randomId(); + + await expect( + organizationApi.jit.deleteRole(organization.id, organizationRoleId) + ).rejects.toMatchInlineSnapshot('[HTTPError: Request failed with status code 404 Not Found]'); + }); + + it('should return 422 when adding a non-existent organization role', async () => { + const organization = await organizationApi.create({ name: `jit-role:${randomString()}` }); + const organizationRoleId = randomId(); + + await expect( + organizationApi.jit.addRole(organization.id, [organizationRoleId]) + ).rejects.toMatchInlineSnapshot( + '[HTTPError: Request failed with status code 422 Unprocessable Entity]' + ); + }); + + it('should do nothing when adding an organization role that already exists', async () => { + const organization = await organizationApi.create({ name: `jit-role:${randomString()}` }); + const organizationRoles = await Promise.all([ + organizationApi.roleApi.create({ + name: `jit-role:${randomString()}`, + }), + organizationApi.roleApi.create({ + name: `jit-role:${randomString()}`, + }), + ]); + + await organizationApi.jit.addRole(organization.id, [organizationRoles[0].id]); + await expect( + organizationApi.jit.addRole(organization.id, [ + organizationRoles[0].id, + organizationRoles[1].id, + ]) + ).resolves.toBeUndefined(); + }); + + it('should be able to replace organization roles', async () => { + const organization = await organizationApi.create({ name: `jit-role:${randomString()}` }); + const organizationRoles = await Promise.all( + Array.from({ length: 2 }, async () => + organizationApi.roleApi.create({ + name: `jit-role:${randomString()}`, + }) + ) + ); + + await organizationApi.jit.replaceRoles( + organization.id, + organizationRoles.map(({ id }) => id) + ); + await expect(organizationApi.jit.getRoles(organization.id)).resolves.toEqual( + expect.arrayContaining(organizationRoles.map(({ id }) => expect.objectContaining({ id }))) + ); + }); + }); +});