amadan

oficina / home Public

Merge branch 'fix-keymail-oauth-continuation' into main

claude pushed by paul@keymail.dev d4b2eeb3d1ca6d0f384e40aa812eb2dcda2fa190 parent 6bf0944parent 9a4dcd7
8 files changed, +64 −13
  • internal/home/assertion_fresh_test.go +4 −2
  • internal/home/continuation.go +11 −0
  • internal/home/docs_chooser.go +1 −3
  • internal/home/docs_continue_test.go +1 −1
  • internal/home/keymail_continue_test.go +38 −0
  • internal/home/keymailstub_test.go +5 −3
  • internal/home/signin.go +1 −1
  • internal/home/templates/docs_continue.html +3 −3

CI passed — run details

diff --git a/internal/home/assertion_fresh_test.go b/internal/home/assertion_fresh_test.go
index 4be5d9c..3f789c1 100644
--- a/internal/home/assertion_fresh_test.go
+++ b/internal/home/assertion_fresh_test.go
@@ -127,9 +127,11 @@ func TestOrdinaryIntentKeepsKeymailAndInvalidIntentCannotRequestFreshness(t *tes
intent.Value = base64.RawURLEncoding.EncodeToString(sealed)
}
r := exchangeRequest(p.credential.App, "POST", Origin+"/signin", Origin, url.Values{"address": {sealtest.CanaryAddress}}, cookies...)
- if r.Code != 303 || r.Header().Get("Location") == "/signin/sent" {
- t.Fatal("ordinary or invalid intent forced mail", r.Code)
+ _, links := continuationTargets(t, r.Body.String())
+ if len(links) != 1 || !strings.Contains(links[0], "/oauth/authorize?") {
+ t.Fatal("ordinary or invalid intent lost Keymail continuation", r.Code)
}
+ assertContinuationPage(t, r, links[0])
})
}
}
diff --git a/internal/home/continuation.go b/internal/home/continuation.go
new file mode 100644
index 0000000..358ac78
--- /dev/null
+++ b/internal/home/continuation.go
@@ -0,0 +1,11 @@
+package home
+
+import "net/http"
+
+// renderContinuation ends the submitted form response before external navigation.
+// Callers supply a validated app destination or the sign-in flow's provider URL.
+func (a *App) renderContinuation(w http.ResponseWriter, destination, title string) {
+ w.Header().Set("Cache-Control", "no-store")
+ w.Header().Set("Referrer-Policy", "no-referrer")
+ a.render(w, http.StatusOK, "docs_continue.html", struct{ Destination, Title string }{destination, title})
+}
diff --git a/internal/home/docs_chooser.go b/internal/home/docs_chooser.go
index 53c4136..b4513e5 100644
--- a/internal/home/docs_chooser.go
+++ b/internal/home/docs_chooser.go
@@ -153,7 +153,5 @@ func (c *docsChooser) continueToDocs(w http.ResponseWriter, choice directorydata
// Callers supply either a validated stored app origin plus the fixed app path,
// or the validated directory origin plus the fixed sign-in continuation path.
func (c *docsChooser) renderContinuation(w http.ResponseWriter, destination string) {
- w.Header().Set("Cache-Control", "no-store")
- w.Header().Set("Referrer-Policy", "no-referrer")
- c.app.render(w, http.StatusOK, "docs_continue.html", struct{ Destination string }{destination})
+ c.app.renderContinuation(w, destination, "Continue to Docs")
}
diff --git a/internal/home/docs_continue_test.go b/internal/home/docs_continue_test.go
index 549afd2..17aaae3 100644
--- a/internal/home/docs_continue_test.go
+++ b/internal/home/docs_continue_test.go
@@ -89,7 +89,7 @@ func TestDocsContinueRendererEscapesIdenticalTargets(t *testing.T) {
// Exercise real template contexts independently of the fixed runtime URL.
want := "https://alpha.docs.oficina.works/auth/start?return_to=%2F&probe=%22%3C"
r := httptest.NewRecorder()
- f.p.directory.render(r, 200, "docs_continue.html", struct{ Destination string }{want})
+ f.p.directory.render(r, 200, "docs_continue.html", struct{ Destination, Title string }{want, "Continue to Docs"})
refresh, links := continuationTargets(t, r.Body.String())
if len(refresh) != 1 || refresh[0] != "0;url="+want || len(links) != 1 || links[0] != want || !strings.Contains(r.Body.String(), "&probe=") {
t.Fatal(refresh, links, r.Body.String())
diff --git a/internal/home/keymail_continue_test.go b/internal/home/keymail_continue_test.go
new file mode 100644
index 0000000..e4422e4
--- /dev/null
+++ b/internal/home/keymail_continue_test.go
@@ -0,0 +1,38 @@
+package home
+
+import (
+ "net/http"
+ "strings"
+ "testing"
+)
+
+func TestKeymailSignInContinuesWithoutExternalPostRedirect(t *testing.T) {
+ app := newTestApp(t)
+ const addr = "person@keymail.invalid"
+ stub := newKeymailStub(t, app, addr)
+ policy := app.SecurityPolicy()
+ rec := signIn(t, app, addr)
+ if rec.Code != http.StatusOK {
+ t.Fatalf("POST sign-in must end before external navigation: %d", rec.Code)
+ }
+ refresh, links := continuationTargets(t, rec.Body.String())
+ if len(links) != 1 || len(refresh) != 1 || !strings.HasPrefix(links[0], stub.srv.URL+"/") {
+ t.Fatal("missing exact provider continuation")
+ }
+ assertContinuationPage(t, rec, links[0])
+ if app.SecurityPolicy() != policy || strings.Contains(policy, stub.host) {
+ t.Fatal("CSP broadened to provider")
+ }
+ if sessionCookie(rec) != nil {
+ t.Fatal("continuation minted a session")
+ }
+ var pending *http.Cookie
+ for _, c := range rec.Result().Cookies() {
+ if c.Name == pendingCookie {
+ pending = c
+ }
+ }
+ if pending == nil || pending.Value == "" || !pending.HttpOnly || !pending.Secure || pending.SameSite != http.SameSiteLaxMode {
+ t.Fatal("pending cookie lost")
+ }
+}
diff --git a/internal/home/keymailstub_test.go b/internal/home/keymailstub_test.go
index 1cde613..b3f82ba 100644
--- a/internal/home/keymailstub_test.go
+++ b/internal/home/keymailstub_test.go
@@ -134,10 +134,12 @@ func writeJSON(t *testing.T, w http.ResponseWriter, v any) {
func beginKeymail(t *testing.T, app *testApp, address string) (*http.Cookie, string) {
t.Helper()
rec := signIn(t, app, address)
- if rec.Code != http.StatusSeeOther {
- t.Fatalf("POST /signin = %d, want 303", rec.Code)
+ _, links := continuationTargets(t, rec.Body.String())
+ if len(links) != 1 {
+ t.Fatal("missing provider continuation")
}
- loc := rec.Header().Get("Location")
+ loc := links[0]
+ assertContinuationPage(t, rec, loc)
if !strings.Contains(loc, "/oauth/authorize") {
t.Fatalf("sign-in went to %q, which is not the keymail authorize URL — "+
"classification did not take the keymail branch", loc)
diff --git a/internal/home/signin.go b/internal/home/signin.go
index e72329f..d66be07 100644
--- a/internal/home/signin.go
+++ b/internal/home/signin.go
@@ -210,7 +210,7 @@ func (a *App) handleSigninBegin(w http.ResponseWriter, r *http.Request) {
if next.Kind == signin.NextKeymail {
a.setPending(w, next.Pending)
- http.Redirect(w, r, next.Redirect, http.StatusSeeOther)
+ a.renderContinuation(w, next.Redirect, "Continue to Keymail")
return
}
http.Redirect(w, r, "/signin/sent", http.StatusSeeOther)
diff --git a/internal/home/templates/docs_continue.html b/internal/home/templates/docs_continue.html
index ff03281..89695a2 100644
--- a/internal/home/templates/docs_continue.html
+++ b/internal/home/templates/docs_continue.html
@@ -1,6 +1,6 @@
-{{define "title"}}Continue to Docs{{end}}
+{{define "title"}}{{.Title}}{{end}}
{{define "head"}}<meta http-equiv="refresh" content="0;url={{.Destination}}">{{end}}
{{define "content"}}
-<h1>Continue to Docs</h1>
-<p><a href="{{.Destination}}" referrerpolicy="no-referrer">Continue to Docs</a></p>
+<h1>{{.Title}}</h1>
+<p><a href="{{.Destination}}" referrerpolicy="no-referrer">{{.Title}}</a></p>
{{end}}