fix: issue a login session only while the guest session and account password are unchanged
This commit is contained in:
1 parent
e103b75d4b
commit
d5e1de09db
9 files changed
+167
-20
No files matched your search
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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") {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 应仍然有效")
|
||||
}
|
||||
}
|
||||
@@ -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)
|
||||
|
||||
@@ -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 哈希重复")
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user