Repository navigation
[codex] fix reborn google oauth decode and preview host login - #5388
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThree security hardening changes: (1) ChangesCanonical Host Redirect for Login Initiation
Google ID-token decoding hardening
OAuth env credential whitespace trimming
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
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 |
github-actions
Bot
added
size: M
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors Google ID token validation to bypass the jsonwebtoken signature validation configuration, avoiding the need for a dummy RSA key. It introduces a custom decode_google_id_token function that manually validates the algorithm, audience, and issuer. Feedback suggests addressing clock skew in the manual token expiration check, as the previous implementation automatically allowed a 60-second leeway.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // directly from Google. We still validate `alg`, `aud`, `iss`, | ||
| // and `exp` explicitly so the dependency's crypto backend does | ||
| // not need a fake RSA key just to parse Google's RS256 token. | ||
| let claims = decode_google_id_token(&id_token, &self.client_id)?; |
There was a problem hiding this comment.
The manual expiration check (claims.exp <= now) performed after decoding does not account for clock skew. Previously, jsonwebtoken::Validation::default() was used, which automatically allowed a default leeway of 60 seconds to accommodate minor clock differences between the host and Google's servers.
Without any leeway, minor clock drift can cause legitimate tokens to be rejected immediately. Consider adding a small leeway (e.g., 60 seconds) to the expiration check (e.g., claims.exp + 60 <= now) or validating the expiration inside decode_google_id_token with a leeway.
|
🚅 Deployed to the ironclaw-pr-5388 environment in ironclaw-ci-preview
|
github-actions
Bot
added
scope: channel/web
serrrfirat
changed the title
github-actions
Bot
added
size: XL
A Google login that reached the callback (valid client_id + registered
redirect_uri, so authorization succeeded) failed at the token exchange
with `invalid_client`, surfacing as `login_error=exchange_failed`
("Could not complete sign-in with the provider"). The client_secret is
the one credential a provider checks only at the token endpoint, never
at authorization, so a malformed secret sails through the login redirect
and is rejected only at code exchange.
Root cause: `non_empty_env` filtered on `.trim()` but returned the RAW
value, so an `IRONCLAW_REBORN_WEBUI_*_CLIENT_SECRET` pasted into a
deployment dashboard with a trailing newline / surrounding space was
forwarded to the provider verbatim.
- `non_empty_env` now returns the trimmed value and warns (naming the
variable) when it had to strip whitespace, so a malformed secret is
visible at boot.
- Add a redacted startup diagnostic (provider, client_id, secret length
— never the secret) so a wrong/mismatched secret is diagnosable from
boot logs without capturing a live login.
- Regression tests: `non_empty_env_trims_surrounding_whitespace`
(helper) and `whitespace_padded_oauth_credentials_still_configure_provider`
(through the `sso_startup_config_from_env` caller).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
51d3d2b to
676b5a3
Compare
github-actions
Bot
added
size: L
This branch was successfully deployed
1 participant
Summary
id_tokendecoding for real GoogleRS256tokens after thejsonwebtoken10.x bump./auth/login/{provider}toIRONCLAW_REBORN_WEBUI_BASE_URLbefore creating pending OAuth state, so Railway preview/custom domains cannot send the browser to a callback host that cannot see the minted state.Root Cause
Initial failure: Google returns
RS256ID tokens. The provider disabled signature verification but still calledjsonwebtoken::decodewithDecodingKey::from_secret(&[]). Withjsonwebtoken10.4.0 plus theaws_lc_rscrypto backend,decodestill builds a verifier from the token header before skipping signature validation, soRS256tokens paired with an HMAC empty key fail withInvalidKeyFormat.Second failure found in preview: the Railway PR web URL was
https://ironclaw-ironclaw-pr-5388.up.railway.app, but the OAuth callback configured in the generated Google authorization URL washttps://ironclaw-ci-preview.up.railway.app/auth/callback/google. A live fake-code flow that started on the PR web host redirected to the callback host and returned/v2?login_error=invalid_state, proving the pending state was minted on a host/process the callback could not read. The login route now redirects to the configured canonical base URL before state creation.The dependency break was exposed by #5271, which bumped
jsonwebtokento 10.4.0. The fragile empty-key pattern originated in the original WebUI v2 Google SSO implementation.Validation
cargo fmt --checkcargo test -p ironclaw_reborn_webui_ingress --all-features --test google_oauth_routes -- --nocapturecargo test -p ironclaw_reborn_webui_ingress --all-features --lib exchange_code_ -- --nocaptureCARGO_TARGET_DIR=target/codex-clippy-1.95 RUSTC=/Users/firatsertgoz/.rustup/toolchains/1.95.0-aarch64-apple-darwin/bin/rustc /Users/firatsertgoz/.rustup/toolchains/1.95.0-aarch64-apple-darwin/bin/cargo-clippy clippy -p ironclaw_reborn_webui_ingress --all-features --tests -- -D warnings