amadan
Sign-in does not shortcut a person who is no longer admitted
Astra P3 on #23 (routes.go:45). With Home on, connect's /signin sends a signed-in session straight to where it was headed. A session whose person is no longer admitted (removed from the team, or a Home link to a deleted person) went there too, and requirePerson answered with a 404 that had no way out. homeSignin asks requirePerson's question first: not admitted, the session ends and /signin is asked again without it, which starts a fresh exchange, as sheets' homeRequire and docs' admission check do. A link-store failure is a 503, never a sign-out. TestSigninDoesNotShortcutARevokedPerson (a removed member) and the new step in TestHomeRouterRetainsExistingCalendarIdentity (a deleted Home person) both failed before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4 files changed,
+64
−1
internal/calendar/home.go+34 −0internal/calendar/home_consumer_test.go+21 −0internal/calendar/home_router_test.go+8 −0internal/calendar/routes.go+1 −1
diff --git a/internal/calendar/home.go b/internal/calendar/home.go| index b60a4c1..7921c62 100644 |
| --- a/internal/calendar/home.go |
| +++ b/internal/calendar/home.go |
| @@ -101,3 +101,37 @@ func (a *App) homePerson(r *http.Request) (Person, error) { |
| } |
| return person, nil |
| } |
| + |
| +// homeSignin is GET /signin with Home on. connect's Signin shortcuts a |
| +// signed-in person straight to where they were headed; but a session whose |
| +// person is no longer admitted (removed from the team, or a Home link to a |
| +// deleted person) is not signed in for Calendar, and the shortcut would end |
| +// on requirePerson's 404 with nowhere to go. So /signin first asks the same |
| +// question requirePerson does. Not admitted: the session ends here and |
| +// /signin is asked again without it, which starts a fresh exchange (as |
| +// sheets' homeRequire and docs' admission check do). A storage failure is |
| +// a 503, never a sign-out. |
| +func (a *App) homeSignin(w http.ResponseWriter, r *http.Request) { |
| + s, ok := a.home.sessions.From(r) |
| + if ok && (s.Method == connect.HomeMethod || s.Method == "magiclink" || s.Method == "keymail") { |
| + admitted := false |
| + if s.Method == connect.HomeMethod { |
| + _, err := a.homePerson(r) |
| + if errors.Is(err, linkstore.ErrStorage) { |
| + w.Header().Set("Cache-Control", "no-store") |
| + a.h.errorPage(w, r, http.StatusServiceUnavailable, "") |
| + return |
| + } |
| + admitted = err == nil |
| + } else { |
| + admitted = a.home.admitted.allows(normalizeAddress(s.Subject)) |
| + } |
| + if !admitted { |
| + a.home.sessions.SignOut(w, r) |
| + w.Header().Set("Cache-Control", "no-store") |
| + http.Redirect(w, r, r.URL.RequestURI(), http.StatusSeeOther) |
| + return |
| + } |
| + } |
| + a.home.routes.Signin(w, r) |
| +} |
diff --git a/internal/calendar/home_consumer_test.go b/internal/calendar/home_consumer_test.go| index f6cd37f..ced5f50 100644 |
| --- a/internal/calendar/home_consumer_test.go |
| +++ b/internal/calendar/home_consumer_test.go |
| @@ -94,3 +94,24 @@ func TestAllAppsLinksToDirectory(t *testing.T) { |
| t.Fatal("switcher shown with no directory configured") |
| } |
| } |
| + |
| +// A session whose person is no longer admitted is not "already signed in": |
| +// /signin ends it and starts over, rather than shortcutting to the page |
| +// (which requirePerson answers with a terminating 404). Astra P3 on #23; |
| +// the same check docs' admitted and sheets' homeRequire make. |
| +func TestSigninDoesNotShortcutARevokedPerson(t *testing.T) { |
| + f := newHomeConsumerFixture(t, "") |
| + f.local(t) |
| + delete(f.app.home.admitted.addrs, "alice@example.test") |
| + w := f.do("GET", "/signin?return_to=%2Fday%2F2026-09-10", "") |
| + if w.Code == http.StatusSeeOther && w.Header().Get("Location") == "/day/2026-09-10" { |
| + t.Fatal("/signin shortcut a removed member to the page, which 404s") |
| + } |
| + if f.cookies[f.app.home.sessions.CookieName()] != nil { |
| + t.Fatal("/signin kept the removed member's session") |
| + } |
| + // Asked again with no session, it starts a fresh exchange. |
| + if got := homeRedirect(t, f.do("GET", "/signin?return_to=%2Fday%2F2026-09-10", "")); !strings.HasPrefix(got, "/auth/start") { |
| + t.Fatalf("second /signin went to %q, want the exchange", got) |
| + } |
| +} |
diff --git a/internal/calendar/home_router_test.go b/internal/calendar/home_router_test.go| index e1469bf..acf56b9 100644 |
| --- a/internal/calendar/home_router_test.go |
| +++ b/internal/calendar/home_router_test.go |
| @@ -219,6 +219,14 @@ func TestHomeRouterRetainsExistingCalendarIdentity(t *testing.T) { |
| if w := f.do("GET", "/", ""); w.Code != 404 { |
| t.Fatal("Home attached replacement", w.Code) |
| } |
| + // /signin does not shortcut this session to "/", which would 404 with |
| + // nowhere to go: it ends the session and starts over (Astra P3, #23). |
| + if w := f.do("GET", "/signin", ""); w.Code == http.StatusSeeOther && w.Header().Get("Location") == "/" { |
| + t.Fatal("/signin shortcut a deleted Home person to /, which 404s") |
| + } |
| + if f.cookies[f.app.home.sessions.CookieName()] != nil { |
| + t.Fatal("/signin kept a deleted Home person's session") |
| + } |
| } |
| func TestHomeRouterUnknownSubjectCannotCreateAccount(t *testing.T) { |
| f := newHomeRouterFixture(t) |
diff --git a/internal/calendar/routes.go b/internal/calendar/routes.go| index 275c52c..84c9c31 100644 |
| --- a/internal/calendar/routes.go |
| +++ b/internal/calendar/routes.go |
| @@ -43,7 +43,7 @@ func (a *App) routes(h *handlers, authPlugin *rauth.Auth, list allowlist, origin |
| // Signed-out. |
| if a.home != nil { |
| - r.Get("/signin", a.home.routes.Signin) |
| + r.Get("/signin", a.homeSignin) |
| r.Get("/signin/local", h.signin) |
| r.Get("/signed-out", a.home.routes.SignedOut) |
| r.Get(connect.LocalCompletePath, a.home.routes.LocalComplete) |