sovrn

Experimental AT protocol email hosting

Auth: post-login return-to redirect (internal paths only)

closed
#0a27bf0 opened by agent Sep 9

Goal

Return the user to the page they were heading to before login, without introducing an open redirect.

Task

  • Session middleware redirects unauthenticated requests to /login?next=<original-path>.
  • Login flow carries next through to the OAuth callback.
  • Callback validates next is strictly an internal path before redirecting: must start with a single /, and reject //, /\, and any : or \ (blocks http://, protocol-relative, and backslash tricks). Default to / when absent or invalid.
  • Never redirect to a path outside the app.

Acceptance

Table-driven test for the next validator: - accept: /domains/x, / - reject: https://evil.example, //evil.example, /\\evil, javascript:alert(1)

Depends on: 40e284e (session middleware redirect) + 70790ed S4.

1 Comment

agent 00a72f7 Sep 10

Implementation complete

Post-login return-to redirect landed, with the validator as the single choke point. Not closing until you’ve reviewed.

Flow

  • SessionAuth.Middleware (internal/appview/ui/session.go) now 303s unauthenticated requests to /login?next=<escaped RequestURI> (path + query preserved).
  • GET /login passes ?next= into LoginPage(next) (handler.go, login.templ), which renders a hidden next field.
  • POST /oauth/login stashes next (form, then query fallback) in a short-lived sovrn_return_to cookie (HttpOnly; Secure; SameSite=Lax; Path=/; MaxAge=300). Cookie transport is required: indigo’s StartAuthFlow generates its own state with no passthrough, and CallbackURL is fixed in client metadata.
  • GET /oauth/callback one-shot consumes + clears the cookie and redirects to the validated target (still 302 Found); absent/invalid defaults to /. LogoutHandler still redirects to bare /login.

Hardening (internal/authbroker/redirect.go: SafeRedirectTarget)

Accept only strings starting with exactly one /; reject // and /\ prefixes; reject any : or \; re-check after PathUnescape (blocks /%2f%2fevil, /%5c, %3a); reject ASCII controls. Stored raw, validated once at consume time.

Tests

  • authbroker/redirect_test.go: acceptance table (accept /domains/x, /; reject https://evil.example, //evil.example, /\evil, javascript:alert(1), plus empty/bare-path/colon/backslash/encoded variants), cookie round-trip + HttpOnly + one-shot clear, evil-defaults-to-/, missing-defaults-to-/, and TestLoginHandlerStashesReturnToCookie (login 400s on garbage handle but still stashes the cookie, which pops to /domains/x).
  • ui: TestSessionAuthRedirectCarriesNext (/domains/d1?msg=x -> /login?next=...), TestLoginPagePreservesNext (hidden field rendered); existing /login expectations updated to /login?next=%2F.
  • go test ./... green, go vet ./... clean, gofmt clean. Regenerated login_templ.go via templ generate; reverted unrelated FileName-churn in other *_templ.go files.