From 3f9cc09b62dd44cf8e8da085bf5fcd190e399150 Mon Sep 17 00:00:00 2001 From: Lenny Date: Thu, 10 Apr 2025 19:01:42 -0400 Subject: [PATCH 1/2] fix: tenant s3 credentials fixes and refactor - migration to use correct cache key in pg_notify cache invalidation - fix s3 credentials count, was always returning 0 (and allowing creation of > 50 credentials) - check for valid uuid on delete credential endpoint (used to throw 500 on insert) - add test coverage - refactor s3 credentials into manager pattern similar to jwks --- ...-tenants-s3-credentials-fix-notify-key.sql | 21 + src/http/error-handler.ts | 2 +- src/http/plugins/db.ts | 2 +- src/http/plugins/jwt.ts | 5 +- src/http/plugins/signature-v4.ts | 4 +- src/http/routes/admin/jwks.ts | 2 +- src/http/routes/admin/s3.ts | 14 +- src/internal/concurrency/mutex.ts | 4 +- src/internal/database/jwks-manager/manager.ts | 2 +- .../database/s3-credentials-manager/index.ts | 1 + .../s3-credentials-manager/manager.ts | 126 +++++ .../s3-credentials-manager/store-knex.ts | 59 +++ .../database/s3-credentials-manager/store.ts | 56 +++ src/internal/database/tenant.ts | 162 +----- src/internal/streams/monitor.ts | 2 +- src/internal/testing/seeder/base-seeder.ts | 8 +- .../testing/seeder/knex-persistence.ts | 2 +- src/internal/testing/seeder/persistence.ts | 2 +- src/storage/backend/s3/adapter.ts | 6 +- src/storage/events/webhook.ts | 2 +- src/test/tenant-jwks.test.ts | 103 ++-- src/test/tenant-s3-credentials.test.ts | 468 ++++++++++++++++++ src/test/tenant.test.ts | 52 +- 23 files changed, 882 insertions(+), 223 deletions(-) create mode 100644 migrations/multitenant/0017-tenants-s3-credentials-fix-notify-key.sql create mode 100644 src/internal/database/s3-credentials-manager/index.ts create mode 100644 src/internal/database/s3-credentials-manager/manager.ts create mode 100644 src/internal/database/s3-credentials-manager/store-knex.ts create mode 100644 src/internal/database/s3-credentials-manager/store.ts create mode 100644 src/test/tenant-s3-credentials.test.ts diff --git a/migrations/multitenant/0017-tenants-s3-credentials-fix-notify-key.sql b/migrations/multitenant/0017-tenants-s3-credentials-fix-notify-key.sql new file mode 100644 index 00000000..2f81d837 --- /dev/null +++ b/migrations/multitenant/0017-tenants-s3-credentials-fix-notify-key.sql @@ -0,0 +1,21 @@ +CREATE OR REPLACE FUNCTION tenants_s3_credentials_update_notify_trigger () + RETURNS TRIGGER +AS $$ +BEGIN + PERFORM + pg_notify('tenants_s3_credentials_update', '"' || NEW.tenant_id || ':' || NEW.access_key || '"'); + RETURN NULL; +END; +$$ + LANGUAGE plpgsql; + +CREATE OR REPLACE FUNCTION tenants_s3_credentials_delete_notify_trigger () + RETURNS TRIGGER +AS $$ +BEGIN + PERFORM + pg_notify('tenants_s3_credentials_update', '"' || OLD.tenant_id || ':' || OLD.access_key || '"'); + RETURN NULL; +END; +$$ + LANGUAGE plpgsql; diff --git a/src/http/error-handler.ts b/src/http/error-handler.ts index 344390eb..d5063989 100644 --- a/src/http/error-handler.ts +++ b/src/http/error-handler.ts @@ -64,7 +64,7 @@ export const setErrorHandler = (app: FastifyInstance) => { // Fastify errors if ('statusCode' in error) { const err = error as FastifyError - return reply.status((error as any).statusCode || 500).send({ + return reply.status(err.statusCode || 500).send({ statusCode: `${err.statusCode}`, error: err.name, message: err.message, diff --git a/src/http/plugins/db.ts b/src/http/plugins/db.ts index 7017f152..de5f81b8 100644 --- a/src/http/plugins/db.ts +++ b/src/http/plugins/db.ts @@ -171,7 +171,7 @@ export const migrations = fastifyPlugin( }) if (dbMigrationStrategy === MultitenantMigrationStrategy.ON_REQUEST) { - const migrationsMutex = createMutexByKey() + const migrationsMutex = createMutexByKey() fastify.addHook('preHandler', async (request) => { // migrations are handled via async migrations diff --git a/src/http/plugins/jwt.ts b/src/http/plugins/jwt.ts index 76fd04c7..f6dd1516 100644 --- a/src/http/plugins/jwt.ts +++ b/src/http/plugins/jwt.ts @@ -25,7 +25,7 @@ export const jwt = fastifyPlugin( fastify.decorateRequest('jwt', '') fastify.decorateRequest('jwtPayload', undefined) - fastify.addHook('preHandler', async (request, reply) => { + fastify.addHook('preHandler', async (request) => { request.jwt = (request.headers.authorization || '').replace(BEARER, '') if (!request.jwt && request.routeOptions.config.allowInvalidJwt) { @@ -41,13 +41,14 @@ export const jwt = fastifyPlugin( request.jwtPayload = payload request.owner = payload.sub request.isAuthenticated = true - } catch (err: any) { + } catch (e) { request.jwtPayload = { role: 'anon' } request.isAuthenticated = false if (request.routeOptions.config.allowInvalidJwt) { return } + const err = e as Error throw ERRORS.AccessDenied(err.message, err) } }) diff --git a/src/http/plugins/signature-v4.ts b/src/http/plugins/signature-v4.ts index 313ab566..6bd35c26 100644 --- a/src/http/plugins/signature-v4.ts +++ b/src/http/plugins/signature-v4.ts @@ -1,6 +1,6 @@ import { FastifyInstance, FastifyRequest } from 'fastify' import fastifyPlugin from 'fastify-plugin' -import { getJwtSecret, getS3CredentialsByAccessKey, getTenantConfig } from '@internal/database' +import { getJwtSecret, getTenantConfig, s3CredentialsManager } from '@internal/database' import { ClientSignature, SignatureV4 } from '@storage/protocols/s3' import { signJWT, verifyJWT } from '@internal/auth' import { ERRORS } from '@internal/errors' @@ -160,7 +160,7 @@ async function createServerSignature(tenantId: string, clientSignature: ClientSi } if (isMultitenant) { - const credential = await getS3CredentialsByAccessKey( + const credential = await s3CredentialsManager.getS3CredentialsByAccessKey( tenantId, clientSignature.credentials.accessKey ) diff --git a/src/http/routes/admin/jwks.ts b/src/http/routes/admin/jwks.ts index 47cba6d4..6085b8be 100644 --- a/src/http/routes/admin/jwks.ts +++ b/src/http/routes/admin/jwks.ts @@ -110,7 +110,7 @@ export default async function routes(fastify: FastifyInstance) { } const result = await jwksManager.addJwk(params.tenantId, body.jwk, body.kind) - return reply.send(result) + return reply.status(201).send(result) } ) diff --git a/src/http/routes/admin/s3.ts b/src/http/routes/admin/s3.ts index d7e04213..961112bf 100644 --- a/src/http/routes/admin/s3.ts +++ b/src/http/routes/admin/s3.ts @@ -1,7 +1,9 @@ import { FastifyInstance, RequestGenericInterface } from 'fastify' import apiKey from '../../plugins/apikey' -import { createS3Credentials, deleteS3Credential, listS3Credentials } from '@internal/database' +import { s3CredentialsManager } from '@internal/database' import { FromSchema } from 'json-schema-to-ts' +import { isUuid } from '@storage/limits' +import { ERRORS } from '@internal/errors' const createCredentialsSchema = { description: 'Create S3 Credentials', @@ -88,7 +90,7 @@ export default async function routes(fastify: FastifyInstance) { schema: createCredentialsSchema, }, async (req, reply) => { - const credentials = await createS3Credentials(req.params.tenantId, { + const credentials = await s3CredentialsManager.createS3Credentials(req.params.tenantId, { description: req.body.description, claims: req.body.claims, }) @@ -106,8 +108,7 @@ export default async function routes(fastify: FastifyInstance) { '/:tenantId/credentials', { schema: listCredentialsSchema }, async (req, reply) => { - const credentials = await listS3Credentials(req.params.tenantId) - + const credentials = await s3CredentialsManager.listS3Credentials(req.params.tenantId) return reply.send(credentials) } ) @@ -116,7 +117,10 @@ export default async function routes(fastify: FastifyInstance) { '/:tenantId/credentials', { schema: deleteCredentialsSchema }, async (req, reply) => { - await deleteS3Credential(req.params.tenantId, req.body.id) + if (!isUuid(req.body.id)) { + throw ERRORS.InvalidParameter('id not uuid') + } + await s3CredentialsManager.deleteS3Credential(req.params.tenantId, req.body.id) return reply.code(204).send() } diff --git a/src/internal/concurrency/mutex.ts b/src/internal/concurrency/mutex.ts index b9a58af6..3d76019a 100644 --- a/src/internal/concurrency/mutex.ts +++ b/src/internal/concurrency/mutex.ts @@ -1,9 +1,9 @@ import { Semaphore } from '@shopify/semaphore' -export function createMutexByKey() { +export function createMutexByKey() { const semaphoreMap = new Map() - return async (key: string, fn: () => Promise) => { + return async (key: string, fn: () => Promise) => { let entry = semaphoreMap.get(key) if (!entry) { entry = { semaphore: new Semaphore(1), count: 0 } diff --git a/src/internal/database/jwks-manager/manager.ts b/src/internal/database/jwks-manager/manager.ts index 35b3ddeb..6ebde584 100644 --- a/src/internal/database/jwks-manager/manager.ts +++ b/src/internal/database/jwks-manager/manager.ts @@ -9,7 +9,7 @@ const TENANTS_JWKS_UPDATE_CHANNEL = 'tenants_jwks_update' const JWK_KIND_STORAGE_URL_SIGNING = 'storage-url-signing-key' const JWK_KID_SEPARATOR = '_' -const tenantJwksMutex = createMutexByKey() +const tenantJwksMutex = createMutexByKey() const tenantJwksConfigCache = new Map() function createJwkKid({ kind, id }: { id: string; kind: string }): string { diff --git a/src/internal/database/s3-credentials-manager/index.ts b/src/internal/database/s3-credentials-manager/index.ts new file mode 100644 index 00000000..0f1b5674 --- /dev/null +++ b/src/internal/database/s3-credentials-manager/index.ts @@ -0,0 +1 @@ +export * from './manager' diff --git a/src/internal/database/s3-credentials-manager/manager.ts b/src/internal/database/s3-credentials-manager/manager.ts new file mode 100644 index 00000000..497fa6c0 --- /dev/null +++ b/src/internal/database/s3-credentials-manager/manager.ts @@ -0,0 +1,126 @@ +import crypto from 'node:crypto' +import { LRUCache } from 'lru-cache' +import objectSizeOf from 'object-sizeof' +import { S3Credentials, S3CredentialsManagerStore, S3CredentialsRaw } from './store' +import { createMutexByKey } from '@internal/concurrency' +import { ERRORS } from '@internal/errors' +import { getConfig } from '../../../config' +import { decrypt, encrypt } from '@internal/auth' +import { PubSubAdapter } from '@internal/pubsub' + +const TENANTS_S3_CREDENTIALS_UPDATE_CHANNEL = 'tenants_s3_credentials_update' + +const tenantS3CredentialsCache = new LRUCache({ + maxSize: 1024 * 1024 * 50, // 50MB + ttl: 1000 * 60 * 60, // 1 hour + sizeCalculation: (value) => objectSizeOf(value), + updateAgeOnGet: true, + allowStale: false, +}) + +const s3CredentialsMutex = createMutexByKey() + +export class S3CredentialsManager { + private dbServiceRole: string + + constructor(private storage: S3CredentialsManagerStore) { + const { dbServiceRole } = getConfig() + this.dbServiceRole = dbServiceRole + } + + /** + * Keeps the in memory config cache up to date + */ + async listenForTenantUpdate(pubSub: PubSubAdapter): Promise { + await pubSub.subscribe(TENANTS_S3_CREDENTIALS_UPDATE_CHANNEL, (cacheKey) => { + tenantS3CredentialsCache.delete(cacheKey) + }) + } + + /** + * Create S3 Credential for a tenant + * @param tenantId + * @param data + */ + async createS3Credentials( + tenantId: string, + data: { description: string; claims?: S3Credentials['claims'] } + ) { + const existingCount = await this.countS3Credentials(tenantId) + + if (existingCount >= 50) { + throw ERRORS.MaximumCredentialsLimit() + } + + const accessKey = crypto.randomBytes(32).toString('hex').slice(0, 32) + const secretKey = crypto.randomBytes(64).toString('hex').slice(0, 64) + + if (data.claims) { + delete data.claims.iss + delete data.claims.issuer + delete data.claims.exp + delete data.claims.iat + } + + const claims = { + ...(data.claims || {}), + role: data.claims?.role ?? this.dbServiceRole, + issuer: `supabase.storage.${tenantId}`, + sub: data.claims?.sub, + } + + const id = await this.storage.insert(tenantId, { + description: data.description, + claims, + accessKey, + secretKey: encrypt(secretKey), + }) + + return { + id, + access_key: accessKey, + secret_key: secretKey, + } + } + + async getS3CredentialsByAccessKey(tenantId: string, accessKey: string): Promise { + const cacheKey = `${tenantId}:${accessKey}` + const cachedCredentials = tenantS3CredentialsCache.get(cacheKey) + + if (cachedCredentials) { + return cachedCredentials + } + + return s3CredentialsMutex(cacheKey, async () => { + const cachedCredentials = tenantS3CredentialsCache.get(cacheKey) + + if (cachedCredentials) { + return cachedCredentials + } + + const data = await this.storage.getOneByAccessKey(tenantId, accessKey) + + if (!data) { + throw ERRORS.MissingS3Credentials() + } + + data.secretKey = decrypt(data.secretKey) + + tenantS3CredentialsCache.set(cacheKey, data) + + return data + }) + } + + deleteS3Credential(tenantId: string, credentialId: string): Promise { + return this.storage.delete(tenantId, credentialId) + } + + listS3Credentials(tenantId: string): Promise { + return this.storage.list(tenantId) + } + + async countS3Credentials(tenantId: string) { + return this.storage.count(tenantId) + } +} diff --git a/src/internal/database/s3-credentials-manager/store-knex.ts b/src/internal/database/s3-credentials-manager/store-knex.ts new file mode 100644 index 00000000..1abc4caf --- /dev/null +++ b/src/internal/database/s3-credentials-manager/store-knex.ts @@ -0,0 +1,59 @@ +import { Knex } from 'knex' +import { + S3Credentials, + S3CredentialsManagerStore, + S3CredentialsRaw, + S3CredentialWithDescription, +} from './store' + +export class S3CredentialsManagerStoreKnex implements S3CredentialsManagerStore { + constructor(private knex: Knex) {} + + async insert(tenantId: string, credential: S3CredentialWithDescription): Promise { + const credentials = await this.knex + .table('tenants_s3_credentials') + .insert({ + tenant_id: tenantId, + description: credential.description, + access_key: credential.accessKey, + secret_key: credential.secretKey, + claims: JSON.stringify(credential.claims), + }) + .returning('id') + return credentials[0].id + } + + list(tenantId: string): Promise { + return this.knex + .table('tenants_s3_credentials') + .select('id', 'description', 'access_key', 'created_at') + .where('tenant_id', tenantId) + .orderBy('created_at', 'asc') + } + + getOneByAccessKey(tenantId: string, accessKey: string): Promise { + return this.knex + .table('tenants_s3_credentials') + .select({ accessKey: 'access_key', secretKey: 'secret_key', claims: 'claims' }) + .where('tenant_id', tenantId) + .where('access_key', accessKey) + .first() + } + + async count(tenantId: string): Promise { + const data = await this.knex + .table('tenants_s3_credentials') + .count<{ count: number }>('id') + .where('tenant_id', tenantId) + .first() + return Number(data?.count || 0) + } + + delete(tenantId: string, credentialId: string): Promise { + return this.knex + .table('tenants_s3_credentials') + .where('tenant_id', tenantId) + .where('id', credentialId) + .delete() + } +} diff --git a/src/internal/database/s3-credentials-manager/store.ts b/src/internal/database/s3-credentials-manager/store.ts new file mode 100644 index 00000000..e5c92c4d --- /dev/null +++ b/src/internal/database/s3-credentials-manager/store.ts @@ -0,0 +1,56 @@ +export interface S3Credentials { + accessKey: string + secretKey: string + claims: { role: string; sub?: string; [key: string]: unknown } +} + +export interface S3CredentialWithDescription extends S3Credentials { + description: string +} + +export interface S3CredentialsRaw { + id: string + description: string + access_key: string + created_at: string +} + +export interface S3CredentialsManagerStore { + /** + * Inserts a new credential and returns the id + * + * @param tenantId + */ + insert(tenantId: string, credential: S3CredentialWithDescription): Promise + + /** + * List all credentials for the specified tenant + * Returns data in the database style (snake case) format because the endpoint is expected to return data in this format + * + * @param tenantId + */ + list(tenantId: string): Promise + + /** + * Get one credential for the specified tenant / access key + * + * @param tenantId + * @param accessKey + */ + getOneByAccessKey(tenantId: string, accessKey: string): Promise + + /** + * Gets the count of credentials for the specified tenant + * + * @param tenantId + */ + count(tenantId: string): Promise + + /** + * Deletes a credential and returns the count of items deleted + * + * @param tenantId + * @param credentialId + */ + delete(tenantId: string, credentialId: string): Promise +} diff --git a/src/internal/database/tenant.ts b/src/internal/database/tenant.ts index bc5a757a..f5a81120 100644 --- a/src/internal/database/tenant.ts +++ b/src/internal/database/tenant.ts @@ -1,16 +1,15 @@ -import crypto from 'node:crypto' import { getConfig, JwksConfig, JwksConfigKey, JwksConfigKeyOCT } from '../../config' -import { decrypt, encrypt, verifyJWT } from '../auth' +import { decrypt, verifyJWT } from '../auth' import { multitenantKnex } from './multitenant-db' import { JwtPayload } from 'jsonwebtoken' import { PubSubAdapter } from '../pubsub' import { createMutexByKey } from '../concurrency' -import { LRUCache } from 'lru-cache' -import objectSizeOf from 'object-sizeof' import { ERRORS } from '@internal/errors' import { DBMigration } from '@internal/database/migrations' import { JWKSManager } from './jwks-manager' import { JWKSManagerStoreKnex } from './jwks-manager/store-knex' +import { S3CredentialsManagerStoreKnex } from './s3-credentials-manager/store-knex' +import { S3CredentialsManager } from './s3-credentials-manager' interface TenantConfig { anonKey?: string @@ -51,28 +50,16 @@ export enum TenantMigrationStatus { FAILED_STALE = 'FAILED_STALE', } -interface S3Credentials { - accessKey: string - secretKey: string - claims: { role: string; sub?: string; [key: string]: unknown } -} - const { isMultitenant, dbServiceRole, serviceKey, jwtSecret } = getConfig() const tenantConfigCache = new Map() -const tenantS3CredentialsCache = new LRUCache({ - maxSize: 1024 * 1024 * 50, // 50MB - ttl: 1000 * 60 * 60, // 1 hour - sizeCalculation: (value) => objectSizeOf(value), - updateAgeOnGet: true, - allowStale: false, -}) - -const tenantMutex = createMutexByKey() -const s3CredentialsMutex = createMutexByKey() +const tenantMutex = createMutexByKey() export const jwksManager = new JWKSManager(new JWKSManagerStoreKnex(multitenantKnex)) +export const s3CredentialsManager = new S3CredentialsManager( + new S3CredentialsManagerStoreKnex(multitenantKnex) +) const singleTenantServiceKey: | { @@ -107,15 +94,15 @@ export async function getTenantConfig(tenantId: string): Promise { } if (tenantConfigCache.has(tenantId)) { - return tenantConfigCache.get(tenantId) as TenantConfig + return tenantConfigCache.get(tenantId)! } return tenantMutex(tenantId, async () => { if (tenantConfigCache.has(tenantId)) { - return tenantConfigCache.get(tenantId) as TenantConfig + return tenantConfigCache.get(tenantId)! } - const tenant = await multitenantKnex('tenants').first().where('id', tenantId) + const tenant = await multitenantKnex.table('tenants').first().where('id', tenantId) if (!tenant) { throw ERRORS.MissingTenantConfig(tenantId) } @@ -173,7 +160,7 @@ export async function getTenantConfig(tenantId: string): Promise { } tenantConfigCache.set(tenantId, config) - return tenantConfigCache.get(tenantId) + return tenantConfigCache.get(tenantId)! }) } @@ -249,7 +236,6 @@ export async function getFeatures(tenantId: string): Promise { } const TENANTS_UPDATE_CHANNEL = 'tenants_update' -const TENANTS_S3_CREDENTIALS_UPDATE_CHANNEL = 'tenants_s3_credentials_update' /** * Keeps the in memory config cache up to date @@ -258,130 +244,6 @@ export async function listenForTenantUpdate(pubSub: PubSubAdapter): Promise { tenantConfigCache.delete(cacheKey) }) - await pubSub.subscribe(TENANTS_S3_CREDENTIALS_UPDATE_CHANNEL, (cacheKey) => { - tenantS3CredentialsCache.delete(cacheKey) - }) - + await s3CredentialsManager.listenForTenantUpdate(pubSub) await jwksManager.listenForTenantUpdate(pubSub) } - -/** - * Create S3 Credential for a tenant - * @param tenantId - * @param data - */ -export async function createS3Credentials( - tenantId: string, - data: { description: string; claims?: S3Credentials['claims'] } -) { - const existingCount = await countS3Credentials(tenantId) - - if (existingCount >= 50) { - throw ERRORS.MaximumCredentialsLimit() - } - - const secretAccessKeyId = crypto.randomBytes(32).toString('hex').slice(0, 32) - const secretAccessKey = crypto.randomBytes(64).toString('hex').slice(0, 64) - - if (data.claims) { - delete data.claims.iss - delete data.claims.issuer - delete data.claims.exp - delete data.claims.iat - } - - data.claims = { - ...(data.claims || {}), - role: data.claims?.role ?? dbServiceRole, - issuer: `supabase.storage.${tenantId}`, - sub: data.claims?.sub, - } - - const credentials = await multitenantKnex - .table('tenants_s3_credentials') - .insert({ - tenant_id: tenantId, - description: data.description, - access_key: secretAccessKeyId, - secret_key: encrypt(secretAccessKey), - claims: JSON.stringify(data.claims), - }) - .returning('id') - - return { - id: credentials[0].id, - access_key: secretAccessKeyId, - secret_key: secretAccessKey, - } -} - -export async function getS3CredentialsByAccessKey( - tenantId: string, - accessKey: string -): Promise { - const cacheKey = `${tenantId}:${accessKey}` - const cachedCredentials = tenantS3CredentialsCache.get(cacheKey) - - if (cachedCredentials) { - return cachedCredentials - } - - return s3CredentialsMutex(cacheKey, async () => { - const cachedCredentials = tenantS3CredentialsCache.get(cacheKey) - - if (cachedCredentials) { - return cachedCredentials - } - - const data = await multitenantKnex - .table('tenants_s3_credentials') - .select('access_key', 'secret_key', 'claims') - .where('tenant_id', tenantId) - .where('access_key', accessKey) - .first() - - if (!data) { - throw ERRORS.MissingS3Credentials() - } - - const secretKey = decrypt(data.secret_key) - - tenantS3CredentialsCache.set(cacheKey, { - accessKey: data.access_key, - secretKey: secretKey, - claims: data.claims, - }) - - return { - accessKey: data.access_key, - secretKey: secretKey, - claims: data.claims, - } - }) -} - -export function deleteS3Credential(tenantId: string, credentialId: string) { - return multitenantKnex - .table('tenants_s3_credentials') - .where('tenant_id', tenantId) - .where('id', credentialId) - .delete() - .returning('id') -} - -export function listS3Credentials(tenantId: string) { - return multitenantKnex - .table('tenants_s3_credentials') - .select('id', 'description', 'access_key', 'created_at') - .where('tenant_id', tenantId) - .orderBy('created_at', 'asc') -} - -export async function countS3Credentials(tenantId: string) { - const data = await multitenantKnex - .table('tenants_s3_credentials') - .count<{ count: number }>('id') - .where('tenant_id', tenantId) - - return Number(data?.count || 0) -} diff --git a/src/internal/streams/monitor.ts b/src/internal/streams/monitor.ts index 4a26f797..7db5dac6 100644 --- a/src/internal/streams/monitor.ts +++ b/src/internal/streams/monitor.ts @@ -12,7 +12,7 @@ export function monitorStream(dataStream: Readable) { const byteCounter = createByteCounterStream() const span = trace.getActiveSpan() - let measures: any[] = [] + let measures: object[] = [] // Handle the 'speed' event to collect speed measurements speedMonitor.on('speed', (bps) => { diff --git a/src/internal/testing/seeder/base-seeder.ts b/src/internal/testing/seeder/base-seeder.ts index 032343ab..9f03eae7 100644 --- a/src/internal/testing/seeder/base-seeder.ts +++ b/src/internal/testing/seeder/base-seeder.ts @@ -2,7 +2,7 @@ import { Persistence } from './persistence' export abstract class Seeder { protected persistence: Persistence - protected records: Map + protected records: Map constructor(persistence: Persistence) { this.persistence = persistence @@ -13,7 +13,7 @@ export abstract class Seeder { * Retrieves all collected records. * @returns A map of table names to their respective records. */ - getAllRecords(): Map { + getAllRecords(): Map { return this.records } @@ -52,7 +52,7 @@ export abstract class Seeder { * @param table - The table name. * @param records - The records to add. */ - protected addRecords(table: string, records: T[]): void { + protected addRecords(table: string, records: T[]): void { if (!this.records.has(table)) { this.records.set(table, []) } @@ -65,7 +65,7 @@ export abstract class Seeder { * @param bindings - Optional bindings for parameterized queries. * @returns The result of the query. */ - protected async rawQuery(query: string, bindings?: any[]): Promise { + protected async rawQuery(query: string, bindings?: object[]): Promise { return this.persistence.rawQuery(query, bindings) } diff --git a/src/internal/testing/seeder/knex-persistence.ts b/src/internal/testing/seeder/knex-persistence.ts index 0942de26..d82e589f 100644 --- a/src/internal/testing/seeder/knex-persistence.ts +++ b/src/internal/testing/seeder/knex-persistence.ts @@ -36,7 +36,7 @@ export class KnexPersistence implements Persistence { } } - async rawQuery(query: string, bindings: any[] = []): Promise { + async rawQuery(query: string, bindings: object[] = []): Promise { if (this.trx) { return this.trx.raw(query, bindings) } diff --git a/src/internal/testing/seeder/persistence.ts b/src/internal/testing/seeder/persistence.ts index 7cc6255b..96ecf33f 100644 --- a/src/internal/testing/seeder/persistence.ts +++ b/src/internal/testing/seeder/persistence.ts @@ -3,5 +3,5 @@ export interface Persistence { beginTransaction(): Promise commitTransaction(): Promise rollbackTransaction(): Promise - rawQuery(query: string, bindings?: any[]): Promise + rawQuery(query: string, bindings?: object[]): Promise } diff --git a/src/storage/backend/s3/adapter.ts b/src/storage/backend/s3/adapter.ts index fdde4da3..257dedab 100644 --- a/src/storage/backend/s3/adapter.ts +++ b/src/storage/backend/s3/adapter.ts @@ -246,7 +246,7 @@ export class S3Backend implements StorageBackendAdapter { eTag: data.CopyObjectResult?.ETag || '', lastModified: data.CopyObjectResult?.LastModified, } - } catch (e: any) { + } catch (e) { throw StorageBackendError.fromError(e) } } @@ -294,7 +294,7 @@ export class S3Backend implements StorageBackendAdapter { keys, nextToken: data.NextContinuationToken, } - } catch (e: any) { + } catch (e) { throw StorageBackendError.fromError(e) } } @@ -348,7 +348,7 @@ export class S3Backend implements StorageBackendAdapter { httpStatusCode: data.$metadata.httpStatusCode || 200, size: data.ContentLength || 0, } - } catch (e: any) { + } catch (e) { throw StorageBackendError.fromError(e) } } diff --git a/src/storage/events/webhook.ts b/src/storage/events/webhook.ts index 2676e933..48154563 100644 --- a/src/storage/events/webhook.ts +++ b/src/storage/events/webhook.ts @@ -114,7 +114,7 @@ export class Webhook extends BaseEvent { } catch (e) { logger.error( { - error: (e as any)?.message, + error: (e as Error)?.message, jodId: job.id, type: 'event', event: job.data.event.type, diff --git a/src/test/tenant-jwks.test.ts b/src/test/tenant-jwks.test.ts index 8fdf774a..f6a97107 100644 --- a/src/test/tenant-jwks.test.ts +++ b/src/test/tenant-jwks.test.ts @@ -164,7 +164,7 @@ describe('Tenant jwks configs', () => { // jsonwebtoken does not support OKP (ed25519/Ed448) keys yet expect(response.statusCode).toBe(400) } else { - expect(response.statusCode).toBe(200) + expect(response.statusCode).toBe(201) const data = response.json<{ kid: string }>() expect(data.kid).toBeTruthy() expect(data.kid.startsWith(kind)).toBe(true) @@ -285,14 +285,17 @@ describe('Tenant jwks configs', () => { test('Config always retrieves concurrent requests from cache', async () => { const listActiveSpy = jest.spyOn(jwksManager['storage'], 'listActive') - const results = await Promise.all([ - jwksManager.getJwksTenantConfig(tenantId), - jwksManager.getJwksTenantConfig(tenantId), - jwksManager.getJwksTenantConfig(tenantId), - ]) - expect(listActiveSpy).toHaveBeenCalledTimes(1) - results.forEach((result, i) => expect(result).toEqual(results[i === 0 ? 1 : 0])) - listActiveSpy.mockRestore() + try { + const results = await Promise.all([ + jwksManager.getJwksTenantConfig(tenantId), + jwksManager.getJwksTenantConfig(tenantId), + jwksManager.getJwksTenantConfig(tenantId), + ]) + expect(listActiveSpy).toHaveBeenCalledTimes(1) + results.forEach((result, i) => expect(result).toEqual(results[i === 0 ? 1 : 0])) + } finally { + listActiveSpy.mockRestore() + } }) test('Generate all jwks status', async () => { @@ -326,25 +329,27 @@ describe('Tenant jwks configs', () => { const queueSpyAwaiter = new Promise((resolve) => { queueInsertSpy.mockImplementationOnce((...args) => resolve(args)) }) - - const response = await adminApp.inject({ - method: 'POST', - url: `/tenants/jwks/generate-all-missing`, - payload: {}, - headers: { - apikey: process.env.ADMIN_API_KEYS, - }, - }) - expect(response.statusCode).toBe(200) - const startData = response.json<{ started: boolean }>() - expect(startData.started).toBe(true) - - await queueSpyAwaiter - expect(queueInsertSpy).toHaveBeenCalledTimes(1) - const [[callArg]] = queueInsertSpy.mock.calls - expect(callArg).toHaveLength(1) - expect(callArg[0]).toMatchObject({ data: { tenantId }, name: 'tenants-jwks-create' }) - queueInsertSpy.mockRestore() + try { + const response = await adminApp.inject({ + method: 'POST', + url: `/tenants/jwks/generate-all-missing`, + payload: {}, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(response.statusCode).toBe(200) + const startData = response.json<{ started: boolean }>() + expect(startData.started).toBe(true) + + await queueSpyAwaiter + expect(queueInsertSpy).toHaveBeenCalledTimes(1) + const [[callArg]] = queueInsertSpy.mock.calls + expect(callArg).toHaveLength(1) + expect(callArg[0]).toMatchObject({ data: { tenantId }, name: 'tenants-jwks-create' }) + } finally { + queueInsertSpy.mockRestore() + } }) test('Generate all jwks when already running', async () => { @@ -352,32 +357,38 @@ describe('Tenant jwks configs', () => { .spyOn(UrlSigningJwkGenerator, 'getGenerationStatus') .mockReturnValueOnce({ running: true, sent: 99 }) - const response = await adminApp.inject({ - method: 'POST', - url: `/tenants/jwks/generate-all-missing`, - payload: {}, - headers: { - apikey: process.env.ADMIN_API_KEYS, - }, - }) - expect(response.statusCode).toBe(400) - expect(statusSpy).toHaveBeenCalledTimes(1) - statusSpy.mockRestore() + try { + const response = await adminApp.inject({ + method: 'POST', + url: `/tenants/jwks/generate-all-missing`, + payload: {}, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(response.statusCode).toBe(400) + expect(statusSpy).toHaveBeenCalledTimes(1) + } finally { + statusSpy.mockRestore() + } }) test('Ensure list tenants exits before yield if no items are returned', async () => { const listTenantsSpy = jest .spyOn(jwksManager['storage'], 'listTenantsWithoutKindPaginated') .mockResolvedValue([]) - const result = jwksManager.listTenantsMissingUrlSigningJwk(new AbortController().signal) + try { + const result = jwksManager.listTenantsMissingUrlSigningJwk(new AbortController().signal) - let iterations = 0 - for await (const _ of result) { - iterations++ + let iterations = 0 + for await (const _ of result) { + iterations++ + } + expect(iterations).toBe(0) + expect(listTenantsSpy).toHaveBeenCalledTimes(1) + } finally { + listTenantsSpy.mockRestore() } - expect(iterations).toBe(0) - expect(listTenantsSpy).toHaveBeenCalledTimes(1) - listTenantsSpy.mockRestore() }) test('Should use url signing jwk and fall back to old jwt secret when the jwk is removed', async () => { diff --git a/src/test/tenant-s3-credentials.test.ts b/src/test/tenant-s3-credentials.test.ts new file mode 100644 index 00000000..ab74149f --- /dev/null +++ b/src/test/tenant-s3-credentials.test.ts @@ -0,0 +1,468 @@ +'use strict' +import { getConfig, mergeConfig } from '../config' + +const { multitenantDatabaseUrl } = getConfig() +mergeConfig({ + pgQueueEnable: true, + isMultitenant: true, +}) + +import dotenv from 'dotenv' +import * as migrate from '../internal/database/migrations/migrate' +import { multitenantKnex } from '../internal/database/multitenant-db' +import { adminApp } from './common' +import { s3CredentialsManager } from '@internal/database' +import { listenForTenantUpdate } from '@internal/database' +import { PostgresPubSub } from '@internal/pubsub' +import { encrypt, signJWT } from '@internal/auth' + +dotenv.config({ path: '.env.test' }) + +const tenantId = 'abc123s3' + +const pubSub = new PostgresPubSub(multitenantDatabaseUrl!) + +// returns a promise that resolves the next time the jwk cache is invalidated +function createS3CredentialsChangeAwaiter(): Promise { + return new Promise((resolve) => { + pubSub.subscriber.notifications.once('tenants_s3_credentials_update', resolve) + }) +} + +beforeAll(async () => { + await migrate.runMultitenantMigrations() + await pubSub.start() + await listenForTenantUpdate(pubSub) + jest.spyOn(migrate, 'runMigrationsOnTenant').mockResolvedValue() +}) + +beforeEach(async () => { + const jwtSecret = 'zzzzzzzzzzz-s3' + const serviceKey = await signJWT({}, jwtSecret, 100) + await adminApp.inject({ + method: 'POST', + url: `/tenants/${tenantId}`, + payload: { + anonKey: 'aaaaaaa', + databaseUrl: 'bbbbbbb', + jwtSecret, + serviceKey, + }, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) +}) + +afterEach(async () => { + await adminApp.inject({ + method: 'DELETE', + url: `/tenants/${tenantId}`, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) +}) + +afterAll(async () => { + await pubSub.close() + await multitenantKnex.destroy() +}) + +describe('Tenant S3 credentials', () => { + test('Add s3 credential without description', async () => { + const response = await adminApp.inject({ + method: 'POST', + url: `/s3/${tenantId}/credentials`, + payload: {}, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(response.statusCode).toBe(400) + }) + + test('Add s3 credential without claim', async () => { + const response = await adminApp.inject({ + method: 'POST', + url: `/s3/${tenantId}/credentials`, + payload: { description: 'blah blah blah' }, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(response.statusCode).toBe(201) + const createJson = await response.json() + expect(Object.keys(createJson)).toHaveLength(4) + expect(createJson.id).toBeTruthy() + expect(createJson.description).toBeTruthy() + expect(createJson.access_key).toBeTruthy() + expect(createJson.secret_key).toBeTruthy() + + // check that item was added + const getResponse = await adminApp.inject({ + method: 'GET', + url: `/s3/${tenantId}/credentials`, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(getResponse.statusCode).toBe(200) + const getJson = await getResponse.json() + expect(getJson).toHaveLength(1) + expect(Object.keys(getJson[0])).toHaveLength(4) + expect(getJson[0]).toMatchObject({ + id: createJson.id, + description: createJson.description, + access_key: createJson.access_key, + created_at: expect.any(String), + }) + }) + + test('Add more than max allowed credentials', async () => { + for (let i = 0; i < 50; i++) { + const response = await adminApp.inject({ + method: 'POST', + url: `/s3/${tenantId}/credentials`, + payload: { description: 'blah blah blah' + i }, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(response.statusCode).toBe(201) + } + const responseFailure = await adminApp.inject({ + method: 'POST', + url: `/s3/${tenantId}/credentials`, + payload: { description: 'one too many' }, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(responseFailure.statusCode).toBe(400) + }) + + test('Add s3 credential with claim', async () => { + const knexTableSpy = jest.spyOn(multitenantKnex, 'table') + try { + const claimKept = { + some: 'other', + stuff: 'here', + role: 'king of the world', + sub: 'marine', + } + const claimRemoved = { + iss: 'abc', + exp: 54321, + iat: 12345, + } + const claims = { + issuer: 'def', + ...claimRemoved, + ...claimKept, + } + const response = await adminApp.inject({ + method: 'POST', + url: `/s3/${tenantId}/credentials`, + payload: { description: 'blah blah blah', claims }, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(response.statusCode).toBe(201) + const createJson = await response.json() + expect(Object.keys(createJson)).toHaveLength(4) + expect(createJson.id).toBeTruthy() + expect(createJson.description).toBeTruthy() + expect(createJson.access_key).toBeTruthy() + expect(createJson.secret_key).toBeTruthy() + expect(knexTableSpy).toHaveBeenCalledTimes(2) // insert and count + + // check that the claims were stored correctly + const keyResult = await s3CredentialsManager.getS3CredentialsByAccessKey( + tenantId, + createJson.access_key + ) + // ensure it was loaded from the database + expect(knexTableSpy).toHaveBeenCalledWith('tenants_s3_credentials') + expect(knexTableSpy).toHaveBeenCalledTimes(3) + expect(keyResult).toMatchObject({ + accessKey: createJson.access_key, + secretKey: createJson.secret_key, + claims: { + issuer: `supabase.storage.${tenantId}`, + ...claimKept, + }, + }) + Object.keys(claimRemoved).forEach((k) => expect(k in keyResult.claims).toBe(false)) + + // load again and ensure it was loaded from cache and not the database + const cacheResult = await s3CredentialsManager.getS3CredentialsByAccessKey( + tenantId, + createJson.access_key + ) + expect(knexTableSpy).toHaveBeenCalledTimes(3) + expect(cacheResult).toMatchObject(keyResult) + } finally { + knexTableSpy.mockRestore() + } + }) + + test('Delete s3 credential with missing payload', async () => { + const deleteResponse = await adminApp.inject({ + method: 'DELETE', + url: `/s3/${tenantId}/credentials`, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(deleteResponse.statusCode).toBe(400) + }) + + test('Delete s3 credential with invalid id', async () => { + const deleteResponse = await adminApp.inject({ + method: 'DELETE', + url: `/s3/${tenantId}/credentials`, + payload: { id: 'abc123' }, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(deleteResponse.statusCode).toBe(400) + }) + + test('Delete s3 credential with not found id', async () => { + const deleteResponse = await adminApp.inject({ + method: 'DELETE', + url: `/s3/${tenantId}/credentials`, + payload: { id: '59e0ddab-3e41-451c-bc42-f8bb1387381d' }, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(deleteResponse.statusCode).toBe(204) + }) + + test('Delete s3 credential', async () => { + const response = await adminApp.inject({ + method: 'POST', + url: `/s3/${tenantId}/credentials`, + payload: { description: 'blah blah blah' }, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(response.statusCode).toBe(201) + const createJson = await response.json() + expect(Object.keys(createJson)).toHaveLength(4) + expect(createJson.id).toBeTruthy() + expect(createJson.description).toBeTruthy() + expect(createJson.access_key).toBeTruthy() + expect(createJson.secret_key).toBeTruthy() + + // check that item was added + const getResponse = await adminApp.inject({ + method: 'GET', + url: `/s3/${tenantId}/credentials`, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(getResponse.statusCode).toBe(200) + const getJson = await getResponse.json() + expect(getJson).toHaveLength(1) + expect(Object.keys(getJson[0])).toHaveLength(4) + expect(getJson[0]).toMatchObject({ + id: createJson.id, + description: createJson.description, + access_key: createJson.access_key, + created_at: expect.any(String), + }) + + // delete item + const deleteResponse = await adminApp.inject({ + method: 'DELETE', + url: `/s3/${tenantId}/credentials`, + payload: { id: createJson.id }, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(deleteResponse.statusCode).toBe(204) + + // check that item was deleted + const getResponse2 = await adminApp.inject({ + method: 'GET', + url: `/s3/${tenantId}/credentials`, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(getResponse2.statusCode).toBe(200) + const getJson2 = await getResponse2.json() + expect(getJson2).toHaveLength(0) + }) + + test('Config always retrieves concurrent requests from cache', async () => { + const getByKeySpy = jest.spyOn(s3CredentialsManager['storage'], 'getOneByAccessKey') + try { + const response = await adminApp.inject({ + method: 'POST', + url: `/s3/${tenantId}/credentials`, + payload: { description: 'blah blah blah' }, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(response.statusCode).toBe(201) + const createJson = await response.json() + + const results = await Promise.all([ + s3CredentialsManager.getS3CredentialsByAccessKey(tenantId, createJson.access_key), + s3CredentialsManager.getS3CredentialsByAccessKey(tenantId, createJson.access_key), + s3CredentialsManager.getS3CredentialsByAccessKey(tenantId, createJson.access_key), + ]) + expect(getByKeySpy).toHaveBeenCalledTimes(1) + results.forEach((result, i) => expect(result).toEqual(results[i === 0 ? 1 : 0])) + expect(results[0].accessKey).toBe(createJson.access_key) + } finally { + getByKeySpy.mockRestore() + } + }) + + test('Ensure cache is cleared on delete', async () => { + const knexTableSpy = jest.spyOn(multitenantKnex, 'table') + const claims = { + issuer: `supabase.storage.${tenantId}`, + role: 'service_role', + } + try { + const response = await adminApp.inject({ + method: 'POST', + url: `/s3/${tenantId}/credentials`, + payload: { description: 'blah blah blah' }, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(response.statusCode).toBe(201) + const createJson = await response.json() + expect(knexTableSpy).toHaveBeenCalledTimes(2) // create and count + + // check that the claims were stored correctly + const keyResult = await s3CredentialsManager.getS3CredentialsByAccessKey( + tenantId, + createJson.access_key + ) + // ensure it was loaded from the database + expect(knexTableSpy).toHaveBeenCalledWith('tenants_s3_credentials') + expect(knexTableSpy).toHaveBeenCalledTimes(3) + expect(keyResult).toEqual({ + accessKey: createJson.access_key, + secretKey: createJson.secret_key, + claims, + }) + + // load again and ensure it was loaded from cache and not the database + const cacheResult = await s3CredentialsManager.getS3CredentialsByAccessKey( + tenantId, + createJson.access_key + ) + expect(knexTableSpy).toHaveBeenCalledTimes(3) + expect(cacheResult).toEqual(keyResult) + + const configAwaiter = createS3CredentialsChangeAwaiter() + + // delete item + const deleteResponse = await adminApp.inject({ + method: 'DELETE', + url: `/s3/${tenantId}/credentials`, + payload: { id: createJson.id }, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(deleteResponse.statusCode).toBe(204) + expect(knexTableSpy).toHaveBeenCalledTimes(4) + + const cacheKey = await configAwaiter + expect(cacheKey).toBe(tenantId + ':' + cacheResult.accessKey) + + // if cache is updated this should throw because it doesn't exist + await expect( + s3CredentialsManager.getS3CredentialsByAccessKey(tenantId, createJson.access_key) + ).rejects.toThrow('The Access Key Id you provided does not exist in our records.') + expect(knexTableSpy).toHaveBeenCalledWith('tenants_s3_credentials') + expect(knexTableSpy).toHaveBeenCalledTimes(5) + } finally { + knexTableSpy.mockRestore() + } + }) + + test('Ensure cache is cleared on update', async () => { + const knexTableSpy = jest.spyOn(multitenantKnex, 'table') + const claims = { + issuer: `supabase.storage.${tenantId}`, + role: 'service_role', + } + try { + const response = await adminApp.inject({ + method: 'POST', + url: `/s3/${tenantId}/credentials`, + payload: { description: 'blah blah blah' }, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + expect(response.statusCode).toBe(201) + const createJson = await response.json() + expect(knexTableSpy).toHaveBeenCalledTimes(2) // create and count + + // check that the claims were stored correctly + const keyResult = await s3CredentialsManager.getS3CredentialsByAccessKey( + tenantId, + createJson.access_key + ) + // ensure it was loaded from the database + expect(knexTableSpy).toHaveBeenCalledWith('tenants_s3_credentials') + expect(knexTableSpy).toHaveBeenCalledTimes(3) + expect(keyResult).toEqual({ + accessKey: createJson.access_key, + secretKey: createJson.secret_key, + claims, + }) + + // load again and ensure it was loaded from cache and not the database + const cacheResult = await s3CredentialsManager.getS3CredentialsByAccessKey( + tenantId, + createJson.access_key + ) + expect(knexTableSpy).toHaveBeenCalledTimes(3) + expect(cacheResult).toEqual(keyResult) + + const configAwaiter = createS3CredentialsChangeAwaiter() + + // update item + const secretKey = 'zzzzzzzzzzzzzzzzz' + await multitenantKnex + .table('tenants_s3_credentials') + .update({ secret_key: encrypt(secretKey) }) + .where('id', createJson.id) + expect(knexTableSpy).toHaveBeenCalledTimes(4) + + const cacheKey = await configAwaiter + expect(cacheKey).toBe(tenantId + ':' + cacheResult.accessKey) + + // load again and ensure it was loaded from cache and not the database + const cacheResult2 = await s3CredentialsManager.getS3CredentialsByAccessKey( + tenantId, + createJson.access_key + ) + expect(knexTableSpy).toHaveBeenCalledWith('tenants_s3_credentials') + expect(knexTableSpy).toHaveBeenCalledTimes(5) + expect(cacheResult2).toEqual({ ...keyResult, secretKey }) + } finally { + knexTableSpy.mockRestore() + } + }) +}) diff --git a/src/test/tenant.test.ts b/src/test/tenant.test.ts index 2c7f20c8..44038ea5 100644 --- a/src/test/tenant.test.ts +++ b/src/test/tenant.test.ts @@ -3,7 +3,12 @@ import dotenv from 'dotenv' import * as migrate from '../internal/database/migrations/migrate' import { multitenantKnex } from '../internal/database/multitenant-db' import { adminApp } from './common' -import { getFeatures, getFileSizeLimit, getServiceKey } from '@internal/database/tenant' +import { + getFeatures, + getFileSizeLimit, + getServiceKey, + getTenantConfig, +} from '@internal/database/tenant' import { signJWT } from '@internal/auth' dotenv.config({ path: '.env.test' }) @@ -303,4 +308,49 @@ describe('Tenant configs', () => { }) expect(getResponse.statusCode).toBe(404) }) + + test('Get tenant config with invalid tenant id expected error', () => { + expect(getTenantConfig('')).rejects.toThrowError('Invalid tenant id') + }) + + test('Get tenant config with unknown tenant id expected error', () => { + expect(getTenantConfig('zzz')).rejects.toThrowError('Missing tenant config for tenant zzz') + }) + + test('Get tenant config always retrieves concurrent requests from cache', async () => { + const knexTableSpy = jest.spyOn(multitenantKnex, 'table') + try { + const tenantId = 'cache-test-abc' + await adminApp.inject({ + method: 'POST', + url: `/tenants/${tenantId}`, + payload, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + + await getTenantConfig(tenantId) + expect(knexTableSpy).toHaveBeenCalledTimes(1) + expect(knexTableSpy).toHaveBeenCalledWith('tenants') + + const results = await Promise.all([ + getTenantConfig(tenantId), + getTenantConfig(tenantId), + getTenantConfig(tenantId), + ]) + expect(knexTableSpy).toHaveBeenCalledTimes(1) + results.forEach((result, i) => expect(result).toEqual(results[i === 0 ? 1 : 0])) + + await adminApp.inject({ + method: 'DELETE', + url: `/tenants/${tenantId}`, + headers: { + apikey: process.env.ADMIN_API_KEYS, + }, + }) + } finally { + knexTableSpy.mockRestore() + } + }) }) From 4959ffa2e99870fc624c4314cb9c6ee4e14a5187 Mon Sep 17 00:00:00 2001 From: Lenny Date: Mon, 28 Apr 2025 12:13:44 -0400 Subject: [PATCH 2/2] move s3 credential manager into protocols/s3 --- src/internal/database/tenant.ts | 4 ++-- .../protocols/s3/credentials-manager}/index.ts | 0 .../protocols/s3/credentials-manager}/manager.ts | 2 +- .../protocols/s3/credentials-manager}/store-knex.ts | 0 .../protocols/s3/credentials-manager}/store.ts | 0 5 files changed, 3 insertions(+), 3 deletions(-) rename src/{internal/database/s3-credentials-manager => storage/protocols/s3/credentials-manager}/index.ts (100%) rename src/{internal/database/s3-credentials-manager => storage/protocols/s3/credentials-manager}/manager.ts (98%) rename src/{internal/database/s3-credentials-manager => storage/protocols/s3/credentials-manager}/store-knex.ts (100%) rename src/{internal/database/s3-credentials-manager => storage/protocols/s3/credentials-manager}/store.ts (100%) diff --git a/src/internal/database/tenant.ts b/src/internal/database/tenant.ts index f5a81120..1fd81eed 100644 --- a/src/internal/database/tenant.ts +++ b/src/internal/database/tenant.ts @@ -8,8 +8,8 @@ import { ERRORS } from '@internal/errors' import { DBMigration } from '@internal/database/migrations' import { JWKSManager } from './jwks-manager' import { JWKSManagerStoreKnex } from './jwks-manager/store-knex' -import { S3CredentialsManagerStoreKnex } from './s3-credentials-manager/store-knex' -import { S3CredentialsManager } from './s3-credentials-manager' +import { S3CredentialsManagerStoreKnex } from '../../storage/protocols/s3/credentials-manager/store-knex' +import { S3CredentialsManager } from '../../storage/protocols/s3/credentials-manager' interface TenantConfig { anonKey?: string diff --git a/src/internal/database/s3-credentials-manager/index.ts b/src/storage/protocols/s3/credentials-manager/index.ts similarity index 100% rename from src/internal/database/s3-credentials-manager/index.ts rename to src/storage/protocols/s3/credentials-manager/index.ts diff --git a/src/internal/database/s3-credentials-manager/manager.ts b/src/storage/protocols/s3/credentials-manager/manager.ts similarity index 98% rename from src/internal/database/s3-credentials-manager/manager.ts rename to src/storage/protocols/s3/credentials-manager/manager.ts index 497fa6c0..caaf2c96 100644 --- a/src/internal/database/s3-credentials-manager/manager.ts +++ b/src/storage/protocols/s3/credentials-manager/manager.ts @@ -4,7 +4,7 @@ import objectSizeOf from 'object-sizeof' import { S3Credentials, S3CredentialsManagerStore, S3CredentialsRaw } from './store' import { createMutexByKey } from '@internal/concurrency' import { ERRORS } from '@internal/errors' -import { getConfig } from '../../../config' +import { getConfig } from '../../../../config' import { decrypt, encrypt } from '@internal/auth' import { PubSubAdapter } from '@internal/pubsub' diff --git a/src/internal/database/s3-credentials-manager/store-knex.ts b/src/storage/protocols/s3/credentials-manager/store-knex.ts similarity index 100% rename from src/internal/database/s3-credentials-manager/store-knex.ts rename to src/storage/protocols/s3/credentials-manager/store-knex.ts diff --git a/src/internal/database/s3-credentials-manager/store.ts b/src/storage/protocols/s3/credentials-manager/store.ts similarity index 100% rename from src/internal/database/s3-credentials-manager/store.ts rename to src/storage/protocols/s3/credentials-manager/store.ts