-
Notifications
You must be signed in to change notification settings - Fork 405
feat: add RBAC contracts, persistence, and bootstrap #1677
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { rolePermissionPredicate } from "./permission-sql"; | ||
|
|
||
| describe("rolePermissionPredicate", () => { | ||
| it("never grants ownership transfer through a custom role", () => { | ||
| const predicate = rolePermissionPredicate("workspace.transfer_ownership"); | ||
|
|
||
| expect(predicate.sql).not.toContain("role_permissions"); | ||
| expect(predicate.values).toEqual(["owner"]); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { | ||
| BUILT_IN_ROLE_KEYS, | ||
| isCustomRolePermission, | ||
| permissionsForBuiltInRole, | ||
| type PermissionId, | ||
| } from "@open-inspect/shared/rbac"; | ||
|
|
||
| /** Builds a parameterized role predicate that enforces built-in and custom-role grant rules. */ | ||
| export function rolePermissionPredicate(permission: PermissionId): { | ||
| sql: string; | ||
| values: string[]; | ||
| } { | ||
| const builtInRoles = BUILT_IN_ROLE_KEYS.filter((role) => | ||
| permissionsForBuiltInRole(role).includes(permission) | ||
| ); | ||
| const customRolePermission = isCustomRolePermission(permission); | ||
| const customRoleSql = customRolePermission | ||
| ? `r.key IS NULL AND EXISTS ( | ||
| SELECT 1 FROM role_permissions custom_permission | ||
| WHERE custom_permission.role_id = r.id | ||
| AND custom_permission.permission_id = ? | ||
| )` | ||
| : "0"; | ||
| return { | ||
| sql: `(r.key IN (${builtInRoles.map(() => "?").join(", ")}) | ||
| OR (${customRoleSql}))`, | ||
| values: [...builtInRoles, ...(customRolePermission ? [permission] : [])], | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| import { | ||
| isRegisteredPermission, | ||
| isCustomRolePermission, | ||
| permissionsForBuiltInRole, | ||
| type BuiltInRoleKey, | ||
| type EffectiveAuthorization, | ||
| type PermissionId, | ||
| type RoleSummary, | ||
| type WorkspaceMember, | ||
| } from "@open-inspect/shared/rbac"; | ||
| import { | ||
| AuthorizationStore, | ||
| type AuthorizationMutationOutcome, | ||
| type AuthorizationRoleRecord, | ||
| } from "../db/authorization-store"; | ||
| import type { SqlDatabase } from "../db/sql-database"; | ||
|
|
||
| /** Represents an authorization denial that can be translated directly to an API response. */ | ||
| export class AuthorizationError extends Error { | ||
| /** Creates a denial with its HTTP status, stable error code, and optional missing grant. */ | ||
| constructor( | ||
| readonly status: number, | ||
| readonly code: string, | ||
| readonly permission?: PermissionId | ||
| ) { | ||
| super(code); | ||
| this.name = "AuthorizationError"; | ||
| } | ||
| } | ||
|
|
||
| /** Signals that RBAC state changed or violated an invariant during a guarded mutation. */ | ||
| export class RbacConflictError extends Error { | ||
| /** Creates a conflict suitable for retry or refreshed administrative state. */ | ||
| constructor(message: string) { | ||
| super(message); | ||
| this.name = "RbacConflictError"; | ||
| } | ||
| } | ||
|
|
||
| /** Resolves effective grants and coordinates invariant-preserving workspace RBAC mutations. */ | ||
| export class AuthorizationService { | ||
| private readonly store: AuthorizationStore; | ||
|
|
||
| /** Creates a service backed by the workspace authorization database. */ | ||
| constructor(db: SqlDatabase) { | ||
| this.store = new AuthorizationStore(db); | ||
| } | ||
|
|
||
| /** Resolves a user's assigned role and grants, withholding all grants while suspended. */ | ||
| async getEffectiveAuthorization(userId: string): Promise<EffectiveAuthorization> { | ||
| const record = await this.store.getEffectiveAuthorization(userId); | ||
| if (!record?.role) throw new AuthorizationError(403, "assignment_required"); | ||
|
|
||
| const permissions = | ||
| record.suspendedAt === null | ||
| ? await this.loadRolePermissions(record.role.id, record.role.key) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This authorization result is assembled from two independent reads. If a custom-role user is reassigned between them, the second query still loads grants for the old role ID, so
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are retaining this behavior intentionally. Authorization is admitted at request start, and an in-flight request may keep the permissions it had at admission even if its role or suspension changes during the request. The first read captures the active assignment and the second resolves that captured role; custom-role grants are not mutable through the current request surface. Administrative mutations continue to revalidate inside their atomic write. |
||
| : []; | ||
|
|
||
| return { | ||
| userId: record.userId, | ||
| suspendedAt: record.suspendedAt, | ||
| role: record.role, | ||
| permissions, | ||
| }; | ||
| } | ||
|
|
||
| /** Returns active authorization when the grant is present, or throws a structured denial. */ | ||
| async requirePermission( | ||
| userId: string, | ||
| permission: PermissionId | ||
| ): Promise<EffectiveAuthorization> { | ||
| const authorization = await this.getEffectiveAuthorization(userId); | ||
| if (authorization.suspendedAt !== null) { | ||
| throw new AuthorizationError(403, "active_user_required"); | ||
| } | ||
| if (!authorization.permissions.includes(permission)) { | ||
| throw new AuthorizationError(403, "permission_required", permission); | ||
| } | ||
| return authorization; | ||
| } | ||
|
|
||
| /** Lists roles with their effective permissions and current assignment counts. */ | ||
| async listRoles(): Promise<RoleSummary[]> { | ||
| const roles = await this.store.listRoles(); | ||
| return Promise.all(roles.map((role) => this.toRoleSummary(role))); | ||
| } | ||
|
|
||
| /** Returns a role's effective authorization summary, or null when it does not exist. */ | ||
| async getRole(roleId: string): Promise<RoleSummary | null> { | ||
| const role = await this.store.getRole(roleId); | ||
| return role ? this.toRoleSummary(role) : null; | ||
| } | ||
|
|
||
| /** Lists assigned workspace members with suspension and role state. */ | ||
| async listMembers(): Promise<WorkspaceMember[]> { | ||
| return this.store.listMembers(); | ||
| } | ||
|
|
||
| /** Replaces a member's role under actor revalidation and ownership invariants. */ | ||
| async replaceMemberRole(input: { | ||
| targetUserId: string; | ||
| roleId: string; | ||
| actorUserId: string; | ||
| requestId: string; | ||
| }): Promise<void> { | ||
| this.requireApplied( | ||
| await this.store.replaceMemberRole({ | ||
| targetUserId: input.targetUserId, | ||
| roleId: input.roleId, | ||
| actorUserId: input.actorUserId, | ||
| requestId: input.requestId, | ||
| now: Date.now(), | ||
| }), | ||
| "Member role precondition conflict" | ||
| ); | ||
| } | ||
|
|
||
| /** Suspends or reactivates a member while preserving an active workspace owner. */ | ||
| async replaceMemberStatus(input: { | ||
| targetUserId: string; | ||
| suspended: boolean; | ||
| actorUserId: string; | ||
| requestId: string; | ||
| }): Promise<void> { | ||
| this.requireApplied( | ||
| await this.store.replaceMemberStatus({ | ||
| targetUserId: input.targetUserId, | ||
| suspended: input.suspended, | ||
| actorUserId: input.actorUserId, | ||
| requestId: input.requestId, | ||
| now: Date.now(), | ||
| }), | ||
| "Member status precondition conflict" | ||
| ); | ||
| } | ||
|
|
||
| private async loadRolePermissions( | ||
| roleId: string, | ||
| roleKey: BuiltInRoleKey | null | ||
| ): Promise<PermissionId[]> { | ||
| if (roleKey) return permissionsForBuiltInRole(roleKey); | ||
| return (await this.store.getCustomRolePermissions(roleId)).filter( | ||
| (permission): permission is PermissionId => | ||
| isRegisteredPermission(permission) && isCustomRolePermission(permission) | ||
| ); | ||
| } | ||
|
|
||
| private async toRoleSummary(role: AuthorizationRoleRecord): Promise<RoleSummary> { | ||
| return { | ||
| ...role, | ||
| permissions: await this.loadRolePermissions(role.id, role.key), | ||
| }; | ||
| } | ||
|
|
||
| private requireApplied(outcome: AuthorizationMutationOutcome, conflictMessage: string): void { | ||
| if (outcome.status === "actor_authorization_changed") { | ||
| throw new RbacConflictError("Actor authorization changed"); | ||
| } | ||
| if (outcome.status === "role_not_found") { | ||
| throw new AuthorizationError(404, "role_not_found"); | ||
| } | ||
| if (outcome.status === "member_not_found") { | ||
| throw new AuthorizationError(404, "member_not_found"); | ||
| } | ||
| if (outcome.status === "conflict") { | ||
| throw new RbacConflictError(conflictMessage); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { AuthorizationStore } from "./authorization-store"; | ||
| import type { SqlDatabase, SqlResult, SqlStatement } from "./sql-database"; | ||
|
|
||
| function result(changes: number, rows: unknown[] = []): SqlResult { | ||
| return { results: rows, meta: { changes } }; | ||
| } | ||
|
|
||
| function fakeDatabase(options: { | ||
| batchResults?: SqlResult[]; | ||
| batchError?: Error; | ||
| allResults?: unknown[]; | ||
| }): SqlDatabase { | ||
| const statement: SqlStatement = { | ||
| bind: () => statement, | ||
| first: async <T>() => null as T | null, | ||
| run: async <T>() => result(0) as SqlResult<T>, | ||
| all: async <T>() => result(0, options.allResults) as SqlResult<T>, | ||
| }; | ||
| return { | ||
| prepare: () => statement, | ||
| batch: async <T>() => { | ||
| if (options.batchError) throw options.batchError; | ||
| return (options.batchResults ?? []) as SqlResult<T>[]; | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| const replaceMemberStatusInput: Parameters<AuthorizationStore["replaceMemberStatus"]>[0] = { | ||
| targetUserId: "target", | ||
| suspended: true, | ||
| actorUserId: "actor", | ||
| requestId: "request", | ||
| now: 100, | ||
| }; | ||
|
|
||
| describe("AuthorizationStore", () => { | ||
| it("maps persistence role fields at the store boundary", async () => { | ||
| const store = new AuthorizationStore( | ||
| fakeDatabase({ | ||
| allResults: [ | ||
| { | ||
| id: "role_custom", | ||
| key: null, | ||
| name: "Custom", | ||
| description: null, | ||
| is_system: 0, | ||
| assignment_count: "4", | ||
| }, | ||
| ], | ||
| }) | ||
| ); | ||
|
|
||
| await expect(store.listRoles()).resolves.toEqual([ | ||
| { | ||
| id: "role_custom", | ||
| key: null, | ||
| name: "Custom", | ||
| description: null, | ||
| assignmentCount: 4, | ||
| }, | ||
| ]); | ||
| }); | ||
|
|
||
| it.each([ | ||
| "applied", | ||
| "actor_authorization_changed", | ||
| "role_not_found", | ||
| "member_not_found", | ||
| "conflict", | ||
| ] as const)("returns the %s member status replacement batch outcome", async (status) => { | ||
| const store = new AuthorizationStore( | ||
| fakeDatabase({ | ||
| batchResults: [result(0, [{ status }]), result(1), result(1), result(1)], | ||
| }) | ||
| ); | ||
|
|
||
| await expect(store.replaceMemberStatus(replaceMemberStatusInput)).resolves.toEqual({ | ||
| status, | ||
| }); | ||
| }); | ||
|
|
||
| it("does not classify an unexpected database failure as a conflict", async () => { | ||
| const failure = new Error("database unavailable"); | ||
| const store = new AuthorizationStore(fakeDatabase({ batchError: failure })); | ||
|
|
||
| await expect(store.replaceMemberStatus(replaceMemberStatusInput)).rejects.toBe(failure); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[deep review] A custom-role authorization is assembled from two database snapshots: assignment/suspension is read here, then grants are read in
loadRolePermissions. A suspension or role replacement between those awaits can return permissions for stale state, including grants for a user who is now suspended. Resolve the user, role, suspension, and custom grants in one store query/atomic snapshot so an authorization decision is one coherent fact rather than orchestration across mutable reads.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are retaining the request-admission semantics intentionally: a request that was authorized at its start remains authorized if role or suspension state changes while it is in flight. The first read captures the active assignment and role used for that admission decision; current administrative writes still revalidate authority atomically.