Skip to content

Commit

Permalink
UI: Fix MFA + SSO login workflow (#28873)
Browse files Browse the repository at this point in the history
* =passback mfa_requirement for oidc login methods

* =pass SAML mfa requirement

* add comments

* add acceptance tests

* add helper

* update callback args for auth jwt

* add changelog

* update changelog

* is this line necessary?

* fetch token data for display name, this commit can be undone when BE fixes VAULT-32462

* change error handling, add comments

* update capitalization

* revert capitalization

* reword changelog

* clarify comments

* Update changelog/28873.txt
  • Loading branch information
hellobontempo authored Dec 17, 2024
1 parent ed46640 commit 741721a
Show file tree
Hide file tree
Showing 12 changed files with 142 additions and 16 deletions.
6 changes: 6 additions & 0 deletions changelog/28873.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
```release-note:bug
ui: Fixes login to web UI when MFA is enabled for OIDC (i.e. azure, auth0) and Okta auth methods
```
```release-note:bug
ui (enterprise): Fixes login to web UI when MFA is enabled for SAML auth methods
```
1 change: 1 addition & 0 deletions ui/app/components/auth-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ export default Component.extend(DEFAULTS, {
data.path = 'okta';
}
}
// calls performAuth in login-form.js which initiates the authenticate @task
return this.performAuth(backend.type, data);
},
handleError(e) {
Expand Down
4 changes: 3 additions & 1 deletion ui/app/components/auth-jwt.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,9 @@ export default Component.extend({
// and show the error on the login screen
return this.cancelLogin(oidcWindow, e);
}
yield this.onSubmit(null, null, resp.auth.client_token);
const { mfa_requirement, client_token } = resp.auth;
// onSubmit calls doSubmit in auth-form.js
yield this.onSubmit({ mfa_requirement }, null, client_token);
}),

async startOIDCAuth() {
Expand Down
6 changes: 4 additions & 2 deletions ui/app/components/auth-saml.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,10 @@ export default class AuthSaml extends Component {
if (!resp?.auth) {
continue;
}
// We've obtained the Vault token for the authentication flow, now log in.
yield this.args.onSubmit(null, null, resp.auth.client_token);
// We've obtained the Vault token for the authentication flow now log in or pass MFA data
const { mfa_requirement, client_token } = resp.auth;
// onSubmit calls doSubmit in auth-form.js
yield this.args.onSubmit({ mfa_requirement }, null, client_token);
this.closeWindow(samlWindow);
return;
} catch (e) {
Expand Down
19 changes: 19 additions & 0 deletions ui/app/components/auth/login-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,32 @@ export default class AuthLoginFormComponent extends Component {
} else {
this.delayAuthMessageReminder.perform();
}

/*
Checking for an mfa_requirement happens in two places.
Login methods submitted using a child of <AuthForm> have custom auth logic where mfa_requirements are collected, if any.
This mfa data is passed to their respective onSubmit callback functions, then intercepted here and passed up to
the parent <Auth::Page> component (which renders the Mfa::MfaForm).
If doSubmit in <AuthForm> is called directly (by the "default" <form> component)
mfa is handled directly by the parent <Auth::Page> component.
*/
if (data?.mfa_requirement) {
const parsedMfaAuthResponse = this.auth._parseMfaResponse(data.mfa_requirement);
// calls onAuthResponse in parent auth/page.js component
this.args.onSuccess(parsedMfaAuthResponse, backendType, data);
// return here because mfa-form.js will finish login/authentication flow after mfa validation
return;
}

const authResponse = yield this.auth.authenticate({
clusterId,
backend: backendType,
data,
selectedAuth,
});

// calls onAuthResponse in auth/page.js
this.args.onSuccess(authResponse, backendType, data);
} catch (e) {
if (!this.auth.mfaError) {
Expand Down
11 changes: 10 additions & 1 deletion ui/app/components/auth/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,25 @@ export default class AuthPage extends Component {
@action
onAuthResponse(authResponse, backend, data) {
const { mfa_requirement } = authResponse;
// if an mfa requirement exists further action is required
/*
Checking for an mfa_requirement happens in two places.
If doSubmit in <AuthForm> is called directly (by the <form> component) mfa is just handled here.
Login methods submitted using a child form component of <AuthForm> are first checked for mfa
in the Auth::LoginForm "authenticate" task, and then that data eventually bubbles up here.
*/
if (mfa_requirement) {
// if an mfa requirement exists further action is required
this.mfaAuthData = { mfa_requirement, backend, data };
} else {
// calls authSuccess in auth.js controller
this.args.onAuthSuccess(authResponse);
}
}

@action
onMfaSuccess(authResponse) {
// calls authSuccess in auth.js controller
this.args.onAuthSuccess(authResponse);
}

Expand Down
1 change: 1 addition & 0 deletions ui/app/components/mfa/mfa-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export default class MfaForm extends Component {
clusterId: this.args.clusterId,
...this.args.authData,
});
// calls onMfaSuccess in auth/page.js
this.args.onSuccess(response);
} catch (error) {
const errors = error.errors || [];
Expand Down
49 changes: 38 additions & 11 deletions ui/app/services/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export default Service.extend({
permissions: service(),
currentCluster: service(),
router: service(),
store: service(),
namespaceService: service('namespace'),

IDLE_TIMEOUT: 3 * 60e3,
Expand Down Expand Up @@ -210,6 +211,12 @@ export default Service.extend({
return this.ajax(url, 'POST', { namespace });
},

async lookupSelf(token) {
return this.store
.adapterFor('application')
.ajax('/v1/auth/token/lookup-self', 'GET', { headers: { 'X-Vault-Token': token } });
},

revokeCurrentToken() {
const namespace = this.authData.userRootNamespace;
const url = '/v1/auth/token/revoke-self';
Expand Down Expand Up @@ -255,7 +262,7 @@ export default Service.extend({
return userRootNamespace;
},

persistAuthData() {
async persistAuthData() {
const [firstArg, resp] = arguments;
const currentNamespace = this.namespaceService.path || '';
// dropdown vs tab format
Expand All @@ -275,18 +282,12 @@ export default Service.extend({
mountPath,
...BACKENDS.find((b) => b.type === backend),
};
let displayName;
if (isArray(currentBackend.displayNamePath)) {
displayName = currentBackend.displayNamePath.map((name) => get(resp, name)).join('/');
} else {
displayName = get(resp, currentBackend.displayNamePath);
}

const { entity_id, policies, renewable, namespace_path } = resp;
const userRootNamespace = this.calculateRootNamespace(currentNamespace, namespace_path, backend);
const data = {
userRootNamespace,
displayName,
displayName: null, // set below
backend: currentBackend,
token: resp.client_token || get(resp, currentBackend.tokenPath),
policies,
Expand All @@ -311,9 +312,7 @@ export default Service.extend({
// this is intentionally not included in setExpirationSettings so we can unit test that method
if (Ember.testing) this.set('allowExpiration', false);

if (!data.displayName) {
data.displayName = (this.getTokenData(tokenName) || {}).displayName;
}
data.displayName = await this.setDisplayName(resp, currentBackend.displayNamePath, tokenName);

this.set('tokens', addToArray(this.tokens, tokenName));
this.setTokenData(tokenName, data);
Expand All @@ -325,6 +324,34 @@ export default Service.extend({
});
},

async setDisplayName(resp, displayNamePath, tokenName) {
let displayName;

// first check if auth response includes a display name
displayName = isArray(displayNamePath)
? displayNamePath.map((name) => get(resp, name)).join('/')
: get(resp, displayNamePath);

// if not, check stored token data
if (!displayName) {
displayName = (this.getTokenData(tokenName) || {}).displayName;
}

// this is a workaround for OIDC/SAML methods WITH mfa configured. at this time mfa/validate endpoint does not
// return display_name (or metadata that includes it) for this auth combination.
// this if block can be removed if/when the API returns display_name on the mfa/validate response.
if (!displayName) {
// if still nothing, request token data as a last resort
try {
const { data } = await this.lookupSelf(resp.client_token);
displayName = data.display_name;
} catch {
// silently fail since we're just trying to set a display name
}
}
return displayName;
},

setTokenData(token, data) {
this.storage(token).setItem(token, data);
},
Expand Down
17 changes: 17 additions & 0 deletions ui/tests/acceptance/oidc-auth-method-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { fakeWindow, buildMessage } from '../helpers/oidc-window-stub';
import sinon from 'sinon';
import { later, _cancelTimers as cancelTimers } from '@ember/runloop';
import { Response } from 'miragejs';
import { setupTotpMfaResponse } from 'vault/tests/helpers/auth/mfa-helpers';

module('Acceptance | oidc auth method', function (hooks) {
setupApplicationTest(hooks);
Expand Down Expand Up @@ -155,4 +156,20 @@ module('Acceptance | oidc auth method', function (hooks) {
await click('[data-test-auth-submit]');
assert.dom('[data-test-message-error-description]').hasText('Error fetching role: permission denied');
});

test('it prompts mfa if configured', async function (assert) {
assert.expect(1);

this.setupMocks(assert);
this.server.get('/auth/foo/oidc/callback', () => setupTotpMfaResponse('foo'));
await this.selectMethod('oidc');
later(() => {
window.postMessage(buildMessage().data, window.origin);
cancelTimers();
}, 50);

await click('[data-test-auth-submit]');
await waitUntil(() => find('[data-test-mfa-form]'));
assert.dom('[data-test-mfa-form]').exists('it renders TOTP MFA form');
});
});
12 changes: 12 additions & 0 deletions ui/tests/acceptance/saml-auth-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 { Response } from 'miragejs';
import authPage from 'vault/tests/pages/auth';
import { fakeWindow } from 'vault/tests/helpers/oidc-window-stub';
import { setupTotpMfaResponse } from 'vault/tests/helpers/auth/mfa-helpers';

module('Acceptance | enterprise saml auth method', function (hooks) {
setupApplicationTest(hooks);
Expand Down Expand Up @@ -158,4 +159,15 @@ module('Acceptance | enterprise saml auth method', function (hooks) {
.dom('[data-test-select="auth-method"]')
.hasValue('saml', 'Previous auth method selected on logout');
});

test('it prompts mfa if configured', async function (assert) {
assert.expect(1);
this.server.put('/auth/saml/token', () => setupTotpMfaResponse('saml'));

await waitUntil(() => find('[data-test-select="auth-method"]'));
await fillIn('[data-test-select="auth-method"]', 'saml');
await click('[data-test-auth-submit]');
await waitUntil(() => find('[data-test-mfa-form]'));
assert.dom('[data-test-mfa-form]').exists('it renders TOTP MFA form');
});
});
24 changes: 24 additions & 0 deletions ui/tests/helpers/auth/mfa-helpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* Copyright (c) HashiCorp, Inc.
* SPDX-License-Identifier: BUSL-1.1
*/

export const setupTotpMfaResponse = (mountPath) => ({
auth: {
mfa_requirement: {
mfa_request_id: '0edf0945-da02-1300-9a0a-cb052cd94eb4',
mfa_constraints: {
[mountPath]: {
any: [
{
type: 'totp',
id: '7028db82-7de3-01d7-26b5-84b147c80966',
uses_passcode: true,
},
],
},
},
},
num_uses: 0,
},
});
8 changes: 7 additions & 1 deletion ui/tests/integration/components/auth-jwt-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,13 @@ module('Integration | Component | auth jwt', function (hooks) {
});
this.window.trigger('message', buildMessage());
await settled();
assert.ok(this.handler.withArgs(null, null, 'token').calledOnce, 'calls the onSubmit handler with token');
const [callbackData, , token] = this.handler.lastCall.args;
assert.propEqual(
callbackData,
{ mfa_requirement: undefined },
'mfa_requirement is undefined if not returned by response'
);
assert.strictEqual(token, 'token', 'calls the onSubmit handler with token');
});

test('oidc: fails silently when event origin does not match window origin', async function (assert) {
Expand Down

0 comments on commit 741721a

Please sign in to comment.