Repository navigation
feat(auth): OIDC SSO + Keycloak dev tooling + workflow trigger fix - #93
Merged
Merged
Conversation
mongodb 3.6.0 pinned hickory-resolver/hickory-proto 0.25 for its mongodb+srv SRV stub resolver, leaving one vulnerable hickory-proto 0.25.2 copy in the tree even after the workspace moved to 0.26.1. mongodb 3.7.0 updated its hickory dependency to 0.26. The lockfile now resolves a single hickory-proto / hickory-resolver 0.26.1 across the whole workspace — the DNS CVEs (NSEC3 unbounded loop, O(n^2) name-compression) are fully cleared, with no residual. Verified: cargo check --bin temps compiles clean against mongodb 3.7.0.
OIDC SSO (OpenID Connect single sign-on)
- New temps-auth/src/oidc_{errors,handler,service,types}.rs backed by the
openidconnect 3.x crate. PKCE code-flow with discovery, nonce/state, userinfo,
and just-in-time user provisioning.
- Admin endpoints (POST/GET/PATCH/DELETE /auth/oidc/providers,
POST /auth/oidc/providers/{id}/test) for IdP CRUD.
- Public flow endpoints (GET /auth/oidc/providers,
POST /auth/oidc/{provider}/start, GET /auth/oidc/{provider}/callback).
- Two migrations:
m20260522_000001_oidc_sso: creates oidc_providers + oidc_login_states,
adds users.oidc_provider_id + users.oidc_subject with a unique
compound index for the (provider_id, subject) federation key.
m20260522_000002_oidc_role_mappings: adds oidc_role_mappings so
groups / claims from the IdP map to Temps roles automatically.
- Web: /settings/auth page (AuthSettingsPage, CreateOidcProviderPage,
OidcProviderForm, OidcRoleMappingsCard) for IdP CRUD + role mapping;
"Continue with {provider}" buttons on the login form.
- Local Keycloak dev environment under tools/keycloak-dev/ (docker-compose
+ realm export + setup script) so contributors can stand up a working
SSO flow in one command. Default realm contains the dev client secret
"temps-ee-dev-secret" — intentional, dev-only, never used in prod.
Console extension points (web/packages/console-kit/)
- New local workspace package @temps-sdk/console-kit exporting
ConsoleExtensionsProvider + useConsoleExtensions. Downstream consoles
(notably temps-ee/web) can inject extra routes into the authenticated
shell without forking App.tsx.
- rsbuild resolves the package via an explicit resolve.alias so dev
works even when node_modules/@temps-sdk/console-kit is stale or missing.
- Parallel web/packages/ui/ scaffolded for shared shadcn/ui primitives
(continuation of ADR 0003 — OSS↔EE frontend reuse).
Workflow trigger save fix (AgentSettingsDialog)
- Dialog initialised triggerNewIssue / triggerRegression / triggerManual
to `?? true` and emitted every trigger block on save, so opening Edit
and clicking Save would silently turn on error.new_issue,
error.regression, and manual triggers even when the YAML omitted them.
- Defaults now flip to `?? false`; save payload only includes
error / manual / schedule blocks when the corresponding inputs are
actually set. Round-tripping Edit → Save with no UI changes now leaves
the workflow's triggers byte-identical.
Plumbing
- 30+ plugin.rs files migrated from `PluginRoutes { router: routes }`
literal to `PluginRoutes::new(routes)` constructor (continuation of
the temps-core::plugin route-override work).
- temps-cli serve/console.rs + reset_password.rs + setup.rs propagate
the new users.{oidc_provider_id, oidc_subject} columns through the
ActiveModel insert sites.
…iterals
The OIDC SSO migration in m20260522_000001 added two new columns to the
users table (oidc_subject + oidc_provider_id), so every hand-written
`users::Model { ... }` struct literal across the workspace needed the two
fields. CI surfaced 6 sites (5 files) where test helpers built mock
AuthContexts and were now missing both fields:
- crates/temps-analytics-events/src/handlers/events_handler.rs
- crates/temps-auth/tests/context_tests.rs
- crates/temps-deployments/src/handlers/deployments.rs
- crates/temps-email/src/handlers/tracking_tests.rs
- crates/temps-log-aggregator/src/handlers/log_handler.rs (×2)
All test users default to `None` for both fields — they're not OIDC-backed.
Also applies cargo fmt across the OIDC handler/service/types and the
plugin route literals touched in 581d71c.
Clippy's `collapsible_if` lint flagged the `if insert { if self.created_at.is_not_set() { ... } }`
pattern. Collapsed into a single `if insert && self.created_at.is_not_set()`.
No behaviour change.
H1 — Audit MFA-gated SSO logins
oidc_handler::complete_oidc_login wrote LoginAudit only on the non-MFA
path, so an attacker who completed an IdP login but stopped at MFA
never appeared in audit logs. Now writes a `oidc-mfa-pending` audit row
before the MFA early-return; the MFA-verify endpoint's existing audit
on success forms the second leg of the trail.
Drive-by: replaced the `mfa_cookie.parse().unwrap()` with proper error
propagation so we don't crash on a malformed cookie header.
H2 — Log abusive callback probes
`/auth/oidc/callback` returned a generic warn! for every error. Now
distinguishes `StateNotFound` (possible probe / replay of a never-issued
state) and `StateExpired` from ordinary failures, emitting structured
warn! lines under the `temps_auth::oidc::abuse` target with the caller
IP + UA + age. SOC tooling can alert on that target without scraping
free-form error text. The browser-facing redirect is unchanged so we
don't leak which states are valid.
H3 — Cache decrypted client secret with the discovery metadata
`core_client_for_provider` re-ran `EncryptionService::decrypt_string`
on every authorize and token-exchange call even though the discovery
metadata was cached for an hour. CachedClient now stores the decrypted
secret alongside metadata, plus the original ciphertext so the cache
detects out-of-band secret rotation (direct SQL fixups). All existing
invalidations (create / update / delete provider) already drop the
cache entry, so secret rotation through the API picks up immediately.
H4 — Normalise empty scopes on update
`create_provider` defaulted blank scopes to "openid email profile";
`update_provider` silently persisted them as "". After such an update,
start_login built an empty scopes vector — invisible on Keycloak (uses
default client scopes), fatal on Okta (requires an explicit scope set).
Extracted both paths to a shared `normalize_scopes` helper so the
fallback is consistent.
Tests
- 2 new unit tests for normalize_scopes (empty + whitespace fallback,
caller-value preservation).
- All 10 OIDC unit tests pass (was 8); workspace clippy clean with
-D warnings.
…nts UI OIDC discovery and token exchange were broken end-to-end against Auth0. Three separate bugs all surfaced as cryptic top-line errors because openidconnect's DiscoveryError and RequestTokenError hide their underlying cause behind Error::source(). * TLS - workspace openidconnect was declared with default-features = false, features = ["reqwest"], which dropped the bundled rustls-tls. The reqwest::Client built by async_http_client then had no TLS connector and hyper rejected every https:// URL with "invalid URL, scheme is not http". Added rustls-tls back explicitly. * Trailing slash - normalize_issuer_url stripped trailing slashes, but OIDC Core 16.13 requires the discovery issuer field to match the URL we asked about byte-for-byte. Auth0 publishes its issuer with a trailing slash; stripping it made every discovery fail with "unexpected issuer URI". Now we only trim whitespace. * updated_at as ISO string - Auth0 returns updated_at in id_tokens as RFC 3339 when the upstream connection is Google/social, violating OIDC Core 5.1. Added accept-rfc3339-timestamps to openidconnect features so the Timestamp enum accepts strings as well as numbers. Error surface * New describe_discovery_error walks Error::source() so discovery failures (Parse, Request, UrlParse) emit the full cause chain instead of just the variant label. * Token-exchange .map_err now pattern-matches RequestTokenError: surfaces the IdP's parsed OAuth error: error_description for ServerResponse, the raw body for Parse, and the source chain for Request. Implemented as an inline closure because the workspace pulls two reqwest versions (oauth2 0.4 pins 0.11; everything else uses 0.12) and any named type signature picks the wrong one. Tests * Unit tests around normalize_issuer_url updated to assert trailing-slash preservation, whitespace trimming, and scheme enforcement. * New describe_discovery_error_walks_source_chain test against a hand-rolled 3-deep error chain. * New opt-in live integration test oidc_discovery_live.rs (TEMPS_RUN_LIVE_OIDC_TESTS=1) that hits the real Auth0 tenant and asserts both the trailing-slash success path and the no-slash failure mode, so a future regression that re-strips the slash gets caught. Drive-by clippy fixes encountered on this branch: * temps-proxy/src/server.rs: removed duplicate #[allow(clippy::too_many_arguments)] on setup_proxy_server. * temps-proxy/src/proxy.rs: dropped a needless & on body.len() in the 404 Content-Length header insert. Also included (work-in-progress on this branch): * admin-gate plugin scaffold (temps-core::admin_gate, plugin handler and service in temps-cli, AdminGateCard settings UI) * OIDC provider detail page + form refactors * Agents page split (AgentDetailPage / AgentEditPage replacing the monolithic AgentSettingsDialog) + autopilot wiring * Generated web/src/api/client SDK refreshed for the new endpoints
… MEDIUM)
Second-pass security audit on top of the OIDC SSO + admin-gate work
turned up one HIGH and five MEDIUM findings. This commit closes them
all. cargo check, cargo fmt --check, cargo clippy --workspace
--all-targets --all-features -- -D warnings, and cargo test --lib -p
temps-auth (167/167) are all green.
HIGH
* Raw IdP error_description was being reflected into the callback's
?reason= query param and ended up in the browser address bar,
history, and Referer headers. A malicious or chatty IdP could leak
internal state (locked-account reasons, internal IPs, tenant
policy text) through that channel. The handler now emits a short
opaque code ("idp_error") to the browser and logs the raw
description server-side only. The same opaque-code policy now
also covers the missing-code/state branch ("callback_invalid").
Login.tsx ships a code -> friendly-message table so the user
still sees something legible.
MEDIUM
* Backslash open redirect in validate_return_to. /\evil.com passed
both checks (starts with /, does not start with //), and Chrome /
Edge normalize \ to /, turning it into //evil.com -> external
host -> post-auth phishing. The validator now refuses any
backslash and any control char (CR / LF / NUL / tab) for
defence-in-depth against response-splitting. Same checks added to
the MfaVerify return_to handler on the frontend.
* enabled flag was not re-checked on the callback path. An admin
disabling a provider in an incident-response scenario still let
every in-flight login within the 10-minute LOGIN_STATE_TTL window
complete successfully. complete_oidc_login now re-checks
provider.enabled after get_provider and returns ProviderDisabled
otherwise.
* JWKS stale-cache outage on signing-key rotation. The discovery
doc (with embedded JWKS) is cached for 1h; when an IdP rotates
its JWK normally, every login failed with IdTokenInvalid until
TTL expiry. exchange_code now retries verification exactly once
with a forced discovery refresh when the failure text looks like
a signing-key problem (no matching key / kid / signature / jwks).
Narrow trigger set so claim-validation failures (bad audience,
expired token, nonce mismatch) don't amplify into discovery
storms or mask real config bugs. New core_client_for_provider_refresh
helper + looks_like_jwks_rotation heuristic + 1 unit test.
* Admin gate was fail-OPEN on DB load error. If the settings row
was corrupt, partially migrated, or had a deleted admin_gate key,
the boot path silently installed a noop config -- meaning the
management surface became reachable from any IP/host. An attacker
with DB write access could disable the gate by corrupting one
row. Now fail-CLOSED: load errors propagate as boot failures
with an explicit error! log telling the operator what to fix
(repair the row or set TEMPS_ADMIN_* env vars). "No row found"
remains the intentional default-noop case.
* consume_login_state was a non-atomic SELECT-then-DELETE. Under
concurrent callbacks (browser double-submit, retry) two requests
could both pass the SELECT before either ran the DELETE, then
both proceed into exchange_code with the same nonce + PKCE
verifier. IdP single-use on the auth code is the outer gate, but
this is still a broken atomicity guarantee in a security-critical
path. Replaced with a single DELETE ... RETURNING * via
Statement::from_sql_and_values (same pattern used in
error_alert_service.rs). Atomic at the DB level, single round-trip.
Frontend
* Login.tsx: OIDC_ERROR_MESSAGES table maps every code emitted by
login_error_code_for() to a friendly user-facing string. Defaults
to "SSO sign-in failed." for unknown codes so a future server-side
code addition without a frontend update degrades gracefully
rather than rendering the raw token.
* MfaVerify.tsx: now reads ?return_to= from the URL set by the SSO
callback redirect (preferred over sessionStorage for tab-switch
and private-window flows). Applies the same allow-list that
validate_return_to enforces server-side: starts with /, not //,
no \.
Tests
* New: looks_like_jwks_rotation_matches_signing_key_failures_only
proves the heuristic triggers on real signing-key text and stays
silent on claim-validation failures.
* New: validate_return_to_rejects_backslash_open_redirect covers
/\evil.com plus a few path-component-style attempts.
* New: validate_return_to_rejects_control_chars covers CRLF / NUL
/ tab response-splitting payloads.
* New: validate_return_to_rejects_absolute_and_scheme_relative
consolidates the existing //evil and https://evil cases plus
javascript: schemes.
* Extended sanitize_return_to_rejects_open_redirect with the
backslash case (the public-facing sanitize wrapper used by the
callback handler).
* Existing 162 tests still pass; total is now 167/167.
Drive-by
* temps-proxy: has_route_for_host trait method so the admin gate
recognizes legitimate project deployment hosts (HTTP route table,
SNI/TLS map, wildcards) AND operator-defined custom routes. Was
needed by an admin-gate hostname check that landed on the branch
while this round was in flight -- bundled here so the proxy
build stays consistent.
1 of 3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
temps-auth/src/oidc_*module, two migrations (oidc_providers+oidc_login_states+users.oidc_provider_id/oidc_subject, thenoidc_role_mappings), admin CRUD + public flow endpoints, full/settings/authweb UI, and a one-command Keycloak dev environment undertools/keycloak-dev/.web/packages/console-kit/local workspace package (ConsoleExtensionsProvider+useConsoleExtensions) so downstream consoles (temps-ee/web) inject routes into the auth shell without forkingApp.tsx.AgentSettingsDialogwas silently enablingerror.new_issue/error.regression/manualtriggers on any Save (defaults?? true, payload unconditional). Flipped defaults to?? falseand made the save payload conditional — round-tripping Edit→Save with no UI changes now leavestriggersbyte-identical.PluginRoutes { router }literal toPluginRoutes::new(routes); CLI ActiveModel insert sites updated for the newusers.oidc_*columns.mainfcd54381mongodb 3.6 → 3.7 bump (closes the residual hickory 0.25 CVE chain) is rolled into this branch for traceability.Test plan
cargo check --all-targets --workspacecleancargo test -p temps-auth oidcpassescargo test -p temps-migrationspasses (verify m20260522 migrations apply + roll back)tools/keycloak-dev/(./setup.sh) and complete the SSO login flow againstlocalhost:9080/settings/auth/new, run "Test provider", complete a browser login, verify just-in-time user creationAgentSettingsDialog, click Save with no changes, diff the resulting YAML —triggersmust be byte-identical (regression check for the trigger-save bug)temps-ee/webstill builds with@temps-sdk/console-kitlinked from this workspace