修复密码重置遗漏撤销客户端授权码并补充事务隔离回归
Some checks are pending
Security Checks / security (push) Waiting to run

This commit is contained in:
smail 2026-09-20 02:29:08 +08:00
parent 4e47dbff6c
commit 7aec99de89
8 changed files with 110 additions and 8 deletions

View File

@ -38,7 +38,7 @@ func newPublicSecurityFixture(t *testing.T) (*recovery.Service, *webauthnservice
&models.Sessions{},
&models.SecurityQuestion{},
&models.RecoveryCode{},
&models.WebAuthnCredential{}, &models.ProtocolCredential{},
&models.WebAuthnCredential{}, &models.ProtocolCredential{}, &models.ClientCredential{},
} {
if err := engine.Sync2(table); err != nil {
t.Fatal(err)

View File

@ -28,13 +28,17 @@ func resetIntegrationDatabase(t *testing.T) (*xorm.Engine, []models.User) {
}
t.Cleanup(func() { _ = engine.Close() })
if err := engine.Sync2(&models.User{}, &models.SecurityQuestion{}, &models.RecoveryCode{},
&models.WebAuthnCredential{}, &models.ProtocolCredential{}); err != nil {
&models.WebAuthnCredential{}, &models.ProtocolCredential{}, &models.ClientCredential{}); err != nil {
t.Fatal(err)
}
hash, err := password.HashUserPassword(integrationOldPassword)
if err != nil {
t.Fatal(err)
}
clientHash, err := password.Hash(credentials.CleanAuthCode(integrationOldPassword))
if err != nil {
t.Fatal(err)
}
users := []models.User{
{Account: "root", Name: "恢复管理员", Role: "super_admin", IsAdmin: 1,
Password: hash, SessionVersion: 3, SecurityVersion: 5, MustChangePassword: 1},
@ -49,6 +53,7 @@ func resetIntegrationDatabase(t *testing.T) (*xorm.Engine, []models.User) {
if _, err := engine.Insert(
&models.SecurityQuestion{UserID: user.ID, Question: "测试问题", AnswerHash: hash, Version: 1},
&models.RecoveryCode{UserID: user.ID, CodeHash: hash, CodeVersion: 1},
&models.ClientCredential{UserID: user.ID, ClientName: "测试客户端", CodeHash: clientHash, CodeVersion: 1},
&models.WebAuthnCredential{UserID: user.ID, CredentialID: user.Account + "-key",
CredentialData: "test-data", DisplayName: "测试密钥", CreatedAt: time.Now()},
); err != nil {
@ -79,6 +84,7 @@ func assertResetCredentialState(t *testing.T, engine *xorm.Engine, userID int, r
{&models.RecoveryCode{}, "user_id = ? AND used_at IS NULL", want},
{&models.WebAuthnCredential{}, "user_id = ? AND revoked_at IS NULL", want},
{&models.ProtocolCredential{}, "user_id = ? AND revoked_at IS NULL", 3 * want},
{&models.ClientCredential{}, "user_id = ? AND revoked_at IS NULL", want},
} {
count, err := engine.Where(query.where, userID).Count(query.bean)
if err != nil || count != query.count {
@ -112,6 +118,9 @@ func TestUnixResetCommitsPasswordAndRevokesAllProtocolCodes(t *testing.T) {
if matched, err := service.CheckProtocolCode(users[0].ID, protocol, integrationOldPassword); err != nil || !matched {
t.Fatalf("重置前协议 %s 的凭据不可用:%v", protocol, err)
}
if matched, err := service.CheckClientOrProtocolCode(users[0].ID, protocol, credentials.CleanAuthCode(integrationOldPassword)); err != nil || !matched {
t.Fatalf("重置前协议 %s 的客户端码不可用:%v", protocol, err)
}
}
path := filepath.Join(t.TempDir(), "control.sock")
// 隔离夹具仅信任测试进程的 UID;不改变生产入口只信任 root 的配置。
@ -178,6 +187,10 @@ func TestUnixResetCommitsPasswordAndRevokesAllProtocolCodes(t *testing.T) {
if err != nil || matched != (index == 1) {
t.Fatalf("重置后用户 %d 协议 %s 的凭据验证结果错误:%v", user.ID, protocol, err)
}
matched, err = service.CheckClientOrProtocolCode(user.ID, protocol, credentials.CleanAuthCode(integrationOldPassword))
if err != nil || matched != (index == 1) {
t.Fatalf("重置后用户 %d 协议 %s 的客户端码验证结果错误:%v", user.ID, protocol, err)
}
}
}
}
@ -186,6 +199,7 @@ func TestResetSuperAdminRollsBackEveryRevocationStage(t *testing.T) {
for _, stage := range []struct{ table, operation string }{
{"recovery_code", "UPDATE"}, {"security_question", "DELETE"},
{"webauthn_credential", "UPDATE"}, {"protocol_credential", "UPDATE"},
{"client_credential", "UPDATE"},
} {
t.Run(stage.table, func(t *testing.T) {
engine, users := resetIntegrationDatabase(t)

View File

@ -19,7 +19,7 @@ func TestResetSuperAdminOnlyRevokesTargetSessionsAndCredentials(t *testing.T) {
t.Fatal(err)
}
defer engine.Close()
for _, table := range []any{&models.User{}, &models.Sessions{}, &models.SecurityQuestion{}, &models.RecoveryCode{}, &models.WebAuthnCredential{}, &models.ProtocolCredential{}} {
for _, table := range []any{&models.User{}, &models.Sessions{}, &models.SecurityQuestion{}, &models.RecoveryCode{}, &models.WebAuthnCredential{}, &models.ProtocolCredential{}, &models.ClientCredential{}} {
if err := engine.Sync2(table); err != nil {
t.Fatal(err)
}

View File

@ -30,7 +30,7 @@ func verificationFixture(t *testing.T) (*VerificationService, models.User, *stri
t.Fatal(err)
}
t.Cleanup(func() { _ = engine.Close() })
if err := engine.Sync2(&models.User{}, &models.ExternalEmail{}, &models.TOTPAuthenticator{}, &models.SecurityQuestion{}, &models.RecoveryCode{}, &models.ProtocolCredential{}, &models.WebAuthnCredential{}); err != nil {
if err := engine.Sync2(&models.User{}, &models.ExternalEmail{}, &models.TOTPAuthenticator{}, &models.SecurityQuestion{}, &models.RecoveryCode{}, &models.ProtocolCredential{}, &models.WebAuthnCredential{}, &models.ClientCredential{}); err != nil {
t.Fatal(err)
}
user := models.User{Account: "root", Role: "super_admin", SessionVersion: 2, SecurityVersion: 3}

View File

@ -0,0 +1,81 @@
package credentials
import (
"strings"
"testing"
)
func TestRevokeAllRejectsClientCodesAndPreservesOtherUsers(t *testing.T) {
for _, mode := range []string{"standalone", "transaction", "rollback"} {
t.Run(mode, func(t *testing.T) {
service := newFixture(t)
service.SetVaultRoot(t.TempDir())
codes := make(map[int]string)
for _, userID := range []int{1, 2} {
code, _, err := service.CreateClientCredential(userID, "测试客户端")
if err != nil {
t.Fatal(err)
}
codes[userID] = code
if matched, err := service.CheckClientOrProtocolCode(userID, ProtocolSMTP, code); err != nil || !matched {
t.Fatalf("撤销前客户端码无法登录:%v", err)
}
}
if mode == "standalone" {
if err := service.RevokeAll(1); err != nil {
t.Fatal(err)
}
} else {
tx := service.engine.NewSession()
defer tx.Close()
if err := tx.Begin(); err != nil {
t.Fatal(err)
}
if err := service.RevokeAllWithSession(tx, 1); err != nil {
t.Fatal(err)
}
var err error
if mode == "rollback" {
err = tx.Rollback()
} else {
err = tx.Commit()
}
if err != nil {
t.Fatal(err)
}
}
for userID, code := range codes {
want := userID == 2 || mode == "rollback"
for _, protocol := range SupportedProtocols() {
matched, err := service.CheckClientOrProtocolCode(userID, protocol, code)
if err != nil || matched != want {
t.Fatalf("用户 %d 协议 %s 的撤销状态错误:%v", userID, protocol, err)
}
}
revealed, err := service.RevealClientCredentials(userID)
if err != nil || (len(revealed) > 0) != want {
t.Fatalf("用户 %d 的授权码展示状态错误:%v", userID, err)
}
}
})
}
}
func TestRevokeAllRollsBackWhenClientRevocationFails(t *testing.T) {
service := newFixture(t)
if err := service.SetSecurityQuestion(1, "测试问题", "测试答案"); err != nil {
t.Fatal(err)
}
if _, _, err := service.CreateClientCredential(1, "测试客户端"); err != nil {
t.Fatal(err)
}
if _, err := service.engine.Exec("CREATE TRIGGER reject_client_revocation BEFORE UPDATE ON client_credential BEGIN SELECT RAISE(ABORT, 'injected revocation failure'); END"); err != nil {
t.Fatal(err)
}
if err := service.RevokeAll(1); err == nil || !strings.Contains(err.Error(), "injected revocation failure") {
t.Fatalf("未命中客户端撤销故障:%v", err)
}
if matched, err := service.VerifySecurityQuestion(1, "测试答案"); err != nil || !matched {
t.Fatalf("前序撤销未回滚:%v", err)
}
}

View File

@ -292,7 +292,7 @@ func (s *Service) RevokeRecoveryCodes(userID int) error {
return nil
}
// RevokeAll 撤销用户所有恢复码和密保问题,供密码重置后的统一凭据失效使用。
// RevokeAll 撤销用户的恢复码、密保问题、安全密钥及邮件授权码。
func (s *Service) RevokeAll(userID int) error {
if s == nil || s.engine == nil {
return ErrUnavailable
@ -305,7 +305,7 @@ func (s *Service) RevokeAll(userID int) error {
return revokeAllOnSession(s.engine.NewSession(), s.now, userID)
}
// RevokeAllWithSession 在调用方事务中撤销恢复码并删除密保问题。
// RevokeAllWithSession 在调用方事务中撤销用户凭据,与密码更新共同提交或回滚。
func (s *Service) RevokeAllWithSession(session *xorm.Session, userID int) error {
if s == nil || s.engine == nil || session == nil {
return ErrUnavailable
@ -329,6 +329,9 @@ func (s *Service) RevokeAllWithSession(session *xorm.Session, userID int) error
if _, err := session.Where("user_id = ? AND revoked_at IS NULL", userID).Cols("revoked_at", "updated_at").Update(&models.ProtocolCredential{RevokedAt: &now}); err != nil {
return fmt.Errorf("revoke protocol credentials: %w", err)
}
if _, err := session.Where("user_id = ? AND revoked_at IS NULL", userID).Cols("revoked_at", "updated_at").Update(&models.ClientCredential{RevokedAt: &now}); err != nil {
return fmt.Errorf("revoke client credentials: %w", err)
}
return nil
}
@ -358,6 +361,10 @@ func revokeAllOnSession(session *xorm.Session, now func() time.Time, userID int)
_ = session.Rollback()
return fmt.Errorf("revoke protocol credentials: %w", err)
}
if _, err := session.Where("user_id = ? AND revoked_at IS NULL", userID).Cols("revoked_at", "updated_at").Update(&models.ClientCredential{RevokedAt: &when}); err != nil {
_ = session.Rollback()
return fmt.Errorf("revoke client credentials: %w", err)
}
if err := session.Commit(); err != nil {
return fmt.Errorf("commit credential revocation: %w", err)
}

View File

@ -19,7 +19,7 @@ func newFixture(t *testing.T) *Service {
t.Fatal(err)
}
t.Cleanup(func() { _ = engine.Close() })
if err := engine.Sync2(&models.User{}, &models.SecurityQuestion{}, &models.RecoveryCode{}, &models.WebAuthnCredential{}, &models.ProtocolCredential{}); err != nil {
if err := engine.Sync2(&models.User{}, &models.SecurityQuestion{}, &models.RecoveryCode{}, &models.WebAuthnCredential{}, &models.ProtocolCredential{}, &models.ClientCredential{}); err != nil {
t.Fatal(err)
}
if _, err := engine.Insert(&models.User{Account: "alice", Name: "Alice", Password: "unused", Role: "user"}); err != nil {

View File

@ -23,7 +23,7 @@ func newRecoveryFixture(t *testing.T) (*Service, *verification.Service, *xorm.En
t.Fatal(err)
}
t.Cleanup(func() { _ = engine.Close() })
for _, table := range []any{&models.User{}, &models.ExternalEmail{}, &models.Sessions{}, &models.SecurityQuestion{}, &models.RecoveryCode{}, &models.WebAuthnCredential{}, &models.ProtocolCredential{}} {
for _, table := range []any{&models.User{}, &models.ExternalEmail{}, &models.Sessions{}, &models.SecurityQuestion{}, &models.RecoveryCode{}, &models.WebAuthnCredential{}, &models.ProtocolCredential{}, &models.ClientCredential{}} {
if err := engine.Sync2(table); err != nil {
t.Fatal(err)
}