From 0dcfaf0a650b9f197c178967084c7b2c49361553 Mon Sep 17 00:00:00 2001 From: iamxvbaba <28732408+iamxvbaba@users.noreply.github.com> Date: Fri, 24 Jul 2026 21:39:00 +0800 Subject: [PATCH] fix: sync remote authorization revoke logout --- internal/app/auth/service.go | 62 +--------- internal/app/auth/service_test.go | 18 ++- internal/rpc/account.go | 3 +- internal/rpc/auth.go | 7 +- internal/rpc/auth_gate.go | 5 + .../rpc/remote_authorization_revoke_test.go | 112 ++++++++++++++++++ internal/store/memory/auth.go | 33 ++---- internal/store/postgres/account_lifecycle.go | 5 +- internal/store/postgres/authkey.go | 5 +- internal/store/postgres/authorization.go | 49 ++++++-- .../authorization_revoke_integration_test.go | 59 ++++----- 11 files changed, 216 insertions(+), 142 deletions(-) create mode 100644 internal/rpc/remote_authorization_revoke_test.go diff --git a/internal/app/auth/service.go b/internal/app/auth/service.go index f2fe1f4a..c1789de8 100644 --- a/internal/app/auth/service.go +++ b/internal/app/auth/service.go @@ -1328,18 +1328,7 @@ func (s *Service) ResetAuthorization(ctx context.Context, userID, hash int64) (d if revoker, ok := s.auths.(authorizationRevoker); ok { return revoker.RevokeByHash(ctx, userID, hash) } - target, found, err := s.authorizationByHash(ctx, userID, hash) - if err != nil || !found { - return target, found, err - } - if err := s.deleteAuthKey(ctx, target.AuthKeyID); err != nil { - return target, true, err - } - deleted, found, err := s.auths.DeleteByHash(ctx, userID, hash) - if err != nil || !found { - return deleted, found, err - } - return deleted, true, nil + return s.auths.DeleteByHash(ctx, userID, hash) } func (s *Service) ResetAuthorizations(ctx context.Context, userID int64, keepAuthKeyID [8]byte) ([]domain.Authorization, error) { @@ -1349,54 +1338,7 @@ func (s *Service) ResetAuthorizations(ctx context.Context, userID int64, keepAut if revoker, ok := s.auths.(authorizationRevoker); ok { return revoker.RevokeByUserExcept(ctx, userID, keepAuthKeyID) } - targets, err := s.authorizationsByUserExcept(ctx, userID, keepAuthKeyID) - if err != nil { - return nil, err - } - for _, a := range targets { - if err := s.deleteAuthKey(ctx, a.AuthKeyID); err != nil { - return nil, err - } - } - deleted, err := s.auths.DeleteByUserExcept(ctx, userID, keepAuthKeyID) - if err != nil { - return nil, err - } - return deleted, nil -} - -func (s *Service) deleteAuthKey(ctx context.Context, authKeyID [8]byte) error { - if s == nil || s.authKeys == nil || authKeyID == ([8]byte{}) { - return nil - } - return s.authKeys.Delete(ctx, authKeyID) -} - -func (s *Service) authorizationByHash(ctx context.Context, userID, hash int64) (domain.Authorization, bool, error) { - items, err := s.auths.ListByUser(ctx, userID) - if err != nil { - return domain.Authorization{}, false, err - } - for _, a := range items { - if a.Hash == hash { - return a, true, nil - } - } - return domain.Authorization{}, false, nil -} - -func (s *Service) authorizationsByUserExcept(ctx context.Context, userID int64, keepAuthKeyID [8]byte) ([]domain.Authorization, error) { - items, err := s.auths.ListByUser(ctx, userID) - if err != nil { - return nil, err - } - out := make([]domain.Authorization, 0, len(items)) - for _, a := range items { - if a.AuthKeyID != keepAuthKeyID { - out = append(out, a) - } - } - return out, nil + return s.auths.DeleteByUserExcept(ctx, userID, keepAuthKeyID) } func (s *Service) bind(ctx context.Context, auth domain.Authorization, userID int64) error { diff --git a/internal/app/auth/service_test.go b/internal/app/auth/service_test.go index 3eef2980..1c652383 100644 --- a/internal/app/auth/service_test.go +++ b/internal/app/auth/service_test.go @@ -535,7 +535,7 @@ func TestLogOutThenSignInSameAuthKeySwitchesUser(t *testing.T) { } } -func TestResetAuthorizationDeletesProtocolAuthKey(t *testing.T) { +func TestResetAuthorizationKeepsProtocolAuthKeyForRPCLogout(t *testing.T) { ctx := context.Background() authz := memory.NewAuthorizationStore() keys := memory.NewAuthKeyStore() @@ -562,15 +562,15 @@ func TestResetAuthorizationDeletesProtocolAuthKey(t *testing.T) { if err != nil || !found || deleted.AuthKeyID != key { t.Fatalf("ResetAuthorization deleted=%x found=%v err=%v, want key %x", deleted.AuthKeyID, found, err, key) } - if _, found, err := keys.Get(ctx, key); err != nil || found { - t.Fatalf("auth key after reset found=%v err=%v, want missing", found, err) + if _, found, err := keys.Get(ctx, key); err != nil || !found { + t.Fatalf("auth key after reset found=%v err=%v, want present for RPC 401", found, err) } if _, found, err := svc.UserID(ctx, key); err != nil || found { t.Fatalf("user after reset found=%v err=%v, want missing", found, err) } } -func TestResetAuthorizationsDeletesOnlyRevokedProtocolAuthKeys(t *testing.T) { +func TestResetAuthorizationsKeepsRevokedProtocolAuthKeys(t *testing.T) { ctx := context.Background() authz := memory.NewAuthorizationStore() keys := memory.NewAuthKeyStore() @@ -600,12 +600,18 @@ func TestResetAuthorizationsDeletesOnlyRevokedProtocolAuthKeys(t *testing.T) { if err != nil || len(deleted) != 1 || deleted[0].AuthKeyID != revoked { t.Fatalf("ResetAuthorizations deleted=%v err=%v, want revoked key", deleted, err) } - if _, found, err := keys.Get(ctx, revoked); err != nil || found { - t.Fatalf("revoked auth key found=%v err=%v, want missing", found, err) + if _, found, err := keys.Get(ctx, revoked); err != nil || !found { + t.Fatalf("revoked auth key found=%v err=%v, want present for RPC 401", found, err) } if _, found, err := keys.Get(ctx, keep); err != nil || !found { t.Fatalf("kept auth key found=%v err=%v, want present", found, err) } + if _, found, err := svc.UserID(ctx, revoked); err != nil || found { + t.Fatalf("revoked user found=%v err=%v, want missing", found, err) + } + if got, found, err := svc.UserID(ctx, keep); err != nil || !found || got != u.ID { + t.Fatalf("kept user=%d found=%v err=%v, want %d", got, found, err, u.ID) + } } func TestSignUpWritesOfficialLoginMessage(t *testing.T) { diff --git a/internal/rpc/account.go b/internal/rpc/account.go index 53bb5aca..c1aa82a8 100644 --- a/internal/rpc/account.go +++ b/internal/rpc/account.go @@ -593,7 +593,8 @@ func (r *Router) onAccountResetAuthorization(ctx context.Context, hash int64) (b } r.revokeAuthKeySessions(deleted.AuthKeyID) _ = r.clearAuthKeyState(ctx, deleted.AuthKeyID) - // P1 修复：撤销该会话销毁其 auth_key，级联 discard 该设备绑定的活跃密聊并通知对端。 + // 撤销该设备的业务授权后，级联 discard 其绑定的活跃密聊并通知对端。 + // 协议 auth key 必须保留，供客户端重连后取得 AUTH_KEY_UNREGISTERED。 r.discardSecretChatsForAuthKey(ctx, businessAuthKeyInt64(deleted.AuthKeyID), userID) return true, nil } diff --git a/internal/rpc/auth.go b/internal/rpc/auth.go index c1c5eee1..2d664438 100644 --- a/internal/rpc/auth.go +++ b/internal/rpc/auth.go @@ -594,7 +594,8 @@ func (r *Router) onAuthResetAuthorizations(ctx context.Context) (bool, error) { for _, a := range deleted { r.revokeAuthKeySessions(a.AuthKeyID) _ = r.clearAuthKeyState(ctx, a.AuthKeyID) - // P1 修复：撤销其它会话同样销毁其 auth_key，级联 discard 该设备绑定的活跃密聊并通知对端。 + // 撤销其它会话会删除其业务 authorization；协议 key 保留用于让客户端 + // 重连后取得 AUTH_KEY_UNREGISTERED。密聊仍按设备授权边界 discard 并通知对端。 r.discardSecretChatsForAuthKey(ctx, businessAuthKeyInt64(a.AuthKeyID), userID) } return true, nil @@ -865,8 +866,8 @@ func (r *Router) onAuthLogOut(ctx context.Context) (*tg.AuthLoggedOut, error) { r.presence.clearSession(key) } } - // P1 修复：登出销毁本设备 perm auth_key 后，级联 discard 其绑定的活跃密聊并通知对端 - //（否则对端继续往死 auth_key 投递成静默死链）。best-effort，不阻断登出。 + // 登出撤销本设备 authorization 后，级联 discard 其绑定的活跃密聊并通知对端 + //（否则对端继续往已退出设备投递成静默死链）。best-effort，不阻断登出。 if userErr == nil && userID != 0 { r.discardSecretChatsForAuthKey(ctx, businessAuthKeyInt64(id), userID) } diff --git a/internal/rpc/auth_gate.go b/internal/rpc/auth_gate.go index 474cde98..682a668b 100644 --- a/internal/rpc/auth_gate.go +++ b/internal/rpc/auth_gate.go @@ -12,6 +12,11 @@ import "github.com/iamxvbaba/td/tg" func rpcAllowedWithoutAuthorization(id uint32) bool { switch id { case tg.AuthBindTempAuthKeyRequestTypeID, + // TWeb handles a 401 from a remotely revoked session by sending + // auth.logOut before it clears IndexedDB/local authorization state. + // This cleanup RPC is idempotent when no authorization remains; rejecting + // it with another 401 makes Web repeat its startup/logout cycle forever. + tg.AuthLogOutRequestTypeID, tg.AuthExportLoginTokenRequestTypeID, tg.AuthImportLoginTokenRequestTypeID, tg.AuthAcceptLoginTokenRequestTypeID, diff --git a/internal/rpc/remote_authorization_revoke_test.go b/internal/rpc/remote_authorization_revoke_test.go new file mode 100644 index 00000000..545eba6d --- /dev/null +++ b/internal/rpc/remote_authorization_revoke_test.go @@ -0,0 +1,112 @@ +package rpc + +import ( + "context" + "testing" + + "github.com/iamxvbaba/td/bin" + "github.com/iamxvbaba/td/clock" + "github.com/iamxvbaba/td/tg" + "github.com/iamxvbaba/td/tgerr" + "go.uber.org/zap/zaptest" + + appauth "telesrv/internal/app/auth" + "telesrv/internal/domain" + "telesrv/internal/store" + "telesrv/internal/store/memory" +) + +func TestAccountResetAuthorizationKeepsProtocolKeyAndReturnsRPC401(t *testing.T) { + ctx := context.Background() + currentAuthKeyID := [8]byte{0x71} + targetAuthKeyID := [8]byte{0x72} + const ( + userID = int64(1000000001) + targetHash = int64(2026072401) + ) + + authKeys := memory.NewAuthKeyStore() + authorizations := memory.NewAuthorizationStore() + for _, authKeyID := range [][8]byte{currentAuthKeyID, targetAuthKeyID} { + if err := authKeys.Save(ctx, store.AuthKeyData{ID: authKeyID}); err != nil { + t.Fatalf("save auth key %x: %v", authKeyID, err) + } + } + authService := appauth.NewService(nil, authorizations, nil, authKeys, nil, "12345") + if err := authorizations.Bind(ctx, domain.Authorization{ + AuthKeyID: currentAuthKeyID, + UserID: userID, + Hash: 2026072400, + }); err != nil { + t.Fatalf("bind current authorization: %v", err) + } + if err := authorizations.Bind(ctx, domain.Authorization{ + AuthKeyID: targetAuthKeyID, + UserID: userID, + Hash: targetHash, + }); err != nil { + t.Fatalf("bind target authorization: %v", err) + } + + r := New(Config{}, Deps{ + Auth: authService, + Files: &fakeFiles{}, + }, zaptest.NewLogger(t), clock.System) + + var warmTarget bin.Buffer + if err := (&tg.UploadSaveFilePartRequest{ + FileID: 1, + FilePart: 0, + Bytes: []byte{1}, + }).Encode(&warmTarget); err != nil { + t.Fatalf("encode target warm-up RPC: %v", err) + } + if _, err := r.Dispatch(ctx, targetAuthKeyID, 101, &warmTarget); err != nil { + t.Fatalf("target warm-up RPC: %v", err) + } + + var reset bin.Buffer + if err := (&tg.AccountResetAuthorizationRequest{Hash: targetHash}).Encode(&reset); err != nil { + t.Fatalf("encode account.resetAuthorization: %v", err) + } + if result, err := r.Dispatch(ctx, currentAuthKeyID, 102, &reset); err != nil { + t.Fatalf("account.resetAuthorization: %v", err) + } else if value, ok := dispatchCanonicalValue(result).(bool); !ok || !value { + t.Fatalf("account.resetAuthorization result = %#v, want true", dispatchCanonicalValue(result)) + } + + if _, found, err := authKeys.Get(ctx, targetAuthKeyID); err != nil || !found { + t.Fatalf("target protocol auth key found=%v err=%v, want retained", found, err) + } + if _, found, err := authorizations.ByAuthKey(ctx, targetAuthKeyID); err != nil || found { + t.Fatalf("target business authorization found=%v err=%v, want removed", found, err) + } + if current, found, err := authorizations.ByAuthKey(ctx, currentAuthKeyID); err != nil || !found || current.UserID != userID { + t.Fatalf("current authorization=%+v found=%v err=%v, want retained user %d", current, found, err, userID) + } + + var afterRevoke bin.Buffer + if err := (&tg.UploadSaveFilePartRequest{ + FileID: 1, + FilePart: 1, + Bytes: []byte{2}, + }).Encode(&afterRevoke); err != nil { + t.Fatalf("encode target post-revoke RPC: %v", err) + } + if _, err := r.Dispatch(ctx, targetAuthKeyID, 103, &afterRevoke); !tgerr.Is(err, "AUTH_KEY_UNREGISTERED") { + t.Fatalf("target post-revoke RPC err=%v, want AUTH_KEY_UNREGISTERED", err) + } + + var logout bin.Buffer + if err := (&tg.AuthLogOutRequest{}).Encode(&logout); err != nil { + t.Fatalf("encode target auth.logOut cleanup: %v", err) + } + if result, err := r.Dispatch(ctx, targetAuthKeyID, 104, &logout); err != nil { + t.Fatalf("target auth.logOut cleanup: %v", err) + } else if _, ok := dispatchCanonicalValue(result).(*tg.AuthLoggedOut); !ok { + t.Fatalf("target auth.logOut cleanup result=%#v, want *tg.AuthLoggedOut", dispatchCanonicalValue(result)) + } + if _, found, err := authKeys.Get(ctx, targetAuthKeyID); err != nil || !found { + t.Fatalf("target protocol auth key after logout cleanup found=%v err=%v, want retained", found, err) + } +} diff --git a/internal/store/memory/auth.go b/internal/store/memory/auth.go index 7a28693a..d5bd0cb6 100644 --- a/internal/store/memory/auth.go +++ b/internal/store/memory/auth.go @@ -461,8 +461,9 @@ func (s *AuthorizationStore) DeleteByHash(_ context.Context, userID, hash int64) return domain.Authorization{}, false, nil } -// RevokeByHash mirrors PostgreSQL's protocol-key revocation boundary when this -// authorization projection is linked to an in-memory auth-key authority. +// RevokeByHash removes only the business authorization. The protocol auth key +// and any temp binding stay usable for MTProto decryption so a kicked client can +// reconnect and receive AUTH_KEY_UNREGISTERED from the RPC gate. func (s *AuthorizationStore) RevokeByHash(ctx context.Context, userID, hash int64) (domain.Authorization, bool, error) { s.linkMu.RLock() defer s.linkMu.RUnlock() @@ -471,27 +472,17 @@ func (s *AuthorizationStore) RevokeByHash(ctx context.Context, userID, hash int6 } s.authKeys.mu.Lock() s.mu.Lock() - var ( - targetID [8]byte - target domain.Authorization - found bool - ) for id, a := range s.m { if a.UserID == userID && a.Hash == hash { - targetID, target, found = id, a, true - break + delete(s.m, id) + s.mu.Unlock() + s.authKeys.mu.Unlock() + return a, true, nil } } - if !found { - s.mu.Unlock() - s.authKeys.mu.Unlock() - return domain.Authorization{}, false, nil - } - deletedIDs := s.authKeys.deleteProtocolAuthKeyLocked(targetID) - s.authKeys.deleteAuthorizationMirrorsWithHeldLocked(deletedIDs, s) s.mu.Unlock() s.authKeys.mu.Unlock() - return target, true, nil + return domain.Authorization{}, false, nil } func (s *AuthorizationStore) DeleteByUserExcept(_ context.Context, userID int64, keepAuthKeyID [8]byte) ([]domain.Authorization, error) { @@ -517,18 +508,12 @@ func (s *AuthorizationStore) RevokeByUserExcept(ctx context.Context, userID int6 s.authKeys.mu.Lock() s.mu.Lock() out := make([]domain.Authorization, 0) - targets := make([][8]byte, 0) for id, a := range s.m { if a.UserID == userID && id != keepAuthKeyID { out = append(out, a) - targets = append(targets, id) + delete(s.m, id) } } - deletedIDs := make([][8]byte, 0, len(targets)) - for _, id := range targets { - deletedIDs = append(deletedIDs, s.authKeys.deleteProtocolAuthKeyLocked(id)...) - } - s.authKeys.deleteAuthorizationMirrorsWithHeldLocked(deletedIDs, s) s.mu.Unlock() s.authKeys.mu.Unlock() return out, nil diff --git a/internal/store/postgres/account_lifecycle.go b/internal/store/postgres/account_lifecycle.go index 45373504..a6be9ecd 100644 --- a/internal/store/postgres/account_lifecycle.go +++ b/internal/store/postgres/account_lifecycle.go @@ -505,7 +505,10 @@ FROM authorizations WHERE auth_key_id = $1 AND user_id = $2 FOR UPDATE`, id, use if !found { return nil, nil } - if err := deleteRevocationTargetsTx(ctx, tx, []int64{id}); err != nil { + // Cancelling a pending account deletion deliberately retires the requester + // protocol identity; unlike remote device revocation, this path does not need + // to preserve the key for a client-visible RPC 401 transition. + if err := deleteProtocolAuthIdentitiesTx(ctx, tx, []int64{id}); err != nil { return nil, err } return []domain.Authorization{a}, nil diff --git a/internal/store/postgres/authkey.go b/internal/store/postgres/authkey.go index 834decca..76c61c11 100644 --- a/internal/store/postgres/authkey.go +++ b/internal/store/postgres/authkey.go @@ -195,8 +195,9 @@ WHERE auth_key_id = $1 // // 同时清理把本 key 当作 perm key 的 temp auth key 行：temp_auth_key_bindings.temp_auth_key_id // 侧有外键 ON DELETE CASCADE，删除 temp key 会自动清绑定；perm_auth_key_id 侧由 -// RESTRICT FK 防止悬空，因此被踢/销毁 perm key 时必须先把关联 temp key 一并删掉。否则 Web/上传连接用 -// raw temp key 重连时仍能进入 RPC 层，只得到 AUTH_KEY_UNREGISTERED，而不是连接层 404。 +// RESTRICT FK 防止悬空，因此显式销毁 perm key 时必须先把关联 temp key 一并删掉。 +// 远程撤销 authorization 不得调用本方法：被踢客户端必须保留协议 key，重连进入 RPC +// 层后取得 AUTH_KEY_UNREGISTERED，而不是只收到连接层 -404。 func (s *AuthKeyStore) Delete(ctx context.Context, id [8]byte) error { return withAuthIdentityTx(ctx, s.db, "delete auth key", func(tx pgx.Tx) error { return deleteAuthKeyTx(ctx, tx, authKeyIDToInt64(id)) diff --git a/internal/store/postgres/authorization.go b/internal/store/postgres/authorization.go index c0000d6c..3cd11265 100644 --- a/internal/store/postgres/authorization.go +++ b/internal/store/postgres/authorization.go @@ -252,9 +252,10 @@ RETURNING auth_key_id, user_id, hash, layer, device_model, platform, system_vers return a, true, nil } -// RevokeByHash 删除协议 auth_key 作为远程踢设备的持久化事实入口。 -// authorizations 通过 FK cascade 删除；update_states 没有 auth_keys FK，必须显式清理； -// 关联 temp auth key 也显式删除，避免 raw temp key 重连。 +// RevokeByHash 是远程踢设备的持久化事实入口：删除业务 authorization 与 +// device update state，但保留 permanent/temp 协议 key 和 binding。这样被踢客户端 +// 重连后仍可完成 MTProto 解密，并由 RPC gate 返回 AUTH_KEY_UNREGISTERED；若先删除 +// 协议 key，客户端只能收到 transport -404，无法可靠清理本地登录态。 func (s *AuthorizationStore) RevokeByHash(ctx context.Context, userID, hash int64) (domain.Authorization, bool, error) { var ( a domain.Authorization @@ -315,7 +316,7 @@ FOR UPDATE`, candidate, userID, hash)) if !found { return domain.Authorization{}, false, nil } - if err := deleteRevocationTargetsTx(ctx, tx, []int64{candidate}); err != nil { + if err := deleteRevokedAuthorizationStateTx(ctx, tx, []int64{candidate}); err != nil { return domain.Authorization{}, false, err } return a, true, nil @@ -349,7 +350,8 @@ RETURNING auth_key_id, user_id, hash, layer, device_model, platform, system_vers return out, nil } -// RevokeByUserExcept 批量删除协议 auth_key，保留 keepAuthKeyID 对应的当前设备。 +// RevokeByUserExcept 批量删除业务 authorization，保留 keepAuthKeyID 对应的当前设备； +// 被撤销设备的协议 key/binding 保留，以便重连后取得 RPC 401 并完成客户端退出。 func (s *AuthorizationStore) RevokeByUserExcept(ctx context.Context, userID int64, keepAuthKeyID [8]byte) ([]domain.Authorization, error) { var out []domain.Authorization err := withAuthIdentityTx(ctx, s.db, "revoke authorizations by user", func(tx pgx.Tx) error { @@ -449,13 +451,38 @@ FOR UPDATE`, userID, keepAuthKeyID, candidates) for i := range out { targets[i] = authKeyIDToInt64(out[i].AuthKeyID) } - if err := deleteRevocationTargetsTx(ctx, tx, targets); err != nil { + if err := deleteRevokedAuthorizationStateTx(ctx, tx, targets); err != nil { return nil, err } return out, nil } -func deleteRevocationTargetsTx(ctx context.Context, tx pgx.Tx, authKeyIDs []int64) error { +func deleteRevokedAuthorizationStateTx(ctx context.Context, tx pgx.Tx, authKeyIDs []int64) error { + if len(authKeyIDs) == 0 { + return nil + } + if _, err := tx.Exec(ctx, ` +DELETE FROM update_states +WHERE auth_key_id = ANY($1::bigint[])`, authKeyIDs); err != nil { + return fmt.Errorf("delete revoked update states: %w", err) + } + tag, err := tx.Exec(ctx, ` +DELETE FROM authorizations +WHERE auth_key_id = ANY($1::bigint[])`, authKeyIDs) + if err != nil { + return fmt.Errorf("delete revoked authorizations: %w", err) + } + if tag.RowsAffected() != int64(len(authKeyIDs)) { + return fmt.Errorf("delete revoked authorizations: deleted %d of %d locked targets", tag.RowsAffected(), len(authKeyIDs)) + } + return nil +} + +// deleteProtocolAuthIdentitiesTx permanently removes permanent identities and +// their derived temp keys. Remote account.resetAuthorization/ +// auth.resetAuthorizations must not use this helper: those clients need the +// protocol key long enough to reconnect and receive an RPC-level 401. +func deleteProtocolAuthIdentitiesTx(ctx context.Context, tx pgx.Tx, authKeyIDs []int64) error { if len(authKeyIDs) == 0 { return nil } @@ -466,21 +493,21 @@ WHERE auth_key_id IN ( FROM temp_auth_key_bindings WHERE perm_auth_key_id = ANY($1::bigint[]) )`, authKeyIDs); err != nil { - return fmt.Errorf("delete revoked temporary auth keys: %w", err) + return fmt.Errorf("delete temporary auth keys: %w", err) } if _, err := tx.Exec(ctx, ` DELETE FROM update_states WHERE auth_key_id = ANY($1::bigint[])`, authKeyIDs); err != nil { - return fmt.Errorf("delete revoked update states: %w", err) + return fmt.Errorf("delete protocol identity update states: %w", err) } tag, err := tx.Exec(ctx, ` DELETE FROM auth_keys WHERE auth_key_id = ANY($1::bigint[])`, authKeyIDs) if err != nil { - return fmt.Errorf("delete revoked permanent auth keys: %w", err) + return fmt.Errorf("delete permanent auth keys: %w", err) } if tag.RowsAffected() != int64(len(authKeyIDs)) { - return fmt.Errorf("delete revoked permanent auth keys: deleted %d of %d locked targets", tag.RowsAffected(), len(authKeyIDs)) + return fmt.Errorf("delete permanent auth keys: deleted %d of %d locked targets", tag.RowsAffected(), len(authKeyIDs)) } return nil } diff --git a/internal/store/postgres/authorization_revoke_integration_test.go b/internal/store/postgres/authorization_revoke_integration_test.go index eadb6970..83384a00 100644 --- a/internal/store/postgres/authorization_revoke_integration_test.go +++ b/internal/store/postgres/authorization_revoke_integration_test.go @@ -2,7 +2,6 @@ package postgres import ( "context" - "errors" "fmt" "testing" "time" @@ -13,7 +12,7 @@ import ( "telesrv/internal/store" ) -func TestAuthorizationStoreRevokeByHashDeletesProtocolKeyCascadePostgres(t *testing.T) { +func TestAuthorizationStoreRevokeByHashKeepsProtocolIdentityPostgres(t *testing.T) { pool := testPool(t) ctx := context.Background() userID := createRevokeTestUser(t, ctx, pool, "hash") @@ -43,14 +42,15 @@ func TestAuthorizationStoreRevokeByHashDeletesProtocolKeyCascadePostgres(t *test if err := NewUpdateStateStore(pool).Save(ctx, perm, userID, domain.UpdateState{Pts: 11, Date: int(time.Now().Unix())}); err != nil { t.Fatalf("save update state: %v", err) } - if err := NewTempAuthKeyBindingStore(pool).Save(ctx, domain.TempAuthKeyBinding{ + binding := domain.TempAuthKeyBinding{ TempAuthKeyID: temp, PermAuthKeyID: authKeyIDToInt64(perm), Nonce: 1, TempSessionID: 2, ExpiresAt: tempExpiry, EncryptedMessage: []byte{1, 2, 3, 4}, - }); err != nil { + } + if err := NewTempAuthKeyBindingStore(pool).Save(ctx, binding); err != nil { t.Fatalf("save temp binding: %v", err) } @@ -61,14 +61,14 @@ func TestAuthorizationStoreRevokeByHashDeletesProtocolKeyCascadePostgres(t *test if deleted.AuthKeyID != perm || !deleted.PasswordPending { t.Fatalf("deleted authorization = %+v, want perm key and password_pending", deleted) } - assertRevokeTestMissingAuthKey(t, ctx, keys, perm) - assertRevokeTestMissingAuthKey(t, ctx, keys, temp) + assertRevokeTestPresentAuthKey(t, ctx, keys, perm) + assertRevokeTestPresentAuthKey(t, ctx, keys, temp) assertRevokeTestNoAuthorization(t, ctx, auths, perm) assertRevokeTestTableCount(t, ctx, pool, "update_states", "auth_key_id", authKeyIDToInt64(perm), 0) - assertRevokeTestTableCount(t, ctx, pool, "temp_auth_key_bindings", "temp_auth_key_id", authKeyIDToInt64(temp), 0) + assertTempIdentityBinding(t, ctx, NewTempAuthKeyBindingStore(pool), binding) } -func TestAuthorizationStoreRevokeByUserExceptDeletesOnlyRevokedKeysPostgres(t *testing.T) { +func TestAuthorizationStoreRevokeByUserExceptKeepsRevokedProtocolIdentitiesPostgres(t *testing.T) { pool := testPool(t) ctx := context.Background() userID := createRevokeTestUser(t, ctx, pool, "bulk") @@ -87,14 +87,15 @@ func TestAuthorizationStoreRevokeByUserExceptDeletesOnlyRevokedKeysPostgres(t *t } tempExpiry := int(time.Now().Add(time.Hour).Unix()) saveRevokeTestAuthKey(t, ctx, keys, tempForTwo, tempExpiry) - if err := NewTempAuthKeyBindingStore(pool).Save(ctx, domain.TempAuthKeyBinding{ + binding := domain.TempAuthKeyBinding{ TempAuthKeyID: tempForTwo, PermAuthKeyID: authKeyIDToInt64(revokedTwo), Nonce: 3, TempSessionID: 4, ExpiresAt: tempExpiry, EncryptedMessage: []byte{5, 6, 7, 8}, - }); err != nil { + } + if err := NewTempAuthKeyBindingStore(pool).Save(ctx, binding); err != nil { t.Fatalf("save temp binding: %v", err) } @@ -107,9 +108,12 @@ func TestAuthorizationStoreRevokeByUserExceptDeletesOnlyRevokedKeysPostgres(t *t } assertRevokeTestPresentAuthKey(t, ctx, keys, keep) assertRevokeTestPresentAuthorization(t, ctx, auths, keep) - assertRevokeTestMissingAuthKey(t, ctx, keys, revokedOne) - assertRevokeTestMissingAuthKey(t, ctx, keys, revokedTwo) - assertRevokeTestMissingAuthKey(t, ctx, keys, tempForTwo) + assertRevokeTestPresentAuthKey(t, ctx, keys, revokedOne) + assertRevokeTestPresentAuthKey(t, ctx, keys, revokedTwo) + assertRevokeTestPresentAuthKey(t, ctx, keys, tempForTwo) + assertRevokeTestNoAuthorization(t, ctx, auths, revokedOne) + assertRevokeTestNoAuthorization(t, ctx, auths, revokedTwo) + assertTempIdentityBinding(t, ctx, NewTempAuthKeyBindingStore(pool), binding) } func TestAuthorizationStoreUpdateClientInfoMergesPostgres(t *testing.T) { @@ -155,7 +159,7 @@ func TestAuthorizationStoreUpdateClientInfoMergesPostgres(t *testing.T) { } } -func TestAuthorizationStoreRevokeByHashConcurrentTempBindLeavesNoDanglingStatePostgres(t *testing.T) { +func TestAuthorizationStoreRevokeByHashConcurrentTempBindKeepsProtocolIdentityPostgres(t *testing.T) { pool := testPool(t) ctx := context.Background() userID := createRevokeTestUser(t, ctx, pool, "bind-revoke-race") @@ -203,28 +207,17 @@ func TestAuthorizationStoreRevokeByHashConcurrentTempBindLeavesNoDanglingStatePo close(start) bindErr := <-bindResult - if bindErr != nil && !errors.Is(bindErr, store.ErrAuthKeyBindingInvalid) { + if bindErr != nil { t.Fatalf("attempt %d bind/revoke race bind error = %v", attempt, bindErr) } revoked := <-revokeResults if revoked.err != nil || !revoked.found { t.Fatalf("attempt %d bind/revoke race found=%v err=%v", attempt, revoked.found, revoked.err) } - if _, found, err := bindings.GetByTemp(ctx, temp); err != nil || found { - t.Fatalf("attempt %d dangling binding found=%v err=%v", attempt, found, err) - } - assertRevokeTestMissingAuthKey(t, ctx, keys, perm) + assertTempIdentityBinding(t, ctx, bindings, candidate) + assertRevokeTestPresentAuthKey(t, ctx, keys, perm) + assertRevokeTestPresentAuthKey(t, ctx, keys, temp) assertRevokeTestNoAuthorization(t, ctx, auths, perm) - if bindErr == nil { - assertRevokeTestMissingAuthKey(t, ctx, keys, temp) - } else { - assertTempIdentityAuthKeyExpiry(t, ctx, keys, temp, tempExpiry) - assertRevokeTestNoAuthorization(t, ctx, auths, temp) - if err := keys.Delete(ctx, temp); err != nil { - t.Fatalf("attempt %d clean unbound loser temp: %v", attempt, err) - } - assertRevokeTestMissingAuthKey(t, ctx, keys, temp) - } } } @@ -520,12 +513,10 @@ func TestAuthorizationStoreRevokeByUserExceptPartiallySkipsTransferredCandidateP t.Fatalf("old A state for transferred key found=%v err=%v, want absent", found, err) } - assertRevokeTestMissingAuthKey(t, testCtx, keys, revoked) - assertRevokeTestMissingAuthKey(t, testCtx, keys, revokedTemp) + assertRevokeTestPresentAuthKey(t, testCtx, keys, revoked) + assertRevokeTestPresentAuthKey(t, testCtx, keys, revokedTemp) assertRevokeTestNoAuthorization(t, testCtx, auths, revoked) - if _, found, err := bindings.GetByTemp(testCtx, revokedTemp); err != nil || found { - t.Fatalf("revoked temp binding found=%v err=%v, want absent", found, err) - } + assertTempIdentityBinding(t, testCtx, bindings, revokedBinding) if _, found, err := states.Get(testCtx, revoked, userA); err != nil || found { t.Fatalf("revoked A state found=%v err=%v, want absent", found, err) }