From e10f66c07a8f4a8d42a03bb7edda14d4bf9ba31d Mon Sep 17 00:00:00 2001 From: Elias Schneider Date: Wed, 22 Jul 2026 18:19:07 +0200 Subject: [PATCH] fix: show only accessible clients on "My Apps" page --- backend/internal/service/e2etest_service.go | 3 ++ backend/internal/service/oidc_service.go | 22 +++------- backend/internal/service/oidc_service_test.go | 43 +++++++++++++++++++ 3 files changed, 51 insertions(+), 17 deletions(-) diff --git a/backend/internal/service/e2etest_service.go b/backend/internal/service/e2etest_service.go index 0d25f85a..22b36638 100644 --- a/backend/internal/service/e2etest_service.go +++ b/backend/internal/service/e2etest_service.go @@ -220,6 +220,9 @@ func (s *TestService) SeedDatabase(baseURL string) error { LogoutCallbackURLs: model.UrlList{"http://tailscale.localhost/auth/logout/callback"}, IsGroupRestricted: true, CreatedByID: new(users[0].ID), + AllowedUserGroups: []model.UserGroup{ + userGroups[0], + }, }, { Base: model.Base{ diff --git a/backend/internal/service/oidc_service.go b/backend/internal/service/oidc_service.go index 5f259a48..805e6076 100644 --- a/backend/internal/service/oidc_service.go +++ b/backend/internal/service/oidc_service.go @@ -587,23 +587,11 @@ func (s *OidcService) ListAccessibleOidcClients(ctx context.Context, userID stri query := tx. WithContext(ctx). Model(&model.OidcClient{}). - Preload("UserAuthorizedOidcClients", "user_id = ?", userID) - - // If user has no groups, only return clients with no allowed user groups - if len(userGroupIDs) == 0 { - query = query.Where(`NOT EXISTS ( - SELECT 1 FROM oidc_clients_allowed_user_groups - WHERE oidc_clients_allowed_user_groups.oidc_client_id = oidc_clients.id)`) - } else { - query = query.Where(` - NOT EXISTS ( - SELECT 1 FROM oidc_clients_allowed_user_groups - WHERE oidc_clients_allowed_user_groups.oidc_client_id = oidc_clients.id - ) OR EXISTS ( - SELECT 1 FROM oidc_clients_allowed_user_groups - WHERE oidc_clients_allowed_user_groups.oidc_client_id = oidc_clients.id - AND oidc_clients_allowed_user_groups.user_group_id IN (?))`, userGroupIDs) - } + Preload("UserAuthorizedOidcClients", "user_id = ?", userID). + Where(`oidc_clients.is_group_restricted = ? OR EXISTS ( + SELECT 1 FROM oidc_clients_allowed_user_groups + WHERE oidc_clients_allowed_user_groups.oidc_client_id = oidc_clients.id + AND oidc_clients_allowed_user_groups.user_group_id IN (?))`, false, userGroupIDs) var clients []model.OidcClient diff --git a/backend/internal/service/oidc_service_test.go b/backend/internal/service/oidc_service_test.go index 2d769e71..0f233c8a 100644 --- a/backend/internal/service/oidc_service_test.go +++ b/backend/internal/service/oidc_service_test.go @@ -14,6 +14,7 @@ import ( "github.com/pocket-id/pocket-id/backend/internal/dto" "github.com/pocket-id/pocket-id/backend/internal/model" "github.com/pocket-id/pocket-id/backend/internal/storage" + "github.com/pocket-id/pocket-id/backend/internal/utils" testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" ) @@ -538,3 +539,45 @@ func TestOidcService_UpdateClient_description(t *testing.T) { require.NoError(t, err) assert.Empty(t, fetched.Description) } + +func TestOidcService_ListAccessibleOidcClients_requiresExplicitGroupPermission(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + s, err := NewOidcService(db, nil, nil, nil, nil, nil) + require.NoError(t, err) + + allowedGroup := model.UserGroup{Name: "allowed", FriendlyName: "Allowed"} + otherGroup := model.UserGroup{Name: "other", FriendlyName: "Other"} + require.NoError(t, db.Create(&allowedGroup).Error) + require.NoError(t, db.Create(&otherGroup).Error) + + userWithGroup := model.User{Username: "with-group", UserGroups: []model.UserGroup{allowedGroup}} + userWithoutGroup := model.User{Username: "without-group"} + require.NoError(t, db.Create(&userWithGroup).Error) + require.NoError(t, db.Create(&userWithoutGroup).Error) + + clients := []model.OidcClient{ + {Name: "Unrestricted", CallbackURLs: model.UrlList{"https://unrestricted.example.com/callback"}}, + {Name: "Restricted without groups", CallbackURLs: model.UrlList{"https://empty.example.com/callback"}, IsGroupRestricted: true}, + {Name: "Restricted to user group", CallbackURLs: model.UrlList{"https://allowed.example.com/callback"}, IsGroupRestricted: true, AllowedUserGroups: []model.UserGroup{allowedGroup}}, + {Name: "Restricted to other group", CallbackURLs: model.UrlList{"https://other.example.com/callback"}, IsGroupRestricted: true, AllowedUserGroups: []model.UserGroup{otherGroup}}, + } + for i := range clients { + require.NoError(t, db.Create(&clients[i]).Error) + } + + groupClients, _, err := s.ListAccessibleOidcClients(t.Context(), userWithGroup.ID, utils.ListRequestOptions{}) + require.NoError(t, err) + assert.ElementsMatch(t, []string{"Unrestricted", "Restricted to user group"}, accessibleClientNames(groupClients)) + + noGroupClients, _, err := s.ListAccessibleOidcClients(t.Context(), userWithoutGroup.ID, utils.ListRequestOptions{}) + require.NoError(t, err) + assert.Equal(t, []string{"Unrestricted"}, accessibleClientNames(noGroupClients)) +} + +func accessibleClientNames(clients []dto.AccessibleOidcClientDto) []string { + names := make([]string, len(clients)) + for i := range clients { + names[i] = clients[i].Name + } + return names +}