diff --git a/appview/knots/knots.go b/appview/knots/knots.go index 4cc3de49..015bbd4c 100644 --- a/appview/knots/knots.go +++ b/appview/knots/knots.go @@ -217,6 +217,10 @@ func (k *Knots) register(w http.ResponseWriter, r *http.Request) { }) if err != nil { l.Error("failed to put record", "err", err) + if k.OAuth.HandlePermanentAuthErrFromRequest(r, err) { + k.Pages.Notice(w, noticeId, "Your login expired. Log out, log back in, and try again.") + return + } fail() return } @@ -578,6 +582,10 @@ func (k *Knots) addMember(w http.ResponseWriter, r *http.Request) { }) if err != nil { l.Error("failed to add record to PDS", "err", err) + if k.OAuth.HandlePermanentAuthErrFromRequest(r, err) { + k.Pages.Notice(w, noticeId, "Your login expired. Log out, log back in, and try again.") + return + } k.Pages.Notice(w, noticeId, "Failed to add record to PDS, try again later.") return } @@ -660,6 +668,10 @@ func (k *Knots) removeMember(w http.ResponseWriter, r *http.Request) { }) if err != nil { l.Error("failed to delete record from PDS", "err", err) + if k.OAuth.HandlePermanentAuthErrFromRequest(r, err) { + k.Pages.Notice(w, noticeId, "Your login expired. Log out, log back in, and try again.") + return + } k.Pages.Notice(w, noticeId, "Failed to delete record from PDS, try again later.") return } diff --git a/appview/oauth/cache_test.go b/appview/oauth/cache_test.go index 64b5699e..2284ef2f 100644 --- a/appview/oauth/cache_test.go +++ b/appview/oauth/cache_test.go @@ -5,6 +5,7 @@ import ( "errors" "io" "log/slog" + "strings" "sync" "sync/atomic" "testing" @@ -177,6 +178,92 @@ func TestHandlePermanentAuthErrEvictsAndLogsOut(t *testing.T) { } } +func (s *stubStore) countForDid(did syntax.DID) int { + s.mu.Lock() + defer s.mu.Unlock() + prefix := string(did) + ":" + n := 0 + for k := range s.data { + if strings.HasPrefix(k, prefix) { + n++ + } + } + return n +} + +func TestHandlePermanentAuthErrLeavesSiblingSessions(t *testing.T) { + o, store := newTestOAuth(t) + + base := store.data[store.key("did:plc:boltless", "sess1")] + for _, sessId := range []string{"sess2", "sess3"} { + s := base + s.SessionID = sessId + if err := store.SaveSession(context.Background(), s); err != nil { + t.Fatalf("seed %s: %v", sessId, err) + } + } + if got := store.countForDid("did:plc:boltless"); got != 3 { + t.Fatalf("seeded %d sessions, want 3", got) + } + + handled := o.HandlePermanentAuthErr( + context.Background(), "did:plc:boltless", "sess1", + errors.New("auth server request failed (HTTP 400): invalid_grant"), + ) + if !handled { + t.Fatal("HandlePermanentAuthErr returned false") + } + + remaining := store.countForDid("did:plc:boltless") + t.Logf("sessions remaining for DID after one permanent-auth-err: %d", remaining) + if remaining != 2 { + t.Fatalf("got %d sessions remaining, want 2 (handler clears only the named sessId)", remaining) + } +} + +func TestDiscardSupersededSession(t *testing.T) { + const did = "did:plc:boltless" + + t.Run("re-login discards the old session", func(t *testing.T) { + o, store := newTestOAuth(t) + registry := &AccountRegistry{Accounts: []AccountInfo{{Did: did, SessionId: "sess1"}}} + + o.discardSupersededSession(context.Background(), registry, did, "sess2") + + if _, ok := store.data[store.key(did, "sess1")]; ok { + t.Fatal("old session survived re-login") + } + if got := store.deleteCalls.Load(); got != 1 { + t.Fatalf("DeleteSession called %d times, want 1", got) + } + }) + + t.Run("same sessId is a no-op, never deletes the live session", func(t *testing.T) { + o, store := newTestOAuth(t) + registry := &AccountRegistry{Accounts: []AccountInfo{{Did: did, SessionId: "sess1"}}} + + o.discardSupersededSession(context.Background(), registry, did, "sess1") + + if _, ok := store.data[store.key(did, "sess1")]; !ok { + t.Fatal("deleted the session that is still in use") + } + if got := store.deleteCalls.Load(); got != 0 { + t.Fatalf("DeleteSession called %d times, want 0", got) + } + }) + + t.Run("unknown did is a no-op", func(t *testing.T) { + o, store := newTestOAuth(t) + registry := &AccountRegistry{Accounts: []AccountInfo{}} + + o.discardSupersededSession(context.Background(), registry, did, "sess2") + + if got := store.deleteCalls.Load(); got != 0 { + t.Fatalf("DeleteSession called %d times, want 0", got) + } + }) +} + func TestHandlePermanentAuthErrIgnoresTransient(t *testing.T) { o, store := newTestOAuth(t) if _, err := o.resumeSession(context.Background(), "did:plc:boltless", "sess1"); err != nil { diff --git a/appview/oauth/oauth.go b/appview/oauth/oauth.go index 74af1873..95734d5f 100644 --- a/appview/oauth/oauth.go +++ b/appview/oauth/oauth.go @@ -81,17 +81,37 @@ func (o *OAuth) EvictSession(did syntax.DID, sessionId string) { o.sessionCache.Remove(sessionCacheKey(did, sessionId)) } +func (o *OAuth) discardSession(ctx context.Context, did syntax.DID, sessionId string) error { + o.EvictSession(did, sessionId) + err := o.ClientApp.Logout(ctx, did, sessionId) + o.EvictSession(did, sessionId) + return err +} + +func (o *OAuth) discardSupersededSession(ctx context.Context, registry *AccountRegistry, did syntax.DID, newSessionId string) { + existing := registry.FindAccount(did.String()) + if existing == nil || existing.SessionId == newSessionId { + return + } + if err := o.discardSession(ctx, did, existing.SessionId); err != nil { + o.Logger.Warn("failed to discard superseded session on re-login", "did", did, "err", err) + } +} + func (o *OAuth) HandlePermanentAuthErr(ctx context.Context, did syntax.DID, sessionId string, err error) bool { if !IsPermanentAuthErr(err) { return false } - o.EvictSession(did, sessionId) - if logoutErr := o.ClientApp.Logout(ctx, did, sessionId); logoutErr != nil { + if logoutErr := o.discardSession(ctx, did, sessionId); logoutErr != nil { o.Logger.Warn("store logout after permanent auth error failed", "did", did, "err", logoutErr) } return true } +func (o *OAuth) HandlePermanentAuthErrFromRequest(r *http.Request, err error) bool { + return o.HandlePermanentAuthErr(r.Context(), o.GetDidFromCookie(r), o.GetSessIdFromCookie(r), err) +} + func New(config *config.Config, ph posthog.Client, db *db.DB, enforcer *rbac.Enforcer, res *idresolver.Resolver, logger *slog.Logger) (*OAuth, error) { var oauthConfig oauth.ClientConfig var clientUri string @@ -180,6 +200,7 @@ func (o *OAuth) SaveSession(w http.ResponseWriter, r *http.Request, sessData *oa } registry := o.GetAccounts(r) + o.discardSupersededSession(r.Context(), registry, sessData.AccountDID, sessData.SessionID) if err := registry.AddAccount(sessData.AccountDID.String(), handle, sessData.SessionID); err != nil { return err } @@ -228,14 +249,10 @@ func (o *OAuth) DeleteSession(w http.ResponseWriter, r *http.Request) error { sessId := userSession.Values[SessionId].(string) - o.EvictSession(sessDid, sessId) - - // delete the session - err1 := o.ClientApp.Logout(r.Context(), sessDid, sessId) + err1 := o.discardSession(r.Context(), sessDid, sessId) if err1 != nil { err1 = fmt.Errorf("failed to logout: %w", err1) } - o.EvictSession(sessDid, sessId) // remove the cookie userSession.Options.MaxAge = -1 @@ -288,9 +305,7 @@ func (o *OAuth) RemoveAccount(w http.ResponseWriter, r *http.Request, targetDid did, err := syntax.ParseDID(targetDid) if err == nil { - o.EvictSession(did, account.SessionId) - _ = o.ClientApp.Logout(r.Context(), did, account.SessionId) - o.EvictSession(did, account.SessionId) + _ = o.discardSession(r.Context(), did, account.SessionId) } registry.RemoveAccount(targetDid)