From 47abbd8cb6084d5db874b88be52f35b34c2e5549 Mon Sep 17 00:00:00 2001 From: Darcy Ye Date: Tue, 30 May 2023 00:54:52 +0800 Subject: [PATCH] feat(cloud,schemas): add PATCH /tenants/:id API (#3881) --- packages/cloud/src/libraries/tenants.ts | 7 + packages/cloud/src/queries/tenants.ts | 22 +++ packages/cloud/src/routes/tenants.test.ts | 156 +++++++++++------- packages/cloud/src/routes/tenants.ts | 39 +++++ packages/cloud/src/test-utils/libraries.ts | 6 +- packages/integration-tests/src/api/tenant.ts | 15 +- .../src/tests/api-cloud/tenant.test.ts | 20 ++- 7 files changed, 201 insertions(+), 64 deletions(-) diff --git a/packages/cloud/src/libraries/tenants.ts b/packages/cloud/src/libraries/tenants.ts index 78a9f00b3..1da8cdd65 100644 --- a/packages/cloud/src/libraries/tenants.ts +++ b/packages/cloud/src/libraries/tenants.ts @@ -20,6 +20,7 @@ import { createAdminData, createAdminDataInAdminTenant, getManagementApiResourceIndicator, + type PatchTenant, } from '@logto/schemas'; import { generateStandardId } from '@logto/shared'; import { appendPath } from '@silverhand/essentials'; @@ -70,6 +71,12 @@ export class TenantsLibrary { })); } + async updateTenantById(tenantId: string, payload: PatchTenant): Promise { + const { id, name, tag } = await this.queries.tenants.updateTenantById(tenantId, payload); + + return { id, name, tag, indicator: getManagementApiResourceIndicator(id) }; + } + async createNewTenant( forUserId: string, payload: Pick diff --git a/packages/cloud/src/queries/tenants.ts b/packages/cloud/src/queries/tenants.ts index 46fda6519..bf96e93e7 100644 --- a/packages/cloud/src/queries/tenants.ts +++ b/packages/cloud/src/queries/tenants.ts @@ -7,8 +7,10 @@ import { PredefinedScope, type AdminData, type CreateTenant, + type PatchTenant, type CreateRolesScope, type TenantInfo, + type TenantModel, } from '@logto/schemas'; import { generateStandardId } from '@logto/shared'; import type { PostgreSql } from '@withtyped/postgres'; @@ -46,6 +48,25 @@ export const createTenantsQueries = (client: Queryable) => { ) ); + const updateTenantById = async (tenantId: string, rawPayload: PatchTenant) => { + const payload: Record = Object.fromEntries( + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + Object.entries(rawPayload).filter(([_, value]) => value !== undefined) + ); + const tenant = await client.maybeOne(sql` + update tenants + set ${Object.entries(payload).map(([key, value]) => sql`${id(key)}=${jsonIfNeeded(value)}`)} + where id = ${tenantId} + returning *; + `); + + if (!tenant) { + throw new Error(`Tenant ${tenantId} not found.`); + } + + return tenant; + }; + const createTenantRole = async (parentRole: string, role: string, password: string) => client.query(sql` create role ${id(role)} with inherit login @@ -129,6 +150,7 @@ export const createTenantsQueries = (client: Queryable) => { return { getManagementApiLikeIndicatorsForUser, insertTenant, + updateTenantById, createTenantRole, insertAdminData, getTenantById, diff --git a/packages/cloud/src/routes/tenants.test.ts b/packages/cloud/src/routes/tenants.test.ts index 006a5aab0..332667133 100644 --- a/packages/cloud/src/routes/tenants.test.ts +++ b/packages/cloud/src/routes/tenants.test.ts @@ -56,74 +56,110 @@ describe('POST /api/tenants', () => { tag: TenantTag.Development, indicator: 'https://foo.bar', }; - library.getAvailableTenants.mockResolvedValueOnce([]); - library.createNewTenant.mockResolvedValueOnce(tenant); + library.createNewTenant.mockImplementationOnce(async (_, payload) => { + return { ...tenant, ...payload }; + }); await router.routes()( buildRequestAuthContext('POST /tenants', { - body: { name: 'tenant_a', tag: TenantTag.Development }, + body: { name: 'tenant_named', tag: TenantTag.Staging }, })([CloudScope.CreateTenant]), async ({ json, status }) => { - expect(json).toBe(tenant); - expect(status).toBe(201); - }, - createHttpContext() - ); - }); - - it('should be able to create a new tenant with `create:tenant` scope even if user has a tenant', async () => { - const tenantA: TenantInfo = { - id: 'tenant_a', - name: 'tenant_a', - tag: TenantTag.Development, - indicator: 'https://foo.bar', - }; - const tenantB: TenantInfo = { - id: 'tenant_b', - name: 'tenant_b', - tag: TenantTag.Development, - indicator: 'https://foo.baz', - }; - library.getAvailableTenants.mockResolvedValueOnce([tenantA]); - library.createNewTenant.mockResolvedValueOnce(tenantB); - - await router.routes()( - buildRequestAuthContext('POST /tenants', { - body: { name: 'tenant_b', tag: TenantTag.Development }, - })([CloudScope.CreateTenant]), - async ({ json, status }) => { - expect(json).toBe(tenantB); - expect(status).toBe(201); - }, - createHttpContext() - ); - }); - - it('should be able to create a new tenant with `manage:tenant` scope even if user has a tenant', async () => { - const tenantA: TenantInfo = { - id: 'tenant_a', - name: 'tenant_a', - tag: TenantTag.Development, - indicator: 'https://foo.bar', - }; - const tenantB: TenantInfo = { - id: 'tenant_b', - name: 'tenant_b', - tag: TenantTag.Development, - indicator: 'https://foo.baz', - }; - library.getAvailableTenants.mockResolvedValueOnce([tenantA]); - library.createNewTenant.mockResolvedValueOnce(tenantB); - - await router.routes()( - buildRequestAuthContext('POST /tenants', { - body: { name: 'tenant_b', tag: TenantTag.Development }, - })([CloudScope.ManageTenant]), - async ({ json, status }) => { - expect(json).toBe(tenantB); + expect(json).toStrictEqual({ ...tenant, name: 'tenant_named', tag: TenantTag.Staging }); expect(status).toBe(201); }, createHttpContext() ); }); }); + +describe('PATCH /api/tenants/:tenantId', () => { + const library = new MockTenantsLibrary(); + const router = tenantsRoutes(library); + + it('should throw 403 when lack of permission', async () => { + // Library.getAvailableTenants.mockResolvedValueOnce([]); + + await expect( + router.routes()( + buildRequestAuthContext('PATCH /tenants/tenant_a', { body: {} })(), + noop, + createHttpContext() + ) + ).rejects.toMatchObject({ status: 403 }); + }); + + it('should throw 404 operating unavailable tenants', async () => { + const tenant: TenantInfo = { + id: 'tenant_a', + name: 'tenant_a', + tag: TenantTag.Development, + indicator: 'https://foo.bar', + }; + library.getAvailableTenants.mockResolvedValueOnce([tenant]); + + await expect( + router.routes()( + buildRequestAuthContext('PATCH /tenants/tenant_b', { body: {} })([ + CloudScope.ManageTenantSelf, + ]), + noop, + createHttpContext() + ) + ).rejects.toThrow(); + }); + + it('should be able to update arbitrary tenant with `ManageTenant` scope', async () => { + const tenant: TenantInfo = { + id: 'tenant_a', + name: 'tenant_a', + tag: TenantTag.Development, + indicator: 'https://foo.bar', + }; + // Library.getAvailableTenants.mockResolvedValueOnce([]); + library.updateTenantById.mockImplementationOnce(async (_, payload): Promise => { + return { ...tenant, ...payload }; + }); + + await router.routes()( + buildRequestAuthContext('PATCH /tenants/tenant_a', { + body: { + name: 'tenant_b', + tag: TenantTag.Staging, + }, + })([CloudScope.ManageTenant]), + async ({ json, status }) => { + expect(json).toStrictEqual({ ...tenant, name: 'tenant_b', tag: TenantTag.Staging }); + expect(status).toBe(200); + }, + createHttpContext() + ); + }); + + it('should be able to update available tenant with `ManageTenantSelf` scope', async () => { + const tenant: TenantInfo = { + id: 'tenant_a', + name: 'tenant_a', + tag: TenantTag.Development, + indicator: 'https://foo.bar', + }; + library.getAvailableTenants.mockResolvedValueOnce([tenant]); + library.updateTenantById.mockImplementationOnce(async (_, payload): Promise => { + return { ...tenant, ...payload }; + }); + + await router.routes()( + buildRequestAuthContext('PATCH /tenants/tenant_a', { + body: { + name: 'tenant_b', + tag: TenantTag.Staging, + }, + })([CloudScope.ManageTenant]), + async ({ json, status }) => { + expect(json).toStrictEqual({ ...tenant, name: 'tenant_b', tag: TenantTag.Staging }); + expect(status).toBe(200); + }, + createHttpContext() + ); + }); +}); diff --git a/packages/cloud/src/routes/tenants.ts b/packages/cloud/src/routes/tenants.ts index 5fddcd42a..9f769fffb 100644 --- a/packages/cloud/src/routes/tenants.ts +++ b/packages/cloud/src/routes/tenants.ts @@ -1,4 +1,5 @@ import { CloudScope, tenantInfoGuard, createTenantGuard } from '@logto/schemas'; +import { assert } from '@silverhand/essentials'; import { createRouter, RequestError } from '@withtyped/server'; import type { TenantsLibrary } from '#src/libraries/tenants.js'; @@ -13,6 +14,44 @@ export const tenantsRoutes = (library: TenantsLibrary) => status: 200, }); }) + .patch( + '/:tenantId', + { + body: createTenantGuard.pick({ name: true, tag: true }).partial(), + response: tenantInfoGuard, + }, + async (context, next) => { + /** Users w/o either `ManageTenant` or `ManageTenantSelf` scope does not have permission. */ + if ( + ![CloudScope.ManageTenant, CloudScope.ManageTenantSelf].some((scope) => + context.auth.scopes.includes(scope) + ) + ) { + throw new RequestError('Forbidden due to lack of permission.', 403); + } + + /** Should throw 404 when users with `ManageTenantSelf` scope are attempting to change an unavailable tenant. */ + if (!context.auth.scopes.includes(CloudScope.ManageTenant)) { + const availableTenants = await library.getAvailableTenants(context.auth.id); + assert( + availableTenants.map(({ id }) => id).includes(context.guarded.params.tenantId), + new RequestError( + `Can not find tenant whose id is '${context.guarded.params.tenantId}'.`, + 404 + ) + ); + } + + return next({ + ...context, + json: await library.updateTenantById( + context.guarded.params.tenantId, + context.guarded.body + ), + status: 200, + }); + } + ) .post( '/', { diff --git a/packages/cloud/src/test-utils/libraries.ts b/packages/cloud/src/test-utils/libraries.ts index f675835a4..f5e210c59 100644 --- a/packages/cloud/src/test-utils/libraries.ts +++ b/packages/cloud/src/test-utils/libraries.ts @@ -1,4 +1,4 @@ -import type { ServiceLogType, TenantInfo } from '@logto/schemas'; +import type { ServiceLogType, TenantInfo, TenantTag } from '@logto/schemas'; import type { ServicesLibrary } from '#src/libraries/services.js'; import type { TenantsLibrary } from '#src/libraries/tenants.js'; @@ -13,6 +13,10 @@ export class MockTenantsLibrary implements TenantsLibrary { public getAvailableTenants = jest.fn, [string]>(); public createNewTenant = jest.fn, [string, Record]>(); + public updateTenantById = jest.fn< + Promise, + [string, { name?: string; tag?: TenantTag }] + >(); } export class MockServicesLibrary implements ServicesLibrary { diff --git a/packages/integration-tests/src/api/tenant.ts b/packages/integration-tests/src/api/tenant.ts index 4414c186c..31f7360a4 100644 --- a/packages/integration-tests/src/api/tenant.ts +++ b/packages/integration-tests/src/api/tenant.ts @@ -1,4 +1,4 @@ -import type { CreateTenant, TenantInfo } from '@logto/schemas'; +import type { CreateTenant, TenantInfo, TenantTag } from '@logto/schemas'; import { cloudApi } from './api.js'; @@ -22,3 +22,16 @@ export const getTenants = async (accessToken: string) => { .get('tenants') .json(); }; + +export const updateTenant = async ( + accessToken: string, + tenantId: string, + payload: { name?: string; tag?: TenantTag } +) => { + return cloudApi + .extend({ + headers: { authorization: `Bearer ${accessToken}` }, + }) + .patch(`tenants/${tenantId}`, { json: payload }) + .json(); +}; diff --git a/packages/integration-tests/src/tests/api-cloud/tenant.test.ts b/packages/integration-tests/src/tests/api-cloud/tenant.test.ts index 24bab6ff7..86a746de1 100644 --- a/packages/integration-tests/src/tests/api-cloud/tenant.test.ts +++ b/packages/integration-tests/src/tests/api-cloud/tenant.test.ts @@ -11,7 +11,7 @@ import { } from '@logto/schemas'; import { authedAdminTenantApi } from '#src/api/api.js'; -import { createTenant, getTenants } from '#src/api/tenant.js'; +import { updateTenant, createTenant, getTenants } from '#src/api/tenant.js'; import { createUserAndSignInToCloudClient } from '#src/helpers/admin-tenant.js'; describe('Tenant APIs', () => { @@ -36,10 +36,17 @@ describe('Tenant APIs', () => { expect(tenant).toHaveProperty('tag', payload.tag); expect(tenant).toHaveProperty('name', payload.name); } + const tenant2Updated = await updateTenant(accessToken, tenant2.id, { + tag: TenantTag.Staging, + name: 'tenant2-updated', + }); + expect(tenant2Updated.id).toEqual(tenant2.id); + expect(tenant2Updated).toHaveProperty('tag', TenantTag.Staging); + expect(tenant2Updated).toHaveProperty('name', 'tenant2-updated'); const tenants = await getTenants(accessToken); expect(tenants.length).toBeGreaterThan(2); expect(tenants.find((tenant) => tenant.id === tenant1.id)).toStrictEqual(tenant1); - expect(tenants.find((tenant) => tenant.id === tenant2.id)).toStrictEqual(tenant2); + expect(tenants.find((tenant) => tenant.id === tenant2Updated.id)).toStrictEqual(tenant2Updated); }); it('should be able to create multiple tenants for `user` role', async () => { @@ -67,6 +74,15 @@ describe('Tenant APIs', () => { expect(tenants.length).toEqual(2); expect(tenants.find((tenant) => tenant.id === tenant1.id)).toStrictEqual(tenant1); expect(tenants.find((tenant) => tenant.id === tenant2.id)).toStrictEqual(tenant2); + const { client: anotherClient } = await createUserAndSignInToCloudClient(AdminTenantRole.User); + const anotherAccessToken = await anotherClient.getAccessToken(cloudApiIndicator); + const anotherTenant = await createTenant(anotherAccessToken, { + name: 'another-tenant', + tag: TenantTag.Development, + }); + await expect( + updateTenant(accessToken, anotherTenant.id, { name: 'another-tenant-updated' }) + ).rejects.toThrow(); }); it('`user` role should have `CloudScope.ManageTenantSelf` scope', async () => {