From 6b09da2f5d9c14e656dce28c84479404a232250a Mon Sep 17 00:00:00 2001 From: Gao Sun Date: Thu, 9 Feb 2023 18:31:14 +0800 Subject: [PATCH] refactor: fix alteration --- .github/workflows/integration-test.yml | 10 +++- .github/workflows/main.yml | 4 -- .scripts/check-database.js | 3 -- .scripts/compare-database.js | 14 ++---- packages/core/jest.setup.js | 15 +----- packages/core/package.json | 4 +- packages/core/src/app/init.ts | 11 +++- packages/core/src/env-set/index.ts | 16 ++++-- packages/core/src/index.ts | 7 ++- packages/core/src/libraries/hook.test.ts | 1 - packages/core/src/middleware/koa-auth.test.ts | 25 +++++----- packages/core/src/oidc/adapter.test.ts | 8 +-- packages/core/src/oidc/init.test.ts | 4 +- packages/core/src/oidc/utils.test.ts | 10 ++-- packages/core/src/tenants/Tenant.test.ts | 4 ++ packages/core/src/tenants/Tenant.ts | 2 +- packages/core/src/tenants/index.ts | 1 + packages/core/src/tenants/utils.ts | 17 +++---- packages/core/src/test-utils/env-set.ts | 5 ++ packages/core/src/test-utils/tenant.ts | 4 +- packages/integration-tests/package.json | 2 +- .../next-1675788753-multi-tenancy-rls.ts | 50 +++++++++++++++---- packages/schemas/package.json | 2 +- packages/schemas/src/models/hooks.ts | 4 +- packages/schemas/src/models/tenants.ts | 1 - packages/schemas/tables/_after_all.sql | 17 +++++++ pnpm-lock.yaml | 28 +++++------ 27 files changed, 161 insertions(+), 108 deletions(-) delete mode 100644 .scripts/check-database.js create mode 100644 packages/core/src/test-utils/env-set.ts diff --git a/.github/workflows/integration-test.yml b/.github/workflows/integration-test.yml index 16a0ae409..58635272d 100644 --- a/.github/workflows/integration-test.yml +++ b/.github/workflows/integration-test.yml @@ -88,7 +88,7 @@ jobs: - name: Run Logto working-directory: logto/ - run: npm start & + run: nohup npm start > nohup.out 2> nohup.err < /dev/null & env: INTEGRATION_TEST: true @@ -101,3 +101,11 @@ jobs: cd tests/packages/integration-tests pnpm build pnpm test:${{ matrix.test_target }} + + - name: Show logs + working-directory: logto/ + run: cat nohup.out + + - name: Show error logs + working-directory: logto/ + run: cat nohup.err diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index c654913d7..85db7712a 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -173,10 +173,6 @@ jobs: run: node .scripts/compare-database.js fresh old # ** End ** - - name: Check database - working-directory: ./fresh - run: node .scripts/check-database.js fresh - - name: Check alteration databases working-directory: ./fresh run: node .scripts/check-alterations-sequence.js diff --git a/.scripts/check-database.js b/.scripts/check-database.js deleted file mode 100644 index 26165cf3e..000000000 --- a/.scripts/check-database.js +++ /dev/null @@ -1,3 +0,0 @@ -throw new Error('not implemented'); - -// TODO: check tables have tenant_id diff --git a/.scripts/compare-database.js b/.scripts/compare-database.js index 9f6f09331..a6a10a1d0 100644 --- a/.scripts/compare-database.js +++ b/.scripts/compare-database.js @@ -117,20 +117,12 @@ assert.deepStrictEqual(...manifests); const queryDatabaseData = async (database) => { const pool = new pg.Pool({ database, user: 'postgres', password: 'postgres' }); const result = await Promise.all(manifests[0].tables - .filter(({ table_name }) => !['logto_configs', '_logto_configs'].includes(table_name)) + // system configs are usually generated or time-relative, ignore for now + .filter(({ table_name }) => !['logto_configs', '_logto_configs', 'systems'].includes(table_name)) .map(async ({ table_name }) => { const { rows } = await pool.query(/* sql */`select * from ${table_name};`); - if (table_name === 'systems') { - return [ - table_name, - rows.map(({ value, ...rest }) => - ({ ...rest, value: omit(value, 'createdAt', 'updatedAt') }) - ), - ]; - } - - return [table_name, omitArray(rows, 'created_at', 'updated_at', 'secret')]; + return [table_name, omitArray(rows, 'created_at', 'updated_at', 'secret', 'db_user', 'db_user_password')]; }) ); diff --git a/packages/core/jest.setup.js b/packages/core/jest.setup.js index 1ee99e947..0cd06ef71 100644 --- a/packages/core/jest.setup.js +++ b/packages/core/jest.setup.js @@ -3,7 +3,6 @@ */ import { createMockUtils } from '@logto/shared/esm'; -import { createMockQueryResult, createMockPool } from 'slonik'; const { jest } = import.meta; const { mockEsm, mockEsmWithActual, mockEsmDefault } = createMockUtils(jest); @@ -12,6 +11,7 @@ process.env.DB_URL = 'postgres://mock.db.url'; process.env.ENDPOINT = 'https://logto.test'; process.env.NODE_ENV = 'test'; +/* Mock for EnvSet */ mockEsm('#src/libraries/logto-config.js', () => ({ createLogtoConfigLibrary: () => ({ getOidcConfigs: () => ({}) }), })); @@ -24,6 +24,7 @@ mockEsm('#src/env-set/check-alteration-state.js', () => ({ mockEsmDefault('#src/env-set/oidc.js', () => () => ({ issuer: 'https://logto.test/oidc', })); +/* End */ await mockEsmWithActual('#src/env-set/index.js', () => ({ MountedApps: { @@ -33,18 +34,6 @@ await mockEsmWithActual('#src/env-set/index.js', () => ({ DemoApp: 'demo-app', Welcome: 'welcome', }, - // TODO: Remove after clean up of default env sets - default: { - get oidc() { - return { - issuer: 'https://logto.test/oidc', - }; - }, - get pool() { - return createMockPool({ query: async () => createMockQueryResult([]) }); - }, - load: jest.fn(), - }, })); // Logger is not considered in all test cases diff --git a/packages/core/package.json b/packages/core/package.json index 25d9e7a50..7db6f7a04 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -34,8 +34,8 @@ "@logto/schemas": "workspace:*", "@logto/shared": "workspace:*", "@silverhand/essentials": "2.1.0", - "@withtyped/postgres": "^0.4.1", - "@withtyped/server": "^0.4.1", + "@withtyped/postgres": "^0.5.1", + "@withtyped/server": "^0.5.1", "chalk": "^5.0.0", "clean-deep": "^3.4.0", "date-fns": "^2.29.3", diff --git a/packages/core/src/app/init.ts b/packages/core/src/app/init.ts index 1370d7cb7..6af14b6d8 100644 --- a/packages/core/src/app/init.ts +++ b/packages/core/src/app/init.ts @@ -17,8 +17,15 @@ const logListening = (type: 'core' | 'admin' = 'core') => { }; const getTenantId = () => { - if (!EnvSet.values.isDomainBasedMultiTenancy) { - return (!EnvSet.values.isProduction && EnvSet.values.developmentTenantId) || defaultTenant; + const { isDomainBasedMultiTenancy, isProduction, isIntegrationTest, developmentTenantId } = + EnvSet.values; + + if (!isDomainBasedMultiTenancy) { + if ((!isProduction || isIntegrationTest) && developmentTenantId) { + return developmentTenantId; + } + + return defaultTenant; } throw new Error('Not implemented'); diff --git a/packages/core/src/env-set/index.ts b/packages/core/src/env-set/index.ts index 5b529a117..b95341d92 100644 --- a/packages/core/src/env-set/index.ts +++ b/packages/core/src/env-set/index.ts @@ -7,7 +7,6 @@ import { createLogtoConfigLibrary } from '#src/libraries/logto-config.js'; import { appendPath } from '#src/utils/url.js'; import GlobalValues from './GlobalValues.js'; -import { checkAlterationState } from './check-alteration-state.js'; import createPool from './create-pool.js'; import createQueryClient from './create-query-client.js'; import loadOidcValues from './oidc.js'; @@ -23,12 +22,20 @@ export enum MountedApps { export class EnvSet { static values = new GlobalValues(); - static default = new EnvSet(EnvSet.values.dbUrl); static get isTest() { return this.values.isTest; } + static get dbUrl() { + return this.values.dbUrl; + } + + static queryClient = createQueryClient(this.dbUrl, this.isTest); + + /** @deprecated Only for backward compatibility; Will be replaced soon. */ + static pool = createPool(this.dbUrl, this.isTest); + #pool: Optional; // Use another pool for `withtyped` while adopting the new model, // as we cannot extract the original PgPool from slonik @@ -76,12 +83,11 @@ export class EnvSet { this.#queryClient = createQueryClient(this.databaseUrl, EnvSet.isTest); const { getOidcConfigs } = createLogtoConfigLibrary(pool); - const [, oidcConfigs] = await Promise.all([checkAlterationState(pool), getOidcConfigs()]); + + const oidcConfigs = await getOidcConfigs(); this.#oidc = await loadOidcValues( appendPath(EnvSet.values.endpoint, '/oidc').toString(), oidcConfigs ); } } - -await EnvSet.default.load(); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 78a36aa86..2001b241b 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -3,6 +3,8 @@ import dotenv from 'dotenv'; import { findUp } from 'find-up'; import Koa from 'koa'; +import { checkAlterationState } from './env-set/check-alteration-state.js'; + dotenv.config({ path: await findUp('.env', {}) }); // Import after env has been configured @@ -17,7 +19,10 @@ try { }); await initI18n(); await loadConnectorFactories(); - await checkRowLevelSecurity(EnvSet.default.queryClient); + await Promise.all([ + checkRowLevelSecurity(EnvSet.queryClient), + checkAlterationState(await EnvSet.pool), + ]); // Import last until init completed const { default: initApp } = await import('./app/init.js'); diff --git a/packages/core/src/libraries/hook.test.ts b/packages/core/src/libraries/hook.test.ts index 068196b6a..65443520e 100644 --- a/packages/core/src/libraries/hook.test.ts +++ b/packages/core/src/libraries/hook.test.ts @@ -27,7 +27,6 @@ const queryFunction = jest.fn(); const url = 'https://logto.gg'; const hook: InferModelType = { - tenantId: undefined, id: 'foo', event: HookEvent.PostSignIn, config: { headers: { bar: 'baz' }, url, retries: 3 }, diff --git a/packages/core/src/middleware/koa-auth.test.ts b/packages/core/src/middleware/koa-auth.test.ts index 0e431c197..6964f107e 100644 --- a/packages/core/src/middleware/koa-auth.test.ts +++ b/packages/core/src/middleware/koa-auth.test.ts @@ -6,6 +6,7 @@ import Sinon from 'sinon'; import { EnvSet } from '#src/env-set/index.js'; import RequestError from '#src/errors/RequestError/index.js'; +import { mockEnvSet } from '#src/test-utils/env-set.js'; import { createContextWithRouteParameters } from '#src/utils/test-utils.js'; import type { WithAuthContext } from './koa-auth.js'; @@ -63,7 +64,7 @@ describe('koaAuth middleware', () => { developmentUserId: 'foo', }); - await koaAuth(EnvSet.default)(ctx, next); + await koaAuth(mockEnvSet)(ctx, next); expect(ctx.auth).toEqual({ type: 'user', id: 'foo' }); stub.restore(); @@ -78,7 +79,7 @@ describe('koaAuth middleware', () => { }, }; - await koaAuth(EnvSet.default)(mockCtx, next); + await koaAuth(mockEnvSet)(mockCtx, next); expect(mockCtx.auth).toEqual({ type: 'user', id: 'foo' }); }); @@ -90,7 +91,7 @@ describe('koaAuth middleware', () => { isIntegrationTest: true, }); - await koaAuth(EnvSet.default)(ctx, next); + await koaAuth(mockEnvSet)(ctx, next); expect(ctx.auth).toEqual({ type: 'user', id: 'foo' }); stub.restore(); @@ -111,7 +112,7 @@ describe('koaAuth middleware', () => { }, }; - await koaAuth(EnvSet.default)(mockCtx, next); + await koaAuth(mockEnvSet)(mockCtx, next); expect(mockCtx.auth).toEqual({ type: 'user', id: 'foo' }); stub.restore(); @@ -124,12 +125,12 @@ describe('koaAuth middleware', () => { authorization: 'Bearer access_token', }, }; - await koaAuth(EnvSet.default)(ctx, next); + await koaAuth(mockEnvSet)(ctx, next); expect(ctx.auth).toEqual({ type: 'user', id: 'fooUser' }); }); it('expect to throw if authorization header is missing', async () => { - await expect(koaAuth(EnvSet.default)(ctx, next)).rejects.toMatchError(authHeaderMissingError); + await expect(koaAuth(mockEnvSet)(ctx, next)).rejects.toMatchError(authHeaderMissingError); }); it('expect to throw if authorization header token type not recognized ', async () => { @@ -140,7 +141,7 @@ describe('koaAuth middleware', () => { }, }; - await expect(koaAuth(EnvSet.default)(ctx, next)).rejects.toMatchError(tokenNotSupportedError); + await expect(koaAuth(mockEnvSet)(ctx, next)).rejects.toMatchError(tokenNotSupportedError); }); it('expect to throw if jwt sub is missing', async () => { @@ -153,7 +154,7 @@ describe('koaAuth middleware', () => { }, }; - await expect(koaAuth(EnvSet.default)(ctx, next)).rejects.toMatchError(jwtSubMissingError); + await expect(koaAuth(mockEnvSet)(ctx, next)).rejects.toMatchError(jwtSubMissingError); }); it('expect to have `client` type per jwt verify result', async () => { @@ -166,7 +167,7 @@ describe('koaAuth middleware', () => { }, }; - await koaAuth(EnvSet.default)(ctx, next); + await koaAuth(mockEnvSet)(ctx, next); expect(ctx.auth).toEqual({ type: 'app', id: 'bar' }); }); @@ -180,7 +181,7 @@ describe('koaAuth middleware', () => { }, }; - await expect(koaAuth(EnvSet.default, UserRole.Admin)(ctx, next)).rejects.toMatchError( + await expect(koaAuth(mockEnvSet, UserRole.Admin)(ctx, next)).rejects.toMatchError( forbiddenError ); }); @@ -197,7 +198,7 @@ describe('koaAuth middleware', () => { }, }; - await expect(koaAuth(EnvSet.default, UserRole.Admin)(ctx, next)).rejects.toMatchError( + await expect(koaAuth(mockEnvSet, UserRole.Admin)(ctx, next)).rejects.toMatchError( forbiddenError ); }); @@ -213,7 +214,7 @@ describe('koaAuth middleware', () => { }, }; - await expect(koaAuth(EnvSet.default)(ctx, next)).rejects.toMatchError( + await expect(koaAuth(mockEnvSet)(ctx, next)).rejects.toMatchError( new RequestError({ code: 'auth.unauthorized', status: 401 }, new Error('unknown error')) ); }); diff --git a/packages/core/src/oidc/adapter.test.ts b/packages/core/src/oidc/adapter.test.ts index ef15d8342..23e39e0bb 100644 --- a/packages/core/src/oidc/adapter.test.ts +++ b/packages/core/src/oidc/adapter.test.ts @@ -3,7 +3,7 @@ import { createMockUtils } from '@logto/shared/esm'; import snakecaseKeys from 'snakecase-keys'; import { mockApplication } from '#src/__mocks__/index.js'; -import { EnvSet } from '#src/env-set/index.js'; +import { mockEnvSet } from '#src/test-utils/env-set.js'; import { MockQueries } from '#src/test-utils/tenant.js'; import { getConstantClientMetadata } from './utils.js'; @@ -48,7 +48,7 @@ const now = Date.now(); describe('postgres Adapter', () => { it('Client Modal', async () => { const rejectError = new Error('Not implemented'); - const adapter = postgresAdapter(EnvSet.default, queries, 'Client'); + const adapter = postgresAdapter(mockEnvSet, queries, 'Client'); await expect(adapter.upsert('client', {}, 0)).rejects.toMatchError(rejectError); await expect(adapter.findByUserCode('foo')).rejects.toMatchError(rejectError); @@ -72,7 +72,7 @@ describe('postgres Adapter', () => { client_id, client_name, client_secret, - ...getConstantClientMetadata(EnvSet.default, type), + ...getConstantClientMetadata(mockEnvSet, type), ...snakecaseKeys(oidcClientMetadata), ...customClientMetadata, }); @@ -85,7 +85,7 @@ describe('postgres Adapter', () => { const id = 'fooId'; const grantId = 'grantId'; const expireAt = 60; - const adapter = postgresAdapter(EnvSet.default, queries, modelName); + const adapter = postgresAdapter(mockEnvSet, queries, modelName); await adapter.upsert(id, { uid, userCode }, expireAt); expect(upsertInstance).toBeCalledWith({ diff --git a/packages/core/src/oidc/init.test.ts b/packages/core/src/oidc/init.test.ts index f16d92764..212f63204 100644 --- a/packages/core/src/oidc/init.test.ts +++ b/packages/core/src/oidc/init.test.ts @@ -1,4 +1,4 @@ -import { EnvSet } from '#src/env-set/index.js'; +import { mockEnvSet } from '#src/test-utils/env-set.js'; import { MockTenant } from '#src/test-utils/tenant.js'; import initOidc from './init.js'; @@ -7,6 +7,6 @@ describe('oidc provider init', () => { it('init should not throw', async () => { const { queries, libraries } = new MockTenant(); - expect(() => initOidc(EnvSet.default, queries, libraries)).not.toThrow(); + expect(() => initOidc(mockEnvSet, queries, libraries)).not.toThrow(); }); }); diff --git a/packages/core/src/oidc/utils.test.ts b/packages/core/src/oidc/utils.test.ts index fed59cc9a..b45862457 100644 --- a/packages/core/src/oidc/utils.test.ts +++ b/packages/core/src/oidc/utils.test.ts @@ -1,6 +1,6 @@ import { ApplicationType, CustomClientMetadataKey, GrantType } from '@logto/schemas'; -import { EnvSet } from '#src/env-set/index.js'; +import { mockEnvSet } from '#src/test-utils/env-set.js'; import { isOriginAllowed, @@ -10,22 +10,22 @@ import { } from './utils.js'; describe('getConstantClientMetadata()', () => { - expect(getConstantClientMetadata(EnvSet.default, ApplicationType.SPA)).toEqual({ + expect(getConstantClientMetadata(mockEnvSet, ApplicationType.SPA)).toEqual({ application_type: 'web', grant_types: [GrantType.AuthorizationCode, GrantType.RefreshToken], token_endpoint_auth_method: 'none', }); - expect(getConstantClientMetadata(EnvSet.default, ApplicationType.Native)).toEqual({ + expect(getConstantClientMetadata(mockEnvSet, ApplicationType.Native)).toEqual({ application_type: 'native', grant_types: [GrantType.AuthorizationCode, GrantType.RefreshToken], token_endpoint_auth_method: 'none', }); - expect(getConstantClientMetadata(EnvSet.default, ApplicationType.Traditional)).toEqual({ + expect(getConstantClientMetadata(mockEnvSet, ApplicationType.Traditional)).toEqual({ application_type: 'web', grant_types: [GrantType.AuthorizationCode, GrantType.RefreshToken], token_endpoint_auth_method: 'client_secret_basic', }); - expect(getConstantClientMetadata(EnvSet.default, ApplicationType.MachineToMachine)).toEqual({ + expect(getConstantClientMetadata(mockEnvSet, ApplicationType.MachineToMachine)).toEqual({ application_type: 'web', grant_types: [GrantType.ClientCredentials], token_endpoint_auth_method: 'client_secret_basic', diff --git a/packages/core/src/tenants/Tenant.test.ts b/packages/core/src/tenants/Tenant.test.ts index 870a4344b..40f8a303c 100644 --- a/packages/core/src/tenants/Tenant.test.ts +++ b/packages/core/src/tenants/Tenant.test.ts @@ -25,6 +25,10 @@ const middlewareList = [ return mock; }); +mockEsm('./utils.js', () => ({ + getTenantDatabaseDsn: async () => 'postgres://mock.db.url', +})); + // eslint-disable-next-line unicorn/consistent-function-scoping mockEsmDefault('#src/oidc/init.js', () => () => createMockProvider()); diff --git a/packages/core/src/tenants/Tenant.ts b/packages/core/src/tenants/Tenant.ts index 603d6bb93..1afde1df8 100644 --- a/packages/core/src/tenants/Tenant.ts +++ b/packages/core/src/tenants/Tenant.ts @@ -28,7 +28,7 @@ import { getTenantDatabaseDsn } from './utils.js'; export default class Tenant implements TenantContext { static async create(id: string): Promise { // Treat the default database URL as the management URL - const envSet = new EnvSet(await getTenantDatabaseDsn(EnvSet.default, id)); + const envSet = new EnvSet(await getTenantDatabaseDsn(id)); await envSet.load(); return new Tenant(envSet, id); diff --git a/packages/core/src/tenants/index.ts b/packages/core/src/tenants/index.ts index 22b3107f9..365acc433 100644 --- a/packages/core/src/tenants/index.ts +++ b/packages/core/src/tenants/index.ts @@ -12,6 +12,7 @@ class TenantPool { return tenant; } + console.log('Init tenant:', tenantId); const newTenant = await Tenant.create(tenantId); this.cache.set(tenantId, newTenant); diff --git a/packages/core/src/tenants/utils.ts b/packages/core/src/tenants/utils.ts index cb81e1b23..4c0097aea 100644 --- a/packages/core/src/tenants/utils.ts +++ b/packages/core/src/tenants/utils.ts @@ -6,20 +6,21 @@ import { identifier, sql } from '@withtyped/postgres'; import type { QueryClient } from '@withtyped/server'; import { parseDsn, stringifyDsn } from 'slonik'; -import type { EnvSet } from '#src/env-set/index.js'; +import { EnvSet } from '#src/env-set/index.js'; /** * This function is to fetch the tenant password for the corresponding Postgres user. * * In multi-tenancy mode, Logto should ALWAYS use a restricted user with RLS enforced to ensure data isolation between tenants. */ -export const getTenantDatabaseDsn = async (defaultEnvSet: EnvSet, tenantId: string) => { +export const getTenantDatabaseDsn = async (tenantId: string) => { + const { queryClient, dbUrl } = EnvSet; const { tableName, rawKeys: { id, dbUser, dbUserPassword }, } = Tenants; - const { rows } = await defaultEnvSet.queryClient.query(sql` + const { rows } = await queryClient.query(sql` select ${identifier(dbUser)}, ${identifier(dbUserPassword)} from ${identifier(tableName)} where ${identifier(id)} = ${tenantId} @@ -29,14 +30,14 @@ export const getTenantDatabaseDsn = async (defaultEnvSet: EnvSet, tenantId: stri throw new Error(`Cannot find valid tenant credentials for ID ${tenantId}`); } - const options = parseDsn(defaultEnvSet.databaseUrl); + const options = parseDsn(dbUrl); const username = rows[0][dbUser]; const password = rows[0][dbUserPassword]; return stringifyDsn({ ...options, - username: conditional(!username && String(username)), - password: conditional(!password && String(password)), + username: conditional(typeof username === 'string' && username), + password: conditional(typeof password === 'string' && password), }); }; @@ -48,9 +49,7 @@ export const checkRowLevelSecurity = async (client: QueryClient) => { and rowsecurity=false `); - if ( - rows.some(({ tablename }) => tablename !== Systems.table && tablename !== Tenants.tableName) - ) { + if (rows.some(({ tablename }) => tablename !== Systems.table)) { throw new Error( 'Row-level security has to be enforced on EVERY business table when starting Logto.\n' + `Found following table(s) without RLS: ${rows diff --git a/packages/core/src/test-utils/env-set.ts b/packages/core/src/test-utils/env-set.ts new file mode 100644 index 000000000..b135710e5 --- /dev/null +++ b/packages/core/src/test-utils/env-set.ts @@ -0,0 +1,5 @@ +import { EnvSet } from '#src/env-set/index.js'; + +export const mockEnvSet = new EnvSet(EnvSet.values.dbUrl); + +await mockEnvSet.load(); diff --git a/packages/core/src/test-utils/tenant.ts b/packages/core/src/test-utils/tenant.ts index b74698ebd..95fa0d7f7 100644 --- a/packages/core/src/test-utils/tenant.ts +++ b/packages/core/src/test-utils/tenant.ts @@ -1,11 +1,11 @@ import { createMockPool, createMockQueryResult } from 'slonik'; -import { EnvSet } from '#src/env-set/index.js'; import { createModelRouters } from '#src/model-routers/index.js'; import Libraries from '#src/tenants/Libraries.js'; import Queries from '#src/tenants/Queries.js'; import type TenantContext from '#src/tenants/TenantContext.js'; +import { mockEnvSet } from './env-set.js'; import type { GrantMock } from './oidc-provider.js'; import { createMockProvider } from './oidc-provider.js'; import { MockQueryClient } from './query-client.js'; @@ -45,7 +45,7 @@ export type DeepPartial = T extends object export type Partial2 = { [key in keyof T]?: Partial }; export class MockTenant implements TenantContext { - public envSet = EnvSet.default; + public envSet = mockEnvSet; public queries: Queries; public libraries: Libraries; public modelRouters = createModelRouters(new MockQueryClient()); diff --git a/packages/integration-tests/package.json b/packages/integration-tests/package.json index 03522ec9f..6cebc9fd3 100644 --- a/packages/integration-tests/package.json +++ b/packages/integration-tests/package.json @@ -53,6 +53,6 @@ }, "prettier": "@silverhand/eslint-config/.prettierrc", "dependencies": { - "@withtyped/server": "^0.4.1" + "@withtyped/server": "^0.5.1" } } diff --git a/packages/schemas/alterations/next-1675788753-multi-tenancy-rls.ts b/packages/schemas/alterations/next-1675788753-multi-tenancy-rls.ts index c1a41f23e..ebe3b594d 100644 --- a/packages/schemas/alterations/next-1675788753-multi-tenancy-rls.ts +++ b/packages/schemas/alterations/next-1675788753-multi-tenancy-rls.ts @@ -44,15 +44,34 @@ const alteration: AlterationScript = { await pool.query(sql` alter table hooks add column tenant_id varchar(21) not null default 'default' - references tenants (id) on update cascade on delete cascade; + references tenants (id) on update cascade on delete cascade, + alter column id type varchar(21); -- OK to downsize since we use length 21 for ID generation in core + alter table hooks alter column tenant_id drop default; + create index hooks__id on hooks (tenant_id, id); + + drop index hooks__event; + create index hooks__event on hooks (tenant_id, event); + + create trigger set_tenant_id before insert on hooks + for each row execute procedure set_tenant_id(); + `); + + // Add db_user column to tenants table + await pool.query(sql` + alter table tenants + add column db_user varchar(128), + add constraint tenants__db_user + unique (db_user); `); // Create role and setup privileges const baseRole = `logto_tenant_${database}`; const baseRoleId = getId(baseRole); + + // See `_after_all.sql` for comments await pool.query(sql` create role ${baseRoleId} noinherit; @@ -65,19 +84,21 @@ const alteration: AlterationScript = { on table tenants from ${baseRoleId}; + grant select (id, db_user) + on table tenants + to ${baseRoleId}; + + alter table tenants enable row level security; + + create policy tenants_tenant_id on tenants + to ${baseRoleId} + using (db_user = current_user); + revoke all privileges on table systems from ${baseRoleId}; `); - // Add db_user column to tenants table - await pool.query(sql` - alter table tenants - add column db_user varchar(128), - add constraint tenants__db_user - unique (db_user); - `); - // Enable RLS await Promise.all( tables.map(async (tableName) => @@ -130,6 +151,9 @@ const alteration: AlterationScript = { in schema public from ${baseRoleId}; + drop policy tenants_tenant_id on tenants; + alter table tenants disable row level security; + drop role ${baseRoleId}; `); @@ -139,13 +163,17 @@ const alteration: AlterationScript = { drop column db_user; `); - console.log('3'); // Revert hooks table from multi-tenancy await pool.query(sql` drop index hooks__id; alter table hooks - drop column tenant_id; + drop column tenant_id, + alter column id type varchar(32); + + create index hooks__event on hooks (event); + + drop trigger set_tenant_id on hooks; `); }, }; diff --git a/packages/schemas/package.json b/packages/schemas/package.json index 2d8bfda43..3dea0b0fd 100644 --- a/packages/schemas/package.json +++ b/packages/schemas/package.json @@ -83,7 +83,7 @@ "@logto/language-kit": "workspace:*", "@logto/phrases": "workspace:*", "@logto/phrases-ui": "workspace:*", - "@withtyped/server": "^0.4.1", + "@withtyped/server": "^0.5.1", "zod": "^3.20.2" } } diff --git a/packages/schemas/src/models/hooks.ts b/packages/schemas/src/models/hooks.ts index a8210ee4e..bcd7446c6 100644 --- a/packages/schemas/src/models/hooks.ts +++ b/packages/schemas/src/models/hooks.ts @@ -58,7 +58,7 @@ export const Hooks = createModel(/* sql */ ` create index hooks__event on hooks (tenant_id, event); `) - .extend('tenantId', z.string().optional()) .extend('id', { default: () => generateStandardId(), readonly: true }) .extend('event', z.nativeEnum(HookEvent)) // Tried to use `.refine()` to show the correct error path, but not working. - .extend('config', hookConfigGuard); + .extend('config', hookConfigGuard) + .exclude('tenantId'); diff --git a/packages/schemas/src/models/tenants.ts b/packages/schemas/src/models/tenants.ts index dbfb8fdfe..e2bd0fd35 100644 --- a/packages/schemas/src/models/tenants.ts +++ b/packages/schemas/src/models/tenants.ts @@ -10,6 +10,5 @@ export const Tenants = createModel(/* sql */ ` constraint tenants__db_user unique (db_user) ); - /* no_after_each */ `); diff --git a/packages/schemas/tables/_after_all.sql b/packages/schemas/tables/_after_all.sql index a3e8d56f7..e9571bd7a 100644 --- a/packages/schemas/tables/_after_all.sql +++ b/packages/schemas/tables/_after_all.sql @@ -5,10 +5,27 @@ grant select, insert, update, delete in schema public to logto_tenant_${database}; +-- Security policies for tenants table -- + revoke all privileges on table tenants from logto_tenant_${database}; +/* Allow limited select to perform RLS query in `after_each` (using select ... from tenants ...) */ +grant select (id, db_user) + on table tenants + to logto_tenant_${database}; + +alter table tenants enable row level security; + +/* Create RLS policy to minimize the privilege */ +create policy tenants_tenant_id on tenants + to logto_tenant_${database} + using (db_user = current_user); + +-- End -- + +/* Revoke all privileges on systems table for tenant roles */ revoke all privileges on table systems from logto_tenant_${database}; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0b4571efb..b72286380 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -276,8 +276,8 @@ importers: '@types/semver': ^7.3.12 '@types/sinon': ^10.0.13 '@types/supertest': ^2.0.11 - '@withtyped/postgres': ^0.4.1 - '@withtyped/server': ^0.4.1 + '@withtyped/postgres': ^0.5.1 + '@withtyped/server': ^0.5.1 chalk: ^5.0.0 clean-deep: ^3.4.0 copyfiles: ^2.4.1 @@ -335,8 +335,8 @@ importers: '@logto/schemas': link:../schemas '@logto/shared': link:../shared '@silverhand/essentials': 2.1.0 - '@withtyped/postgres': 0.4.1_@withtyped+server@0.4.1 - '@withtyped/server': 0.4.1 + '@withtyped/postgres': 0.5.1_@withtyped+server@0.5.1 + '@withtyped/server': 0.5.1 chalk: 5.1.2 clean-deep: 3.4.0 date-fns: 2.29.3 @@ -481,7 +481,7 @@ importers: '@types/jest': ^29.1.2 '@types/jest-environment-puppeteer': ^5.0.2 '@types/node': ^18.11.18 - '@withtyped/server': ^0.4.1 + '@withtyped/server': ^0.5.1 dotenv: ^16.0.0 eslint: ^8.21.0 got: ^12.5.3 @@ -495,7 +495,7 @@ importers: text-encoder: ^0.0.4 typescript: ^4.9.4 dependencies: - '@withtyped/server': 0.4.1 + '@withtyped/server': 0.5.1 devDependencies: '@jest/types': 29.1.2 '@logto/connector-kit': link:../toolkit/connector-kit @@ -585,7 +585,7 @@ importers: '@types/jest': ^29.1.2 '@types/node': ^18.11.18 '@types/pluralize': ^0.0.29 - '@withtyped/server': ^0.4.1 + '@withtyped/server': ^0.5.1 camelcase: ^7.0.0 eslint: ^8.21.0 jest: ^29.1.2 @@ -603,7 +603,7 @@ importers: '@logto/language-kit': link:../toolkit/language-kit '@logto/phrases': link:../phrases '@logto/phrases-ui': link:../phrases-ui - '@withtyped/server': 0.4.1 + '@withtyped/server': 0.5.1 zod: 3.20.2 devDependencies: '@silverhand/eslint-config': 1.3.0_k3lfx77tsvurbevhk73p7ygch4 @@ -4463,21 +4463,21 @@ packages: eslint-visitor-keys: 3.3.0 dev: true - /@withtyped/postgres/0.4.1_@withtyped+server@0.4.1: - resolution: {integrity: sha512-UZtwUieJyj3tHxGgiskBPFefpkFZgv7yzlXFMmuyG2NNu6P8mFnCSp4vuycmGEJnYRLFrxDvLhjx1qpVlD3k6A==} + /@withtyped/postgres/0.5.1_@withtyped+server@0.5.1: + resolution: {integrity: sha512-Le4iIHEc4LRgDn4rjnwbGJ/J15PpqEoltgoZAOhYgnZznKBzkp4W3vxbav29x7IMOvzgum+Jo5HOW1q0kRfROg==} peerDependencies: - '@withtyped/server': ^0.4.1 + '@withtyped/server': ^0.5.1 dependencies: '@types/pg': 8.6.6 - '@withtyped/server': 0.4.1 + '@withtyped/server': 0.5.1 '@withtyped/shared': 0.2.0 pg: 8.8.0 transitivePeerDependencies: - pg-native dev: false - /@withtyped/server/0.4.1: - resolution: {integrity: sha512-QMhFntmF2o/e9dEJi84+RL+BySzV6+uayY1dL372OmYCcYftPUYYCctvpTaz5eWUipqV/hL4FaEpK1YVEKYPHQ==} + /@withtyped/server/0.5.1: + resolution: {integrity: sha512-CR7Y4R2YsUNJ7STEzhJjBjCKIJg49r2Jun5tFuTmmH8IAdHacisWPuKyGMz8o8jnatGTBRJNvc2wjjhg0l8ptw==} dependencies: '@withtyped/shared': 0.2.0 dev: false