Skip to content

feat: allow sync admin check - #165

Merged
frodi-karlsson merged 2 commits into
mainfrom
allow-sync-admin-check
Oct 3, 2026
Merged

frodi-karlsson merged 2 commits into
mainfrom
allow-sync-admin-check

Conversation

@frodi-karlsson

@frodi-karlsson frodi-karlsson commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

allows this example usage:

const admin = await adminService.resolveAdmin(req) // once per request

const result = adminService.checkPermissions(admin, ['MY_ROLE'])
const adminInfo = adminService.requireGranted(result) // sync, throws 401/403

it moves logging responsibilities to the caller to decouple from the request:

const result = adminService.checkPermissions(admin, reqPermissions)
if (result.email && !result.authDisabled) {
  logger.log(`${result.email} permissions check: ${result.granted ? 'GRANTED' : 'DENIED'}`)
}
return adminService.requireGranted(result)

@frodi-karlsson frodi-karlsson changed the title allow sync admin check feat: allow sync admin check Oct 2, 2026
@frodi-karlsson
frodi-karlsson force-pushed the allow-sync-admin-check branch 5 times, most recently from a87e829 to b20aff2 Compare October 2, 2026 13:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The refactor affects central authentication and authorization enforcement, warranting final human security review.

Review effort: Balanced
Findings: None

What changed in this PR

Adds synchronous permission evaluation after resolving an administrator once per request.

Changes:

  • Introduces resolveAdmin, checkPermissions, and requireGranted.
  • Preserves existing asynchronous permission APIs and adds focused tests.
  • Adds a reusable request mock and removes a stale pnpm trust exclusion.
File Description
pnpm-workspace.yaml Removes an obsolete package trust-policy exception.
packages/​backend-lib/​src/​test/​mocks.ts Adds a lightweight BackendRequest test helper.
packages/​backend-lib/​src/​admin/​base.admin.service.ts Separates admin resolution, permission checks, and enforcement.
packages/​backend-lib/​src/​admin/​admin.resource.test.ts Tests the new permission workflow and compatibility paths.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@frodi-karlsson
frodi-karlsson marked this pull request as ready for review October 2, 2026 13:55

@kirillgroshkov kirillgroshkov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I trust that you know what you're doing.
The API looks good.

@frodi-karlsson
frodi-karlsson merged commit f2fc8ce into main Oct 3, 2026
4 checks passed
@frodi-karlsson
frodi-karlsson deleted the allow-sync-admin-check branch October 3, 2026 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants