diff --git a/backend/internal/controller/oidc_controller.go b/backend/internal/controller/oidc_controller.go index abf4fc43..fbb2a177 100644 --- a/backend/internal/controller/oidc_controller.go +++ b/backend/internal/controller/oidc_controller.go @@ -470,9 +470,10 @@ func (oc *OidcController) updateAllowedUserGroupsHandler(c *gin.Context) error { // @Summary List authorized clients for current user // @Description Get a paginated list of OIDC clients that the current user has authorized // @Tags OIDC +// @Param search query string false "Search term to filter clients by name" // @Param pagination[page] query int false "Page number for pagination" default(1) // @Param pagination[limit] query int false "Number of items per page" default(20) -// @Param sort[column] query string false "Column to sort by" +// @Param sort[column] query string false "Column to sort by (name or lastUsedAt)" // @Param sort[direction] query string false "Sort direction (asc or desc)" default("asc") // @Param filters[hasLaunchURL] query bool false "Filter clients by whether a launch URL is configured" // @Success 200 {object} dto.Paginated[dto.AuthorizedOidcClientDto] @@ -488,9 +489,10 @@ func (oc *OidcController) listOwnAuthorizedClientsHandler(c *gin.Context) error // @Description Get a paginated list of OIDC clients that a specific user has authorized // @Tags OIDC // @Param id path string true "User ID" +// @Param search query string false "Search term to filter clients by name" // @Param pagination[page] query int false "Page number for pagination" default(1) // @Param pagination[limit] query int false "Number of items per page" default(20) -// @Param sort[column] query string false "Column to sort by" +// @Param sort[column] query string false "Column to sort by (name or lastUsedAt)" // @Param sort[direction] query string false "Sort direction (asc or desc)" default("asc") // @Param filters[hasLaunchURL] query bool false "Filter clients by whether a launch URL is configured" // @Success 200 {object} dto.Paginated[dto.AuthorizedOidcClientDto] @@ -502,9 +504,10 @@ func (oc *OidcController) listAuthorizedClientsHandler(c *gin.Context) error { } func (oc *OidcController) listAuthorizedClients(c *gin.Context, userID string) error { + searchTerm := c.Query("search") listRequestOptions := utils.ParseListRequestOptions(c) - authorizedClients, pagination, err := oc.oidcService.ListAuthorizedClients(c.Request.Context(), userID, listRequestOptions) + authorizedClients, pagination, err := oc.oidcService.ListAuthorizedClients(c.Request.Context(), userID, searchTerm, listRequestOptions) if err != nil { return err } @@ -548,20 +551,22 @@ func (oc *OidcController) revokeOwnClientAuthorizationHandler(c *gin.Context) er // @Summary List accessible OIDC clients for current user // @Description Get a list of OIDC clients that the current user can access // @Tags OIDC +// @Param search query string false "Search term to filter clients by name" // @Param pagination[page] query int false "Page number for pagination" default(1) // @Param pagination[limit] query int false "Number of items per page" default(20) -// @Param sort[column] query string false "Column to sort by" +// @Param sort[column] query string false "Column to sort by (name or lastUsedAt)" // @Param sort[direction] query string false "Sort direction (asc or desc)" default("asc") // @Param filters[hasLaunchURL] query bool false "Filter clients by whether a launch URL is configured" // @Success 200 {object} dto.Paginated[dto.AccessibleOidcClientDto] // @Failure default {object} dto.ErrorDto "Error" // @Router /api/oidc/users/me/clients [get] func (oc *OidcController) listOwnAccessibleClientsHandler(c *gin.Context) error { + searchTerm := c.Query("search") listRequestOptions := utils.ParseListRequestOptions(c) userID := c.GetString("userID") - clients, pagination, err := oc.oidcService.ListAccessibleOidcClients(c.Request.Context(), userID, listRequestOptions) + clients, pagination, err := oc.oidcService.ListAccessibleOidcClients(c.Request.Context(), userID, searchTerm, listRequestOptions) if err != nil { return err } diff --git a/backend/internal/service/oidc_service.go b/backend/internal/service/oidc_service.go index 77de4869..cfe10308 100644 --- a/backend/internal/service/oidc_service.go +++ b/backend/internal/service/oidc_service.go @@ -713,7 +713,7 @@ func (s *OidcService) UpdateAllowedUserGroups(ctx context.Context, id string, in return client, nil } -func (s *OidcService) ListAuthorizedClients(ctx context.Context, userID string, listRequestOptions utils.ListRequestOptions) ([]model.UserAuthorizedOidcClient, utils.PaginationResponse, error) { +func (s *OidcService) ListAuthorizedClients(ctx context.Context, userID string, search string, listRequestOptions utils.ListRequestOptions) ([]model.UserAuthorizedOidcClient, utils.PaginationResponse, error) { tx := s.db.Begin() defer func() { tx.Rollback() @@ -732,15 +732,20 @@ func (s *OidcService) ListAuthorizedClients(ctx context.Context, userID string, return nil, utils.PaginationResponse{}, err } + // Join the clients so the search, the launch URL filter and the name sort can use their columns query := tx. WithContext(ctx). Model(&model.UserAuthorizedOidcClient{}). Preload("Client"). - Where("user_id = ?", userID) + Joins("JOIN oidc_clients ON oidc_clients.id = user_authorized_oidc_clients.client_id"). + Where("user_authorized_oidc_clients.user_id = ?", userID) + + if search != "" { + query = query.Where("LOWER(oidc_clients.name) LIKE ?", "%"+strings.ToLower(search)+"%") + } // Apply the launch URL filter before pagination so hidden authorizations have their own page count if hasLaunchURL, ok := getHasLaunchURLFilter(listRequestOptions); ok { - query = query.Joins("JOIN oidc_clients ON oidc_clients.id = user_authorized_oidc_clients.client_id") if hasLaunchURL { query = query.Where("oidc_clients.launch_url IS NOT NULL AND oidc_clients.launch_url <> ''") } else { @@ -748,6 +753,11 @@ func (s *OidcService) ListAuthorizedClients(ctx context.Context, userID string, } } + // The name lives on the joined client, so the generic sorting of the authorization model cannot handle it + if listRequestOptions.Sort.Column == "name" && utils.IsValidSortDirection(listRequestOptions.Sort.Direction) { + query = query.Order("LOWER(oidc_clients.name) " + utils.NormalizeSortDirection(listRequestOptions.Sort.Direction)) + } + var authorizedClients []model.UserAuthorizedOidcClient response, err := utils.PaginateFilterAndSort(listRequestOptions, query, &authorizedClients) @@ -802,7 +812,7 @@ func (s *OidcService) RevokeAuthorizedClient(ctx context.Context, userID string, return nil } -func (s *OidcService) ListAccessibleOidcClients(ctx context.Context, userID string, listRequestOptions utils.ListRequestOptions) ([]dto.AccessibleOidcClientDto, utils.PaginationResponse, error) { +func (s *OidcService) ListAccessibleOidcClients(ctx context.Context, userID string, search string, listRequestOptions utils.ListRequestOptions) ([]dto.AccessibleOidcClientDto, utils.PaginationResponse, error) { tx := s.db.Begin() defer func() { tx.Rollback() @@ -836,6 +846,10 @@ func (s *OidcService) ListAccessibleOidcClients(ctx context.Context, userID stri WHERE oidc_clients_allowed_user_groups.oidc_client_id = oidc_clients.id AND oidc_clients_allowed_user_groups.user_group_id IN (?))`, false, userGroupIDs) + if search != "" { + query = query.Where("LOWER(oidc_clients.name) LIKE ?", "%"+strings.ToLower(search)+"%") + } + // Apply the launch URL filter before pagination so the app launcher never contains empty pages if hasLaunchURL, ok := getHasLaunchURLFilter(listRequestOptions); ok { if hasLaunchURL { @@ -850,9 +864,11 @@ func (s *OidcService) ListAccessibleOidcClients(ctx context.Context, userID stri // Handle custom sorting for lastUsedAt column var response utils.PaginationResponse if listRequestOptions.Sort.Column == "lastUsedAt" && utils.IsValidSortDirection(listRequestOptions.Sort.Direction) { + // Never used clients share a NULL timestamp, so they are ordered by name to keep pages stable query = query. Joins("LEFT JOIN user_authorized_oidc_clients ON oidc_clients.id = user_authorized_oidc_clients.client_id AND user_authorized_oidc_clients.user_id = ?", userID). - Order("user_authorized_oidc_clients.last_used_at " + listRequestOptions.Sort.Direction + " NULLS LAST") + Order("user_authorized_oidc_clients.last_used_at " + utils.NormalizeSortDirection(listRequestOptions.Sort.Direction) + " NULLS LAST"). + Order("LOWER(oidc_clients.name) ASC") } response, err = utils.PaginateFilterAndSort(listRequestOptions, query, &clients) diff --git a/backend/internal/service/oidc_service_test.go b/backend/internal/service/oidc_service_test.go index d45af17d..d87231fd 100644 --- a/backend/internal/service/oidc_service_test.go +++ b/backend/internal/service/oidc_service_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gorm.io/gorm" "github.com/pocket-id/pocket-id/backend/internal/apperror" "github.com/pocket-id/pocket-id/backend/internal/common" @@ -25,7 +26,7 @@ import ( func TestListAuthorizedClientsRejectsMissingUser(t *testing.T) { service := &OidcService{db: testutils.NewDatabaseForTest(t)} - _, _, err := service.ListAuthorizedClients(t.Context(), "missing-user", utils.ListRequestOptions{}) + _, _, err := service.ListAuthorizedClients(t.Context(), "missing-user", "", utils.ListRequestOptions{}) require.True(t, apperror.IsCode(err, apperror.CodeUserNotFound)) } @@ -1040,11 +1041,11 @@ func TestOidcService_ListAccessibleOidcClients_requiresExplicitGroupPermission(t require.NoError(t, db.Create(&clients[i]).Error) } - groupClients, _, err := s.ListAccessibleOidcClients(t.Context(), userWithGroup.ID, utils.ListRequestOptions{}) + 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{}) + noGroupClients, _, err := s.ListAccessibleOidcClients(t.Context(), userWithoutGroup.ID, "", utils.ListRequestOptions{}) require.NoError(t, err) assert.Equal(t, []string{"Unrestricted"}, accessibleClientNames(noGroupClients)) } @@ -1079,22 +1080,22 @@ func TestOidcService_ListClientViewsFilterByLaunchURLPresence(t *testing.T) { Filters: map[string][]any{"hasLaunchURL": {false}}, } - allClients, allClientsPagination, err := s.ListAccessibleOidcClients(t.Context(), user.ID, utils.ListRequestOptions{}) + allClients, allClientsPagination, err := s.ListAccessibleOidcClients(t.Context(), user.ID, "", utils.ListRequestOptions{}) require.NoError(t, err) assert.Equal(t, int64(3), allClientsPagination.TotalItems) assert.ElementsMatch(t, []string{"Launchable", "Missing launch URL", "Empty launch URL"}, accessibleClientNames(allClients)) - launchableClients, launchablePagination, err := s.ListAccessibleOidcClients(t.Context(), user.ID, withLaunchURL) + launchableClients, launchablePagination, err := s.ListAccessibleOidcClients(t.Context(), user.ID, "", withLaunchURL) require.NoError(t, err) assert.Equal(t, int64(1), launchablePagination.TotalItems) assert.Equal(t, []string{"Launchable"}, accessibleClientNames(launchableClients)) - allAuthorizations, allAuthorizationsPagination, err := s.ListAuthorizedClients(t.Context(), user.ID, utils.ListRequestOptions{}) + allAuthorizations, allAuthorizationsPagination, err := s.ListAuthorizedClients(t.Context(), user.ID, "", utils.ListRequestOptions{}) require.NoError(t, err) assert.Equal(t, int64(3), allAuthorizationsPagination.TotalItems) assert.Len(t, allAuthorizations, 3) - hiddenAuthorizations, hiddenPagination, err := s.ListAuthorizedClients(t.Context(), user.ID, withoutLaunchURL) + hiddenAuthorizations, hiddenPagination, err := s.ListAuthorizedClients(t.Context(), user.ID, "", withoutLaunchURL) require.NoError(t, err) assert.Equal(t, int64(2), hiddenPagination.TotalItems) assert.ElementsMatch(t, []string{"Missing launch URL", "Empty launch URL"}, []string{ @@ -1103,6 +1104,138 @@ func TestOidcService_ListClientViewsFilterByLaunchURLPresence(t *testing.T) { }) } +func TestOidcService_ListAccessibleOidcClients_searchAndSort(t *testing.T) { + forEachTestDatabase(t, func(t *testing.T, db *gorm.DB) { + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil, nil) + require.NoError(t, err) + + user := model.User{Username: "search-and-sort"} + require.NoError(t, db.Create(&user).Error) + + clients := []model.OidcClient{ + {Name: "alpha"}, + {Name: "Bravo"}, + {Name: "charlie Cloud"}, + {Name: "Delta"}, + } + for i := range clients { + require.NoError(t, db.Create(&clients[i]).Error) + } + + // Only Bravo and Delta have been used, so the other two have no last used timestamp + now := time.Now() + require.NoError(t, db.Create(&model.UserAuthorizedOidcClient{UserID: user.ID, ClientID: clients[1].ID, LastUsedAt: datatype.DateTime(now.Add(-time.Hour))}).Error) + require.NoError(t, db.Create(&model.UserAuthorizedOidcClient{UserID: user.ID, ClientID: clients[3].ID, LastUsedAt: datatype.DateTime(now)}).Error) + + list := func(search string, column string, direction string) []string { + options := utils.ListRequestOptions{} + options.Sort.Column = column + options.Sort.Direction = direction + result, _, err := s.ListAccessibleOidcClients(t.Context(), user.ID, search, options) + require.NoError(t, err) + return accessibleClientNames(result) + } + + t.Run("sorts by last used with never used clients last in name order", func(t *testing.T) { + assert.Equal(t, []string{"Delta", "Bravo", "alpha", "charlie Cloud"}, list("", "lastUsedAt", "DESC")) + }) + + t.Run("sorts by name case-insensitively", func(t *testing.T) { + assert.Equal(t, []string{"alpha", "Bravo", "charlie Cloud", "Delta"}, list("", "name", "asc")) + assert.Equal(t, []string{"Delta", "charlie Cloud", "Bravo", "alpha"}, list("", "name", "desc")) + }) + + t.Run("searches the name case-insensitively", func(t *testing.T) { + assert.Equal(t, []string{"charlie Cloud"}, list("CLOUD", "name", "asc")) + assert.Equal(t, []string{"Delta"}, list("lTa", "name", "asc")) + assert.Empty(t, list("missing", "name", "asc")) + }) + + t.Run("combines search and sort", func(t *testing.T) { + assert.Equal(t, []string{"Bravo", "charlie Cloud"}, list("r", "lastUsedAt", "desc")) + }) + }) +} + +func TestOidcService_ListAuthorizedClients_searchAndSort(t *testing.T) { + forEachTestDatabase(t, func(t *testing.T, db *gorm.DB) { + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil, nil) + require.NoError(t, err) + + user := model.User{Username: "authorized-search-and-sort"} + require.NoError(t, db.Create(&user).Error) + + launchURL := "https://launchable.example.com" + clients := []model.OidcClient{ + {Name: "alpha"}, + {Name: "Bravo"}, + {Name: "charlie Cloud"}, + {Name: "Launchable", LaunchURL: &launchURL}, + } + lastUsedOffsets := []time.Duration{-3 * time.Hour, -time.Hour, -2 * time.Hour, 0} + now := time.Now() + for i := range clients { + require.NoError(t, db.Create(&clients[i]).Error) + require.NoError(t, db.Create(&model.UserAuthorizedOidcClient{ + UserID: user.ID, + ClientID: clients[i].ID, + LastUsedAt: datatype.DateTime(now.Add(lastUsedOffsets[i])), + }).Error) + } + + list := func(search string, hasLaunchURL *bool, column string, direction string) []string { + options := utils.ListRequestOptions{} + options.Sort.Column = column + options.Sort.Direction = direction + if hasLaunchURL != nil { + options.Filters = map[string][]any{"hasLaunchURL": {*hasLaunchURL}} + } + result, _, err := s.ListAuthorizedClients(t.Context(), user.ID, search, options) + require.NoError(t, err) + names := make([]string, len(result)) + for i := range result { + names[i] = result[i].Client.Name + } + return names + } + hidden := false + + t.Run("sorts hidden clients by name case-insensitively", func(t *testing.T) { + assert.Equal(t, []string{"alpha", "Bravo", "charlie Cloud"}, list("", &hidden, "name", "asc")) + assert.Equal(t, []string{"charlie Cloud", "Bravo", "alpha"}, list("", &hidden, "name", "desc")) + }) + + t.Run("sorts hidden clients by last used", func(t *testing.T) { + assert.Equal(t, []string{"Bravo", "charlie Cloud", "alpha"}, list("", &hidden, "lastUsedAt", "desc")) + }) + + t.Run("searches the client name case-insensitively", func(t *testing.T) { + assert.Equal(t, []string{"charlie Cloud"}, list("cloud", &hidden, "name", "asc")) + assert.Equal(t, []string{"Launchable"}, list("LAUNCH", nil, "name", "asc")) + assert.Empty(t, list("launch", &hidden, "name", "asc")) + }) + }) +} + +// forEachTestDatabase runs the test on SQLite and Postgres because LIKE and NULL ordering behave differently on them +func forEachTestDatabase(t *testing.T, test func(t *testing.T, db *gorm.DB)) { + t.Run("sqlite", func(t *testing.T) { + test(t, testutils.NewDatabaseForTest(t)) + }) + t.Run("postgres", func(t *testing.T) { + db := testutils.NewPostgresDatabaseForTest(t) + + // Column types such as DateTime pick their encoding from the configured provider + previousProvider := common.EnvConfig.DbProvider + common.EnvConfig.DbProvider = common.DbProviderPostgres + t.Cleanup(func() { + common.EnvConfig.DbProvider = previousProvider + }) + + test(t, db) + }) +} + func accessibleClientNames(clients []dto.AccessibleOidcClientDto) []string { names := make([]string, len(clients)) for i := range clients { diff --git a/frontend/messages/en.json b/frontend/messages/en.json index a0dca327..e401bbf1 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -497,6 +497,13 @@ "show_all_apps": "Show all apps", "show_hidden_apps": "Show hidden apps", "hide_all_apps": "Hide all apps", + "sort_by": "Sort by", + "recently_used": "Recently used", + "name_a_to_z": "Name (A-Z)", + "name_z_to_a": "Name (Z-A)", + "search_apps": "Search apps", + "no_apps_match_your_search": "No apps match your search", + "try_a_different_search_term": "Try a different search term.", "revoke_access": "Revoke Access", "revoke_access_description": "Revoke access to {#b}{clientName}{/b}. {#b}{clientName}{/b} will no longer be able to access your account information.", "revoke_access_successful": "The access to {clientName} has been successfully revoked.", diff --git a/frontend/src/lib/components/list-pagination.svelte b/frontend/src/lib/components/list-pagination.svelte new file mode 100644 index 00000000..f256e80c --- /dev/null +++ b/frontend/src/lib/components/list-pagination.svelte @@ -0,0 +1,82 @@ + + +{#if !hidden} +
+
+

{m.items_per_page()}

+ onPageSizeChange(Number(v))} + > + + {pagination?.itemsPerPage} + + + {#each pageSizes as size (size)} + {size} + {/each} + + +
+ + {#snippet children({ pages })} + + + + + {#each pages as page (page.key)} + {#if page.type !== 'ellipsis' && page.value != 0} + + + {page.value} + + + {/if} + {/each} + + + + + {/snippet} + +
+{/if} diff --git a/frontend/src/lib/components/table/advanced-table.svelte b/frontend/src/lib/components/table/advanced-table.svelte index 0e3793de..cc18e31a 100644 --- a/frontend/src/lib/components/table/advanced-table.svelte +++ b/frontend/src/lib/components/table/advanced-table.svelte @@ -2,9 +2,8 @@ import { goto } from '$app/navigation'; import { page as currentPage } from '$app/state'; import ActionsMenu from '#lib/components/actions-menu.svelte'; + import ListPagination from '#lib/components/list-pagination.svelte'; import Checkbox from '#lib/components/ui/checkbox/checkbox.svelte'; - import * as Pagination from '#lib/components/ui/pagination/index.ts'; - import * as Select from '#lib/components/ui/select/index.ts'; import * as Table from '#lib/components/ui/table/index.js'; import Empty from '#lib/icons/empty.svelte'; import { m } from '#lib/paraglide/messages.js'; @@ -317,50 +316,10 @@ {/if} -
-
-

{m.items_per_page()}

- onPageSizeChange(Number(v))} - > - - {items?.pagination.itemsPerPage} - - - {#each availablePageSizes as size (size)} - {size} - {/each} - - -
- - {#snippet children({ pages })} - - - - - {#each pages as page (page.key)} - {#if page.type !== 'ellipsis' && page.value != 0} - - - {page.value} - - - {/if} - {/each} - - - - - {/snippet} - -
+ {/if} diff --git a/frontend/src/routes/settings/apps/+page.svelte b/frontend/src/routes/settings/apps/+page.svelte index 8999c31f..641fcf5d 100644 --- a/frontend/src/routes/settings/apps/+page.svelte +++ b/frontend/src/routes/settings/apps/+page.svelte @@ -1,23 +1,35 @@