diff --git a/backend/internal/oidc/authorization_service.go b/backend/internal/oidc/authorization_service.go index 7eeb88a0..026606dd 100644 --- a/backend/internal/oidc/authorization_service.go +++ b/backend/internal/oidc/authorization_service.go @@ -4,6 +4,7 @@ import ( "context" "errors" "log/slog" + "maps" "net/url" "slices" "strconv" @@ -451,15 +452,18 @@ func (r interactionRequirements) any() bool { func (s *authorizationService) createInteractionSession(ctx context.Context, requester fosite.AuthorizeRequester, requestParams map[string]string, userID string, requirements interactionRequirements) (InteractionSession, error) { parameters := make(map[string]string, len(requestParams)) - for key, value := range requestParams { - parameters[key] = value + maps.Copy(parameters, requestParams) + + scopes := requester.GetRequestedScopes() + if scopes == nil { + scopes = []string{} } return s.interactionSessionService.create(ctx, InteractionSession{ Base: model.Base{ ID: requester.GetID(), }, - Scopes: datatype.StringList(requester.GetRequestedScopes()), + Scopes: datatype.StringList(scopes), ClientID: requester.GetClient().GetID(), UserID: utils.PtrOrNil(userID), ConsentRequired: requirements.ConsentRequired, diff --git a/backend/internal/oidc/device_service.go b/backend/internal/oidc/device_service.go index 6ea04232..ed702d2c 100644 --- a/backend/internal/oidc/device_service.go +++ b/backend/internal/oidc/device_service.go @@ -187,8 +187,13 @@ func (s *deviceService) getDeviceCodeInfo(ctx context.Context, userCode, userID authorizationRequired = consentRequired(hasAuthorizedClient, client.SkipConsent, nil) } + scope := request.GetRequestedScopes() + if scope == nil { + scope = []string{} + } + // Resolve friendly names for the requested custom-API permissions so the device consent screen matches the browser flow - scopeInfo, err := s.authorizationService.resolveScopeInfoForRequest(ctx, resource, request.GetRequestedScopes()) + scopeInfo, err := s.authorizationService.resolveScopeInfoForRequest(ctx, resource, scope) if err != nil { return nil, err } @@ -206,7 +211,7 @@ func (s *deviceService) getDeviceCodeInfo(ctx context.Context, userCode, userID LaunchURL: client.LaunchURL, RequiresReauthentication: client.RequiresReauthentication, }, - Scope: request.GetRequestedScopes(), + Scope: scope, ScopeInfo: scopeInfo, AuthorizationRequired: authorizationRequired, ReauthenticationRequired: client.RequiresReauthentication, diff --git a/backend/internal/oidc/interaction_session_dto.go b/backend/internal/oidc/interaction_session_dto.go index 7450b1db..a485c903 100644 --- a/backend/internal/oidc/interaction_session_dto.go +++ b/backend/internal/oidc/interaction_session_dto.go @@ -43,9 +43,17 @@ func newInteractionSessionForUser(interactionSession InteractionSession) (intera currentStep = requiredSteps[0] } + scopes := interactionSession.Scopes + if scopes == nil { + scopes = []string{} + } + if requiredSteps == nil { + requiredSteps = []interactionStep{} + } + return interactionSessionForUser{ ID: interactionSession.ID, - Scopes: interactionSession.Scopes, + Scopes: scopes, Client: client, CurrentStep: currentStep, RequiredSteps: requiredSteps, diff --git a/frontend/src/lib/components/scope-list.svelte b/frontend/src/lib/components/scope-list.svelte index 83eb038d..a7a26176 100644 --- a/frontend/src/lib/components/scope-list.svelte +++ b/frontend/src/lib/components/scope-list.svelte @@ -5,26 +5,28 @@ import { LucideKeyRound, LucideMail, LucideUser, LucideUsers } from '@lucide/svelte'; import ScopeItem from './scope-item.svelte'; - let { scopes, scopeInfo = [] }: { scopes: string[]; scopeInfo?: InteractionScopeInfo[] } = - $props(); + let { + scopes, + scopeInfo = [] + }: { scopes?: string[] | null; scopeInfo?: InteractionScopeInfo[] | null } = $props(); const standardScopes = ['openid', 'profile', 'email', 'groups', 'offline_access']; - const infoByKey = $derived(new Map(scopeInfo.map((info) => [info.key, info]))); - const customScopes = $derived(scopes.filter((scope) => !standardScopes.includes(scope))); + const infoByKey = $derived(new Map((scopeInfo || []).map((info) => [info.key, info]))); + const customScopes = $derived((scopes || []).filter((scope) => !standardScopes.includes(scope))); - {#if scopes.includes('email')} + {#if (scopes || []).includes('email')} {/if} - {#if scopes.includes('profile')} + {#if (scopes || []).includes('profile')} {/if} - {#if scopes.includes('groups')} + {#if (scopes || []).includes('groups')} - + diff --git a/frontend/src/routes/interaction/+page.svelte b/frontend/src/routes/interaction/+page.svelte index 24734ebe..fd438552 100644 --- a/frontend/src/routes/interaction/+page.svelte +++ b/frontend/src/routes/interaction/+page.svelte @@ -181,7 +181,10 @@

- + diff --git a/tests/specs/oidc.spec.ts b/tests/specs/oidc.spec.ts index 4ca4d382..76b55713 100644 --- a/tests/specs/oidc.spec.ts +++ b/tests/specs/oidc.spec.ts @@ -74,6 +74,44 @@ test('Authorize client requesting offline_access scope', async ({ page }) => { expect(callbackUrl.searchParams.get('error')).toBeNull(); }); +test('Authorize existing client without scopes', async ({ page }) => { + const oidcClient = oidcClients.nextcloud; + const urlParams = new URLSearchParams({ + client_id: oidcClient.id, + response_type: 'code', + redirect_uri: oidcClient.callbackUrl, + state: 'no-scope-state', + nonce: 'no-scope-nonce' + }); + + await expectCallbackRedirect(page, oidcClient.callbackUrl, () => + page.goto(`/authorize?${urlParams.toString()}`) + ); +}); + +test('Authorize new client without scopes', async ({ page }) => { + const oidcClient = oidcClients.immich; + const urlParams = new URLSearchParams({ + client_id: oidcClient.id, + response_type: 'code', + redirect_uri: oidcClient.callbackUrl, + state: 'no-scope-new-client', + nonce: 'no-scope-new-nonce' + }); + + await page.goto(`/authorize?${urlParams.toString()}`); + + // With no scopes there is nothing to display, but the page should still allow sign-in + await expect(page.getByRole('button', { name: 'Sign in' })).toBeVisible(); + + const callbackUrl = await expectCallbackRedirect(page, oidcClient.callbackUrl, () => + page.getByRole('button', { name: 'Sign in' }).click() + ); + expect(callbackUrl.searchParams.get('code')).toBeTruthy(); + expect(callbackUrl.searchParams.get('error')).toBeNull(); + expect(callbackUrl.searchParams.get('state')).toBe('no-scope-new-client'); +}); + test('Authorize new client while not signed in', async ({ page }) => { const oidcClient = oidcClients.immich; const urlParams = createUrlParams(oidcClient);