From d5e1de09dba86425e540f2ffeb7f0a80faad840b Mon Sep 17 00:00:00 2001 From: DiheChen Date: Sun, 8 Nov 2026 22:43:38 +0800 Subject: [PATCH] fix: issue a login session only while the guest session and account password are unchanged --- db/queries/members.sql | 6 +- .../postgres/internal/query/members.sql.go | 23 ++++-- internal/adapter/out/postgres/members.go | 17 ++++- internal/adapter/out/postgres/members_test.go | 70 ++++++++++++++++++- internal/adapter/out/postgres/tickets_test.go | 4 +- internal/app/identity/account.go | 8 ++- internal/app/identity/account_test.go | 34 +++++++++ internal/app/port/member.go | 14 ++-- internal/app/port/porttest/store.go | 11 ++- 9 files changed, 167 insertions(+), 20 deletions(-) diff --git a/db/queries/members.sql b/db/queries/members.sql index 99529b0..f1ea313 100644 --- a/db/queries/members.sql +++ b/db/queries/members.sql @@ -30,7 +30,11 @@ SELECT * FROM members WHERE username = @username; -- name: SetMemberPassword :execrows UPDATE members SET password_hash = @password_hash WHERE id = @id AND username IS NOT NULL; --- name: DeleteSession :exec +-- name: LockMemberPassword :one +-- FOR SHARE 让修改密码等到这次登录提交之后，它随后删除会话时会删掉这次登录签发的会话。 +SELECT coalesce(password_hash, '')::text AS password_hash FROM members WHERE id = @id FOR SHARE; + +-- name: DeleteSession :execrows DELETE FROM member_sessions WHERE token_hash = @token_hash; -- name: DeleteMemberSessions :many diff --git a/internal/adapter/out/postgres/internal/query/members.sql.go b/internal/adapter/out/postgres/internal/query/members.sql.go index b21db11..da65327 100644 --- a/internal/adapter/out/postgres/internal/query/members.sql.go +++ b/internal/adapter/out/postgres/internal/query/members.sql.go @@ -91,13 +91,28 @@ func (q *Queries) DeleteMemberSessions(ctx context.Context, arg DeleteMemberSess return items, nil } -const deleteSession = `-- name: DeleteSession :exec +const deleteSession = `-- name: DeleteSession :execrows DELETE FROM member_sessions WHERE token_hash = $1 ` -func (q *Queries) DeleteSession(ctx context.Context, tokenHash []byte) error { - _, err := q.db.Exec(ctx, deleteSession, tokenHash) - return err +func (q *Queries) DeleteSession(ctx context.Context, tokenHash []byte) (int64, error) { + result, err := q.db.Exec(ctx, deleteSession, tokenHash) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const lockMemberPassword = `-- name: LockMemberPassword :one +SELECT coalesce(password_hash, '')::text AS password_hash FROM members WHERE id = $1 FOR SHARE +` + +// FOR SHARE 让修改密码等到这次登录提交之后，它随后删除会话时会删掉这次登录签发的会话。 +func (q *Queries) LockMemberPassword(ctx context.Context, id string) (string, error) { + row := q.db.QueryRow(ctx, lockMemberPassword, id) + var password_hash string + err := row.Scan(&password_hash) + return password_hash, err } const memberByToken = `-- name: MemberByToken :one diff --git a/internal/adapter/out/postgres/members.go b/internal/adapter/out/postgres/members.go index e358ec0..680125e 100644 --- a/internal/adapter/out/postgres/members.go +++ b/internal/adapter/out/postgres/members.go @@ -99,12 +99,23 @@ func (s *Store) AccountByUsername(ctx context.Context, username string) (port.Ac return port.Account{Member: memberOf(row), Username: orEmpty(row.Username), PasswordHash: orEmpty(row.PasswordHash)}, nil } -func (s *Store) ReplaceSession(ctx context.Context, oldTokenHash []byte, memberID string, newTokenHash []byte) error { +func (s *Store) ReplaceSession(ctx context.Context, oldTokenHash []byte, memberID string, newTokenHash []byte, passwordHash string) error { err := pgx.BeginFunc(ctx, s.pool, func(tx pgx.Tx) error { q := s.q.WithTx(tx) - if err := q.DeleteSession(ctx, oldTokenHash); err != nil { + current, err := q.LockMemberPassword(ctx, memberID) + if err != nil { return err } + if current != passwordHash { + return port.ErrAccountPasswordChanged + } + n, err := q.DeleteSession(ctx, oldTokenHash) + if err != nil { + return err + } + if n == 0 { + return port.ErrSessionNotFound + } return q.CreateSession(ctx, query.CreateSessionParams{TokenHash: newTokenHash, MemberID: memberID}) }) if err != nil { @@ -114,7 +125,7 @@ func (s *Store) ReplaceSession(ctx context.Context, oldTokenHash []byte, memberI } func (s *Store) DeleteSession(ctx context.Context, tokenHash []byte) error { - if err := s.q.DeleteSession(ctx, tokenHash); err != nil { + if _, err := s.q.DeleteSession(ctx, tokenHash); err != nil { return fmt.Errorf("删除会话：%w", err) } return nil diff --git a/internal/adapter/out/postgres/members_test.go b/internal/adapter/out/postgres/members_test.go index d99a690..4240160 100644 --- a/internal/adapter/out/postgres/members_test.go +++ b/internal/adapter/out/postgres/members_test.go @@ -150,7 +150,7 @@ func TestMemberSessions(t *testing.T) { createMember(t, store, "guest-1", "游客-3c4d") createMember(t, store, "guest-2", "游客-5e6f") for _, s := range []struct{ guest, token string }{{"guest-1", "alice-2"}, {"guest-2", "alice-3"}} { - if err := store.ReplaceSession(t.Context(), tokenHash(s.guest), alice.ID, tokenHash(s.token)); err != nil { + if err := store.ReplaceSession(t.Context(), tokenHash(s.guest), alice.ID, tokenHash(s.token), ""); err != nil { t.Fatal(err) } if got := ownerOf(t, store, s.guest); got != "" { @@ -186,7 +186,7 @@ func TestSetPassword(t *testing.T) { t.Fatal(err) } createMember(t, store, "other", "游客-5e6f") - if err := store.ReplaceSession(t.Context(), tokenHash("other"), alice.ID, tokenHash("alice-2")); err != nil { + if err := store.ReplaceSession(t.Context(), tokenHash("other"), alice.ID, tokenHash("alice-2"), "old"); err != nil { t.Fatal(err) } if deleted, err := store.SetPassword(t.Context(), alice.ID, "new", tokenHash("alice-1")); err != nil || !sameHashes(deleted, [][]byte{tokenHash("alice-2")}) { @@ -205,3 +205,69 @@ func TestSetPassword(t *testing.T) { t.Errorf("不指定保留的会话时应删除全部会话，alice-1 仍属于 %s", got) } } + +func TestReplaceSessionChecksGuestSessionAndPassword(t *testing.T) { + store, _ := newStore(t) + alice := createMember(t, store, "alice-1", "游客-1a2b") + if err := store.Register(t.Context(), alice.ID, "alice", "h1"); err != nil { + t.Fatal(err) + } + createMember(t, store, "guest", "游客-3c4d") + + if err := store.ReplaceSession(t.Context(), tokenHash("gone"), alice.ID, tokenHash("alice-2"), "h1"); !errors.Is(err, port.ErrSessionNotFound) { + t.Errorf("游客的会话已经删除时 = %v，期望 ErrSessionNotFound", err) + } + if err := store.ReplaceSession(t.Context(), tokenHash("guest"), alice.ID, tokenHash("alice-2"), "h0"); !errors.Is(err, port.ErrAccountPasswordChanged) { + t.Errorf("密码哈希已经变了时 = %v，期望 ErrAccountPasswordChanged", err) + } + if ownerOf(t, store, "alice-2") != "" || ownerOf(t, store, "guest") == "" { + t.Error("失败时不应创建新会话，也不应删除游客的会话") + } + if err := store.ReplaceSession(t.Context(), tokenHash("guest"), alice.ID, tokenHash("alice-2"), "h1"); err != nil { + t.Fatal(err) + } + if err := store.ReplaceSession(t.Context(), tokenHash("guest"), alice.ID, tokenHash("alice-3"), "h1"); !errors.Is(err, port.ErrSessionNotFound) { + t.Errorf("同一个游客会话再登录一次 = %v，期望 ErrSessionNotFound", err) + } + if ownerOf(t, store, "alice-3") != "" { + t.Error("同一个游客会话只能换出一个新会话") + } +} + +func TestReplaceSessionWaitsForPasswordChange(t *testing.T) { + store, pool := newStore(t) + alice := createMember(t, store, "alice-1", "游客-1a2b") + if err := store.Register(t.Context(), alice.ID, "alice", "h1"); err != nil { + t.Fatal(err) + } + createMember(t, store, "guest", "游客-3c4d") + + // 模拟 SetPassword 的事务，先不提交。 + tx, err := pool.Begin(t.Context()) + if err != nil { + t.Fatal(err) + } + defer func() { _ = tx.Rollback(t.Context()) }() + if _, err := tx.Exec(t.Context(), "UPDATE members SET password_hash = 'h2' WHERE id = $1", alice.ID); err != nil { + t.Fatal(err) + } + if _, err := tx.Exec(t.Context(), "DELETE FROM member_sessions WHERE member_id = $1", alice.ID); err != nil { + t.Fatal(err) + } + + result := make(chan error, 1) + go func() { + result <- store.ReplaceSession(t.Context(), tokenHash("guest"), alice.ID, tokenHash("alice-2"), "h1") + }() + waitForLockWait(t, pool, 1) + if err := tx.Commit(t.Context()); err != nil { + t.Fatal(err) + } + + if err := <-result; !errors.Is(err, port.ErrAccountPasswordChanged) { + t.Errorf("改密码提交之后，登录的错误 = %v，期望 ErrAccountPasswordChanged", err) + } + if got := ownerOf(t, store, "alice-2"); got != "" { + t.Errorf("改密码之后还留下了按旧密码签发的会话，属于 %s", got) + } +} diff --git a/internal/adapter/out/postgres/tickets_test.go b/internal/adapter/out/postgres/tickets_test.go index a4902a5..ed321bb 100644 --- a/internal/adapter/out/postgres/tickets_test.go +++ b/internal/adapter/out/postgres/tickets_test.go @@ -47,7 +47,7 @@ func TestTicketsBelongToSessions(t *testing.T) { createMember(t, store, guest, "游客") } for _, s := range []struct{ guest, token string }{{"guest-2", "alice-2"}, {"guest-3", "alice-3"}} { - if err := store.ReplaceSession(t.Context(), tokenHash(s.guest), alice.ID, tokenHash(s.token)); err != nil { + if err := store.ReplaceSession(t.Context(), tokenHash(s.guest), alice.ID, tokenHash(s.token), ""); err != nil { t.Fatal(err) } } @@ -76,7 +76,7 @@ func TestTicketsBelongToSessions(t *testing.T) { if !valid("alice-1") || valid("alice-2") || valid("alice-3") { t.Error("删除会话之后，只有保留的会话的票据仍然有效") } - if err := store.ReplaceSession(t.Context(), tokenHash("guest-4"), alice.ID, tokenHash("alice-4")); err != nil { + if err := store.ReplaceSession(t.Context(), tokenHash("guest-4"), alice.ID, tokenHash("alice-4"), ""); err != nil { t.Fatal(err) } if valid("guest-4") { diff --git a/internal/app/identity/account.go b/internal/app/identity/account.go index 90161eb..c6e0f62 100644 --- a/internal/app/identity/account.go +++ b/internal/app/identity/account.go @@ -99,7 +99,13 @@ func (a *Accounts) Login(ctx context.Context, current member.Member, currentToke } token, hash := newSecret() guest := hashSecret(currentToken) - if err := a.members.ReplaceSession(ctx, guest, account.Member.ID, hash); err != nil { + err = a.members.ReplaceSession(ctx, guest, account.Member.ID, hash, account.PasswordHash) + switch { + case errors.Is(err, port.ErrSessionNotFound): + return Session{}, Account{}, ErrInvalidMemberToken + case errors.Is(err, port.ErrAccountPasswordChanged): + return Session{}, Account{}, ErrInvalidCredentials + case err != nil: return Session{}, Account{}, err } if err := a.gateway.RevokeSessions(ctx, [][]byte{guest}); err != nil { diff --git a/internal/app/identity/account_test.go b/internal/app/identity/account_test.go index ee7987c..4c029b9 100644 --- a/internal/app/identity/account_test.go +++ b/internal/app/identity/account_test.go @@ -416,3 +416,37 @@ func TestAccountOperationsFailWhenRevocationFails(t *testing.T) { } } } + +// 在登录校验密码之后、签发新会话之前修改账号的密码。 +type passwordChangedMembers struct { + *porttest.Store +} + +func (s passwordChangedMembers) ReplaceSession(ctx context.Context, oldTokenHash []byte, memberID string, newTokenHash []byte, passwordHash string) error { + if _, err := s.SetPassword(ctx, memberID, "changed", nil); err != nil { + return err + } + return s.Store.ReplaceSession(ctx, oldTokenHash, memberID, newTokenHash, passwordHash) +} + +func TestLoginRejectedWhenGuestSessionOrPasswordChangesMeanwhile(t *testing.T) { + a, s, store, _, _ := newAccounts(t) + registerAccount(t, a, s, "alice", "correct horse") + guest := createMember(t, s) + if err := store.DeleteSession(t.Context(), hashSecret(guest.MemberToken)); err != nil { + t.Fatal(err) + } + if _, _, err := a.Login(t.Context(), guest.Member, guest.MemberToken, clientIP, "alice", "correct horse"); !errors.Is(err, ErrInvalidMemberToken) { + t.Errorf("游客的会话在校验之后失效，Login = %v，期望 ErrInvalidMemberToken", err) + } + + node := porttest.NewRedis().Node("a") + changing := NewAccounts(passwordChangedMembers{store}, store, &fakeQuota{exhausted: map[string]bool{}}, newGateway(store, node, node)) + guest = createMember(t, s) + if _, _, err := changing.Login(t.Context(), guest.Member, guest.MemberToken, clientIP, "alice", "correct horse"); !errors.Is(err, ErrInvalidCredentials) { + t.Errorf("密码在校验之后被修改，Login = %v，期望 ErrInvalidCredentials", err) + } + if _, ok := tokenOwner(t, store, guest.MemberToken); !ok { + t.Error("登录失败时游客的 token 应仍然有效") + } +} diff --git a/internal/app/port/member.go b/internal/app/port/member.go index 6eb1a8b..c52b20a 100644 --- a/internal/app/port/member.go +++ b/internal/app/port/member.go @@ -8,9 +8,11 @@ import ( ) var ( - ErrMemberNotFound = errors.New("成员不存在") - ErrUsernameTaken = errors.New("用户名已被占用") - ErrAlreadyRegistered = errors.New("已经注册过了") + ErrMemberNotFound = errors.New("成员不存在") + ErrUsernameTaken = errors.New("用户名已被占用") + ErrAlreadyRegistered = errors.New("已经注册过了") + ErrSessionNotFound = errors.New("会话不存在") + ErrAccountPasswordChanged = errors.New("账号密码已修改") ) // 设置了用户名的成员。 @@ -33,8 +35,10 @@ type Members interface { // 用户名已属于其他成员时返回 ErrUsernameTaken，成员已经注册过时返回 ErrAlreadyRegistered。 Register(ctx context.Context, memberID, username, passwordHash string) error AccountByUsername(ctx context.Context, username string) (Account, error) - // 在同一个事务中删除 oldTokenHash 的会话，给 memberID 创建 newTokenHash 的会话。 - ReplaceSession(ctx context.Context, oldTokenHash []byte, memberID string, newTokenHash []byte) error + // 在同一个事务中删除 oldTokenHash 的会话，给 memberID 创建 newTokenHash 的会话。oldTokenHash 的会话已经不存在时返回 + // ErrSessionNotFound；memberID 的密码哈希已经不是 passwordHash 时返回 ErrAccountPasswordChanged：密码在校验之后被修改过， + // 修改时对会话的删除已经执行，此时创建的会话不会再被删除。两种情况都不做任何修改。 + ReplaceSession(ctx context.Context, oldTokenHash []byte, memberID string, newTokenHash []byte, passwordHash string) error DeleteSession(ctx context.Context, tokenHash []byte) error // 删除成员除 keepTokenHash 以外的会话，keepTokenHash 为 nil 时全部删除。返回被删除的会话的 token 哈希。 DeleteSessions(ctx context.Context, memberID string, keepTokenHash []byte) ([][]byte, error) diff --git a/internal/app/port/porttest/store.go b/internal/app/port/porttest/store.go index 06ab746..e0800e7 100644 --- a/internal/app/port/porttest/store.go +++ b/internal/app/port/porttest/store.go @@ -178,12 +178,19 @@ func (s *Store) AccountByUsername(_ context.Context, username string) (port.Acco return port.Account{}, port.ErrMemberNotFound } -func (s *Store) ReplaceSession(_ context.Context, oldTokenHash []byte, memberID string, newTokenHash []byte) error { +func (s *Store) ReplaceSession(_ context.Context, oldTokenHash []byte, memberID string, newTokenHash []byte, passwordHash string) error { s.mu.Lock() defer s.mu.Unlock() - if s.member(memberID) == nil { + m := s.member(memberID) + if m == nil { return fmt.Errorf("成员 %s 不存在", memberID) } + if m.passwordHash != passwordHash { + return port.ErrAccountPasswordChanged + } + if _, ok := s.sessions[string(oldTokenHash)]; !ok { + return port.ErrSessionNotFound + } if _, ok := s.sessions[string(newTokenHash)]; ok { return errors.New("token 哈希重复") }