AI Workspace - Configuration Convention & the /me Endpoint#2642
Conversation
- Updated the configuration reference to clarify the use of environment variables and interpolation tokens for secrets. - Introduced a section on managing sensitive information, emphasizing the use of mounted secret files and environment variables for the OIDC client secret. - Adjusted the examples in the authentication setup to reflect the new configuration practices. - Improved consistency in naming conventions for environment variables across documentation and code. - Added tests to validate the loading of configuration values from environment variables and files, ensuring proper error handling for missing or invalid configurations.
- Restructured the configuration in `config.toml` to group keys into TOML tables for better organization and clarity. - Updated the documentation to reflect the new configuration format, emphasizing the use of interpolation tokens for environment variables and secrets. - Clarified the handling of OIDC settings, including client ID and secret, ensuring they are referenced correctly in the configuration. - Improved consistency in naming conventions for environment variables across documentation and code. - Added tests to validate the loading of configuration values from environment variables and files, ensuring proper error handling for missing or invalid configurations.
- Updated the secrets management documentation to clarify the required environment variable for the Platform API encryption. - Modified the Dockerfile to remove unnecessary GOFLAGS, streamlining the build process. - Improved error handling in the configuration loading logic to ensure invalid authentication modes are properly reported. - Added tests for the shipped configuration to validate loading behavior with and without environment variables, ensuring correct defaults and overrides. - Adjusted the OIDC claim mappings in the configuration to align with the BFF defaults, enhancing consistency across components.
Dependency Validation ResultsDependency name: github.com/pelletier/go-toml/v2 Next Steps
|
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughThe PR migrates AI Workspace to tokenized TOML configuration, adds structured OIDC and Platform API settings, updates BFF loading and browser-safe runtime configuration, and aligns deployment, frontend, build, test, and authentication documentation. ChangesAI Workspace configuration migration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
portals/ai-workspace/bff/internal/config/runtime_config.go (2)
67-79: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor:
auth_modeinbrowserSafeKeysis always overwritten by the forced value below.Since
out[runtimeKey("auth_mode")] = cfg.AuthMode(Line 78) unconditionally overwrites whatever the allowlist loop set for"auth_mode"(Line 31), keeping"auth_mode"inbrowserSafeKeysis dead weight — the raws["auth_mode"]value it would surface is discarded either way. Not incorrect, just redundant.♻️ Optional cleanup
var browserSafeKeys = []string{ // Identity of the deployment "domain", - "auth_mode", "default_org_region",🤖 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/runtime_config.go` around lines 67 - 79, Remove "auth_mode" from browserSafeKeys, since buildRuntimeConfig always sets the corresponding runtime key from cfg.AuthMode after processing the allowlist. Keep the existing forced auth_mode assignment and all other allowlisted keys unchanged.
28-53: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConfirm
moesif_app_api_keyis Moesif's publishable/collector key, not a secret.Moesif does have a "Publishable Application Id" explicitly meant for browser JS use, distinct from its private "Secret Application Id" — so
moesif_app_api_keyis plausibly safe here. Given this file's entire purpose is to prevent secrets from reaching the browser, and the name is easy to confuse with a private API key, please double-check that the config value intended for this key is always the publishable Moesif Application Id and never a secret/management key, and consider a more explicit name (e.g.moesif_publishable_app_id) to reduce future misconfiguration risk.🤖 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/runtime_config.go` around lines 28 - 53, Verify the configuration source for browserSafeKeys and confirm moesif_app_api_key is always the publishable Moesif Application Id, never a secret or management key; if the contract is ambiguous, rename the configuration key and all consumers to an explicit publishable-app-id name and update the allowlist accordingly.
🤖 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 `@docs/ai-workspace/authentication/oidc-auth.md`:
- Around line 33-60: Merge the split [oidc] TOML sections so each example
contains one [oidc] table, placing redirect_url, post_logout_redirect_url, and
client_secret before [oidc.claim_mappings]. Apply this to
docs/ai-workspace/authentication/oidc-auth.md lines 33-60 and
docs/ai-workspace/authentication/asgardeo-setup.md lines 156-179; do not leave
duplicate [oidc] headers or require readers to manually combine snippets.
In `@docs/ai-workspace/configuration.md`:
- Line 167: Revise the configuration documentation sentence near the
environment-variable override guidance to state that only keys containing an {{
env }} token can be overridden via APIP_AIW_ plus the uppercased key path. Do
not claim that arbitrary literal or absent config keys accept environment
overrides.
In `@portals/ai-workspace/bff/internal/config/settings.go`:
- Around line 160-171: Update settings.getdur to reject durations that are zero
or negative after successful time.ParseDuration parsing, returning a startup
error with the setting key and invalid value; continue returning the default for
missing or empty values and preserve valid positive durations.
In `@portals/ai-workspace/bff/main.go`:
- Around line 178-183: Update the TLS-related log and error messages in the
startup configuration flow to reference the actual TOML keys using `[tls]
cert_file`, `[tls] key_file`, and `[tls] enabled` (or equivalent dotted `tls.*`
notation), replacing bare underscore-joined names such as `tls_cert_file` and
`tls_enabled`. Apply this consistently to the affected messages near the TLS
checks while preserving their existing behavior.
In `@portals/ai-workspace/configs/config-template.toml`:
- Around line 231-233: Update the scope configuration default near the scope
setting so it never emits the literal “...” as an OAuth scope. Use the BFF’s
built-in full-scope fallback by making the template default empty, or replace it
with the complete valid scope list; preserve the documented behavior that
leaving APIP_AIW_OIDC_SCOPE unset requests the full ap:* set.
In `@portals/ai-workspace/vite.config.ts`:
- Around line 71-76: Update the Vite configuration around envPrefix so
APIP_AIW_OIDC_CLIENT_SECRET and any other secrets are not exposed through
import.meta.env. Keep only browser-safe AIW variables in the client-exposed
prefix, using a server-only namespace or explicit allowlist for secret
configuration while preserving the existing runtime naming contract for safe
values.
---
Nitpick comments:
In `@portals/ai-workspace/bff/internal/config/runtime_config.go`:
- Around line 67-79: Remove "auth_mode" from browserSafeKeys, since
buildRuntimeConfig always sets the corresponding runtime key from cfg.AuthMode
after processing the allowlist. Keep the existing forced auth_mode assignment
and all other allowlisted keys unchanged.
- Around line 28-53: Verify the configuration source for browserSafeKeys and
confirm moesif_app_api_key is always the publishable Moesif Application Id,
never a secret or management key; if the contract is ambiguous, rename the
configuration key and all consumers to an explicit publishable-app-id name and
update the allowlist accordingly.
🪄 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
Run ID: 96946a97-3526-42d6-b36d-94287cf23f04
⛔ Files ignored due to path filters (1)
portals/ai-workspace/bff/go.sumis excluded by!**/*.sum
📒 Files selected for processing (30)
docs/ai-workspace/authentication/asgardeo-setup.mddocs/ai-workspace/authentication/oidc-auth.mddocs/ai-workspace/configuration.mddocs/ai-workspace/features/secrets-management.mdportals/ai-workspace/.env.exampleportals/ai-workspace/Dockerfileportals/ai-workspace/Makefileportals/ai-workspace/README.mdportals/ai-workspace/bff/go.modportals/ai-workspace/bff/internal/config/config.goportals/ai-workspace/bff/internal/config/config_test.goportals/ai-workspace/bff/internal/config/runtime_config.goportals/ai-workspace/bff/internal/config/settings.goportals/ai-workspace/bff/internal/config/shipped_config_test.goportals/ai-workspace/bff/internal/config/toml_mapping.goportals/ai-workspace/bff/internal/proxy/reverse_proxy_test.goportals/ai-workspace/bff/internal/proxy/transport.goportals/ai-workspace/bff/internal/server/composite_handlers_test.goportals/ai-workspace/bff/main.goportals/ai-workspace/configs/config-template.tomlportals/ai-workspace/configs/config.tomlportals/ai-workspace/docker-compose.yamlportals/ai-workspace/production/README.mdportals/ai-workspace/src/apis/platformApis.tsportals/ai-workspace/src/auth/logout.tsportals/ai-workspace/src/clients/choreoApiClient.tsportals/ai-workspace/src/config.env.tsportals/ai-workspace/src/contexts/ChoreoUserContext.tsxportals/ai-workspace/src/utils/logger.tsportals/ai-workspace/vite.config.ts
💤 Files with no reviewable changes (1)
- portals/ai-workspace/bff/internal/config/toml_mapping.go
- Refined the environment variable override mechanism in the configuration documentation to clarify the use of interpolation tokens for sensitive values. - Added BFF-side redirect URLs and client secret handling in the authentication setup documentation, ensuring clarity on their usage and security. - Improved the error messages in the configuration loading logic for better clarity on required fields and conditions. - Introduced a new TOML parser for the AI Workspace configuration, ensuring strict adherence to the expected format and enhancing error handling. - Added tests for the new TOML parser to validate its functionality and robustness against various input scenarios.
Dependency Validation ResultsDependency name: github.com/gorilla/websocket Dependency name: golang.org/x/crypto |
| # certificate is self-signed. | ||
| - APIP_AIW_PLATFORM_API_URL=https://platform-api:9243 | ||
| - APIP_AIW_PLATFORM_API_TLS_SKIP_VERIFY=true | ||
| - APIP_DEMO_MODE=${APIP_DEMO_MODE:-true} | ||
| # ── OIDC — to delegate login to any OIDC provider, set auth_mode = "oidc" and |
There was a problem hiding this comment.
all these configs from the dockerfile will be removed from the next effort and .env file mount will be used
AI Workspace BFF — Configuration Convention & the
/meEndpointThis note captures two related pieces of the AI Workspace BFF: the restructured
configuration model, and the
GET /mepermissions hop the BFF makes to thePlatform API. They are documented together because the
/mecall exists toresolve a gap that the configuration model deliberately refuses to paper over.
{{ env }}/{{ file }}interpolation in gatewayconfig.toml#25921. Configuration restructuring
1.1 One source of truth:
config.tomlThe BFF loads all configuration from a single
config.toml(
internal/config/settings.go,config.go). There is no implicit environmentoverlay on top of the file — a key takes its value from the environment or a
mounted secret only when its own interpolation token says so:
Consequences of this design:
than implied by a naming rule or a precedence ladder. If a key has no token, it
simply cannot be set from the environment.
secret is resolved the same way as
log_level— via a{{ env }}/{{ file }}token — so there is no separate secret-loading path.
{{ env }}variable with no default,or an unreadable/disallowed
{{ file }}path, aborts startup rather thansilently yielding an empty credential.
Tokens are resolved by the shared
common/configinterpolatelibrary.{{ file }}reads are constrained to an allowlist (
/etc/ai-workspace,/secrets/ai-workspaceby default, overridable via
APIP_CONFIG_FILE_SOURCE_ALLOWLIST).1.2 Naming convention:
APIP_AIW_The AI Workspace namespaces its environment variables with the
APIP_AIW_prefix — mirroring the Platform API's
APIP_CP_and the Developer Portal'sAPIP_DP_. The convention is: a key's variable is its dotted config path,uppercased, dots→underscores, prefixed.
Crucially this is a convention, not a binding — the prefix documents the
expected variable name, but the environment only reaches config through the
explicit
{{ env "NAME" }}token. A token may name any variable.1.3 Flattened, typed access
The decoded TOML tree is flattened to dotted keys (
[platform_api] url→platform_api.url). Values are read through small typed accessors that decidewhat a bad value should do:
get/getbool/getdur— a malformed boolean or duration failsstartup rather than silently reverting to the default (a typo shouldn't
quietly change behaviour).
getint64— used for defensive size ceilings (e.g.MaxMeResponseBytes); anunset, malformed, or non-positive value degrades to the safe default
instead of blocking startup, because a size bound is a guard rail, not a
correctness knob.
auth_modeis validated explicitly: anything other thanbasicoroidcfailsstartup, so a typo can't silently degrade to basic auth.
1.4 Browser-safe runtime config (allowlist, not filter)
The SPA reads a subset of config from
window.__RUNTIME_CONFIG__. The set ofkeys that reach the browser is an allowlist (
browserSafeKeysinruntime_config.go), not a filter:an upstream URL, a cookie setting — cannot leak into the browser merely by
being added to
config.toml.[oidc] client_secret/client_id/authorityare deliberately absent: theBFF performs the entire OIDC handshake, so the SPA needs no client identity.
the browser only ever talks to the BFF, never the Platform API directly.
The runtime key spelling (
runtimeKey) matches the{{ env }}token spelling, soa value has one name across
config.toml, the environment, Vite'simport.meta.env, andwindow.__RUNTIME_CONFIG__.1.5 Claim-mapping mirror
[oidc.claim_mappings]deliberately mirrors the Platform API's[auth.idp.claim_mappings]key for key. Both sides describe the same IDPtoken, so they must agree — naming them identically makes drift obvious. The same
config entry drives both the BFF's session mapping and the SPA's identity display,
keeping the two layers in sync from one source.
2. The
GET /meendpoint2.1 What it is
internal/server/permissions.gocalls the Platform API endpoint:It returns the signed-in user's effective identity and permissions — the
roles they hold and the scopes those roles grant:
The BFF issues this call on the user's behalf (forwarding their JWT) and folds the
result into the server-side session.
2.2 Why it matters — the problem it solves
The SPA gates every UI control on
hasPermission(scope). So the session mustcarry a scope list. But there is a mode where the token does not provide one:
The Platform API already knows how to expand roles into scopes — it does this via
roles.yamlfor its own authorization decisions.GET /mereturns that sameexpansion. This is the important part:
second copy of the role→scope mapping shipped to the browser.
Platform API, the workspace UI follows automatically — nothing to re-ship.
2.3 How it behaves (
enrichPermissions)/mecall. The enrichment only runs to filla gap the token left.
/melookup fails, the user is left withno scopes — the app hides privileged controls rather than offering actions
the API would then reject. It logs a warning and continues; it does not block
login.
platform_roleclaim, so thedisplayed role label falls back to the first IDP role from
/me.2.4 Defensive bound:
MaxMeResponseBytesThe
/mepayload is a small identity record, so the response read is capped(
platform_api.max_me_response_bytes, default 1 MiB) viaio.LimitReader.Anything larger is treated as an upstream fault, not something to buffer into
memory. Per §1.3, a non-positive or malformed configured limit falls back to the
safe default rather than failing startup.
3. How the two connect
/medoesgetint64degrades to safe defaultMaxMeResponseBytesThe through-line: a single authoritative source, resolved at the layer that
enforces it, never re-implemented in the browser. The configuration model keeps
authorization data off the client;
/mesupplies it on demand from the servicethat owns it.