From e0e5d34cc5dfa8293e1f5bfb21cbbcf70f2aa84a Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 9 Sep 2026 08:56:09 +0000 Subject: [PATCH 1/5] feat(storage)!: complete container/folder/file authz Register Container, Folder, and File as a filesystem-shaped ReBAC tree using the current oncePeerUp authz lifecycle. Client file APIs no longer create missing containers. An omitted folder resolves to a personal cnd_/ folder, and creating under another user's missing personal root is denied. Fixes #1173 BREAKING CHANGE: Client storage APIs return 404 for missing containers instead of creating them. An omitted folder now creates cnd_/. --- CHANGELOG.md | 10 + modules/storage/README.md | 24 ++ modules/storage/package.json | 2 +- modules/storage/src/Storage.ts | 10 +- modules/storage/src/admin/adminFile.ts | 155 +++++---- modules/storage/src/admin/index.ts | 86 ++--- modules/storage/src/authz/bootstrap.test.ts | 71 ++++ modules/storage/src/authz/bootstrap.ts | 23 ++ modules/storage/src/authz/cascade.test.ts | 164 +++++++++ modules/storage/src/authz/cascade.ts | 84 +++++ modules/storage/src/authz/folders.test.ts | 107 ++++++ modules/storage/src/authz/folders.ts | 133 ++++++++ modules/storage/src/authz/helpers.test.ts | 143 ++++++++ modules/storage/src/authz/helpers.ts | 101 ++++++ modules/storage/src/authz/index.ts | 52 ++- modules/storage/src/authz/relations.test.ts | 314 ++++++++++++++++++ modules/storage/src/authz/relations.ts | 193 +++++++++++ .../storage/src/handlers/file.authz.test.ts | 291 ++++++++++++++++ modules/storage/src/handlers/file.ts | 279 ++++++++-------- modules/storage/src/providers/local/index.ts | 2 +- modules/storage/src/utils/index.ts | 12 +- 21 files changed, 1968 insertions(+), 288 deletions(-) create mode 100644 modules/storage/README.md create mode 100644 modules/storage/src/authz/bootstrap.test.ts create mode 100644 modules/storage/src/authz/bootstrap.ts create mode 100644 modules/storage/src/authz/cascade.test.ts create mode 100644 modules/storage/src/authz/cascade.ts create mode 100644 modules/storage/src/authz/folders.test.ts create mode 100644 modules/storage/src/authz/folders.ts create mode 100644 modules/storage/src/authz/helpers.test.ts create mode 100644 modules/storage/src/authz/helpers.ts create mode 100644 modules/storage/src/authz/relations.test.ts create mode 100644 modules/storage/src/authz/relations.ts create mode 100644 modules/storage/src/handlers/file.authz.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 937aa3289..8a8048f85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +## [Unreleased] + +### ⚠ BREAKING CHANGES + +* **storage:** Client APIs no longer create missing containers (404). An omitted folder now resolves to a personal `cnd_/` folder. Client list-files is deferred. + +### Features + +* **storage:** complete filesystem-shaped ReBAC for Container, Folder, and File ([#1173](https://github.com/ConduitPlatform/Conduit/issues/1173)) + ## [0.17.0-alpha.6](https://github.com/ConduitPlatform/Conduit/compare/v0.17.0-alpha.5...v0.17.0-alpha.6) (2026-07-26) diff --git a/modules/storage/README.md b/modules/storage/README.md new file mode 100644 index 000000000..332828d35 --- /dev/null +++ b/modules/storage/README.md @@ -0,0 +1,24 @@ +# Storage + +Filesystem-shaped authorization for containers, folders, and files. Client list-files is deferred. + +## Client breaking changes + +These apply to Storage Client routes and gRPC calls that use the user file handlers. + +- **Missing containers are not created.** Creating or updating a file with a container that does not already exist returns `404 Not Found`. `allowContainerCreation` still only affects Admin implicit container creation. +- **Omitted folder becomes a personal folder.** If `folder` is omitted on Client file create, Storage uses `cnd_/`. Passing `/` still stores at the container root. +- **Personal-folder squat is denied.** Creating a missing `cnd_/` path returns `403 Permission Denied`. A user may create their own `cnd_/`. If another user's personal root already exists, normal folder edit checks apply. + +Admin-only container create remains available. Public file reads without auth, module schema ownership, and existing public URI / CDN / content-disposition / local URL upload behavior are unchanged. + +## Authorization tree + +When `authorization.enabled` is true, Storage registers `Container`, `Folder`, and `File` resources and maintains owner relations that follow the path: + +- A container may own first-level folders, or files stored at `/`. +- A folder owns nested folders and files. +- The default container is never owned. It is created on both the database and the storage provider if missing. +- An optional `scope` (for example `Team:`) is attached as an extra owner when provided. Admin folder create without scope only attaches the container as the first-folder owner. + +Folder delete removes nested folders/files and all of their relations. Container delete pages those cleanups and also clears `Container` relations. diff --git a/modules/storage/package.json b/modules/storage/package.json index 592e2eb8d..e0baebc4b 100644 --- a/modules/storage/package.json +++ b/modules/storage/package.json @@ -30,7 +30,7 @@ "prepare": "npm run build", "build:docker": "docker build -t ghcr.io/conduitplatform/storage:latest -f ./Dockerfile ../../ && docker push ghcr.io/conduitplatform/storage:latest", "generateTypes": "sh build.sh", - "test": "tsc -p tsconfig.test.json && node --test dist-test/config/config.test.js dist-test/adapter/StorageParamAdapter.test.js dist-test/migrations/fileUriMigration.test.js dist-test/providers/aws/acl.test.js dist-test/providers/google/folderMarkers.test.js dist-test/providers/google/deleteFolder.test.js dist-test/providers/google/publicAccess.test.js dist-test/utils/filePrivacy.test.js" + "test": "tsc -p tsconfig.test.json && node --test dist-test/config/config.test.js dist-test/adapter/StorageParamAdapter.test.js dist-test/migrations/fileUriMigration.test.js dist-test/providers/aws/acl.test.js dist-test/providers/google/folderMarkers.test.js dist-test/providers/google/deleteFolder.test.js dist-test/providers/google/publicAccess.test.js dist-test/utils/filePrivacy.test.js dist-test/authz/helpers.test.js dist-test/authz/relations.test.js dist-test/authz/folders.test.js dist-test/authz/cascade.test.js dist-test/authz/bootstrap.test.js dist-test/handlers/file.authz.test.js" }, "keywords": [], "author": "", diff --git a/modules/storage/src/Storage.ts b/modules/storage/src/Storage.ts index d47b66e67..ea363db7d 100644 --- a/modules/storage/src/Storage.ts +++ b/modules/storage/src/Storage.ts @@ -50,7 +50,8 @@ import { type ImportResult, } from '@conduitplatform/module-tools'; import { StorageParamAdapter } from './adapter/StorageParamAdapter.js'; -import { FileResource } from './authz/index.js'; +import { ContainerResource, FileResource, FolderResource } from './authz/index.js'; +import { ensureDefaultContainer } from './authz/bootstrap.js'; import { AdminFileHandlers } from './admin/adminFile.js'; import { randomBytes } from 'node:crypto'; import { fileURLToPath } from 'node:url'; @@ -248,9 +249,11 @@ export default class Storage extends ManagedModule { this._storageAuthzResourceDispose?.(); this._storageAuthzResourceDispose = this.grpcSdk.oncePeerUp( 'authorization', - () => { + async () => { this._storageAuthzResourceDispose = null; - this.grpcSdk.authorization!.defineResource(FileResource); + await this.grpcSdk.authorization!.defineResource(ContainerResource); + await this.grpcSdk.authorization!.defineResource(FolderResource); + await this.grpcSdk.authorization!.defineResource(FileResource); }, ); } else { @@ -269,6 +272,7 @@ export default class Storage extends ManagedModule { }); this._fileHandlers.updateProvider(this.storageProvider); this._adminFileHandlers.updateProvider(this.storageProvider); + await ensureDefaultContainer(this.storageProvider); // Run the public container migration once after provider is configured if (!this.publicContainerMigrationRan && provider !== 'local') { this.publicContainerMigrationRan = true; diff --git a/modules/storage/src/admin/adminFile.ts b/modules/storage/src/admin/adminFile.ts index 4340f2820..b63aa714a 100644 --- a/modules/storage/src/admin/adminFile.ts +++ b/modules/storage/src/admin/adminFile.ts @@ -15,13 +15,19 @@ import { _updateFile, _updateFileUploadUrl, applyCdnHost, - deepPathHandler, normalizeFolderPath, resolvePublicFileAccessUrl, sanitizeFileForResponse, storeNewFile, validateName, } from '../utils/index.js'; +import { rethrowGrpcOrInternal, resolveFileId, resolveScope } from '../authz/helpers.js'; +import { findOrCreateFolders } from '../authz/folders.js'; +import { + createFileRelations, + deleteAllRelationsSafe, + updateFileRelations, +} from '../authz/relations.js'; export class AdminFileHandlers { private readonly database: DatabaseProvider; @@ -57,13 +63,23 @@ export class AdminFileHandlers { async createFile(call: ParsedRouterRequest): Promise { const { name, alias, data, container, mimeType, isPublic } = call.request.params; + const scope = resolveScope(call.request); const folder = normalizeFolderPath(call.request.params.folder); const config = ConfigController.getInstance().config; const usedContainer = isNil(container) ? config.defaultContainer : await this.findOrCreateContainer(container, isPublic); if (folder !== '/') { - await this.findOrCreateFolders(folder, usedContainer, isPublic); + await findOrCreateFolders( + this.grpcSdk, + this.storageProvider, + folder, + usedContainer, + { + isPublic, + scope, + }, + ); } const validatedName = await validateName(name, folder, usedContainer); if (!isString(data)) { @@ -71,7 +87,7 @@ export class AdminFileHandlers { } try { - return await storeNewFile(this.storageProvider, { + const file = await storeNewFile(this.storageProvider, { name: validatedName, alias, data, @@ -80,28 +96,37 @@ export class AdminFileHandlers { isPublic, mimeType, }); + await createFileRelations(this.grpcSdk, file, { scope }); + return file; } catch (e) { - throw new GrpcError( - status.INTERNAL, - (e as Error).message ?? 'Something went wrong', - ); + rethrowGrpcOrInternal(e); } } async createFileUploadUrl(call: ParsedRouterRequest): Promise { const { name, alias, container, size = 0, mimeType, isPublic } = call.request.params; + const scope = resolveScope(call.request); const folder = normalizeFolderPath(call.request.params.folder); const config = ConfigController.getInstance().config; const usedContainer = isNil(container) ? config.defaultContainer : await this.findOrCreateContainer(container, isPublic); if (folder !== '/') { - await this.findOrCreateFolders(folder, usedContainer, isPublic); + await findOrCreateFolders( + this.grpcSdk, + this.storageProvider, + folder, + usedContainer, + { + isPublic, + scope, + }, + ); } const validatedName = await validateName(name, folder, usedContainer); try { - return await _createFileUploadUrl(this.storageProvider, { + const result = await _createFileUploadUrl(this.storageProvider, { container: usedContainer, folder, isPublic, @@ -110,17 +135,15 @@ export class AdminFileHandlers { size, mimeType, }); + await createFileRelations(this.grpcSdk, result.file, { scope }); + return result; } catch (e) { - throw new GrpcError( - status.INTERNAL, - (e as Error).message ?? 'Something went wrong', - ); + rethrowGrpcOrInternal(e); } } async updateFileUploadUrl(call: ParsedRouterRequest): Promise { - const { id, alias, mimeType, size } = call.request.params; - const found = await File.getInstance().findOne({ _id: id }); + const found = await File.getInstance().findOne({ _id: resolveFileId(call.request) }); if (isNil(found)) { throw new GrpcError(status.NOT_FOUND, 'File does not exist'); } @@ -129,25 +152,26 @@ export class AdminFileHandlers { found, ); try { - return await _updateFileUploadUrl(this.storageProvider, found, { + const result = await _updateFileUploadUrl(this.storageProvider, found, { name, - alias, + alias: call.request.params.alias, folder, container, - mimeType: mimeType ?? found.mimeType, - size, + mimeType: call.request.params.mimeType ?? found.mimeType, + size: call.request.params.size, + }); + await updateFileRelations(this.grpcSdk, found, result.file, { + scope: resolveScope(call.request), }); + return result; } catch (e) { - throw new GrpcError( - status.INTERNAL, - (e as Error).message ?? 'Something went wrong', - ); + rethrowGrpcOrInternal(e); } } async updateFile(call: ParsedRouterRequest): Promise { - const { id, alias, data, mimeType } = call.request.params; - const found = await File.getInstance().findOne({ _id: id }); + const { alias, data, mimeType } = call.request.params; + const found = await File.getInstance().findOne({ _id: resolveFileId(call.request) }); if (isNil(found)) { throw new GrpcError(status.NOT_FOUND, 'File does not exist'); } @@ -156,28 +180,30 @@ export class AdminFileHandlers { found, ); try { - return await _updateFile(this.storageProvider, found, { + const updated = (await _updateFile(this.storageProvider, found, { name, alias, folder, container, data: Buffer.from(data, 'base64'), mimeType: mimeType ?? found.mimeType, + })) as File; + await updateFileRelations(this.grpcSdk, found, updated, { + scope: resolveScope(call.request), }); + return updated; } catch (e) { - throw new GrpcError( - status.INTERNAL, - (e as Error).message ?? 'Something went wrong!', - ); + rethrowGrpcOrInternal(e); } } async deleteFile(call: ParsedRouterRequest): Promise { - if (!isString(call.request.params.id)) { + const id = resolveFileId(call.request); + if (!isString(id)) { throw new GrpcError(status.INVALID_ARGUMENT, 'The provided id is invalid'); } try { - const found = await File.getInstance().findOne({ _id: call.request.params.id }); + const found = await File.getInstance().findOne({ _id: id }); if (isNil(found)) { throw new GrpcError(status.NOT_FOUND, 'File does not exist'); } @@ -187,15 +213,13 @@ export class AdminFileHandlers { if (!success) { throw new GrpcError(status.INTERNAL, 'File could not be deleted'); } - await File.getInstance().deleteOne({ _id: call.request.params.id }); + await File.getInstance().deleteOne({ _id: id }); ConduitGrpcSdk.Metrics?.decrement('files_total'); ConduitGrpcSdk.Metrics?.decrement('storage_size_bytes_total', found.size); + await deleteAllRelationsSafe(this.grpcSdk, { resource: `File:${id}` }); return { success: true }; } catch (e) { - throw new GrpcError( - status.INTERNAL, - (e as Error).message ?? 'Something went wrong!', - ); + rethrowGrpcOrInternal(e); } } @@ -226,19 +250,17 @@ export class AdminFileHandlers { } return { redirect: url }; } catch (e) { - throw new GrpcError( - status.INTERNAL, - (e as Error).message ?? 'Something went wrong!', - ); + rethrowGrpcOrInternal(e); } } async getFileData(call: ParsedRouterRequest): Promise { - if (!isString(call.request.params.id)) { + const id = resolveFileId(call.request); + if (!isString(id)) { throw new GrpcError(status.INVALID_ARGUMENT, 'The provided id is invalid'); } try { - const file = await File.getInstance().findOne({ _id: call.request.params.id }); + const file = await File.getInstance().findOne({ _id: id }); if (isNil(file)) { throw new GrpcError(status.NOT_FOUND, 'File does not exist'); } @@ -255,10 +277,7 @@ export class AdminFileHandlers { } return { data: data.toString('base64') }; } catch (e) { - throw new GrpcError( - status.INTERNAL, - (e as Error).message ?? 'Something went wrong!', - ); + rethrowGrpcOrInternal(e); } } @@ -267,7 +286,6 @@ export class AdminFileHandlers { isPublic?: boolean, ): Promise { const config = ConfigController.getInstance().config; - // the container is sent from the client const found = await _StorageContainer.getInstance().findOne({ name: container, }); @@ -290,36 +308,6 @@ export class AdminFileHandlers { return container; } - async findOrCreateFolders( - folderPath: string, - container: string, - isPublic?: boolean, - lastExistsHandler?: () => void, - ): Promise<_StorageFolder[]> { - const createdFolders: _StorageFolder[] = []; - let folder: _StorageFolder | null = null; - await deepPathHandler(folderPath, async (folderPath, isLast) => { - folder = await _StorageFolder - .getInstance() - .findOne({ name: folderPath, container }); - if (isNil(folder)) { - folder = await _StorageFolder.getInstance().create({ - name: folderPath, - container, - isPublic, - }); - createdFolders.push(folder); - const exists = await this.storage.container(container).folderExists(folderPath); - if (!exists) { - await this.storage.container(container).createFolder(folderPath); - } - } else if (isLast) { - lastExistsHandler?.(); - } - }); - return createdFolders; - } - private async validateFilenameAndContainer(call: ParsedRouterRequest, file: File) { const { name, folder, container } = call.request.params; const newName = name ?? file.name; @@ -329,7 +317,16 @@ export class AdminFileHandlers { } const newFolder = isNil(folder) ? file.folder : normalizeFolderPath(folder); if (newFolder !== file.folder && newFolder !== '/') { - await this.findOrCreateFolders(newFolder, newContainer); + await findOrCreateFolders( + this.grpcSdk, + this.storageProvider, + newFolder, + newContainer, + { + isPublic: file.isPublic, + scope: resolveScope(call.request), + }, + ); } const isDataUpdate = newName === file.name && diff --git a/modules/storage/src/admin/index.ts b/modules/storage/src/admin/index.ts index e0a58476c..8fbe6888b 100644 --- a/modules/storage/src/admin/index.ts +++ b/modules/storage/src/admin/index.ts @@ -12,6 +12,7 @@ import { ConduitBoolean, ConduitNumber, ConduitString, + ConfigController, GrpcServer, RoutingManager, } from '@conduitplatform/module-tools'; @@ -20,6 +21,10 @@ import { isEmpty, isNil } from 'lodash-es'; import { _StorageContainer, _StorageFolder, File } from '../models/index.js'; import { normalizeFolderPath, sanitizeFilesForResponse } from '../utils/index.js'; import { AdminFileHandlers } from './adminFile.js'; +import { rethrowGrpcOrInternal, resolveScope } from '../authz/helpers.js'; +import { findOrCreateFolders } from '../authz/folders.js'; +import { createContainerOwnerRelation } from '../authz/relations.js'; +import { deleteContainerTree, deleteFolderTree } from '../authz/cascade.js'; export class AdminRoutes { private readonly routingManager: RoutingManager; @@ -97,6 +102,7 @@ export class AdminRoutes { async createFolder(call: ParsedRouterRequest): Promise { const { container, isPublic } = call.request.params; + const scope = resolveScope(call.request); const name = normalizeFolderPath(call.request.params.name); if (name === '/') { throw new GrpcError(status.INVALID_ARGUMENT, 'Folder name may not be empty'); @@ -105,16 +111,19 @@ export class AdminRoutes { .getInstance() .findOne({ name: container }); if (isNil(containerDocument)) { - await this._createContainer(container, isPublic).catch((e: Error) => { - throw new GrpcError(status.INTERNAL, e.message); - }); + await this._createContainer(container, isPublic, scope); } - const createdFolders = await this.fileHandlers.findOrCreateFolders( + const createdFolders = await findOrCreateFolders( + this.grpcSdk, + this.fileHandlers.storage, name, container, - isPublic, - () => { - throw new GrpcError(status.ALREADY_EXISTS, 'Folder already exists'); + { + isPublic, + scope, + lastExistsHandler: () => { + throw new GrpcError(status.ALREADY_EXISTS, 'Folder already exists'); + }, }, ); return createdFolders[createdFolders.length - 1]; @@ -128,19 +137,8 @@ export class AdminRoutes { }); if (isNil(folder)) { throw new GrpcError(status.NOT_FOUND, 'Folder does not exist'); - } else { - await this.fileHandlers.storage - .container(folder.container) - .deleteFolder(folder.name); - await _StorageFolder.getInstance().deleteOne({ - name: folder.name, - container: folder.container, - }); - await File.getInstance().deleteMany({ - folder: folder.name, - container: folder.container, - }); } + await deleteFolderTree(this.grpcSdk, this.fileHandlers.storage, folder); return 'OK'; } @@ -158,7 +156,7 @@ export class AdminRoutes { async createContainer(call: ParsedRouterRequest): Promise { const { name, isPublic } = call.request.params; - return await this._createContainer(name, isPublic); + return await this._createContainer(name, isPublic, resolveScope(call.request)); } async deleteContainer(call: ParsedRouterRequest): Promise { @@ -169,29 +167,17 @@ export class AdminRoutes { }); if (isNil(container)) { throw new GrpcError(status.NOT_FOUND, 'Container does not exist'); - } else { - await this.fileHandlers.storage.deleteContainer(container.name); - await _StorageContainer.getInstance().deleteOne({ - _id: id, - }); - await File.getInstance().deleteMany({ - container: container.name, - }); - await _StorageFolder.getInstance().deleteMany({ - container: container.name, - }); } + await deleteContainerTree(this.grpcSdk, this.fileHandlers.storage, container); return container; } catch (e) { - throw new GrpcError( - status.INTERNAL, - (e as Error).message ?? 'Something went wrong', - ); + rethrowGrpcOrInternal(e); } } private registerAdminRoutes() { this.routingManager.clear(); + const authzEnabled = ConfigController.getInstance().config.authorization.enabled; this.routingManager.route( { path: '/files/:id', @@ -238,6 +224,9 @@ export class AdminRoutes { mimeType: ConduitString.Optional, isPublic: ConduitBoolean.Optional, }, + queryParams: { + ...(authzEnabled && { scope: { type: TYPE.String, required: false } }), + }, }, new ConduitRouteReturnDefinition('CreateFile', File.name), this.fileHandlers.createFile.bind(this.fileHandlers), @@ -253,6 +242,9 @@ export class AdminRoutes { container: { type: TYPE.String, required: false }, isPublic: TYPE.Boolean, }, + queryParams: { + ...(authzEnabled && { scope: { type: TYPE.String, required: false } }), + }, action: ConduitRouteActions.POST, path: '/files/upload', description: `Creates a new file and provides a URL to upload it to.`, @@ -279,6 +271,9 @@ export class AdminRoutes { data: ConduitString.Required, mimeType: ConduitString.Optional, }, + queryParams: { + ...(authzEnabled && { scope: { type: TYPE.String, required: false } }), + }, }, new ConduitRouteReturnDefinition('PatchFile', File.name), this.fileHandlers.updateFile.bind(this.fileHandlers), @@ -296,6 +291,9 @@ export class AdminRoutes { mimeType: ConduitString.Optional, size: ConduitNumber.Optional, }, + queryParams: { + ...(authzEnabled && { scope: { type: TYPE.String, required: false } }), + }, action: ConduitRouteActions.PATCH, path: '/files/upload/:id', description: `Updates a file and provides a URL to upload its data to.`, @@ -383,6 +381,9 @@ export class AdminRoutes { container: ConduitString.Required, isPublic: ConduitBoolean.Optional, }, + queryParams: { + ...(authzEnabled && { scope: { type: TYPE.String, required: false } }), + }, }, new ConduitRouteReturnDefinition('CreateFolder', _StorageFolder.name), this.createFolder.bind(this), @@ -425,6 +426,9 @@ export class AdminRoutes { name: ConduitString.Required, isPublic: ConduitBoolean.Optional, }, + queryParams: { + ...(authzEnabled && { scope: { type: TYPE.String, required: false } }), + }, }, new ConduitRouteReturnDefinition(_StorageContainer.name), this.createContainer.bind(this), @@ -444,7 +448,11 @@ export class AdminRoutes { this.routingManager.registerRoutes(); } - private async _createContainer(name: string, isPublic: boolean | undefined) { + private async _createContainer( + name: string, + isPublic: boolean | undefined, + scope?: string, + ) { try { let container = await _StorageContainer.getInstance().findOne({ name, @@ -459,15 +467,13 @@ export class AdminRoutes { name, isPublic, }); + await createContainerOwnerRelation(this.grpcSdk, container, scope); } else { throw new GrpcError(status.ALREADY_EXISTS, 'Container already exists'); } return container; } catch (e) { - throw new GrpcError( - status.INTERNAL, - (e as Error).message ?? 'Something went wrong', - ); + rethrowGrpcOrInternal(e); } } } diff --git a/modules/storage/src/authz/bootstrap.test.ts b/modules/storage/src/authz/bootstrap.test.ts new file mode 100644 index 000000000..72d84bdcf --- /dev/null +++ b/modules/storage/src/authz/bootstrap.test.ts @@ -0,0 +1,71 @@ +import { afterEach, describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { ConfigController } from '@conduitplatform/module-tools'; +import { _StorageContainer } from '../models/index.js'; +import { ensureDefaultContainer } from './bootstrap.js'; + +const originalConfig = ConfigController.getInstance().config; +const originalGetInstance = _StorageContainer.getInstance.bind(_StorageContainer); + +afterEach(() => { + ConfigController.getInstance().config = originalConfig; + _StorageContainer.getInstance = originalGetInstance; +}); + +describe('ensureDefaultContainer', () => { + it('creates a missing default container in DB and on the provider without owning it', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + }; + let created: { name: string; isPublic?: boolean } | undefined; + _StorageContainer.getInstance = (() => ({ + findOne: async () => null, + create: async (doc: { name: string; isPublic?: boolean }) => { + created = doc; + return { _id: 'c1', ...doc }; + }, + })) as unknown as typeof _StorageContainer.getInstance; + + const providerCalls: string[] = []; + const container = await ensureDefaultContainer({ + containerExists: async (name: string) => { + providerCalls.push(`exists:${name}`); + return false; + }, + createContainer: async (name: string) => { + providerCalls.push(`create:${name}`); + return true; + }, + } as never); + + assert.equal(container.name, 'conduit'); + assert.equal(created?.isPublic, false); + assert.deepEqual(providerCalls, ['exists:conduit', 'create:conduit']); + }); + + it('is idempotent when the default container already exists on DB and provider', async () => { + ConfigController.getInstance().config = { + defaultContainer: 'conduit', + }; + let createDb = 0; + _StorageContainer.getInstance = (() => ({ + findOne: async () => ({ _id: 'c1', name: 'conduit' }), + create: async () => { + createDb += 1; + return { _id: 'c1', name: 'conduit' }; + }, + })) as unknown as typeof _StorageContainer.getInstance; + + let createProvider = 0; + await ensureDefaultContainer({ + containerExists: async () => true, + createContainer: async () => { + createProvider += 1; + return true; + }, + } as never); + assert.equal(createDb, 0); + assert.equal(createProvider, 0); + }); +}); diff --git a/modules/storage/src/authz/bootstrap.ts b/modules/storage/src/authz/bootstrap.ts new file mode 100644 index 000000000..28613642f --- /dev/null +++ b/modules/storage/src/authz/bootstrap.ts @@ -0,0 +1,23 @@ +import { IStorageProvider } from '../interfaces/index.js'; +import { _StorageContainer } from '../models/index.js'; +import { defaultContainerName } from './helpers.js'; + +export async function ensureDefaultContainer( + storageProvider: IStorageProvider, +): Promise<_StorageContainer> { + const name = defaultContainerName(); + let container = await _StorageContainer.getInstance().findOne({ name }); + if (!container) { + container = await _StorageContainer.getInstance().create({ + name, + isPublic: false, + }); + } + + const exists = await storageProvider.containerExists(name); + if (exists !== true) { + await storageProvider.createContainer(name); + } + + return container; +} diff --git a/modules/storage/src/authz/cascade.test.ts b/modules/storage/src/authz/cascade.test.ts new file mode 100644 index 000000000..7c5def50f --- /dev/null +++ b/modules/storage/src/authz/cascade.test.ts @@ -0,0 +1,164 @@ +import { afterEach, describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { ConfigController } from '@conduitplatform/module-tools'; +import { ConduitGrpcSdk } from '@conduitplatform/grpc-sdk'; +import { _StorageContainer, _StorageFolder, File } from '../models/index.js'; +import { folderPrefixRegex } from './helpers.js'; +import { deleteContainerTree, deleteFolderTree } from './cascade.js'; + +const originalConfig = ConfigController.getInstance().config; +const originalFileGetInstance = File.getInstance.bind(File); +const originalFolderGetInstance = _StorageFolder.getInstance.bind(_StorageFolder); +const originalContainerGetInstance = + _StorageContainer.getInstance.bind(_StorageContainer); + +afterEach(() => { + ConfigController.getInstance().config = originalConfig; + File.getInstance = originalFileGetInstance; + _StorageFolder.getInstance = originalFolderGetInstance; + _StorageContainer.getInstance = originalContainerGetInstance; +}); + +describe('folder delete prefix', () => { + it('escapes regex so foo.bar/ does not match fooXbar/', () => { + const prefix = folderPrefixRegex('foo.bar/'); + const re = new RegExp(prefix.$regex); + assert.equal(re.test('foo.bar/'), true); + assert.equal(re.test('foo.bar/nested/'), true); + assert.equal(re.test('fooXbar/'), false); + }); +}); + +describe('deleteFolderTree', () => { + it('deletes relations for every nested folder and file, then the provider and DB once', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + }; + const deletedRelations: Array<{ subject?: string; resource?: string }> = []; + const deletedFolders: object[] = []; + const deletedFiles: object[] = []; + let providerDeletes = 0; + + _StorageFolder.getInstance = (() => ({ + findMany: async (_query: object, options?: { skip?: number; limit?: number }) => { + const all = [{ _id: 'dir1' }, { _id: 'dir2' }]; + return all.slice( + options?.skip ?? 0, + (options?.skip ?? 0) + (options?.limit ?? 100), + ); + }, + deleteMany: async (query: object) => { + deletedFolders.push(query); + }, + })) as unknown as typeof _StorageFolder.getInstance; + File.getInstance = (() => ({ + findMany: async (_query: object, options?: { skip?: number; limit?: number }) => { + const all = [{ _id: 'file1' }, { _id: 'file2' }]; + return all.slice( + options?.skip ?? 0, + (options?.skip ?? 0) + (options?.limit ?? 100), + ); + }, + deleteMany: async (query: object) => { + deletedFiles.push(query); + }, + })) as unknown as typeof File.getInstance; + + const grpcSdk = { + authorization: { + deleteAllRelations: async (query: { subject?: string; resource?: string }) => { + deletedRelations.push(query); + }, + }, + } as unknown as ConduitGrpcSdk; + const storage = { + container: () => ({ + deleteFolder: async () => { + providerDeletes += 1; + return true; + }, + }), + }; + + await deleteFolderTree( + grpcSdk, + storage as never, + { _id: 'dir1', name: 'docs/', container: 'conduit' } as never, + ); + + const resources = deletedRelations.map(item => item.resource).filter(Boolean); + const subjects = deletedRelations.map(item => item.subject).filter(Boolean); + assert.deepEqual(resources.sort(), [ + 'File:file1', + 'File:file2', + 'Folder:dir1', + 'Folder:dir2', + ]); + assert.deepEqual(subjects.sort(), ['Folder:dir1', 'Folder:dir2']); + assert.equal(providerDeletes, 1); + assert.equal(deletedFolders.length, 1); + assert.equal(deletedFiles.length, 1); + assert.deepEqual(deletedFolders[0], { + name: folderPrefixRegex('docs/'), + container: 'conduit', + }); + }); +}); + +describe('deleteContainerTree', () => { + it('pages file and folder ids and also clears Container relations', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + }; + const deletedRelations: Array<{ subject?: string; resource?: string }> = []; + File.getInstance = (() => ({ + findMany: async (_query: object, options?: { skip?: number; limit?: number }) => { + const all = Array.from({ length: 3 }, (_, i) => ({ _id: `file${i}` })); + return all.slice( + options?.skip ?? 0, + (options?.skip ?? 0) + (options?.limit ?? 2), + ); + }, + deleteMany: async () => undefined, + })) as unknown as typeof File.getInstance; + _StorageFolder.getInstance = (() => ({ + findMany: async () => [{ _id: 'dir1' }], + deleteMany: async () => undefined, + })) as unknown as typeof _StorageFolder.getInstance; + _StorageContainer.getInstance = (() => ({ + deleteOne: async () => undefined, + })) as unknown as typeof _StorageContainer.getInstance; + + const grpcSdk = { + authorization: { + deleteAllRelations: async (query: { subject?: string; resource?: string }) => { + deletedRelations.push(query); + }, + }, + } as unknown as ConduitGrpcSdk; + const storage = { + deleteContainer: async () => true, + }; + + await deleteContainerTree( + grpcSdk, + storage as never, + { _id: 'c1', name: 'photos' } as never, + ); + + assert.equal( + deletedRelations.some(item => item.resource === 'Container:c1'), + true, + ); + assert.equal( + deletedRelations.some(item => item.subject === 'Container:c1'), + true, + ); + assert.equal( + deletedRelations.filter(item => item.resource?.startsWith('File:')).length, + 3, + ); + }); +}); diff --git a/modules/storage/src/authz/cascade.ts b/modules/storage/src/authz/cascade.ts new file mode 100644 index 000000000..905cba27d --- /dev/null +++ b/modules/storage/src/authz/cascade.ts @@ -0,0 +1,84 @@ +import { ConduitGrpcSdk } from '@conduitplatform/grpc-sdk'; +import { IStorageProvider } from '../interfaces/index.js'; +import { _StorageContainer, _StorageFolder, File } from '../models/index.js'; +import { folderPrefixRegex, isAuthzEnabled } from './helpers.js'; +import { + deleteAllRelationsSafe, + deleteRelationsForResources, + deleteRelationsForSubjects, + forEachDocumentPage, +} from './relations.js'; + +export async function deleteFolderTree( + grpcSdk: ConduitGrpcSdk, + storageProvider: IStorageProvider, + folder: _StorageFolder, +): Promise { + const prefix = folderPrefixRegex(folder.name); + const folderQuery = { name: prefix, container: folder.container }; + const fileQuery = { folder: prefix, container: folder.container }; + + if (isAuthzEnabled()) { + await forEachDocumentPage( + (skip, limit) => + _StorageFolder + .getInstance() + .findMany(folderQuery, { select: '_id', skip, limit }), + async folders => { + const ids = folders.map(doc => `Folder:${doc._id}`); + await deleteRelationsForResources(grpcSdk, ids); + await deleteRelationsForSubjects(grpcSdk, ids); + }, + ); + await forEachDocumentPage( + (skip, limit) => + File.getInstance().findMany(fileQuery, { select: '_id', skip, limit }), + async files => { + await deleteRelationsForResources( + grpcSdk, + files.map(doc => `File:${doc._id}`), + ); + }, + ); + } + + await storageProvider.container(folder.container).deleteFolder(folder.name); + await File.getInstance().deleteMany(fileQuery); + await _StorageFolder.getInstance().deleteMany(folderQuery); +} + +export async function deleteContainerTree( + grpcSdk: ConduitGrpcSdk, + storageProvider: IStorageProvider, + container: _StorageContainer, +): Promise { + const query = { container: container.name }; + + if (isAuthzEnabled()) { + await forEachDocumentPage( + (skip, limit) => File.getInstance().findMany(query, { select: '_id', skip, limit }), + async files => { + await deleteRelationsForResources( + grpcSdk, + files.map(doc => `File:${doc._id}`), + ); + }, + ); + await forEachDocumentPage( + (skip, limit) => + _StorageFolder.getInstance().findMany(query, { select: '_id', skip, limit }), + async folders => { + const ids = folders.map(doc => `Folder:${doc._id}`); + await deleteRelationsForResources(grpcSdk, ids); + await deleteRelationsForSubjects(grpcSdk, ids); + }, + ); + await deleteAllRelationsSafe(grpcSdk, { subject: `Container:${container._id}` }); + await deleteAllRelationsSafe(grpcSdk, { resource: `Container:${container._id}` }); + } + + await storageProvider.deleteContainer(container.name); + await File.getInstance().deleteMany(query); + await _StorageFolder.getInstance().deleteMany(query); + await _StorageContainer.getInstance().deleteOne({ _id: container._id }); +} diff --git a/modules/storage/src/authz/folders.test.ts b/modules/storage/src/authz/folders.test.ts new file mode 100644 index 000000000..a11ed2ce0 --- /dev/null +++ b/modules/storage/src/authz/folders.test.ts @@ -0,0 +1,107 @@ +import { afterEach, describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { status } from '@grpc/grpc-js'; +import { ConduitGrpcSdk, GrpcError } from '@conduitplatform/grpc-sdk'; +import { ConfigController } from '@conduitplatform/module-tools'; +import { _StorageContainer, _StorageFolder } from '../models/index.js'; +import { assertNoPersonalFolderSquat, findOrCreateFolders } from './folders.js'; + +const originalConfig = ConfigController.getInstance().config; +const originalContainerGetInstance = + _StorageContainer.getInstance.bind(_StorageContainer); +const originalFolderGetInstance = _StorageFolder.getInstance.bind(_StorageFolder); + +afterEach(() => { + ConfigController.getInstance().config = originalConfig; + _StorageContainer.getInstance = originalContainerGetInstance; + _StorageFolder.getInstance = originalFolderGetInstance; +}); + +describe('personal folder squat', () => { + it('denies creating under another user personal root when it is missing', async () => { + _StorageFolder.getInstance = (() => ({ + findOne: async () => null, + })) as unknown as typeof _StorageFolder.getInstance; + + await assert.rejects( + () => assertNoPersonalFolderSquat('cnd_other/', 'self', 'conduit'), + (error: unknown) => + error instanceof GrpcError && error.code === status.PERMISSION_DENIED, + ); + await assert.rejects( + () => assertNoPersonalFolderSquat('cnd_other/sub/', 'self', 'conduit'), + (error: unknown) => + error instanceof GrpcError && error.code === status.PERMISSION_DENIED, + ); + }); + + it('allows a user to create their own missing personal folder', async () => { + _StorageFolder.getInstance = (() => ({ + findOne: async () => null, + })) as unknown as typeof _StorageFolder.getInstance; + await assertNoPersonalFolderSquat('cnd_self/', 'self', 'conduit'); + }); + + it('allows a path under another personal root when that root already exists', async () => { + _StorageFolder.getInstance = (() => ({ + findOne: async (query: { name?: string }) => + query.name === 'cnd_other/' ? { _id: 'dir1' } : null, + })) as unknown as typeof _StorageFolder.getInstance; + await assertNoPersonalFolderSquat('cnd_other/sub/', 'self', 'conduit'); + }); +}); + +describe('findOrCreateFolders', () => { + it('does not create a scope owner when admin omits scope', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + }; + const created: Array<{ subject: string; resource: string }> = []; + const folders: Array<{ _id: string; name: string; container: string }> = []; + _StorageContainer.getInstance = (() => ({ + findOne: async () => ({ _id: 'c1', name: 'conduit' }), + })) as unknown as typeof _StorageContainer.getInstance; + _StorageFolder.getInstance = (() => ({ + findOne: async (query: { name?: string }) => + folders.find(folder => folder.name === query.name) ?? null, + create: async (doc: { name: string; container: string }) => { + const createdDoc = { _id: `dir-${folders.length + 1}`, ...doc }; + folders.push(createdDoc); + return createdDoc; + }, + })) as unknown as typeof _StorageFolder.getInstance; + + const grpcSdk = { + authorization: { + createRelation: async (relation: { subject: string; resource: string }) => { + created.push(relation); + }, + }, + } as unknown as ConduitGrpcSdk; + const storage = { + container: () => ({ + folderExists: async () => false, + createFolder: async () => true, + }), + }; + + const result = await findOrCreateFolders( + grpcSdk, + storage as never, + 'docs/nested/', + 'conduit', + ); + assert.equal(result.length, 2); + assert.deepEqual( + created.map(relation => relation.subject), + ['Container:c1', 'Folder:dir-1'], + ); + assert.equal( + created.some( + relation => relation.subject == null || relation.subject === 'undefined', + ), + false, + ); + }); +}); diff --git a/modules/storage/src/authz/folders.ts b/modules/storage/src/authz/folders.ts new file mode 100644 index 000000000..6c9f41857 --- /dev/null +++ b/modules/storage/src/authz/folders.ts @@ -0,0 +1,133 @@ +import { ConduitGrpcSdk, GrpcError } from '@conduitplatform/grpc-sdk'; +import { status } from '@grpc/grpc-js'; +import { isNil } from 'lodash-es'; +import { IStorageProvider } from '../interfaces/index.js'; +import { _StorageContainer, _StorageFolder } from '../models/index.js'; +import { getNestedPaths } from '../utils/index.js'; +import { createFolderOwnerRelations } from './relations.js'; +import { + isAuthzEnabled, + parsePersonalFolderOwner, + personalFolderName, +} from './helpers.js'; + +export async function assertNoPersonalFolderSquat( + folder: string, + userId: string, + container: string, +): Promise { + const ownerId = parsePersonalFolderOwner(folder); + if (!ownerId || ownerId === userId) { + return; + } + const personalRoot = personalFolderName(ownerId); + const existing = await _StorageFolder + .getInstance() + .findOne({ name: personalRoot, container }); + if (isNil(existing)) { + throw new GrpcError( + status.PERMISSION_DENIED, + 'You are not allowed to create this folder', + ); + } +} + +async function assertCanEditFolder( + grpcSdk: ConduitGrpcSdk, + subject: string, + folder: _StorageFolder, +): Promise { + const allowed = await grpcSdk.authorization?.can({ + subject, + actions: ['edit'], + resource: `Folder:${folder._id}`, + }); + if (!allowed || !allowed.allow) { + throw new GrpcError( + status.PERMISSION_DENIED, + `You are not allowed to edit files in folder ${folder.name}`, + ); + } +} + +export async function assertFolderEditAccess( + grpcSdk: ConduitGrpcSdk, + container: string, + folder: string, + subject: string, +): Promise { + if (!isAuthzEnabled()) { + return; + } + const folderDoc = await _StorageFolder + .getInstance() + .findOne({ name: folder, container }); + if (folderDoc) { + await assertCanEditFolder(grpcSdk, subject, folderDoc); + return; + } + + const nestedPaths = getNestedPaths(folder); + for (let i = nestedPaths.length - 2; i >= 0; i--) { + const parent = await _StorageFolder + .getInstance() + .findOne({ name: nestedPaths[i], container }); + if (parent) { + await assertCanEditFolder(grpcSdk, subject, parent); + return; + } + } +} + +export async function findOrCreateFolders( + grpcSdk: ConduitGrpcSdk, + storageProvider: IStorageProvider, + folderPath: string, + container: string, + options?: { + isPublic?: boolean; + scope?: string; + lastExistsHandler?: () => void; + }, +): Promise<_StorageFolder[]> { + const containerDoc = await _StorageContainer.getInstance().findOne({ name: container }); + if (!containerDoc) { + throw new GrpcError(status.NOT_FOUND, 'Container does not exist'); + } + + const createdFolders: _StorageFolder[] = []; + const nestedPaths = getNestedPaths(folderPath); + let previousFolder: _StorageFolder | null = null; + + for (let i = 0; i < nestedPaths.length; i++) { + const currentPath = nestedPaths[i]; + const isLast = i === nestedPaths.length - 1; + let folder = await _StorageFolder + .getInstance() + .findOne({ name: currentPath, container }); + + if (isNil(folder)) { + folder = await _StorageFolder.getInstance().create({ + name: currentPath, + container, + isPublic: options?.isPublic, + }); + createdFolders.push(folder); + const exists = await storageProvider.container(container).folderExists(currentPath); + if (!exists) { + await storageProvider.container(container).createFolder(currentPath); + } + await createFolderOwnerRelations(grpcSdk, folder, { + isFirst: i === 0, + containerId: containerDoc._id, + parentFolderId: previousFolder?._id, + scope: options?.scope, + }); + } else if (isLast) { + options?.lastExistsHandler?.(); + } + previousFolder = folder; + } + + return createdFolders; +} diff --git a/modules/storage/src/authz/helpers.test.ts b/modules/storage/src/authz/helpers.test.ts new file mode 100644 index 000000000..6a4db615c --- /dev/null +++ b/modules/storage/src/authz/helpers.test.ts @@ -0,0 +1,143 @@ +import { afterEach, describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { status } from '@grpc/grpc-js'; +import { GrpcError } from '@conduitplatform/grpc-sdk'; +import { ConfigController } from '@conduitplatform/module-tools'; +import { + actorSubject, + escapeRegex, + folderPrefixRegex, + isAuthzEnabled, + isDefaultContainer, + isUsableSubject, + parsePersonalFolderOwner, + personalFolderName, + resolveClientFolder, + resolveFileId, + resolveScope, + resolveUserId, + rethrowGrpcOrInternal, +} from './helpers.js'; + +const originalConfig = ConfigController.getInstance().config; + +afterEach(() => { + ConfigController.getInstance().config = originalConfig; +}); + +describe('personal folder resolution', () => { + it('uses cnd_/ when the folder is omitted', () => { + assert.equal(resolveClientFolder(undefined, 'user-1'), 'cnd_user-1/'); + assert.equal(resolveClientFolder('', 'user-1'), 'cnd_user-1/'); + assert.equal(resolveClientFolder(' ', 'user-1'), 'cnd_user-1/'); + assert.equal(personalFolderName('user-1'), 'cnd_user-1/'); + }); + + it('keeps an explicit root or named folder', () => { + assert.equal(resolveClientFolder('/', 'user-1'), '/'); + assert.equal(resolveClientFolder('docs', 'user-1'), 'docs/'); + assert.equal(resolveClientFolder('docs/nested', 'user-1'), 'docs/nested/'); + }); + + it('parses the personal-folder owner from a path', () => { + assert.equal(parsePersonalFolderOwner('cnd_other/'), 'other'); + assert.equal(parsePersonalFolderOwner('cnd_other/sub/'), 'other'); + assert.equal(parsePersonalFolderOwner('docs/'), undefined); + }); +}); + +describe('request field resolution', () => { + it('reads gRPC delete ids from params and falls back to urlParams', () => { + assert.equal( + resolveFileId({ params: { id: 'from-params' }, urlParams: {} }), + 'from-params', + ); + assert.equal( + resolveFileId({ params: {}, urlParams: { id: 'from-url' } }), + 'from-url', + ); + assert.equal( + resolveFileId({ params: { id: 'from-params' }, urlParams: { id: 'from-url' } }), + 'from-params', + ); + assert.equal(resolveFileId({ params: {}, urlParams: {} }), undefined); + }); + + it('resolves scope from queryParams or params', () => { + assert.equal( + resolveScope({ queryParams: { scope: 'Team:t1' }, params: {} }), + 'Team:t1', + ); + assert.equal( + resolveScope({ queryParams: {}, params: { scope: 'Team:t2' } }), + 'Team:t2', + ); + }); + + it('does not throw when user is missing', () => { + assert.equal(resolveUserId({ context: {} }), undefined); + assert.equal(resolveUserId({ context: { user: {} } }), undefined); + assert.equal(resolveUserId({ context: { user: { _id: '' } } }), undefined); + assert.equal(resolveUserId({ context: { user: { _id: 'u1' } } }), 'u1'); + assert.equal(actorSubject({ context: { user: { _id: 'u1' } } }), 'User:u1'); + assert.equal( + actorSubject({ + context: { user: { _id: 'u1' } }, + queryParams: { scope: 'Team:t1' }, + }), + 'Team:t1', + ); + }); +}); + +describe('authz helpers', () => { + it('escapes regex metacharacters for folder prefix deletes', () => { + assert.equal(escapeRegex('foo.bar/'), 'foo\\.bar/'); + assert.deepEqual(folderPrefixRegex('foo.bar/'), { $regex: '^foo\\.bar/' }); + }); + + it('treats empty subjects as unusable', () => { + assert.equal(isUsableSubject(undefined), false); + assert.equal(isUsableSubject(''), false); + assert.equal(isUsableSubject('User:1'), true); + }); + + it('reads authz and default container from config', () => { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + }; + assert.equal(isAuthzEnabled(), true); + assert.equal(isDefaultContainer('conduit'), true); + assert.equal(isDefaultContainer('other'), false); + + ConfigController.getInstance().config = { + authorization: { enabled: false }, + defaultContainer: 'conduit', + }; + assert.equal(isAuthzEnabled(), false); + }); + + it('rethrows GrpcError 403/404 instead of wrapping them as INTERNAL', () => { + const denied = new GrpcError(status.PERMISSION_DENIED, 'nope'); + assert.throws( + () => rethrowGrpcOrInternal(denied), + (error: unknown) => { + return error instanceof GrpcError && error.code === status.PERMISSION_DENIED; + }, + ); + const missing = new GrpcError(status.NOT_FOUND, 'gone'); + assert.throws( + () => rethrowGrpcOrInternal(missing), + (error: unknown) => { + return error instanceof GrpcError && error.code === status.NOT_FOUND; + }, + ); + assert.throws( + () => rethrowGrpcOrInternal(new Error('boom')), + (error: unknown) => { + return error instanceof GrpcError && error.code === status.INTERNAL; + }, + ); + }); +}); diff --git a/modules/storage/src/authz/helpers.ts b/modules/storage/src/authz/helpers.ts new file mode 100644 index 000000000..6e7ed027e --- /dev/null +++ b/modules/storage/src/authz/helpers.ts @@ -0,0 +1,101 @@ +import { GrpcError, Indexable } from '@conduitplatform/grpc-sdk'; +import { ConfigController } from '@conduitplatform/module-tools'; +import { status } from '@grpc/grpc-js'; +import { normalizeFolderPath } from '../utils/index.js'; + +export const RELATION_PAGE_SIZE = 100; + +export function isAuthzEnabled(): boolean { + return ConfigController.getInstance().config.authorization?.enabled === true; +} + +export function defaultContainerName(): string { + return ConfigController.getInstance().config.defaultContainer as string; +} + +export function isDefaultContainer(name: string): boolean { + return name === defaultContainerName(); +} + +export function personalFolderName(userId: string): string { + return normalizeFolderPath(`cnd_${userId}`); +} + +export function resolveClientFolder( + folderParam: string | undefined, + userId: string, +): string { + if (folderParam == null || folderParam.trim() === '') { + return personalFolderName(userId); + } + return normalizeFolderPath(folderParam); +} + +export function parsePersonalFolderOwner(folder: string): string | undefined { + const match = /^cnd_([^/]+)\//.exec(folder); + return match?.[1]; +} + +export function escapeRegex(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +export function folderPrefixRegex(folderName: string): { $regex: string } { + return { $regex: `^${escapeRegex(folderName)}` }; +} + +export function isUsableSubject(subject?: string | null): subject is string { + return typeof subject === 'string' && subject.length > 0; +} + +export function resolveFileId(request: Indexable): string | undefined { + const id = request.params?.id ?? request.urlParams?.id; + return typeof id === 'string' && id.length > 0 ? id : undefined; +} + +export function resolveScope(request: Indexable): string | undefined { + const scope = request.queryParams?.scope ?? request.params?.scope; + return typeof scope === 'string' && scope.length > 0 ? scope : undefined; +} + +export function resolveUserId(request: Indexable): string | undefined { + const id = request.context?.user?._id; + return typeof id === 'string' && id.length > 0 ? id : undefined; +} + +export function actorSubject(request: Indexable): string | undefined { + const scope = resolveScope(request); + if (isUsableSubject(scope)) return scope; + const userId = resolveUserId(request); + return userId ? `User:${userId}` : undefined; +} + +export function rethrowGrpcOrInternal( + error: unknown, + fallback = 'Something went wrong', +): never { + if (error instanceof GrpcError) { + throw error; + } + throw new GrpcError(status.INTERNAL, (error as Error).message ?? fallback); +} + +export function isMissingRelationError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error); + return ( + message.includes('No relations found') || message.includes('Relation does not exist') + ); +} + +export async function ignoreMissingRelation( + work: () => Promise, +): Promise { + try { + return await work(); + } catch (error) { + if (isMissingRelationError(error)) { + return; + } + throw error; + } +} diff --git a/modules/storage/src/authz/index.ts b/modules/storage/src/authz/index.ts index ab3a35dc8..7ad644ef1 100644 --- a/modules/storage/src/authz/index.ts +++ b/modules/storage/src/authz/index.ts @@ -1,23 +1,39 @@ import { ConduitAuthorizedResource } from '@conduitplatform/grpc-sdk'; +const storageRelations = { + owner: ['*'], + reader: ['*'], + editor: ['*'], +}; + +const storagePermissions = { + read: [ + 'owner', + 'reader', + 'editor', + 'reader->read', + 'editor->edit', + 'owner->read', + 'owner->edit', + ], + edit: ['owner', 'editor', 'editor->edit', 'owner->edit'], + delete: ['owner', 'owner->edit'], +}; + +export const ContainerResource = new ConduitAuthorizedResource( + 'Container', + storageRelations, + storagePermissions, +); + +export const FolderResource = new ConduitAuthorizedResource( + 'Folder', + storageRelations, + storagePermissions, +); + export const FileResource = new ConduitAuthorizedResource( 'File', - { - owner: ['*'], - reader: ['*'], - editor: ['*'], - }, - { - read: [ - 'owner', - 'reader', - 'editor', - 'reader->read', - 'editor->edit', - 'owner->read', - 'owner->edit', - ], - edit: ['owner', 'editor', 'editor->edit', 'owner->edit'], - delete: ['owner', 'owner->edit'], - }, + storageRelations, + storagePermissions, ); diff --git a/modules/storage/src/authz/relations.test.ts b/modules/storage/src/authz/relations.test.ts new file mode 100644 index 000000000..52e643276 --- /dev/null +++ b/modules/storage/src/authz/relations.test.ts @@ -0,0 +1,314 @@ +import { afterEach, describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { ConfigController } from '@conduitplatform/module-tools'; +import { ConduitGrpcSdk } from '@conduitplatform/grpc-sdk'; +import { _StorageContainer, _StorageFolder } from '../models/index.js'; +import { + createContainerOwnerRelation, + createFileRelations, + createFolderOwnerRelations, + createOwnerRelation, + forEachDocumentPage, + updateFileRelations, +} from './relations.js'; + +const originalConfig = ConfigController.getInstance().config; +const originalContainerGetInstance = + _StorageContainer.getInstance.bind(_StorageContainer); +const originalFolderGetInstance = _StorageFolder.getInstance.bind(_StorageFolder); + +afterEach(() => { + ConfigController.getInstance().config = originalConfig; + _StorageContainer.getInstance = originalContainerGetInstance; + _StorageFolder.getInstance = originalFolderGetInstance; +}); + +function enableAuthz(defaultContainer = 'conduit') { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer, + }; +} + +function disableAuthz() { + ConfigController.getInstance().config = { + authorization: { enabled: false }, + defaultContainer: 'conduit', + }; +} + +function stubLookups(args: { + containers?: Array<{ _id: string; name: string }>; + folders?: Array<{ _id: string; name: string; container: string }>; +}) { + _StorageContainer.getInstance = (() => ({ + findOne: async (query: { name?: string }) => + args.containers?.find(doc => doc.name === query.name) ?? null, + })) as unknown as typeof _StorageContainer.getInstance; + _StorageFolder.getInstance = (() => ({ + findOne: async (query: { name?: string; container?: string }) => + args.folders?.find( + doc => doc.name === query.name && doc.container === query.container, + ) ?? null, + })) as unknown as typeof _StorageFolder.getInstance; +} + +function fakeSdk() { + const created: Array<{ subject: string; relation: string; resource: string }> = []; + const deleted: Array<{ subject: string; relation: string; resource: string }> = []; + const grpcSdk = { + authorization: { + createRelation: async (relation: { + subject: string; + relation: string; + resource: string; + }) => { + created.push(relation); + }, + deleteRelation: async (relation: { + subject: string; + relation: string; + resource: string; + }) => { + deleted.push(relation); + }, + }, + } as unknown as ConduitGrpcSdk; + return { grpcSdk, created, deleted }; +} + +describe('relation subjects', () => { + it('never creates a relation with an undefined subject', async () => { + enableAuthz(); + const { grpcSdk, created } = fakeSdk(); + await createOwnerRelation(grpcSdk, undefined, 'File:1'); + await createOwnerRelation(grpcSdk, '', 'File:1'); + assert.deepEqual(created, []); + }); + + it('does not own the default container even when a scope is provided', async () => { + enableAuthz(); + const { grpcSdk, created } = fakeSdk(); + await createContainerOwnerRelation( + grpcSdk, + { _id: 'c1', name: 'conduit' } as never, + 'Team:t1', + ); + assert.deepEqual(created, []); + }); + + it('attaches scope to a non-default container', async () => { + enableAuthz(); + const { grpcSdk, created } = fakeSdk(); + await createContainerOwnerRelation( + grpcSdk, + { _id: 'c2', name: 'team-bucket' } as never, + 'Team:t1', + ); + assert.deepEqual(created, [ + { subject: 'Team:t1', relation: 'owner', resource: 'Container:c2' }, + ]); + }); + + it('skips all relation writes when authz is disabled', async () => { + disableAuthz(); + const { grpcSdk, created } = fakeSdk(); + await createOwnerRelation(grpcSdk, 'User:1', 'File:1'); + assert.deepEqual(created, []); + }); +}); + +describe('file relation tree', () => { + it('attaches container + scope for a root file', async () => { + enableAuthz(); + stubLookups({ containers: [{ _id: 'c1', name: 'photos' }] }); + const { grpcSdk, created } = fakeSdk(); + await createFileRelations( + grpcSdk, + { _id: 'f1', container: 'photos', folder: '/' } as never, + { scope: 'Team:t1' }, + ); + assert.deepEqual(created, [ + { subject: 'Container:c1', relation: 'owner', resource: 'File:f1' }, + { subject: 'Team:t1', relation: 'owner', resource: 'File:f1' }, + ]); + }); + + it('attaches the creating user when a root file has no scope', async () => { + enableAuthz(); + stubLookups({ containers: [{ _id: 'c1', name: 'conduit' }] }); + const { grpcSdk, created } = fakeSdk(); + await createFileRelations( + grpcSdk, + { _id: 'f1', container: 'conduit', folder: '/' } as never, + { userId: 'u1' }, + ); + assert.deepEqual(created, [ + { subject: 'Container:c1', relation: 'owner', resource: 'File:f1' }, + { subject: 'User:u1', relation: 'owner', resource: 'File:f1' }, + ]); + }); + + it('attaches only the folder owner for a nested file without scope', async () => { + enableAuthz(); + stubLookups({ + folders: [{ _id: 'dir1', name: 'cnd_u1/', container: 'conduit' }], + }); + const { grpcSdk, created } = fakeSdk(); + await createFileRelations( + grpcSdk, + { _id: 'f1', container: 'conduit', folder: 'cnd_u1/' } as never, + { userId: 'u1' }, + ); + assert.deepEqual(created, [ + { subject: 'Folder:dir1', relation: 'owner', resource: 'File:f1' }, + ]); + }); +}); + +describe('move owners', () => { + it('rewires container-only moves at /', async () => { + enableAuthz(); + stubLookups({ + containers: [ + { _id: 'c1', name: 'old' }, + { _id: 'c2', name: 'new' }, + ], + }); + const { grpcSdk, created, deleted } = fakeSdk(); + await updateFileRelations( + grpcSdk, + { _id: 'f1', container: 'old', folder: '/' }, + { _id: 'f1', container: 'new', folder: '/' }, + ); + assert.deepEqual(deleted, [ + { subject: 'Container:c1', relation: 'owner', resource: 'File:f1' }, + ]); + assert.deepEqual(created, [ + { subject: 'Container:c2', relation: 'owner', resource: 'File:f1' }, + ]); + }); + + it('moves / to a folder by dropping the container owner', async () => { + enableAuthz(); + stubLookups({ + containers: [{ _id: 'c1', name: 'conduit' }], + folders: [{ _id: 'dir1', name: 'docs/', container: 'conduit' }], + }); + const { grpcSdk, created, deleted } = fakeSdk(); + await updateFileRelations( + grpcSdk, + { _id: 'f1', container: 'conduit', folder: '/' }, + { _id: 'f1', container: 'conduit', folder: 'docs/' }, + { scope: 'Team:t1' }, + ); + assert.deepEqual(deleted, [ + { subject: 'Container:c1', relation: 'owner', resource: 'File:f1' }, + ]); + assert.deepEqual(created, [ + { subject: 'Folder:dir1', relation: 'owner', resource: 'File:f1' }, + { subject: 'Team:t1', relation: 'owner', resource: 'File:f1' }, + ]); + }); + + it('moves a folder file back to / by attaching the container', async () => { + enableAuthz(); + stubLookups({ + containers: [{ _id: 'c1', name: 'conduit' }], + folders: [{ _id: 'dir1', name: 'docs/', container: 'conduit' }], + }); + const { grpcSdk, created, deleted } = fakeSdk(); + await updateFileRelations( + grpcSdk, + { _id: 'f1', container: 'conduit', folder: 'docs/' }, + { _id: 'f1', container: 'conduit', folder: '/' }, + ); + assert.deepEqual(deleted, [ + { subject: 'Folder:dir1', relation: 'owner', resource: 'File:f1' }, + ]); + assert.deepEqual(created, [ + { subject: 'Container:c1', relation: 'owner', resource: 'File:f1' }, + ]); + }); + + it('treats the same folder path in another container as a new folder owner', async () => { + enableAuthz(); + stubLookups({ + folders: [ + { _id: 'dir-old', name: 'docs/', container: 'old' }, + { _id: 'dir-new', name: 'docs/', container: 'new' }, + ], + }); + const { grpcSdk, created, deleted } = fakeSdk(); + await updateFileRelations( + grpcSdk, + { _id: 'f1', container: 'old', folder: 'docs/' }, + { _id: 'f1', container: 'new', folder: 'docs/' }, + ); + assert.deepEqual(deleted, [ + { subject: 'Folder:dir-old', relation: 'owner', resource: 'File:f1' }, + ]); + assert.deepEqual(created, [ + { subject: 'Folder:dir-new', relation: 'owner', resource: 'File:f1' }, + ]); + }); +}); + +describe('folder owners', () => { + it('owns the first folder with the container only when admin omits scope', async () => { + enableAuthz(); + const { grpcSdk, created } = fakeSdk(); + await createFolderOwnerRelations(grpcSdk, { _id: 'dir1' } as never, { + isFirst: true, + containerId: 'c1', + }); + assert.deepEqual(created, [ + { subject: 'Container:c1', relation: 'owner', resource: 'Folder:dir1' }, + ]); + assert.equal( + created.some(relation => relation.subject === 'undefined'), + false, + ); + }); + + it('attaches scope on a first folder and parent folder on nested folders', async () => { + enableAuthz(); + const { grpcSdk, created } = fakeSdk(); + await createFolderOwnerRelations(grpcSdk, { _id: 'dir1' } as never, { + isFirst: true, + containerId: 'c1', + scope: 'Team:t1', + }); + await createFolderOwnerRelations(grpcSdk, { _id: 'dir2' } as never, { + isFirst: false, + containerId: 'c1', + parentFolderId: 'dir1', + }); + assert.deepEqual(created, [ + { subject: 'Container:c1', relation: 'owner', resource: 'Folder:dir1' }, + { subject: 'Team:t1', relation: 'owner', resource: 'Folder:dir1' }, + { subject: 'Folder:dir1', relation: 'owner', resource: 'Folder:dir2' }, + ]); + }); +}); + +describe('paginated relation cleanup', () => { + it('walks pages instead of loading every id at once', async () => { + const pages = [[{ _id: '1' }, { _id: '2' }], [{ _id: '3' }]]; + const seen: string[][] = []; + let calls = 0; + await forEachDocumentPage( + async (skip, limit) => { + assert.equal(limit, 2); + calls += 1; + return pages[skip / 2] ?? []; + }, + async docs => { + seen.push(docs.map(doc => doc._id)); + }, + 2, + ); + assert.equal(calls, 2); + assert.deepEqual(seen, [['1', '2'], ['3']]); + }); +}); diff --git a/modules/storage/src/authz/relations.ts b/modules/storage/src/authz/relations.ts new file mode 100644 index 000000000..8c1531b25 --- /dev/null +++ b/modules/storage/src/authz/relations.ts @@ -0,0 +1,193 @@ +import { ConduitGrpcSdk, GrpcError } from '@conduitplatform/grpc-sdk'; +import { status } from '@grpc/grpc-js'; +import { _StorageContainer, _StorageFolder, File } from '../models/index.js'; +import { + ignoreMissingRelation, + isAuthzEnabled, + isDefaultContainer, + isUsableSubject, + RELATION_PAGE_SIZE, +} from './helpers.js'; + +export async function createOwnerRelation( + grpcSdk: ConduitGrpcSdk, + subject: string | undefined, + resource: string, +): Promise { + if (!isAuthzEnabled() || !isUsableSubject(subject)) { + return; + } + await grpcSdk.authorization?.createRelation({ + subject, + relation: 'owner', + resource, + }); +} + +export async function deleteOwnerRelation( + grpcSdk: ConduitGrpcSdk, + subject: string | undefined, + resource: string, +): Promise { + if (!isAuthzEnabled() || !isUsableSubject(subject)) { + return; + } + await ignoreMissingRelation(() => + grpcSdk.authorization!.deleteRelation({ + subject, + relation: 'owner', + resource, + }), + ); +} + +export async function deleteAllRelationsSafe( + grpcSdk: ConduitGrpcSdk, + query: { subject?: string; resource?: string }, +): Promise { + if (!isAuthzEnabled()) { + return; + } + await ignoreMissingRelation(() => grpcSdk.authorization!.deleteAllRelations(query)); +} + +export async function resolveStructuralOwner( + file: Pick, +): Promise { + if (file.folder === '/') { + const containerDoc = await _StorageContainer + .getInstance() + .findOne({ name: file.container }); + if (!containerDoc) { + throw new GrpcError(status.NOT_FOUND, 'Container does not exist'); + } + return `Container:${containerDoc._id}`; + } + const folderDoc = await _StorageFolder + .getInstance() + .findOne({ name: file.folder, container: file.container }); + if (!folderDoc) { + throw new GrpcError(status.NOT_FOUND, 'Folder does not exist'); + } + return `Folder:${folderDoc._id}`; +} + +export async function createFileRelations( + grpcSdk: ConduitGrpcSdk, + file: File, + options?: { scope?: string; userId?: string }, +): Promise { + if (!isAuthzEnabled()) { + return; + } + const structuralOwner = await resolveStructuralOwner(file); + await createOwnerRelation(grpcSdk, structuralOwner, `File:${file._id}`); + if (isUsableSubject(options?.scope)) { + await createOwnerRelation(grpcSdk, options.scope, `File:${file._id}`); + return; + } + if (file.folder === '/' && options?.userId) { + await createOwnerRelation(grpcSdk, `User:${options.userId}`, `File:${file._id}`); + } +} + +export async function updateFileRelations( + grpcSdk: ConduitGrpcSdk, + previous: Pick, + updated: Pick, + options?: { scope?: string }, +): Promise { + if (!isAuthzEnabled()) { + return; + } + const samePlace = + previous.container === updated.container && previous.folder === updated.folder; + if (!samePlace) { + const oldOwner = await resolveStructuralOwner(previous); + const newOwner = await resolveStructuralOwner(updated); + if (oldOwner !== newOwner) { + await deleteOwnerRelation(grpcSdk, oldOwner, `File:${updated._id}`); + await createOwnerRelation(grpcSdk, newOwner, `File:${updated._id}`); + } + } + if (isUsableSubject(options?.scope)) { + await createOwnerRelation(grpcSdk, options.scope, `File:${updated._id}`); + } +} + +export async function createFolderOwnerRelations( + grpcSdk: ConduitGrpcSdk, + folder: _StorageFolder, + options: { + isFirst: boolean; + containerId: string; + parentFolderId?: string; + scope?: string; + }, +): Promise { + if (!isAuthzEnabled()) { + return; + } + const resource = `Folder:${folder._id}`; + if (options.isFirst) { + await createOwnerRelation(grpcSdk, `Container:${options.containerId}`, resource); + await createOwnerRelation(grpcSdk, options.scope, resource); + return; + } + if (!options.parentFolderId) { + throw new GrpcError(status.INTERNAL, 'Parent folder is required for nested folders'); + } + await createOwnerRelation(grpcSdk, `Folder:${options.parentFolderId}`, resource); +} + +export async function createContainerOwnerRelation( + grpcSdk: ConduitGrpcSdk, + container: _StorageContainer, + scope?: string, +): Promise { + if ( + !isAuthzEnabled() || + isDefaultContainer(container.name) || + !isUsableSubject(scope) + ) { + return; + } + await createOwnerRelation(grpcSdk, scope, `Container:${container._id}`); +} + +export async function forEachDocumentPage( + fetchPage: (skip: number, limit: number) => Promise, + handler: (docs: T[]) => Promise, + pageSize: number = RELATION_PAGE_SIZE, +): Promise { + let skip = 0; + while (true) { + const page = await fetchPage(skip, pageSize); + if (page.length === 0) { + return; + } + await handler(page); + if (page.length < pageSize) { + return; + } + skip += pageSize; + } +} + +export async function deleteRelationsForResources( + grpcSdk: ConduitGrpcSdk, + resources: string[], +): Promise { + await Promise.all( + resources.map(resource => deleteAllRelationsSafe(grpcSdk, { resource })), + ); +} + +export async function deleteRelationsForSubjects( + grpcSdk: ConduitGrpcSdk, + subjects: string[], +): Promise { + await Promise.all( + subjects.map(subject => deleteAllRelationsSafe(grpcSdk, { subject })), + ); +} diff --git a/modules/storage/src/handlers/file.authz.test.ts b/modules/storage/src/handlers/file.authz.test.ts new file mode 100644 index 000000000..64ae75051 --- /dev/null +++ b/modules/storage/src/handlers/file.authz.test.ts @@ -0,0 +1,291 @@ +import { afterEach, describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { status } from '@grpc/grpc-js'; +import { + ConduitGrpcSdk, + GrpcError, + ParsedRouterRequest, +} from '@conduitplatform/grpc-sdk'; +import { ConfigController } from '@conduitplatform/module-tools'; +import { _StorageContainer, File } from '../models/index.js'; +import { FileHandlers } from './file.js'; + +const originalConfig = ConfigController.getInstance().config; +const originalContainerGetInstance = + _StorageContainer.getInstance.bind(_StorageContainer); +const originalFileGetInstance = File.getInstance.bind(File); + +afterEach(() => { + ConfigController.getInstance().config = originalConfig; + _StorageContainer.getInstance = originalContainerGetInstance; + File.getInstance = originalFileGetInstance; +}); + +function request( + overrides: Partial = {}, +): ParsedRouterRequest { + return { + request: { + params: {}, + urlParams: {}, + queryParams: {}, + bodyParams: {}, + path: '', + headers: {}, + rawHeaders: [], + rawBody: Buffer.alloc(0), + context: {}, + cookies: {}, + ...overrides, + }, + } as ParsedRouterRequest; +} + +function stubModels(args: { + containers?: Array<{ _id: string; name: string }>; + files?: Array>; +}) { + _StorageContainer.getInstance = (() => ({ + findOne: async (query: { name?: string }) => + args.containers?.find(doc => doc.name === query.name) ?? null, + findMany: async (query: { name?: { $in: string[] } }) => { + const names = query.name?.$in; + return names + ? (args.containers ?? []).filter(doc => names.includes(doc.name)) + : (args.containers ?? []); + }, + })) as unknown as typeof _StorageContainer.getInstance; + File.getInstance = (() => ({ + findOne: async (query: { _id?: string }) => + args.files?.find(doc => doc._id === query._id) ?? null, + deleteOne: async () => undefined, + })) as unknown as typeof File.getInstance; +} + +function handlers(authz: { + can?: (input: { actions: string[]; resource: string }) => Promise<{ allow: boolean }>; + deleteAllRelations?: () => Promise; +}) { + const grpcSdk = { + databaseProvider: {}, + authorization: { + can: authz.can ?? (async () => ({ allow: true })), + deleteAllRelations: authz.deleteAllRelations ?? (async () => undefined), + createRelation: async () => undefined, + }, + } as unknown as ConduitGrpcSdk; + const storage = { + container: () => ({ + delete: async () => true, + getSignedUrl: async () => 'https://signed.example/file', + }), + }; + return new FileHandlers(grpcSdk, storage as never); +} + +describe('client container create', () => { + it('returns 404 for a missing container even when allowContainerCreation is true', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + allowContainerCreation: true, + }; + stubModels({ containers: [{ _id: 'c1', name: 'conduit' }] }); + const fileHandlers = handlers({}); + await assert.rejects( + () => + fileHandlers.createFile( + request({ + params: { name: 'a.txt', data: 'Zg==', container: 'missing' }, + context: { user: { _id: 'u1' } }, + }), + ), + (error: unknown) => + error instanceof GrpcError && + error.code === status.NOT_FOUND && + error.message === 'Container does not exist', + ); + }); +}); + +describe('safe user access', () => { + it('denies a private get without a user instead of throwing TypeError', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + }; + stubModels({ + files: [ + { + _id: 'file-1', + isPublic: false, + name: 'secret.txt', + container: 'conduit', + folder: '/', + }, + ], + }); + const fileHandlers = handlers({}); + await assert.rejects( + () => fileHandlers.getFile(request({ params: { id: 'file-1' }, context: {} })), + (error: unknown) => + error instanceof GrpcError && error.code === status.PERMISSION_DENIED, + ); + }); + + it('allows public file read without auth', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + }; + stubModels({ + containers: [{ _id: 'c1', name: 'public' }], + files: [ + { + _id: 'file-2', + isPublic: true, + name: 'banner.png', + container: 'public', + folder: '/', + url: 'https://cdn.example/banner.png', + }, + ], + }); + const fileHandlers = handlers({ + can: async () => { + throw new Error('authz should not run for public reads'); + }, + }); + const result = (await fileHandlers.getFile( + request({ params: { id: 'file-2' }, context: {} }), + )) as { _id: string }; + assert.equal(result._id, 'file-2'); + }); +}); + +describe('denied delete status', () => { + it('keeps PERMISSION_DENIED instead of wrapping it as INTERNAL 500', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + }; + stubModels({ + files: [ + { + _id: 'file-3', + isPublic: false, + name: 'a.txt', + container: 'conduit', + folder: '/', + }, + ], + }); + const fileHandlers = handlers({ + can: async () => ({ allow: false }), + }); + await assert.rejects( + () => + fileHandlers.deleteFile( + request({ + params: { id: 'file-3' }, + context: { user: { _id: 'u1' } }, + }), + ), + (error: unknown) => + error instanceof GrpcError && error.code === status.PERMISSION_DENIED, + ); + }); +}); + +describe('gRPC deleteFile id', () => { + it('deletes using params.id when urlParams is empty', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: false }, + defaultContainer: 'conduit', + }; + let deletedId: string | undefined; + File.getInstance = (() => ({ + findOne: async (query: { _id?: string }) => + query._id === 'grpc-file' + ? { + _id: 'grpc-file', + container: 'conduit', + folder: '/', + name: 'a.txt', + size: 1, + } + : null, + deleteOne: async (query: { _id?: string }) => { + deletedId = query._id; + }, + })) as unknown as typeof File.getInstance; + const fileHandlers = handlers({}); + const result = (await fileHandlers.deleteFile( + request({ + params: { id: 'grpc-file' }, + urlParams: {}, + context: { user: { _id: 'u1' } }, + }), + )) as { success: boolean }; + assert.equal(result.success, true); + assert.equal(deletedId, 'grpc-file'); + }); +}); + +describe('scope and team', () => { + it('denies create when the user cannot edit the given scope', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + }; + stubModels({ containers: [{ _id: 'c1', name: 'conduit' }] }); + const fileHandlers = handlers({ + can: async input => ({ allow: input.resource !== 'Team:t1' }), + }); + await assert.rejects( + () => + fileHandlers.fileAccessCheck( + 'create', + request({ + params: { scope: 'Team:t1' }, + queryParams: { scope: 'Team:t1' }, + context: { user: { _id: 'u1' } }, + }).request, + undefined, + 'conduit', + ), + (error: unknown) => + error instanceof GrpcError && + error.code === status.PERMISSION_DENIED && + error.message === 'You are not allowed to create files in this scope', + ); + }); +}); + +describe('authz disabled', () => { + it('does not call authorization.can during create access checks', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: false }, + defaultContainer: 'conduit', + allowContainerCreation: true, + }; + stubModels({ containers: [{ _id: 'c1', name: 'conduit' }] }); + let canCalls = 0; + const fileHandlers = handlers({ + can: async () => { + canCalls += 1; + return { allow: false }; + }, + }); + await fileHandlers.fileAccessCheck( + 'create', + request({ + params: {}, + context: { user: { _id: 'u1' } }, + }).request, + undefined, + 'conduit', + ); + assert.equal(canCalls, 0); + }); +}); diff --git a/modules/storage/src/handlers/file.ts b/modules/storage/src/handlers/file.ts index 696591c22..651cafecb 100644 --- a/modules/storage/src/handlers/file.ts +++ b/modules/storage/src/handlers/file.ts @@ -2,7 +2,6 @@ import { ConduitGrpcSdk, DatabaseProvider, GrpcError, - Indexable, ParsedRouterRequest, UnparsedRouterResponse, } from '@conduitplatform/grpc-sdk'; @@ -16,13 +15,32 @@ import { _updateFile, _updateFileUploadUrl, applyCdnHost, - deepPathHandler, normalizeFolderPath, resolvePublicFileAccessUrl, sanitizeFileForResponse, storeNewFile, validateName, } from '../utils/index.js'; +import { + actorSubject, + isAuthzEnabled, + isDefaultContainer, + rethrowGrpcOrInternal, + resolveClientFolder, + resolveFileId, + resolveScope, + resolveUserId, +} from '../authz/helpers.js'; +import { + assertFolderEditAccess, + assertNoPersonalFolderSquat, + findOrCreateFolders, +} from '../authz/folders.js'; +import { + createFileRelations, + deleteAllRelationsSafe, + updateFileRelations, +} from '../authz/relations.js'; export class FileHandlers { private readonly database: DatabaseProvider; @@ -49,18 +67,25 @@ export class FileHandlers { async fileAccessCheck( action: 'read' | 'create' | 'edit' | 'delete', - request: Indexable, + request: ParsedRouterRequest['request'], file?: File, + container?: string, ) { - if (!request.context.user) { + const userId = resolveUserId(request); + if (!userId) { throw new GrpcError(status.PERMISSION_DENIED, 'File access is not public'); } - if (ConfigController.getInstance().config.authorization.enabled) { - if (action === 'create' && request.queryParams.scope) { + if (!isAuthzEnabled()) { + return; + } + + const scope = resolveScope(request); + if (action === 'create') { + if (scope) { const allowed = await this.grpcSdk.authorization?.can({ - subject: `User:${request.context.user._id}`, - actions: ['read'], - resource: request.params.scope, + subject: `User:${userId}`, + actions: ['edit'], + resource: scope, }); if (!allowed || !allowed.allow) { throw new GrpcError( @@ -69,44 +94,43 @@ export class FileHandlers { ); } } - if (['read', 'edit', 'delete'].includes(action)) { - const allowed = await this.grpcSdk.authorization?.can({ - subject: `User:${request.context.user._id}`, - actions: [action], - resource: `File:${file!._id}`, + if (container && !isDefaultContainer(container)) { + const containerDoc = await _StorageContainer.getInstance().findOne({ + name: container, }); - if (!allowed || !allowed.allow) { - throw new GrpcError(status.PERMISSION_DENIED, 'You do not have access to file'); + if (!containerDoc) { + throw new GrpcError(status.NOT_FOUND, 'Container does not exist'); } - } - } - } - - async fileAccessAdd(file: File, request: Indexable) { - if (ConfigController.getInstance().config.authorization.enabled) { - if (request.queryParams.scope) { const allowed = await this.grpcSdk.authorization?.can({ - subject: `User:${request.context.user._id}`, - actions: ['read'], - resource: request.params.scope, + subject: scope ?? `User:${userId}`, + actions: ['edit'], + resource: `Container:${containerDoc._id}`, }); if (!allowed || !allowed.allow) { throw new GrpcError( status.PERMISSION_DENIED, - 'You are not allowed to create files in this scope', + 'You are not allowed to create files in this container', ); } } - await this.grpcSdk.authorization?.createRelation({ - subject: request.params.scope ?? `User:${request.context.user._id}`, - relation: 'owner', - resource: `File:${file._id}`, - }); + return; + } + + const allowed = await this.grpcSdk.authorization?.can({ + subject: `User:${userId}`, + actions: [action], + resource: `File:${file!._id}`, + }); + if (!allowed || !allowed.allow) { + throw new GrpcError( + status.PERMISSION_DENIED, + `You are not allowed to ${action} this file`, + ); } } async getFile(call: ParsedRouterRequest): Promise { - const file = await File.getInstance().findOne({ _id: call.request.params.id }); + const file = await File.getInstance().findOne({ _id: resolveFileId(call.request) }); if (isNil(file)) { throw new GrpcError(status.NOT_FOUND, 'File does not exist'); } @@ -119,15 +143,14 @@ export class FileHandlers { async createFile(call: ParsedRouterRequest): Promise { const { name, alias, data, container, mimeType, isPublic } = call.request.params; - await this.fileAccessCheck('create', call.request); - const folder = normalizeFolderPath(call.request.params.folder); - const config = ConfigController.getInstance().config; - const usedContainer = isNil(container) - ? config.defaultContainer - : await this.findOrCreateContainer(container, isPublic); - if (folder !== '/') { - await this.findOrCreateFolders(folder, usedContainer, isPublic); + const userId = resolveUserId(call.request); + if (!userId) { + throw new GrpcError(status.PERMISSION_DENIED, 'File access is not public'); } + const usedContainer = await this.resolveClientContainer(container); + await this.fileAccessCheck('create', call.request, undefined, usedContainer); + const folder = resolveClientFolder(call.request.params.folder, userId); + await this.prepareClientFolder(call, usedContainer, folder, isPublic); const validatedName = await validateName(name, folder, usedContainer); try { const file = await storeNewFile(this.storageProvider, { @@ -139,27 +162,26 @@ export class FileHandlers { isPublic, mimeType, }); - await this.fileAccessAdd(file, call.request); + await createFileRelations(this.grpcSdk, file, { + scope: resolveScope(call.request), + userId, + }); return file; } catch (e) { - throw new GrpcError( - status.INTERNAL, - (e as Error).message ?? 'Something went wrong', - ); + rethrowGrpcOrInternal(e); } } async createFileUploadUrl(call: ParsedRouterRequest): Promise { const { name, alias, container, size = 0, mimeType, isPublic } = call.request.params; - await this.fileAccessCheck('create', call.request); - const folder = normalizeFolderPath(call.request.params.folder); - const config = ConfigController.getInstance().config; - const usedContainer = isNil(container) - ? config.defaultContainer - : await this.findOrCreateContainer(container, isPublic); - if (folder !== '/') { - await this.findOrCreateFolders(folder, usedContainer, isPublic); + const userId = resolveUserId(call.request); + if (!userId) { + throw new GrpcError(status.PERMISSION_DENIED, 'File access is not public'); } + const usedContainer = await this.resolveClientContainer(container); + await this.fileAccessCheck('create', call.request, undefined, usedContainer); + const folder = resolveClientFolder(call.request.params.folder, userId); + await this.prepareClientFolder(call, usedContainer, folder, isPublic); const validatedName = await validateName(name, folder, usedContainer); try { const { file, url } = await _createFileUploadUrl(this.storageProvider, { @@ -171,19 +193,19 @@ export class FileHandlers { size, mimeType, }); - await this.fileAccessAdd(file, call.request); + await createFileRelations(this.grpcSdk, file, { + scope: resolveScope(call.request), + userId, + }); return { file, url }; } catch (e) { - throw new GrpcError( - status.INTERNAL, - (e as Error).message ?? 'Something went wrong', - ); + rethrowGrpcOrInternal(e); } } async updateFileUploadUrl(call: ParsedRouterRequest): Promise { - const { id, alias, mimeType, size } = call.request.params; - const found = await File.getInstance().findOne({ _id: id }); + const { alias, mimeType, size } = call.request.params; + const found = await File.getInstance().findOne({ _id: resolveFileId(call.request) }); if (isNil(found)) { throw new GrpcError(status.NOT_FOUND, 'File does not exist'); } @@ -193,7 +215,7 @@ export class FileHandlers { found, ); try { - return await _updateFileUploadUrl(this.storageProvider, found, { + const result = await _updateFileUploadUrl(this.storageProvider, found, { name, alias, folder, @@ -201,17 +223,18 @@ export class FileHandlers { mimeType: mimeType ?? found.mimeType, size, }); + await updateFileRelations(this.grpcSdk, found, result.file, { + scope: resolveScope(call.request), + }); + return result; } catch (e) { - throw new GrpcError( - status.INTERNAL, - (e as Error).message ?? 'Something went wrong', - ); + rethrowGrpcOrInternal(e); } } async updateFile(call: ParsedRouterRequest): Promise { - const { id, alias, data, mimeType } = call.request.params; - const found = await File.getInstance().findOne({ _id: id }); + const { alias, data, mimeType } = call.request.params; + const found = await File.getInstance().findOne({ _id: resolveFileId(call.request) }); if (isNil(found)) { throw new GrpcError(status.NOT_FOUND, 'File does not exist'); } @@ -221,28 +244,30 @@ export class FileHandlers { found, ); try { - return await _updateFile(this.storageProvider, found, { + const updated = (await _updateFile(this.storageProvider, found, { name, alias, folder, container, data: Buffer.from(data, 'base64'), mimeType: mimeType ?? found.mimeType, + })) as File; + await updateFileRelations(this.grpcSdk, found, updated, { + scope: resolveScope(call.request), }); + return updated; } catch (e) { - throw new GrpcError( - status.INTERNAL, - (e as Error).message ?? 'Something went wrong!', - ); + rethrowGrpcOrInternal(e); } } async deleteFile(call: ParsedRouterRequest): Promise { - if (!isString(call.request.params.id)) { + const id = resolveFileId(call.request); + if (!isString(id)) { throw new GrpcError(status.INVALID_ARGUMENT, 'The provided id is invalid'); } try { - const found = await File.getInstance().findOne({ _id: call.request.params.id }); + const found = await File.getInstance().findOne({ _id: id }); if (isNil(found)) { throw new GrpcError(status.NOT_FOUND, 'File does not exist'); } @@ -253,21 +278,21 @@ export class FileHandlers { if (!success) { throw new GrpcError(status.INTERNAL, 'File could not be deleted'); } - await File.getInstance().deleteOne({ _id: call.request.params.id }); + await File.getInstance().deleteOne({ _id: id }); ConduitGrpcSdk.Metrics?.decrement('files_total'); ConduitGrpcSdk.Metrics?.decrement('storage_size_bytes_total', found.size); + await deleteAllRelationsSafe(this.grpcSdk, { resource: `File:${id}` }); return { success: true }; } catch (e) { - throw new GrpcError( - status.INTERNAL, - (e as Error).message ?? 'Something went wrong!', - ); + rethrowGrpcOrInternal(e); } } async getFileUrl(call: ParsedRouterRequest): Promise { try { - const found = await File.getInstance().findOne({ _id: call.request.params.id }); + const found = await File.getInstance().findOne({ + _id: resolveFileId(call.request) ?? call.request.params.id, + }); if (isNil(found)) { throw new GrpcError(status.NOT_FOUND, 'File does not exist'); } @@ -293,19 +318,17 @@ export class FileHandlers { } return { redirect: url }; } catch (e) { - throw new GrpcError( - status.INTERNAL, - (e as Error).message ?? 'Something went wrong!', - ); + rethrowGrpcOrInternal(e); } } async getFileData(call: ParsedRouterRequest): Promise { - if (!isString(call.request.params.id)) { + const id = resolveFileId(call.request); + if (!isString(id)) { throw new GrpcError(status.INVALID_ARGUMENT, 'The provided id is invalid'); } try { - const file = await File.getInstance().findOne({ _id: call.request.params.id }); + const file = await File.getInstance().findOne({ _id: id }); if (isNil(file)) { throw new GrpcError(status.NOT_FOUND, 'File does not exist'); } @@ -323,69 +346,42 @@ export class FileHandlers { } return { data: data.toString('base64') }; } catch (e) { - throw new GrpcError( - status.INTERNAL, - (e as Error).message ?? 'Something went wrong!', - ); + rethrowGrpcOrInternal(e); } } - private async findOrCreateContainer( - container: string, - isPublic?: boolean, - ): Promise { + private async resolveClientContainer(container?: string): Promise { const config = ConfigController.getInstance().config; - // the container is sent from the client - const found = await _StorageContainer.getInstance().findOne({ - name: container, - }); + const name = isNil(container) ? config.defaultContainer : container; + const found = await _StorageContainer.getInstance().findOne({ name }); if (!found) { - if (!config.allowContainerCreation) { - throw new GrpcError( - status.PERMISSION_DENIED, - 'Container creation is not allowed!', - ); - } - const exists = await this.storageProvider.containerExists(container); - if (!exists) { - await this.storageProvider.createContainer(container, isPublic); - } - await _StorageContainer.getInstance().create({ - name: container, - isPublic, - }); + throw new GrpcError(status.NOT_FOUND, 'Container does not exist'); } - return container; + return name; } - async findOrCreateFolders( - folderPath: string, + private async prepareClientFolder( + call: ParsedRouterRequest, container: string, + folder: string, isPublic?: boolean, - lastExistsHandler?: () => void, - ): Promise<_StorageFolder[]> { - const createdFolders: _StorageFolder[] = []; - let folder: _StorageFolder | null = null; - await deepPathHandler(folderPath, async (folderPath, isLast) => { - folder = await _StorageFolder - .getInstance() - .findOne({ name: folderPath, container }); - if (isNil(folder)) { - folder = await _StorageFolder.getInstance().create({ - name: folderPath, - container, - isPublic, - }); - createdFolders.push(folder); - const exists = await this.storage.container(container).folderExists(folderPath); - if (!exists) { - await this.storage.container(container).createFolder(folderPath); - } - } else if (isLast) { - lastExistsHandler?.(); - } + ) { + if (folder === '/') { + return; + } + const userId = resolveUserId(call.request); + if (!userId) { + throw new GrpcError(status.PERMISSION_DENIED, 'File access is not public'); + } + await assertNoPersonalFolderSquat(folder, userId, container); + const subject = actorSubject(call.request); + if (subject) { + await assertFolderEditAccess(this.grpcSdk, container, folder, subject); + } + await findOrCreateFolders(this.grpcSdk, this.storageProvider, folder, container, { + isPublic, + scope: subject, }); - return createdFolders; } private async validateFilenameAndContainer(call: ParsedRouterRequest, file: File) { @@ -393,11 +389,12 @@ export class FileHandlers { const newName = name ?? file.name; const newContainer = container ?? file.container; if (newContainer !== file.container) { - await this.findOrCreateContainer(newContainer); + await this.resolveClientContainer(newContainer); + await this.fileAccessCheck('create', call.request, undefined, newContainer); } const newFolder = isNil(folder) ? file.folder : normalizeFolderPath(folder); if (newFolder !== file.folder && newFolder !== '/') { - await this.findOrCreateFolders(newFolder, newContainer); + await this.prepareClientFolder(call, newContainer, newFolder, file.isPublic); } const isDataUpdate = newName === file.name && diff --git a/modules/storage/src/providers/local/index.ts b/modules/storage/src/providers/local/index.ts index 21c8b5aff..decc2451b 100644 --- a/modules/storage/src/providers/local/index.ts +++ b/modules/storage/src/providers/local/index.ts @@ -329,7 +329,7 @@ export class LocalStorage implements IStorageProvider { } deleteContainer(name: string): Promise { - return this.deleteFolder(name); + return this.container(name).deleteFolder(name); } deleteFolder(name: string): Promise { diff --git a/modules/storage/src/utils/index.ts b/modules/storage/src/utils/index.ts index 4babd0900..489a11fe4 100644 --- a/modules/storage/src/utils/index.ts +++ b/modules/storage/src/utils/index.ts @@ -51,7 +51,7 @@ export function normalizeFolderPath(folderPath?: string) { return `${path.normalize(folderPath.trim()).replace(/^\/|\/$/g, '')}/`; } -function getNestedPaths(inputPath: string): string[] { +export function getNestedPaths(inputPath: string): string[] { const paths: string[] = []; const strippedPath = !inputPath.trim() ? '' @@ -458,10 +458,12 @@ export async function sanitizeFilesForResponse(files: File[]): Promise { } const containerNames = [...new Set(files.map(file => file.container))]; - const containers = await _StorageContainer.getInstance().findMany( - { name: { $in: containerNames } }, - { select: 'name isPublic', readPreference: 'primary' }, - ); + const containers = await _StorageContainer + .getInstance() + .findMany( + { name: { $in: containerNames } }, + { select: 'name isPublic', readPreference: 'primary' }, + ); const containerIsPublic = new Map( containers.map(container => [container.name, container.isPublic ?? false]), ); From ce51f35cefc348d7da32df17a799f6e2190c2dc9 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 9 Sep 2026 08:59:49 +0000 Subject: [PATCH 2/5] refactor(storage): trim leftover authz handler slop Share admin container/folder setup, resolve file ids consistently, and drop the unused getFileUrl fallback. --- modules/storage/README.md | 2 +- modules/storage/src/admin/adminFile.ts | 75 ++++++++++++-------------- modules/storage/src/handlers/file.ts | 7 ++- modules/storage/src/utils/index.ts | 10 ++-- 4 files changed, 44 insertions(+), 50 deletions(-) diff --git a/modules/storage/README.md b/modules/storage/README.md index 332828d35..ce8cb469d 100644 --- a/modules/storage/README.md +++ b/modules/storage/README.md @@ -1,6 +1,6 @@ # Storage -Filesystem-shaped authorization for containers, folders, and files. Client list-files is deferred. +Authorization for containers, folders, and files. Client list-files is deferred. ## Client breaking changes diff --git a/modules/storage/src/admin/adminFile.ts b/modules/storage/src/admin/adminFile.ts index b63aa714a..1ebe14f6b 100644 --- a/modules/storage/src/admin/adminFile.ts +++ b/modules/storage/src/admin/adminFile.ts @@ -53,7 +53,7 @@ export class AdminFileHandlers { } async getFile(call: ParsedRouterRequest): Promise { - const file = await File.getInstance().findOne({ _id: call.request.params.id }); + const file = await File.getInstance().findOne({ _id: resolveFileId(call.request) }); if (isNil(file)) { throw new GrpcError(status.NOT_FOUND, 'File does not exist'); } @@ -65,22 +65,8 @@ export class AdminFileHandlers { const { name, alias, data, container, mimeType, isPublic } = call.request.params; const scope = resolveScope(call.request); const folder = normalizeFolderPath(call.request.params.folder); - const config = ConfigController.getInstance().config; - const usedContainer = isNil(container) - ? config.defaultContainer - : await this.findOrCreateContainer(container, isPublic); - if (folder !== '/') { - await findOrCreateFolders( - this.grpcSdk, - this.storageProvider, - folder, - usedContainer, - { - isPublic, - scope, - }, - ); - } + const usedContainer = await this.resolveAdminContainer(container, isPublic); + await this.ensureAdminFolder(folder, usedContainer, isPublic, scope); const validatedName = await validateName(name, folder, usedContainer); if (!isString(data)) { throw new GrpcError(status.INVALID_ARGUMENT, 'Invalid data provided'); @@ -107,22 +93,8 @@ export class AdminFileHandlers { const { name, alias, container, size = 0, mimeType, isPublic } = call.request.params; const scope = resolveScope(call.request); const folder = normalizeFolderPath(call.request.params.folder); - const config = ConfigController.getInstance().config; - const usedContainer = isNil(container) - ? config.defaultContainer - : await this.findOrCreateContainer(container, isPublic); - if (folder !== '/') { - await findOrCreateFolders( - this.grpcSdk, - this.storageProvider, - folder, - usedContainer, - { - isPublic, - scope, - }, - ); - } + const usedContainer = await this.resolveAdminContainer(container, isPublic); + await this.ensureAdminFolder(folder, usedContainer, isPublic, scope); const validatedName = await validateName(name, folder, usedContainer); try { @@ -225,7 +197,7 @@ export class AdminFileHandlers { async getFileUrl(call: ParsedRouterRequest): Promise { try { - const found = await File.getInstance().findOne({ _id: call.request.params.id }); + const found = await File.getInstance().findOne({ _id: resolveFileId(call.request) }); if (isNil(found)) { throw new GrpcError(status.NOT_FOUND, 'File does not exist'); } @@ -281,6 +253,31 @@ export class AdminFileHandlers { } } + private async resolveAdminContainer( + container?: string, + isPublic?: boolean, + ): Promise { + if (isNil(container)) { + return ConfigController.getInstance().config.defaultContainer; + } + return this.findOrCreateContainer(container, isPublic); + } + + private async ensureAdminFolder( + folder: string, + container: string, + isPublic?: boolean, + scope?: string, + ): Promise { + if (folder === '/') { + return; + } + await findOrCreateFolders(this.grpcSdk, this.storageProvider, folder, container, { + isPublic, + scope, + }); + } + private async findOrCreateContainer( container: string, isPublic?: boolean, @@ -317,15 +314,11 @@ export class AdminFileHandlers { } const newFolder = isNil(folder) ? file.folder : normalizeFolderPath(folder); if (newFolder !== file.folder && newFolder !== '/') { - await findOrCreateFolders( - this.grpcSdk, - this.storageProvider, + await this.ensureAdminFolder( newFolder, newContainer, - { - isPublic: file.isPublic, - scope: resolveScope(call.request), - }, + file.isPublic, + resolveScope(call.request), ); } const isDataUpdate = diff --git a/modules/storage/src/handlers/file.ts b/modules/storage/src/handlers/file.ts index 651cafecb..219ccb2a1 100644 --- a/modules/storage/src/handlers/file.ts +++ b/modules/storage/src/handlers/file.ts @@ -116,10 +116,13 @@ export class FileHandlers { return; } + if (!file) { + throw new GrpcError(status.NOT_FOUND, 'File does not exist'); + } const allowed = await this.grpcSdk.authorization?.can({ subject: `User:${userId}`, actions: [action], - resource: `File:${file!._id}`, + resource: `File:${file._id}`, }); if (!allowed || !allowed.allow) { throw new GrpcError( @@ -291,7 +294,7 @@ export class FileHandlers { async getFileUrl(call: ParsedRouterRequest): Promise { try { const found = await File.getInstance().findOne({ - _id: resolveFileId(call.request) ?? call.request.params.id, + _id: resolveFileId(call.request), }); if (isNil(found)) { throw new GrpcError(status.NOT_FOUND, 'File does not exist'); diff --git a/modules/storage/src/utils/index.ts b/modules/storage/src/utils/index.ts index 489a11fe4..a1030c089 100644 --- a/modules/storage/src/utils/index.ts +++ b/modules/storage/src/utils/index.ts @@ -458,12 +458,10 @@ export async function sanitizeFilesForResponse(files: File[]): Promise { } const containerNames = [...new Set(files.map(file => file.container))]; - const containers = await _StorageContainer - .getInstance() - .findMany( - { name: { $in: containerNames } }, - { select: 'name isPublic', readPreference: 'primary' }, - ); + const containers = await _StorageContainer.getInstance().findMany( + { name: { $in: containerNames } }, + { select: 'name isPublic', readPreference: 'primary' }, + ); const containerIsPublic = new Map( containers.map(container => [container.name, container.isPublic ?? false]), ); From 1246935ce53ad54dec839641b6e10b08d10831ad Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 9 Sep 2026 09:04:29 +0000 Subject: [PATCH 3/5] test(storage): move authz tests into src/__tests__ Keep implementation files in authz/ and handlers/ and collect the authz coverage in one dedicated test folder. --- modules/storage/package.json | 2 +- modules/storage/src/{authz => __tests__}/bootstrap.test.ts | 2 +- modules/storage/src/{authz => __tests__}/cascade.test.ts | 4 ++-- .../storage/src/{handlers => __tests__}/file.authz.test.ts | 2 +- modules/storage/src/{authz => __tests__}/folders.test.ts | 2 +- modules/storage/src/{authz => __tests__}/helpers.test.ts | 2 +- modules/storage/src/{authz => __tests__}/relations.test.ts | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) rename modules/storage/src/{authz => __tests__}/bootstrap.test.ts (97%) rename modules/storage/src/{authz => __tests__}/cascade.test.ts (97%) rename modules/storage/src/{handlers => __tests__}/file.authz.test.ts (99%) rename modules/storage/src/{authz => __tests__}/folders.test.ts (99%) rename modules/storage/src/{authz => __tests__}/helpers.test.ts (99%) rename modules/storage/src/{authz => __tests__}/relations.test.ts (99%) diff --git a/modules/storage/package.json b/modules/storage/package.json index e0baebc4b..58922f13e 100644 --- a/modules/storage/package.json +++ b/modules/storage/package.json @@ -30,7 +30,7 @@ "prepare": "npm run build", "build:docker": "docker build -t ghcr.io/conduitplatform/storage:latest -f ./Dockerfile ../../ && docker push ghcr.io/conduitplatform/storage:latest", "generateTypes": "sh build.sh", - "test": "tsc -p tsconfig.test.json && node --test dist-test/config/config.test.js dist-test/adapter/StorageParamAdapter.test.js dist-test/migrations/fileUriMigration.test.js dist-test/providers/aws/acl.test.js dist-test/providers/google/folderMarkers.test.js dist-test/providers/google/deleteFolder.test.js dist-test/providers/google/publicAccess.test.js dist-test/utils/filePrivacy.test.js dist-test/authz/helpers.test.js dist-test/authz/relations.test.js dist-test/authz/folders.test.js dist-test/authz/cascade.test.js dist-test/authz/bootstrap.test.js dist-test/handlers/file.authz.test.js" + "test": "tsc -p tsconfig.test.json && node --test dist-test/config/config.test.js dist-test/adapter/StorageParamAdapter.test.js dist-test/migrations/fileUriMigration.test.js dist-test/providers/aws/acl.test.js dist-test/providers/google/folderMarkers.test.js dist-test/providers/google/deleteFolder.test.js dist-test/providers/google/publicAccess.test.js dist-test/utils/filePrivacy.test.js dist-test/__tests__/helpers.test.js dist-test/__tests__/relations.test.js dist-test/__tests__/folders.test.js dist-test/__tests__/cascade.test.js dist-test/__tests__/bootstrap.test.js dist-test/__tests__/file.authz.test.js" }, "keywords": [], "author": "", diff --git a/modules/storage/src/authz/bootstrap.test.ts b/modules/storage/src/__tests__/bootstrap.test.ts similarity index 97% rename from modules/storage/src/authz/bootstrap.test.ts rename to modules/storage/src/__tests__/bootstrap.test.ts index 72d84bdcf..44350c806 100644 --- a/modules/storage/src/authz/bootstrap.test.ts +++ b/modules/storage/src/__tests__/bootstrap.test.ts @@ -2,7 +2,7 @@ import { afterEach, describe, it } from 'node:test'; import assert from 'node:assert/strict'; import { ConfigController } from '@conduitplatform/module-tools'; import { _StorageContainer } from '../models/index.js'; -import { ensureDefaultContainer } from './bootstrap.js'; +import { ensureDefaultContainer } from '../authz/bootstrap.js'; const originalConfig = ConfigController.getInstance().config; const originalGetInstance = _StorageContainer.getInstance.bind(_StorageContainer); diff --git a/modules/storage/src/authz/cascade.test.ts b/modules/storage/src/__tests__/cascade.test.ts similarity index 97% rename from modules/storage/src/authz/cascade.test.ts rename to modules/storage/src/__tests__/cascade.test.ts index 7c5def50f..946e72cd6 100644 --- a/modules/storage/src/authz/cascade.test.ts +++ b/modules/storage/src/__tests__/cascade.test.ts @@ -3,8 +3,8 @@ import assert from 'node:assert/strict'; import { ConfigController } from '@conduitplatform/module-tools'; import { ConduitGrpcSdk } from '@conduitplatform/grpc-sdk'; import { _StorageContainer, _StorageFolder, File } from '../models/index.js'; -import { folderPrefixRegex } from './helpers.js'; -import { deleteContainerTree, deleteFolderTree } from './cascade.js'; +import { folderPrefixRegex } from '../authz/helpers.js'; +import { deleteContainerTree, deleteFolderTree } from '../authz/cascade.js'; const originalConfig = ConfigController.getInstance().config; const originalFileGetInstance = File.getInstance.bind(File); diff --git a/modules/storage/src/handlers/file.authz.test.ts b/modules/storage/src/__tests__/file.authz.test.ts similarity index 99% rename from modules/storage/src/handlers/file.authz.test.ts rename to modules/storage/src/__tests__/file.authz.test.ts index 64ae75051..22bd231ba 100644 --- a/modules/storage/src/handlers/file.authz.test.ts +++ b/modules/storage/src/__tests__/file.authz.test.ts @@ -8,7 +8,7 @@ import { } from '@conduitplatform/grpc-sdk'; import { ConfigController } from '@conduitplatform/module-tools'; import { _StorageContainer, File } from '../models/index.js'; -import { FileHandlers } from './file.js'; +import { FileHandlers } from '../handlers/file.js'; const originalConfig = ConfigController.getInstance().config; const originalContainerGetInstance = diff --git a/modules/storage/src/authz/folders.test.ts b/modules/storage/src/__tests__/folders.test.ts similarity index 99% rename from modules/storage/src/authz/folders.test.ts rename to modules/storage/src/__tests__/folders.test.ts index a11ed2ce0..e85bcb192 100644 --- a/modules/storage/src/authz/folders.test.ts +++ b/modules/storage/src/__tests__/folders.test.ts @@ -4,7 +4,7 @@ import { status } from '@grpc/grpc-js'; import { ConduitGrpcSdk, GrpcError } from '@conduitplatform/grpc-sdk'; import { ConfigController } from '@conduitplatform/module-tools'; import { _StorageContainer, _StorageFolder } from '../models/index.js'; -import { assertNoPersonalFolderSquat, findOrCreateFolders } from './folders.js'; +import { assertNoPersonalFolderSquat, findOrCreateFolders } from '../authz/folders.js'; const originalConfig = ConfigController.getInstance().config; const originalContainerGetInstance = diff --git a/modules/storage/src/authz/helpers.test.ts b/modules/storage/src/__tests__/helpers.test.ts similarity index 99% rename from modules/storage/src/authz/helpers.test.ts rename to modules/storage/src/__tests__/helpers.test.ts index 6a4db615c..8eb5d6c19 100644 --- a/modules/storage/src/authz/helpers.test.ts +++ b/modules/storage/src/__tests__/helpers.test.ts @@ -17,7 +17,7 @@ import { resolveScope, resolveUserId, rethrowGrpcOrInternal, -} from './helpers.js'; +} from '../authz/helpers.js'; const originalConfig = ConfigController.getInstance().config; diff --git a/modules/storage/src/authz/relations.test.ts b/modules/storage/src/__tests__/relations.test.ts similarity index 99% rename from modules/storage/src/authz/relations.test.ts rename to modules/storage/src/__tests__/relations.test.ts index 52e643276..b27a41216 100644 --- a/modules/storage/src/authz/relations.test.ts +++ b/modules/storage/src/__tests__/relations.test.ts @@ -10,7 +10,7 @@ import { createOwnerRelation, forEachDocumentPage, updateFileRelations, -} from './relations.js'; +} from '../authz/relations.js'; const originalConfig = ConfigController.getInstance().config; const originalContainerGetInstance = From c9ac5304453dcac298abb6b480f03255f2a833cc Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 15 Sep 2026 11:32:10 +0000 Subject: [PATCH 4/5] fix(storage): make filesystem ReBAC upgrade-safe Skip can(edit) on leftover unowned folders and named containers, heal them on first write, and always stamp scope ?? User on File creates. Deny unmanaged cnd_/ squats. No second flag and no reconstruct job. Old files are not backfilled. Sibling-file A1 scans were explicitly rejected. --- CHANGELOG.md | 4 +- modules/storage/README.md | 19 +- modules/storage/src/__tests__/cascade.test.ts | 65 +++++ .../storage/src/__tests__/file.authz.test.ts | 154 +++++++++++ modules/storage/src/__tests__/folders.test.ts | 261 ++++++++++++++++-- .../storage/src/__tests__/relations.test.ts | 179 +++++++++++- modules/storage/src/admin/adminFile.ts | 9 +- modules/storage/src/authz/folders.ts | 37 ++- modules/storage/src/authz/relations.ts | 94 ++++++- modules/storage/src/handlers/file.ts | 44 +-- 10 files changed, 800 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a8048f85..82996bc22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,11 @@ All notable changes to this project will be documented in this file. See [standa ### ⚠ BREAKING CHANGES -* **storage:** Client APIs no longer create missing containers (404). An omitted folder now resolves to a personal `cnd_/` folder. Client list-files is deferred. +* **storage:** Client APIs no longer create missing containers (404). An omitted folder now resolves to a personal `cnd_/` folder. Creating under a missing or unmanaged `cnd_/` path is denied. File create with `scope` now requires `edit` on that scope, not `read`. Client list-files is deferred. ### Features -* **storage:** complete filesystem-shaped ReBAC for Container, Folder, and File ([#1173](https://github.com/ConduitPlatform/Conduit/issues/1173)) +* **storage:** complete filesystem-shaped ReBAC for Container, Folder, and File behind `authorization.enabled` ([#1173](https://github.com/ConduitPlatform/Conduit/issues/1173)). Leftover unowned folders/containers are unmanaged until the first write heals them; old files are not backfilled. There is no reconstruct job and no second filesystem flag. Provision named containers via Admin. Products that share prefixes (for example fyllo) should keep authorization off until they have per-user folder roots and per-file grants or privileged fetch. ## [0.17.0-alpha.6](https://github.com/ConduitPlatform/Conduit/compare/v0.17.0-alpha.5...v0.17.0-alpha.6) (2026-07-26) diff --git a/modules/storage/README.md b/modules/storage/README.md index ce8cb469d..07aa13a68 100644 --- a/modules/storage/README.md +++ b/modules/storage/README.md @@ -8,17 +8,28 @@ These apply to Storage Client routes and gRPC calls that use the user file handl - **Missing containers are not created.** Creating or updating a file with a container that does not already exist returns `404 Not Found`. `allowContainerCreation` still only affects Admin implicit container creation. - **Omitted folder becomes a personal folder.** If `folder` is omitted on Client file create, Storage uses `cnd_/`. Passing `/` still stores at the container root. -- **Personal-folder squat is denied.** Creating a missing `cnd_/` path returns `403 Permission Denied`. A user may create their own `cnd_/`. If another user's personal root already exists, normal folder edit checks apply. +- **Personal-folder squat is denied.** Creating a missing `cnd_/` path returns `403 Permission Denied`. When `authorization.enabled` is true, an existing but **unmanaged** `cnd_/` root is also denied. A user may create their own `cnd_/`. If another user's personal root already exists **and is managed**, normal folder edit checks apply. +- **Scope create requires `edit`.** Creating a file with `scope` now requires `edit` on that scope, not `read`. -Admin-only container create remains available. Public file reads without auth, module schema ownership, and existing public URI / CDN / content-disposition / local URL upload behavior are unchanged. +Admin-only container create remains available. Public file reads without auth (`getFile` / `getFileUrl`), module schema ownership, and existing public URI / CDN / content-disposition / local URL upload behavior are unchanged. ## Authorization tree -When `authorization.enabled` is true, Storage registers `Container`, `Folder`, and `File` resources and maintains owner relations that follow the path: +When `authorization.enabled` is true, Storage registers `Container`, `Folder`, and `File` resources and maintains owner relations that follow the path. There is **no** second `authorization.filesystem.enabled` flag. - A container may own first-level folders, or files stored at `/`. - A folder owns nested folders and files. +- Client file creates also stamp `scope ?? User:` on the File so the creator can `can(File)` even if the folder has no owners yet. - The default container is never owned. It is created on both the database and the storage provider if missing. - An optional `scope` (for example `Team:`) is attached as an extra owner when provided. Admin folder create without scope only attaches the container as the first-folder owner. -Folder delete removes nested folders/files and all of their relations. Container delete pages those cleanups and also clears `Container` relations. +**Upgrade / leftover data:** there is no reconstruct-indexes job and old files are not backfilled. A leftover folder or non-default container with no owner/editor/reader relations is **unmanaged**: folder/container `can(edit)` is skipped, and the first successful write heals it by attaching the current subject (plus Container/parent links). After that, normal `can(edit)` applies. Old private files without a File relation stay Client-inaccessible; Admin can still read/update/delete them. + +Folder delete removes nested folders/files and all of their relations. Container delete pages those cleanups and also clears `Container` relations. File moves always try to drop the old structural Folder/Container owner (ignore missing) and add the new one. + +## Provisioning notes + +- Provision named containers via **Admin**. Client APIs will not create them. +- Do **not** enable `authorization.enabled` until the product has a folder ownership model **and** either per-file grants or a privileged fetch path. +- Shared prefixes (`docs/`, team drops, fyllo-style common roots) become first-writer-wins on the first Client write after enable, then exclusive to that subject unless relations are granted. +- Fyllo-like apps that share prefixes should keep `authorization.enabled: false` until they do that separate product work. This module does not migrate those apps. diff --git a/modules/storage/src/__tests__/cascade.test.ts b/modules/storage/src/__tests__/cascade.test.ts index 946e72cd6..69436319f 100644 --- a/modules/storage/src/__tests__/cascade.test.ts +++ b/modules/storage/src/__tests__/cascade.test.ts @@ -104,6 +104,71 @@ describe('deleteFolderTree', () => { container: 'conduit', }); }); + + it('succeeds when deleteAllRelations reports no relations found', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + }; + _StorageFolder.getInstance = (() => ({ + findMany: async () => [{ _id: 'dir1' }], + deleteMany: async () => undefined, + })) as unknown as typeof _StorageFolder.getInstance; + File.getInstance = (() => ({ + findMany: async () => [{ _id: 'file1' }], + deleteMany: async () => undefined, + })) as unknown as typeof File.getInstance; + + const grpcSdk = { + authorization: { + deleteAllRelations: async () => { + throw new Error('No relations found'); + }, + }, + } as unknown as ConduitGrpcSdk; + const storage = { + container: () => ({ + deleteFolder: async () => true, + }), + }; + + await deleteFolderTree( + grpcSdk, + storage as never, + { _id: 'dir1', name: 'docs/', container: 'conduit' } as never, + ); + }); +}); + +describe('authz disabled cascade', () => { + it('does not call deleteAllRelations when authorization is off', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: false }, + defaultContainer: 'conduit', + }; + let relationDeletes = 0; + _StorageFolder.getInstance = (() => ({ + findMany: async () => [{ _id: 'dir1' }], + deleteMany: async () => undefined, + })) as unknown as typeof _StorageFolder.getInstance; + File.getInstance = (() => ({ + findMany: async () => [{ _id: 'file1' }], + deleteMany: async () => undefined, + })) as unknown as typeof File.getInstance; + const grpcSdk = { + authorization: { + deleteAllRelations: async () => { + relationDeletes += 1; + }, + }, + } as unknown as ConduitGrpcSdk; + await deleteFolderTree( + grpcSdk, + { container: () => ({ deleteFolder: async () => true }) } as never, + { _id: 'dir1', name: 'docs/', container: 'conduit' } as never, + ); + assert.equal(relationDeletes, 0); + }); }); describe('deleteContainerTree', () => { diff --git a/modules/storage/src/__tests__/file.authz.test.ts b/modules/storage/src/__tests__/file.authz.test.ts index 22bd231ba..958206374 100644 --- a/modules/storage/src/__tests__/file.authz.test.ts +++ b/modules/storage/src/__tests__/file.authz.test.ts @@ -65,13 +65,27 @@ function stubModels(args: { function handlers(authz: { can?: (input: { actions: string[]; resource: string }) => Promise<{ allow: boolean }>; deleteAllRelations?: () => Promise; + findRelation?: (input: { resource: string }) => Promise<{ + relations: Array<{ relation: string }>; + count: number; + }>; + managed?: string[]; }) { + const managed = new Set(authz.managed ?? []); const grpcSdk = { databaseProvider: {}, authorization: { can: authz.can ?? (async () => ({ allow: true })), deleteAllRelations: authz.deleteAllRelations ?? (async () => undefined), createRelation: async () => undefined, + findRelation: + authz.findRelation ?? + (async ({ resource }: { resource: string }) => { + const relations = managed.has(resource) + ? [{ relation: 'owner', resource }] + : []; + return { relations, count: relations.length }; + }), }, } as unknown as ConduitGrpcSdk; const storage = { @@ -262,6 +276,146 @@ describe('scope and team', () => { }); }); +describe('container create access', () => { + it('skips can(edit) on an unmanaged named container', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + }; + stubModels({ containers: [{ _id: 'c2', name: 'photos' }] }); + let canCalls = 0; + const fileHandlers = handlers({ + can: async () => { + canCalls += 1; + return { allow: false }; + }, + }); + await fileHandlers.fileAccessCheck( + 'create', + request({ + params: {}, + context: { user: { _id: 'u1' } }, + }).request, + undefined, + 'photos', + ); + assert.equal(canCalls, 0); + }); + + it('fails closed when findRelation throws for a named container', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + }; + stubModels({ containers: [{ _id: 'c2', name: 'photos' }] }); + const fileHandlers = handlers({ + findRelation: async () => { + throw new Error('authorization down'); + }, + }); + await assert.rejects( + () => + fileHandlers.fileAccessCheck( + 'create', + request({ + params: {}, + context: { user: { _id: 'u1' } }, + }).request, + undefined, + 'photos', + ), + (error: unknown) => + error instanceof Error && error.message === 'authorization down', + ); + }); + + it('keeps can(edit) on a managed named container', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + }; + stubModels({ containers: [{ _id: 'c2', name: 'photos' }] }); + const fileHandlers = handlers({ + managed: ['Container:c2'], + can: async () => ({ allow: false }), + }); + await assert.rejects( + () => + fileHandlers.fileAccessCheck( + 'create', + request({ + params: {}, + context: { user: { _id: 'u1' } }, + }).request, + undefined, + 'photos', + ), + (error: unknown) => + error instanceof GrpcError && + error.code === status.PERMISSION_DENIED && + error.message === 'You are not allowed to create files in this container', + ); + }); +}); + +describe('old files are not backfilled', () => { + it('denies client edit/delete of a private file with no File relations', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + }; + stubModels({ + files: [ + { + _id: 'legacy', + isPublic: false, + name: 'old.txt', + container: 'conduit', + folder: 'docs/', + }, + ], + }); + const fileHandlers = handlers({ + can: async () => ({ allow: false }), + }); + await assert.rejects( + () => + fileHandlers.fileAccessCheck( + 'edit', + request({ context: { user: { _id: 'u1' } } }).request, + { _id: 'legacy' } as never, + ), + (error: unknown) => + error instanceof GrpcError && error.code === status.PERMISSION_DENIED, + ); + await assert.rejects( + () => + fileHandlers.fileAccessCheck( + 'delete', + request({ context: { user: { _id: 'u1' } } }).request, + { _id: 'legacy' } as never, + ), + (error: unknown) => + error instanceof GrpcError && error.code === status.PERMISSION_DENIED, + ); + }); + + it('allows in-place edit when the file already has a User owner', async () => { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + }; + const fileHandlers = handlers({ + can: async input => ({ allow: input.resource === 'File:owned' }), + }); + await fileHandlers.fileAccessCheck( + 'edit', + request({ context: { user: { _id: 'u1' } } }).request, + { _id: 'owned' } as never, + ); + }); +}); + describe('authz disabled', () => { it('does not call authorization.can during create access checks', async () => { ConfigController.getInstance().config = { diff --git a/modules/storage/src/__tests__/folders.test.ts b/modules/storage/src/__tests__/folders.test.ts index e85bcb192..cfc4e9025 100644 --- a/modules/storage/src/__tests__/folders.test.ts +++ b/modules/storage/src/__tests__/folders.test.ts @@ -4,7 +4,11 @@ import { status } from '@grpc/grpc-js'; import { ConduitGrpcSdk, GrpcError } from '@conduitplatform/grpc-sdk'; import { ConfigController } from '@conduitplatform/module-tools'; import { _StorageContainer, _StorageFolder } from '../models/index.js'; -import { assertNoPersonalFolderSquat, findOrCreateFolders } from '../authz/folders.js'; +import { + assertFolderEditAccess, + assertNoPersonalFolderSquat, + findOrCreateFolders, +} from '../authz/folders.js'; const originalConfig = ConfigController.getInstance().config; const originalContainerGetInstance = @@ -17,47 +21,172 @@ afterEach(() => { _StorageFolder.getInstance = originalFolderGetInstance; }); +function enableAuthz() { + ConfigController.getInstance().config = { + authorization: { enabled: true }, + defaultContainer: 'conduit', + }; +} + +function disableAuthz() { + ConfigController.getInstance().config = { + authorization: { enabled: false }, + defaultContainer: 'conduit', + }; +} + +function fakeSdk(args?: { + managed?: string[]; + findRelationError?: Error; + created?: Array<{ subject: string; relation: string; resource: string }>; +}) { + const created = args?.created ?? []; + const managed = new Set(args?.managed ?? []); + return { + grpcSdk: { + authorization: { + findRelation: async ({ resource }: { resource: string }) => { + if (args?.findRelationError) { + throw args.findRelationError; + } + const relations = managed.has(resource) + ? [{ subject: 'User:owner', relation: 'owner', resource }] + : []; + return { relations, count: relations.length }; + }, + createRelation: async (relation: { + subject: string; + relation: string; + resource: string; + }) => { + created.push(relation); + }, + can: async ({ subject, resource }: { subject: string; resource: string }) => ({ + allow: managed.has(resource) && subject === 'User:owner', + }), + }, + } as unknown as ConduitGrpcSdk, + created, + }; +} + describe('personal folder squat', () => { it('denies creating under another user personal root when it is missing', async () => { + disableAuthz(); _StorageFolder.getInstance = (() => ({ findOne: async () => null, })) as unknown as typeof _StorageFolder.getInstance; + const { grpcSdk } = fakeSdk(); await assert.rejects( - () => assertNoPersonalFolderSquat('cnd_other/', 'self', 'conduit'), + () => assertNoPersonalFolderSquat(grpcSdk, 'cnd_other/', 'self', 'conduit'), (error: unknown) => error instanceof GrpcError && error.code === status.PERMISSION_DENIED, ); await assert.rejects( - () => assertNoPersonalFolderSquat('cnd_other/sub/', 'self', 'conduit'), + () => assertNoPersonalFolderSquat(grpcSdk, 'cnd_other/sub/', 'self', 'conduit'), (error: unknown) => error instanceof GrpcError && error.code === status.PERMISSION_DENIED, ); }); it('allows a user to create their own missing personal folder', async () => { + disableAuthz(); _StorageFolder.getInstance = (() => ({ findOne: async () => null, })) as unknown as typeof _StorageFolder.getInstance; - await assertNoPersonalFolderSquat('cnd_self/', 'self', 'conduit'); + const { grpcSdk } = fakeSdk(); + await assertNoPersonalFolderSquat(grpcSdk, 'cnd_self/', 'self', 'conduit'); }); - it('allows a path under another personal root when that root already exists', async () => { + it('allows a path under another personal root when authz is off and that root exists', async () => { + disableAuthz(); _StorageFolder.getInstance = (() => ({ findOne: async (query: { name?: string }) => query.name === 'cnd_other/' ? { _id: 'dir1' } : null, })) as unknown as typeof _StorageFolder.getInstance; - await assertNoPersonalFolderSquat('cnd_other/sub/', 'self', 'conduit'); + const { grpcSdk } = fakeSdk(); + await assertNoPersonalFolderSquat(grpcSdk, 'cnd_other/sub/', 'self', 'conduit'); + }); + + it('denies an existing unmanaged personal root of another user when authz is on', async () => { + enableAuthz(); + _StorageFolder.getInstance = (() => ({ + findOne: async (query: { name?: string }) => + query.name === 'cnd_other/' ? { _id: 'dir1' } : null, + })) as unknown as typeof _StorageFolder.getInstance; + const { grpcSdk } = fakeSdk(); + await assert.rejects( + () => assertNoPersonalFolderSquat(grpcSdk, 'cnd_other/', 'self', 'conduit'), + (error: unknown) => + error instanceof GrpcError && error.code === status.PERMISSION_DENIED, + ); + }); + + it('allows a managed personal root of another user so folder can(edit) applies next', async () => { + enableAuthz(); + _StorageFolder.getInstance = (() => ({ + findOne: async (query: { name?: string }) => + query.name === 'cnd_other/' ? { _id: 'dir1' } : null, + })) as unknown as typeof _StorageFolder.getInstance; + const { grpcSdk } = fakeSdk({ managed: ['Folder:dir1'] }); + await assertNoPersonalFolderSquat(grpcSdk, 'cnd_other/sub/', 'self', 'conduit'); + }); +}); + +describe('assertFolderEditAccess', () => { + it('skips can(edit) when the whole path is leftover and unmanaged', async () => { + enableAuthz(); + _StorageFolder.getInstance = (() => ({ + findOne: async (query: { name?: string }) => + query.name === 'docs/' ? { _id: 'docs', name: 'docs/' } : null, + })) as unknown as typeof _StorageFolder.getInstance; + let canCalls = 0; + const { grpcSdk } = fakeSdk(); + grpcSdk.authorization!.can = async () => { + canCalls += 1; + return { allow: false }; + }; + await assertFolderEditAccess(grpcSdk, 'conduit', 'docs/', 'User:alice'); + assert.equal(canCalls, 0); + }); + + it('walks through an unmanaged child and can(edit) the managed parent', async () => { + enableAuthz(); + _StorageFolder.getInstance = (() => ({ + findOne: async (query: { name?: string }) => { + if (query.name === 'docs/secret/') return { _id: 'secret', name: 'docs/secret/' }; + if (query.name === 'docs/') return { _id: 'docs', name: 'docs/' }; + return null; + }, + })) as unknown as typeof _StorageFolder.getInstance; + const { grpcSdk } = fakeSdk({ managed: ['Folder:docs'] }); + await assertFolderEditAccess(grpcSdk, 'conduit', 'docs/secret/', 'User:owner'); + await assert.rejects( + () => assertFolderEditAccess(grpcSdk, 'conduit', 'docs/secret/', 'User:bob'), + (error: unknown) => + error instanceof GrpcError && error.code === status.PERMISSION_DENIED, + ); + }); + + it('fails closed when findRelation throws', async () => { + enableAuthz(); + _StorageFolder.getInstance = (() => ({ + findOne: async () => ({ _id: 'docs', name: 'docs/' }), + })) as unknown as typeof _StorageFolder.getInstance; + const { grpcSdk } = fakeSdk({ findRelationError: new Error('authorization down') }); + await assert.rejects( + () => assertFolderEditAccess(grpcSdk, 'conduit', 'docs/', 'User:alice'), + (error: unknown) => + error instanceof Error && error.message === 'authorization down', + ); }); }); describe('findOrCreateFolders', () => { it('does not create a scope owner when admin omits scope', async () => { - ConfigController.getInstance().config = { - authorization: { enabled: true }, - defaultContainer: 'conduit', - }; - const created: Array<{ subject: string; resource: string }> = []; + enableAuthz(); + const created: Array<{ subject: string; relation: string; resource: string }> = []; const folders: Array<{ _id: string; name: string; container: string }> = []; _StorageContainer.getInstance = (() => ({ findOne: async () => ({ _id: 'c1', name: 'conduit' }), @@ -72,13 +201,7 @@ describe('findOrCreateFolders', () => { }, })) as unknown as typeof _StorageFolder.getInstance; - const grpcSdk = { - authorization: { - createRelation: async (relation: { subject: string; resource: string }) => { - created.push(relation); - }, - }, - } as unknown as ConduitGrpcSdk; + const { grpcSdk } = fakeSdk({ created }); const storage = { container: () => ({ folderExists: async () => false, @@ -104,4 +227,106 @@ describe('findOrCreateFolders', () => { false, ); }); + + it('heals leftover unowned folders and a named container on write', async () => { + enableAuthz(); + const created: Array<{ subject: string; relation: string; resource: string }> = []; + const folders = [ + { _id: 'docs', name: 'docs/', container: 'photos' }, + { _id: 'nested', name: 'docs/nested/', container: 'photos' }, + ]; + _StorageContainer.getInstance = (() => ({ + findOne: async () => ({ _id: 'c2', name: 'photos' }), + })) as unknown as typeof _StorageContainer.getInstance; + _StorageFolder.getInstance = (() => ({ + findOne: async (query: { name?: string }) => + folders.find(folder => folder.name === query.name) ?? null, + })) as unknown as typeof _StorageFolder.getInstance; + + const { grpcSdk } = fakeSdk({ created }); + const storage = { + container: () => ({ + folderExists: async () => true, + createFolder: async () => true, + }), + }; + + const result = await findOrCreateFolders( + grpcSdk, + storage as never, + 'docs/nested/', + 'photos', + { scope: 'User:alice' }, + ); + assert.equal(result.length, 0); + assert.deepEqual(created, [ + { subject: 'User:alice', relation: 'owner', resource: 'Container:c2' }, + { subject: 'Container:c2', relation: 'owner', resource: 'Folder:docs' }, + { subject: 'User:alice', relation: 'owner', resource: 'Folder:docs' }, + { subject: 'Folder:docs', relation: 'owner', resource: 'Folder:nested' }, + { subject: 'User:alice', relation: 'owner', resource: 'Folder:nested' }, + ]); + }); + + it('does not rewrite relations on a folder that is already managed', async () => { + enableAuthz(); + const created: Array<{ subject: string; relation: string; resource: string }> = []; + _StorageContainer.getInstance = (() => ({ + findOne: async () => ({ _id: 'c1', name: 'conduit' }), + })) as unknown as typeof _StorageContainer.getInstance; + _StorageFolder.getInstance = (() => ({ + findOne: async () => ({ _id: 'docs', name: 'docs/', container: 'conduit' }), + })) as unknown as typeof _StorageFolder.getInstance; + + const { grpcSdk } = fakeSdk({ created, managed: ['Folder:docs'] }); + await findOrCreateFolders( + grpcSdk, + { + container: () => ({ + folderExists: async () => true, + createFolder: async () => true, + }), + } as never, + 'docs/', + 'conduit', + { scope: 'User:bob' }, + ); + assert.deepEqual(created, []); + }); + + it('heals a named container when writing at /', async () => { + enableAuthz(); + const created: Array<{ subject: string; relation: string; resource: string }> = []; + _StorageContainer.getInstance = (() => ({ + findOne: async () => ({ _id: 'c2', name: 'photos' }), + })) as unknown as typeof _StorageContainer.getInstance; + const { grpcSdk } = fakeSdk({ created }); + await findOrCreateFolders( + grpcSdk, + { container: () => ({}) } as never, + '/', + 'photos', + { scope: 'Team:t1' }, + ); + assert.deepEqual(created, [ + { subject: 'Team:t1', relation: 'owner', resource: 'Container:c2' }, + ]); + }); + + it('never owns the default container when writing at /', async () => { + enableAuthz(); + const created: Array<{ subject: string; relation: string; resource: string }> = []; + _StorageContainer.getInstance = (() => ({ + findOne: async () => ({ _id: 'c1', name: 'conduit' }), + })) as unknown as typeof _StorageContainer.getInstance; + const { grpcSdk } = fakeSdk({ created }); + await findOrCreateFolders( + grpcSdk, + { container: () => ({}) } as never, + '/', + 'conduit', + { scope: 'User:alice' }, + ); + assert.deepEqual(created, []); + }); }); diff --git a/modules/storage/src/__tests__/relations.test.ts b/modules/storage/src/__tests__/relations.test.ts index b27a41216..95deb6773 100644 --- a/modules/storage/src/__tests__/relations.test.ts +++ b/modules/storage/src/__tests__/relations.test.ts @@ -9,6 +9,9 @@ import { createFolderOwnerRelations, createOwnerRelation, forEachDocumentPage, + hasManagedRelations, + healUnmanagedContainer, + healUnmanagedFolder, updateFileRelations, } from '../authz/relations.js'; @@ -53,9 +56,10 @@ function stubLookups(args: { })) as unknown as typeof _StorageFolder.getInstance; } -function fakeSdk() { +function fakeSdk(args?: { managed?: string[]; findRelationError?: Error }) { const created: Array<{ subject: string; relation: string; resource: string }> = []; const deleted: Array<{ subject: string; relation: string; resource: string }> = []; + const managed = new Set(args?.managed ?? []); const grpcSdk = { authorization: { createRelation: async (relation: { @@ -72,6 +76,15 @@ function fakeSdk() { }) => { deleted.push(relation); }, + findRelation: async ({ resource }: { resource: string }) => { + if (args?.findRelationError) { + throw args.findRelationError; + } + const relations = managed.has(resource) + ? [{ subject: 'User:owner', relation: 'owner', resource }] + : []; + return { relations, count: relations.length }; + }, }, } as unknown as ConduitGrpcSdk; return { grpcSdk, created, deleted }; @@ -149,7 +162,7 @@ describe('file relation tree', () => { ]); }); - it('attaches only the folder owner for a nested file without scope', async () => { + it('also stamps the creating user on a nested file without scope', async () => { enableAuthz(); stubLookups({ folders: [{ _id: 'dir1', name: 'cnd_u1/', container: 'conduit' }], @@ -160,6 +173,40 @@ describe('file relation tree', () => { { _id: 'f1', container: 'conduit', folder: 'cnd_u1/' } as never, { userId: 'u1' }, ); + assert.deepEqual(created, [ + { subject: 'Folder:dir1', relation: 'owner', resource: 'File:f1' }, + { subject: 'User:u1', relation: 'owner', resource: 'File:f1' }, + ]); + }); + + it('stamps scope instead of User when both are provided', async () => { + enableAuthz(); + stubLookups({ + folders: [{ _id: 'dir1', name: 'docs/', container: 'conduit' }], + }); + const { grpcSdk, created } = fakeSdk(); + await createFileRelations( + grpcSdk, + { _id: 'f1', container: 'conduit', folder: 'docs/' } as never, + { scope: 'Team:t1', userId: 'u1' }, + ); + assert.deepEqual(created, [ + { subject: 'Folder:dir1', relation: 'owner', resource: 'File:f1' }, + { subject: 'Team:t1', relation: 'owner', resource: 'File:f1' }, + ]); + }); + + it('does not invent a user on admin creates without userId or scope', async () => { + enableAuthz(); + stubLookups({ + folders: [{ _id: 'dir1', name: 'docs/', container: 'conduit' }], + }); + const { grpcSdk, created } = fakeSdk(); + await createFileRelations(grpcSdk, { + _id: 'f1', + container: 'conduit', + folder: 'docs/', + } as never); assert.deepEqual(created, [ { subject: 'Folder:dir1', relation: 'owner', resource: 'File:f1' }, ]); @@ -252,6 +299,134 @@ describe('move owners', () => { { subject: 'Folder:dir-new', relation: 'owner', resource: 'File:f1' }, ]); }); + + it('adds the new structural owner when the old folder row is missing', async () => { + enableAuthz(); + stubLookups({ + folders: [{ _id: 'dir-new', name: 'docs/', container: 'conduit' }], + }); + const { grpcSdk, created, deleted } = fakeSdk(); + await updateFileRelations( + grpcSdk, + { _id: 'f1', container: 'conduit', folder: 'gone/' }, + { _id: 'f1', container: 'conduit', folder: 'docs/' }, + ); + assert.deepEqual(deleted, []); + assert.deepEqual(created, [ + { subject: 'Folder:dir-new', relation: 'owner', resource: 'File:f1' }, + ]); + }); + + it('ignores a missing old structural relation and still adds the new owner', async () => { + enableAuthz(); + stubLookups({ + folders: [ + { _id: 'dir-old', name: 'old/', container: 'conduit' }, + { _id: 'dir-new', name: 'docs/', container: 'conduit' }, + ], + }); + const { grpcSdk, created, deleted } = fakeSdk(); + grpcSdk.authorization!.deleteRelation = async relation => { + deleted.push(relation); + throw new Error('No relations found'); + }; + await updateFileRelations( + grpcSdk, + { _id: 'f1', container: 'conduit', folder: 'old/' }, + { _id: 'f1', container: 'conduit', folder: 'docs/' }, + ); + assert.equal(deleted.length, 1); + assert.deepEqual(created, [ + { subject: 'Folder:dir-new', relation: 'owner', resource: 'File:f1' }, + ]); + }); + + it('does not rewire relations when authz is disabled', async () => { + disableAuthz(); + stubLookups({ + folders: [ + { _id: 'dir-old', name: 'old/', container: 'conduit' }, + { _id: 'dir-new', name: 'docs/', container: 'conduit' }, + ], + }); + const { grpcSdk, created, deleted } = fakeSdk(); + await updateFileRelations( + grpcSdk, + { _id: 'f1', container: 'conduit', folder: 'old/' }, + { _id: 'f1', container: 'conduit', folder: 'docs/' }, + ); + assert.deepEqual(deleted, []); + assert.deepEqual(created, []); + }); + + it('adds a scope owner on update without removing an existing user owner', async () => { + enableAuthz(); + stubLookups({ + folders: [{ _id: 'dir1', name: 'docs/', container: 'conduit' }], + }); + const { grpcSdk, created, deleted } = fakeSdk(); + await updateFileRelations( + grpcSdk, + { _id: 'f1', container: 'conduit', folder: 'docs/' }, + { _id: 'f1', container: 'conduit', folder: 'docs/' }, + { scope: 'Team:t1' }, + ); + assert.deepEqual(deleted, []); + assert.deepEqual(created, [ + { subject: 'Team:t1', relation: 'owner', resource: 'File:f1' }, + ]); + }); +}); + +describe('managed relation probe', () => { + it('treats owner/editor/reader rows as managed and empty as unmanaged', async () => { + enableAuthz(); + const { grpcSdk } = fakeSdk({ managed: ['Folder:1'] }); + assert.equal(await hasManagedRelations(grpcSdk, 'Folder:1'), true); + assert.equal(await hasManagedRelations(grpcSdk, 'Folder:2'), false); + }); + + it('fails closed when findRelation throws', async () => { + enableAuthz(); + const { grpcSdk } = fakeSdk({ findRelationError: new Error('down') }); + await assert.rejects(() => hasManagedRelations(grpcSdk, 'Folder:1'), /down/); + }); + + it('does not heal a managed container or the default container', async () => { + enableAuthz(); + const { grpcSdk, created } = fakeSdk({ managed: ['Container:c2'] }); + await healUnmanagedContainer( + grpcSdk, + { _id: 'c1', name: 'conduit' } as never, + 'User:alice', + ); + await healUnmanagedContainer( + grpcSdk, + { _id: 'c2', name: 'photos' } as never, + 'User:alice', + ); + assert.deepEqual(created, []); + }); + + it('heals an unowned named container and first folder like a new first folder', async () => { + enableAuthz(); + const { grpcSdk, created } = fakeSdk(); + await healUnmanagedContainer( + grpcSdk, + { _id: 'c2', name: 'photos' } as never, + 'User:alice', + ); + await healUnmanagedFolder(grpcSdk, { _id: 'docs' } as never, { + isFirst: true, + containerId: 'c2', + scope: 'User:alice', + }); + assert.deepEqual(created, [ + { subject: 'User:alice', relation: 'owner', resource: 'Container:c2' }, + { subject: 'Container:c2', relation: 'owner', resource: 'Folder:docs' }, + { subject: 'User:alice', relation: 'owner', resource: 'Folder:docs' }, + ]); + }); }); describe('folder owners', () => { diff --git a/modules/storage/src/admin/adminFile.ts b/modules/storage/src/admin/adminFile.ts index 1ebe14f6b..ba305b755 100644 --- a/modules/storage/src/admin/adminFile.ts +++ b/modules/storage/src/admin/adminFile.ts @@ -197,7 +197,9 @@ export class AdminFileHandlers { async getFileUrl(call: ParsedRouterRequest): Promise { try { - const found = await File.getInstance().findOne({ _id: resolveFileId(call.request) }); + const found = await File.getInstance().findOne({ + _id: resolveFileId(call.request), + }); if (isNil(found)) { throw new GrpcError(status.NOT_FOUND, 'File does not exist'); } @@ -269,9 +271,6 @@ export class AdminFileHandlers { isPublic?: boolean, scope?: string, ): Promise { - if (folder === '/') { - return; - } await findOrCreateFolders(this.grpcSdk, this.storageProvider, folder, container, { isPublic, scope, @@ -313,7 +312,7 @@ export class AdminFileHandlers { await this.findOrCreateContainer(newContainer); } const newFolder = isNil(folder) ? file.folder : normalizeFolderPath(folder); - if (newFolder !== file.folder && newFolder !== '/') { + if (newFolder !== file.folder) { await this.ensureAdminFolder( newFolder, newContainer, diff --git a/modules/storage/src/authz/folders.ts b/modules/storage/src/authz/folders.ts index 6c9f41857..3485c20ce 100644 --- a/modules/storage/src/authz/folders.ts +++ b/modules/storage/src/authz/folders.ts @@ -4,7 +4,12 @@ import { isNil } from 'lodash-es'; import { IStorageProvider } from '../interfaces/index.js'; import { _StorageContainer, _StorageFolder } from '../models/index.js'; import { getNestedPaths } from '../utils/index.js'; -import { createFolderOwnerRelations } from './relations.js'; +import { + createFolderOwnerRelations, + hasManagedRelations, + healUnmanagedContainer, + healUnmanagedFolder, +} from './relations.js'; import { isAuthzEnabled, parsePersonalFolderOwner, @@ -12,6 +17,7 @@ import { } from './helpers.js'; export async function assertNoPersonalFolderSquat( + grpcSdk: ConduitGrpcSdk, folder: string, userId: string, container: string, @@ -30,6 +36,15 @@ export async function assertNoPersonalFolderSquat( 'You are not allowed to create this folder', ); } + if (!isAuthzEnabled()) { + return; + } + if (!(await hasManagedRelations(grpcSdk, `Folder:${existing._id}`))) { + throw new GrpcError( + status.PERMISSION_DENIED, + 'You are not allowed to create this folder', + ); + } } async function assertCanEditFolder( @@ -62,7 +77,7 @@ export async function assertFolderEditAccess( const folderDoc = await _StorageFolder .getInstance() .findOne({ name: folder, container }); - if (folderDoc) { + if (folderDoc && (await hasManagedRelations(grpcSdk, `Folder:${folderDoc._id}`))) { await assertCanEditFolder(grpcSdk, subject, folderDoc); return; } @@ -72,7 +87,10 @@ export async function assertFolderEditAccess( const parent = await _StorageFolder .getInstance() .findOne({ name: nestedPaths[i], container }); - if (parent) { + if (!parent) { + continue; + } + if (await hasManagedRelations(grpcSdk, `Folder:${parent._id}`)) { await assertCanEditFolder(grpcSdk, subject, parent); return; } @@ -94,6 +112,7 @@ export async function findOrCreateFolders( if (!containerDoc) { throw new GrpcError(status.NOT_FOUND, 'Container does not exist'); } + await healUnmanagedContainer(grpcSdk, containerDoc, options?.scope); const createdFolders: _StorageFolder[] = []; const nestedPaths = getNestedPaths(folderPath); @@ -123,8 +142,16 @@ export async function findOrCreateFolders( parentFolderId: previousFolder?._id, scope: options?.scope, }); - } else if (isLast) { - options?.lastExistsHandler?.(); + } else { + await healUnmanagedFolder(grpcSdk, folder, { + isFirst: i === 0, + containerId: containerDoc._id, + parentFolderId: previousFolder?._id, + scope: options?.scope, + }); + if (isLast) { + options?.lastExistsHandler?.(); + } } previousFolder = folder; } diff --git a/modules/storage/src/authz/relations.ts b/modules/storage/src/authz/relations.ts index 8c1531b25..798f5fd24 100644 --- a/modules/storage/src/authz/relations.ts +++ b/modules/storage/src/authz/relations.ts @@ -9,6 +9,8 @@ import { RELATION_PAGE_SIZE, } from './helpers.js'; +const MANAGED_RELATIONS = new Set(['owner', 'editor', 'reader']); + export async function createOwnerRelation( grpcSdk: ConduitGrpcSdk, subject: string | undefined, @@ -24,6 +26,50 @@ export async function createOwnerRelation( }); } +export async function hasManagedRelations( + grpcSdk: ConduitGrpcSdk, + resource: string, +): Promise { + if (!isAuthzEnabled()) { + return false; + } + const result = await grpcSdk.authorization!.findRelation({ + resource, + skip: 0, + limit: 10, + }); + const relations = result?.relations ?? []; + if (relations.some(relation => MANAGED_RELATIONS.has(relation.relation))) { + return true; + } + return (result?.count ?? 0) > 0 && relations.length === 0; +} + +export async function healUnmanagedOwner( + grpcSdk: ConduitGrpcSdk, + subject: string | undefined, + resource: string, +): Promise { + if (!isAuthzEnabled() || !isUsableSubject(subject)) { + return; + } + if (await hasManagedRelations(grpcSdk, resource)) { + return; + } + await createOwnerRelation(grpcSdk, subject, resource); +} + +export async function healUnmanagedContainer( + grpcSdk: ConduitGrpcSdk, + container: Pick<_StorageContainer, '_id' | 'name'>, + subject?: string, +): Promise { + if (isDefaultContainer(container.name)) { + return; + } + await healUnmanagedOwner(grpcSdk, subject, `Container:${container._id}`); +} + export async function deleteOwnerRelation( grpcSdk: ConduitGrpcSdk, subject: string | undefined, @@ -82,13 +128,12 @@ export async function createFileRelations( } const structuralOwner = await resolveStructuralOwner(file); await createOwnerRelation(grpcSdk, structuralOwner, `File:${file._id}`); - if (isUsableSubject(options?.scope)) { - await createOwnerRelation(grpcSdk, options.scope, `File:${file._id}`); - return; - } - if (file.folder === '/' && options?.userId) { - await createOwnerRelation(grpcSdk, `User:${options.userId}`, `File:${file._id}`); - } + const actor = isUsableSubject(options?.scope) + ? options.scope + : options?.userId + ? `User:${options.userId}` + : undefined; + await createOwnerRelation(grpcSdk, actor, `File:${file._id}`); } export async function updateFileRelations( @@ -103,10 +148,19 @@ export async function updateFileRelations( const samePlace = previous.container === updated.container && previous.folder === updated.folder; if (!samePlace) { - const oldOwner = await resolveStructuralOwner(previous); + let oldOwner: string | undefined; + try { + oldOwner = await resolveStructuralOwner(previous); + } catch (error) { + if (!(error instanceof GrpcError && error.code === status.NOT_FOUND)) { + throw error; + } + } const newOwner = await resolveStructuralOwner(updated); - if (oldOwner !== newOwner) { + if (oldOwner && oldOwner !== newOwner) { await deleteOwnerRelation(grpcSdk, oldOwner, `File:${updated._id}`); + } + if (!oldOwner || oldOwner !== newOwner) { await createOwnerRelation(grpcSdk, newOwner, `File:${updated._id}`); } } @@ -140,6 +194,28 @@ export async function createFolderOwnerRelations( await createOwnerRelation(grpcSdk, `Folder:${options.parentFolderId}`, resource); } +export async function healUnmanagedFolder( + grpcSdk: ConduitGrpcSdk, + folder: _StorageFolder, + options: { + isFirst: boolean; + containerId: string; + parentFolderId?: string; + scope?: string; + }, +): Promise { + if (!isAuthzEnabled()) { + return; + } + if (await hasManagedRelations(grpcSdk, `Folder:${folder._id}`)) { + return; + } + await createFolderOwnerRelations(grpcSdk, folder, options); + if (!options.isFirst && isUsableSubject(options.scope)) { + await createOwnerRelation(grpcSdk, options.scope, `Folder:${folder._id}`); + } +} + export async function createContainerOwnerRelation( grpcSdk: ConduitGrpcSdk, container: _StorageContainer, diff --git a/modules/storage/src/handlers/file.ts b/modules/storage/src/handlers/file.ts index 219ccb2a1..2a18f4c93 100644 --- a/modules/storage/src/handlers/file.ts +++ b/modules/storage/src/handlers/file.ts @@ -39,6 +39,7 @@ import { import { createFileRelations, deleteAllRelationsSafe, + hasManagedRelations, updateFileRelations, } from '../authz/relations.js'; @@ -101,16 +102,18 @@ export class FileHandlers { if (!containerDoc) { throw new GrpcError(status.NOT_FOUND, 'Container does not exist'); } - const allowed = await this.grpcSdk.authorization?.can({ - subject: scope ?? `User:${userId}`, - actions: ['edit'], - resource: `Container:${containerDoc._id}`, - }); - if (!allowed || !allowed.allow) { - throw new GrpcError( - status.PERMISSION_DENIED, - 'You are not allowed to create files in this container', - ); + if (await hasManagedRelations(this.grpcSdk, `Container:${containerDoc._id}`)) { + const allowed = await this.grpcSdk.authorization?.can({ + subject: scope ?? `User:${userId}`, + actions: ['edit'], + resource: `Container:${containerDoc._id}`, + }); + if (!allowed || !allowed.allow) { + throw new GrpcError( + status.PERMISSION_DENIED, + 'You are not allowed to create files in this container', + ); + } } } return; @@ -369,17 +372,16 @@ export class FileHandlers { folder: string, isPublic?: boolean, ) { - if (folder === '/') { - return; - } - const userId = resolveUserId(call.request); - if (!userId) { - throw new GrpcError(status.PERMISSION_DENIED, 'File access is not public'); - } - await assertNoPersonalFolderSquat(folder, userId, container); const subject = actorSubject(call.request); - if (subject) { - await assertFolderEditAccess(this.grpcSdk, container, folder, subject); + if (folder !== '/') { + const userId = resolveUserId(call.request); + if (!userId) { + throw new GrpcError(status.PERMISSION_DENIED, 'File access is not public'); + } + await assertNoPersonalFolderSquat(this.grpcSdk, folder, userId, container); + if (subject) { + await assertFolderEditAccess(this.grpcSdk, container, folder, subject); + } } await findOrCreateFolders(this.grpcSdk, this.storageProvider, folder, container, { isPublic, @@ -396,7 +398,7 @@ export class FileHandlers { await this.fileAccessCheck('create', call.request, undefined, newContainer); } const newFolder = isNil(folder) ? file.folder : normalizeFolderPath(folder); - if (newFolder !== file.folder && newFolder !== '/') { + if (newFolder !== file.folder) { await this.prepareClientFolder(call, newContainer, newFolder, file.isPublic); } const isDataUpdate = From 1eec07a64b63bb28287f60fbfc05e8c8265ac0d0 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 15 Sep 2026 11:47:15 +0000 Subject: [PATCH 5/5] docs(storage): note Admin scope is optional but needed for Client writes Admin without scope still owns a folder via the container only. On the default container that locks Client users out; pass a scope when the folder should stay Client-writable. --- CHANGELOG.md | 2 +- modules/storage/README.md | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82996bc22..ec962e115 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ All notable changes to this project will be documented in this file. See [standa ### Features -* **storage:** complete filesystem-shaped ReBAC for Container, Folder, and File behind `authorization.enabled` ([#1173](https://github.com/ConduitPlatform/Conduit/issues/1173)). Leftover unowned folders/containers are unmanaged until the first write heals them; old files are not backfilled. There is no reconstruct job and no second filesystem flag. Provision named containers via Admin. Products that share prefixes (for example fyllo) should keep authorization off until they have per-user folder roots and per-file grants or privileged fetch. +* **storage:** complete filesystem-shaped ReBAC for Container, Folder, and File behind `authorization.enabled` ([#1173](https://github.com/ConduitPlatform/Conduit/issues/1173)). Leftover unowned folders/containers are unmanaged until the first write heals them; old files are not backfilled. There is no reconstruct job and no second filesystem flag. Provision named containers via Admin. Admin writes without `scope` make a folder container-owned (Client 403 on the default container); pass a scope if Client users should keep writing. Products that share prefixes (for example fyllo) should keep authorization off until they have per-user folder roots and per-file grants or privileged fetch. ## [0.17.0-alpha.6](https://github.com/ConduitPlatform/Conduit/compare/v0.17.0-alpha.5...v0.17.0-alpha.6) (2026-07-26) diff --git a/modules/storage/README.md b/modules/storage/README.md index 07aa13a68..57ee9630d 100644 --- a/modules/storage/README.md +++ b/modules/storage/README.md @@ -21,15 +21,18 @@ When `authorization.enabled` is true, Storage registers `Container`, `Folder`, a - A folder owns nested folders and files. - Client file creates also stamp `scope ?? User:` on the File so the creator can `can(File)` even if the folder has no owners yet. - The default container is never owned. It is created on both the database and the storage provider if missing. -- An optional `scope` (for example `Team:`) is attached as an extra owner when provided. Admin folder create without scope only attaches the container as the first-folder owner. +- An optional `scope` (for example `Team:`) is attached as an extra owner when provided. Admin folder create without scope only attaches the container as the first-folder owner. Scope is optional and is not rejected when missing. **Upgrade / leftover data:** there is no reconstruct-indexes job and old files are not backfilled. A leftover folder or non-default container with no owner/editor/reader relations is **unmanaged**: folder/container `can(edit)` is skipped, and the first successful write heals it by attaching the current subject (plus Container/parent links). After that, normal `can(edit)` applies. Old private files without a File relation stay Client-inaccessible; Admin can still read/update/delete them. +If Admin writes into a leftover (or new) folder **without** `scope`, the folder becomes container-owned. On the default container that means Client users will get `403` on later writes. That is expected. To keep the folder Client-writable, Admin must pass a `scope`, or let a Client user write first so they become the owner. + Folder delete removes nested folders/files and all of their relations. Container delete pages those cleanups and also clears `Container` relations. File moves always try to drop the old structural Folder/Container owner (ignore missing) and add the new one. ## Provisioning notes - Provision named containers via **Admin**. Client APIs will not create them. +- For shared leftover folders that Client users should keep writing to, Admin should pass a `scope` (for example `Team:`). Omitting scope is valid for Admin-only trees; it is not an error. - Do **not** enable `authorization.enabled` until the product has a folder ownership model **and** either per-file grants or a privileged fetch path. - Shared prefixes (`docs/`, team drops, fyllo-style common roots) become first-writer-wins on the first Client write after enable, then exclusive to that subject unless relations are granted. - Fyllo-like apps that share prefixes should keep `authorization.enabled: false` until they do that separate product work. This module does not migrate those apps.