diff --git a/backend/internal/dto/oidc_dto.go b/backend/internal/dto/oidc_dto.go index adc2f6c6..427c0dca 100644 --- a/backend/internal/dto/oidc_dto.go +++ b/backend/internal/dto/oidc_dto.go @@ -21,6 +21,7 @@ type OidcClientDto struct { SkipConsent bool `json:"skipConsent"` Credentials OidcClientCredentialsDto `json:"credentials"` IsGroupRestricted bool `json:"isGroupRestricted"` + PkceSupported bool `json:"pkceSupported,omitempty"` } type OidcClientWithAllowedUserGroupsDto struct { diff --git a/backend/internal/model/oidc.go b/backend/internal/model/oidc.go index 68882dc9..c0e4ad97 100644 --- a/backend/internal/model/oidc.go +++ b/backend/internal/model/oidc.go @@ -36,6 +36,7 @@ type OidcClient struct { Credentials OidcClientCredentials LaunchURL *string IsGroupRestricted bool `sortable:"true" filterable:"true"` + PkceSupported bool `sortable:"true" filterable:"true"` AllowedUserGroups []UserGroup `gorm:"many2many:oidc_clients_allowed_user_groups;"` CreatedByID *string diff --git a/backend/internal/oidc/authorization_service.go b/backend/internal/oidc/authorization_service.go index 7ac07b90..4222da08 100644 --- a/backend/internal/oidc/authorization_service.go +++ b/backend/internal/oidc/authorization_service.go @@ -133,12 +133,28 @@ func (s *authorizationService) authorize(ctx context.Context, input authorizeInp now: time.Now().UTC(), } + codeChallenge := input.requester.GetRequestForm().Get("code_challenge") + var result authorizationResult err = withTx(ctx, s.db, func(ctx context.Context) error { var err error result, err = s.authorizeAuthenticated(ctx, req) - return err + if err != nil { + return err + } + + if codeChallenge != "" && + !client.PkceEnabled && + !client.PkceSupported { + + tx := dbFromContext(ctx, s.db) + + _ = flagPkceSupportedClient(ctx, client.GetID(), tx) + } + + return nil }) + if err != nil { return authorizationResult{}, err } @@ -229,6 +245,20 @@ func (s *authorizationService) authorizeAuthenticated(ctx context.Context, req a return authorizationResult{Session: session}, nil } +func flagPkceSupportedClient(ctx context.Context, clientID string, tx *gorm.DB) error { + err := tx. + WithContext(ctx). + Model(&model.OidcClient{}). + Where("id = ?", clientID). + Update("pkce_supported", true). + Error + + if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) { + return err + } + return nil +} + // resolveRequirements determines the interaction steps still required; the requirements // of a resumed interaction session win over the ones derived from the request. func (s *authorizationService) resolveRequirements(ctx context.Context, req authorizeRequest, interactionSession *InteractionSession) (interactionRequirements, time.Time, error) { diff --git a/backend/internal/service/oidc_service.go b/backend/internal/service/oidc_service.go index c4d549e6..d2c95a44 100644 --- a/backend/internal/service/oidc_service.go +++ b/backend/internal/service/oidc_service.go @@ -218,6 +218,10 @@ func updateOIDCClientModelFromDto(client *model.OidcClient, input *dto.OidcClien client.IsPublic = input.IsPublic // PKCE is required for public clients client.PkceEnabled = input.IsPublic || input.PkceEnabled + // Reset any pkce support prompt if previously flagged + if !input.PkceEnabled { + client.PkceSupported = false + } client.RequiresReauthentication = input.RequiresReauthentication client.RequiresPushedAuthorizationRequests = input.RequiresPushedAuthorizationRequests client.SkipConsent = input.SkipConsent diff --git a/backend/resources/migrations/postgres/20260726153900_add_pkce_supported_to_oidc_clients.down.sql b/backend/resources/migrations/postgres/20260726153900_add_pkce_supported_to_oidc_clients.down.sql new file mode 100644 index 00000000..53fbe9a8 --- /dev/null +++ b/backend/resources/migrations/postgres/20260726153900_add_pkce_supported_to_oidc_clients.down.sql @@ -0,0 +1 @@ +ALTER TABLE oidc_clients DROP COLUMN pkce_supported; \ No newline at end of file diff --git a/backend/resources/migrations/postgres/20260726153900_add_pkce_supported_to_oidc_clients.up.sql b/backend/resources/migrations/postgres/20260726153900_add_pkce_supported_to_oidc_clients.up.sql new file mode 100644 index 00000000..24e8308d --- /dev/null +++ b/backend/resources/migrations/postgres/20260726153900_add_pkce_supported_to_oidc_clients.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE oidc_clients +ADD COLUMN pkce_supported BOOLEAN NOT NULL DEFAULT FALSE; \ No newline at end of file diff --git a/backend/resources/migrations/sqlite/20260726153900_add_pkce_supported_to_oidc_clients.down.sql b/backend/resources/migrations/sqlite/20260726153900_add_pkce_supported_to_oidc_clients.down.sql new file mode 100644 index 00000000..e75e7878 --- /dev/null +++ b/backend/resources/migrations/sqlite/20260726153900_add_pkce_supported_to_oidc_clients.down.sql @@ -0,0 +1,7 @@ +PRAGMA foreign_keys=OFF; +BEGIN; + +ALTER TABLE oidc_clients DROP COLUMN pkce_supported; + +COMMIT; +PRAGMA foreign_keys=ON; \ No newline at end of file diff --git a/backend/resources/migrations/sqlite/20260726153900_add_pkce_supported_to_oidc_clients.up.sql b/backend/resources/migrations/sqlite/20260726153900_add_pkce_supported_to_oidc_clients.up.sql new file mode 100644 index 00000000..25a9a328 --- /dev/null +++ b/backend/resources/migrations/sqlite/20260726153900_add_pkce_supported_to_oidc_clients.up.sql @@ -0,0 +1,8 @@ +PRAGMA foreign_keys= OFF; +BEGIN; + +ALTER TABLE oidc_clients + ADD COLUMN pkce_supported BOOLEAN NOT NULL DEFAULT 0; + +COMMIT; +PRAGMA foreign_keys= ON; \ No newline at end of file diff --git a/frontend/messages/en.json b/frontend/messages/en.json index eae64b8d..076658d1 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -530,5 +530,7 @@ "emails_verified_by_default_description": "When enabled, users' email addresses will be marked as verified by default upon signup or when their email address is changed.", "user_has_no_passkeys_yet": "This user has no passkeys yet.", "replay_protection": "Replay Protection", - "replay_protection_description": "If enabled the provided token can only be used once. If your provider uses the same token multiple times, you may need to disable this option." + "replay_protection_description": "If enabled the provided token can only be used once. If your provider uses the same token multiple times, you may need to disable this option.", + "pkce_supported_client_title": "This client supports PKCE", + "pkce_supported_client_description": "This client supports Proof Key for Code Exchange (PKCE). PKCE is a security feature that helps protect against certain attacks during the OAuth 2.0 authorization process. It's recommended to enable it." } diff --git a/frontend/src/lib/types/oidc.type.ts b/frontend/src/lib/types/oidc.type.ts index 6cec6e4e..729d4788 100644 --- a/frontend/src/lib/types/oidc.type.ts +++ b/frontend/src/lib/types/oidc.type.ts @@ -32,6 +32,7 @@ export type OidcClient = OidcClientMetaData & { credentials?: OidcClientCredentials; launchURL?: string; isGroupRestricted: boolean; + pkceSupported: boolean; }; export type OidcClientWithAllowedUserGroups = OidcClient & { @@ -42,7 +43,7 @@ export type OidcClientWithAllowedUserGroupsCount = OidcClient & { allowedUserGroupsCount: number; }; -export type OidcClientUpdate = Omit; +export type OidcClientUpdate = Omit; export type OidcClientCreate = OidcClientUpdate & { id?: string; }; diff --git a/frontend/src/routes/settings/admin/oidc-clients/[id]/+page.svelte b/frontend/src/routes/settings/admin/oidc-clients/[id]/+page.svelte index 4c573913..4270bcf1 100644 --- a/frontend/src/routes/settings/admin/oidc-clients/[id]/+page.svelte +++ b/frontend/src/routes/settings/admin/oidc-clients/[id]/+page.svelte @@ -4,6 +4,7 @@ import CollapsibleCard from '$lib/components/collapsible-card.svelte'; import { openConfirmDialog } from '$lib/components/confirm-dialog'; import CopyToClipboard from '$lib/components/copy-to-clipboard.svelte'; + import * as Alert from '$lib/components/ui/alert'; import { Button } from '$lib/components/ui/button'; import * as Card from '$lib/components/ui/card'; import * as Field from '$lib/components/ui/field'; @@ -16,7 +17,7 @@ import type { ScimServiceProviderCreate } from '$lib/types/scim.type'; import { cachedOidcClientLogo } from '$lib/utils/cached-image-util'; import { axiosErrorToast } from '$lib/utils/error-util'; - import { LucideChevronLeft, LucideRefreshCcw } from '@lucide/svelte'; + import { LucideChevronLeft, LucideInfo, LucideRefreshCcw } from '@lucide/svelte'; import { toast } from 'svelte-sonner'; import { slide } from 'svelte/transition'; import { backNavigate } from '../../users/navigate-back-util'; @@ -90,6 +91,9 @@ if (updatedClient.darkLogo !== undefined || updatedClient.darkLogoUrl !== undefined) { client.hasDarkLogo = updatedClient.darkLogo !== null || !!updatedClient.darkLogoUrl; } + if (updatedClient.pkceEnabled) { + client.pkceEnabled = updatedClient.pkceEnabled; + } toast.success(m.oidc_client_updated_successfully()); }) .catch((e) => { @@ -209,11 +213,22 @@ > {/snippet} +{#if client.pkceSupported && !client.pkceEnabled} + + + {m.pkce_supported_client_title()} + + {m.pkce_supported_client_description()} + + +{/if} +
+ {client.name} diff --git a/frontend/src/routes/settings/admin/oidc-clients/oidc-client-form.svelte b/frontend/src/routes/settings/admin/oidc-clients/oidc-client-form.svelte index 05edd566..9da86c8c 100644 --- a/frontend/src/routes/settings/admin/oidc-clients/oidc-client-form.svelte +++ b/frontend/src/routes/settings/admin/oidc-clients/oidc-client-form.svelte @@ -57,7 +57,8 @@ federatedIdentities: existingClient?.credentials?.federatedIdentities || [] }, logoUrl: '', - darkLogoUrl: '' + darkLogoUrl: '', + pkceSupported: existingClient?.pkceSupported || false }; const formSchema = z.object({ @@ -98,6 +99,8 @@ type FormSchema = typeof formSchema; const { inputs, errors, ...form } = createForm(formSchema, client); + const pkcePromptNeeded = $derived(!$inputs.pkceEnabled.value && client.pkceSupported); + async function onSubmit() { const data = form.validate(); if (!data) return; @@ -215,13 +218,19 @@ }} bind:checked={$inputs.isPublic.value} /> - +
+ +
{ logoutCallbackURLs: [], isPublic: false, pkceEnabled: false, + pkceSupported: false, requiresReauthentication: false, requiresPushedAuthorizationRequests: true, credentials: { federatedIdentities: [] }, diff --git a/tests/specs/oidc.spec.ts b/tests/specs/oidc.spec.ts index 3d2afa0f..53d5d364 100644 --- a/tests/specs/oidc.spec.ts +++ b/tests/specs/oidc.spec.ts @@ -683,6 +683,7 @@ test('Device authorization flow forces reauthentication when client requires it' logoutCallbackURLs: [client.logoutCallbackUrl], isPublic: false, pkceEnabled: false, + pkceSupported: false, requiresReauthentication: true, requiresPushedAuthorizationRequests: false, credentials: { federatedIdentities: [] }, @@ -807,6 +808,7 @@ test('Forces reauthentication when client requires it', async ({ page, request } logoutCallbackURLs: [oidcClients.nextcloud.logoutCallbackUrl], isPublic: false, pkceEnabled: false, + pkceSupported: false, requiresReauthentication: true, requiresPushedAuthorizationRequests: false, credentials: { federatedIdentities: [] }, @@ -1363,6 +1365,7 @@ test.describe('Pushed Authorization Requests (PAR)', () => { logoutCallbackURLs: [], isPublic: true, pkceEnabled: true, + pkceSupported: false, requiresReauthentication: false, requiresPushedAuthorizationRequests: false, credentials: { federatedIdentities: [] }, @@ -1405,6 +1408,7 @@ test.describe('Pushed Authorization Requests (PAR)', () => { logoutCallbackURLs: [], isPublic: false, pkceEnabled: false, + pkceSupported: false, requiresReauthentication: false, requiresPushedAuthorizationRequests: true, credentials: { federatedIdentities: [] },