amadan

oficina / home Public

Merge branch 'fix/handoff-diagnostics' into main

claude pushed by paul@keymail.dev 66c6c56abab7b99633f95dc5d822160e3f31c4f4 parent cd476a2parent fde9070
5 files changed, +185 −12
  • internal/home/directory.go +1 −1
  • internal/home/exchange.go +11 −11
  • internal/home/handoff_failure.go +77 −0
  • internal/home/handoff_failure_test.go +90 −0
  • internal/home/templates/handoff_failed.html +6 −0

CI cancelled — superseded by a newer push — run details

diff --git a/internal/home/directory.go b/internal/home/directory.go
index a84aeac..9ce7ec9 100644
--- a/internal/home/directory.go
+++ b/internal/home/directory.go
@@ -48,7 +48,7 @@ func newDirectory(cfg Runtime, d *db.DB, logger *slog.Logger) (*App, error) {
}, Complete: func(w http.ResponseWriter, r *http.Request, c assertion.Claims) error {
return a.sessions.SignIn(w, r, sessions.Session{Subject: c.Subject, Method: "assertion", AuthTime: time.Unix(c.AuthTime, 0)})
}, Fail: func(w http.ResponseWriter, r *http.Request, status int) {
- a.render(w, status, "signin_failed.html", map[string]any{})
+ a.failDirectoryHandoff(w, r, status)
}})
if err != nil {
return nil, err
diff --git a/internal/home/exchange.go b/internal/home/exchange.go
index e139590..6b950e9 100644
--- a/internal/home/exchange.go
+++ b/internal/home/exchange.go
@@ -28,7 +28,7 @@ func (a *App) handleAuthorize(w http.ResponseWriter, r *http.Request) {
maxAge := 0
if values, present := q["max_age"]; present {
if len(values) != 1 || values[0] != "300" {
- a.signinFailed(w)
+ a.failAssertion(w, authorizeAge)
return
}
maxAge = 300
@@ -38,7 +38,7 @@ func (a *App) handleAuthorize(w http.ResponseWriter, r *http.Request) {
wantFields++
}
if err != nil || len(q) != wantFields || len(q["consumer"]) != 1 || len(q["nonce"]) != 1 || !handoff.ValidNonce(q.Get("nonce")) || a.runtime.consumers[q.Get("consumer")] == "" {
- a.signinFailed(w)
+ a.failAssertion(w, authorizeRequest)
return
}
intent := assertionIntent{Consumer: q.Get("consumer"), Nonce: q.Get("nonce"), Expires: time.Now().Unix() + 600, MaxAge: maxAge}
@@ -49,12 +49,12 @@ func (a *App) handleAuthorize(w http.ResponseWriter, r *http.Request) {
}
b, err := json.Marshal(intent)
if err != nil {
- a.signinFailed(w)
+ a.failAssertion(w, authorizeEncode)
return
}
sealed, err := a.secrets.Sealer.Seal("assertion.intent", b)
if err != nil {
- a.signinFailed(w)
+ a.failAssertion(w, authorizeSeal)
return
}
http.SetCookie(w, &http.Cookie{Name: assertionIntentCookie, Value: base64.RawURLEncoding.EncodeToString(sealed), Path: "/", MaxAge: 600, Secure: true, HttpOnly: true, SameSite: http.SameSiteLaxMode})
@@ -65,23 +65,23 @@ func (a *App) handleAssertion(w http.ResponseWriter, r *http.Request) {
// no-referrer would turn the cross-origin form POST's Origin into null.
w.Header().Set("Referrer-Policy", "strict-origin-when-cross-origin")
if r.URL.RawQuery != "" {
- a.signinFailed(w)
+ a.failAssertion(w, assertionQuery)
return
}
now := time.Now()
intent, ok := a.readAssertionIntent(r, now)
if !ok {
- a.signinFailed(w)
+ a.failAssertion(w, assertionIntentInvalid)
return
}
target, ok := a.runtime.consumers[intent.Consumer]
if !ok {
- a.signinFailed(w)
+ a.failAssertion(w, assertionConsumer)
return
}
session, ok := sessions.Current(r)
if !ok {
- a.signinFailed(w)
+ a.failAssertion(w, assertionSession)
return
}
authTime := session.AuthTime
@@ -89,7 +89,7 @@ func (a *App) handleAssertion(w http.ResponseWriter, r *http.Request) {
authTime = session.At
}
if intent.Subject != "" && intent.Subject != session.Subject {
- a.signinFailed(w)
+ a.failAssertion(w, assertionSubject)
return
}
if intent.MaxAge != 0 && (!sessions.Fresh(session, time.Duration(intent.MaxAge)*time.Second, now) || authTime.After(now)) {
@@ -98,12 +98,12 @@ func (a *App) handleAssertion(w http.ResponseWriter, r *http.Request) {
}
id, err := assertion.RandomID()
if err != nil {
- a.signinFailed(w)
+ a.failAssertion(w, assertionRandom)
return
}
token, err := assertion.Sign(a.runtime.signer, assertion.Claims{Version: 1, Issuer: a.origins.Credential, Audience: target, Subject: session.Subject, KeyID: a.runtime.keyID, ID: id, Nonce: intent.Nonce, IssuedAt: now.Unix(), ExpiresAt: now.Unix() + 120, AuthTime: authTime.Unix()})
if err != nil {
- a.signinFailed(w)
+ a.failAssertion(w, assertionSign)
return
}
http.SetCookie(w, &http.Cookie{Name: assertionIntentCookie, Value: "", Path: "/", MaxAge: -1, Secure: true, HttpOnly: true, SameSite: http.SameSiteLaxMode})
diff --git a/internal/home/handoff_failure.go b/internal/home/handoff_failure.go
new file mode 100644
index 0000000..555eea6
--- /dev/null
+++ b/internal/home/handoff_failure.go
@@ -0,0 +1,77 @@
+package home
+
+import (
+ "net/http"
+
+ "amadan.net/oficina/home/handoff"
+)
+
+type handoffFailureCode uint8
+
+const (
+ authorizeAge handoffFailureCode = iota
+ authorizeRequest
+ authorizeEncode
+ authorizeSeal
+ assertionQuery
+ assertionIntentInvalid
+ assertionConsumer
+ assertionSession
+ assertionSubject
+ assertionRandom
+ assertionSign
+)
+
+// Only this fixed vocabulary reaches logs; no request or error is accepted.
+func (a *App) failAssertion(w http.ResponseWriter, code handoffFailureCode) {
+ stage, reason := "assertion", "unknown"
+ switch code {
+ case authorizeAge:
+ stage, reason = "authorize", "invalid_age"
+ case authorizeRequest:
+ stage, reason = "authorize", "invalid_request"
+ case authorizeEncode:
+ stage, reason = "authorize", "intent_encode"
+ case authorizeSeal:
+ stage, reason = "authorize", "intent_seal"
+ case assertionQuery:
+ reason = "unexpected_query"
+ case assertionIntentInvalid:
+ reason = "invalid_intent"
+ case assertionConsumer:
+ reason = "unknown_consumer"
+ case assertionSession:
+ reason = "missing_session"
+ case assertionSubject:
+ reason = "subject_mismatch"
+ case assertionRandom:
+ reason = "random_id"
+ case assertionSign:
+ reason = "signing"
+ }
+ a.logger.Warn("home handoff failed", "stage", stage, "reason", reason)
+ a.renderHandoffFailure(w, http.StatusBadRequest)
+}
+
+func (a *App) failDirectoryHandoff(w http.ResponseWriter, r *http.Request, status int) {
+ stage := "directory_unknown"
+ switch r.URL.Path {
+ case handoff.StartPath:
+ stage = "directory_start"
+ case handoff.CallbackPath:
+ stage = "directory_callback"
+ case handoff.FinishPath:
+ stage = "directory_finish"
+ }
+ reason := "refused"
+ if status == http.StatusServiceUnavailable {
+ reason = "unavailable"
+ }
+ a.logger.Warn("home handoff failed", "stage", stage, "reason", reason)
+ a.renderHandoffFailure(w, status)
+}
+
+func (a *App) renderHandoffFailure(w http.ResponseWriter, status int) {
+ w.Header().Set("Cache-Control", "no-store")
+ a.render(w, status, "handoff_failed.html", map[string]any{"HomeURL": a.origins.Directory})
+}
diff --git a/internal/home/handoff_failure_test.go b/internal/home/handoff_failure_test.go
new file mode 100644
index 0000000..092f393
--- /dev/null
+++ b/internal/home/handoff_failure_test.go
@@ -0,0 +1,90 @@
+package home
+
+import (
+ "net/http"
+ "net/url"
+ "strings"
+ "testing"
+
+ "amadan.net/oficina/home/handoff"
+ "amadan.net/oficina/home/internal/home/sealtest"
+)
+
+func TestHandoffFailuresHaveFixedPrivateDiagnostics(t *testing.T) {
+ p := newExchangePair(t)
+ credential := signInFully(t, p.credential, sealtest.CanaryAddress)
+ const secret = "private-address@example.invalid"
+ cases := []struct {
+ app *App
+ method, path, origin, stage, reason string
+ form url.Values
+ cookies []*http.Cookie
+ }{
+ {p.credential.App, "GET", Origin + "/authorize?consumer=" + secret + "&nonce=" + secret, "", "authorize", "invalid_request", nil, nil},
+ {p.credential.App, "GET", Origin + "/authorize/resume?private=" + secret, "", "assertion", "unexpected_query", nil, nil},
+ {p.credential.App, "GET", Origin + "/authorize/resume", "", "assertion", "invalid_intent", nil, []*http.Cookie{{Name: assertionIntentCookie, Value: secret}}},
+ {p.directory, "GET", directoryOrigin + handoff.StartPath + "?return_to=https://" + secret, "", "directory_start", "refused", nil, nil},
+ {p.directory, "POST", directoryOrigin + handoff.CallbackPath, Origin, "directory_callback", "refused", url.Values{"assertion": {secret}}, nil},
+ {p.directory, "GET", directoryOrigin + handoff.FinishPath, "", "directory_finish", "refused", nil, []*http.Cookie{{Name: "__Host-oficina-assertion-inbox", Value: secret}}},
+ }
+ for _, tc := range cases {
+ p.logs.Reset()
+ if tc.app == p.credential.App {
+ tc.cookies = append(tc.cookies, credential)
+ }
+ response := exchangeRequest(tc.app, tc.method, tc.path, tc.origin, tc.form, tc.cookies...)
+ if response.Code != 400 || response.Header().Get("Cache-Control") != "no-store" {
+ t.Fatal(response.Code, response.Header())
+ }
+ body := response.Body.String()
+ if !strings.Contains(body, "Could not finish signing in") || strings.Contains(body, "fifteen minutes") || !strings.Contains(body, `href="`+directoryOrigin+`"`) || strings.Contains(body, secret) {
+ t.Fatal("wrong handoff failure page", body)
+ }
+ log := p.logs.String()
+ if !strings.Contains(log, "stage="+tc.stage) || !strings.Contains(log, "reason="+tc.reason) || strings.Count(strings.TrimSpace(log), "\n") != 0 {
+ t.Fatal("wrong diagnostic", log)
+ }
+ for _, forbidden := range []string{secret, "nonce=", "consumer=", "https://", "token=", "cookie=", "subject="} {
+ if strings.Contains(log, forbidden) {
+ t.Fatal("diagnostic leaked request data", forbidden)
+ }
+ }
+ }
+ p.logs.Reset()
+ failedLink := exchangeRequest(p.credential.App, "POST", Origin+linkPath, Origin, url.Values{"token": {secret}})
+ if failedLink.Code != 400 || !strings.Contains(failedLink.Body.String(), "Sign-in links work once and expire after fifteen minutes") || strings.Contains(p.logs.String(), "home handoff failed") {
+ t.Fatal("magic-link failure changed", failedLink.Code)
+ }
+}
+
+func TestDirectorySubjectConflictLogsStageWithoutSwitchingIdentity(t *testing.T) {
+ f := newMountedChooser(t)
+ p := f.p
+ previous := f.session(t, chooserOther)
+ credential := signInFully(t, p.credential, sealtest.CanaryAddress)
+ pending, intent := p.start(t)
+ token, _ := p.mint(t, intent, credential)
+ callback := exchangeRequest(p.directory, "POST", directoryOrigin+handoff.CallbackPath, Origin, url.Values{"assertion": {token}})
+ if callback.Code != 303 {
+ t.Fatal(callback.Code)
+ }
+ inbox := namedCookie(t, callback, "__Host-oficina-assertion-inbox")
+ p.logs.Reset()
+ finish := exchangeRequest(p.directory, "GET", directoryOrigin+handoff.FinishPath, "", nil, pending, inbox, previous)
+ if finish.Code != 400 || sessionCookie(finish) != nil {
+ t.Fatal("conflicting identity accepted", finish.Code)
+ }
+ log := p.logs.String()
+ if !strings.Contains(log, "stage=directory_finish reason=refused") {
+ t.Fatal("missing finish diagnostic", log)
+ }
+ for _, secret := range []string{chooserOther, token, pending.Value, inbox.Value, sealtest.CanaryAddress, subjectOf(t, p.credential, credential)} {
+ if strings.Contains(log, secret) {
+ t.Fatal("identity or token leaked")
+ }
+ }
+ page := exchangeRequest(p.directory, "GET", directoryOrigin+"/", "", nil, previous)
+ if page.Code != 200 {
+ t.Fatal("previous session was changed", page.Code)
+ }
+}
diff --git a/internal/home/templates/handoff_failed.html b/internal/home/templates/handoff_failed.html
new file mode 100644
index 0000000..0017972
--- /dev/null
+++ b/internal/home/templates/handoff_failed.html
@@ -0,0 +1,6 @@
+{{define "title"}}Could not finish signing in · Oficina{{end}}
+{{define "content"}}
+<h1>Could not finish signing in</h1>
+<p>Return to Home and open the app again.</p>
+<p><a href="{{.HomeURL}}">Back to Home</a></p>
+{{end}}