Skip to content

Commit

Permalink
Backport 1.13.x: UI/update auth form to fetchRoles after a namespace …
Browse files Browse the repository at this point in the history
…is inputted, prior to OIDC auth hashicorp#19541 (hashicorp#19661)

* UI/update auth form to fetchRoles after a namespace is inputted, prior to OIDC auth (hashicorp#19541)

* re-fetch roles if there is a namespace

* remove redundant conditional

* reorder oidc auth operations

* add test

* test cleanup

* add changelog

* UI: fix enterprise test failures (hashicorp#19671)

* move oidc tests into new file

* remove module from namespace test

* remove entered line

* add logout to afterEach hook

* remove ns test

* move test setup to within test

* use logout.visit() instead

* updates oidc auth namespaces test

* reverts to authPage logout

---------

Co-authored-by: Jordan Reimer <zofskeez@gmail.com>

---------

Co-authored-by: Jordan Reimer <zofskeez@gmail.com>
  • Loading branch information
hellobontempo and zofskeez authored Mar 23, 2023
1 parent 9ab8152 commit 996dc56
Show file tree
Hide file tree
Showing 7 changed files with 129 additions and 23 deletions.
3 changes: 3 additions & 0 deletions changelog/19541.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
ui: fixes oidc tabs in auth form submitting with the root's default_role value after a namespace has been inputted
```
18 changes: 9 additions & 9 deletions ui/app/components/auth-jwt.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export default Component.extend({

let { namespace, path, state, code } = oidcState;

// The namespace can be either be passed as a query paramter, or be embedded
// The namespace can be either be passed as a query parameter, or be embedded
// in the state param in the format `<state_id>,ns=<namespace>`. So if
// `namespace` is empty, check for namespace in state as well.
if (namespace === '' || this.featureFlagService.managedNamespaceRoot) {
Expand Down Expand Up @@ -171,6 +171,14 @@ export default Component.extend({
if (e && e.preventDefault) {
e.preventDefault();
}
try {
await this.fetchRole.perform(this.roleName, { debounce: false });
} catch (error) {
// this task could be cancelled if the instances in didReceiveAttrs resolve after this was started
if (error?.name !== 'TaskCancelation') {
throw error;
}
}
if (!this.isOIDC || !this.role || !this.role.authUrl) {
let message = this.errorMessage;
if (!this.role) {
Expand All @@ -182,14 +190,6 @@ export default Component.extend({
this.onError(message);
return;
}
try {
await this.fetchRole.perform(this.roleName, { debounce: false });
} catch (error) {
// this task could be cancelled if the instances in didReceiveAttrs resolve after this was started
if (error?.name !== 'TaskCancelation') {
throw error;
}
}
const win = this.getWindow();

const POPUP_WIDTH = 500;
Expand Down
24 changes: 11 additions & 13 deletions ui/app/templates/components/auth-form.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"is-active"
""
}}
data-test-auth-method
data-test-auth-method={{method.id}}
>
<LinkTo
@route="vault.cluster.auth"
Expand All @@ -32,18 +32,16 @@
</li>
{{/let}}
{{/each}}
{{#if this.hasMethodsWithPath}}
<li class={{unless this.selectedAuthIsPath "is-active" ""}} data-test-auth-method>
<LinkTo
@route="vault.cluster.auth"
@model={{this.cluster.name}}
@query={{hash with="token"}}
data-test-auth-method-link="other"
>
Other
</LinkTo>
</li>
{{/if}}
<li class={{unless this.selectedAuthIsPath "is-active" ""}} data-test-auth-method>
<LinkTo
@route="vault.cluster.auth"
@model={{this.cluster.name}}
@query={{hash with="token"}}
data-test-auth-method-link="other"
>
Other
</LinkTo>
</li>
</ul>
</nav>
{{/if}}
Expand Down
1 change: 1 addition & 0 deletions ui/app/templates/vault/cluster/auth.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
<div class="field">
<div class="control">
<input
data-test-auth-form-ns-input
value={{this.namespaceQueryParam}}
placeholder="/ (Root)"
oninput={{perform this.updateNamespace value="target.value"}}
Expand Down
1 change: 0 additions & 1 deletion ui/tests/acceptance/enterprise-namespaces-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { click, settled, visit, fillIn, currentURL } from '@ember/test-helpers';
import { module, test } from 'qunit';
import { setupApplicationTest } from 'ember-qunit';
import { create } from 'ember-cli-page-object';

import consoleClass from 'vault/tests/pages/components/console/ui-panel';
import authPage from 'vault/tests/pages/auth';
import logout from 'vault/tests/pages/logout';
Expand Down
91 changes: 91 additions & 0 deletions ui/tests/acceptance/enterprise-oidc-namespace-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { visit, currentURL } from '@ember/test-helpers';
import { module, test } from 'qunit';
import { setupApplicationTest } from 'ember-qunit';
import { create } from 'ember-cli-page-object';
import { setupMirage } from 'ember-cli-mirage/test-support';
import parseURL from 'core/utils/parse-url';
import consoleClass from 'vault/tests/pages/components/console/ui-panel';
import authPage from 'vault/tests/pages/auth';

const shell = create(consoleClass);

const createNS = async (name) => {
await shell.runCommands(`write sys/namespaces/${name} -force`);
};
const SELECTORS = {
authTab: (path) => `[data-test-auth-method="${path}"] a`,
};

module('Acceptance | Enterprise | oidc auth namespace test', function (hooks) {
setupApplicationTest(hooks);
setupMirage(hooks);

hooks.beforeEach(async function () {
this.namespace = 'test-ns';
this.rootOidc = 'root-oidc';
this.nsOidc = 'ns-oidc';

this.server.post(`/auth/:path/config`, () => {});

this.enableOidc = (path, role = '') => {
return shell.runCommands([
`write sys/auth/${path} type=oidc`,
`write auth/${path}/config default_role="${role}" oidc_discovery_url="https://example.com"`,
// show method as tab
`write sys/auth/${path}/tune listing_visibility="unauth"`,
]);
};

this.disableOidc = (path) => shell.runCommands([`delete /sys/auth/${path}`]);
});

test('oidc: request is made to auth_url when a namespace is inputted', async function (assert) {
assert.expect(5);

this.server.post(`/auth/${this.rootOidc}/oidc/auth_url`, (schema, req) => {
const { redirect_uri } = JSON.parse(req.requestBody);
const { pathname, search } = parseURL(redirect_uri);
assert.strictEqual(
pathname + search,
`/ui/vault/auth/${this.rootOidc}/oidc/callback`,
'request made to auth_url when the login page is visited'
);
});
this.server.post(`/auth/${this.nsOidc}/oidc/auth_url`, (schema, req) => {
const { redirect_uri } = JSON.parse(req.requestBody);
const { pathname, search } = parseURL(redirect_uri);
assert.strictEqual(
pathname + search,
`/ui/vault/auth/${this.nsOidc}/oidc/callback?namespace=${this.namespace}`,
'request made to correct auth_url when namespace is filled in'
);
});

await authPage.login();
// enable oidc in root namespace, without default role
await this.enableOidc(this.rootOidc);
// create child namespace to enable oidc
await createNS(this.namespace);
// enable oidc in child namespace with default role
await authPage.loginNs(this.namespace);
await this.enableOidc(this.nsOidc, `${this.nsOidc}-role`);
await authPage.logout();

await visit('/vault/auth');
assert.dom(SELECTORS.authTab(this.rootOidc)).exists('renders oidc method tab for root');
await authPage.namespaceInput(this.namespace);
assert.strictEqual(
currentURL(),
`/vault/auth?namespace=${this.namespace}&with=${this.nsOidc}%2F`,
'url updates with namespace value'
);
assert.dom(SELECTORS.authTab(this.nsOidc)).exists('renders oidc method tab for child namespace');

// disable methods to cleanup test state for re-running
await authPage.login();
await this.disableOidc(this.rootOidc);
await this.disableOidc(this.nsOidc);
await shell.runCommands([`delete /sys/auth/${this.namespace}`]);
await authPage.logout();
});
});
14 changes: 14 additions & 0 deletions ui/tests/pages/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export default create({
tokenInput: fillable('[data-test-token]'),
usernameInput: fillable('[data-test-username]'),
passwordInput: fillable('[data-test-password]'),
namespaceInput: fillable('[data-test-auth-form-ns-input]'),
login: async function (token) {
// make sure we're always logged out and logged back in
await this.logout();
Expand Down Expand Up @@ -39,4 +40,17 @@ export default create({
await this.passwordInput(password).submit();
return;
},
loginNs: async function (ns) {
// make sure we're always logged out and logged back in
await this.logout();
await settled();
// clear session storage to ensure we have a clean state
window.localStorage.clear();
await this.visit({ with: 'token' });
await settled();
await this.namespaceInput(ns);
await settled();
await this.tokenInput(rootToken).submit();
return;
},
});

0 comments on commit 996dc56

Please sign in to comment.