Skip to content
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.

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

UI: Fix oidc auth method missing default_role field #28539

Merged
merged 7 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions changelog/28539.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
ui: fix `default_role` input missing from oidc auth method configuration form
```
9 changes: 5 additions & 4 deletions ui/app/utils/openapi-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,26 +136,27 @@ export function filterPathsByItemType(pathInfo: PathsInfo, itemType: string): Pa
* This object maps model names to the openAPI path that hydrates the model, given the backend path.
*/
const OPENAPI_POWERED_MODELS = {
'role-ssh': (backend: string) => `/v1/${backend}/roles/example?help=1`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

alphabetized!

'auth-config/azure': (backend: string) => `/v1/auth/${backend}/config?help=1`,
'auth-config/cert': (backend: string) => `/v1/auth/${backend}/config?help=1`,
'auth-config/gcp': (backend: string) => `/v1/auth/${backend}/config?help=1`,
'auth-config/github': (backend: string) => `/v1/auth/${backend}/config?help=1`,
'auth-config/jwt': (backend: string) => `/v1/auth/${backend}/config?help=1`,
'auth-config/kubernetes': (backend: string) => `/v1/auth/${backend}/config?help=1`,
'auth-config/ldap': (backend: string) => `/v1/auth/${backend}/config?help=1`,
'auth-config/oidc': (backend: string) => `/v1/auth/${backend}/config?help=1`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the only new one!

'auth-config/okta': (backend: string) => `/v1/auth/${backend}/config?help=1`,
'auth-config/radius': (backend: string) => `/v1/auth/${backend}/config?help=1`,
'kmip/config': (backend: string) => `/v1/${backend}/config?help=1`,
'kmip/role': (backend: string) => `/v1/${backend}/scope/example/role/example?help=1`,
'pki/role': (backend: string) => `/v1/${backend}/roles/example?help=1`,
'pki/tidy': (backend: string) => `/v1/${backend}/config/auto-tidy?help=1`,
'pki/sign-intermediate': (backend: string) => `/v1/${backend}/issuer/example/sign-intermediate?help=1`,
'pki/certificate/generate': (backend: string) => `/v1/${backend}/issue/example?help=1`,
'pki/certificate/sign': (backend: string) => `/v1/${backend}/sign/example?help=1`,
'pki/config/acme': (backend: string) => `/v1/${backend}/config/acme?help=1`,
'pki/config/cluster': (backend: string) => `/v1/${backend}/config/cluster?help=1`,
'pki/config/urls': (backend: string) => `/v1/${backend}/config/urls?help=1`,
'pki/role': (backend: string) => `/v1/${backend}/roles/example?help=1`,
'pki/sign-intermediate': (backend: string) => `/v1/${backend}/issuer/example/sign-intermediate?help=1`,
'pki/tidy': (backend: string) => `/v1/${backend}/config/auto-tidy?help=1`,
'role-ssh': (backend: string) => `/v1/${backend}/roles/example?help=1`,
};

export function getHelpUrlForModel(modelType: string, backend: string) {
Expand Down
2 changes: 1 addition & 1 deletion ui/lib/core/addon/components/masked-input.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
aria-label={{or @name "masked input"}}
{{on "change" this.onChange}}
{{on "keyup" (fn this.handleKeyUp @name)}}
data-test-textarea={{or @name ""}}
data-test-input={{or @name ""}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was a lot of logic in tests because this selector differed from the standard pattern, updated it and relevant selectors to make future tests easier to write

Copy link
Contributor

Choose a reason for hiding this comment

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

I've also run into this. Thank you!

/>
{{/if}}
{{#if @allowCopy}}
Expand Down
10 changes: 4 additions & 6 deletions ui/tests/acceptance/auth-list-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ import { GENERAL } from 'vault/tests/helpers/general-selectors';
const SELECTORS = {
backendLink: (path) => `[data-test-auth-backend-link="${path}"]`,
createUser: '[data-test-entity-create-link="user"]',
input: (attr) => `[data-test-input="${attr}"]`,
password: '[data-test-textarea]',
saveBtn: '[data-test-save-config]',
methods: '[data-test-access-methods] a',
listItem: '[data-test-list-item-content]',
Expand Down Expand Up @@ -49,8 +47,8 @@ module('Acceptance | auth backend list', function (hooks) {
await click(SELECTORS.backendLink(this.path1));
assert.dom(GENERAL.emptyStateTitle).exists('shows empty state');
await click(SELECTORS.createUser);
await fillIn(SELECTORS.input('username'), this.user1);
await fillIn(SELECTORS.password, this.user1);
await fillIn(GENERAL.inputByAttr('username'), this.user1);
await fillIn(GENERAL.inputByAttr('password'), this.user1);
await click(SELECTORS.saveBtn);
assert.strictEqual(currentURL(), `/vault/access/${this.path1}/item/user`);

Expand All @@ -61,8 +59,8 @@ module('Acceptance | auth backend list', function (hooks) {
await click(SELECTORS.backendLink(this.path2));
assert.dom(GENERAL.emptyStateTitle).exists('shows empty state');
await click(SELECTORS.createUser);
await fillIn(SELECTORS.input('username'), this.user2);
await fillIn(SELECTORS.password, this.user2);
await fillIn(GENERAL.inputByAttr('username'), this.user2);
await fillIn(GENERAL.inputByAttr('password'), this.user2);
await click(SELECTORS.saveBtn);
assert.strictEqual(currentURL(), `/vault/access/${this.path2}/item/user`);
// Confirm that the user was created. There was a bug where the apiPath was not being updated when toggling between auth routes.
Expand Down
216 changes: 216 additions & 0 deletions ui/tests/acceptance/auth/enable-tune-form-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
/**
* Copyright (c) HashiCorp, Inc.
* SPDX-License-Identifier: BUSL-1.1
*/

import { module, test } from 'qunit';
import { setupApplicationTest } from 'ember-qunit';
import { v4 as uuidv4 } from 'uuid';

import { login } from 'vault/tests/helpers/auth/auth-helpers';
import { visit } from '@ember/test-helpers';
import { deleteAuthCmd, runCmd } from 'vault/tests/helpers/commands';
import testHelper from './test-helper';
import { GENERAL } from 'vault/tests/helpers/general-selectors';

// These models use openAPI so we assert the form inputs using an acceptance test
// The default selector is to use GENERAL.inputByAttr()
// custom fields should be added to the this.customSelectorss object
module('Acceptance | auth enable tune form test', function (hooks) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only added tests for azure, jwt, oidc, okta and ldap for now. We can expand as needed

setupApplicationTest(hooks);
hooks.beforeEach(async function () {
// these tend to be the same across models because they share the same mount-config model
// if necessary, they can be overridden in the individual module
this.mountFields = [
'path',
'description',
'local',
'sealWrap',
'config.listingVisibility',
'config.defaultLeaseTtl',
'config.maxLeaseTtl',
'config.tokenType',
'config.auditNonHmacRequestKeys',
'config.auditNonHmacResponseKeys',
'config.passthroughRequestHeaders',
'config.allowedResponseHeaders',
'config.pluginVersion',
];
});

module('azure', function (hooks) {
hooks.beforeEach(async function () {
this.type = 'azure';
this.path = `${this.type}-${uuidv4()}`;
this.tuneFields = [
'environment',
'identityTokenAudience',
'identityTokenTtl',
'maxRetries',
'maxRetryDelay',
'resource',
'retryDelay',
'rootPasswordTtl',
'tenantId',
];
this.tuneToggles = { 'Azure Options': ['clientId', 'clientSecret'] };
await login();
return visit('/vault/settings/auth/enable');
});
hooks.afterEach(async function () {
await runCmd(deleteAuthCmd(this.path), false);
});
testHelper(test);
});

module('jwt', function (hooks) {
hooks.beforeEach(async function () {
this.type = 'jwt';
this.path = `${this.type}-${uuidv4()}`;
this.customSelectors = {
providerConfig: `${GENERAL.fieldByAttr('providerConfig')} textarea`,
};
this.tuneFields = [
'defaultRole',
'jwksCaPem',
'jwksUrl',
'namespaceInState',
'oidcDiscoveryUrl',
'oidcResponseMode',
'oidcResponseTypes',
'providerConfig',
'unsupportedCriticalCertExtensions',
];
this.tuneToggles = {
'JWT Options': [
'oidcClientId',
'oidcClientSecret',
'oidcDiscoveryCaPem',
'jwtValidationPubkeys',
'jwtSupportedAlgs',
'boundIssuer',
],
};
await login();
return visit('/vault/settings/auth/enable');
});
hooks.afterEach(async function () {
await runCmd(deleteAuthCmd(this.path), false);
});
testHelper(test);
});

module('ldap', function (hooks) {
hooks.beforeEach(async function () {
this.type = 'ldap';
this.path = `${this.type}-${uuidv4()}`;
this.tuneFields = [
'url',
'caseSensitiveNames',
'connectionTimeout',
'dereferenceAliases',
'maxPageSize',
'passwordPolicy',
'requestTimeout',
'tokenBoundCidrs',
'tokenExplicitMaxTtl',
'tokenMaxTtl',
'tokenNoDefaultPolicy',
'tokenNumUses',
'tokenPeriod',
'tokenPolicies',
'tokenTtl',
'tokenType',
'usePre111GroupCnBehavior',
'usernameAsAlias',
];
this.tuneToggles = {
'LDAP Options': [
'starttls',
'insecureTls',
'discoverdn',
'denyNullBind',
'tlsMinVersion',
'tlsMaxVersion',
'certificate',
'clientTlsCert',
'clientTlsKey',
'userattr',
'upndomain',
'anonymousGroupSearch',
],
'Customize User Search': ['binddn', 'userdn', 'bindpass', 'userfilter'],
'Customize Group Membership Search': ['groupfilter', 'groupattr', 'groupdn', 'useTokenGroups'],
};
await login();
return visit('/vault/settings/auth/enable');
});
hooks.afterEach(async function () {
await runCmd(deleteAuthCmd(this.path), false);
});
testHelper(test);
});

module('oidc', function (hooks) {
hooks.beforeEach(async function () {
this.type = 'oidc';
this.path = `${this.type}-${uuidv4()}`;
this.customSelectors = {
providerConfig: `${GENERAL.fieldByAttr('providerConfig')} textarea`,
};
this.tuneFields = [
'oidcDiscoveryUrl',
'defaultRole',
'jwksCaPem',
'jwksUrl',
'oidcResponseMode',
'oidcResponseTypes',
'namespaceInState',
'providerConfig',
'unsupportedCriticalCertExtensions',
];
this.tuneToggles = {
'OIDC Options': [
'oidcClientId',
'oidcClientSecret',
'oidcDiscoveryCaPem',
'jwtValidationPubkeys',
'jwtSupportedAlgs',
'boundIssuer',
],
};
await login();
return visit('/vault/settings/auth/enable');
});
hooks.afterEach(async function () {
await runCmd(deleteAuthCmd(this.path), false);
});
testHelper(test);
});

module('okta', function (hooks) {
hooks.beforeEach(async function () {
this.type = 'okta';
this.path = `${this.type}-${uuidv4()}`;
this.tuneFields = [
'orgName',
'tokenBoundCidrs',
'tokenExplicitMaxTtl',
'tokenMaxTtl',
'tokenNoDefaultPolicy',
'tokenNumUses',
'tokenPeriod',
'tokenPolicies',
'tokenTtl',
'tokenType',
];
this.tuneToggles = { Options: ['apiToken', 'baseUrl', 'bypassOktaMfa'] };
await login();
return visit('/vault/settings/auth/enable');
});
hooks.afterEach(async function () {
await runCmd(deleteAuthCmd(this.path), false);
});
testHelper(test);
});
});
49 changes: 49 additions & 0 deletions ui/tests/acceptance/auth/test-helper.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* Copyright (c) HashiCorp, Inc.
* SPDX-License-Identifier: BUSL-1.1
*/

import { click, currentURL, fillIn } from '@ember/test-helpers';
import { GENERAL } from 'vault/tests/helpers/general-selectors';

const SELECTORS = {
mountType: (name) => `[data-test-mount-type="${name}"]`,
submit: '[data-test-mount-submit]',
};

const assertFields = (assert, fields, customSelectors = {}) => {
fields.forEach((param) => {
if (Object.keys(customSelectors).includes(param)) {
assert.dom(customSelectors[param]).exists();
} else {
assert.dom(GENERAL.inputByAttr(param)).exists();
}
});
};
export default (test) => {
test('it renders mount fields', async function (assert) {
await click(SELECTORS.mountType(this.type));
await click(GENERAL.toggleGroup('Method Options'));
assertFields(assert, this.mountFields, this.customSelectors);
});

test('it renders tune fields', async function (assert) {
// enable auth method to check tune fields
await click(SELECTORS.mountType(this.type));
await fillIn(GENERAL.inputByAttr('path'), this.path);
await click(SELECTORS.submit);
assert.strictEqual(
currentURL(),
`/vault/settings/auth/configure/${this.path}/configuration`,
`${this.type}: it mounts navigates to tune form`
);

assertFields(assert, this.tuneFields, this.customSelectors);

for (const toggle in this.tuneToggles) {
const fields = this.tuneToggles[toggle];
await click(GENERAL.toggleGroup(toggle));
assertFields(assert, fields, this.customSelectors);
}
});
};
14 changes: 4 additions & 10 deletions ui/tests/acceptance/mfa-method-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { setupMirage } from 'ember-cli-mirage/test-support';
import mfaConfigHandler from 'vault/mirage/handlers/mfa-config';
import { Response } from 'miragejs';
import { underscore } from '@ember/string';
import { GENERAL } from 'vault/tests/helpers/general-selectors';

module('Acceptance | mfa-method', function (hooks) {
setupApplicationTest(hooks);
Expand Down Expand Up @@ -181,17 +182,10 @@ module('Acceptance | mfa-method', function (hooks) {
.dom('[data-test-inline-error-message]')
.exists({ count: required.length }, `Required field validations display for ${type}`);

for (const [i, field] of required.entries()) {
let inputType = 'input';
// this is less than ideal but updating the test selectors in masked-input break a bunch of tests
// add value to the masked input text area data-test attributes for selection
if (['secret_key', 'integration_key'].includes(field)) {
inputType = 'textarea';
const textareas = this.element.querySelectorAll('[data-test-textarea]');
textareas[i].setAttribute('data-test-textarea', field);
}
await fillIn(`[data-test-${inputType}="${field}"]`, 'foo');
for (const field of required) {
await fillIn(GENERAL.inputByAttr(field), 'foo');
}

await click('[data-test-mfa-create-save]');
assert.strictEqual(
currentRouteName(),
Expand Down
8 changes: 4 additions & 4 deletions ui/tests/acceptance/reset-password-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,11 @@ module('Acceptance | reset password', function (hooks) {
);

assert.dom('[data-test-title]').hasText('Reset password', 'page title');
await fillIn('[data-test-textarea]', 'newpassword');
await fillIn('[data-test-input="reset-password"]', 'newpassword');
await click('[data-test-reset-password-save]');
await waitFor('[data-test-flash-message]');
assert.dom('[data-test-flash-message]').hasText(`Success ${SUCCESS_MESSAGE}`);
assert.dom('[data-test-textarea]').hasValue('', 'Resets input after save');
assert.dom('[data-test-input="reset-password"]').hasValue('', 'Resets input after save');
});

test('allows password reset for userpass users logged in via tab', async function (assert) {
Expand Down Expand Up @@ -91,10 +91,10 @@ module('Acceptance | reset password', function (hooks) {
);

assert.dom('[data-test-title]').hasText('Reset password', 'page title');
await fillIn('[data-test-textarea]', 'newpassword');
await fillIn('[data-test-input="reset-password"]', 'newpassword');
await click('[data-test-reset-password-save]');
await waitFor('[data-test-flash-message]');
assert.dom('[data-test-flash-message]').hasText(`Success ${SUCCESS_MESSAGE}`);
assert.dom('[data-test-textarea]').hasValue('', 'Resets input after save');
assert.dom('[data-test-input="reset-password"]').hasValue('', 'Resets input after save');
});
});
Loading
Loading