TheArchive/zitadel
1
0
Fork 0
mirror of https://github.com/zitadel/zitadel.git synced 2025-11-03 12:32:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(console): Improve SAML SP form typing (#10557)

Browse Source 
This PR fixes a problem for the SAML provider in console where the
binding selection was not correctly applied when editing existing
providers

# Which Problems Are Solved

- SAML provider binding selection was not correctly applied when editing
existing providers
- Form used untyped reactive forms leading to potential runtime errors
- Hardcoded enum handling made the code fragile to API changes

# How the Problems Are Solved

- Created reusable utility functions (enum.utils.ts) that properly
convert between numeric enum values (from backend) and string keys (for
form controls)
- Improved type safety: Migrated from
UntypedFormGroup/UntypedFormControl to strongly typed
FormGroup<SAMLProviderForm> with FormControl<T>

(cherry picked from commit b6a2b7c70e)
This commit is contained in:
Max Peintner
2025-08-27 09:29:14 +02:00
committed by Livio Spring
parent ce00cf22db
commit fe96299b8f
3 changed files with 162 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
console/src/app/modules/providers/provider-saml-sp/provider-saml-sp.component.html
View File

@@ -54,7 +54,7 @@
</cnsl-form-field>
<cnsl-form-field class="formfield">
<cnsl-label>{{ 'IDP.SAML.BINDING' | translate }}</cnsl-label>
<mat-select formControlName="binding" [compareWith]="compareBinding">
<mat-select formControlName="binding">
<mat-option *ngFor="let binding of bindingValues" [value]="binding">{{ binding }}</mat-option>
</mat-select>
</cnsl-form-field>
@@ -73,7 +73,7 @@
<div class="transient-info">
<cnsl-form-field class="formfield">
<cnsl-label>{{ 'IDP.SAML.NAMEIDFORMAT' | translate }}</cnsl-label>
<mat-select formControlName="nameIdFormat" [compareWith]="compareNameIDFormat">
<mat-select formControlName="nameIdFormat">
<mat-option *ngFor="let nameIdFormat of nameIDFormatValues" [value]="nameIdFormat">{{
nameIdFormat
}}</mat-option>

144
console/src/app/modules/providers/provider-saml-sp/provider-saml-sp.component.ts
View File

@@ -7,7 +7,7 @@ import {
SAMLBinding,
SAMLNameIDFormat,
} from '../../../proto/generated/zitadel/idp_pb';
import { AbstractControl, FormGroup, UntypedFormControl, UntypedFormGroup } from '@angular/forms';
import { AbstractControl, FormControl, FormGroup } from '@angular/forms';
import { PolicyComponentServiceType } from '../../policies/policy-component-types.enum';
import { ManagementService } from '../../../services/mgmt.service';
import { AdminService } from '../../../services/admin.service';
@@ -30,6 +30,18 @@ import {
import { Environment, EnvironmentService } from '../../../services/environment.service';
import { filter, map } from 'rxjs/operators';
import { ProviderNextService } from '../provider-next/provider-next.service';
import { getEnumKeys, getEnumKeyFromValue, convertEnumValuesToKeys } from '../../../utils/enum.utils';
interface SAMLProviderForm {
name: FormControl<string>;
metadataXml: FormControl<string>;
metadataUrl: FormControl<string>;
binding: FormControl<string>;
withSignedRequest: FormControl<boolean>;
nameIdFormat: FormControl<string>;
transientMappingAttributeName: FormControl<string>;
federatedLogoutEnabled: FormControl<boolean>;
}
@Component({
selector: 'cnsl-provider-saml-sp',
@@ -41,7 +53,7 @@ export class ProviderSamlSpComponent {
public id: string | null = '';
public loading: boolean = false;
public provider?: Provider.AsObject;
public form!: FormGroup;
public form!: FormGroup<SAMLProviderForm>;
public showOptional: boolean = false;
public options: Options = new Options()
.setIsCreationAllowed(true)
@@ -51,8 +63,8 @@ export class ProviderSamlSpComponent {
public serviceType: PolicyComponentServiceType = PolicyComponentServiceType.MGMT;
// DEPRECATED: use service$ instead
private service!: ManagementService | AdminService;
bindingValues: string[] = Object.keys(SAMLBinding);
nameIDFormatValues: string[] = Object.keys(SAMLNameIDFormat);
bindingValues: string[] = getEnumKeys(SAMLBinding);
nameIDFormatValues: string[] = getEnumKeys(SAMLNameIDFormat);
public justCreated$: BehaviorSubject<string> = new BehaviorSubject<string>('');
public justActivated$ = new BehaviorSubject<boolean>(false);
@@ -118,16 +130,20 @@ export class ProviderSamlSpComponent {
}
private _initializeForm(): void {
this.form = new UntypedFormGroup(
const defaultBinding = getEnumKeyFromValue(SAMLBinding, SAMLBinding.SAML_BINDING_POST) || this.bindingValues[0];
const defaultNameIdFormat =
getEnumKeyFromValue(SAMLNameIDFormat, SAMLNameIDFormat.SAML_NAME_ID_FORMAT_PERSISTENT) || this.nameIDFormatValues[0];
this.form = new FormGroup<SAMLProviderForm>(
{
name: new UntypedFormControl('', [requiredValidator]),
metadataXml: new UntypedFormControl('', []),
metadataUrl: new UntypedFormControl('', []),
binding: new UntypedFormControl(this.bindingValues[0], [requiredValidator]),
withSignedRequest: new UntypedFormControl(true, [requiredValidator]),
nameIdFormat: new UntypedFormControl(SAMLNameIDFormat.SAML_NAME_ID_FORMAT_PERSISTENT, []),
transientMappingAttributeName: new UntypedFormControl('', []),
federatedLogoutEnabled: new UntypedFormControl(false, []),
name: new FormControl('', { nonNullable: true, validators: [requiredValidator] }),
metadataXml: new FormControl('', { nonNullable: true }),
metadataUrl: new FormControl('', { nonNullable: true }),
binding: new FormControl(defaultBinding, { nonNullable: true, validators: [requiredValidator] }),
withSignedRequest: new FormControl(true, { nonNullable: true, validators: [requiredValidator] }),
nameIdFormat: new FormControl(defaultNameIdFormat, { nonNullable: true }),
transientMappingAttributeName: new FormControl('', { nonNullable: true }),
federatedLogoutEnabled: new FormControl(false, { nonNullable: true }),
},
atLeastOneIsFilled('metadataXml', 'metadataUrl'),
);
@@ -197,21 +213,19 @@ export class ProviderSamlSpComponent {
: new AdminUpdateSAMLProviderRequest();
req.setId(this.provider?.id || this.justCreated$.value);
req.setName(this.name?.value);
if (this.metadataXml?.value) {
req.setName(this.name.value);
if (this.metadataXml.value) {
req.setMetadataUrl('');
req.setMetadataXml(this.metadataXml?.value);
req.setMetadataXml(this.metadataXml.value);
} else {
req.setMetadataXml('');
req.setMetadataUrl(this.metadataUrl?.value);
req.setMetadataUrl(this.metadataUrl.value);
}
req.setWithSignedRequest(this.withSignedRequest?.value);
// @ts-ignore
req.setBinding(SAMLBinding[this.binding?.value]);
// @ts-ignore
req.setNameIdFormat(SAMLNameIDFormat[this.nameIDFormat?.value]);
req.setTransientMappingAttributeName(this.transientMapping?.value);
req.setFederatedLogoutEnabled(this.federatedLogoutEnabled?.value);
req.setWithSignedRequest(this.withSignedRequest.value);
req.setBinding(SAMLBinding[this.binding.value as keyof typeof SAMLBinding]);
req.setNameIdFormat(SAMLNameIDFormat[this.nameIDFormat.value as keyof typeof SAMLNameIDFormat]);
req.setTransientMappingAttributeName(this.transientMapping.value);
req.setFederatedLogoutEnabled(this.federatedLogoutEnabled.value);
req.setProviderOptions(this.options);
this.loading = true;
@@ -235,24 +249,22 @@ export class ProviderSamlSpComponent {
this.serviceType === PolicyComponentServiceType.MGMT
? new MgmtAddSAMLProviderRequest()
: new AdminAddSAMLProviderRequest();
req.setName(this.name?.value);
if (this.metadataXml?.value) {
req.setName(this.name.value);
if (this.metadataXml.value) {
req.setMetadataUrl('');
req.setMetadataXml(this.metadataXml?.value);
req.setMetadataXml(this.metadataXml.value);
} else {
req.setMetadataXml('');
req.setMetadataUrl(this.metadataUrl?.value);
req.setMetadataUrl(this.metadataUrl.value);
}
req.setProviderOptions(this.options);
// @ts-ignore
req.setBinding(SAMLBinding[this.binding?.value]);
req.setWithSignedRequest(this.withSignedRequest?.value);
req.setBinding(SAMLBinding[this.binding.value as keyof typeof SAMLBinding]);
req.setWithSignedRequest(this.withSignedRequest.value);
if (this.nameIDFormat) {
// @ts-ignore
req.setNameIdFormat(SAMLNameIDFormat[this.nameIDFormat.value]);
req.setNameIdFormat(SAMLNameIDFormat[this.nameIDFormat.value as keyof typeof SAMLNameIDFormat]);
}
req.setTransientMappingAttributeName(this.transientMapping?.value);
req.setFederatedLogoutEnabled(this.federatedLogoutEnabled?.value);
req.setTransientMappingAttributeName(this.transientMapping.value);
req.setFederatedLogoutEnabled(this.federatedLogoutEnabled.value);
this.loading = true;
this.service
.addSAMLProvider(req)
@@ -281,9 +293,20 @@ export class ProviderSamlSpComponent {
.then((resp) => {
this.provider = resp.idp;
this.loading = false;
this.name?.setValue(this.provider?.name);
this.name.setValue(this.provider?.name || '');
if (this.provider?.config?.saml) {
this.form.patchValue(this.provider.config.saml);
const samlConfig = this.provider.config.saml;
const bindingKey = getEnumKeyFromValue(SAMLBinding, samlConfig.binding) || '';
const nameIdFormatKey = getEnumKeyFromValue(SAMLNameIDFormat, samlConfig.nameIdFormat) || '';
this.form.patchValue({
metadataXml: typeof samlConfig.metadataXml === 'string' ? samlConfig.metadataXml : '',
binding: bindingKey,
withSignedRequest: samlConfig.withSignedRequest,
nameIdFormat: nameIdFormatKey,
transientMappingAttributeName: samlConfig.transientMappingAttributeName || '',
federatedLogoutEnabled: samlConfig.federatedLogoutEnabled || false,
});
}
})
.catch((error) => {
@@ -296,50 +319,35 @@ export class ProviderSamlSpComponent {
this._location.back();
}
compareBinding(value: string, index: number) {
if (value) {
return value === Object.keys(SAMLBinding)[index];
}
return false;
private get name(): FormControl<string> {
return this.form.controls.name;
}
compareNameIDFormat(value: string, index: number) {
console.log(value, index);
if (value) {
return value === Object.keys(SAMLNameIDFormat)[index];
}
return false;
private get metadataXml(): FormControl<string> {
return this.form.controls.metadataXml;
}
private get name(): AbstractControl | null {
return this.form.get('name');
private get metadataUrl(): FormControl<string> {
return this.form.controls.metadataUrl;
}
private get metadataXml(): AbstractControl | null {
return this.form.get('metadataXml');
private get binding(): FormControl<string> {
return this.form.controls.binding;
}
private get metadataUrl(): AbstractControl | null {
return this.form.get('metadataUrl');
private get withSignedRequest(): FormControl<boolean> {
return this.form.controls.withSignedRequest;
}
private get binding(): AbstractControl | null {
return this.form.get('binding');
private get nameIDFormat(): FormControl<string> {
return this.form.controls.nameIdFormat;
}
private get withSignedRequest(): AbstractControl | null {
return this.form.get('withSignedRequest');
private get transientMapping(): FormControl<string> {
return this.form.controls.transientMappingAttributeName;
}
private get nameIDFormat(): AbstractControl | null {
return this.form.get('nameIdFormat');
}
private get transientMapping(): AbstractControl | null {
return this.form.get('transientMappingAttributeName');
}
private get federatedLogoutEnabled(): AbstractControl | null {
return this.form.get('federatedLogoutEnabled');
private get federatedLogoutEnabled(): FormControl<boolean> {
return this.form.controls.federatedLogoutEnabled;
}
}

84
console/src/app/utils/enum.utils.ts Normal file
View File

@@ -0,0 +1,84 @@
/**
* Utility functions for working with TypeScript enums in Angular forms
*
* Example usage:
* ```typescript
* // Get dropdown options
* const bindingOptions = getEnumKeys(SAMLBinding);
*
* // Convert backend enum values to form-friendly string keys
* const formConfig = convertEnumValuesToKeys(backendConfig, {
* binding: SAMLBinding,
* nameIdFormat: SAMLNameIDFormat
* });
*
* // Find specific enum key
* const defaultBinding = getEnumKeyFromValue(SAMLBinding, SAMLBinding.SAML_BINDING_POST);
* ```
*/
// Type constraint for TypeScript numeric enums
type NumericEnum = Record<string, string | number> & Record<number, string>;
/**
* Get string keys from a TypeScript enum, excluding numeric reverse mappings
* @param enumObject The enum object
* @returns Array of string keys
* @example
* ```typescript
* const bindingOptions = getEnumKeys(SAMLBinding);
* // Returns: ['SAML_BINDING_UNSPECIFIED', 'SAML_BINDING_POST', ...]
* ```
*/
export function getEnumKeys<T extends NumericEnum>(enumObject: T): string[] {
return Object.keys(enumObject).filter((key) => isNaN(Number(key)));
}
/**
* Find the string key for a given numeric enum value
* @param enumObject The enum object
* @param value The numeric enum value
* @returns The corresponding string key or undefined if not found
* @example
* ```typescript
* const key = getEnumKeyFromValue(SAMLBinding, 1);
* // Returns: 'SAML_BINDING_POST'
* ```
*/
export function getEnumKeyFromValue<T extends NumericEnum>(enumObject: T, value: number): string | undefined {
return Object.keys(enumObject).find((key) => enumObject[key] === value && isNaN(Number(key)));
}
/**
* Convert enum values to string keys for form controls
* @param config Object containing enum values to convert
* @param enumMappings Object mapping config property names to enum objects
* @returns Modified config object with string keys
* @example
* ```typescript
* const formConfig = convertEnumValuesToKeys(samlConfig, {
* binding: SAMLBinding,
* nameIdFormat: SAMLNameIDFormat
* });
* // Converts: { binding: 1, nameIdFormat: 2 }
* // To: { binding: 'SAML_BINDING_POST', nameIdFormat: 'SAML_NAME_ID_FORMAT_PERSISTENT' }
* ```
*/
export function convertEnumValuesToKeys<T extends Record<string, unknown>>(
config: T,
enumMappings: { [K in keyof Partial<T>]: NumericEnum },
): T {
const converted = { ...config };
for (const [propertyName, enumObject] of Object.entries(enumMappings)) {
const typedPropertyName = propertyName as keyof T;
if (converted[typedPropertyName] !== undefined && typeof converted[typedPropertyName] === 'number') {
const key = getEnumKeyFromValue(enumObject, converted[typedPropertyName] as number);
if (key) {
(converted as Record<string, unknown>)[propertyName] = key;
}
}
}
return converted;
}