nginx+auth 07 — downstream gateway-JWT verification (R1) in analytics + identity - #1777
Conversation
|
Important Review skippedToo many files! This PR contains 127 files, which is 27 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 20c73978ba5e6af2f6675d2af55cb6ea924d5d6a and 311010f. ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (164)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change enables downstream gateway-JWT verification for analytics and identity, replaces header-based tenant and caller authority with signed claims, updates authenticator claim and discovery contracts, adopts the published OIDC authentication plugin, and adds a Step-07 composed end-to-end verification suite. ChangesGateway JWT and authenticator contract
Analytics authentication
Identity authentication
Step-07 downstream verification
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
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: 6
🧹 Nitpick comments (8)
src/backend/services/identity/tests/Insight.Identity.Tests.Integration/PersonsEndpointTests.cs (1)
125-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicates the JWT-building helper in
TestApplicationFactory.BuildJwt.Noted for consolidation with
TestApplicationFactory.cs— see consolidated comment below.🤖 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 `@src/backend/services/identity/tests/Insight.Identity.Tests.Integration/PersonsEndpointTests.cs` around lines 125 - 138, Remove the duplicated BuildUnverifiedJwt helper and reuse TestApplicationFactory.BuildJwt for constructing the test JWT. Update the affected tests to call the shared helper while preserving the existing person and tenant claims and unverified-signature behavior.src/backend/libs/authverify/Cargo.toml (1)
14-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider moving
base64to the workspace dependency table.Every other dependency here uses
{ workspace = true };base64 = "0.22"is pinned locally instead, which can drift from any other crate'sbase64version over time.♻️ Proposed fix
-base64 = "0.22" +base64 = { workspace = true }And add
base64 = "0.22"under[workspace.dependencies]insrc/backend/Cargo.toml.🤖 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 `@src/backend/libs/authverify/Cargo.toml` around lines 14 - 26, Move the base64 dependency version declaration to the workspace dependency table in the backend Cargo.toml, then update the authverify crate’s dependency entry to use workspace = true like the other dependencies. Preserve the existing 0.22 version.src/backend/libs/authverify/src/map.rs (1)
136-159: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSelector matching is a raw string compare, not UUID-normalized.
resolve_tenantcomparesselectortotenants[]entries with==on the raw strings. A selector that's UUID-equivalent but differs in casing/format would be wrongly rejected (400/403) even though it's a valid pick. The siblingGatewayTenantContext.cs(identity) — which this crate's docs claim parity with — parses both sides toGuidfirst, avoiding this.♻️ Proposed fix
fn resolve_tenant(tenants: &[String], selector: Option<&str>) -> Result<Uuid, AuthVerifyError> { + let selector_uuid = selector.map(parse_tenant).transpose()?; match tenants { [] => Ok(Uuid::nil()), [only] => { - if let Some(sel) = selector - && sel != only - { - return Err(AuthVerifyError::TenantSelectorNotGranted); - } - parse_tenant(only) + let only_uuid = parse_tenant(only)?; + if let Some(sel) = selector_uuid + && sel != only_uuid + { + return Err(AuthVerifyError::TenantSelectorNotGranted); + } + Ok(only_uuid) } _ => { - let sel = selector.ok_or(AuthVerifyError::TenantSelectorMissing)?; - if !tenants.iter().any(|t| t == sel) { + let sel = selector_uuid.ok_or(AuthVerifyError::TenantSelectorMissing)?; + if !tenants.iter().any(|t| parse_tenant(t) == Ok(sel)) { return Err(AuthVerifyError::TenantSelectorNotGranted); } - parse_tenant(sel) + Ok(sel) } } }🤖 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 `@src/backend/libs/authverify/src/map.rs` around lines 136 - 159, Update resolve_tenant to compare tenant selectors by parsed UUID identity rather than raw string equality. In the single-tenant and multi-tenant branches, parse the selector and tenant entries with parse_tenant (or equivalent UUID parsing), accept UUID-equivalent casing/format variations, and preserve the existing TenantSelectorMissing and TenantSelectorNotGranted errors for invalid or non-member selectors.src/backend/services/identity/tests/Insight.Identity.Tests.Integration/TestApplicationFactory.cs (1)
112-128: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend
BuildJwtto support array-valued claims instead of duplicating it elsewhere.Noted for consolidation with
PersonsEndpointTests.BuildUnverifiedJwt— see consolidated comment below.🤖 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 `@src/backend/services/identity/tests/Insight.Identity.Tests.Integration/TestApplicationFactory.cs` around lines 112 - 128, Extend TestApplicationFactory.BuildJwt to accept and serialize array-valued claims in addition to string-valued claims, preserving the existing string-claim behavior and JWT encoding. Consolidate the shared array-claim construction needed by PersonsEndpointTests.BuildUnverifiedJwt into this helper instead of duplicating JWT-building logic elsewhere.src/backend/services/identity/src/Insight.Identity.Api/Endpoints/SubchartEndpoints.cs (1)
71-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame two
ProblemResponseshapes (tenant_unresolved,caller_unresolved) duplicated across 8 route handlers. Both files build the identical gateway-JWT error responses inline rather than via a shared helper (contrast withEndpointHelpers.GateResult, which already centralizes the admin-check errors). Extracting one or two helpers intoEndpointHelpers.cswould keep the four call sites from drifting on wording/status codes over time.
src/backend/services/identity/src/Insight.Identity.Api/Endpoints/SubchartEndpoints.cs#L71-L76: replace with a sharedTenantUnresolved()/CallerUnresolved()helper call.src/backend/services/identity/src/Insight.Identity.Api/Endpoints/SubchartEndpoints.cs#L82-L87: same replacement for the caller-unresolved shape.src/backend/services/identity/src/Insight.Identity.Api/Endpoints/SubchartEndpoints.cs#L129-L134: same replacement for the second tenant-unresolved instance.src/backend/services/identity/src/Insight.Identity.Api/Endpoints/SubchartEndpoints.cs#L140-L145: same replacement for the second caller-unresolved instance.src/backend/services/identity/src/Insight.Identity.Api/Endpoints/PersonsEndpoints.cs#L45-L46: replace with the shared tenant-unresolved helper.src/backend/services/identity/src/Insight.Identity.Api/Endpoints/PersonsEndpoints.cs#L56-L57: replace with the shared caller-unresolved helper.src/backend/services/identity/src/Insight.Identity.Api/Endpoints/PersonsEndpoints.cs#L101-L102: replace with the shared tenant-unresolved helper.src/backend/services/identity/src/Insight.Identity.Api/Endpoints/PersonsEndpoints.cs#L112-L113: replace with the shared caller-unresolved helper.🤖 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 `@src/backend/services/identity/src/Insight.Identity.Api/Endpoints/SubchartEndpoints.cs` around lines 71 - 76, Duplicate gateway-JWT ProblemResponse construction is spread across eight handlers; centralize the tenant-unresolved and caller-unresolved shapes in EndpointHelpers alongside GateResult. Add TenantUnresolved() and CallerUnresolved() helpers preserving the existing wording, status, and selector details, then replace the inline responses at src/backend/services/identity/src/Insight.Identity.Api/Endpoints/SubchartEndpoints.cs lines 71-76, 82-87, 129-134, and 140-145, and at src/backend/services/identity/src/Insight.Identity.Api/Endpoints/PersonsEndpoints.cs lines 45-46, 56-57, 101-102, and 112-113 with the corresponding helper calls.src/backend/services/identity/tests/Insight.Identity.Tests.Unit/GatewayTenantContextTests.cs (1)
15-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReturn the concrete
DefaultHttpContextto avoid interface dispatch (CA1859).Both test utility methods instantiate a
DefaultHttpContextbut return it as anHttpContextinterface. Returning the concrete type improves performance by avoiding interface dispatch, addressing the CA1859 static analysis hint.
src/backend/services/identity/tests/Insight.Identity.Tests.Unit/GatewayTenantContextTests.cs#L15-L27: Change the return type ofContextfromHttpContexttoDefaultHttpContext.src/backend/services/identity/tests/Insight.Identity.Tests.Unit/SubjectCallerContextTests.cs#L13-L20: Change the return type ofWithSubfromHttpContexttoDefaultHttpContext.🤖 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 `@src/backend/services/identity/tests/Insight.Identity.Tests.Unit/GatewayTenantContextTests.cs` around lines 15 - 27, Update the Context helper in src/backend/services/identity/tests/Insight.Identity.Tests.Unit/GatewayTenantContextTests.cs:15-27 and the WithSub helper in src/backend/services/identity/tests/Insight.Identity.Tests.Unit/SubjectCallerContextTests.cs:13-20 to return DefaultHttpContext instead of HttpContext, preserving their existing construction and behavior.Source: Linters/SAST tools
src/backend/services/gateway/tests/step07/README.md (2)
42-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSpecify a language for the fenced code block.
Static analysis tools flag this block because it lacks a specified language. Using
bashorshellresolves the warning.♻️ Proposed fix
-``` +```bash pip install pytest pyjwt cryptography🤖 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 `@src/backend/services/gateway/tests/step07/README.md` around lines 42 - 43, Update the fenced code block containing the pip install command in the README to declare bash or shell as its language, preserving the command unchanged.Source: Linters/SAST tools
13-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSpecify a language for the fenced code block.
Static analysis tools flag this block because it lacks a specified language. Using
textresolves the warning.♻️ Proposed fix
-``` +```text fakeidp ─▶ authenticator ─▶ gateway ─▶ {analytics (Rust), identity (.NET)}🤖 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 `@src/backend/services/gateway/tests/step07/README.md` around lines 13 - 14, Update the fenced code block in the README diagram to declare the text language, preserving the diagram content unchanged.Source: Linters/SAST tools
🤖 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 `@src/backend/libs/authverify/src/middleware.rs`:
- Around line 72-85: Update bearer_token to recognize the Bearer authentication
scheme case-insensitively for every casing, while preserving the existing
whitespace trimming and empty-token rejection behavior.
- Around line 28-47: Update bearer_token to normalize the RFC 7235 Bearer scheme
case-insensitively before trimming and parsing the token, so inputs such as
“BEARER <jwt>” are accepted alongside existing casings while preserving the
current validation behavior.
In `@src/backend/services/analytics/src/infra/identity/mod.rs`:
- Around line 73-75: Update the URL construction in the identity request flow
around the `url` and `self.http.get` calls to use `reqwest::Url` and append
`v1`, `persons`, and the email as separate path segments. Avoid interpolating
`email` into a formatted URL so reserved characters are percent-encoded while
preserving the existing GET request behavior.
In `@src/backend/services/gateway/tests/step07/conftest.py`:
- Around line 29-31: Add the missing AUTHZ_CACHE_MAX_AGE export in the step07
test configuration module, matching the authenticator’s configured cache
duration of 3 seconds in docker-compose.step07.e2e.yml so test_gateway.py can
import it.
In `@src/backend/services/identity/helm/templates/deployment.yaml`:
- Around line 93-100: Update the gateway environment-variable block in the
deployment template to guard IDENTITY__identity__auth_gateway_issuer and
IDENTITY__identity__auth_gateway_jwks_url independently rather than with a
combined or condition. Render each entry only when its corresponding
.Values.gateway.issuer or .Values.gateway.jwksUrl is set, preserving
existingSecret/envFrom values for unset keys.
In `@src/backend/services/identity/src/Insight.Identity.Api/Program.cs`:
- Around line 145-170: Update the JWT TokenValidationParameters in AddJwtBearer
to assign jwksConfigManager through ConfigurationManager and remove the
IssuerSigningKeyResolver callback. Preserve the existing issuer, audience,
lifetime, signing-key, and algorithm validation settings while allowing the
framework to manage JWKS retrieval without synchronous blocking.
---
Nitpick comments:
In `@src/backend/libs/authverify/Cargo.toml`:
- Around line 14-26: Move the base64 dependency version declaration to the
workspace dependency table in the backend Cargo.toml, then update the authverify
crate’s dependency entry to use workspace = true like the other dependencies.
Preserve the existing 0.22 version.
In `@src/backend/libs/authverify/src/map.rs`:
- Around line 136-159: Update resolve_tenant to compare tenant selectors by
parsed UUID identity rather than raw string equality. In the single-tenant and
multi-tenant branches, parse the selector and tenant entries with parse_tenant
(or equivalent UUID parsing), accept UUID-equivalent casing/format variations,
and preserve the existing TenantSelectorMissing and TenantSelectorNotGranted
errors for invalid or non-member selectors.
In `@src/backend/services/gateway/tests/step07/README.md`:
- Around line 42-43: Update the fenced code block containing the pip install
command in the README to declare bash or shell as its language, preserving the
command unchanged.
- Around line 13-14: Update the fenced code block in the README diagram to
declare the text language, preserving the diagram content unchanged.
In
`@src/backend/services/identity/src/Insight.Identity.Api/Endpoints/SubchartEndpoints.cs`:
- Around line 71-76: Duplicate gateway-JWT ProblemResponse construction is
spread across eight handlers; centralize the tenant-unresolved and
caller-unresolved shapes in EndpointHelpers alongside GateResult. Add
TenantUnresolved() and CallerUnresolved() helpers preserving the existing
wording, status, and selector details, then replace the inline responses at
src/backend/services/identity/src/Insight.Identity.Api/Endpoints/SubchartEndpoints.cs
lines 71-76, 82-87, 129-134, and 140-145, and at
src/backend/services/identity/src/Insight.Identity.Api/Endpoints/PersonsEndpoints.cs
lines 45-46, 56-57, 101-102, and 112-113 with the corresponding helper calls.
In
`@src/backend/services/identity/tests/Insight.Identity.Tests.Integration/PersonsEndpointTests.cs`:
- Around line 125-138: Remove the duplicated BuildUnverifiedJwt helper and reuse
TestApplicationFactory.BuildJwt for constructing the test JWT. Update the
affected tests to call the shared helper while preserving the existing person
and tenant claims and unverified-signature behavior.
In
`@src/backend/services/identity/tests/Insight.Identity.Tests.Integration/TestApplicationFactory.cs`:
- Around line 112-128: Extend TestApplicationFactory.BuildJwt to accept and
serialize array-valued claims in addition to string-valued claims, preserving
the existing string-claim behavior and JWT encoding. Consolidate the shared
array-claim construction needed by PersonsEndpointTests.BuildUnverifiedJwt into
this helper instead of duplicating JWT-building logic elsewhere.
In
`@src/backend/services/identity/tests/Insight.Identity.Tests.Unit/GatewayTenantContextTests.cs`:
- Around line 15-27: Update the Context helper in
src/backend/services/identity/tests/Insight.Identity.Tests.Unit/GatewayTenantContextTests.cs:15-27
and the WithSub helper in
src/backend/services/identity/tests/Insight.Identity.Tests.Unit/SubjectCallerContextTests.cs:13-20
to return DefaultHttpContext instead of HttpContext, preserving their existing
construction and behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f4a3df6a-1449-4d2f-85b5-3c582eeb49ae
📥 Commits
Reviewing files that changed from the base of the PR and between 604b69b and 4de452e243ba5e47e71387b93497a178e68db110.
⛔ Files ignored due to path filters (1)
src/backend/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (51)
docker-compose.ymldocs/components/backend/gateway/DESIGN.mdsrc/backend/Cargo.tomlsrc/backend/libs/authverify/Cargo.tomlsrc/backend/libs/authverify/src/lib.rssrc/backend/libs/authverify/src/map.rssrc/backend/libs/authverify/src/middleware.rssrc/backend/services/analytics/Cargo.tomlsrc/backend/services/analytics/config/insight.yamlsrc/backend/services/analytics/helm/templates/configmap.yamlsrc/backend/services/analytics/helm/templates/deployment.yamlsrc/backend/services/analytics/helm/values.yamlsrc/backend/services/analytics/src/api/handlers.rssrc/backend/services/analytics/src/api/http_live_tests.rssrc/backend/services/analytics/src/api/mod.rssrc/backend/services/analytics/src/api/tenant_resolution_tests.rssrc/backend/services/analytics/src/auth.rssrc/backend/services/analytics/src/domain/auth.rssrc/backend/services/analytics/src/infra/identity/mod.rssrc/backend/services/analytics/src/main.rssrc/backend/services/gateway/tests/step07/README.mdsrc/backend/services/gateway/tests/step07/conftest.pysrc/backend/services/gateway/tests/step07/docker-compose.step07.e2e.ymlsrc/backend/services/gateway/tests/step07/pytest.inisrc/backend/services/gateway/tests/step07/routes.step07.e2e.yamlsrc/backend/services/gateway/tests/step07/run-e2e.shsrc/backend/services/gateway/tests/step07/test_downstream.pysrc/backend/services/identity/helm/templates/deployment.yamlsrc/backend/services/identity/helm/values.yamlsrc/backend/services/identity/src/Insight.Identity.Api/Auth/GatewayTenantContext.cssrc/backend/services/identity/src/Insight.Identity.Api/Auth/HeaderCallerContext.cssrc/backend/services/identity/src/Insight.Identity.Api/Auth/HeaderTenantContext.cssrc/backend/services/identity/src/Insight.Identity.Api/Auth/JwksRetriever.cssrc/backend/services/identity/src/Insight.Identity.Api/Auth/JwtTenantContext.cssrc/backend/services/identity/src/Insight.Identity.Api/Auth/SubjectCallerContext.cssrc/backend/services/identity/src/Insight.Identity.Api/Auth/TenantSelectorException.cssrc/backend/services/identity/src/Insight.Identity.Api/Configuration/AppOptions.cssrc/backend/services/identity/src/Insight.Identity.Api/Endpoints/EndpointHelpers.cssrc/backend/services/identity/src/Insight.Identity.Api/Endpoints/PersonsEndpoints.cssrc/backend/services/identity/src/Insight.Identity.Api/Endpoints/SubchartEndpoints.cssrc/backend/services/identity/src/Insight.Identity.Api/Program.cssrc/backend/services/identity/src/Insight.Identity.Api/appsettings.yamlsrc/backend/services/identity/tests/Insight.Identity.Tests.Integration/JwtCallerResolveTests.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Integration/PersonsEndpointTests.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Integration/ProfilesEndpointTests.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Integration/TestApplicationFactory.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Unit/GatewayTenantContextTests.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Unit/HeaderCallerContextTests.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Unit/HeaderTenantContextTests.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Unit/JwtTenantContextTests.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Unit/SubjectCallerContextTests.cs
💤 Files with no reviewable changes (10)
- src/backend/services/identity/tests/Insight.Identity.Tests.Unit/HeaderTenantContextTests.cs
- src/backend/services/analytics/src/api/tenant_resolution_tests.rs
- src/backend/services/identity/src/Insight.Identity.Api/Auth/JwtTenantContext.cs
- src/backend/services/identity/src/Insight.Identity.Api/Auth/HeaderTenantContext.cs
- src/backend/services/identity/tests/Insight.Identity.Tests.Unit/HeaderCallerContextTests.cs
- src/backend/services/analytics/src/auth.rs
- src/backend/services/identity/tests/Insight.Identity.Tests.Integration/ProfilesEndpointTests.cs
- src/backend/services/identity/src/Insight.Identity.Api/Auth/HeaderCallerContext.cs
- src/backend/services/identity/tests/Insight.Identity.Tests.Unit/JwtTenantContextTests.cs
- src/backend/services/identity/tests/Insight.Identity.Tests.Integration/JwtCallerResolveTests.cs
e51fe9d to
9eb5c46
Compare
|
Updated: the gateway JWT now uses RS256 (was ES256). Rationale — the downstream verifier's JWKS parser ( EC/EdDSA are the better long-term choice (≈64-byte signatures + faster verify vs RS256's ≈256 bytes — the JWT rides in a header on every downstream request), but need broader Verified locally end-to-end via docker: the step-07 e2e (5 §D scenarios) and the step-05 gateway e2e both pass with RS256. |
9eb5c46 to
babcf46
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/backend/services/gateway/tests/step07/README.md (2)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSpecify a language for the fenced code block.
Adding a language identifier (like
text) to fenced code blocks resolves markdown linting warnings.♻️ Proposed refactor
-``` +```text🤖 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 `@src/backend/services/gateway/tests/step07/README.md` at line 13, Update the fenced code block in the step07 README to include an explicit language identifier, using text for this non-code content, while preserving the block’s existing contents.Source: Linters/SAST tools
42-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSpecify a language for the fenced code block.
Adding a language identifier (like
bash) to fenced code blocks improves readability with proper syntax highlighting and resolves markdown linting warnings.♻️ Proposed refactor
-``` +```bash🤖 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 `@src/backend/services/gateway/tests/step07/README.md` at line 42, Update the fenced code block in the step07 README to include an appropriate language identifier, such as bash, immediately after the opening fence while preserving the block’s contents.Source: Linters/SAST tools
src/backend/services/analytics/Cargo.toml (1)
42-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
workspace = trueforoidc-authn-plugin.src/backend/Cargo.tomlalready listsplugins/oidc-authn-pluginas a workspace member, so this can match the other dependency entries here and avoid the relative path.🤖 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 `@src/backend/services/analytics/Cargo.toml` at line 42, Update the oidc-authn-plugin dependency entry in the analytics service manifest to use the workspace dependency declaration instead of the relative path, matching the existing workspace dependency entries and the member already defined in src/backend/Cargo.toml.
🤖 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.
Nitpick comments:
In `@src/backend/services/analytics/Cargo.toml`:
- Line 42: Update the oidc-authn-plugin dependency entry in the analytics
service manifest to use the workspace dependency declaration instead of the
relative path, matching the existing workspace dependency entries and the member
already defined in src/backend/Cargo.toml.
In `@src/backend/services/gateway/tests/step07/README.md`:
- Line 13: Update the fenced code block in the step07 README to include an
explicit language identifier, using text for this non-code content, while
preserving the block’s existing contents.
- Line 42: Update the fenced code block in the step07 README to include an
appropriate language identifier, such as bash, immediately after the opening
fence while preserving the block’s contents.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fca4d5ef-f7df-4d2e-8c25-7bf63b1641a6
📥 Commits
Reviewing files that changed from the base of the PR and between 4de452e243ba5e47e71387b93497a178e68db110 and babcf466eabc9e45c3f213678d70961af109ec33.
⛔ Files ignored due to path filters (1)
src/backend/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (68)
dev-compose.shdocker-compose.ymldocs/components/backend/authenticator/DESIGN.mddocs/components/backend/gateway/DESIGN.mdsrc/backend/Cargo.tomlsrc/backend/libs/authverify/Cargo.tomlsrc/backend/libs/authverify/src/lib.rssrc/backend/libs/authverify/src/map.rssrc/backend/libs/authverify/src/middleware.rssrc/backend/services/analytics/Cargo.tomlsrc/backend/services/analytics/Dockerfilesrc/backend/services/analytics/config/insight.yamlsrc/backend/services/analytics/helm/templates/configmap.yamlsrc/backend/services/analytics/helm/templates/deployment.yamlsrc/backend/services/analytics/helm/values.yamlsrc/backend/services/analytics/src/api/handlers.rssrc/backend/services/analytics/src/api/http_live_tests.rssrc/backend/services/analytics/src/api/mod.rssrc/backend/services/analytics/src/api/tenant_resolution_tests.rssrc/backend/services/analytics/src/auth.rssrc/backend/services/analytics/src/domain/auth.rssrc/backend/services/analytics/src/infra/identity/mod.rssrc/backend/services/analytics/src/main.rssrc/backend/services/api-gateway/Dockerfilesrc/backend/services/authenticator/Cargo.tomlsrc/backend/services/authenticator/Dockerfilesrc/backend/services/authenticator/helm/templates/deployment.yamlsrc/backend/services/authenticator/helm/values.yamlsrc/backend/services/authenticator/src/config.rssrc/backend/services/authenticator/src/gear.rssrc/backend/services/authenticator/src/jwt.rssrc/backend/services/authenticator/tests/e2e_login_loop.rssrc/backend/services/authenticator/tests/run-e2e.shsrc/backend/services/fakeidp/Dockerfilesrc/backend/services/gateway/tests/conftest.pysrc/backend/services/gateway/tests/pytest.inisrc/backend/services/gateway/tests/step07/README.mdsrc/backend/services/gateway/tests/step07/conftest.pysrc/backend/services/gateway/tests/step07/docker-compose.step07.e2e.ymlsrc/backend/services/gateway/tests/step07/pytest.inisrc/backend/services/gateway/tests/step07/routes.step07.e2e.yamlsrc/backend/services/gateway/tests/step07/run-e2e.shsrc/backend/services/gateway/tests/step07/test_downstream.pysrc/backend/services/gateway/tests/test_gateway.pysrc/backend/services/identity/helm/templates/deployment.yamlsrc/backend/services/identity/helm/values.yamlsrc/backend/services/identity/src/Insight.Identity.Api/Auth/GatewayTenantContext.cssrc/backend/services/identity/src/Insight.Identity.Api/Auth/HeaderCallerContext.cssrc/backend/services/identity/src/Insight.Identity.Api/Auth/HeaderTenantContext.cssrc/backend/services/identity/src/Insight.Identity.Api/Auth/JwksRetriever.cssrc/backend/services/identity/src/Insight.Identity.Api/Auth/JwtTenantContext.cssrc/backend/services/identity/src/Insight.Identity.Api/Auth/SubjectCallerContext.cssrc/backend/services/identity/src/Insight.Identity.Api/Auth/TenantSelectorException.cssrc/backend/services/identity/src/Insight.Identity.Api/Configuration/AppOptions.cssrc/backend/services/identity/src/Insight.Identity.Api/Endpoints/EndpointHelpers.cssrc/backend/services/identity/src/Insight.Identity.Api/Endpoints/PersonsEndpoints.cssrc/backend/services/identity/src/Insight.Identity.Api/Endpoints/SubchartEndpoints.cssrc/backend/services/identity/src/Insight.Identity.Api/Program.cssrc/backend/services/identity/src/Insight.Identity.Api/appsettings.yamlsrc/backend/services/identity/tests/Insight.Identity.Tests.Integration/JwtCallerResolveTests.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Integration/PersonsEndpointTests.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Integration/ProfilesEndpointTests.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Integration/TestApplicationFactory.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Unit/GatewayTenantContextTests.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Unit/HeaderCallerContextTests.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Unit/HeaderTenantContextTests.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Unit/JwtTenantContextTests.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Unit/SubjectCallerContextTests.cs
💤 Files with no reviewable changes (10)
- src/backend/services/identity/tests/Insight.Identity.Tests.Unit/HeaderTenantContextTests.cs
- src/backend/services/identity/tests/Insight.Identity.Tests.Unit/HeaderCallerContextTests.cs
- src/backend/services/identity/src/Insight.Identity.Api/Auth/HeaderTenantContext.cs
- src/backend/services/identity/tests/Insight.Identity.Tests.Unit/JwtTenantContextTests.cs
- src/backend/services/identity/src/Insight.Identity.Api/Auth/JwtTenantContext.cs
- src/backend/services/analytics/src/api/tenant_resolution_tests.rs
- src/backend/services/identity/tests/Insight.Identity.Tests.Integration/ProfilesEndpointTests.cs
- src/backend/services/identity/src/Insight.Identity.Api/Auth/HeaderCallerContext.cs
- src/backend/services/analytics/src/auth.rs
- src/backend/services/identity/tests/Insight.Identity.Tests.Integration/JwtCallerResolveTests.cs
🚧 Files skipped from review as they are similar to previous changes (32)
- src/backend/services/gateway/tests/step07/pytest.ini
- src/backend/services/identity/tests/Insight.Identity.Tests.Unit/SubjectCallerContextTests.cs
- src/backend/services/gateway/tests/step07/routes.step07.e2e.yaml
- src/backend/services/identity/helm/templates/deployment.yaml
- src/backend/libs/authverify/src/lib.rs
- src/backend/services/identity/src/Insight.Identity.Api/appsettings.yaml
- src/backend/services/identity/src/Insight.Identity.Api/Auth/TenantSelectorException.cs
- src/backend/services/gateway/tests/step07/run-e2e.sh
- src/backend/libs/authverify/Cargo.toml
- src/backend/services/analytics/helm/templates/deployment.yaml
- src/backend/services/identity/src/Insight.Identity.Api/Auth/JwksRetriever.cs
- src/backend/services/identity/helm/values.yaml
- src/backend/services/identity/src/Insight.Identity.Api/Auth/SubjectCallerContext.cs
- src/backend/services/analytics/helm/templates/configmap.yaml
- src/backend/services/analytics/src/infra/identity/mod.rs
- docker-compose.yml
- src/backend/Cargo.toml
- src/backend/services/analytics/src/domain/auth.rs
- src/backend/services/identity/tests/Insight.Identity.Tests.Unit/GatewayTenantContextTests.cs
- src/backend/services/analytics/helm/values.yaml
- src/backend/services/identity/src/Insight.Identity.Api/Program.cs
- src/backend/services/identity/src/Insight.Identity.Api/Auth/GatewayTenantContext.cs
- src/backend/services/analytics/src/api/handlers.rs
- src/backend/services/analytics/config/insight.yaml
- src/backend/services/identity/src/Insight.Identity.Api/Endpoints/PersonsEndpoints.cs
- src/backend/services/identity/src/Insight.Identity.Api/Configuration/AppOptions.cs
- src/backend/services/analytics/src/api/mod.rs
- src/backend/libs/authverify/src/middleware.rs
- src/backend/services/analytics/src/main.rs
- src/backend/libs/authverify/src/map.rs
- src/backend/services/identity/tests/Insight.Identity.Tests.Integration/PersonsEndpointTests.cs
- src/backend/services/gateway/tests/step07/test_downstream.py
Reworked per review — plugin-native verification, ES256, single tenantPushed
New surface: the plugin resolves JWKS via OIDC discovery (https-only), so the authenticator now serves step07 e2e: 6/6 green in docker — login→200, analytics+identity direct/no-JWT→401, valid-signature-missing-
|
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 `@src/backend/services/authenticator/src/service_token.rs`:
- Around line 234-244: Update token_handler to reject requests when
requested_tenants.len() > 1, returning the existing appropriate validation/error
response before calling build_service_claims. Preserve single-tenant handling
and avoid allowing build_service_claims to silently select only the first
tenant.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4fc857c3-29e5-4532-a384-c1983befa094
📥 Commits
Reviewing files that changed from the base of the PR and between babcf466eabc9e45c3f213678d70961af109ec33 and 20c73978ba5e6af2f6675d2af55cb6ea924d5d6a.
⛔ Files ignored due to path filters (1)
src/backend/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (41)
dev-compose.shdocker-compose.ymldocs/components/backend/authenticator/DESIGN.mddocs/components/backend/gateway/DESIGN.mdsrc/backend/Cargo.tomlsrc/backend/plugins/oidc-authn-plugin/Cargo.tomlsrc/backend/plugins/oidc-authn-plugin/README.mdsrc/backend/plugins/oidc-authn-plugin/src/config.rssrc/backend/plugins/oidc-authn-plugin/src/domain/client.rssrc/backend/plugins/oidc-authn-plugin/src/domain/mod.rssrc/backend/plugins/oidc-authn-plugin/src/domain/service.rssrc/backend/plugins/oidc-authn-plugin/src/lib.rssrc/backend/plugins/oidc-authn-plugin/src/module.rssrc/backend/services/analytics/Cargo.tomlsrc/backend/services/analytics/Dockerfilesrc/backend/services/analytics/config/insight.yamlsrc/backend/services/analytics/helm/templates/configmap.yamlsrc/backend/services/analytics/helm/templates/deployment.yamlsrc/backend/services/analytics/helm/values.yamlsrc/backend/services/analytics/src/api/mod.rssrc/backend/services/api-gateway/Cargo.tomlsrc/backend/services/api-gateway/Dockerfilesrc/backend/services/authenticator/Cargo.tomlsrc/backend/services/authenticator/Dockerfilesrc/backend/services/authenticator/src/api/handlers.rssrc/backend/services/authenticator/src/api/mod.rssrc/backend/services/authenticator/src/jwt.rssrc/backend/services/authenticator/src/service_token.rssrc/backend/services/authenticator/tests/e2e_login_loop.rssrc/backend/services/authenticator/tests/run-e2e.shsrc/backend/services/fakeidp/Dockerfilesrc/backend/services/gateway/tests/step07/analytics.step07.yamlsrc/backend/services/gateway/tests/step07/authn-tls.confsrc/backend/services/gateway/tests/step07/conftest.pysrc/backend/services/gateway/tests/step07/docker-compose.step07.e2e.ymlsrc/backend/services/gateway/tests/step07/test_downstream.pysrc/backend/services/identity/src/Insight.Identity.Api/Auth/GatewayTenantContext.cssrc/backend/services/identity/src/Insight.Identity.Api/Endpoints/PersonsEndpoints.cssrc/backend/services/identity/src/Insight.Identity.Api/Endpoints/SubchartEndpoints.cssrc/backend/services/identity/src/Insight.Identity.Api/Program.cssrc/backend/services/identity/tests/Insight.Identity.Tests.Unit/GatewayTenantContextTests.cs
💤 Files with no reviewable changes (10)
- src/backend/plugins/oidc-authn-plugin/src/lib.rs
- src/backend/plugins/oidc-authn-plugin/Cargo.toml
- src/backend/plugins/oidc-authn-plugin/src/module.rs
- src/backend/plugins/oidc-authn-plugin/src/domain/mod.rs
- src/backend/plugins/oidc-authn-plugin/src/domain/service.rs
- src/backend/plugins/oidc-authn-plugin/README.md
- src/backend/plugins/oidc-authn-plugin/src/domain/client.rs
- src/backend/plugins/oidc-authn-plugin/src/config.rs
- src/backend/services/authenticator/Dockerfile
- src/backend/services/fakeidp/Dockerfile
🚧 Files skipped from review as they are similar to previous changes (4)
- src/backend/services/identity/src/Insight.Identity.Api/Endpoints/PersonsEndpoints.cs
- src/backend/services/analytics/helm/values.yaml
- src/backend/services/gateway/tests/step07/docker-compose.step07.e2e.yml
- src/backend/services/identity/src/Insight.Identity.Api/Program.cs
20c7397 to
a909b2f
Compare
a909b2f to
dbc8dc1
Compare
1a4f7fc to
5d80844
Compare
dda510c to
3236e66
Compare
3236e66 to
4658410
Compare
4658410 to
be16e99
Compare
be16e99 to
46aa803
Compare
46aa803 to
b06deb3
Compare
b06deb3 to
72d0562
Compare
72d0562 to
6ae1f04
Compare
6ae1f04 to
6c46996
Compare
…ric#1583) — verification, dev-compose + k8s, drop legacy api-gateway Squashed delivery of the NGINX_BFF work on this branch. Every request now carries an ES256 gateway JWT and every downstream verifies it — no auth_disabled anywhere. Step 07 — downstream verification (R1): - analytics verifies the gateway JWT via the published cf-gears-oidc-authn-plugin (ES256, single tenant_id, claim_mapping); identity (.NET) via JwtBearer (ES256, plain-http JWKS). Deleted the insight fork plugin + the authverify wrapper + the X-Tenant-ID selector; CI matrix greened. identity-resolution integrates the same downstream gateway-JWT auth. Full-auth dev-compose: - nginx `gateway` is the sole :8080 edge (auth_request -> authenticator, JWT injection); authn-tls sidecar fronts the authenticator discovery/JWKS over https for the plugin; login runs against fakeidp; the no-auth config is retired. - gateway boots without a frontend (lazy SPA upstream); login-time person resolution via a service-only Identity endpoint; build accepts multiple targets. Kubernetes (umbrella chart 0.4.0, BREAKING): - add gateway / authenticator / fakeidp subcharts; drop apiGateway; remove all authDisabled validators; frontend made conditional. - authenticator authn-tls sidecar (cert-manager local-ca) + analytics CA mount; gateway ingress + strip_prefix; one canonical issuer everywhere. - gitops: local + functional-ci overlays; compose-app-secrets.sh composes the authenticator config + identity gateway-auth env; sealed ES256 signing key. Legacy api-gateway removed from compose, the umbrella chart, and the repo (crate, Dockerfile, helm chart, workspace member, CI jobs). cf-gears-api-gateway (the toolkit REST-host library) and oidc-authn-plugin are retained. Local-k8s IdP decision (ADR-0001): fakeidp for all dev/test envs (CI, compose, local k8s — exposed via an ingress `/idp` rewrite with a localhost callback for the __Host-sid secure-context rule; it injects tenant_id, zero extra infra). A production broker is deferred (authenticator IdP-agnostic; Casdoor/Keycloak noted as candidates); Dex and heavy-DB brokers evaluated and rejected. Verified on OrbStack (curl + real-browser Playwright): login -> session -> analytics + identity both 200, 401 without. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech> Addressed PR constructorfabric#1777 review feedback: - gateway routing: prefix + service + service-url (no double v1) — `/api/analytics/v1/…`, `/api/identity/v1/…` (strip_prefix kept). - analytics → identity: migrated off the DEPRECATED `GET /v1/persons/{email}` to `POST /v1/profiles` (maps ProfileResponse → the existing Person model; analytics' own contract unchanged). - authenticator: reject service-token requests naming >1 tenant. - identity Helm: per-value `with` guards so an unset chart value can't clobber the secret-supplied gateway env. - ES256 (not RS256) corrected in analytics comments + the authenticator Helm chart signing-key docs. - frontend re-enabled (deploy=true) in CI + local overlays (image pulled from ghcr). - renamed the `step07` e2e suite → `downstream-verify`; exported AUTHZ_CACHE_MAX_AGE in its conftest. - docs: deleted the stale LOCAL_DEV.md; refreshed CONTRIBUTING.md to the nginx-gateway + fakeidp full-auth reality; trimmed noisy config comments. Deferred (spec-governance, not gateway routing): the `/api/v1/<svc>` convention in the cfs-governed backend spec docs across all 8 services + the versioning NFR. authenticator /auth/me: include the person's `email` (stored in the session at login from the id_token) alongside user/tenants/roles, so the email-keyed SPA can self-locate under the cookie/BFF model. compose: default AUTHENTICATOR_REDIRECT_URI to the SPA's browser origin (http://localhost:3000/auth/callback) so the __Host-sid cookie lands where the SPA runs; document the browser-login prerequisites (redirect origin + a `127.0.0.1 fakeidp` hosts entry so the browser resolves the fakeidp issuer). gitignore: ignore the whole dev-compose key/cert dirs (authenticator-dev-keys/, authn-tls-certs/) rather than just *.pem, so a stray non-.pem key generated at runtime can't be swept in by `git add -A`. dev-compose: browser OIDC works from scratch. `up` auto-detects the host IP and defaults FAKEIDP_ISSUER + AUTHENTICATOR_OIDC_ISSUER to http://<host-ip>:8084 — an IP literal the browser reaches without an HTTPS-upgrade (a hostname is upgraded and fails against http-only fakeidp; localhost is the container itself) and the containers reach too. Skipped when an issuer is pinned (real IdP) or no host IP is found (offline → falls back to fakeidp:8084 for the curl/e2e path). fix(dev-compose): analytics home_dir out of /app so auto-reload doesn't crash-loop. Compose watchexec watches /app (the binary's dir); with home_dir=/app/data the gear's own startup writes tripped a restart mid-migration → concurrent migrators → Duplicate column/key → init crash-loop (and a log flood). Move it to /tmp. fix(gateway): frontUrl must be a FQDN. routegen emits the SPA proxy_pass through nginx's runtime resolver, which ignores /etc/resolv.conf search domains, so the short name `insight-frontend` failed to resolve (502 on /). Use the release/namespace/clusterDomain FQDN like authenticatorUrl. Signed-off-by: Anton Zelenov <antonz@constructor.tech> k8s local: add an insight-keycloak subchart (real-OIDC dev IdP), gated by keycloak.deploy; the local overlay flips keycloak on + fakeidp off and points authenticator.oidc at the pre-seeded insight-authenticator confidential client. `make deploy ENV=local` gains a keycloak-realm step that generates the realm (gen-realm.py) + applies it as the insight-keycloak-realm ConfigMap. Signed-off-by: Anton Zelenov <antonz@constructor.tech> authenticator: drop offline_access from the default oidc_scopes (it requests an offline/survives-logout refresh token — wrong for a session-bound BFF, and it demanded the offline_access role on Keycloak). The auth-code flow still yields a session-bound refresh token; add offline_access via the `oidc_scopes` setting only for an IdP that refuses one otherwise (e.g. Entra). Also surface the actual IdP failure in logs: unwrap RequestTokenError::ServerResponse (error + error_description) and log the full anyhow chain, instead of a bare "code exchange". Signed-off-by: Anton Zelenov <antonz@constructor.tech>
…enant One and only one tenant per token (EPIC constructorfabric#1583), everywhere: - fakeidp + Keycloak realm emit a single-string `tenant_id` claim (the redundant `tenants` array mapper is gone; carol keeps one tenant). - authenticator: IdpIdentity/PersonResolution/SessionRecord and /auth/me carry `tenant_id: String` (no Vec, no .first() picks). - idp.tenant_claim names the claim per IdP (default `tenant_id`; Entra: `tid`; an array value is tolerated — first entry wins). New idp.default_tenant_id covers claim-less IdPs (e.g. Okta) until the Identity membership API (constructorfabric#1687) / Keycloak broker (constructorfabric#1782); empty keeps fail-closed. - service tokens: the /internal/token form field is `tenant_id` (single; a comma-joined list is rejected), SDK bearer()/fetch()/post() take one tenant id. - gitops composer plumbs authenticator.oidc.tenantClaim / defaultTenantId. Signed-off-by: Anton Zelenov <antonz@constructor.tech>
…idc Secret Confidential-client secret (Entra/BFF) comes from the cluster Secret insight-oidc (key oidc-client-secret, Passbolt-sealed); values.yaml stays a fallback for non-sensitive dev IdP secrets (Keycloak baked client). Signed-off-by: Anton Zelenov <antonz@constructor.tech>
…ient secret Signed-off-by: Anton Zelenov <antonz@constructor.tech>
dd54dda to
311010f
Compare
First domain of the remaining .NET port: admin-gated CRUD over the global
`roles` table, 1:1 with the .NET RolesEndpoints (ADR-0013).
- GET /v1/roles — list all roles (ListResponse shape, next_cursor null).
- POST /v1/roles — create; validates name (non-empty, <=64); pre-checks the
unique name for a friendly 409 (already_exists) instead of an opaque 500;
201 + Location /v1/roles/{id}.
- DELETE /v1/roles/{id} — hard delete with an atomic in-use guard
(TryDeleteRoleIfUnused); 204 on success, 404 if missing, and a precondition
error when the role still has active person_roles.
Infra: roles_repo gains Role + get_by_name/get_by_id/list_all/insert_role/
try_delete_if_unused/count_active_assignments_any_tenant (SQL verbatim from
Sql.Roles.cs). Extract the shared admin gate (require_admin/resolve_caller) out
of seed.rs into api/gate.rs so roles/person-roles/visibility reuse it; add
AccessError (gate 403) + RoleError.
Divergence (documented): the .NET "role in use" 422 has no gears canonical
equivalent, so it is surfaced as failed_precondition (400) — same spirit as the
gts:// vs urn: error-type divergence.
Tests: role-name validation + resolve_caller (moved with the gate). clippy
clean. Live business-logic e2e is deferred to the gateway-JWT harness (#1753):
post-#1777 the host enforces gateway-JWT auth, so endpoints now require a signed
JWT — verified here that routes register and the authn gate returns canonical 401.
Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
Implements step 07 of the nginx+auth EPIC: the R1 rule as code — every downstream service verifies the gateway JWT itself, mandatory, fail-closed, no production disable knob. Tenant identity comes only from the signed JWT;
X-Tenant-IDis a selector among the signedtenants[](G2), never an authority.Algorithm is ES256 (§9.6, already shipped by the authenticator) → the existing
oidc-authn-pluginand .NET 9JwtBearerboth validate it with no plugin extension and no .NET blocker.A · shared
authverifycrate (src/backend/libs/authverify/)Pure claim→
SecurityContextmapping (§11.5):sub→subject (service:<name>→UUIDv5 +subject_type=service),roles→token_scopes, plus the G2 tenant selector — single tenant → that tenant (present selector must match, else 403); multiple → selector required (400) and must be a member (403); none → nil (cross-tenant service token). Therequire_gateway_contextaxum layer maps the host-verified token and fails closed, incl. a defense-in-depth guard that refuses the host's auth-disabled default context. 15 unit tests.B · analytics
Host auth enabled (
oidc-authn-pluginverifies signature/iss/aud/exp vs the authenticator JWKS) + theauthverifylayer. Deleted theauth_disabledtrust path and theX-Insight-Tenant-Idheader (auth.rs). Outbound identity calls forward the incomingAuthorization(G1).C · identity (.NET)
Full
JwtBearervalidation (ES256 pinned, issuer=gateway origin, aud=internal-services, JWKS viaConfigurationManager<JsonWebKeySet>since the authenticator serves no discovery doc) + aRequireAuthenticatedUserfallback policy (health/openapi anonymous). NewGatewayTenantContext(G2, hard-deny on rejected selection → 400/403) andSubjectCallerContext(sub=person_id). Raw customer-IdP andX-Insight-*trust paths removed.D · e2e (
src/backend/services/gateway/tests/step07/)Compose stack (fakeidp + authenticator + gateway + real analytics + real identity + MariaDB) and pytest covering the five §D scenarios: login→200, direct-no-JWT→401, multi-tenant selector 200/403/400, service token accepted with
roles:["service"], and a JWT-less request reaching analytics→401.Decisions
X-Tenant-ID(per G2 / the SPA);X-Insight-Tenant-Idremoved.subdirectly (person_id); IdP-account/email lookups +X-Insight-Person-Idremoved.Notes / follow-ups
POST /v1/persons-seednow requires auth (R1) — the dev seed script must obtain a service token.execute_metric_querystill skips tenant filtering on ClickHouse reads.Gateway
DESIGN.mdupdated and cfs-validated. An independent security review passed; its three findings (a MEDIUM identity fall-through on rejected selection, a defense-in-depth guard, a stale comment) are all fixed in this branch.Refs #1583
Closes #1590
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation