diff --git a/.changeset/audit-unauthorized-access-attempts.md b/.changeset/audit-unauthorized-access-attempts.md new file mode 100644 index 00000000..230b0223 --- /dev/null +++ b/.changeset/audit-unauthorized-access-attempts.md @@ -0,0 +1,9 @@ +--- +'@openora/core': patch +--- + +The iam service's own super-admin / grant-escalation checks (`NotSuperAdminError`, `GrantEscalationError`) threw before ever reaching `AdminGuard.assert()`, so they produced no `identity.user.unauthorized_access` event even though the request genuinely hit the backend and was genuinely denied. + +- `IamService.assertSuperAdmin` and the no-escalation check in `setRolePermissions` now emit `identity.user.unauthorized_access` before throwing, so these service-level denials are audited the same way `AdminGuard` denials always have been. + +Frontend route guards are deliberately out of scope: a client-side redirect never sends the underlying request, so it is not a reliable audit signal - only a real backend request that hits `AdminGuard.assert()` (or one of the two service-level checks above) is. diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 501a4a71..e76db6d6 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,19 +1,15 @@ ## Summary - + ## Why - + -## Worth knowing +## Alternatives considered - + -- [ ] `pnpm verify` is green (typecheck + lint + boundaries + module-shape + tests) -- [ ] `pnpm check:drift` is green (catalog / OpenAPI not stale) - run `pnpm regen` if not -- [ ] New cross-module talk goes through events / command ports / contracts / the `/schema` subpath (no direct module imports) -- [ ] New data tables carry `tenantId` and are RLS-covered (`pnpm regen` runs `gen:rls`) -- [ ] No secrets, real player data, or internal/customer names added +## Risks -Closes ANPI-XXX / BF-XXX + diff --git a/CODEOWNERS b/CODEOWNERS index e0d7879d..d80597fe 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1,12 +1 @@ -# Code owners for oss (the platform core). -# GitHub uses this to request reviewers on PRs into protected branches. -# -# Ownership by area, not by person - team can change: -# - Backend core (contracts, domains, server, tooling) -> platform maintainer + backend dev -# - Frontend SDK (react consumption layer) -> frontend owner, reviewed with the maintainer -# Consumers read this repo as upstream; they don't edit it. - -* @zaxovaiko @agniev-a-hub @klaudia-blazyczek-blurify - -# Frontend SDK (react consumption layer) -/packages/core/src/react/ @klaudia-blazyczek-blurify @zaxovaiko +* @zaxovaiko @agniev-a-hub @klaudia-blazyczek-blurify @marek-chmielowski-blurify diff --git a/docs/catalog.json b/docs/catalog.json index 7e6401b2..38a5b1b0 100644 --- a/docs/catalog.json +++ b/docs/catalog.json @@ -741,6 +741,7 @@ "identity.user.phone_login", "identity.user.reactivated", "identity.user.registered", + "identity.user.unauthorized_access", "identity.user.unlocked", "notifications.created", "player.level.changed", diff --git a/packages/core/src/iam/AGENTS.md b/packages/core/src/iam/AGENTS.md index f1e1015b..b0b09aec 100644 --- a/packages/core/src/iam/AGENTS.md +++ b/packages/core/src/iam/AGENTS.md @@ -25,6 +25,7 @@ Three totally ordered levels: `no_access` < `read` < `read_write`. Storage is sp - Override invitation email: `ctx.provide(SEND_EMAIL, ...)` in a later-loading overlay. - React to onboarding: `ctx.events.on('iam.invitation.accepted', ...)`. +- Frontend route guards are NOT audited - a client-side redirect never sends the underlying request, so it is not a reliable signal that the user attempted to access protected data (only a real backend request that hits `AdminGuard.assert()` is). `assertSuperAdmin` and the no-escalation check in `setRolePermissions` emit the same `identity.user.unauthorized_access` event before throwing `NotSuperAdminError`/`GrantEscalationError`, so service-level denials that never reach `AdminGuard.assert()` are audited too. - Add permission modules: edit `statement` in `packages/core/src/server/auth/permissions.ts` - catalog, levels, and validation all derive from it. - Each `iam.role.*` payload carries an explicit `actorId` (the envelope does not); the audit module subscribes to all of them. - Binds `ADMIN_PLAYER_ACTIVITY` (`adapters/admin-player-activity.ts`) - the back-office player-activity report (registrations over time, DAU/WAU/MAU trend, 7d/30d retention cohorts). It reads the `user`/`session` tables via identity's read-only `/schema` subpath rather than identity binding the port itself - iam already `dependsOn: ['identity']` and centralizes the other admin-reporting-style ports, so this keeps that a single seam for admin-console. "Active" is defined as a session row whose `updatedAt` falls in the window (better-auth refreshes it on continued use) - see the one-line comment on `getActiveUsersTrend` before changing it, this is a deliberate simplification, not a hard requirement. diff --git a/packages/core/src/iam/__tests__/iam.service.int.test.ts b/packages/core/src/iam/__tests__/iam.service.int.test.ts index e9e8ead9..299fabca 100644 --- a/packages/core/src/iam/__tests__/iam.service.int.test.ts +++ b/packages/core/src/iam/__tests__/iam.service.int.test.ts @@ -197,9 +197,10 @@ describe('DbAdminPermissionResolver caching (real Redis read-through)', () => { }); describe('IamService.setRolePermissions', () => { - it('rejects a non-super-admin caller', async () => { + it('rejects a non-super-admin caller and audits the denial', async () => { const role = await seedRole(); - const svc = new IamService(db.drizzle, makeEventBus(), makeEmail()); + const events = makeEventBus(); + const svc = new IamService(db.drizzle, events, makeEmail()); await expect( svc.setRolePermissions({ roleId: role.id, @@ -207,6 +208,15 @@ describe('IamService.setRolePermissions', () => { caller: SUPPORT_CALLER, }), ).rejects.toBeInstanceOf(NotSuperAdminError); + expect(events.emit).toHaveBeenCalledWith( + 'identity.user.unauthorized_access', + expect.objectContaining({ + userId: SUPPORT_CALLER.userId, + resource: 'admin', + action: 'update', + role: 'support', + }), + ); }); it('throws RoleNotFoundError when the role does not exist', async () => { diff --git a/packages/core/src/iam/service/iam.service.ts b/packages/core/src/iam/service/iam.service.ts index 3d348c89..a139408f 100644 --- a/packages/core/src/iam/service/iam.service.ts +++ b/packages/core/src/iam/service/iam.service.ts @@ -293,10 +293,22 @@ export class IamService { private async assertSuperAdmin(caller: Caller) { if (!(await this.isSuperAdmin(caller))) { + this.emitDenied(caller, 'admin', 'update'); throw new NotSuperAdminError(); } } + private emitDenied(caller: Caller, resource: string, action: string) { + this.events.emit('identity.user.unauthorized_access', { + userId: caller.userId, + resource, + action, + ip: caller.ip, + userAgent: caller.userAgent, + role: caller.role, + }); + } + listCatalog() { return buildCatalog(); } @@ -488,6 +500,7 @@ export class IamService { } const have = callerMap[g.resource] ?? 'no_access'; if (!isLevelSufficient(have, g.level)) { + this.emitDenied(input.caller, g.resource, 'update'); throw new GrantEscalationError(); } } diff --git a/packages/core/src/server/auth/__tests__/admin-guard.int.test.ts b/packages/core/src/server/auth/__tests__/admin-guard.int.test.ts index 7dd7d7b4..e15bc678 100644 --- a/packages/core/src/server/auth/__tests__/admin-guard.int.test.ts +++ b/packages/core/src/server/auth/__tests__/admin-guard.int.test.ts @@ -16,7 +16,10 @@ const ADMIN_HEADERS = { 'x-real-ip': '127.0.0.1', 'user-agent': 'Mozilla/5.0' }; function makeGuard({ userId, grants, -}: { userId?: string; grants?: { resource: string; action: string }[] } = {}) { +}: { + userId?: string; + grants?: { resource: string; action: string }[]; +} = {}) { const events = makeEventBus(); const sessions = mock({ resolveUserId: vi.fn(async () => userId) }); const permissionResolver = grants diff --git a/packages/core/src/server/auth/admin-guard.ts b/packages/core/src/server/auth/admin-guard.ts index 2501130f..ba33df8a 100644 --- a/packages/core/src/server/auth/admin-guard.ts +++ b/packages/core/src/server/auth/admin-guard.ts @@ -4,6 +4,7 @@ import { AuthGuardReasonSchema, type Token, type AdminPermissionResolver, + type AdminGrant, type ClientMeta, } from '@openora/core/contracts'; import { DrizzleService } from '../db/index.js'; @@ -105,43 +106,42 @@ export class AdminGuard { } if (resource !== undefined && action !== undefined) { - const grants = this.permissionResolver - ? await this.permissionResolver.getGrants(userId) - : null; - - if (grants !== null) { - const allowed = grants.some((g) => g.resource === resource && g.action === action); - if (!allowed) { - this.emitUnauthorized(userId, userRecord.role, resource, action, ip, userAgent); - throw new ORPCError('FORBIDDEN', { - message: `Missing permission: ${String(resource)}:${String(action)}`, - data: { - reason: AuthGuardReasonSchema.enum.permission_denied, - resource: String(resource), - action: String(action), - }, - }); - } - } else { - // Bootstrap path: seed admin has user.role='admin' but no DB assignment row. DB revocation is NOT authoritative while a static role still grants - revoke the static role to fully deny. - const check = userRole.authorize({ [resource]: [action] }); - if (!check.success) { - this.emitUnauthorized(userId, userRecord.role, resource, action, ip, userAgent); - throw new ORPCError('FORBIDDEN', { - message: `Missing permission: ${String(resource)}:${String(action)}`, - data: { - reason: AuthGuardReasonSchema.enum.permission_denied, - resource: String(resource), - action: String(action), - }, - }); - } + const grants = await this.resolveGrants(userId); + const allowed = this.checkGrant(grants, userRole, resource, action); + if (!allowed) { + this.emitUnauthorized(userId, userRecord.role, resource, action, ip, userAgent); + throw new ORPCError('FORBIDDEN', { + message: `Missing permission: ${String(resource)}:${String(action)}`, + data: { + reason: AuthGuardReasonSchema.enum.permission_denied, + resource: String(resource), + action: String(action), + }, + }); } } return { userId, role: userRecord.role, ip, userAgent }; } + private resolveGrants(userId: string): Promise { + return this.permissionResolver + ? this.permissionResolver.getGrants(userId) + : Promise.resolve(null); + } + + private checkGrant( + grants: AdminGrant[] | null, + userRole: (typeof roles)[keyof typeof roles] | undefined, + resource: string, + action: string, + ): boolean { + if (grants !== null) { + return grants.some((g) => g.resource === resource && g.action === action); + } + return userRole?.authorize({ [resource]: [action] }).success ?? false; + } + private emitUnauthorized( userId: string, role: string | undefined,