diff --git a/backend/internal/controller/oidc_controller.go b/backend/internal/controller/oidc_controller.go index 94d4afce..66a25569 100644 --- a/backend/internal/controller/oidc_controller.go +++ b/backend/internal/controller/oidc_controller.go @@ -394,6 +394,7 @@ func (oc *OidcController) updateAllowedUserGroupsHandler(c *gin.Context) error { // @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[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] // @Router /api/oidc/users/me/authorized-clients [get] func (oc *OidcController) listOwnAuthorizedClientsHandler(c *gin.Context) error { @@ -410,6 +411,7 @@ func (oc *OidcController) listOwnAuthorizedClientsHandler(c *gin.Context) error // @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[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] // @Router /api/oidc/users/{id}/authorized-clients [get] func (oc *OidcController) listAuthorizedClientsHandler(c *gin.Context) error { @@ -467,6 +469,7 @@ func (oc *OidcController) revokeOwnClientAuthorizationHandler(c *gin.Context) er // @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[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] // @Router /api/oidc/users/me/clients [get] func (oc *OidcController) listOwnAccessibleClientsHandler(c *gin.Context) error { diff --git a/backend/internal/service/e2etest_service.go b/backend/internal/service/e2etest_service.go index 3bc1ba91..37e3cfa0 100644 --- a/backend/internal/service/e2etest_service.go +++ b/backend/internal/service/e2etest_service.go @@ -192,6 +192,7 @@ func (s *TestService) SeedDatabase(baseURL string) error { ID: "606c7782-f2b1-49e5-8ea9-26eb1b06d018", }, Name: "Immich", + LaunchURL: new("https://immich.local"), Secret: "$2a$10$Ak.FP8riD1ssy2AGGbG.gOpnp/rBpymd74j0nxNMtW0GG1Lb4gzxe", // PYjrE9u4v9GVqXKi52eur0eb2Ci4kc0x CallbackURLs: datatype.StringList{"http://immich.localhost/auth/callback"}, CreatedByID: new(users[1].ID), diff --git a/backend/internal/service/oidc_service.go b/backend/internal/service/oidc_service.go index 9498ee16..2e5664da 100644 --- a/backend/internal/service/oidc_service.go +++ b/backend/internal/service/oidc_service.go @@ -592,6 +592,16 @@ func (s *OidcService) ListAuthorizedClients(ctx context.Context, userID string, Preload("Client"). Where("user_id = ?", userID) + // 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 { + query = query.Where("oidc_clients.launch_url IS NULL OR oidc_clients.launch_url = ''") + } + } + var authorizedClients []model.UserAuthorizedOidcClient response, err := utils.PaginateFilterAndSort(listRequestOptions, query, &authorizedClients) @@ -667,6 +677,15 @@ 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) + // Apply the launch URL filter before pagination so the app launcher never contains empty pages + if hasLaunchURL, ok := getHasLaunchURLFilter(listRequestOptions); ok { + if hasLaunchURL { + query = query.Where("oidc_clients.launch_url IS NOT NULL AND oidc_clients.launch_url <> ''") + } else { + query = query.Where("oidc_clients.launch_url IS NULL OR oidc_clients.launch_url = ''") + } + } + var clients []model.OidcClient // Handle custom sorting for lastUsedAt column @@ -705,6 +724,16 @@ func (s *OidcService) ListAccessibleOidcClients(ctx context.Context, userID stri return dtos, response, err } +func getHasLaunchURLFilter(listRequestOptions utils.ListRequestOptions) (bool, bool) { + values := listRequestOptions.Filters["hasLaunchURL"] + if len(values) == 0 { + return false, false + } + + hasLaunchURL, ok := values[0].(bool) + return hasLaunchURL, ok +} + func (s *OidcService) GetClientPreview(ctx context.Context, clientID string, userID string, scopes []string, authenticationMethod string) (*dto.OidcClientPreviewDto, error) { client, err := s.getClientInternal(ctx, clientID, s.db, false) if err != nil { diff --git a/backend/internal/service/oidc_service_test.go b/backend/internal/service/oidc_service_test.go index 73fdd0e2..e3acc047 100644 --- a/backend/internal/service/oidc_service_test.go +++ b/backend/internal/service/oidc_service_test.go @@ -858,6 +858,60 @@ func TestOidcService_ListAccessibleOidcClients_requiresExplicitGroupPermission(t assert.Equal(t, []string{"Unrestricted"}, accessibleClientNames(noGroupClients)) } +func TestOidcService_ListClientViewsFilterByLaunchURLPresence(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + require.NoError(t, err) + + user := model.User{Username: "launch-url-filter"} + require.NoError(t, db.Create(&user).Error) + + launchURL := "https://launchable.example.com" + emptyLaunchURL := "" + clients := []model.OidcClient{ + {Name: "Launchable", LaunchURL: &launchURL}, + {Name: "Missing launch URL"}, + {Name: "Empty launch URL", LaunchURL: &emptyLaunchURL}, + } + 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, + }).Error) + } + + withLaunchURL := utils.ListRequestOptions{ + Filters: map[string][]any{"hasLaunchURL": {true}}, + } + withoutLaunchURL := utils.ListRequestOptions{ + Filters: map[string][]any{"hasLaunchURL": {false}}, + } + + 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) + 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{}) + 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) + require.NoError(t, err) + assert.Equal(t, int64(2), hiddenPagination.TotalItems) + assert.ElementsMatch(t, []string{"Missing launch URL", "Empty launch URL"}, []string{ + hiddenAuthorizations[0].Client.Name, + hiddenAuthorizations[1].Client.Name, + }) +} + 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 8a368501..f4d46cf6 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -491,6 +491,9 @@ "client_name_description": "The name of the client that shows in the Pocket ID UI.", "client_description": "Description", "client_description_description": "An optional description of the client that shows in the Pocket ID UI.", + "show_all_apps": "Show all apps", + "show_hidden_apps": "Show hidden apps", + "hide_all_apps": "Hide all apps", "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/ui/empty/empty-content.svelte b/frontend/src/lib/components/ui/empty/empty-content.svelte new file mode 100644 index 00000000..15451b44 --- /dev/null +++ b/frontend/src/lib/components/ui/empty/empty-content.svelte @@ -0,0 +1,23 @@ + + +
+ {@render children?.()} +
diff --git a/frontend/src/lib/components/ui/empty/empty-description.svelte b/frontend/src/lib/components/ui/empty/empty-description.svelte new file mode 100644 index 00000000..f540551c --- /dev/null +++ b/frontend/src/lib/components/ui/empty/empty-description.svelte @@ -0,0 +1,23 @@ + + +
a]:underline [&>a]:underline-offset-4 [&>a:hover]:text-primary', + className + )} + {...restProps} +> + {@render children?.()} +
diff --git a/frontend/src/lib/components/ui/empty/empty-header.svelte b/frontend/src/lib/components/ui/empty/empty-header.svelte new file mode 100644 index 00000000..09569974 --- /dev/null +++ b/frontend/src/lib/components/ui/empty/empty-header.svelte @@ -0,0 +1,20 @@ + + +
+ {@render children?.()} +
diff --git a/frontend/src/lib/components/ui/empty/empty-media.svelte b/frontend/src/lib/components/ui/empty/empty-media.svelte new file mode 100644 index 00000000..db2d1033 --- /dev/null +++ b/frontend/src/lib/components/ui/empty/empty-media.svelte @@ -0,0 +1,41 @@ + + + + +
+ {@render children?.()} +
diff --git a/frontend/src/lib/components/ui/empty/empty-title.svelte b/frontend/src/lib/components/ui/empty/empty-title.svelte new file mode 100644 index 00000000..8d00c2b2 --- /dev/null +++ b/frontend/src/lib/components/ui/empty/empty-title.svelte @@ -0,0 +1,20 @@ + + +
+ {@render children?.()} +
diff --git a/frontend/src/lib/components/ui/empty/empty.svelte b/frontend/src/lib/components/ui/empty/empty.svelte new file mode 100644 index 00000000..fcaa4328 --- /dev/null +++ b/frontend/src/lib/components/ui/empty/empty.svelte @@ -0,0 +1,23 @@ + + +
+ {@render children?.()} +
diff --git a/frontend/src/lib/components/ui/empty/index.ts b/frontend/src/lib/components/ui/empty/index.ts new file mode 100644 index 00000000..c74f8f31 --- /dev/null +++ b/frontend/src/lib/components/ui/empty/index.ts @@ -0,0 +1,22 @@ +import Content from './empty-content.svelte'; +import Description from './empty-description.svelte'; +import Header from './empty-header.svelte'; +import Media from './empty-media.svelte'; +import Title from './empty-title.svelte'; +import Root from './empty.svelte'; + +export { + Root, + Header, + Media, + Title, + Description, + Content, + // + Root as Empty, + Header as EmptyHeader, + Media as EmptyMedia, + Title as EmptyTitle, + Description as EmptyDescription, + Content as EmptyContent +}; diff --git a/frontend/src/lib/components/ui/tooltip/tooltip-content.svelte b/frontend/src/lib/components/ui/tooltip/tooltip-content.svelte index d3d2097d..35b95fef 100644 --- a/frontend/src/lib/components/ui/tooltip/tooltip-content.svelte +++ b/frontend/src/lib/components/ui/tooltip/tooltip-content.svelte @@ -27,17 +27,21 @@ {sideOffset} {side} class={cn( - 'data-open:animate-in data-open:fade-in-0 data-open:zoom-in-95 data-[state=delayed-open]:animate-in data-[state=delayed-open]:fade-in-0 data-[state=delayed-open]:zoom-in-95 data-closed:animate-out data-closed:fade-out-0 data-closed:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 inline-flex items-center gap-1.5 rounded-xl px-3 py-1.5 text-xs has-data-[slot=kbd]:pr-1.5 **:data-[slot=kbd]:relative **:data-[slot=kbd]:isolate **:data-[slot=kbd]:z-50 **:data-[slot=kbd]:rounded-lg bg-foreground text-background z-50 w-fit max-w-xs origin-(--bits-tooltip-content-transform-origin)', + 'data-open:animate-in data-open:fade-in-0 data-open:zoom-in-95 data-[state=delayed-open]:animate-in data-[state=delayed-open]:fade-in-0 data-[state=delayed-open]:zoom-in-95 data-closed:animate-out data-closed:fade-out-0 data-closed:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 ring-foreground/5 text-popover-foreground relative isolate inline-flex w-fit max-w-xs origin-(--bits-tooltip-content-transform-origin) items-center gap-1.5 rounded-xl px-3 py-1.5 text-xs shadow-lg ring-1 has-data-[slot=kbd]:pr-1.5 **:data-[slot=kbd]:relative **:data-[slot=kbd]:isolate **:data-[slot=kbd]:z-50 **:data-[slot=kbd]:rounded-lg z-50', className )} {...restProps} > + {@render children?.()} {#snippet child({ props })}
+ + diff --git a/frontend/src/lib/services/oidc-service.ts b/frontend/src/lib/services/oidc-service.ts index 1fcf6abc..5234b85a 100644 --- a/frontend/src/lib/services/oidc-service.ts +++ b/frontend/src/lib/services/oidc-service.ts @@ -1,6 +1,7 @@ import type { ListRequestOptions, Paginated } from '$lib/types/list-request.type'; import type { AccessibleOidcClient, + AuthorizedOidcClient, CompleteInteractionResponse, InteractionSession, InteractionStep, @@ -121,6 +122,11 @@ class OidcService extends APIService { return res.data as Paginated; }; + listOwnAuthorizedClients = async (options?: ListRequestOptions) => { + const res = await this.api.get('/oidc/users/me/authorized-clients', { params: options }); + return res.data as Paginated; + }; + revokeOwnAuthorizedClient = async (clientId: string) => { await this.api.delete(`/oidc/users/me/authorized-clients/${encodeClientIdParam(clientId)}`); }; diff --git a/frontend/src/lib/types/oidc.type.ts b/frontend/src/lib/types/oidc.type.ts index 8ed34ddf..f7463454 100644 --- a/frontend/src/lib/types/oidc.type.ts +++ b/frontend/src/lib/types/oidc.type.ts @@ -94,6 +94,12 @@ export type AccessibleOidcClient = OidcClientMetaData & { lastUsedAt: Date | null; }; +export type AuthorizedOidcClient = { + scope: string; + client: OidcClientMetaData; + lastUsedAt: Date; +}; + export type InteractionStep = 'authenticate' | 'select_account' | 'reauthenticate' | 'consent'; export type InteractionScopeInfo = { diff --git a/frontend/src/routes/settings/apps/+page.svelte b/frontend/src/routes/settings/apps/+page.svelte index 9ffb02dd..a8d01963 100644 --- a/frontend/src/routes/settings/apps/+page.svelte +++ b/frontend/src/routes/settings/apps/+page.svelte @@ -1,28 +1,65 @@ - +
{#if client.hasLogo} @@ -124,17 +125,18 @@ {:else}
{/if} - + {#if client.launchURL} + + {/if}
diff --git a/tests/data.ts b/tests/data.ts index f6e256ae..92e9c8f2 100644 --- a/tests/data.ts +++ b/tests/data.ts @@ -37,7 +37,8 @@ export const oidcClients = { id: '606c7782-f2b1-49e5-8ea9-26eb1b06d018', name: 'Immich', callbackUrl: 'http://immich.localhost/auth/callback', - secret: 'PYjrE9u4v9GVqXKi52eur0eb2Ci4kc0x' + secret: 'PYjrE9u4v9GVqXKi52eur0eb2Ci4kc0x', + launchURL: 'https://immich.local' }, tailscale: { id: '7c21a609-96b5-4011-9900-272b8d31a9d1', diff --git a/tests/resources/export/database.json b/tests/resources/export/database.json index bd47b034..b27915c8 100644 --- a/tests/resources/export/database.json +++ b/tests/resources/export/database.json @@ -124,7 +124,7 @@ "image_type": null, "is_group_restricted": true, "is_public": false, - "launch_url": null, + "launch_url": "https://immich.local", "logout_callback_urls": "bnVsbA==", "metadata_expires_at": null, "metadata_grant_types": "bnVsbA==", diff --git a/tests/specs/apps-dashboard.spec.ts b/tests/specs/apps-dashboard.spec.ts index 3473c8e4..0145774a 100644 --- a/tests/specs/apps-dashboard.spec.ts +++ b/tests/specs/apps-dashboard.spec.ts @@ -5,63 +5,71 @@ import { cleanupBackend } from '../utils/cleanup.util'; test.beforeEach(async () => await cleanupBackend()); -test('Dashboard shows all clients in the correct order', async ({ page }) => { - const client1 = oidcClients.tailscale; - const client2 = oidcClients.nextcloud; +test('Dashboard shows only clients with launch URLs in the correct order', async ({ page }) => { + const client1 = oidcClients.nextcloud; + const client2 = oidcClients.immich; await page.goto('/settings/apps'); - await expect(page.getByTestId('authorized-oidc-client-card')).toHaveCount(7); + const appCards = page.getByRole('article'); + await expect(appCards).toHaveCount(2); // Should be first - const card1 = page.getByTestId('authorized-oidc-client-card').first(); + const card1 = appCards.first(); await expect(card1.getByRole('heading')).toHaveText(client1.name); + await expect(card1.getByText(new URL(client1.launchURL).hostname)).toBeVisible(); - const card2 = page.getByTestId('authorized-oidc-client-card').nth(1); + const card2 = page.getByRole('article', { name: client2.name }); await expect(card2.getByRole('heading', { name: client2.name })).toBeVisible(); await expect(card2.getByText(new URL(client2.launchURL).hostname)).toBeVisible(); + + await expect(page.getByRole('article', { name: oidcClients.tailscale.name })).toHaveCount(0); }); test.describe('Dashboard shows only clients where user has access', () => { - test("User can't see one client", async ({ page }) => { + test("User can't see a restricted launchable client", async ({ page }) => { await authUtil.changeUser(page, 'craig'); - const notVisibleClient = oidcClients.immich; - await page.goto('/settings/apps'); - const cards = page.getByTestId('authorized-oidc-client-card'); - - await expect(cards).toHaveCount(6); - - const cardTexts = await cards.allTextContents(); - expect(cardTexts.some((text) => text.includes(notVisibleClient.name))).toBe(false); + await expect(page.getByRole('article')).toHaveCount(1); + await expect(page.getByRole('article', { name: oidcClients.nextcloud.name })).toBeVisible(); + await expect(page.getByRole('article', { name: oidcClients.immich.name })).toHaveCount(0); }); - test('User can see all clients', async ({ page }) => { + + test('User can see every accessible launchable client', async ({ page }) => { await page.goto('/settings/apps'); - const cards = page.getByTestId('authorized-oidc-client-card'); - await expect(cards).toHaveCount(7); + + await expect(page.getByRole('article')).toHaveCount(2); + await expect(page.getByRole('article', { name: oidcClients.nextcloud.name })).toBeVisible(); + await expect(page.getByRole('article', { name: oidcClients.immich.name })).toBeVisible(); }); }); -test('Revoke authorized client', async ({ page }) => { +test('Show and revoke a hidden authorized client in the app grid', async ({ page }) => { const client = oidcClients.tailscale; await page.goto('/settings/apps'); - const card = page.getByTestId('authorized-oidc-client-card').filter({ hasText: client.name }); + const appCards = page.getByRole('article'); + const clientCard = page.getByRole('article', { name: client.name }); + await expect(clientCard).toHaveCount(0); - card.getByRole('button', { name: 'Toggle menu' }).click(); + await page.getByRole('button', { name: /Show all apps/ }).click(); + await expect(appCards).toHaveCount(4); + await expect(clientCard).toBeVisible(); + await expect(page.getByRole('main').getByRole('separator')).toBeVisible(); + await expect(clientCard.getByRole('link', { name: 'Launch' })).toHaveCount(0); + await expect(clientCard.getByRole('button', { name: 'Revoke' })).toHaveCount(0); + await clientCard.getByRole('button', { name: 'Toggle menu' }).click(); await page.getByRole('menuitem', { name: 'Revoke' }).click(); - await page.getByRole('button', { name: 'Revoke' }).click(); + await page.getByRole('alertdialog').getByRole('button', { name: 'Revoke' }).click(); - await expect(page.locator('[data-type="success"]')).toHaveText( - `The access to ${client.name} has been successfully revoked.` - ); - - // The ... ago text should be gone as there is no last access anymore - await expect(card).not.toContainText('ago'); + await expect( + page.getByText(`The access to ${client.name} has been successfully revoked.`, { exact: true }) + ).toBeVisible(); + await expect(clientCard).toHaveCount(0); }); test('Launch authorized client', async ({ page }) => { @@ -69,11 +77,11 @@ test('Launch authorized client', async ({ page }) => { await page.goto('/settings/apps'); - const card1 = page.getByTestId('authorized-oidc-client-card').first(); - await expect(card1.getByRole('button', { name: 'Launch' })).toBeDisabled(); + const appCards = page.getByRole('article'); + await expect(appCards.getByRole('link', { name: 'Launch' })).toHaveCount(2); - const card2 = page.getByTestId('authorized-oidc-client-card').nth(1); - await expect(card2.getByRole('link', { name: 'Launch' })).toHaveAttribute( + const card = page.getByRole('article', { name: client.name }); + await expect(card.getByRole('link', { name: 'Launch' })).toHaveAttribute( 'href', client.launchURL );