From 22ff1acde3e28c48230541cefd35baf29ed0d1ea Mon Sep 17 00:00:00 2001 From: wucm667 Date: Wed, 20 May 2026 15:52:00 +0800 Subject: [PATCH] =?UTF-8?q?fix(auth):=20=E5=81=9C=E7=94=A8/=E5=88=A0?= =?UTF-8?q?=E9=99=A4=E5=88=86=E7=BB=84=E5=90=8E=E9=98=BB=E6=96=AD=20API=20?= =?UTF-8?q?Key?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...llowed_groups_contract_integration_test.go | 8 +- backend/internal/repository/group_repo.go | 17 +--- .../server/middleware/api_key_auth.go | 26 +++++ .../server/middleware/api_key_auth_google.go | 4 + .../server/middleware/api_key_auth_test.go | 98 +++++++++++++++++++ .../service/admin_service_delete_test.go | 32 ++++++ .../service/api_key_auth_cache_impl.go | 2 +- 7 files changed, 169 insertions(+), 18 deletions(-) diff --git a/backend/internal/repository/allowed_groups_contract_integration_test.go b/backend/internal/repository/allowed_groups_contract_integration_test.go index b0af0d54..5aa95a91 100644 --- a/backend/internal/repository/allowed_groups_contract_integration_test.go +++ b/backend/internal/repository/allowed_groups_contract_integration_test.go @@ -80,7 +80,7 @@ func TestUserRepository_RemoveGroupFromAllowedGroups_RemovesAllOccurrences(t *te require.NotContains(t, u2After.AllowedGroups, targetGroup.ID) } -func TestGroupRepository_DeleteCascade_RemovesAllowedGroupsAndClearsApiKeys(t *testing.T) { +func TestGroupRepository_DeleteCascade_PreservesApiKeyGroupID(t *testing.T) { ctx := context.Background() tx := testEntTx(t) entClient := tx.Client() @@ -138,8 +138,10 @@ func TestGroupRepository_DeleteCascade_RemovesAllowedGroupsAndClearsApiKeys(t *t require.NotContains(t, uAfter.AllowedGroups, targetGroup.ID) require.Contains(t, uAfter.AllowedGroups, otherGroup.ID) - // API keys bound to the deleted group should have group_id cleared. + // API keys keep their group_id so auth can reject keys bound to a deleted group. keyAfter, err := apiKeyRepo.GetByID(ctx, key.ID) require.NoError(t, err) - require.Nil(t, keyAfter.GroupID) + require.NotNil(t, keyAfter.GroupID) + require.Equal(t, targetGroup.ID, *keyAfter.GroupID) + require.Nil(t, keyAfter.Group) } diff --git a/backend/internal/repository/group_repo.go b/backend/internal/repository/group_repo.go index 9b6377bc..c7005c86 100644 --- a/backend/internal/repository/group_repo.go +++ b/backend/internal/repository/group_repo.go @@ -9,7 +9,6 @@ import ( "strings" dbent "github.com/Wei-Shaw/sub2api/ent" - "github.com/Wei-Shaw/sub2api/ent/apikey" "github.com/Wei-Shaw/sub2api/ent/group" "github.com/Wei-Shaw/sub2api/internal/pkg/logger" "github.com/Wei-Shaw/sub2api/internal/pkg/pagination" @@ -636,28 +635,18 @@ func (r *groupRepository) DeleteCascade(ctx context.Context, id int64) ([]int64, } } - // 2. Clear group_id for api keys bound to this group. - // 仅更新未软删除的记录，避免修改已删除数据，保证审计与历史回溯一致性。 - // 与 APIKeyRepository 的软删除语义保持一致，减少跨模块行为差异。 - if _, err := txClient.APIKey.Update(). - Where(apikey.GroupIDEQ(id), apikey.DeletedAtIsNil()). - ClearGroupID(). - Save(ctx); err != nil { - return nil, err - } - - // 3. Remove the group id from user_allowed_groups join table. + // 2. Remove the group id from user_allowed_groups join table. // Legacy users.allowed_groups 列已弃用，不再同步。 if _, err := exec.ExecContext(ctx, "DELETE FROM user_allowed_groups WHERE group_id = $1", id); err != nil { return nil, err } - // 4. Delete account_groups join rows. + // 3. Delete account_groups join rows. if _, err := exec.ExecContext(ctx, "DELETE FROM account_groups WHERE group_id = $1", id); err != nil { return nil, err } - // 5. Soft-delete group itself. + // 4. Soft-delete group itself. if _, err := txClient.Group.Delete().Where(group.IDEQ(id)).Exec(ctx); err != nil { return nil, err } diff --git a/backend/internal/server/middleware/api_key_auth.go b/backend/internal/server/middleware/api_key_auth.go index c15f534e..ee439cda 100644 --- a/backend/internal/server/middleware/api_key_auth.go +++ b/backend/internal/server/middleware/api_key_auth.go @@ -109,6 +109,9 @@ func apiKeyAuthWithSubscription(apiKeyService *service.APIKeyService, subscripti AbortWithError(c, 401, "USER_INACTIVE", "User account is not active") return } + if abortIfAPIKeyGroupUnavailable(c, apiKey) { + return + } // ── 4. SimpleMode → early return ───────────────────────────── @@ -251,3 +254,26 @@ func setGroupContext(c *gin.Context, group *service.Group) { ctx := context.WithValue(c.Request.Context(), ctxkey.Group, group) c.Request = c.Request.WithContext(ctx) } + +func abortIfAPIKeyGroupUnavailable(c *gin.Context, apiKey *service.APIKey) bool { + code, message, ok := validateAPIKeyGroupAvailable(apiKey) + if ok { + return false + } + AbortWithError(c, 403, code, message) + return true +} + +func validateAPIKeyGroupAvailable(apiKey *service.APIKey) (string, string, bool) { + if apiKey == nil || apiKey.GroupID == nil { + return "", "", true + } + group := apiKey.Group + if group == nil || strings.EqualFold(group.Status, "deleted") { + return "GROUP_DELETED", "API Key 所属分组已删除", false + } + if !group.IsActive() { + return "GROUP_DISABLED", "API Key 所属分组已停用", false + } + return "", "", true +} diff --git a/backend/internal/server/middleware/api_key_auth_google.go b/backend/internal/server/middleware/api_key_auth_google.go index 84d93edc..3ed71f71 100644 --- a/backend/internal/server/middleware/api_key_auth_google.go +++ b/backend/internal/server/middleware/api_key_auth_google.go @@ -54,6 +54,10 @@ func APIKeyAuthWithSubscriptionGoogle(apiKeyService *service.APIKeyService, subs abortWithGoogleError(c, 401, "User account is not active") return } + if _, message, ok := validateAPIKeyGroupAvailable(apiKey); !ok { + abortWithGoogleError(c, 403, message) + return + } // 简易模式：跳过余额和订阅检查 if cfg.RunMode == config.RunModeSimple { diff --git a/backend/internal/server/middleware/api_key_auth_test.go b/backend/internal/server/middleware/api_key_auth_test.go index d6760d8d..a00f70c7 100644 --- a/backend/internal/server/middleware/api_key_auth_test.go +++ b/backend/internal/server/middleware/api_key_auth_test.go @@ -300,6 +300,104 @@ func TestAPIKeyAuthOverwritesInvalidContextGroup(t *testing.T) { require.Equal(t, http.StatusOK, w.Code) } +func TestAPIKeyAuthRejectsUnavailableGroup(t *testing.T) { + gin.SetMode(gin.TestMode) + + groupID := int64(101) + user := &service.User{ + ID: 7, + Role: service.RoleUser, + Status: service.StatusActive, + Balance: 10, + Concurrency: 3, + } + + tests := []struct { + name string + group *service.Group + wantStatus int + wantCode string + }{ + { + name: "active group passes", + group: &service.Group{ + ID: groupID, + Name: "active", + Status: service.StatusActive, + Platform: service.PlatformAnthropic, + Hydrated: true, + }, + wantStatus: http.StatusOK, + }, + { + name: "disabled group is forbidden", + group: &service.Group{ + ID: groupID, + Name: "disabled", + Status: service.StatusDisabled, + Platform: service.PlatformAnthropic, + Hydrated: true, + }, + wantStatus: http.StatusForbidden, + wantCode: "GROUP_DISABLED", + }, + { + name: "deleted status group is forbidden", + group: &service.Group{ + ID: groupID, + Name: "deleted", + Status: "deleted", + Platform: service.PlatformAnthropic, + Hydrated: true, + }, + wantStatus: http.StatusForbidden, + wantCode: "GROUP_DELETED", + }, + { + name: "missing group edge is forbidden", + group: nil, + wantStatus: http.StatusForbidden, + wantCode: "GROUP_DELETED", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + apiKey := &service.APIKey{ + ID: 100, + UserID: user.ID, + GroupID: &groupID, + Key: "test-key", + Status: service.StatusActive, + User: user, + Group: tt.group, + } + apiKeyRepo := &stubApiKeyRepo{ + getByKey: func(ctx context.Context, key string) (*service.APIKey, error) { + if key != apiKey.Key { + return nil, service.ErrAPIKeyNotFound + } + clone := *apiKey + return &clone, nil + }, + } + cfg := &config.Config{RunMode: config.RunModeStandard} + apiKeyService := service.NewAPIKeyService(apiKeyRepo, nil, nil, nil, nil, nil, cfg) + router := newAuthTestRouter(apiKeyService, nil, cfg) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/t", nil) + req.Header.Set("x-api-key", apiKey.Key) + router.ServeHTTP(w, req) + + require.Equal(t, tt.wantStatus, w.Code) + if tt.wantCode != "" { + require.Contains(t, w.Body.String(), tt.wantCode) + } + }) + } +} + func TestAPIKeyAuthIPRestrictionDoesNotTrustSpoofedForwardHeaders(t *testing.T) { gin.SetMode(gin.TestMode) diff --git a/backend/internal/service/admin_service_delete_test.go b/backend/internal/service/admin_service_delete_test.go index a9492a1d..c1da92d1 100644 --- a/backend/internal/service/admin_service_delete_test.go +++ b/backend/internal/service/admin_service_delete_test.go @@ -244,6 +244,21 @@ func (s *groupRepoStub) UpdateSortOrders(ctx context.Context, updates []GroupSor return nil } +type deleteGroupAPIKeyRepoStub struct { + apiKeyRepoStubForGroupUpdate + keys []string + listErr error + listGroupIDs []int64 +} + +func (s *deleteGroupAPIKeyRepoStub) ListKeysByGroupID(ctx context.Context, groupID int64) ([]string, error) { + s.listGroupIDs = append(s.listGroupIDs, groupID) + if s.listErr != nil { + return nil, s.listErr + } + return s.keys, nil +} + type proxyRepoStub struct { deleteErr error countErr error @@ -500,6 +515,23 @@ func TestAdminService_DeleteGroup_Success_WithCacheInvalidation(t *testing.T) { }, calls) } +func TestAdminService_DeleteGroup_InvalidatesAuthCacheForBoundKeys(t *testing.T) { + repo := &groupRepoStub{} + apiKeyRepo := &deleteGroupAPIKeyRepoStub{keys: []string{"k1", "k2"}} + invalidator := &authCacheInvalidatorStub{} + svc := &adminServiceImpl{ + groupRepo: repo, + apiKeyRepo: apiKeyRepo, + authCacheInvalidator: invalidator, + } + + err := svc.DeleteGroup(context.Background(), 5) + require.NoError(t, err) + require.Equal(t, []int64{5}, repo.deleteCalls) + require.Equal(t, []int64{5}, apiKeyRepo.listGroupIDs) + require.Equal(t, []string{"k1", "k2"}, invalidator.keys) +} + func TestAdminService_DeleteGroup_NotFound(t *testing.T) { repo := &groupRepoStub{deleteErr: ErrGroupNotFound} svc := &adminServiceImpl{groupRepo: repo} diff --git a/backend/internal/service/api_key_auth_cache_impl.go b/backend/internal/service/api_key_auth_cache_impl.go index 877888b1..c752ce28 100644 --- a/backend/internal/service/api_key_auth_cache_impl.go +++ b/backend/internal/service/api_key_auth_cache_impl.go @@ -14,7 +14,7 @@ import ( "github.com/dgraph-io/ristretto" ) -const apiKeyAuthSnapshotVersion = 9 // v9: added API Key name for audit logs +const apiKeyAuthSnapshotVersion = 10 // v10: reload snapshots for group availability checks type apiKeyAuthCacheConfig struct { l1Size int