nginx+auth 07 — downstream gateway-JWT verification (R1) in analytics + identity#1777
Conversation
|
Important Review skippedToo many files! This PR contains 109 files, which is 9 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: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (144)
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
⛔ 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
| builder.Services | ||
| .AddAuthentication(JwtBearerDefaults.AuthenticationScheme) | ||
| .AddJwtBearer(options => | ||
| { | ||
| options.RequireHttpsMetadata = false; | ||
| // Keep JWT claim names as-is (`email`, `sub`, `oid`, …) so we | ||
| // can read them by their short JWT names. Without this, the | ||
| // default JwtBearer pipeline rewrites `email` / `sub` / `name` | ||
| // into long ClaimTypes.* URIs (a legacy WS-Federation hand-me- | ||
| // down), and `FindFirst("email")` would return null while | ||
| // `oid` (not in the rewrite table) would still work — easy to | ||
| // miss in review. False keeps every claim under one rule. | ||
| // Keep JWT claim names as-is (`sub`, `tenants`, `roles`, …) so resolvers | ||
| // read them by their short names rather than the long ClaimTypes.* URIs | ||
| // the default pipeline would rewrite them to. | ||
| options.MapInboundClaims = false; | ||
| options.TokenValidationParameters = new TokenValidationParameters | ||
| { | ||
| ValidateIssuer = false, | ||
| ValidateAudience = false, | ||
| ValidateLifetime = false, | ||
| ValidateIssuerSigningKey = false, | ||
| RequireSignedTokens = false, | ||
| // Accept any token shape; do not enforce signature. Returning | ||
| // a parsed JsonWebToken short-circuits the default signature | ||
| // verifier and lets the claim pipeline run. | ||
| SignatureValidator = (token, _) => new JsonWebToken(token), | ||
| ValidateIssuer = true, | ||
| ValidIssuer = gatewayIssuer, | ||
| ValidateAudience = true, | ||
| ValidAudience = "internal-services", | ||
| ValidateLifetime = true, | ||
| ValidateIssuerSigningKey = true, | ||
| RequireSignedTokens = true, | ||
| // Pin the algorithm to the authenticator's ES256 — reject `alg=none` | ||
| // and RS/HS confusion outright. | ||
| ValidAlgorithms = new[] { SecurityAlgorithms.EcdsaSha256 }, | ||
| IssuerSigningKeyResolver = (_, _, _, _) => | ||
| jwksConfigManager.GetConfigurationAsync(CancellationToken.None) | ||
| .GetAwaiter().GetResult().GetSigningKeys(), | ||
| }; | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant project files and package versions.
git ls-files 'src/backend/services/identity/src/Insight.Identity.Api/*' \
'src/backend/services/identity/src/*/*.csproj' \
'src/backend/services/identity/**/*csproj' \
'Directory.Packages.props' \
'global.json' \
| sed 's#^`#FILE` #'
printf '\n--- Program.cs context ---\n'
sed -n '120,190p' src/backend/services/identity/src/Insight.Identity.Api/Program.cs
printf '\n--- Project/package files mentioning Microsoft.IdentityModel or JwtBearer ---\n'
rg -n --hidden --glob '!**/bin/**' --glob '!**/obj/**' \
'Microsoft\.IdentityModel|JwtBearer|System\.IdentityModel|JsonWebKeySet|ConfigurationManager<|BaseConfigurationManager' \
src/backend/services/identity/src Directory.Packages.props global.jsonRepository: constructorfabric/insight
Length of output: 9270
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the exact package version references if present.
for f in $(git ls-files 'src/backend/services/identity/src/**/*.csproj' 'Directory.Packages.props'); do
echo "===== $f ====="
sed -n '1,240p' "$f"
echo
doneRepository: constructorfabric/insight
Length of output: 3080
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for any custom JWKS retriever/configuration code that might provide async-safe behavior.
rg -n --hidden --glob '!**/bin/**' --glob '!**/obj/**' \
'JwksRetriever|JsonWebKeySet|IssuerSigningKeyResolver|ConfigurationManager<|GetConfigurationAsync\(CancellationToken\.None\)|RequireHttps = false|ValidAlgorithms|MapInboundClaims = false' \
src/backend/services/identity/srcRepository: constructorfabric/insight
Length of output: 1851
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print the package version references and auth wiring.
files=$(git ls-files 'src/backend/services/identity/src/**/*.csproj' 'Directory.Packages.props')
for f in $files; do
echo "===== $f ====="
sed -n '1,220p' "$f"
done
echo "===== Program.cs ====="
sed -n '120,190p' src/backend/services/identity/src/Insight.Identity.Api/Program.csRepository: constructorfabric/insight
Length of output: 6629
🌐 Web query:
Microsoft.IdentityModel.Tokens BaseConfigurationManager JsonWebKeySet async JWKS source JwtBearer IssuerSigningKeyResolver GetConfigurationAsync
💡 Result:
In the Microsoft.IdentityModel.Tokens library, BaseConfigurationManager is an abstract base class designed to manage the retrieval and caching of configuration data, such as signing keys [1][2]. It is intended to be extended by specific implementations [3][2]. The primary mechanism for obtaining configuration asynchronously is the GetBaseConfigurationAsync method [3][1]. When working with JWKS (JSON Web Key Set) and JWT validation in ASP.NET Core, the recommended approach is to use a ConfigurationManager (which inherits from BaseConfigurationManager) rather than manually implementing the obsolete or synchronous-prone IssuerSigningKeyResolver delegate [4][5][6]. Key concepts for your implementation: 1. BaseConfigurationManager / ConfigurationManager: By using ConfigurationManager (e.g., with OpenIdConnectConfiguration), the library handles background refreshing of signing keys automatically, preventing thread starvation [7][6]. You can assign this manager to the TokenValidationParameters.ConfigurationManager property [6][8]. 2. GetConfigurationAsync: This is the primary asynchronous method used by ConfigurationManager to fetch metadata (including JWKS) [7]. It includes logic to return cached results if the refresh interval has not elapsed [7][9]. 3. IssuerSigningKeyResolver vs. ConfigurationManager: While TokenValidationParameters.IssuerSigningKeyResolver exists, it is a synchronous delegate and is generally considered legacy for JWKS-based validation [5][10]. Using the ConfigurationManager property is the modern, preferred way to handle dynamic signing key updates [6]. 4. Processing JWKS: If you must manually process a JWKS endpoint, you can deserialize the response into a JsonWebKeySet object [11]. This class provides a GetSigningKeys method to extract the keys into a format suitable for token validation [12][13][14]. Example pattern for ConfigurationManager: var configurationManager = new ConfigurationManager( "https://your-authority/.well-known/openid-configuration", new OpenIdConnectConfigurationRetriever, new HttpDocumentRetriever); validationParameters.ConfigurationManager = configurationManager; This setup allows the identity library to manage the key lifecycle, including fetching and periodic rotation, without requiring custom asynchronous code inside validation delegates [7][6].
Citations:
- 1: https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.tokens.baseconfigurationmanager?view=msal-web-dotnet-latest
- 2: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/dev/src/Microsoft.IdentityModel.Tokens/BaseConfigurationManager.cs
- 3: https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.tokens.baseconfigurationmanager.getbaseconfigurationasync?view=msal-web-dotnet-latest
- 4: https://stackoverflow.com/questions/66892866/issuersigningkeyresolver-call-async-method
- 5: https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.tokens.tokenvalidationparameters.issuersigningkeyresolver?view=msal-web-dotnet-latest
- 6: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/wiki/Validating-tokens-using-TokenValidationParameters.ConfigurationManager
- 7: https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.protocols.configurationmanager-1.getconfigurationasync?view=msal-web-dotnet-latest
- 8: https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.tokens.tokenvalidationparameters.configurationmanager?view=msal-web-dotnet-latest
- 9: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/dev/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs
- 10: https://stackoverflow.com/questions/53244446/how-to-validate-aws-cognito-jwt-in-net-core-web-api-using-addjwtbearer
- 11: https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.tokens.jsonwebkeyset?view=msal-web-dotnet-latest
- 12: https://stackoverflow.com/questions/70895630/how-do-you-inject-well-known-jwks-json-into-addjwtbearer-during-aspnet-sta
- 13: Microsoft.AspNetCore.Authentication.JwtBearer - Jwks Endpoint dotnet/aspnetcore#24309
- 14: https://stackoverflow.com/questions/40623346/how-do-i-validate-a-jwt-using-jwtsecuritytokenhandler-and-a-jwks-endpoint
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "ROOT=$(pwd)"
git ls-files | rg 'Program\.cs$|csproj$|Directory\.Packages\.props$|global\.json$' | head -200Repository: constructorfabric/insight
Length of output: 784
🌐 Web query:
Microsoft.IdentityModel.Protocols JsonWebKeySetConfigurationRetriever TokenValidationParameters.ConfigurationManager JWKS BaseConfigurationManager async
💡 Result:
In the Microsoft.IdentityModel ecosystem, these components work together to handle the dynamic retrieval and caching of OpenID Connect or JWKS metadata [1][2]. ConfigurationManager This is the primary class used to manage the retrieval of configuration data (such as OpenIdConnectConfiguration) [1]. It supports asynchronous operations to fetch, cache, and periodically refresh metadata from a remote endpoint [1][3]. JsonWebKeySetConfigurationRetriever This class is a specific implementation of IConfigurationRetriever intended for scenarios where you need to retrieve a JsonWebKeySet (JWKS) rather than a full OpenID Connect configuration [1]. It is used as a parameter when instantiating a ConfigurationManager [1]. TokenValidationParameters.ConfigurationManager This property in TokenValidationParameters allows you to associate a ConfigurationManager instance with token validation [4][2]. When set, the validation logic uses this manager to dynamically obtain issuer signing keys from the configured metadata endpoint, rather than relying on static keys [4][2]. This is essential for supporting key rotation [5]. BaseConfigurationManager BaseConfigurationManager is the base class for ConfigurationManager [6]. It defines the core contract for managing configuration, including the GetBaseConfigurationAsync method, which handles caching and refresh logic [7][6]. It is typically extended by specific implementations; custom configuration managers can inherit from this to implement specialized retrieval logic [7]. Async Usage Pattern When using these components, the standard approach is to perform configuration retrieval asynchronously [3]: var configManager = new ConfigurationManager( metadataAddress: "https://example.com/.well-known/jwks.json", configRetriever: new JsonWebKeySetConfigurationRetriever); // Retrieve the configuration (will return from cache if available and not expired) var jwks = await configManager.GetConfigurationAsync(CancellationToken.None); // Use in TokenValidationParameters var validationParameters = new TokenValidationParameters { ConfigurationManager = configManager }; If a failure occurs during validation due to an unknown key, you can call configManager.RequestRefresh to force the manager to ignore the cache on the next request and fetch fresh data [5].
Citations:
- 1: https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.protocols.configurationmanager-1?view=msal-web-dotnet-latest
- 2: https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.tokens.tokenvalidationparameters?view=msal-web-dotnet-latest
- 3: https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.protocols.configurationmanager-1.getconfigurationasync?view=msal-web-dotnet-latest
- 4: https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.tokens.tokenvalidationparameters.configurationmanager?view=msal-web-dotnet-latest
- 5: How to reset OpenIdConnectConfiguration when IssuerSigningKeys changes? dotnet/aspnetcore#27067
- 6: https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.tokens.baseconfigurationmanager?view=msal-web-dotnet-latest
- 7: https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.tokens.baseconfigurationmanager.getbaseconfigurationasync?view=msal-web-dotnet-latest
🌐 Web query:
"JsonWebKeySetConfigurationRetriever" Microsoft.IdentityModel "ConfigurationManager<JsonWebKeySet>"
💡 Result:
There is no public class named JsonWebKeySetConfigurationRetriever in the Microsoft.IdentityModel libraries. The functionality commonly associated with retrieving a JsonWebKeySet (JWKS) via a ConfigurationManager is typically achieved by implementing a custom IConfigurationRetriever or by extending the existing OpenIdConnectConfigurationRetriever [1][2][3]. The ConfigurationManager class, found in the Microsoft.IdentityModel.Protocols namespace, is designed to manage the retrieval, caching, and automatic refreshing of configuration data [4][5]. When using this class for token validation, it is often configured on the TokenValidationParameters.ConfigurationManager property [2][6]. Key points regarding this architecture: 1. Configuration Retrieval: To retrieve JWKS metadata specifically, developers often create a class that implements IConfigurationRetriever (where T is typically OpenIdConnectConfiguration) [1][2]. Inside the GetConfigurationAsync method, the retriever fetches the JSON document from the JWKS URI and parses it into a JsonWebKeySet object [1][7][3]. 2. Custom Implementation: If you require a specialized retriever for JWKS, you can implement a custom retriever that performs a GET request to the JWKS endpoint and returns the populated configuration [3]. This allows you to preserve the caching and rotation benefits provided by the ConfigurationManager while specifically targeting key retrieval [3]. 3. Standard Pattern: The OpenIdConnectConfigurationRetriever is the standard implementation used for OpenID Connect discovery; it retrieves the OIDC metadata document, extracts the jwks_uri, and then fetches the JWKS [1]. If you have encountered references to a JsonWebKeySetConfigurationRetriever in code or documentation, it is likely a custom, internal, or project-specific class rather than a part of the official Microsoft.IdentityModel.Protocols assemblies [5][3].
Citations:
- 1: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/dev/src/Microsoft.IdentityModel.Protocols.OpenIdConnect/Configuration/OpenIdConnectConfigurationRetriever.cs
- 2: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/wiki/Validating-tokens-using-TokenValidationParameters.ConfigurationManager
- 3: fix: use raw JWKS retriever instead of OIDC discovery retriever workos/workos-dotnet#267
- 4: https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.protocols.configurationmanager-1?view=msal-web-dotnet-latest
- 5: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/dev/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs
- 6: https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.tokens.tokenvalidationparameters.configurationmanager?view=msal-web-dotnet-latest
- 7: https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.tokens.jsonwebkeyset?view=msal-web-dotnet-latest
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' src/backend/services/identity/src/Insight.Identity.Api/Auth/JwksRetriever.csRepository: constructorfabric/insight
Length of output: 1229
Avoid blocking JWKS refresh in token validation. Wire jwksConfigManager through TokenValidationParameters.ConfigurationManager instead of IssuerSigningKeyResolver; the current GetConfigurationAsync(...).GetResult() path can stall authenticated requests when the cache is cold or the JWKS endpoint is slow.
🤖 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/Program.cs` around
lines 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.
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
⛔ 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
…ES256, single tenant_id) Rework of the downstream-verification cut per PR constructorfabric#1777 review: - Drop the insight-local fork `plugins/oidc-authn-plugin` and the bespoke `authverify` crate; consume the published `cf-gears-oidc-authn-plugin` (0.1.1), which verifies the JWT and maps claims to a SecurityContext via configurable `claim_mapping` — no wrapper, no `X-Tenant-ID` selector. - Gateway JWT back to ES256 (EC P-256); the real plugin's JWKS goes through jsonwebtoken's EC-capable JwkSet. Smaller signatures + faster verify. - Single signed `tenant_id` claim (drop multi-tenant `tenants[]` + selector); the plugin requires it (subject_tenant_id) so "no tenant, no auth" is native. Roles serialize as a space-delimited string (token_scopes). Service tokens get a per-service UUID `sub` + `sub_type=service` (no `service:<name>`). - Authenticator serves `/.well-known/openid-configuration` (the plugin resolves JWKS via OIDC discovery, https-only). Dev/e2e front the well-known endpoints with a self-signed TLS sidecar trusted via `custom_ca_certificate_paths`. - identity (.NET): JwtBearer pinned to ES256; read the single `tenant_id` claim; drop the tenants[]/`X-Tenant-ID` selector + TenantSelectorException. - analytics: plugin config in config + Helm ConfigMap (rendered from gateway.*); drop the dead flat oidc env. step07 e2e green (6/6 in docker): login->200, direct/no-JWT->401 (analytics + identity), valid-signature-missing-tenant_id->401, service-token->200, forged ->401. Workspace build/test/clippy clean. Authenticator + gateway DESIGN updated (cfs validate green). gears-rust#4215 re-scoped (toolkit-auth-only, not a blocker). Refs constructorfabric#1583, constructorfabric#1590. Signed-off-by: Anton Zelenov <antonz@constructor.tech>
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
⛔ 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
…ES256, single tenant_id) Rework of the downstream-verification cut per PR constructorfabric#1777 review: - Drop the insight-local fork `plugins/oidc-authn-plugin` and the bespoke `authverify` crate; consume the published `cf-gears-oidc-authn-plugin` (0.1.1), which verifies the JWT and maps claims to a SecurityContext via configurable `claim_mapping` — no wrapper, no `X-Tenant-ID` selector. - Gateway JWT back to ES256 (EC P-256); the real plugin's JWKS goes through jsonwebtoken's EC-capable JwkSet. Smaller signatures + faster verify. - Single signed `tenant_id` claim (drop multi-tenant `tenants[]` + selector); the plugin requires it (subject_tenant_id) so "no tenant, no auth" is native. Roles serialize as a space-delimited string (token_scopes). Service tokens get a per-service UUID `sub` + `sub_type=service` (no `service:<name>`). - Authenticator serves `/.well-known/openid-configuration` (the plugin resolves JWKS via OIDC discovery, https-only). Dev/e2e front the well-known endpoints with a self-signed TLS sidecar trusted via `custom_ca_certificate_paths`. - identity (.NET): JwtBearer pinned to ES256; read the single `tenant_id` claim; drop the tenants[]/`X-Tenant-ID` selector + TenantSelectorException. - analytics: plugin config in config + Helm ConfigMap (rendered from gateway.*); drop the dead flat oidc env. step07 e2e green (6/6 in docker): login->200, direct/no-JWT->401 (analytics + identity), valid-signature-missing-tenant_id->401, service-token->200, forged ->401. Workspace build/test/clippy clean. Authenticator + gateway DESIGN updated (cfs validate green). gears-rust#4215 re-scoped (toolkit-auth-only, not a blocker). Refs constructorfabric#1583, constructorfabric#1590. Signed-off-by: Anton Zelenov <antonz@constructor.tech>
Each red on PR constructorfabric#1777 traced to the rework, not the design: - Rust — oidc-authn-plugin: drop the stale coverage component in scripts/ci/components.py — that crate (the insight fork) was deleted, so its fmt/clippy matrix job ran `cargo fmt -p oidc-authn-plugin` against a package that no longer exists. - .NET — identity (CA1859): the two unit-test helpers (`Context`, `WithSub`) only ever return DefaultHttpContext; narrow the return type (analyzer-as-error). - Identity OpenAPI spec drift + integration tests: the fail-closed gateway-JWT wiring requires auth_gateway_issuer/jwks_url, which Program.cs reads BEFORE builder.Build() — so set them as IDENTITY__-prefixed env in the test factory (the one source that is both visible pre-Build and wins over appsettings' empty default; a ConfigureAppConfiguration source applies only at Build). - Identity tenant tests: the hand-built gateway JWT now carries a single `tenant_id` (was `tenants[]`), matching GatewayTenantContext; ES256 headers. - e2e (step-05 gateway): generate an EC P-256 signing key (the authenticator's p256 loader rejects RSA) and assert alg=ES256 — the RS256 detour leftover made the authenticator fail to boot, timing out the whole stack. - api / metrics / coverage gates (analytics 401 MISSING_BEARER): the ingestion bronze→API rig tests the data path directly (no gateway), so force api-gateway.auth_disabled=true — step 07 flipped the committed default to false for R1 (proven by the step-07 gateway e2e), which the rig inherited. Also fix a latent F821 (`build` → `locate_binary`) surfaced by linting the file. Refs constructorfabric#1583, constructorfabric#1590. Signed-off-by: Anton Zelenov <antonz@constructor.tech>
20c7397 to
a909b2f
Compare
Each red on PR constructorfabric#1777 traced to the rework, not the design: - Rust — oidc-authn-plugin: drop the stale coverage component in scripts/ci/components.py — that crate (the insight fork) was deleted, so its fmt/clippy matrix job ran `cargo fmt -p oidc-authn-plugin` against a package that no longer exists. - .NET — identity (CA1859): the two unit-test helpers (`Context`, `WithSub`) only ever return DefaultHttpContext; narrow the return type (analyzer-as-error). - .NET — identity (no-caller tests): step 07's RequireAuthenticatedUser fallback now 401s a token-less request at the middleware (empty body) before the endpoint runs, so the `caller_unresolved` body assertions saw empty JSON. The test factory now always attaches a bearer; a null caller means an authenticated-but-`sub`-less token so the endpoint's caller_unresolved path is reached (the true token-less 401 is covered by the compose e2e). - Identity OpenAPI spec drift + integration tests: the fail-closed gateway-JWT wiring requires auth_gateway_issuer/jwks_url, which Program.cs reads BEFORE builder.Build() — so set them as IDENTITY__-prefixed env in the test factory (the one source both visible pre-Build and winning over appsettings' empty default; a ConfigureAppConfiguration source applies only at Build). - Identity tenant tests: the hand-built gateway JWT now carries a single `tenant_id` (was `tenants[]`), matching GatewayTenantContext; ES256 headers. - e2e (step-05 gateway): generate an EC P-256 signing key (the authenticator's p256 loader rejects RSA) and assert alg=ES256 — the RS256 detour leftover made the authenticator fail to boot, timing out the whole stack. - api / metrics / coverage gates (analytics 401 MISSING_BEARER): the ingestion bronze→API rig tests the data path directly (no gateway), so force api-gateway.auth_disabled=true — step 07 flipped the committed default to false for R1 (proven by the step-07 gateway e2e), which the rig inherited. Also fix a latent F821 (`build` → `locate_binary`) surfaced by linting the file. Refs constructorfabric#1583, constructorfabric#1590. Signed-off-by: Anton Zelenov <antonz@constructor.tech>
a909b2f to
dbc8dc1
Compare
…ES256, single tenant_id) Rework of the downstream-verification cut per PR constructorfabric#1777 review: - Drop the insight-local fork `plugins/oidc-authn-plugin` and the bespoke `authverify` crate; consume the published `cf-gears-oidc-authn-plugin` (0.1.1), which verifies the JWT and maps claims to a SecurityContext via configurable `claim_mapping` — no wrapper, no `X-Tenant-ID` selector. - Gateway JWT back to ES256 (EC P-256); the real plugin's JWKS goes through jsonwebtoken's EC-capable JwkSet. Smaller signatures + faster verify. - Single signed `tenant_id` claim (drop multi-tenant `tenants[]` + selector); the plugin requires it (subject_tenant_id) so "no tenant, no auth" is native. Roles serialize as a space-delimited string (token_scopes). Service tokens get a per-service UUID `sub` + `sub_type=service` (no `service:<name>`). - Authenticator serves `/.well-known/openid-configuration` (the plugin resolves JWKS via OIDC discovery, https-only). Dev/e2e front the well-known endpoints with a self-signed TLS sidecar trusted via `custom_ca_certificate_paths`. - identity (.NET): JwtBearer pinned to ES256; read the single `tenant_id` claim; drop the tenants[]/`X-Tenant-ID` selector + TenantSelectorException. - analytics: plugin config in config + Helm ConfigMap (rendered from gateway.*); drop the dead flat oidc env. step07 e2e green (6/6 in docker): login->200, direct/no-JWT->401 (analytics + identity), valid-signature-missing-tenant_id->401, service-token->200, forged ->401. Workspace build/test/clippy clean. Authenticator + gateway DESIGN updated (cfs validate green). gears-rust#4215 re-scoped (toolkit-auth-only, not a blocker). Refs constructorfabric#1583, constructorfabric#1590. Signed-off-by: Anton Zelenov <antonz@constructor.tech>
Each red on PR constructorfabric#1777 traced to the rework, not the design: - Rust — oidc-authn-plugin: drop the stale coverage component in scripts/ci/components.py — that crate (the insight fork) was deleted, so its fmt/clippy matrix job ran `cargo fmt -p oidc-authn-plugin` against a package that no longer exists. - .NET — identity (CA1859): the two unit-test helpers (`Context`, `WithSub`) only ever return DefaultHttpContext; narrow the return type (analyzer-as-error). - .NET — identity (no-caller tests): step 07's RequireAuthenticatedUser fallback now 401s a token-less request at the middleware (empty body) before the endpoint runs, so the `caller_unresolved` body assertions saw empty JSON. The test factory now always attaches a bearer; a null caller means an authenticated-but-`sub`-less token so the endpoint's caller_unresolved path is reached (the true token-less 401 is covered by the compose e2e). - Identity OpenAPI spec drift + integration tests: the fail-closed gateway-JWT wiring requires auth_gateway_issuer/jwks_url, which Program.cs reads BEFORE builder.Build() — so set them as IDENTITY__-prefixed env in the test factory (the one source both visible pre-Build and winning over appsettings' empty default; a ConfigureAppConfiguration source applies only at Build). - Identity tenant tests: the hand-built gateway JWT now carries a single `tenant_id` (was `tenants[]`), matching GatewayTenantContext; ES256 headers. - e2e (step-05 gateway): generate an EC P-256 signing key (the authenticator's p256 loader rejects RSA) and assert alg=ES256 — the RS256 detour leftover made the authenticator fail to boot, timing out the whole stack. - api / metrics / coverage gates (analytics 401 MISSING_BEARER): the ingestion bronze→API rig tests the data path directly (no gateway), so force api-gateway.auth_disabled=true — step 07 flipped the committed default to false for R1 (proven by the step-07 gateway e2e), which the rig inherited. Also fix a latent F821 (`build` → `locate_binary`) surfaced by linting the file. Refs constructorfabric#1583, constructorfabric#1590. Signed-off-by: Anton Zelenov <antonz@constructor.tech>
1a4f7fc to
5d80844
Compare
| - X-Real-IP | ||
| - Forwarded | ||
| routes: | ||
| # strip_prefix turns `/api/v1/analytics/v1/metrics` into the service's own |
There was a problem hiding this comment.
Why you invent double v1? It should be /api/analytics/v1/metrics, i.e. prefix + service name + service url
| deploy: true | ||
|
|
||
| frontend: | ||
| replicaCount: 1 |
There was a problem hiding this comment.
Why you dropped FE deploy from CI?
b93ac29 to
708c1d6
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.
708c1d6 to
7070d7f
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.
7070d7f to
de5e1c5
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.
de5e1c5 to
b1a1124
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).
b1a1124 to
a3eeba2
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`.
a3eeba2 to
19ad096
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).
19ad096 to
d09a95b
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).
d09a95b to
d83f344
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.
d83f344 to
103ce7d
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>
103ce7d to
41a1eb3
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>
41a1eb3 to
fed9ce8
Compare
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