fix: /authorize endpoint crashes when list of scopes is empty (#1575)

This commit is contained in:
Alessandro (Ale) Segala
2026-07-08 17:48:32 -07:00
committed by GitHub
parent 6734585712
commit b2711ced99
7 changed files with 78 additions and 15 deletions

View File

@@ -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,

View File

@@ -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,

View File

@@ -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,

View File

@@ -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)));
</script>
<Item.Group data-testid="scopes" class="gap-1">
{#if scopes.includes('email')}
{#if (scopes || []).includes('email')}
<ScopeItem icon={LucideMail} name={m.email()} description={m.view_your_email_address()} />
{/if}
{#if scopes.includes('profile')}
{#if (scopes || []).includes('profile')}
<ScopeItem
icon={LucideUser}
name={m.profile()}
description={m.view_your_profile_information()}
/>
{/if}
{#if scopes.includes('groups')}
{#if (scopes || []).includes('groups')}
<ScopeItem
icon={LucideUsers}
name={m.groups()}

View File

@@ -132,7 +132,10 @@
</p>
</Card.Header>
<Card.Content data-testid="scopes">
<ScopeList scopes={deviceInfo!.scope} scopeInfo={deviceInfo!.scopeInfo} />
<ScopeList
scopes={deviceInfo!.scope || []}
scopeInfo={deviceInfo!.scopeInfo || []}
/>
</Card.Content>
</Card.Root>
</div>

View File

@@ -181,7 +181,10 @@
</p>
</Card.Header>
<Card.Content>
<ScopeList scopes={interactionSession.scopes} scopeInfo={interactionSession.scopeInfo} />
<ScopeList
scopes={(interactionSession.scopes || [])}
scopeInfo={interactionSession.scopeInfo ?? []}
/>
</Card.Content>
</Card.Root>
</div>

View File

@@ -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);