Skip to content

Ignore replayed OIDC callback when session is already authenticated#1343

Open
lovasoa wants to merge 2 commits into
mainfrom
investigate-cold-start-redirect-loop
Open

Ignore replayed OIDC callback when session is already authenticated#1343
lovasoa wants to merge 2 commits into
mainfrom
investigate-cold-start-redirect-loop

Conversation

@lovasoa

@lovasoa lovasoa commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Motivation

  • Prevent a short redirect loop on cold browser start where parallel protected-page loads can race through the OIDC flow and a replayed callback (missing the temporary state cookie) would restart login even though the session is already authenticated.

Description

  • In handle_oidc_callback_error add a branch that detects a missing temporary OIDC state cookie via is_missing_tmp_login_flow_state_cookie_error and an already-authenticated session via get_authenticated_user_info, logs a warning, clears the redirect-count cookie, and redirects to / instead of restarting the login flow.
  • Introduce the helper is_missing_tmp_login_flow_state_cookie_error to recognize the specific error string for a missing sqlpage_oidc_state_* cookie.
  • Add an integration test test_oidc_replayed_callback_after_login_does_not_restart_login in tests/oidc/mod.rs that reproduces the replayed-callback scenario and asserts the callback is ignored and redirects to /.
  • Update CHANGELOG.md with a note describing that OIDC login no longer restarts when a browser replays an already-consumed callback.

Testing

  • Ran the test suite with cargo test, including the new OIDC test test_oidc_replayed_callback_after_login_does_not_restart_login, and it passed.
  • Existing OIDC tests such as test_oidc_happy_path were executed and continued to pass.

Codex Task

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a0f8327795

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/webserver/oidc.rs
)
{
log::warn!("Ignoring replayed OIDC callback for an already authenticated session: {e:#}");
let mut resp = build_redirect_response("/".to_string());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Respect site_prefix when ignoring replayed callbacks

When site_prefix is configured and SQLPage is mounted only under that prefix, this new recovery path sends the browser to the host root instead of the SQLPage app. A replayed callback for /my-app/sqlpage/oidc_callback would therefore redirect to /, which may be handled by another app or by the reverse proxy rather than reaching SQLPage's prefix redirect; use the configured prefixed home URL for this fallback.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant