feat: allow custom client IDs (#864)

This commit is contained in:
Elias Schneider
2025-08-23 18:41:05 +02:00
committed by GitHub
parent 625f235740
commit a5efb95065
15 changed files with 151 additions and 56 deletions

View File

@@ -368,3 +368,13 @@ func (e *OpenSignupDisabledError) Error() string {
func (e *OpenSignupDisabledError) HttpStatusCode() int {
return http.StatusForbidden
}
type ClientIdAlreadyExistsError struct{}
func (e *ClientIdAlreadyExistsError) Error() string {
return "Client ID already in use"
}
func (e *ClientIdAlreadyExistsError) HttpStatusCode() int {
return http.StatusBadRequest
}

View File

@@ -492,11 +492,11 @@ func (oc *OidcController) deleteClientHandler(c *gin.Context) {
// @Accept json
// @Produce json
// @Param id path string true "Client ID"
// @Param client body dto.OidcClientCreateDto true "Client information"
// @Param client body dto.OidcClientUpdateDto true "Client information"
// @Success 200 {object} dto.OidcClientWithAllowedUserGroupsDto "Updated client"
// @Router /api/oidc/clients/{id} [put]
func (oc *OidcController) updateClientHandler(c *gin.Context) {
var input dto.OidcClientCreateDto
var input dto.OidcClientUpdateDto
if err := c.ShouldBindJSON(&input); err != nil {
_ = c.Error(err)
return

View File

@@ -29,7 +29,7 @@ type OidcClientWithAllowedGroupsCountDto struct {
AllowedUserGroupsCount int64 `json:"allowedUserGroupsCount"`
}
type OidcClientCreateDto struct {
type OidcClientUpdateDto struct {
Name string `json:"name" binding:"required,max=50" unorm:"nfc"`
CallbackURLs []string `json:"callbackURLs"`
LogoutCallbackURLs []string `json:"logoutCallbackURLs"`
@@ -40,6 +40,11 @@ type OidcClientCreateDto struct {
LaunchURL *string `json:"launchURL" binding:"omitempty,url"`
}
type OidcClientCreateDto struct {
OidcClientUpdateDto
ID string `json:"id" binding:"omitempty,client_id,min=2,max=128"`
}
type OidcClientCredentialsDto struct {
FederatedIdentities []OidcClientFederatedIdentityDto `json:"federatedIdentities,omitempty"`
}

View File

@@ -18,6 +18,8 @@ func init() {
// [a-zA-Z0-9]$ : The username must end with an alphanumeric character
var validateUsernameRegex = regexp.MustCompile("^[a-zA-Z0-9][a-zA-Z0-9_.@-]*[a-zA-Z0-9]$")
var validateClientIDRegex = regexp.MustCompile("^[a-zA-Z0-9._-]+$")
// Maximum allowed value for TTLs
const maxTTL = 31 * 24 * time.Hour
@@ -28,6 +30,14 @@ func init() {
if err != nil {
panic("Failed to register custom validation for username: " + err.Error())
}
err = v.RegisterValidation("client_id", func(fl validator.FieldLevel) bool {
return validateClientIDRegex.MatchString(fl.Field().String())
})
if err != nil {
panic("Failed to register custom validation for client_id: " + err.Error())
}
err = v.RegisterValidation("ttl", func(fl validator.FieldLevel) bool {
ttl, ok := fl.Field().Interface().(utils.JSONDuration)
if !ok {

View File

@@ -670,22 +670,28 @@ func (s *OidcService) ListClients(ctx context.Context, name string, sortedPagina
func (s *OidcService) CreateClient(ctx context.Context, input dto.OidcClientCreateDto, userID string) (model.OidcClient, error) {
client := model.OidcClient{
Base: model.Base{
ID: input.ID,
},
CreatedByID: utils.Ptr(userID),
}
updateOIDCClientModelFromDto(&client, &input)
updateOIDCClientModelFromDto(&client, &input.OidcClientUpdateDto)
err := s.db.
WithContext(ctx).
Create(&client).
Error
if err != nil {
if errors.Is(err, gorm.ErrDuplicatedKey) {
return model.OidcClient{}, &common.ClientIdAlreadyExistsError{}
}
return model.OidcClient{}, err
}
return client, nil
}
func (s *OidcService) UpdateClient(ctx context.Context, clientID string, input dto.OidcClientCreateDto) (model.OidcClient, error) {
func (s *OidcService) UpdateClient(ctx context.Context, clientID string, input dto.OidcClientUpdateDto) (model.OidcClient, error) {
tx := s.db.Begin()
defer func() {
tx.Rollback()
@@ -719,7 +725,7 @@ func (s *OidcService) UpdateClient(ctx context.Context, clientID string, input d
return client, nil
}
func updateOIDCClientModelFromDto(client *model.OidcClient, input *dto.OidcClientCreateDto) {
func updateOIDCClientModelFromDto(client *model.OidcClient, input *dto.OidcClientUpdateDto) {
// Base fields
client.Name = input.Name
client.CallbackURLs = input.CallbackURLs

View File

@@ -171,8 +171,10 @@ func TestOidcService_verifyClientCredentialsInternal(t *testing.T) {
// Create the test clients
// 1. Confidential client
confidentialClient, err := s.CreateClient(t.Context(), dto.OidcClientCreateDto{
Name: "Confidential Client",
CallbackURLs: []string{"https://example.com/callback"},
OidcClientUpdateDto: dto.OidcClientUpdateDto{
Name: "Confidential Client",
CallbackURLs: []string{"https://example.com/callback"},
},
}, "test-user-id")
require.NoError(t, err)
@@ -182,20 +184,24 @@ func TestOidcService_verifyClientCredentialsInternal(t *testing.T) {
// 2. Public client
publicClient, err := s.CreateClient(t.Context(), dto.OidcClientCreateDto{
Name: "Public Client",
CallbackURLs: []string{"https://example.com/callback"},
IsPublic: true,
OidcClientUpdateDto: dto.OidcClientUpdateDto{
Name: "Public Client",
CallbackURLs: []string{"https://example.com/callback"},
IsPublic: true,
},
}, "test-user-id")
require.NoError(t, err)
// 3. Confidential client with federated identity
federatedClient, err := s.CreateClient(t.Context(), dto.OidcClientCreateDto{
Name: "Federated Client",
CallbackURLs: []string{"https://example.com/callback"},
OidcClientUpdateDto: dto.OidcClientUpdateDto{
Name: "Federated Client",
CallbackURLs: []string{"https://example.com/callback"},
},
}, "test-user-id")
require.NoError(t, err)
federatedClient, err = s.UpdateClient(t.Context(), federatedClient.ID, dto.OidcClientCreateDto{
federatedClient, err = s.UpdateClient(t.Context(), federatedClient.ID, dto.OidcClientUpdateDto{
Name: federatedClient.Name,
CallbackURLs: federatedClient.CallbackURLs,
Credentials: dto.OidcClientCredentialsDto{

View File

@@ -437,5 +437,8 @@
"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.",
"last_signed_in_ago": "Last signed in {time} ago"
"last_signed_in_ago": "Last signed in {time} ago",
"invalid_client_id": "Client ID can only contain letters, numbers, underscores, and hyphens",
"custom_client_id_description": "Set a custom client ID if this is required by your application. Otherwise, leave it blank to generate a random one.",
"generated": "Generated"
}

View File

@@ -4,6 +4,7 @@ import type {
OidcClient,
OidcClientCreate,
OidcClientMetaData,
OidcClientUpdate,
OidcClientWithAllowedUserGroups,
OidcClientWithAllowedUserGroupsCount,
OidcDeviceCodeInfo
@@ -67,7 +68,7 @@ class OidcService extends APIService {
return (await this.api.get(`/oidc/clients/${id}/meta`)).data as OidcClientMetaData;
}
async updateClient(id: string, client: OidcClientCreate) {
async updateClient(id: string, client: OidcClientUpdate) {
return (await this.api.put(`/oidc/clients/${id}`, client)).data as OidcClient;
}

View File

@@ -37,7 +37,13 @@ export type OidcClientWithAllowedUserGroupsCount = OidcClient & {
allowedUserGroupsCount: number;
};
export type OidcClientCreate = Omit<OidcClient, 'id' | 'logoURL' | 'hasLogo'>;
export type OidcClientUpdate = Omit<OidcClient, 'id' | 'logoURL' | 'hasLogo'>;
export type OidcClientCreate = OidcClientUpdate & {
id?: string;
};
export type OidcClientUpdateWithLogo = OidcClientUpdate & {
logo: File | null | undefined;
};
export type OidcClientCreateWithLogo = OidcClientCreate & {
logo: File | null | undefined;

View File

@@ -1,9 +1,7 @@
import z from 'zod/v4';
export const optionalString = z
.string()
.transform((v) => (v === '' ? undefined : v))
.optional();
export const emptyToUndefined = <T>(validation: z.ZodType<T>) =>
z.preprocess((v) => (v === '' ? undefined : v), validation);
export const optionalUrl = z
.url()

View File

@@ -5,7 +5,7 @@
import type { ApiKeyCreate } from '$lib/types/api-key.type';
import { preventDefault } from '$lib/utils/event-util';
import { createForm } from '$lib/utils/form-util';
import { optionalString } from '$lib/utils/zod-util';
import { emptyToUndefined } from '$lib/utils/zod-util';
import { z } from 'zod/v4';
let {
@@ -28,7 +28,7 @@
const formSchema = z.object({
name: z.string().min(3).max(50),
description: optionalString,
description: emptyToUndefined(z.string().optional()),
expiresAt: z.date().min(new Date(), m.expiration_date_must_be_in_the_future())
});

View File

@@ -70,7 +70,7 @@
{#if expandAddClient}
<div transition:slide>
<Card.Content>
<OIDCClientForm callback={createOIDCClient} />
<OIDCClientForm mode="create" callback={createOIDCClient} />
</Card.Content>
</div>
{/if}

View File

@@ -179,7 +179,7 @@
</Card.Root>
<Card.Root>
<Card.Content>
<OidcForm existingClient={client} callback={updateClient} />
<OidcForm mode="update" existingClient={client} callback={updateClient} />
</Card.Content>
</Card.Root>
<CollapsibleCard

View File

@@ -6,12 +6,16 @@
import { Button } from '$lib/components/ui/button';
import Label from '$lib/components/ui/label/label.svelte';
import { m } from '$lib/paraglide/messages';
import type { OidcClient, OidcClientCreateWithLogo } from '$lib/types/oidc.type';
import type {
OidcClient,
OidcClientCreateWithLogo,
OidcClientUpdateWithLogo
} from '$lib/types/oidc.type';
import { cachedOidcClientLogo } from '$lib/utils/cached-image-util';
import { preventDefault } from '$lib/utils/event-util';
import { createForm } from '$lib/utils/form-util';
import { cn } from '$lib/utils/style';
import { optionalUrl } from '$lib/utils/zod-util';
import { emptyToUndefined, optionalUrl } from '$lib/utils/zod-util';
import { LucideChevronDown } from '@lucide/svelte';
import { slide } from 'svelte/transition';
import { z } from 'zod/v4';
@@ -20,10 +24,12 @@
let {
callback,
existingClient
existingClient,
mode
}: {
existingClient?: OidcClient;
callback: (user: OidcClientCreateWithLogo) => Promise<boolean>;
callback: (client: OidcClientCreateWithLogo | OidcClientUpdateWithLogo) => Promise<boolean>;
mode: 'create' | 'update';
} = $props();
let isLoading = $state(false);
@@ -34,6 +40,7 @@
);
const client = {
id: '',
name: existingClient?.name || '',
callbackURLs: existingClient?.callbackURLs || [],
logoutCallbackURLs: existingClient?.logoutCallbackURLs || [],
@@ -47,6 +54,16 @@
};
const formSchema = z.object({
id: emptyToUndefined(
z
.string()
.min(2)
.max(128)
.regex(/^[a-zA-Z0-9_-]+$/, {
message: m.invalid_client_id()
})
.optional()
),
name: z.string().min(2).max(50),
callbackURLs: z.array(z.string().nonempty()).default([]),
logoutCallbackURLs: z.array(z.string().nonempty()),
@@ -185,7 +202,16 @@
</div>
{#if showAdvancedOptions}
<div class="mt-5 md:col-span-2" transition:slide={{ duration: 200 }}>
<div class="mt-7 flex flex-col gap-y-7 md:col-span-2" transition:slide={{ duration: 200 }}>
{#if mode == 'create'}
<FormInput
label={m.client_id()}
placeholder={m.generated()}
class="w-full md:w-1/2"
description={m.custom_client_id_description()}
bind:input={$inputs.id}
/>
{/if}
<FederatedIdentitiesInput
client={existingClient}
bind:federatedIdentities={$inputs.credentials.value.federatedIdentities}

View File

@@ -1,40 +1,64 @@
import test, { expect } from '@playwright/test';
import test, { expect, Page } from '@playwright/test';
import { oidcClients } from '../data';
import { cleanupBackend } from '../utils/cleanup.util';
test.beforeEach(async () => await cleanupBackend());
test('Create OIDC client', async ({ page }) => {
await page.goto('/settings/admin/oidc-clients');
const oidcClient = oidcClients.pingvinShare;
test.describe('Create OIDC client', () => {
async function createClientTest(page: Page, clientId?: string) {
const oidcClient = oidcClients.pingvinShare;
await page.goto('/settings/admin/oidc-clients');
await page.getByRole('button', { name: 'Add OIDC Client' }).click();
await page.getByRole('button', { name: 'Add OIDC Client' }).click();
await page.getByLabel('Name').fill(oidcClient.name);
await page.getByLabel('Name').fill(oidcClient.name);
await page.getByLabel('Client Launch URL').fill(oidcClient.launchURL);
await page.getByLabel('Client Launch URL').fill(oidcClient.launchURL);
await page.getByRole('button', { name: 'Add' }).first().click();
await page.getByTestId('callback-url-1').fill(oidcClient.callbackUrl);
await page.getByRole('button', { name: 'Add' }).nth(1).click();
await page.getByTestId('callback-url-1').fill(oidcClient.callbackUrl);
await page.getByRole('button', { name: 'Add another' }).click();
await page.getByTestId('callback-url-2').fill(oidcClient.secondCallbackUrl!);
await page.getByRole('button', { name: 'Add another' }).click();
await page.getByTestId('callback-url-2').fill(oidcClient.secondCallbackUrl);
await page.getByLabel('logo').setInputFiles('assets/pingvin-share-logo.png');
await page.getByRole('button', { name: 'Save' }).click();
await page.getByLabel('logo').setInputFiles('assets/pingvin-share-logo.png');
const clientId = await page.getByTestId('client-id').textContent();
if (clientId) {
await page.getByRole('button', { name: 'Show Advanced Options' }).click();
await page.getByLabel('Client ID').fill(clientId);
}
await expect(page.locator('[data-type="success"]')).toHaveText(
'OIDC client created successfully'
);
expect(clientId?.length).toBe(36);
expect((await page.getByTestId('client-secret').textContent())?.length).toBe(32);
await expect(page.getByLabel('Name')).toHaveValue(oidcClient.name);
await expect(page.getByTestId('callback-url-1')).toHaveValue(oidcClient.callbackUrl);
await expect(page.getByTestId('callback-url-2')).toHaveValue(oidcClient.secondCallbackUrl!);
await expect(page.getByRole('img', { name: `${oidcClient.name} logo` })).toBeVisible();
await page.request
.get(`/api/oidc/clients/${clientId}/logo`)
.then((res) => expect.soft(res.status()).toBe(200));
await page.getByRole('button', { name: 'Save' }).click();
await expect(page.locator('[data-type="success"]')).toHaveText(
'OIDC client created successfully'
);
const resolvedClientId = (await page.getByTestId('client-id').innerText()).trim();
const clientSecret = (await page.getByTestId('client-secret').innerText()).trim();
if (clientId) {
expect(resolvedClientId).toBe(clientId);
} else {
expect(resolvedClientId).toMatch(/^[\w-]{36}$/);
}
expect(clientSecret).toMatch(/^\w{32}$/);
await expect(page.getByLabel('Name')).toHaveValue(oidcClient.name);
await expect(page.getByTestId('callback-url-1')).toHaveValue(oidcClient.callbackUrl);
await expect(page.getByTestId('callback-url-2')).toHaveValue(oidcClient.secondCallbackUrl);
await expect(page.getByRole('img', { name: `${oidcClient.name} logo` })).toBeVisible();
const res = await page.request.get(`/api/oidc/clients/${resolvedClientId}/logo`);
expect(res.ok()).toBeTruthy();
}
test('with auto-generated client ID', async ({ page }) => {
await createClientTest(page);
});
test('with custom client ID', async ({ page }) => {
await createClientTest(page, '123e4567-e89b-12d3-a456-426614174000');
});
});
test('Edit OIDC client', async ({ page }) => {