docs(caddy,runbook): deep links do survive OIDC sign-in, with the evidence #306

Sammanfogat

supernaut sammanfogade 1 incheckning från docs/oidc-redirect-verified in i main

2026-08-01 16:53:41 +00:00

supernaut kommenterad

2026-08-01 14:27:10 +00:00

Ägare

Closes #291 — as not a bug. Deep links do survive OIDC sign-in, including through a second-factor
hop. No fix was needed, no upstream report is warranted, and there is no upstream issue to link.

The issue explicitly warned against closing it on header evidence: "the parameter is accepted" and
"the destination is used" are different assertions, and only an actual login distinguishes them. That
caution is honoured — the load-bearing evidence here is completed logins, not response headers.

Evidence

1. The code, at the tag we run (Forgejo 16.0.1). SignInOAuth sets the cookie
(routers/web/auth/oauth.go:962-965); handleOAuth2SignIn reads it back and honours it before
falling back to / (oauth.go:1443-1449 → RedirectToFirst → context_response.go:53-69).
/explore/repos is not rejected as risky (modules/httplib/url.go:22-40). The 2FA branch does not
clear the cookie, and both completion paths read it (auth.go:389-398, webauthn.go:178-184). OAuth2
routes are not under reqSignOut (web.go:856-859), so the query-only short circuit at
web.go:281-285 never applies. All three candidate explanations in the issue are therefore false.

2. Production. Over seven days of Caddy access logs, the callback's own Location header was
/explore/repos ×5, /user/webauthn ×6, /user/two_factor ×2 — and / zero times. One journey
end to end: callback → 303 /user/webauthn → POST /user/webauthn/assertion 200 →
GET /explore/repos 200.

3. What the original report actually caught. The reported walkthrough is in the log one minute
before the issue was filed, and at 20:47:49 the deep link worked. The reporter then deleted their
WebAuthn credential at 20:48:14, which dropped subsequent logins onto TOTP — where
POST /user/two_factor returned 500. That is the retired-SECRET_KEY AEAD failure already
recorded in the runbook, which notes that removing a passkey is exactly what surfaces it. The login
never completed, so the / seen in the browser was Forgejo's anonymous landing page (which itself
redirects / → /explore → /explore/repos), not a post-login destination.

So the observation was real and the diagnosis was inverted: this is #292, not a redirect bug.

SameSite=Lax was ruled out by reasoning rather than testing: the session cookie carries identical
attributes and must reach the callback for PKCE and state to verify, so if Lax blocked that hop no
login would ever succeed.

What changed

Only comments and documentation.

ansible/roles/caddy/templates/Caddyfile.j2 — the comment there had made precisely the inference the
issue warns against, marking the behaviour "VERIFIED" on the strength of the cookie being set. Now
states what is actually verified and how.
docs/runbook.md — the finding, the evidence and the #292 connection, so nobody re-investigates this
from scratch.

Applying

Nothing to apply. A converge will report changed on the Caddyfile template and restart Caddy, which
is harmless — worth bundling with the next real apply rather than restarting Caddy for a comment.

Closes #291 — **as not a bug.** Deep links do survive OIDC sign-in, including through a second-factor hop. No fix was needed, no upstream report is warranted, and there is no upstream issue to link. The issue explicitly warned against closing it on header evidence: *"the parameter is accepted" and "the destination is used" are different assertions, and only an actual login distinguishes them.* That caution is honoured — the load-bearing evidence here is **completed logins**, not response headers. ## Evidence **1. The code, at the tag we run** (Forgejo 16.0.1). `SignInOAuth` sets the cookie (`routers/web/auth/oauth.go:962-965`); `handleOAuth2SignIn` **reads it back** and honours it before falling back to `/` (`oauth.go:1443-1449` → `RedirectToFirst` → `context_response.go:53-69`). `/explore/repos` is not rejected as risky (`modules/httplib/url.go:22-40`). The 2FA branch does not clear the cookie, and both completion paths read it (`auth.go:389-398`, `webauthn.go:178-184`). OAuth2 routes are not under `reqSignOut` (`web.go:856-859`), so the query-only short circuit at `web.go:281-285` never applies. All three candidate explanations in the issue are therefore false. **2. Production.** Over seven days of Caddy access logs, the callback's own `Location` header was `/explore/repos` ×5, `/user/webauthn` ×6, `/user/two_factor` ×2 — and **`/` zero times**. One journey end to end: callback → `303 /user/webauthn` → `POST /user/webauthn/assertion 200` → `GET /explore/repos 200`. **3. What the original report actually caught.** The reported walkthrough is in the log one minute before the issue was filed, and at 20:47:49 the deep link **worked**. The reporter then deleted their WebAuthn credential at 20:48:14, which dropped subsequent logins onto TOTP — where `POST /user/two_factor` returned **500**. That is the retired-`SECRET_KEY` AEAD failure already recorded in the runbook, which notes that removing a passkey is exactly what surfaces it. The login never completed, so the `/` seen in the browser was Forgejo's anonymous landing page (which itself redirects `/` → `/explore` → `/explore/repos`), not a post-login destination. So the observation was real and the diagnosis was inverted: this is #292, not a redirect bug. `SameSite=Lax` was ruled out by reasoning rather than testing: the session cookie carries identical attributes and must reach the callback for PKCE and state to verify, so if `Lax` blocked that hop no login would ever succeed. ## What changed Only comments and documentation. - `ansible/roles/caddy/templates/Caddyfile.j2` — the comment there had made precisely the inference the issue warns against, marking the behaviour "VERIFIED" on the strength of the cookie being set. Now states what is actually verified and how. - `docs/runbook.md` — the finding, the evidence and the #292 connection, so nobody re-investigates this from scratch. ## Applying Nothing to apply. A converge will report `changed` on the Caddyfile template and restart Caddy, which is harmless — worth bundling with the next real apply rather than restarting Caddy for a comment.

supernaut lade till 1 incheckning

2026-08-01 14:27:11 +00:00

docs(caddy,runbook): deep links do survive OIDC sign-in, with the evidence

ci / ci (pull_request) Successful in 1m25s

Detaljer

08bf765985

Closes #291, which asked either for a fix or for confirmed upstream behaviour with a link. Neither
applies: **there is no bug.** `redirect_to` survives OIDC sign-in, including through a second-factor
hop. Nothing to fix, nothing to report upstream, and no code change here — only the finding, written
where the next person will look for it.

The issue explicitly warned that header evidence is not enough to close it ("the parameter is
accepted" and "the destination is used" are different claims), so both halves are established
independently.

## Code, at the pinned tag (Forgejo 16.0.1)

None of the three candidate explanations in the issue holds:

- `SignInOAuth` stores `?redirect_to=` in the cookie (`routers/web/auth/oauth.go:962-965`).
- `handleOAuth2SignIn` **reads that cookie back** and honours it before the fallback to `/`
  (`oauth.go:1443-1449`), via `RedirectToFirst`, which accepts any non-risky same-site path
  (`services/context/context_response.go:53-69`, `modules/httplib/url.go:22-40` —
  `/explore/repos` is not risky).
- The second-factor branch does not clear the cookie, and both completions read it too:
  `/user/two_factor` and `/user/webauthn` through `handleSignInFull`
  (`routers/web/auth/auth.go:389-398`, `webauthn.go:178-184`).

The OAuth2 routes are also not under `reqSignOut` (`routers/web/web.go:856-859`), so the
already-signed-in short circuit at `web.go:281-285` — the one place that reads the query parameter
without the cookie — never runs on this path.

## Production, from completed logins

The decisive artefact is the **callback's own `Location` header** in Caddy's access log, not the URL
the browser ends on. Across seven days every callback either returned the destination
(`/explore/repos`, 5) or the enrolled second factor (`/user/webauthn`, 6; `/user/two_factor`, 2). The
count redirecting to `/` was **zero**. One journey is visible end to end: callback →
`303 /user/webauthn` → `POST /user/webauthn/assertion 200` → `GET /explore/repos 200`.

## What the report actually caught

The walkthrough is in the log, one minute before the issue was filed, and it is the runbook entry
directly above the new one. At 20:47:49 the callback returned `/user/webauthn` and the login finished
on `/explore/repos` — the deep link worked. The reporter then deleted their WebAuthn credential
(`POST /user/settings/security/webauthn/delete`, 20:48:14), which dropped the next two logins onto
the TOTP page, where `POST /user/two_factor` was returning **500** — the retired-`SECRET_KEY` AEAD
failure already documented in the runbook, which that entry notes is usually surfaced by removing a
passkey. The login never completed, so the `/` in the browser was the **anonymous landing page**
(which Forgejo itself redirects `/` → `/explore` → `/explore/repos`), not a post-login destination.

A stalled second factor and a discarded destination look identical from the address bar. They are
distinguishable in one place, so the runbook entry carries the LogQL query and the reading: a
`Location` of `/user/webauthn` or `/user/two_factor` means the target is still pending, not lost.

## The Caddyfile comment was the load-bearing error

It claimed "VERIFIED that /user/oauth2/<source> does the same" from evidence for the cookie being
set — exactly the inference #291 warns against, sitting in the config that the issue suspected. It
now cites the completed-login evidence and the second-factor hop, and points at the runbook.

## Verification

Read-only throughout; nothing applied.

- Live header check: `/user/login?redirect_to=%2Fexplore%2Frepos` → `302` preserving the query →
  `/user/oauth2/Gitborg%20Auth?redirect_to=…` → `307` to Kanidm plus
  `set-cookie: redirect_to=%2Fexplore%2Frepos; Path=/`. Confirms Caddy's `{query}` pass-through.
- `SameSite=Lax` was ruled out as a cause without a test: the session cookie carries the same
  attributes and must reach the callback for PKCE/state to verify at all, so if Lax blocked the hop
  no OIDC login would ever succeed.
- `prettier -c` and `markdownlint-cli2` clean.

## Not done

No fresh login was performed — that needs a real credential, and the archived logins already answer
the question more completely than one more would. The reported symptom is therefore explained rather
than reproduced. Also worth noting the corollary the issue drew: `/user/login?local=1` remains the
only route to Forgejo's own login page, and it is visible in this same log window being used exactly
that way.

supernaut sammanfogade incheckning de41acd6d1 till main

2026-08-01 16:53:41 +00:00

supernaut tog bort grenen docs/oidc-redirect-verified

2026-08-01 16:53:41 +00:00

supernaut refererade denna ändringsförfrågan från en incheckning

2026-08-01 16:53:42 +00:00

docs(caddy,runbook): deep links do survive OIDC sign-in, with the evidence (#306)

supernaut refererade denna ändringsförfrågan från en incheckning

2026-08-03 09:41:35 +00:00

docs(caddy,runbook): deep links do survive OIDC sign-in, with the evidence (#306)

Logga in för att delta i denna konversation.

Inga granskare