From fa51973e6d3a040e17c4b8c53850d2ea6ae068d0 Mon Sep 17 00:00:00 2001 From: Dharini Date: Thu, 4 Jun 2026 15:40:24 -0700 Subject: [PATCH] chore: make sub claim mapping readonly after creation (#3306) --- addons/api/addon/serializers/auth-method.js | 12 +++ .../unit/serializers/auth-method-test.js | 86 +++++++++++++++++++ .../form/auth-method/oidc/index.hbs | 4 +- .../components/form/auth-method/oidc/index.js | 29 +++++++ .../acceptance/auth-methods/create-test.js | 3 + .../acceptance/auth-methods/selectors.js | 3 +- .../acceptance/auth-methods/update-test.js | 76 ++++++++++++++-- 7 files changed, 201 insertions(+), 12 deletions(-) diff --git a/addons/api/addon/serializers/auth-method.js b/addons/api/addon/serializers/auth-method.js index 4b9940999..2ad329ab8 100644 --- a/addons/api/addon/serializers/auth-method.js +++ b/addons/api/addon/serializers/auth-method.js @@ -48,6 +48,18 @@ export default class AuthMethodSerializer extends ApplicationSerializer { } else { delete serialized.attributes.state; } + + // Don't send account_claim_maps on update if there's a saved 'sub' mapping + // The API doesn't allow updating when 'sub' claim mappings exist + if (!snapshot.record.isNew && snapshot.record.account_claim_maps) { + const hasSubClaimMap = snapshot.record.account_claim_maps.some( + (item) => item?.value === 'sub', + ); + + if (hasSubClaimMap) { + delete serialized.attributes.account_claim_maps; + } + } return serialized; } diff --git a/addons/api/tests/unit/serializers/auth-method-test.js b/addons/api/tests/unit/serializers/auth-method-test.js index 75297023e..6c8d5ff71 100644 --- a/addons/api/tests/unit/serializers/auth-method-test.js +++ b/addons/api/tests/unit/serializers/auth-method-test.js @@ -506,4 +506,90 @@ module('Unit | Serializer | auth method', function (hooks) { assert.strictEqual(record.client_certificate_key, ''); assert.strictEqual(record.bind_password, ''); }); + + test('it includes account_claim_maps when serializing new OIDC records', function (assert) { + const store = this.owner.lookup('service:store'); + const serializer = store.serializerFor('auth-method'); + const record = store.createRecord('auth-method', { + type: TYPE_AUTH_METHOD_OIDC, + name: 'Test OIDC', + account_claim_maps: [ + { key: 'email', value: 'email' }, + { key: 'name', value: 'sub' }, + ], + }); + const snapshot = record._createSnapshot(); + const serializedRecord = serializer.serialize(snapshot); + + assert.ok( + serializedRecord.attributes.account_claim_maps, + 'account_claim_maps is included for new records', + ); + + assert.deepEqual(serializedRecord.attributes.account_claim_maps, [ + 'email=email', + 'name=sub', + ]); + }); + + test('it excludes account_claim_maps when serializing existing OIDC records with "sub" mapping', function (assert) { + const store = this.owner.lookup('service:store'); + const serializer = store.serializerFor('auth-method'); + + const record = store.push({ + data: { + id: 'oidc123', + type: 'auth-method', + attributes: { + type: TYPE_AUTH_METHOD_OIDC, + name: 'Test OIDC', + account_claim_maps: [ + { key: 'email', value: 'email' }, + { key: 'name', value: 'sub' }, + ], + }, + }, + }); + + const snapshot = record._createSnapshot(); + const serializedRecord = serializer.serialize(snapshot); + + assert.notOk( + serializedRecord.attributes.account_claim_maps, + 'account_claim_maps is excluded when updating with "sub" mapping', + ); + }); + + test('it includes account_claim_maps when serializing existing OIDC records without "sub" mapping', function (assert) { + const store = this.owner.lookup('service:store'); + const serializer = store.serializerFor('auth-method'); + + const record = store.push({ + data: { + id: 'oidc456', + type: 'auth-method', + attributes: { + type: TYPE_AUTH_METHOD_OIDC, + name: 'Test OIDC', + account_claim_maps: [ + { key: 'email', value: 'email' }, + { key: 'name', value: 'name' }, + ], + }, + }, + }); + + const snapshot = record._createSnapshot(); + const serializedRecord = serializer.serialize(snapshot); + + assert.ok( + serializedRecord.attributes.account_claim_maps, + 'account_claim_maps is included when updating without "sub" mapping', + ); + + assert.deepEqual(serializedRecord.attributes.account_claim_maps, [ + 'email=email', + 'name=name', + ]); + }); }); diff --git a/ui/admin/app/components/form/auth-method/oidc/index.hbs b/ui/admin/app/components/form/auth-method/oidc/index.hbs index b62b37d8a..154740e02 100644 --- a/ui/admin/app/components/form/auth-method/oidc/index.hbs +++ b/ui/admin/app/components/form/auth-method/oidc/index.hbs @@ -242,7 +242,7 @@ @data={{@model.account_claim_maps}} @legend={{t 'form.account_claim_maps.label'}} @helperText={{t 'form.account_claim_maps.help'}} - @disabled={{form.disabled}} + @disabled={{or form.disabled this.hasSavedSubClaimMap}} @isOptional={{true}} > <:row as |R|> @@ -264,7 +264,7 @@ - {{#each this.toClaims as |claim|}} + {{#each (this.toClaimOptions R.rowData) as |claim|}} diff --git a/ui/admin/app/components/form/auth-method/oidc/index.js b/ui/admin/app/components/form/auth-method/oidc/index.js index 0419e43f2..f078cbbbe 100644 --- a/ui/admin/app/components/form/auth-method/oidc/index.js +++ b/ui/admin/app/components/form/auth-method/oidc/index.js @@ -39,6 +39,35 @@ export default class FormAuthMethodOidcComponent extends Component { @tracked selectedPrompts = this.parsePromptsArray(); @tracked skipPromptsList = this.isToggleChecked(); + /** + * @returns {boolean} + */ + get hasSavedSubClaimMap() { + return ( + !this.args.model?.isNew && + this.args.model.account_claim_maps?.some((item) => item?.value === 'sub') + ); + } + + /** + * Filter 'sub' from dropdown options when updating + * On creation, show all options including sub. + * On update, hide sub unless it's the current value of the row (to display it when disabled). + * @param {Object} rowData - The current row data + * @returns {string[]} + */ + toClaimOptions = (rowData) => { + // On creation, show all options + if (this.args.model?.isNew) { + return this.toClaims; + } + + // On update, filter "sub" unless it's the current value + return this.toClaims.filter( + (claim) => claim !== 'sub' || rowData?.value === 'sub', + ); + }; + /** * @returns {string} */ diff --git a/ui/admin/tests/acceptance/auth-methods/create-test.js b/ui/admin/tests/acceptance/auth-methods/create-test.js index 88084c3a1..a9d4f58f3 100644 --- a/ui/admin/tests/acceptance/auth-methods/create-test.js +++ b/ui/admin/tests/acceptance/auth-methods/create-test.js @@ -120,6 +120,9 @@ module('Acceptance | auth-methods | create', function (hooks) { ); await click(selectors.FIELD_CLAIMS_SCOPES_ADD_BTN); + // Verify Account Claim Maps field is editable during creation + assert.dom(selectors.FIELD_ACCOUNT_CLAIM_MAPS).isNotDisabled(); + await fillIn( selectors.FIELD_ACCOUNT_CLAIM_MAPS_FROM_CLAIM, selectors.FIELD_ACCOUNT_CLAIM_MAPS_FROM_CLAIM_VALUE, diff --git a/ui/admin/tests/acceptance/auth-methods/selectors.js b/ui/admin/tests/acceptance/auth-methods/selectors.js index 17821088b..26c09a1e6 100644 --- a/ui/admin/tests/acceptance/auth-methods/selectors.js +++ b/ui/admin/tests/acceptance/auth-methods/selectors.js @@ -30,12 +30,13 @@ export const FIELD_CLAIMS_SCOPES_ADD_BTN = '[name=claims_scopes] [data-test-add-button]'; export const FIELD_CLAIMS_SCOPES_DELETE_BTN = '[name=claims_scopes] [data-test-delete-button]'; +export const FIELD_ACCOUNT_CLAIM_MAPS = '[name=account_claim_maps]'; export const FIELD_ACCOUNT_CLAIM_MAPS_FROM_CLAIM = '[name=account_claim_maps] [data-test-key-input]'; export const FIELD_ACCOUNT_CLAIM_MAPS_FROM_CLAIM_VALUE = 'from_claim'; export const FIELD_ACCOUNT_CLAIM_MAPS_TO_CLAIM = '[name=account_claim_maps] [data-test-value-select]'; -export const FIELD_ACCOUNT_CLAIM_MAPS_TO_CLAIM_VALUE = 'email'; +export const FIELD_ACCOUNT_CLAIM_MAPS_TO_CLAIM_VALUE = 'sub'; export const FIELD_ACCOUNT_CLAIM_MAPS_DELETE_BTN = '[name=account_claim_maps] [data-test-delete-button]'; export const FIELD_IDP_CERTS = '[name=idp_ca_certs] [data-test-value-input]'; diff --git a/ui/admin/tests/acceptance/auth-methods/update-test.js b/ui/admin/tests/acceptance/auth-methods/update-test.js index eabdf1bac..5fab662b5 100644 --- a/ui/admin/tests/acceptance/auth-methods/update-test.js +++ b/ui/admin/tests/acceptance/auth-methods/update-test.js @@ -169,14 +169,16 @@ module('Acceptance | auth-methods | update', function (hooks) { await click(element); } - await fillIn( - selectors.FIELD_ACCOUNT_CLAIM_MAPS_FROM_CLAIM, - selectors.FIELD_ACCOUNT_CLAIM_MAPS_FROM_CLAIM_VALUE, - ); - await select( - selectors.FIELD_ACCOUNT_CLAIM_MAPS_TO_CLAIM, - selectors.FIELD_ACCOUNT_CLAIM_MAPS_TO_CLAIM_VALUE, - ); + await fillIn(selectors.FIELD_ACCOUNT_CLAIM_MAPS_FROM_CLAIM, 'test_claim'); + + // Verify "sub" option is not available during update + await click(selectors.FIELD_ACCOUNT_CLAIM_MAPS_TO_CLAIM); + assert + .dom(`${selectors.FIELD_ACCOUNT_CLAIM_MAPS_TO_CLAIM} option[value="sub"]`) + .doesNotExist('"sub" option is not available during update'); + + // Select 'email' instead since 'sub' is not available + await select(selectors.FIELD_ACCOUNT_CLAIM_MAPS_TO_CLAIM, 'email'); // Remove all certificates const certificatesList = findAll(selectors.FIELD_IDP_CERTS_DELETE_BTN); @@ -223,7 +225,7 @@ module('Acceptance | auth-methods | update', function (hooks) { selectors.FIELD_CLAIMS_SCOPES_VALUE, ]); assert.deepEqual(authMethod.attributes.account_claim_maps, [ - 'from_claim=email', + 'test_claim=email', ]); assert.deepEqual(authMethod.attributes.idp_ca_certs, [ selectors.FIELD_IDP_CERTS_VALUE, @@ -239,6 +241,62 @@ module('Acceptance | auth-methods | update', function (hooks) { assert.deepEqual(authMethod.attributes.prompts, ['none']); }); + test('account claim maps field is disabled when sub mapping exists', async function (assert) { + setRunOptions({ + rules: { + 'color-contrast': { + // [ember-a11y-ignore]: axe rule "color-contrast" automatically ignored on 2026-06-02 + enabled: false, + }, + }, + }); + + // Create OIDC auth method with a 'sub' mapping + instances.authMethod = this.server.create('auth-method', { + scope: instances.scopes.org, + type: TYPE_AUTH_METHOD_OIDC, + }); + instances.authMethod.attributes.account_claim_maps = ['username=sub']; + + await visit(urls.authMethods); + + await click( + commonSelectors.HREF(`${urls.authMethods}/${instances.authMethod.id}`), + ); + await click(commonSelectors.EDIT_BTN); + + assert.dom(selectors.FIELD_ACCOUNT_CLAIM_MAPS).isDisabled(); + }); + + test('account claim maps field is editable when no sub mapping exists', async function (assert) { + setRunOptions({ + rules: { + 'color-contrast': { + // [ember-a11y-ignore]: axe rule "color-contrast" automatically ignored on 2026-06-02 + enabled: false, + }, + }, + }); + + // Create OIDC auth method with email/name mappings (no 'sub') + instances.authMethod = this.server.create('auth-method', { + scope: instances.scopes.org, + type: TYPE_AUTH_METHOD_OIDC, + }); + instances.authMethod.attributes.account_claim_maps = [ + 'username=email', + 'fullname=name', + ]; + + await visit(urls.authMethods); + await click( + commonSelectors.HREF(`${urls.authMethods}/${instances.authMethod.id}`), + ); + await click(commonSelectors.EDIT_BTN); + + assert.dom(selectors.FIELD_ACCOUNT_CLAIM_MAPS).isNotDisabled(); + }); + test('can update an ldap auth method and save changes', async function (assert) { setRunOptions({ rules: {