diff --git a/changelog/28873.txt b/changelog/28873.txt new file mode 100644 index 000000000000..f509c11bbc3b --- /dev/null +++ b/changelog/28873.txt @@ -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 +``` diff --git a/ui/app/components/auth-form.js b/ui/app/components/auth-form.js index d87161b29048..0eb61abbf138 100644 --- a/ui/app/components/auth-form.js +++ b/ui/app/components/auth-form.js @@ -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) { diff --git a/ui/app/components/auth-jwt.js b/ui/app/components/auth-jwt.js index 9cf587d585ae..6f69f223967a 100644 --- a/ui/app/components/auth-jwt.js +++ b/ui/app/components/auth-jwt.js @@ -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() { diff --git a/ui/app/components/auth-saml.js b/ui/app/components/auth-saml.js index 43c9de1dc252..52320f99a56c 100644 --- a/ui/app/components/auth-saml.js +++ b/ui/app/components/auth-saml.js @@ -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) { diff --git a/ui/app/components/auth/login-form.js b/ui/app/components/auth/login-form.js index e3551ce68dc0..49f298308df2 100644 --- a/ui/app/components/auth/login-form.js +++ b/ui/app/components/auth/login-form.js @@ -60,6 +60,24 @@ 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 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 component (which renders the Mfa::MfaForm). + + If doSubmit in is called directly (by the "default"
component) + mfa is handled directly by the parent 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, @@ -67,6 +85,7 @@ export default class AuthLoginFormComponent extends Component { selectedAuth, }); + // calls onAuthResponse in auth/page.js this.args.onSuccess(authResponse, backendType, data); } catch (e) { if (!this.auth.mfaError) { diff --git a/ui/app/components/auth/page.js b/ui/app/components/auth/page.js index 85dc29c20708..bb8e7c010828 100644 --- a/ui/app/components/auth/page.js +++ b/ui/app/components/auth/page.js @@ -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 is called directly (by the component) mfa is just handled here. + + Login methods submitted using a child form component of 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); } diff --git a/ui/app/components/mfa/mfa-form.js b/ui/app/components/mfa/mfa-form.js index c81ff42ff98d..b839f6d0ccce 100644 --- a/ui/app/components/mfa/mfa-form.js +++ b/ui/app/components/mfa/mfa-form.js @@ -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 || []; diff --git a/ui/app/services/auth.js b/ui/app/services/auth.js index 02fd52702bf8..8bc380ccd249 100644 --- a/ui/app/services/auth.js +++ b/ui/app/services/auth.js @@ -29,6 +29,7 @@ export default Service.extend({ permissions: service(), currentCluster: service(), router: service(), + store: service(), namespaceService: service('namespace'), IDLE_TIMEOUT: 3 * 60e3, @@ -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'; @@ -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 @@ -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, @@ -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); @@ -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); }, diff --git a/ui/tests/acceptance/oidc-auth-method-test.js b/ui/tests/acceptance/oidc-auth-method-test.js index 0dcaaffd2fe5..7e64acc1ad91 100644 --- a/ui/tests/acceptance/oidc-auth-method-test.js +++ b/ui/tests/acceptance/oidc-auth-method-test.js @@ -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); @@ -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'); + }); }); diff --git a/ui/tests/acceptance/saml-auth-method-test.js b/ui/tests/acceptance/saml-auth-method-test.js index c95ff37fee22..c87bcff279ce 100644 --- a/ui/tests/acceptance/saml-auth-method-test.js +++ b/ui/tests/acceptance/saml-auth-method-test.js @@ -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); @@ -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'); + }); }); diff --git a/ui/tests/helpers/auth/mfa-helpers.js b/ui/tests/helpers/auth/mfa-helpers.js new file mode 100644 index 000000000000..715cdff53dc6 --- /dev/null +++ b/ui/tests/helpers/auth/mfa-helpers.js @@ -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, + }, +}); diff --git a/ui/tests/integration/components/auth-jwt-test.js b/ui/tests/integration/components/auth-jwt-test.js index 028fa2b926f6..2f2415060ba1 100644 --- a/ui/tests/integration/components/auth-jwt-test.js +++ b/ui/tests/integration/components/auth-jwt-test.js @@ -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) {