Restructure authentication & authorization configuration - #3022
Conversation
📝 WalkthroughWalkthroughThe PR replaces legacy external-token and IDP authorization settings with unified authorization configuration, introduces role-based file authentication and JWT claims, validates namespace-aware role mappings, mounts ChangesAuthorization and role-mapping migration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant StartPlatformAPIServer
participant loadRoleScopeMap
participant validateFileUserRoles
participant AuthLoginHandler
StartPlatformAPIServer->>loadRoleScopeMap: load configured role mappings
loadRoleScopeMap-->>StartPlatformAPIServer: return roleScopeMap
StartPlatformAPIServer->>validateFileUserRoles: validate configured file-user roles
validateFileUserRoles-->>StartPlatformAPIServer: return validation result
StartPlatformAPIServer->>AuthLoginHandler: register handler with roleScopeMap
AuthLoginHandler-->>StartPlatformAPIServer: issue token with effective scopes and roles
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 5
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/production/README.md (1)
116-124: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the full
[platform_api.auth.*]namespace in both production and troubleshooting documentation.The unified configuration contract is rooted at
platform_api; using absolute TOML tables under[auth...]writes settings the Platform API does not read.
portals/ai-workspace/production/README.md#L116-L124: rename the authorization and claim-mapping tables to[platform_api.auth.authorization]and[platform_api.auth.claim_mappings].portals/ai-workspace/README.md#L377-L377: document the local override as[platform_api.auth] mode = "idp".🤖 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/production/README.md` around lines 116 - 124, Update the production documentation tables at portals/ai-workspace/production/README.md:116-124 to use [platform_api.auth.authorization] and [platform_api.auth.claim_mappings]. Also update the local override at portals/ai-workspace/README.md:377 to document [platform_api.auth] mode = "idp".
🧹 Nitpick comments (1)
platform-api/internal/middleware/role_scope_map.go (1)
69-95: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRegex is both looser and stricter than the doc comment claims.
^[a-z0-9_]+:[a-z0-9_:*]+$accepts*anywhere (ap:*:read,dp:***), not just as a:*tail, and rejects-entirely — so a sibling component that names a scope likedp:api-key_readwould fail startup even though this server has no authority over that namespace. Consider anchoring the wildcard to the tail and deciding explicitly whether-is allowed in foreign scope names.♻️ Tighter shape, hyphen-tolerant
-var wellFormedScope = regexp.MustCompile(`^[a-z0-9_]+:[a-z0-9_:*]+$`) +var wellFormedScope = regexp.MustCompile(`^[a-z0-9_-]+:[a-z0-9_-]+(?::[a-z0-9_-]+)*(?::\*)?$`)🤖 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 `@platform-api/internal/middleware/role_scope_map.go` around lines 69 - 95, Update wellFormedScope to enforce the documented namespace:name shape with an optional wildcard only as a trailing :* segment, while allowing hyphens in foreign namespace or scope names such as dp:api-key_read. Keep ValidateRoleScopeMap’s namespace-scoped behavior unchanged: apply this syntax check to all scopes, then validate only PlatformScopePrefix entries against the registry.
🤖 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 `@platform-api/config/config.go`:
- Around line 953-964: Update the file-user validation near the existing
Role/Scopes checks to reject users with Role empty and Scopes non-empty when
auth.authorization.mode is "role". Use the surrounding configuration symbols and
return the same startup-validation error style, while preserving scopes-only
users in non-role authorization modes.
- Around line 900-908: Update validateIDPConfig to reject IDP configurations
with an empty audience list, alongside the existing JWKS URL and issuer checks.
Return a startup validation error identifying auth.idp.audience, while
preserving successful validation for configurations with at least one audience.
In `@platform-api/internal/handler/auth_login.go`:
- Around line 115-121: Update the matching-role claim handling around
matched.Role and claimKey(cm.Roles, "roles") so file-mode tokens remain readable
when claim_mappings.roles is dotted, such as realm_access.roles. Either reject
dotted role mappings in file mode during configuration validation or emit the
corresponding nested claim structure; preserve the existing flat claim behavior
for non-dotted mappings.
In `@platform-api/README.md`:
- Line 9: Update the authentication documentation in the README to consistently
use the runtime configuration keys public_key_file and private_key_file instead
of public_key and private_key, including the references around internal_token
configuration. Also align the idp-mode guidance with the corresponding
references in portals/ai-workspace/distribution/README.md.
In `@platform-api/resources/roles.yaml`:
- Around line 76-97: Update ValidateRoleScopeMap to validate every dp:* scope in
roles.yaml against the Developer Portal’s declared scope set, rejecting unknown
or typoed names during startup/config validation while preserving existing
malformed-scope and ap:* validation.
---
Outside diff comments:
In `@portals/ai-workspace/production/README.md`:
- Around line 116-124: Update the production documentation tables at
portals/ai-workspace/production/README.md:116-124 to use
[platform_api.auth.authorization] and [platform_api.auth.claim_mappings]. Also
update the local override at portals/ai-workspace/README.md:377 to document
[platform_api.auth] mode = "idp".
---
Nitpick comments:
In `@platform-api/internal/middleware/role_scope_map.go`:
- Around line 69-95: Update wellFormedScope to enforce the documented
namespace:name shape with an optional wildcard only as a trailing :* segment,
while allowing hyphens in foreign namespace or scope names such as
dp:api-key_read. Keep ValidateRoleScopeMap’s namespace-scoped behavior
unchanged: apply this syntax check to all scopes, then validate only
PlatformScopePrefix entries against the registry.
🪄 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: eb270f1b-e719-400c-b60f-21523e3baf16
📒 Files selected for processing (30)
distribution/all-in-one/docker-compose.yamlkubernetes/helm/platform-api-helm-chart/templates/configmap.yamlkubernetes/helm/platform-api-helm-chart/values.yamlplatform-api/README.mdplatform-api/config/config-template.tomlplatform-api/config/config.goplatform-api/config/config.tomlplatform-api/config/config_multifile_test.goplatform-api/config/config_test.goplatform-api/config/default_config.goplatform-api/internal/handler/auth_login.goplatform-api/internal/handler/auth_login_test.goplatform-api/internal/middleware/auth.goplatform-api/internal/middleware/auth_role_extraction_test.goplatform-api/internal/middleware/role_scope_map.goplatform-api/internal/middleware/role_scope_map_test.goplatform-api/internal/server/role_scope_map_test.goplatform-api/internal/server/scope_route_coverage_test.goplatform-api/internal/server/server.goplatform-api/plugins/eventgateway/plugin.goplatform-api/resources/roles.yamlportals/ai-workspace/Makefileportals/ai-workspace/README.mdportals/ai-workspace/distribution/README.mdportals/ai-workspace/docker-compose.yamlportals/ai-workspace/production/README.mdportals/developer-portal/Makefileportals/developer-portal/distribution/README.mdportals/developer-portal/docker-compose.platform-api.yamlportals/developer-portal/docker-compose.yaml
…nd enforce flat claim mappings in file mode
…y ensuring organization-level context is set before accessing the Service Provider page.
…ent and enhance claim mapping handling. Update related tests and documentation for clarity on nested claim paths.
…ration files and update references in documentation and tests for consistency.
Restructure authentication & authorization configuration
Purpose
Authorization settings in the Platform API lived under
[platform_api.auth.idp](validation_mode,role_mappings) and[platform_api.auth](scope_validation). That placement made role-based authorization reachable only inidpauth mode, even though a token minted by an enterprise IDP carries the samerolesclaim whether the platform verifies it against a JWKS endpoint or with a local public key. It also left file-mode users with no way to express grants as a role — every user needed a hand-maintained ~100-scope string.This PR separates authentication (how a token is verified) from authorization (what a verified token may do), mirroring the split Kubernetes draws between its authn/authz configs and Envoy draws between JWT providers and rules.
Changes
New
[platform_api.auth.authorization]section — validated in every auth mode, not inside any one mode's branch:auth.scope_validationauth.authorization.enabledauth.idp.validation_modeauth.authorization.mode(scope|role)auth.idp.role_mappingsauth.authorization.role_to_scope_mappingThe mapping setting is named
role_to_scope_mappingrather thanrole_mappings: "mappings" alone didn't say what was mapped to what, and the file it points at is now read by both authorization modes and by file-mode login, so the direction (roles → scopes) is worth spelling out at the config key. The file itself is renamed to match —resources/roles.yaml→resources/role-to-scope-mapping.yaml— since a bareroles.yamlin a mounted config directory reads like a list of roles rather than a grant table.Auth mode renamed
external_token→internal_token. The old name suggested a third-party issuer; the mode actually verifies tokens minted by another trusted platform component holding the matching RSA private key.File-mode users are granted roles, and only roles.
auth.file.users[].scopesis gone;auth.file.users[].rolesnames one or more roles from the mapping file and is the user's entire grant. The login endpoint expands them into the token'sscopeclaim and also emits the role names themselves as therolesclaim — so the same token works whether authorization runs inscopeorrolemode. Privileges are now defined in exactly one place, so no per-user scope string can drift out of step with the roles it was meant to mirror, and the shipped admin no longer carries a ~100-scope literal.rolesis a list, not a single name. A user whose persona spans two shipped roles names both (roles = ["ap_publisher", "ap_subscriber"]) and gets the union of what they grant — most-permissive wins, duplicate scopes collapsed — rather than forcing a sixth role to be defined for the combination, or a per-user scope list to be reintroduced for it. That also matches the shape the rest of the system already uses: a token'srolesclaim is a list, and role authorization mode already unioned across it, so file mode was the one place a multi-role identity couldn't be expressed.role-to-scope-mapping.yaml(formerlyroles.yaml) reworked. Roles are renamed into the platform's own namespace rather than any one IDP's convention, since the same file now serves every auth mode:platform-admin→ap_admin,platform-operator→ap_operator,platform-viewer→ap_viewer.platform-developercovered both API lifecycle and application/subscription ownership, so it splits intoap_publisher(lifecycle) andap_subscriber(own applications, subscriptions and keys). Every role gains sub-resource scopes (ap:rest_api:deployment:manage,ap:gateway:token:manage, …) and Developer Portaldp:*scopes, so one role describes a persona across the whole platform rather than only this server — which is what makes a per-user scope list unnecessary.ValidateRoleScopeMapis now namespace-scoped. Anap:scope must be declared in the OpenAPI spec (unchanged, fail-fast at startup); a scope in another component's namespace is checked for well-formedness only — this server mints those but never enforces them, so it can neither confirm nor deny their existence. The unusedap:devportal:*andap:git:readscopes are removed platform-wide — no spec declared them and no component enforced them (the AI Workspace'sDEVPORTAL_*constants were never referenced), so the minted-scope allowlist that existed to carry them is gone too. The well-formedness check itself was tightened: segments may contain hyphens (a foreign namespace picks its own convention, e.g.dp:api-key_read) and*is accepted only as a trailing segment, not as a free-floating character. Event-gateway scopes stay commented out in the shipped file since they're only declared on a build that compiles in that plugin (-tags experimental).Startup validation added (all fail-closed):
auth.authorization.modemust bescopeorrole— rejected even whenenabled = false, so flipping enforcement back on isn't what surfaces a typo.mode = "role"requires bothclaim_mappings.rolesandrole_to_scope_mapping. Without the mapping file, role names would be used verbatim as scope values.roles, and no entry may be blank — a user without a usable role authenticates successfully and is then denied every request.role_to_scope_mappingto be set, and every role a user names must exist in the loaded file (validateFileUserRoles) — a typo would otherwise surface as a login that succeeds and 403s on everything.Nested claim mappings work in file mode. Every
claim_mappings.*field already accepted a dot-separated path into a nested claim (realm_access.roles, the Keycloak shape) on the read side, but the login endpoint wrote flat keys — so a config using that layout signed a claim literally namedrealm_access.roles, whichresolveClaimPaththen failed to find. For therolesmapping that meant a user who logs in successfully and is then denied every request. The endpoint now writes throughsetClaim, the write-side mirror ofresolveClaimPath: intermediate objects are created and merged into (never replaced), so mappings sharing a prefix coexist and one mapping serves both directions in whichever auth mode is active.Defaults:
claim_mappings.rolesnow defaults to"roles"(what Asgardeo and Entra ID emit, and what the file-mode login endpoint signs), so switching torolemode needs no extra claim wiring.role_to_scope_mappingstays empty inDefaultConfigon purpose — the mapping file is operator-owned and mounted, so a built-in path would make startup depend on a file the image doesn't carry.Packaging:
role-to-scope-mapping.yamlis now mounted at/etc/platform-api/role-to-scope-mapping.yamlin every compose pack (all-in-one, AI Workspace, Developer Portal), and the two portal Makefiles copy/rewrite it into their distributions. It's mounted rather than baked into the image so operators can edit what a role grants. The shippedconfig.tomladmin user is now justroles = ["ap_admin"]. In Helm,config.auth.file.admin.scopesis replaced by a requiredconfig.auth.file.admin.roleslist, and the chart now rendersconfig.auth.authorization.rolesinto its config ConfigMap and mounts it atroleToScopeMapping— so a defaulthelm installin file mode has a mapping file without the operator wiring one up. The chart ships onlyap_adminthere;platform-api/resources/role-to-scope-mapping.yamlremains the full sample set to copy from.Breaking change / migration
Existing configs must be updated. The retired keys no longer map to anything and are silently ignored, so the effective setting becomes the default (
enabled = true,mode = "scope") rather than what the file says:auth.mode = "external_token"must become"internal_token"; startup fails with the mode-list error otherwise. The mapping file itself is renamedroles.yaml→role-to-scope-mapping.yaml, so a custom file needs renaming (or the path in config pointed at wherever you keep it), and one using the oldplatform-*role names needs those renamed too (or the role names in config updated to match).File-mode users must be converted from a scope list to a roles list.
scopesis no longer a recognized key, and a user withoutrolesfails startup:rolesis a list, so a user whose persona spans two shipped roles names both (roles = ["ap_publisher", "ap_subscriber"]) and gets the union of what they grant, most-permissive wins, with duplicate scopes collapsed — rather than needing a role defined for the combination.Since roles are now the only grant,
auth.authorization.role_to_scope_mappingis required in file mode even when authorization runs in the defaultscopemode. A leftoverscopeskey is ignored rather than rejected, but missingrolesfails startup with a message naming the user, so a half-migrated config can't start and silently grant nothing.Docs updated for the new shape:
platform-api/README.md(RBAC section, role table, file-mode granting — including the multi-role example), both portal distribution READMEs,portals/developer-portal/README.md's local-auth example,config-template.toml, and Helmvalues.yamlcomments.Also corrected while in there:
platform-api/README.mddocumented the JWT keys aspublic_key/private_key, but the runtime keys arepublic_key_file/private_key_file— a copied snippet would have been silently ignored and failed the key-required check.portals/ai-workspace/production/README.mdshowed[auth.authorization]/[auth.claim_mappings]without theplatform_apiprefix the surrounding blocks use, andportals/ai-workspace/README.md's local-override row named[auth] mode = "idp".Testing
TestValidateAuthorizationConfigcovering each mode, missingrolesclaim, missing mapping file, unknown/empty mode, and invalid-mode-while-disabled.TestValidateAuthConfig_RoleAuthorizationInInternalTokenMode— asserts role authorization validates outsideidpmode, the case that was previously unreachable.scopemode.effectiveScopes: single-role expansion, cross-namespace roles, duplicate-within-a-role dedupe, multiple roles unioning their scopes, a scope granted by two of the user's roles appearing once, unknown role grants nothing, no roles grants nothing — plus therolesclaim in issued tokens.TestLoadConfig_FileUserRolesList— loads the shippedroleslist shape end to end and asserts{{ env }}interpolation reaches inside array elements. A regression there would hand the raw{{ env … }}string to the role lookup, and the user would silently be granted nothing rather than failing loudly.role_scope_maptests for the namespace-scoped validation (foreign namespace accepted, hyphenated foreign scope accepted, non-trailing wildcard rejected, malformed scope rejected).TestShippedSampleRolesValidateAgainstShippedSpeccovers the shippedrole-to-scope-mapping.yamlagainst the shipped OpenAPI spec, so a pack can't ship a mapping that fails startup.tests/integration-e2enow mounts the shippedrole-to-scope-mapping.yamland grantsap_admin; the developer-portal IT suite getsit/configs/roles-platform-api-it.yamlwith one role per account (dp_admin_it/dp_publisher_it/dp_developer_it) carrying exactly the scope sets those users had.tests/integration-e2e. All three compose files setAPIP_CP_AUTH_FILE_BASED_*variables that reach nothing: platform-api has no env-override layer, and an env var only takes effect via the{{ env }}token that names it — which the fixture toml spellsAPIP_CP_AUTH_FILE_ORGANIZATION_*. Renamed to the names actually read, and dropped..._ENABLED,APIP_CP_WEBHOOK_GATEWAY_TYPE(no such config key), and the wholeAUTH_FILE_BASED_USERSinjection path including itsos.Setenvinsuite_test.go. The@devportaladmin gets itsdp:*scopes fromap_adminin the mountedrole-to-scope-mapping.yaml, which is what was already happening.tests/ai-workspace-cli-e2emigrated. It was entirely pre-restructure and could not have started: no[platform_api]prefix,[auth.file_based], an HMACsecret_key, no RSA keypair, no TLS certs, no role-to-scope mapping. The toml is rewritten to the current shape and the compose now follows theintegration-e2epattern (certgen + jwtkeygen init containers, keys/certs volumes,role-to-scope-mapping.yamlmount).TestSetClaimNestedPaths: flat mapping writes a top-level claim, dotted mapping writes the nested object (and leaves no literal dotted key), two mappings sharing a prefix both survive, deeper nesting creates every intermediate level.go build ./...andgo test ./...pass inplatform-api;go vetclean in both e2e suites.helm templaterenders cleanly; the chart's defaultap_adminscope list was checked to be a subset of the shippedrole-to-scope-mapping.yaml's, so it is covered by the spec-validation test above.Reviewer notes
StartPlatformAPIServer— it now needs the loaded role→scope map, so it's registered afterloadRoleScopeMap. It stays public viacfg.Auth.SkipPaths, unchanged.loadRoleScopeMapis no longer gated on auth mode or authorization mode; it loads whenever a path is configured. Config validation is what requires the path where it's needed.ValidateRoleScopeMapmust run after plugin OpenAPI specs are merged, since plugin-contributedap:scopes are validated against the registry.ap:websub_api:manage/ap:webbroker_api:manage. Those are declared only on an-tags experimentalbuild and no CI job builds one, so they were inert; they are not in theap_adminrole and the fixtures no longer request them.tests/ai-workspace-cli-e2ewas migrated rather than left behind (see the fixture section above). It runs daily in CI and needs Docker, so the config was brought to the current contract but the suite has not been run end to end here — worth one manual run before merging.role_to_scope_mappingrename touches a lot of files but is mechanical: config key, one Go field (Authorization.RoleToScopeMapping), one Helm value (roleToScopeMapping), the filename, and every mount path/subPath/ConfigMap key that named it. The behavioural diff is confined toFileBasedUser.Roles,effectiveScopes,validateFileBasedConfig, andvalidateFileUserRoles.gateway-controller'sauth.idp.role_mappingis a different setting (IDP role → local gateway role, in a separate config tree) and is deliberately left untouched by this rename. Worth a second opinion on whether the asymmetry is acceptable or whether it should follow.-tags experimentalbuild is broken onmain(undefined: api.OrganizationSubscriptionin the event-gateway plugin), so it could not be compiled to verify the plugin's scope contribution end-to-end. Pre-existing and unrelated.