Fix role-based authorization in BFF with role-to-scope mapping support - #3089
Conversation
…support - Introduced AuthorizationConfig to manage authorization modes (scope/role) in AuthConfig. - Updated claim mapping to handle role-based scope expansion. - Added role-to-scope mapping functionality to align with platform API. - Enhanced tests to validate role mode behavior and scope expansion. - Updated configuration template to include role-to-scope mapping settings.
📝 WalkthroughWalkthroughThe AI Workspace BFF now supports scope- and role-based authorization. It validates and loads role mappings at startup, then derives effective scopes from JWT roles. Platform middleware uses the renamed ChangesAuthorization flow
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant IDP
participant BFF
participant RoleScopeMap
participant PlatformAPI
BFF->>RoleScopeMap: Load role-to-scope mapping
IDP->>BFF: Send JWT roles
BFF->>RoleScopeMap: Expand roles into scopes
RoleScopeMap-->>BFF: Return effective scopes
BFF->>PlatformAPI: Send authorized request
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
….in/check.v1 dependencies
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
portals/ai-workspace/bff/internal/config/config.go (1)
312-327: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNormalize
Authorization.Modecase before validating it.
normalize()lowercasesc.Auth.Modebeforevalidate()compares it.c.Auth.Authorization.Modegets no such treatment, sovalidate()rejects a functionally valid value such as"Role"or"SCOPE"with a generic invalid-mode error. Apply the same normalization used forAuth.Mode.🐛 Proposed fix
func (c *Config) normalize() { c.Logging.Level = strings.ToLower(c.Logging.Level) c.Logging.Format = strings.ToLower(c.Logging.Format) c.Auth.Mode = strings.ToLower(c.Auth.Mode) + c.Auth.Authorization.Mode = strings.ToLower(c.Auth.Authorization.Mode)Also applies to: 338-344
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/ai-workspace/bff/internal/config/config.go` around lines 312 - 327, Update Config.normalize to lowercase c.Auth.Authorization.Mode alongside c.Auth.Mode before validation. Preserve the existing authorization configuration and validation flow so case-insensitive values such as “Role” and “SCOPE” are normalized before validate compares them.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@portals/ai-workspace/bff/internal/session/role_scope_map.go`:
- Around line 80-93: Update the mapping-loading function around the existing
os.Stat and os.ReadFile calls to open the cleaned path once, validate the opened
descriptor’s FileInfo with Mode().IsRegular(), and reject non-regular files.
Read through io.LimitReader with a limit of maxMappingBytes+1, reject data
exceeding maxMappingBytes, then parse the bounded content; remove the separate
pre-validation and path-based read so replacements and FIFOs cannot bypass the
size or regular-file checks.
---
Outside diff comments:
In `@portals/ai-workspace/bff/internal/config/config.go`:
- Around line 312-327: Update Config.normalize to lowercase
c.Auth.Authorization.Mode alongside c.Auth.Mode before validation. Preserve the
existing authorization configuration and validation flow so case-insensitive
values such as “Role” and “SCOPE” are normalized before validate compares them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9aabc9c2-b59a-46a2-a460-2c92a15fa8d4
📒 Files selected for processing (13)
platform-api/internal/middleware/auth.goplatform-api/internal/middleware/scope_enforcer_test.goportals/ai-workspace/bff/internal/config/config.goportals/ai-workspace/bff/internal/config/config_test.goportals/ai-workspace/bff/internal/config/default_config.goportals/ai-workspace/bff/internal/config/oidc_scopes_test.goportals/ai-workspace/bff/internal/server/server.goportals/ai-workspace/bff/internal/session/claims.goportals/ai-workspace/bff/internal/session/claims_test.goportals/ai-workspace/bff/internal/session/role_scope_map.goportals/ai-workspace/bff/internal/session/role_scope_map_test.goportals/ai-workspace/configs/config-template.tomlportals/ai-workspace/docker-compose.yaml
- Updated LoadRoleScopeMap function to open the file and validate its type using os.Open and f.Stat. - Changed error message for non-regular files to be more specific. - Enhanced test case to reflect the updated error message for directory paths.
…n mode - Updated the normalize function to apply case-folding to the Auth.Authorization.Mode field. - Improved comments for clarity on the normalization process and its implications.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@portals/ai-workspace/configs/config-template.toml`:
- Line 284: Update the explicit scope value in the template’s scope
configuration to include the standalone ap:api_key:read permission, and extend
the existing OIDC scope test covering this template to assert that the
permission is requested.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d6572bc-2a26-4dde-b8ab-378a0fa6a867
⛔ Files ignored due to path filters (1)
portals/ai-workspace/bff/go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
portals/ai-workspace/bff/go.modportals/ai-workspace/bff/internal/config/config.goportals/ai-workspace/bff/internal/config/oidc_scopes_test.goportals/ai-workspace/bff/internal/session/role_scope_map.goportals/ai-workspace/bff/internal/session/role_scope_map_test.goportals/ai-workspace/configs/config-template.tomlportals/ai-workspace/docker-compose.yamlportals/ai-workspace/src/config.env.ts
💤 Files with no reviewable changes (1)
- portals/ai-workspace/src/config.env.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- portals/ai-workspace/bff/internal/session/role_scope_map.go
- portals/ai-workspace/docker-compose.yaml
- portals/ai-workspace/bff/internal/session/role_scope_map_test.go
- portals/ai-workspace/bff/internal/config/oidc_scopes_test.go
|
issue: #3093 |
Problem
Logging into the AI Workspace against Microsoft Entra ID succeeded, but every operation
in the UI appeared blocked.
/api/sessionreported:{ "user": { "scopes": ["access"], "role": "", ... } }Three independent defects combined to produce that.
1. The BFF read scopes from the wrong place under role authorization. When the
Platform API runs
[auth.authorization] mode = "role", a caller's effective scopes arenot in their token — the server derives them per request by expanding the
rolesclaimthrough
role-to-scope-mapping.yaml(resolvePlatformRoles). The BFF instead read thetoken's
scope/scpclaim verbatim. An Entra token carries noap:*scope at all — itsscpholds only the application's own API scope (access) — so the SPA, which gatesevery action on the scopes
/api/sessionreports, hid everything the Platform API wouldin fact have authorized.
2. An array-valued
rolesclaim was silently dropped.UserFromClaimsread roles viaa
stringtype assertion. Entra emitsroles: ["ap_admin"], so the assertion failed anduser.rolecame back empty — for any IDP using the array form.3.
GET /me/api-keyswas unreachable. It accepts onlyap:api_key:readorap:api_key:all:manage, and the default OIDC scope set requested neither, so listingyour own API keys always returned 403. Found by the new coverage test, not by inspection.
What changed
BFF derives effective scopes the same way the Platform API does
New
[ai_workspace.auth.authorization]table, mirroring[platform_api.auth.authorization]key for key:In
rolemode the BFF expands therolesclaim through the grant table and reports theresult, so
/api/sessionnow returns the real permission list:{ "user": { "scopes": ["ap:organization:manage", "ap:project:manage", ...] } }internal/session/role_scope_map.goloads the table — the same file, in the same shape,that the Platform API reads, and that api-portal already reads through its own
roleScopeMap.js. Both services must mount the same file, or the UI's view of what arole grants drifts from what is enforced;
docker-compose.yamlnow mounts it into theai-workspaceservice alongsideplatform-api.The Platform API remains the only enforcement point. This expansion exists for
display and UI gating.
Fail-closed configuration
modefails startup rather than falling through to reading the scopeclaim — the failure mode there is a UI in which nothing is permitted and no error
saying why.
mode = "role"withoutrole_to_scope_mappingfails startup: the expansion could onlyever yield zero scopes.
name, or declares a role twice fails startup. Duplicate roles are rejected ratherthan last-wins, since one entry would otherwise be silently inert depending on file
order.
A role the operator never mapped granting nothing is a real deny-by-default outcome and
matches what the Platform API decides for the same token; falling back would show
actions as available that then 403.
..on the raw input, before normalization (filepath.Cleanwould collapse traversal into a path that passes a later check), plus null bytes, a
directory, and anything over 1 MiB.
Claim reading
strSliceClaimreads a claim that may be a single string, a space-delimited string, oran array — Asgardeo sends roles as a string, Entra ID as an array.
Default OIDC scopes
Added
ap:api_key:read(defect 3). Deliberately not theap:api_key:all:managealternative: that is the cross-user ownership override (GO-AUTH-019) and must be
requested explicitly by a deployment that wants it, never granted to every session.
The full granular set is otherwise unchanged. A trim to
:manage/:readper resourcewas considered and rejected: an IDP grants the intersection of what is requested and what
the user is entitled to, so trimming would silently strip a least-privilege user's grant
— a user holding only
ap:rest_api:createwould lose it.Worth recording, since it is easy to assume otherwise:
:manageis not hierarchical.scopeSatisfies("ap:gateway:manage", "ap:gateway:token:read")isfalse. Parent:manageworks only where an operation's own OpenAPIsecurityblock lists it as analternative — per-endpoint enumeration, not scope expansion. The event-gateway plugin's
ap:websub_api:deployment:manageandap:webbroker_api:api_key:manageare not coveredby their parents at all.
Platform API
keyPlatformRolesrenamed tokeyRoleswith context value"roles", matching thedefault claim name. No behaviour change.
Tests
role_scope_map_test.go— loading, deduplication, and every rejection path (duplicaterole, missing name, absent
roleslist, malformed YAML, traversal, null byte, missingfile, directory). Also loads the shipped grant table, so it cannot drift into a
shape this loader rejects.
claims_test.go— role-mode expansion for the exact Entra token shape; that thetoken's own
accessscope does not leak through; multiple roles unioning; an unmappedrole granting nothing; scope mode unaffected; roles as a string.
oidc_scopes_test.go— parses both OpenAPI specs and asserts the default scope setsatisfies all 148 scoped operations; that the ownership override is absent; that
offline_accessis present; and that representative granular scopes are stillrequested, so a future trim cannot silently break least-privilege users. This test is
what caught defect 3.
config_test.go— scope mode is the default, role mode parses, role mode without amapping fails, an invalid mode fails, and the mapping path never reaches the browser.
Documentation
New
portals/ai-workspace/production/ENTRA_ID_SETUP.md— the Entra ID counterpart to theexisting Asgardeo guide (
production/README.md). It covers app registration, and thethree ways Entra differs from Asgardeo materially enough to change the setup:
ap:*scopesAlso documented, each with cause and fix, from bringing a real tenant up:
AADSTS90015: Requested query string is too long— the defaultap:*scope set is~5 KB encoded, past Entra's authorize-URL limit. Entra deployments must override
[auth.oidc] scopewith their own resource scope; the shared default is not the thingto change.
AADSTS500011: resource principal not found— Expose an API not completed.requestedAccessTokenVersionleft unset, so tokens carryiss: https://sts.windows.net/<tid>/and every call fails the Platform API's issuercheck while login itself succeeds.
/api/session401 in a redirect loop — the callback failed and the SPA re-triggeredlogin, masking the real error; the reason is logged at debug, not info.
is gated on an
org_handleclaim (AppShellContext.tsx), which Entra does not emit, soPOST /organizationsnever fires and the fallback path dead-ends on "Organization notfound. Please contact your administrator." The guide documents the manual
curl; seeFollow-ups.
ap_admin— no shipped role grantsap:organization:create, and thepre-registration
FOREIGN KEY constraint failedwarning names the raw tenant ID ratherthan the actual cause.
idp_organization_ref_uuid, which the lookup never matches, so it is invisible toIDP-mode auth.
Compatibility
Default
mode = "scope"preserves current behaviour exactly — an operator who never addsthe new table sees no change. Scope-mode deployments (Asgardeo, WSO2 IS) are unaffected
apart from the added
ap:api_key:read.Verification
go build ./...andgo test ./...pass for both the BFF andplatform-api.Not yet verified end to end against a live Entra tenant. After deploying, confirm the
expansion with:
["access"]means the BFF is still in scope mode;[]means the token's roles are not inthe grant table.
Follow-ups (not in this PR)
org_handlecan onboard through the UI. That is most of them,Entra included. The fallback branch in
AppShellContext.tsxshould provision from thetoken's organization id rather than dead-ending on an error.
ListOrganizationsForUsercallsAddMembershipwith an unresolved organization claim, surfacing a FK error instead of"the organization claim does not resolve".
OrganizationResolverMiddlewareknowsresolution failed but does not record it; a
resolvedflag in the context would letthe heal skip and report accurately.
ap:organization:create, soap_admin's blanket:manageis theonly bootstrap path.
session.idle_timeoutis inert. It is parsed and validated but nothing callsStore.Touch, so there is no sliding idle window — an ASVS V3.3.2 gap. Either wire itup or remove the key.
refreshLocksis per-process, so the single-flight guarantee around refresh-tokenrotation disappears with more than one BFF replica.