fix(api): make logout idempotent and hide grants left on a stale epoch

Logout answered an undeclared 404 when its grant was already gone, for
example revoked by another node inside the liveness cache window or by a
concurrent logout. It now treats a missing grant as success and still
evicts the cache entry, so logout always answers 200.

Grants left on an older user epoch (after a password reset through the
existing UI, or a login that raced a password change) are dead but only
deleted when presented. Listing and counting grants now filter on the
user's current epoch, so those grants no longer show up.

dropGrant now deletes before evicting, like RevokeGrant, so a concurrent
cache fill cannot re-cache a grant that is being dropped.

core/apiauth/service.go

@@ -192,11 +192,12 @@ func (s *Service) loadUser(ctx context.Context, userID string) (*model.User, err
 	return u, err
 }
 
+// dropGrant evicts after deleting, so a concurrent fill cannot re-cache the dead grant.
 func (s *Service) dropGrant(ctx context.Context, id string) {
-	s.cache.evict(id)
 	if err := s.ds.Grant().Delete(ctx, id); err != nil {
 		log.Warn(ctx, "API v1: could not delete dead grant", "grant", id, err)
 	}
+	s.cache.evict(id)
 }
 
 // settleEpoch re-reads grant and user in one read transaction: separate reads can straddle a password
@@ -303,11 +304,11 @@ func (s *Service) liveGrant(ctx context.Context, id string, u *model.User) (live
 
 func (s *Service) ListGrants(ctx context.Context, p *Principal, offset, limit int) (model.Grants, int64, error) {
 	idleSince := s.now().Add(-IdleExpiry)
-	grants, err := s.ds.Grant().GetAllForUser(ctx, p.User.ID, idleSince, offset, limit)
+	grants, err := s.ds.Grant().GetAllForUser(ctx, p.User.ID, p.User.TokenEpoch, idleSince, offset, limit)
 	if err != nil {
 		return nil, 0, err
 	}
-	total, err := s.ds.Grant().CountForUser(ctx, p.User.ID, idleSince)
+	total, err := s.ds.Grant().CountForUser(ctx, p.User.ID, p.User.TokenEpoch, idleSince)
 	return grants, total, err
 }
 
@@ -319,8 +320,14 @@ func (s *Service) RevokeGrant(ctx context.Context, p *Principal, grantID string)
 	return nil
 }
 
+// Logout succeeds when the grant is already gone, e.g. revoked by another node or a concurrent logout.
 func (s *Service) Logout(ctx context.Context, p *Principal) error {
-	return s.RevokeGrant(ctx, p, p.GrantID)
+	err := s.RevokeGrant(ctx, p, p.GrantID)
+	if errors.Is(err, model.ErrNotFound) {
+		s.cache.evict(p.GrantID)
+		return nil
+	}
+	return err
 }
 
 // ChangePassword does every check inside the locked transaction, so a reset that lands first is never overwritten.

core/apiauth/service_session_test.go

@@ -199,6 +199,35 @@ var _ = Describe("Service: sessions", func() {
 			Expect([]string{grants[0].ID, grants[1].ID}).To(ContainElements(first.Grant.ID, p.GrantID))
 		})
 
+		It("lists only grants on the user's current epoch", func() {
+			u := createUser(ctx, "pw", false)
+			login(u)
+			u.NewPassword = "reset-by-admin" // old-UI reset leaves the old grant on the previous epoch
+			Expect(realDS.User().Put(ctx, &u)).To(Succeed())
+			issued, err := svc.Login(ctx, u.UserName, "reset-by-admin", meta, nil)
+			Expect(err).ToNot(HaveOccurred())
+			p, err := svc.ResolveGrant(ctx, issued.Secret, "")
+			Expect(err).ToNot(HaveOccurred())
+
+			grants, total, err := svc.ListGrants(ctx, p, 0, 10)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(total).To(Equal(int64(1)))
+			Expect(grants).To(HaveLen(1))
+			Expect(grants[0].ID).To(Equal(issued.Grant.ID))
+		})
+
+		It("logs out successfully when the grant is already gone", func() {
+			u := createUser(ctx, "pw", false)
+			_, p, tok := login(u)
+			_, err := svc.Authenticate(ctx, tok.Token, "")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(realDS.Grant().Delete(ctx, p.GrantID)).To(Succeed()) // another node
+
+			Expect(svc.Logout(ctx, p)).To(Succeed())
+			_, err = svc.Authenticate(ctx, tok.Token, "")
+			Expect(err).To(MatchError(model.ErrInvalidAuth))
+		})
+
 		It("refuses to revoke another user's grant", func() {
 			alice := createUser(ctx, "pw", false)
 			bob := createUser(ctx, "pw", false)

model/grant.go

@@ -50,8 +50,8 @@ type GrantRepository interface {
 	Put(ctx context.Context, g *Grant) error
 	Get(ctx context.Context, id string) (*Grant, error)
 	FindBySecretHash(ctx context.Context, hash string) (*Grant, error)
-	GetAllForUser(ctx context.Context, userID string, idleSince time.Time, offset, limit int) (Grants, error)
-	CountForUser(ctx context.Context, userID string, idleSince time.Time) (int64, error)
+	GetAllForUser(ctx context.Context, userID string, epoch int, idleSince time.Time, offset, limit int) (Grants, error)
+	CountForUser(ctx context.Context, userID string, epoch int, idleSince time.Time) (int64, error)
 	Delete(ctx context.Context, id string) error
 	DeleteForUser(ctx context.Context, userID, id string) error
 	DeleteOtherEpochs(ctx context.Context, userID string, epoch int) error

persistence/grant_repository.go

@@ -60,12 +60,13 @@ func (r *grantRepository) findOne(ctx context.Context, cond Sqlizer) (*model.Gra
 	return &g, nil
 }
 
-func (r *grantRepository) activeForUser(userID string, idleSince time.Time) Sqlizer {
-	return And{Eq{"user_id": userID}, Expr(grantLastActivity+" >= ?", idleSince.UTC())}
+// activeForUser skips grants left on an older epoch: they are dead but only deleted when presented.
+func (r *grantRepository) activeForUser(userID string, epoch int, idleSince time.Time) Sqlizer {
+	return And{Eq{"user_id": userID, "user_epoch": epoch}, Expr(grantLastActivity+" >= ?", idleSince.UTC())}
 }
 
-func (r *grantRepository) GetAllForUser(ctx context.Context, userID string, idleSince time.Time, offset, limit int) (model.Grants, error) {
-	sel := r.newSelect(ctx).Columns("*").Where(r.activeForUser(userID, idleSince)).
+func (r *grantRepository) GetAllForUser(ctx context.Context, userID string, epoch int, idleSince time.Time, offset, limit int) (model.Grants, error) {
+	sel := r.newSelect(ctx).Columns("*").Where(r.activeForUser(userID, epoch, idleSince)).
 		OrderBy("last_used_at IS NULL", "last_used_at desc", "created_at desc", "id").
 		Offset(uint64(offset)).Limit(uint64(limit))
 	var res model.Grants
@@ -73,8 +74,8 @@ func (r *grantRepository) GetAllForUser(ctx context.Context, userID string, idle
 	return res, err
 }
 
-func (r *grantRepository) CountForUser(ctx context.Context, userID string, idleSince time.Time) (int64, error) {
-	return r.count(ctx, Select().Where(r.activeForUser(userID, idleSince)))
+func (r *grantRepository) CountForUser(ctx context.Context, userID string, epoch int, idleSince time.Time) (int64, error) {
+	return r.count(ctx, Select().Where(r.activeForUser(userID, epoch, idleSince)))
 }
 
 func (r *grantRepository) Delete(ctx context.Context, id string) error {

persistence/grant_repository_test.go

@@ -54,7 +54,7 @@ var _ = Describe("GrantRepository", func() {
 		Expect(err).To(MatchError(model.ErrNotFound))
 	})
 
-	It("lists and counts only the user's non-idle grants by lastUsedAt, never-used ones last", func() {
+	It("lists and counts only the user's non-idle grants on the given epoch by lastUsedAt, never-used ones last", func() {
 		old := newGrant(adminUser.ID, "h-old")
 		old.CreatedAt = now.Add(-100 * 24 * time.Hour)
 		usedEarly := newGrant(adminUser.ID, "h-used-early")
@@ -67,18 +67,21 @@ var _ = Describe("GrantRepository", func() {
 		usedLate.LastUsedAt = &lateUse
 		freshNeverUsed := newGrant(adminUser.ID, "h-fresh") // newer than both uses, but never used
 		other := newGrant(regularUser.ID, "h-other")
-		for _, g := range []*model.Grant{old, usedEarly, usedLate, freshNeverUsed, other} {
+		staleEpoch := newGrant(adminUser.ID, "h-stale-epoch")
+		staleEpoch.UserEpoch = 1
+		for _, g := range []*model.Grant{old, usedEarly, usedLate, freshNeverUsed, other, staleEpoch} {
 			Expect(repo.Put(ctx, g)).To(Succeed())
 		}
 		idleSince := now.Add(-90 * 24 * time.Hour)
 
-		list, err := repo.GetAllForUser(ctx, adminUser.ID, idleSince, 0, 10)
+		list, err := repo.GetAllForUser(ctx, adminUser.ID, 0, idleSince, 0, 10)
 		Expect(err).ToNot(HaveOccurred())
+		Expect(list).To(HaveLen(3))
 		Expect([]string{list[0].ID, list[1].ID, list[2].ID}).To(Equal([]string{usedLate.ID, usedEarly.ID, freshNeverUsed.ID}))
 
-		Expect(repo.CountForUser(ctx, adminUser.ID, idleSince)).To(Equal(int64(3)))
+		Expect(repo.CountForUser(ctx, adminUser.ID, 0, idleSince)).To(Equal(int64(3)))
 
-		page, err := repo.GetAllForUser(ctx, adminUser.ID, idleSince, 1, 1)
+		page, err := repo.GetAllForUser(ctx, adminUser.ID, 0, idleSince, 1, 1)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(page).To(HaveLen(1))
 		Expect(page[0].ID).To(Equal(usedEarly.ID))

server/apiv1/auth_test.go

@@ -171,6 +171,17 @@ var _ = Describe("auth endpoints", func() {
 		Expect(call(http.MethodPost, "/api/v1/auth/token", gc.Secret, nil).Code).To(Equal(http.StatusUnauthorized))
 	})
 
+	It("logs out with 200 when another node already revoked the grant", func() {
+		gc := setup()
+		at := mint(gc.Secret, nil)
+		Expect(call(http.MethodGet, "/api/v1/auth/grants", at.AccessToken, nil).Code).To(Equal(http.StatusOK)) // caches the grant
+		Expect(realDS.Grant().Delete(ctx, gc.Grant.Id)).To(Succeed())
+
+		w := call(http.MethodPost, "/api/v1/auth/logout", at.AccessToken, nil)
+		Expect(w.Code).To(Equal(http.StatusOK), w.Body.String())
+		Expect(w.Body.String()).To(ContainSubstring(`"logoutUrl":null`))
+	})
+
 	It("answers 404 for a grant id the caller does not own, and 400 for an over-long id", func() {
 		gc := setup()
 		tok := mint(gc.Secret, nil).AccessToken