Restart the login when a callback carries no state at all

[compscidr]

Follow-up to #638, from a question that caught a real upgrade trap: how will I log in to update the theme?

The trap

#638 made goblog complete GitHub's OAuth flow itself and reject a callback whose state does not match. A login page from before 0.12.0 builds the authorize URL itself and asks for no state, so GitHub returns without one and the callback refuses it.

Reproduced against a local instance with a pre-0.12 theme active:

GET /login?code=REAL_CODE_FROM_GITHUB
Location: /login?error=github          ← bounced, and themes render no error

The only person who can fix that is an admin; updating a theme needs a login; the login is what is broken. min_goblog_version does not help — it guards a theme newer than goblog, not older.

The signal

The absence of a state. /login/github always sends one, so a callback without one is not our flow. Those now discard the code, unexchanged, and start a proper login instead.

The visitor ends up signed in as themselves — which is equally why handing this endpoint someone else's code achieves nothing. No unvalidated code is ever exchanged.

Why it cannot loop

This was my objection to doing it automatically, and it turns out the same signal settles it. The restarted flow does send a state, so a browser whose cookies do not survive comes back with one and falls through to the mismatch branch and its error page. Only a stateless callback restarts, and only our own flow is stateful, so the two cannot alternate.

That is what made this viable as an automatic fix rather than a page telling the admin to visit /login/github by hand.

next rides along when the callback carries one — a pre-0.12 redirect_uri did — so the restart lands where the visitor was going. GithubLogin puts it through SafeNext, so nothing is trusted here.

Tests

• TestGithubCallback_NoStateAtAllRestartsTheFlow — restarts, and the code is not exchanged
• TestGithubCallback_RestartKeepsNext — next carried into the restart
• TestGithubCallback_RestartCannotLoop — a callback that does carry a state, from a browser holding no session, gets the error rather than another restart

TestGithubCallback_EmptyStateBothSidesIsNotAMatch was renamed and tightened: that input now takes the restart path, so the test pins what still matters about it — the unvalidated code is not exchanged and no session is created.

Re-ran the mutation from #638 (state comparison bypassed): four tests still fail, including the new loop test, so the security property survived the restructuring.

Verified against a local instance running a pre-0.12 theme

callback	before	now
?code=… (no state)	/login?error=github	/login/github
?next=/admin/settings&code=…	/login?error=github	/login/github?next=%2Fadmin%2Fsettings
?code=…&state=issued-but-not-stored	/login?error=github	/login?error=github (unchanged — the loop guard)

Still to come

Auto-updating themes and plugins, which would prevent this class of upgrade mismatch rather than recover from one instance of it. Filed separately.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟢 Approval recommended

The change is narrowly scoped, preserves the state-validation security property, and is backed by targeted tests covering the new restart behavior and loop prevention.

Review effort: Lite
Findings: None

What changed in this PR

This PR improves the GitHub OAuth callback handling to recover from legacy (pre-0.12.0) themes that initiate OAuth without a state parameter, which previously caused /login callbacks to be rejected and could lock admins out of the UI needed to update the theme.

Changes:

• Treat callbacks with an absent/empty state as “not our flow”, discard the unvalidated code, and restart a proper /login/github flow (optionally carrying next).
• Update and expand internal OAuth tests to cover the restart behavior, preservation of next, and the “cannot loop” property.

File	Description
auth/​github_oauth.go	Restarts OAuth on callbacks missing state, carrying next into /login/github safely.
auth/​github_oauth_internal_test.go	Adds/updates tests to ensure missing-state callbacks restart without exchanging codes and that restarts can’t loop.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
auth/github_oauth.go	77.77%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!