Skip to content

web/admin: Improve WS-Fed algo selection logic (cherry-pick #20881 to version-2026.2) #21438

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
authentik-automation wants to merge 5 commits into from
Closed

web/admin: Improve WS-Fed algo selection logic (cherry-pick #20881 to version-2026.2) #21438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
63338f4
Cherry-pick #20881 to version-2026.2 (with conflicts)
PeshekDotDev Apr 7, 2026
1d18ee7
fix
PeshekDotDev Apr 7, 2026
e9e8021
Merge branch 'version-2026.2' into cherry-pick/20881-to-version-2026.2
gergosimonyi Apr 9, 2026
5af82c1
Merge branch 'version-2026.2' into cherry-pick/20881-to-version-2026.2
gergosimonyi Apr 9, 2026
2e564b6
Merge branch 'version-2026.2' into cherry-pick/20881-to-version-2026.2
PeshekDotDev Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion ...src/admin/applications/wizard/steps/providers/ak-application-wizard-provider-for-wsfed.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { ApplicationWizardProviderForm } from "./ApplicationWizardProviderForm.j
import { type AkCryptoCertificateSearch } from "#admin/common/ak-crypto-certificate-search";
import { renderForm } from "#admin/providers/wsfed/WSFederationProviderFormForm";

import { type WSFederationProvider } from "@goauthentik/api";
import { KeyTypeEnum, type WSFederationProvider } from "@goauthentik/api";

import { msg } from "@lit/localize";
import { customElement, state } from "@lit/reactive-element/decorators.js";
Expand All @@ -19,11 +19,15 @@ export class ApplicationWizardProviderWSFedForm extends ApplicationWizardProvide
@state()
protected hasSigningKp = false;

@state()
protected signingKeyType: KeyTypeEnum | null = null;

renderForm() {
const setHasSigningKp = (ev: InputEvent) => {
const target = ev.target as AkCryptoCertificateSearch;
Copy link
Copy Markdown
Contributor

@kensternberg-authentik kensternberg-authentik Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider putting a typeguard function here. InputEvent is very generic; checking that the source really is an AkCryptoCertificateSearch might catch someone using this wrong.

if (!target) return;
this.hasSigningKp = !!target.selectedKeypair;
this.signingKeyType = target.selectedKeypair?.keyType ?? KeyTypeEnum.Rsa;
Copy link
Copy Markdown
Contributor

@kensternberg-authentik kensternberg-authentik Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This expression appears three different times, in three different Forms. If the default ever changes, someone's gonna miss one. I wonder if there's a way to extract this and put it somewhere safe.

};

return html` <ak-wizard-title>${this.label}</ak-wizard-title>
Expand All @@ -33,6 +37,7 @@ export class ApplicationWizardProviderWSFedForm extends ApplicationWizardProvide
errors: this.wizard.errors?.provider,
setHasSigningKp,
hasSigningKp: this.hasSigningKp,
signingKeyType: this.signingKeyType,
})}
</form>`;
}
Expand Down
4 changes: 3 additions & 1 deletion web/src/admin/providers/saml/SAMLProviderFormForm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import "#elements/utils/TimeDeltaHelp";
import { propertyMappingsProvider, propertyMappingsSelector } from "./SAMLProviderFormHelpers.js";
import {
availableHashes,
DEFAULT_HASH_ALGORITHM,
digestAlgorithmOptions,
retrieveSignatureAlgorithm,
SAMLSupportedKeyTypes,
Expand Down Expand Up @@ -525,7 +526,8 @@ export function renderForm({
<option
value=${algorithmValue}
?selected=${provider?.signatureAlgorithm === algorithmValue ||
(!isCurrentAlgorithmAvailable && hash === "SHA256")}
(!isCurrentAlgorithmAvailable &&
hash === DEFAULT_HASH_ALGORITHM)}
Copy link
Copy Markdown
Contributor

@kensternberg-authentik kensternberg-authentik Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you touched it, maybe you can fix an issue here: isCurrentAlgorithmAvailable is recalculated on every loop, but doesn't use the variable that changes with each loop (hash). Consider moving the calculation for isCurrentAlgorithmAvailable out of the map() function. Since you only ever check the negation, you could rewrite in to something like currentAlgorithmNotAvailable.

>
${hash}
</option>
Expand Down
2 changes: 2 additions & 0 deletions web/src/admin/providers/saml/SAMLProviderOptions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ export const signatureAlgorithmOptions = toOptions([

export type HashAlgorithm = "SHA1" | "SHA256" | "SHA384" | "SHA512";

export const DEFAULT_HASH_ALGORITHM: HashAlgorithm = "SHA256";

export const availableHashes: HashAlgorithm[] = ["SHA1", "SHA256", "SHA384", "SHA512"];

export const SignatureFamilyByHashAlgorithm: Partial<
Expand Down
7 changes: 6 additions & 1 deletion web/src/admin/providers/wsfed/WSFederationProviderForm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { DEFAULT_CONFIG } from "#common/api/config";
import AkCryptoCertificateSearch from "#admin/common/ak-crypto-certificate-search";
import { BaseProviderForm } from "#admin/providers/BaseProviderForm";

import { ProvidersApi, WSFederationProvider } from "@goauthentik/api";
import { KeyTypeEnum, ProvidersApi, WSFederationProvider } from "@goauthentik/api";

import { html, TemplateResult } from "lit";
import { customElement, state } from "lit/decorators.js";
Expand All @@ -17,6 +17,9 @@ export class WSFederationProviderForm extends BaseProviderForm<WSFederationProvi
@state()
protected hasSigningKp = false;

@state()
protected signingKeyType: KeyTypeEnum | null = null;

async loadInstance(pk: number): Promise<WSFederationProvider> {
const provider = await new ProvidersApi(DEFAULT_CONFIG).providersWsfedRetrieve({
id: pk,
Expand All @@ -42,12 +45,14 @@ export class WSFederationProviderForm extends BaseProviderForm<WSFederationProvi
const target = ev.target as AkCryptoCertificateSearch;
if (!target) return;
this.hasSigningKp = !!target.selectedKeypair;
this.signingKeyType = target.selectedKeypair?.keyType ?? KeyTypeEnum.Rsa;
Copy link
Copy Markdown
Contributor

@kensternberg-authentik kensternberg-authentik Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 47 echoes Line 27. Is there a reason line 48 has no equivalent setter in loadInstance()?

};

return html`${renderForm({
provider: this.instance ?? {},
setHasSigningKp,
hasSigningKp: this.hasSigningKp,
signingKeyType: this.signingKeyType,
})}`;
}
}
Expand Down
64 changes: 53 additions & 11 deletions web/src/admin/providers/wsfed/WSFederationProviderFormForm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,16 @@ import {
propertyMappingsSelector,
} from "#admin/providers/saml/SAMLProviderFormHelpers";
import {
availableHashes,
DEFAULT_HASH_ALGORITHM,
digestAlgorithmOptions,
signatureAlgorithmOptions,
retrieveSignatureAlgorithm,
SAMLSupportedKeyTypes,
} from "#admin/providers/saml/SAMLProviderOptions";

import {
FlowsInstancesListDesignationEnum,
KeyTypeEnum,
PropertymappingsApi,
SAMLNameIDPolicyEnum,
SAMLPropertyMapping,
Expand Down Expand Up @@ -52,14 +56,17 @@ export interface WSFederationProviderFormProps {
errors?: ValidationError;
setHasSigningKp: (ev: InputEvent) => void;
hasSigningKp: boolean;
signingKeyType: KeyTypeEnum | null;
}

export function renderForm({
provider = {},
errors = {},
setHasSigningKp,
hasSigningKp,
signingKeyType,
}: WSFederationProviderFormProps) {
const keyType = signingKeyType ?? KeyTypeEnum.Rsa;
const samlPropertyMappingSearch = async (query?: string) =>
(
await new PropertymappingsApi(DEFAULT_CONFIG).propertymappingsProviderSamlList(
Expand Down Expand Up @@ -170,6 +177,7 @@ export function renderForm({
.certificate=${provider.signingKp}
@input=${setHasSigningKp}
singleton
.allowedKeyTypes=${SAMLSupportedKeyTypes}
></ak-crypto-certificate-search>
<p class="pf-c-form__helper-text">
${msg(
Expand Down Expand Up @@ -202,6 +210,8 @@ export function renderForm({
>
<ak-crypto-certificate-search
.certificate=${provider.encryptionKp}
nokey
.allowedKeyTypes=${SAMLSupportedKeyTypes}
></ak-crypto-certificate-search>
<p class="pf-c-form__helper-text">
${msg("When selected, assertions will be encrypted using this keypair.")}
Expand Down Expand Up @@ -278,23 +288,55 @@ export function renderForm({
</p>
</ak-form-element-horizontal>

<ak-radio-input
name="digestAlgorithm"
<ak-form-element-horizontal
label=${msg("Digest algorithm")}
.options=${digestAlgorithmOptions}
.value=${provider.digestAlgorithm}
required
name="digestAlgorithm"
>
</ak-radio-input>
<select class="pf-c-form-control">
${digestAlgorithmOptions.map(
(opt) => html`
<option
value=${opt.value}
?selected=${provider?.digestAlgorithm === opt.value ||
(!provider?.digestAlgorithm && opt.default)}
>
${opt.label}
</option>
`,
)}
</select>
</ak-form-element-horizontal>

<ak-radio-input
name="signatureAlgorithm"
<ak-form-element-horizontal
label=${msg("Signature algorithm")}
.options=${signatureAlgorithmOptions}
.value=${provider.signatureAlgorithm}
required
name="signatureAlgorithm"
>
</ak-radio-input>
<select class="pf-c-form-control">
${availableHashes.map((hash) => {
const algorithmValue = retrieveSignatureAlgorithm(keyType, hash);
if (!algorithmValue) return nothing;

const isCurrentAlgorithmAvailable = availableHashes.some(
(h) =>
retrieveSignatureAlgorithm(keyType, h) ===
provider?.signatureAlgorithm,
);

return html`
<option
value=${algorithmValue}
?selected=${provider?.signatureAlgorithm === algorithmValue ||
(!isCurrentAlgorithmAvailable &&
hash === DEFAULT_HASH_ALGORITHM)}
>
${hash}
</option>
`;
})}
</select>
</ak-form-element-horizontal>
</div>
</ak-form-group>`;
}
Loading