fix(settings): omit internal provider values

api/src/unraid-api/graph/resolvers/settings/settings.resolver.ts

@@ -17,7 +17,40 @@ import {
 import { ApiSettings } from '@app/unraid-api/graph/resolvers/settings/settings.service.js';
 import { SsoSettings } from '@app/unraid-api/graph/resolvers/settings/sso-settings.model.js';
 import { OidcConfigPersistence } from '@app/unraid-api/graph/resolvers/sso/core/oidc-config.service.js';
-import { OidcProvider } from '@app/unraid-api/graph/resolvers/sso/models/oidc-provider.model.js';
+import {
+    OidcProvider,
+    redactOidcClientSecret,
+} from '@app/unraid-api/graph/resolvers/sso/models/oidc-provider.model.js';
+
+const redactOidcClientSecrets = (values: Record<string, unknown>): Record<string, unknown> => {
+    const sso = values.sso;
+    if (!sso || typeof sso !== 'object' || Array.isArray(sso)) {
+        return values;
+    }
+
+    const providers = (sso as Record<string, unknown>).providers;
+    if (!Array.isArray(providers)) {
+        return values;
+    }
+
+    return {
+        ...values,
+        sso: {
+            ...(sso as Record<string, unknown>),
+            providers: providers.map((provider) => {
+                if (!provider || typeof provider !== 'object' || Array.isArray(provider)) {
+                    return provider;
+                }
+
+                const { clientSecret: _clientSecret, ...safeProvider } = provider as Record<
+                    string,
+                    unknown
+                >;
+                return safeProvider;
+            }),
+        },
+    };
+};
 
 @Resolver(() => Settings)
 export class SettingsResolver {
@@ -120,8 +153,7 @@ export class UnifiedSettingsResolver {
     })
     @ResolveField(() => GraphQLJSON)
     async values() {
-        // Unified settings include persisted OIDC client secrets.
-        return this.userSettings.getAllValues();
+        return redactOidcClientSecrets(await this.userSettings.getAllValues());
     }
 
     @Mutation(() => UpdateSettingsResponse)
@@ -132,14 +164,14 @@ export class UnifiedSettingsResolver {
     async updateSettings(
         @Args('input', { type: () => GraphQLJSON }) input: Record<string, unknown>
     ): Promise<UpdateSettingsResponse> {
-        this.logger.verbose('Updating Settings %O', input);
+        this.logger.verbose('Updating Settings %O', redactOidcClientSecrets(input));
         const { restartRequired, values } = await this.userSettings.updateNamespacedValues(input);
-        this.logger.verbose('Updated Setting Values %O', values);
+        this.logger.verbose('Updated Setting Values %O', redactOidcClientSecrets(values));
         if (restartRequired) {
             // hack: allow time for pending writes to flush
             this.lifecycleService.restartApi({ delayMs: 300 });
         }
-        return { restartRequired, values };
+        return { restartRequired, values: redactOidcClientSecrets(values) };
     }
 }
 
@@ -153,6 +185,7 @@ export class SsoSettingsResolver {
     })
     @ResolveField(() => [OidcProvider], { description: 'List of configured OIDC providers' })
     async oidcProviders(): Promise<OidcProvider[]> {
-        return this.oidcConfig.getProviders();
+        const providers = await this.oidcConfig.getProviders();
+        return providers.map(redactOidcClientSecret);
     }
 }

api/src/unraid-api/graph/resolvers/sso/core/oidc-config.service.ts

@@ -282,11 +282,21 @@ export class OidcConfigPersistence extends ConfigFilePersister<OidcConfig> {
         }
 
         // Clean up the provider object - remove UI-only fields
+        const existingProvider =
+            providers.find((currentProvider) => currentProvider.id === provider.id) ||
+            providers.find(
+                (currentProvider) =>
+                    currentProvider.clientId === provider.clientId &&
+                    (currentProvider.issuer ?? '') === (provider.issuer ?? '') &&
+                    (currentProvider.authorizationEndpoint ?? '') ===
+                        (provider.authorizationEndpoint ?? '') &&
+                    (currentProvider.tokenEndpoint ?? '') === (provider.tokenEndpoint ?? '')
+            );
         const cleanedProvider: OidcProvider = {
             id: provider.id,
             name: provider.name,
             clientId: provider.clientId,
-            clientSecret: provider.clientSecret,
+            clientSecret: provider.clientSecret ?? existingProvider?.clientSecret,
             usePkce: provider.usePkce,
             issuer: provider.issuer,
             authorizationEndpoint: provider.authorizationEndpoint,
@@ -300,7 +310,7 @@ export class OidcConfigPersistence extends ConfigFilePersister<OidcConfig> {
             buttonStyle: provider.buttonStyle,
         };
 
-        const existingIndex = providers.findIndex((p) => p.id === provider.id);
+        const existingIndex = existingProvider ? providers.indexOf(existingProvider) : -1;
         if (existingIndex >= 0) {
             providers[existingIndex] = cleanedProvider;
         } else {
@@ -416,11 +426,32 @@ export class OidcConfigPersistence extends ConfigFilePersister<OidcConfig> {
                     >;
                 }
             ) => {
+                const currentConfig =
+                    this.configService.get<OidcConfig>(this.configKey()) || this.defaultConfig();
+
                 // Process each provider to handle simple mode conversion
                 const processedConfig: OidcConfig = {
                     ...config,
                     providers: config.providers.map((provider) => {
-                        const extendedProvider = provider as OidcProvider & {
+                        const existingProvider =
+                            currentConfig.providers.find(
+                                (currentProvider) => currentProvider.id === provider.id
+                            ) ||
+                            currentConfig.providers.find(
+                                (currentProvider) =>
+                                    currentProvider.clientId === provider.clientId &&
+                                    (currentProvider.issuer ?? '') === (provider.issuer ?? '') &&
+                                    (currentProvider.authorizationEndpoint ?? '') ===
+                                        (provider.authorizationEndpoint ?? '') &&
+                                    (currentProvider.tokenEndpoint ?? '') ===
+                                        (provider.tokenEndpoint ?? '')
+                            );
+                        const providerWithSecret =
+                            provider.clientSecret === undefined &&
+                            existingProvider?.clientSecret !== undefined
+                                ? { ...provider, clientSecret: existingProvider.clientSecret }
+                                : provider;
+                        const extendedProvider = providerWithSecret as OidcProvider & {
                             authorizationMode?: string;
                             simpleAuthorization?: unknown;
                         };

api/src/unraid-api/graph/resolvers/sso/models/oidc-provider.model.ts

@@ -174,3 +174,8 @@ export class OidcProvider {
     @IsOptional()
     buttonStyle?: string;
 }
+
+export const redactOidcClientSecret = (provider: OidcProvider): Omit<OidcProvider, 'clientSecret'> => {
+    const { clientSecret: _clientSecret, ...safeProvider } = provider;
+    return safeProvider;
+};

api/src/unraid-api/graph/resolvers/sso/sso.resolver.ts

@@ -8,7 +8,10 @@ import { UsePermissions } from '@unraid/shared/use-permissions.directive.js';
 import { Public } from '@app/unraid-api/auth/public.decorator.js';
 import { OidcConfigPersistence } from '@app/unraid-api/graph/resolvers/sso/core/oidc-config.service.js';
 import { OidcConfiguration } from '@app/unraid-api/graph/resolvers/sso/models/oidc-configuration.model.js';
-import { OidcProvider } from '@app/unraid-api/graph/resolvers/sso/models/oidc-provider.model.js';
+import {
+    OidcProvider,
+    redactOidcClientSecret,
+} from '@app/unraid-api/graph/resolvers/sso/models/oidc-provider.model.js';
 import { OidcSessionValidation } from '@app/unraid-api/graph/resolvers/sso/models/oidc-session-validation.model.js';
 import { PublicOidcProvider } from '@app/unraid-api/graph/resolvers/sso/models/public-oidc-provider.model.js';
 import { OidcSessionService } from '@app/unraid-api/graph/resolvers/sso/session/oidc-session.service.js';
@@ -76,7 +79,8 @@ export class SsoResolver {
         resource: Resource.CONFIG,
     })
     public async oidcProviders(): Promise<OidcProvider[]> {
-        return this.oidcConfig.getProviders();
+        const providers = await this.oidcConfig.getProviders();
+        return providers.map(redactOidcClientSecret);
     }
 
     @Query(() => OidcProvider, { nullable: true, description: 'Get a specific OIDC provider by ID' })
@@ -87,7 +91,8 @@ export class SsoResolver {
     public async oidcProvider(
         @Args('id', { type: () => PrefixedID }) id: string
     ): Promise<OidcProvider | null> {
-        return this.oidcConfig.getProvider(id);
+        const provider = await this.oidcConfig.getProvider(id);
+        return provider ? redactOidcClientSecret(provider) : null;
     }
 
     @Query(() => OidcConfiguration, { description: 'Get the full OIDC configuration (admin only)' })
@@ -98,7 +103,7 @@ export class SsoResolver {
     public async oidcConfiguration(): Promise<OidcConfiguration> {
         const config = await this.oidcConfig.getConfig();
         return {
-            providers: config?.providers || [],
+            providers: (config?.providers || []).map(redactOidcClientSecret),
             defaultAllowedOrigins: config?.defaultAllowedOrigins || [],
         };
     }