Address review feedback on client secrets

Always serialize the "secrets" field in the credentials DTO, as an empty
list when the client has no secrets, and make it a required property in
the frontend type so consumers never have to handle a missing value.

The federated credentials card now hands its callback only the federated
identities it owns, and the page merges them into the client credentials,
so saving them no longer drops the secrets from the local client object.

Also drop the field comments on OidcClientSecret that restated the code,
keeping only the note about the prefix being empty for migrated secrets.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gcdcb75uaMU5UE1G2niL3L

backend/internal/dto/oidc_dto.go

@@ -91,7 +91,8 @@ type OidcClientSecretCreatedDto struct {
 type OidcClientCredentialsDto struct {
 	FederatedIdentities []OidcClientFederatedIdentityDto `json:"federatedIdentities,omitempty"`
 	// Secrets is read-only: secrets are managed through the dedicated client secret endpoints and any value sent by a client is ignored
-	Secrets []OidcClientSecretDto `json:"secrets,omitempty"`
+	// It is always serialized, as an empty list when the client has no secrets, so clients don't need to handle a missing value
+	Secrets []OidcClientSecretDto `json:"secrets"`
 }
 
 type OidcClientFederatedIdentityDto struct {

backend/internal/dto/oidc_dto_test.go

@@ -106,3 +106,19 @@ func TestOidcClientDto_secrets(t *testing.T) {
 	assert.NotContains(t, string(serialized), "hash-1")
 	assert.Contains(t, string(serialized), `"isActive":true`)
 }
+
+func TestOidcClientDto_secretsAlwaysSerialized(t *testing.T) {
+	client := model.OidcClient{
+		Base: model.Base{ID: "client-id"},
+		Name: "Test Client",
+	}
+
+	var clientDto OidcClientDto
+	require.NoError(t, MapStruct(client, &clientDto))
+	assert.Empty(t, clientDto.Credentials.Secrets)
+
+	// A client without secrets must serialize an empty list rather than omitting the field, so consumers never have to handle a missing value
+	serialized, err := json.Marshal(clientDto)
+	require.NoError(t, err)
+	assert.Contains(t, string(serialized), `"secrets":[]`)
+}

backend/internal/model/oidc.go

@@ -112,14 +112,10 @@ const (
 
 // OidcClientSecret is a single client secret of an OIDC client, stored hashed in the credentials JSON document
 type OidcClientSecret struct {
-	// ID uniquely identifies the secret within the client, so it can be deleted without knowing its value
-	ID string `json:"id"`
-	// Algorithm used to compute Hash
+	ID        string                        `json:"id"`
 	Algorithm OidcClientSecretHashAlgorithm `json:"alg"`
-	// Hash of the secret's value, hex-encoded for SHA-256 secrets and in modular crypt format for bcrypt ones
-	Hash string `json:"hash"`
-	// Prefix contains the first few characters of the secret in clear text, so admins can match a secret to the app that uses it
-	// It is empty for secrets migrated from the single-secret column, whose value was never stored
+	Hash      string                        `json:"hash"`
+	// Prefix is empty for secrets migrated from the single-secret column, whose value was never stored
 	Prefix    string             `json:"prefix,omitempty"`
 	CreatedAt datatype.DateTime  `json:"createdAt"`
 	ExpiresAt *datatype.DateTime `json:"expiresAt,omitempty"`

frontend/src/lib/types/oidc.type.ts

@@ -37,7 +37,7 @@ export type OidcClientSecretCreated = OidcClientSecret & {
 
 export type OidcClientCredentials = {
 	federatedIdentities: OidcClientFederatedIdentity[];
-	secrets?: OidcClientSecret[];
+	secrets: OidcClientSecret[];
 };
 
 export type OidcDiscoveryConfiguration = {

frontend/src/routes/settings/admin/oidc-clients/[id]/+page.svelte

@@ -16,6 +16,7 @@
 	import type {
 		OidcClientCreateWithLogo,
 		OidcClientCredentials,
+		OidcClientFederatedIdentity,
 		OidcClientSecret,
 		OidcClientTokenLifetimes
 	} from '$lib/types/oidc.type';
@@ -125,7 +126,9 @@
 		return success;
 	}
 
-	async function updateFederatedCredentials(credentials: OidcClientCredentials) {
+	async function updateFederatedCredentials(federatedIdentities: OidcClientFederatedIdentity[]) {
+		// Secrets are read-only in this request, but they are carried over so the client object keeps matching what the server has
+		const credentials: OidcClientCredentials = { federatedIdentities, secrets: clientSecrets };
 		const success = await updateClient({ ...client, credentials });
 		if (success) {
 			client.credentials = credentials;

frontend/src/routes/settings/admin/oidc-clients/[id]/oidc-client-federated-credentials-card.svelte

@@ -2,7 +2,7 @@
 	import { Button } from '$lib/components/ui/button';
 	import * as Card from '$lib/components/ui/card';
 	import { m } from '$lib/paraglide/messages';
-	import type { OidcClient, OidcClientCredentials } from '$lib/types/oidc.type';
+	import type { OidcClient, OidcClientFederatedIdentity } from '$lib/types/oidc.type';
 	import { preventDefault } from '$lib/utils/event-util';
 	import { createForm } from '$lib/utils/form-util';
 	import { slide } from 'svelte/transition';
@@ -14,7 +14,7 @@
 		callback
 	}: {
 		client: OidcClient;
-		callback: (credentials: OidcClientCredentials) => Promise<boolean>;
+		callback: (federatedIdentities: OidcClientFederatedIdentity[]) => Promise<boolean>;
 	} = $props();
 
 	let isLoading = $state(false);
@@ -72,7 +72,7 @@
 		if (!data) return;
 
 		isLoading = true;
-		await callback(data.credentials).finally(() => (isLoading = false));
+		await callback(data.credentials.federatedIdentities).finally(() => (isLoading = false));
 	}
 </script>
 

frontend/src/routes/settings/admin/oidc-clients/oidc-client-form.svelte

@@ -109,7 +109,7 @@
 
 		const success = await callback({
 			...data,
-			credentials: existingClient?.credentials ?? { federatedIdentities: [] },
+			credentials: existingClient?.credentials ?? { federatedIdentities: [], secrets: [] },
 			logo: $inputs.logoUrl?.value ? undefined : logo,
 			logoUrl: $inputs.logoUrl?.value,
 			darkLogo: $inputs.darkLogoUrl?.value ? undefined : darkLogo,