feat(fakeidp): fake OIDC provider for dev/e2e (nginx BFF step 03) - #1679
Conversation
Add `fakeidp` — a deliberately silly, in-repo fake OIDC provider so the
authenticator's real login code path (code + PKCE, rotating one-time-use
refresh tokens, RP-initiated and back-channel logout) runs locally and in
CI with no external IdP and no `dev_login` bypass. Implements the §10 G6
decision in cf/NGINX_BFF.md.
Plain axum binary (dash-free name), NOT a toolkit gear, never shipped in a
production image or referenced by a prod chart. Static RS256 keypair and test
users checked into the crate.
Endpoints: discovery, JWKS, instant `/authorize` (no login screen), `/token`
(authorization_code + PKCE and rotating refresh_token grants), `/end_session`.
Test-control hooks: `/_control/revoke/{email}` (→ invalid_grant),
`/_control/backchannel/{email}` (signed logout_token POST), `/_control/outage`
({off|5xx|timeout}), `/_control/state` (debug dump).
Wiring:
- New src/backend workspace member `fakeidp` (lib + thin bin so the
integration test drives the same router in-process).
- Multi-stage Dockerfile (analytics-shaped) + compose service on the default
dev profile (port 8084), building entirely from its own image.
- Registered in scripts/ci/components.py (cover=false, like api-gateway):
fmt + clippy + `cargo test` run in CI, no coverage gate for a test double.
- analytics/api-gateway Dockerfiles copy the new member manifest in their
dep-cache stage so their image builds still resolve the workspace.
- README with a copy-paste code+PKCE login sequence.
- Integration test: full code flow + refresh rotation + revoke → invalid_grant.
EPIC constructorfabric#1583. Closes constructorfabric#1586.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Anton Zelenov <antonz@constructor.tech>
📝 WalkthroughWalkthroughThis PR adds a new Changesfakeidp service
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant FakeIdp
participant RP as Relying Party
Client->>FakeIdp: GET /authorize (PKCE challenge, nonce)
FakeIdp-->>Client: 302 redirect with code
Client->>FakeIdp: POST /token (code, code_verifier)
FakeIdp->>FakeIdp: verify PKCE, consume code
FakeIdp-->>Client: id_token, refresh_token
Client->>FakeIdp: POST /token (refresh_token)
FakeIdp->>FakeIdp: rotate refresh_token
FakeIdp-->>Client: new id_token, new refresh_token
Client->>FakeIdp: POST /_control/revoke/{user}
FakeIdp->>FakeIdp: mark user revoked
Client->>FakeIdp: POST /token (old refresh_token)
FakeIdp-->>Client: 400 invalid_grant
FakeIdp->>RP: POST /_control/backchannel logout_token
RP-->>FakeIdp: 200 OK
Related Issues: Suggested labels: backend, dev-tooling, testing, docker Suggested reviewers: backend maintainers familiar with auth/OIDC flows and Docker Compose infra 🐰 A fake IdP hops into the stack, 🚥 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: 3
🧹 Nitpick comments (1)
src/backend/services/fakeidp/tests/flow.rs (1)
130-189: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood coverage of the rotation/revocation kill paths; consider extending to the remaining control hooks.
Refresh-rotation, reuse-rejection, and revoke-then-refresh-rejection are all solidly covered. The PR explicitly calls out
/end_session,/_control/backchannel, and/_control/outageas required capabilities for this service, but none of them have integration coverage here. Given these are exactly the "hard path" test-control hooks the service exists to provide for e2e, a regression here (e.g. outage mode not actually returning 5xx/timeout, or backchannel not POSTing) would go undetected by this suite.🤖 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/fakeidp/tests/flow.rs` around lines 130 - 189, The existing flow test covers refresh rotation and revoke behavior, but it does not exercise the remaining control hooks. Extend the integration test in the flow suite around the existing token/revoke checks to also verify the `/end_session`, `/_control/backchannel`, and `/_control/outage` paths using the same client setup. Add assertions for their expected behavior so regressions in these hard-path control endpoints are caught alongside the current refresh-token scenarios.
🤖 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/fakeidp/src/lib.rs`:
- Around line 253-265: The minted ID token claims currently omit tenant
information even though User.tenants is available from the fixture. Update
IdTokenClaims and the token-minting path that builds it to include the tenant
hints from User, serializing them into the ID token so downstream e2e flows can
read them. Use the existing IdTokenClaims struct and the minting logic that
populates it to ensure the tenant claims are emitted consistently alongside the
other standard claims.
- Around line 135-139: The refresh flow is losing the original client audience
and always minting ID tokens with default_aud, while the authorization code path
already uses the request’s client_id. Update the refresh-token data and token
issuance path in RefreshEntry and the related refresh grant logic so the
original client_id/audience is stored at sign-in time and reused when rotating
refresh tokens and generating refreshed ID tokens, instead of hardcoding the
default audience.
- Around line 641-646: The back-channel POST in the fakeidp logout flow is using
an unbounded reqwest client, which can hang the control hook indefinitely.
Update the client creation in the logout request path to build the
reqwest::Client with an explicit timeout before calling send(), so the POST
cannot stall forever; use the existing logout request logic around the
client/post/send sequence to locate it.
---
Nitpick comments:
In `@src/backend/services/fakeidp/tests/flow.rs`:
- Around line 130-189: The existing flow test covers refresh rotation and revoke
behavior, but it does not exercise the remaining control hooks. Extend the
integration test in the flow suite around the existing token/revoke checks to
also verify the `/end_session`, `/_control/backchannel`, and `/_control/outage`
paths using the same client setup. Add assertions for their expected behavior so
regressions in these hard-path control endpoints are caught alongside the
current refresh-token scenarios.
🪄 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: 5534a431-c46c-4c06-bb7e-b4a50e70bb58
⛔ Files ignored due to path filters (3)
src/backend/Cargo.lockis excluded by!**/*.locksrc/backend/services/fakeidp/keys/private.pemis excluded by!**/*.pemsrc/backend/services/fakeidp/keys/public.pemis excluded by!**/*.pem
📒 Files selected for processing (12)
docker-compose.ymlscripts/ci/components.pysrc/backend/Cargo.tomlsrc/backend/services/analytics/Dockerfilesrc/backend/services/api-gateway/Dockerfilesrc/backend/services/fakeidp/Cargo.tomlsrc/backend/services/fakeidp/Dockerfilesrc/backend/services/fakeidp/README.mdsrc/backend/services/fakeidp/src/lib.rssrc/backend/services/fakeidp/src/main.rssrc/backend/services/fakeidp/tests/flow.rssrc/backend/services/fakeidp/users.yaml
…ring Address review feedback on the fakeidp step-03 PR: - Generate the RS256 signing keypair at startup instead of checking PEMs into the repo. Nothing is persisted; clients fetch the current key from /jwks. Removes keys/private.pem + the unused keys/public.pem. - Enable coverage collection + the 80% gate in scripts/ci/components.py (it was cover=false like api-gateway, but fakeidp has real tests). Expanded the test suite to exercise every handler and error branch plus a boot test covering Config::from_env / run(); line coverage ~94%. - Wire the dev-impersonation identity: the first users.yaml entry defaults to dev-compose.sh's VITE_DEV_USER_EMAIL default (dev@company.nonpresent), and a new FAKEIDP_DEV_USER_EMAIL env (compose forwards VITE_DEV_USER_EMAIL) overrides it so the default login always matches the seeded dev person. - README: document that NOT using the gears-rust toolkit is intentional (a fast, throwaway fake of a third-party IdP), plus the key-generation and dev-user behaviour. EPIC constructorfabric#1583. Refs constructorfabric#1586. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
The compose target's config convention is env vars in .env.compose (cp'd from
.env.compose.example, then patched by insight-init.sh) with ${VAR}
interpolation in docker-compose.yml — not the template->yaml (cp + yq) render
used on the gitops/Helm path. fakeidp already follows it (FAKEIDP_DEV_USER_EMAIL
is interpolated from the wizard's VITE_DEV_USER_EMAIL); this just declares
FAKEIDP_PORT alongside the other service ports so it's discoverable and
conflict-avoidable like ANALYTICS_PORT/IDENTITY_PORT.
Refs constructorfabric#1586.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Anton Zelenov <antonz@constructor.tech>
- Preserve the original client_id audience across refresh rotation (store it in RefreshEntry at code-grant time and reuse it when minting the rotated id_token) — a non-default-client RP would otherwise reject refreshed tokens. - Emit user tenant hints as a `tenants` claim in the id_token so e2e can assert/map them. - Bound the back-channel POST with a 5s reqwest timeout so a stalled RP can't hang the control hook. Tests: added refresh_preserves_non_default_audience and tenant-claim assertions. Refs constructorfabric#1586. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/backend/services/fakeidp/tests/boot.rs (1)
14-31: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueUnsafe
env::set_varrelies on the test staying single-threaded.
std::env::set_varis only sound here because#[tokio::test]defaults to thecurrent_threadflavor. If this test (or a helper it shares) is ever changed to#[tokio::test(flavor = "multi_thread")], or another test gets added to this binary, the safety invariant silently breaks. Consider pinning the flavor explicitly (#[tokio::test(flavor = "current_thread")]) so the safety contract is enforced by the attribute rather than only by convention/comment.🤖 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/fakeidp/tests/boot.rs` around lines 14 - 31, Pin the test runtime flavor explicitly in boots_from_env_and_honors_dev_user_override so the unsafe std::env::set_var calls remain sound by construction; update the #[tokio::test] attribute on this test to use the current_thread flavor rather than relying on the default and the safety comment. Keep the environment setup as-is, but make the threading contract enforced by the test annotation instead of convention.
🤖 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/fakeidp/tests/boot.rs`:
- Around line 14-31: Pin the test runtime flavor explicitly in
boots_from_env_and_honors_dev_user_override so the unsafe std::env::set_var
calls remain sound by construction; update the #[tokio::test] attribute on this
test to use the current_thread flavor rather than relying on the default and the
safety comment. Keep the environment setup as-is, but make the threading
contract enforced by the test annotation instead of convention.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f208598d-6117-414c-9a8e-8f104e2d7402
📒 Files selected for processing (9)
.env.compose.exampledocker-compose.ymlscripts/ci/components.pysrc/backend/services/fakeidp/Dockerfilesrc/backend/services/fakeidp/README.mdsrc/backend/services/fakeidp/src/lib.rssrc/backend/services/fakeidp/tests/boot.rssrc/backend/services/fakeidp/tests/flow.rssrc/backend/services/fakeidp/users.yaml
✅ Files skipped from review due to trivial changes (1)
- src/backend/services/fakeidp/Dockerfile
🚧 Files skipped from review as they are similar to previous changes (2)
- docker-compose.yml
- src/backend/services/fakeidp/src/lib.rs
Part of the nginx + authorization epic (#1583), step 03 of 11. Closes #1586.
What
Adds
fakeidp— a deliberately silly, in-repo fake OIDC provider so the authenticator's real login code path runs locally and in CI with no external IdP and nodev_loginbypass. Implements the decided design incf/NGINX_BFF.md§10 G6.Endpoints
GET /.well-known/openid-configuration,GET /jwksGET /authorize— no login screen; mints a one-time code bound to (user, nonce, PKCE challenge), instant 302 back toredirect_uri. Optionaluser=<email>(defaults to first user inusers.yaml).POST /token—authorization_code(+ PKCE S256/plain) andrefresh_token(one-time-use rotation; old-token reuse →invalid_grant).GET|POST /end_session— RP-initiated logout.Test-control hooks (the reason it exists — G6)
POST /_control/revoke/{email}— future refreshes for that user →invalid_grant(the IdP-refusal kill path).POST /_control/backchannel/{email}— POSTs a signed OIDClogout_token(withsid,events, freshjti) toFAKEIDP_BACKCHANNEL_URL.POST /_control/outage{"mode":"off"|"5xx"|"timeout"}— makes/tokenmisbehave.GET /_control/state— debug dump.Wiring
src/backendworkspace memberfakeidp(lib + thin bin so the integration test drives the same router in-process).scripts/ci/components.pywithcover=false(likeapi-gateway): fmt + clippy +cargo testrun in CI, no coverage gate for a test double.analytics/api-gatewayDockerfiles copy the new member manifest in their dep-cache stage so their image builds still resolve the workspace (the only touch to existing services — mechanical, no behavior change).Verification (done locally)
cargo test -p fakeidp— full code+PKCE login, refresh rotation, reuse→invalid_grant, revoke→invalid_grant. ✅cargo fmt -p fakeidp -- --check,cargo clippy -p fakeidp --all-targets. ✅logout_tokento a stub RP. ✅docker build+docker runthe image: container starts and serves. ✅Out of scope
The authenticator itself; production hardening (this is a test double — kept silly).
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests