amadan

oficina / home Public

Carry allowlisted credential destination through existing sign-in gate

Paul Campbell pushed by paul@keymail.dev 911ff1ead71f0626b7cf2689bc939a97f0cd1bcb parent efa03d9
4 files changed, +271 −2
  • internal/home/signin.go +5 −2
  • internal/home/signin_return.go +68 −0
  • internal/home/signin_return_factor_test.go +98 −0
  • internal/home/signin_return_test.go +100 −0

CI passed — run details

Intent

full ledger: fix/credential-return

diff --git a/internal/home/signin.go b/internal/home/signin.go
index fcae68f..83be359 100644
--- a/internal/home/signin.go
+++ b/internal/home/signin.go
@@ -155,6 +155,7 @@ func clientIP(r *http.Request) string {
// handleSigninForm renders the address form.
func (a *App) handleSigninForm(w http.ResponseWriter, r *http.Request) {
+ rememberReturn(w, r)
a.render(w, http.StatusOK, "signin.html", map[string]any{})
}
@@ -319,7 +320,9 @@ func (a *App) completeSignIn(w http.ResponseWriter, r *http.Request, id signin.I
//
// Gate redirects and reports true when it has taken over. An account
// with no passkey passes through untouched.
- gated, err := a.passkeys.Gate(w, r, sess)
+ target := readReturn(r)
+ writeReturn(w, "/")
+ gated, err := a.passkeys.Gate(w, requestWithReturn(r, target), sess)
if err != nil {
a.logger.Error("passkey gate", "person", ref, "err", err)
a.signinFailed(w)
@@ -334,7 +337,7 @@ func (a *App) completeSignIn(w http.ResponseWriter, r *http.Request, id signin.I
a.signinFailed(w)
return
}
- http.Redirect(w, r, "/", http.StatusSeeOther)
+ http.Redirect(w, r, target, http.StatusSeeOther)
}
// signinFailed is the one answer every failed redemption gets. One page,
diff --git a/internal/home/signin_return.go b/internal/home/signin_return.go
new file mode 100644
index 0000000..7464359
--- /dev/null
+++ b/internal/home/signin_return.go
@@ -0,0 +1,68 @@
+package home
+
+import (
+ "net/http"
+ "net/url"
+ "time"
+)
+
+const returnCookie = "__Host-oficina-return"
+const returnTTL = 10 * time.Minute
+
+func credentialReturn(value string) string {
+ switch value {
+ case "/passkey/enrol", "/passkey/codes":
+ return value
+ }
+ return "/"
+}
+
+func writeReturn(w http.ResponseWriter, value string) {
+ maxAge := int(returnTTL.Seconds())
+ if value == "/" {
+ value = ""
+ maxAge = -1
+ }
+ http.SetCookie(w, &http.Cookie{Name: returnCookie, Value: value, Path: "/", MaxAge: maxAge, Secure: true, HttpOnly: true, SameSite: http.SameSiteLaxMode})
+}
+
+func readReturn(r *http.Request) string {
+ value := ""
+ count := 0
+ for _, c := range r.Cookies() {
+ if c.Name == returnCookie {
+ value = c.Value
+ count++
+ }
+ }
+ if count != 1 {
+ return "/"
+ }
+ return credentialReturn(value)
+}
+
+func rememberReturn(w http.ResponseWriter, r *http.Request) {
+ q, err := url.ParseQuery(r.URL.RawQuery)
+ if err != nil {
+ writeReturn(w, "/")
+ return
+ }
+ values, present := q["return_to"]
+ if !present && q.Get("force") == "1" && readReturn(r) != "/" {
+ return
+ }
+ target := "/"
+ if len(values) == 1 {
+ target = credentialReturn(values[0])
+ }
+ writeReturn(w, target)
+}
+
+// Gate reads FormValue. Give it only validated navigation intent, so neither
+// the callback query nor the magic-link POST can override the cookie.
+func requestWithReturn(r *http.Request, target string) *http.Request {
+ clone := r.Clone(r.Context())
+ clone.Form = url.Values{"return_to": {target}}
+ clone.PostForm = url.Values{"return_to": {target}}
+ return clone
+}
diff --git a/internal/home/signin_return_factor_test.go b/internal/home/signin_return_factor_test.go
new file mode 100644
index 0000000..09036f0
--- /dev/null
+++ b/internal/home/signin_return_factor_test.go
@@ -0,0 +1,98 @@
+package home
+
+import (
+ "encoding/base64"
+ "encoding/json"
+ "net/http"
+ "net/url"
+ "strings"
+ "testing"
+
+ "github.com/carlosframework/rastrillo/webauthn/authtest"
+)
+
+// Mutation-checked 2026-09-08: bypassing Gate or passing the original
+// request allows factor bypass or hostile FormValue precedence and fails.
+func TestCredentialReturnSecondFactors(t *testing.T) {
+ for _, first := range []string{"magic", "keymail"} {
+ for _, factor := range []string{"passkey", "recovery"} {
+ t.Run(first+factor, func(t *testing.T) {
+ app := newTestApp(t)
+ addr := "person@example.invalid"
+ var session *http.Cookie
+ if first == "keymail" {
+ addr = "person@keymail.invalid"
+ newKeymailStub(t, app, addr)
+ p, state := beginKeymail(t, app, addr)
+ session = sessionCookie(callback(t, app, "code-1", state, p))
+ } else {
+ session = signInFully(t, app, addr)
+ }
+ authenticator := enrol(t, app, session)
+ codes, err := app.passkeys.RegenerateRecoveryCodes(subjectOf(t, app, session))
+ if err != nil {
+ t.Fatal(err)
+ }
+ intent := intentFrom(returnRequest(app, "GET", "/signin?return_to=/passkey/enrol", nil))
+ var gatedCookies []*http.Cookie
+ if first == "keymail" {
+ p, state := beginKeymail(t, app, addr)
+ got := returnRequest(app, "GET", callbackPath+"?code=code-2&state="+url.QueryEscape(state)+"&return_to=/passkey/codes", nil, p, intent)
+ if sessionCookie(got) != nil || got.Header().Get("Location") != confirmPath {
+ t.Fatal("Keymail bypassed second factor")
+ }
+ gatedCookies = got.Result().Cookies()
+ } else {
+ signIn(t, app, addr)
+ got := returnRequest(app, "POST", linkPath+"?return_to=/passkey/codes", url.Values{"token": {capturedLink(t, app)}, "return_to": {"//evil.invalid"}}, intent)
+ if sessionCookie(got) != nil || got.Header().Get("Location") != confirmPath {
+ t.Fatal("magic link bypassed second factor")
+ }
+ gatedCookies = got.Result().Cookies()
+ }
+ var pending *http.Cookie
+ for _, c := range gatedCookies {
+ if strings.Contains(c.Name, "passkey") && c.Value != "" {
+ pending = c
+ }
+ }
+ if pending == nil {
+ t.Fatal("no pending second factor")
+ }
+ denied := returnRequest(app, "POST", "/passkey/signin/recovery", url.Values{"code": {"invalid"}}, pending)
+ if sessionCookie(denied) != nil {
+ t.Fatal("invalid recovery minted session")
+ }
+ if factor == "recovery" {
+ got := returnRequest(app, "POST", "/passkey/signin/recovery", url.Values{"code": {codes[0]}, "return_to": {"/passkey/codes"}}, pending)
+ if got.Header().Get("Location") != "/passkey/enrol" || sessionCookie(got) == nil {
+ t.Fatalf("recovery continuation=%q status=%d", got.Header().Get("Location"), got.Code)
+ }
+ if dest := returnRequest(app, "GET", "/passkey/enrol", nil, sessionCookie(got)); dest.Code != 200 {
+ t.Fatalf("recovery freshness=%d", dest.Code)
+ }
+ } else {
+ challenge := challengeFrom(t, postJSON(t, app, "/passkey/signin/begin", pending, nil))
+ clientData, authData, sig, err := authenticator.Get(challenge, authtest.Options{RPID: app.credentialHost, Origin: Origin})
+ if err != nil {
+ t.Fatal(err)
+ }
+ b64 := base64.RawURLEncoding.EncodeToString
+ got := postJSON(t, app, "/passkey/signin/finish", pending, map[string]string{"id": b64(authenticator.CredID), "clientDataJSON": b64(clientData), "authenticatorData": b64(authData), "signature": b64(sig)})
+ var response struct {
+ To string `json:"to"`
+ }
+ if err := json.Unmarshal(got.Body.Bytes(), &response); err != nil {
+ t.Fatal(err)
+ }
+ if response.To != "/passkey/enrol" || sessionCookie(got) == nil {
+ t.Fatalf("passkey continuation=%q status=%d", response.To, got.Code)
+ }
+ if dest := returnRequest(app, "GET", "/passkey/enrol", nil, sessionCookie(got)); dest.Code != 200 {
+ t.Fatalf("passkey freshness=%d", dest.Code)
+ }
+ }
+ })
+ }
+ }
+}
diff --git a/internal/home/signin_return_test.go b/internal/home/signin_return_test.go
index f3e5715..21e9ef4 100644
--- a/internal/home/signin_return_test.go
+++ b/internal/home/signin_return_test.go
@@ -33,6 +33,8 @@ func intentFrom(w *httptest.ResponseRecorder) *http.Cookie {
return nil
}
+// Mutation-checked 2026-09-08: discarding validated intent restores the
+// original wrong destination; trusting cookie values fails hostile cases.
func TestCredentialReturnMagicLink(t *testing.T) {
for _, target := range []string{"/passkey/enrol", "/passkey/codes"} {
t.Run(target, func(t *testing.T) {
@@ -59,3 +61,101 @@ func TestCredentialReturnMagicLink(t *testing.T) {
})
}
}
+
+func TestCredentialReturnKeymailAndFallback(t *testing.T) {
+ for _, fallback := range []bool{false, true} {
+ t.Run(string(rune('0'+boolInt(fallback))), func(t *testing.T) {
+ app := newTestApp(t)
+ newKeymailStub(t, app, "person@keymail.invalid")
+ intent := intentFrom(returnRequest(app, "GET", "/signin?return_to=%2Fpasskey%2Fcodes", nil))
+ pending, state := beginKeymail(t, app, "person@keymail.invalid")
+ code := "code-1"
+ if fallback {
+ state = "invalid-state"
+ }
+ got := callback(t, app, code, state, pending, intent)
+ if fallback {
+ if got.Header().Get("Location") != "/signin?force=1" || sessionCookie(got) != nil {
+ t.Fatal("invalid Keymail callback did not fall back")
+ }
+ form := returnRequest(app, "GET", got.Header().Get("Location"), nil, intent)
+ if changed := intentFrom(form); changed != nil {
+ intent = changed
+ }
+ returnRequest(app, "POST", "/signin?force=1", url.Values{"address": {"person@keymail.invalid"}}, intent)
+ got = returnRequest(app, "POST", linkPath, url.Values{"token": {capturedLink(t, app)}}, intent)
+ }
+ if got.Header().Get("Location") != "/passkey/codes" || sessionCookie(got) == nil {
+ t.Fatalf("continuation=%q status=%d", got.Header().Get("Location"), got.Code)
+ }
+ })
+ }
+}
+
+func boolInt(b bool) int {
+ if b {
+ return 1
+ }
+ return 0
+}
+
+func TestCredentialReturnHostileAndMissingIntent(t *testing.T) {
+ for _, value := range []string{"https://evil.invalid/", "//evil.invalid/", "/passkey/codes?x=1", "/passkey/enrol/", "%2f%2fevil.invalid", "/passkey/codes\r\nLocation: x", "/"} {
+ t.Run(value, func(t *testing.T) {
+ app := newTestApp(t)
+ c := intentFrom(returnRequest(app, "GET", "/signin?return_to="+url.QueryEscape(value), nil))
+ if c == nil || c.MaxAge >= 0 {
+ t.Fatal("unsafe target retained")
+ }
+ })
+ }
+ for _, query := range []string{"", "?return_to=/passkey/enrol&return_to=/passkey/codes", "?return_to=%zz"} {
+ app := newTestApp(t)
+ old := &http.Cookie{Name: returnCookie, Value: "/passkey/codes"}
+ c := intentFrom(returnRequest(app, "GET", "/signin"+query, nil, old))
+ if c == nil || c.MaxAge >= 0 {
+ t.Fatal("ambiguous/plain sign-in retained old intent")
+ }
+ }
+ for _, cookies := range [][]*http.Cookie{nil, {{Name: returnCookie, Value: "https://evil.invalid"}}, {{Name: returnCookie, Value: "/passkey/codes"}, {Name: returnCookie, Value: "/passkey/enrol"}}} {
+ app := newTestApp(t)
+ signIn(t, app, "person@example.invalid")
+ got := returnRequest(app, "POST", linkPath+"?return_to=/passkey/codes", url.Values{"token": {capturedLink(t, app)}, "return_to": {"/passkey/enrol"}}, cookies...)
+ if got.Header().Get("Location") != "/" || sessionCookie(got) == nil {
+ t.Fatalf("missing/hostile cookie continuation=%q", got.Header().Get("Location"))
+ }
+ }
+}
+
+func TestCredentialReturnCookieGrantsNothing(t *testing.T) {
+ app := newTestApp(t)
+ c := intentFrom(returnRequest(app, "GET", "/signin?return_to=/passkey/codes", nil))
+ if c == nil || !c.Secure || !c.HttpOnly || c.Domain != "" || c.Path != "/" || c.SameSite != http.SameSiteLaxMode || c.MaxAge != 600 {
+ t.Fatalf("invalid intent cookie attributes: %+v", c)
+ }
+ if got := returnRequest(app, "GET", "/passkey/codes", nil, c); got.Code == 200 {
+ t.Fatal("intent granted access without session")
+ }
+ signIn(t, app, "person@example.invalid")
+ got := returnRequest(app, "POST", linkPath, url.Values{"token": {capturedLink(t, app)}}, c)
+ if gone := intentFrom(got); gone == nil || gone.MaxAge >= 0 {
+ t.Fatal("successful completion did not clear intent")
+ }
+}
+
+// Mutation-checked 2026-09-08: replacing AuthTime with time.Now grants
+// access after an old gesture and fails this freshness check.
+func TestCredentialReturnDoesNotRefreshOldKeymailGesture(t *testing.T) {
+ app := newTestApp(t)
+ stub := newKeymailStub(t, app, "person@keymail.invalid")
+ stub.authTime = time.Now().Add(-24 * time.Hour)
+ intent := intentFrom(returnRequest(app, "GET", "/signin?return_to=/passkey/codes", nil))
+ pending, state := beginKeymail(t, app, "person@keymail.invalid")
+ got := callback(t, app, "code-1", state, pending, intent)
+ if sessionCookie(got) == nil || got.Header().Get("Location") != "/passkey/codes" {
+ t.Fatal("verified sign-in lost navigation")
+ }
+ if dest := returnRequest(app, "GET", "/passkey/codes", nil, sessionCookie(got)); dest.Code == 200 {
+ t.Fatal("navigation intent refreshed stale credential authentication")
+ }
+}