mirror of
https://github.com/pocket-id/pocket-id.git
synced 2026-07-20 19:51:27 +02:00
feat: prompt admin with PKCE client support hint (#1499)
Co-authored-by: james <james@goldfish.net> Co-authored-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com> Co-authored-by: Elias Schneider <login@eliasschneider.com> Co-authored-by: Kyle Mendell <kmendell@ofkm.us>
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
ALTER TABLE oidc_clients DROP COLUMN pkce_supported;
|
||||
@@ -0,0 +1,2 @@
|
||||
ALTER TABLE oidc_clients
|
||||
ADD COLUMN pkce_supported BOOLEAN NOT NULL DEFAULT FALSE;
|
||||
@@ -0,0 +1,7 @@
|
||||
PRAGMA foreign_keys=OFF;
|
||||
BEGIN;
|
||||
|
||||
ALTER TABLE oidc_clients DROP COLUMN pkce_supported;
|
||||
|
||||
COMMIT;
|
||||
PRAGMA foreign_keys=ON;
|
||||
@@ -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;
|
||||
@@ -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."
|
||||
}
|
||||
|
||||
@@ -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<OidcClient, 'id' | 'logoURL' | 'hasLogo' | 'hasDarkLogo'>;
|
||||
export type OidcClientUpdate = Omit<OidcClient, 'id' | 'logoURL' | 'hasLogo' | 'hasDarkLogo' | 'pkceSupported'>;
|
||||
export type OidcClientCreate = OidcClientUpdate & {
|
||||
id?: string;
|
||||
};
|
||||
|
||||
@@ -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}
|
||||
<Alert.Root variant="info">
|
||||
<LucideInfo class="size-4" />
|
||||
<Alert.Title>{m.pkce_supported_client_title()}</Alert.Title>
|
||||
<Alert.Description>
|
||||
{m.pkce_supported_client_description()}
|
||||
</Alert.Description>
|
||||
</Alert.Root>
|
||||
{/if}
|
||||
|
||||
<div>
|
||||
<button type="button" class="text-muted-foreground flex text-sm" onclick={backNavigation.go}
|
||||
><LucideChevronLeft class="size-5" /> {m.back()}</button
|
||||
>
|
||||
</div>
|
||||
|
||||
<Card.Root>
|
||||
<Card.Header>
|
||||
<Card.Title>{client.name}</Card.Title>
|
||||
|
||||
@@ -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>(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}
|
||||
/>
|
||||
<SwitchWithLabel
|
||||
id="pkce"
|
||||
label={m.pkce()}
|
||||
description={m.proof_key_code_exchange_is_a_security_feature_to_prevent_csrf_and_authorization_code_interception_attacks()}
|
||||
disabled={$inputs.isPublic.value}
|
||||
bind:checked={$inputs.pkceEnabled.value}
|
||||
/>
|
||||
<div
|
||||
class="rounded-lg transition-all duration-200"
|
||||
class:[&_[data-switch-root]]:ring-2={pkcePromptNeeded}
|
||||
class:[&_[data-switch-root]]:ring-blue-500={pkcePromptNeeded}
|
||||
>
|
||||
<SwitchWithLabel
|
||||
id="pkce"
|
||||
label={m.pkce()}
|
||||
description={m.proof_key_code_exchange_is_a_security_feature_to_prevent_csrf_and_authorization_code_interception_attacks()}
|
||||
disabled={$inputs.isPublic.value}
|
||||
bind:checked={$inputs.pkceEnabled.value}
|
||||
/>
|
||||
</div>
|
||||
<SwitchWithLabel
|
||||
id="requires-reauthentication"
|
||||
label={m.requires_reauthentication()}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"provider": "sqlite",
|
||||
"version": 20260626120000,
|
||||
"version": 20260726153900,
|
||||
"tableOrder": ["users", "user_groups", "oidc_clients", "signup_tokens"],
|
||||
"tables": {
|
||||
"api_keys": [
|
||||
@@ -54,6 +54,7 @@
|
||||
"logout_callback_urls": "WyJodHRwOi8vbmV4dGNsb3VkLmxvY2FsaG9zdC9hdXRoL2xvZ291dC9jYWxsYmFjayJd",
|
||||
"name": "Nextcloud",
|
||||
"pkce_enabled": false,
|
||||
"pkce_supported": false,
|
||||
"requires_pushed_authorization_requests": false,
|
||||
"requires_reauthentication": false,
|
||||
"secret": "$2a$10$9dypwot8nGuCjT6wQWWpJOckZfRprhe2EkwpKizxS/fpVHrOLEJHC",
|
||||
@@ -73,6 +74,7 @@
|
||||
"logout_callback_urls": "bnVsbA==",
|
||||
"name": "Immich",
|
||||
"pkce_enabled": false,
|
||||
"pkce_supported": false,
|
||||
"requires_pushed_authorization_requests": false,
|
||||
"requires_reauthentication": false,
|
||||
"secret": "$2a$10$Ak.FP8riD1ssy2AGGbG.gOpnp/rBpymd74j0nxNMtW0GG1Lb4gzxe",
|
||||
@@ -92,6 +94,7 @@
|
||||
"logout_callback_urls": "WyJodHRwOi8vdGFpbHNjYWxlLmxvY2FsaG9zdC9hdXRoL2xvZ291dC9jYWxsYmFjayJd",
|
||||
"name": "Tailscale",
|
||||
"pkce_enabled": false,
|
||||
"pkce_supported": false,
|
||||
"requires_pushed_authorization_requests": false,
|
||||
"requires_reauthentication": false,
|
||||
"secret": "$2a$10$xcRReBsvkI1XI6FG8xu/pOgzeF00bH5Wy4d/NThwcdi3ZBpVq/B9a",
|
||||
@@ -111,6 +114,7 @@
|
||||
"logout_callback_urls": "bnVsbA==",
|
||||
"name": "Federated",
|
||||
"pkce_enabled": false,
|
||||
"pkce_supported": false,
|
||||
"requires_pushed_authorization_requests": false,
|
||||
"requires_reauthentication": false,
|
||||
"secret": "$2a$10$Ak.FP8riD1ssy2AGGbG.gOpnp/rBpymd74j0nxNMtW0GG1Lb4gzxe",
|
||||
@@ -129,6 +133,7 @@
|
||||
"logout_callback_urls": "bnVsbA==",
|
||||
"name": "SCIM Client",
|
||||
"pkce_enabled": false,
|
||||
"pkce_supported": false,
|
||||
"requires_pushed_authorization_requests": false,
|
||||
"requires_reauthentication": false,
|
||||
"secret": "$2a$10$h4wfa8gI7zavDAxwzSq1sOwYU4e8DwK1XZ8ZweNnY5KzlJ3Iz.qdK",
|
||||
@@ -148,6 +153,7 @@
|
||||
"logout_callback_urls": "bnVsbA==",
|
||||
"name": "PAR Test Client",
|
||||
"pkce_enabled": false,
|
||||
"pkce_supported": false,
|
||||
"requires_pushed_authorization_requests": false,
|
||||
"requires_reauthentication": false,
|
||||
"secret": "$2a$10$9dypwot8nGuCjT6wQWWpJOckZfRprhe2EkwpKizxS/fpVHrOLEJHC",
|
||||
@@ -167,6 +173,7 @@
|
||||
"logout_callback_urls": "bnVsbA==",
|
||||
"name": "Skip Consent Client",
|
||||
"pkce_enabled": false,
|
||||
"pkce_supported": false,
|
||||
"requires_pushed_authorization_requests": false,
|
||||
"requires_reauthentication": false,
|
||||
"secret": "$2a$10$9dypwot8nGuCjT6wQWWpJOckZfRprhe2EkwpKizxS/fpVHrOLEJHC",
|
||||
|
||||
@@ -129,6 +129,7 @@ test('Filter OIDC clients by PAR requirement', async ({ page, request }) => {
|
||||
logoutCallbackURLs: [],
|
||||
isPublic: false,
|
||||
pkceEnabled: false,
|
||||
pkceSupported: false,
|
||||
requiresReauthentication: false,
|
||||
requiresPushedAuthorizationRequests: true,
|
||||
credentials: { federatedIdentities: [] },
|
||||
|
||||
@@ -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: [] },
|
||||
|
||||
Reference in New Issue
Block a user