From ea99048639bc808dc53982071198ab3f670c253e Mon Sep 17 00:00:00 2001 From: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com> Date: Mon, 27 Mar 2023 15:05:26 -0500 Subject: [PATCH] UI: Test business logic for oidc callback params (#19727) Co-authored-by: claire bontempo <68122737+hellobontempo@users.noreply.github.com> --- ui/app/routes/vault/cluster/oidc-callback.js | 48 ++-- .../vault/cluster/oidc-callback-test.js | 264 ++++++++---------- 2 files changed, 146 insertions(+), 166 deletions(-) diff --git a/ui/app/routes/vault/cluster/oidc-callback.js b/ui/app/routes/vault/cluster/oidc-callback.js index a9eecc5e7..39c4f3d55 100644 --- a/ui/app/routes/vault/cluster/oidc-callback.js +++ b/ui/app/routes/vault/cluster/oidc-callback.js @@ -5,33 +5,39 @@ import Route from '@ember/routing/route'; +export function getParamsForCallback(qp, searchString) { + const queryString = decodeURIComponent(searchString); + let { path, code, state, namespace } = qp; + // namespace from state takes precedence over the cluster's ns + if (state?.includes(',ns=')) { + [state, namespace] = state.split(',ns='); + } + // some SSO providers do not return a url-encoded state param + // check for namespace using URLSearchParams instead of paramsFor + const urlParams = new URLSearchParams(queryString); + const checkState = urlParams.get('state'); + if (checkState?.includes(',ns=')) { + [state, namespace] = checkState.split(',ns='); + } + path = window.decodeURIComponent(path); + const payload = { source: 'oidc-callback', path: path || '', code: code || '', state: state || '' }; + if (namespace) { + payload.namespace = namespace; + } + return payload; +} + export default Route.extend({ templateName: 'vault/cluster/oidc-callback', model() { // left blank so we render the template immediately }, afterModel() { - let { auth_path: path, code, state } = this.paramsFor(this.routeName); - let { namespaceQueryParam: namespace } = this.paramsFor('vault.cluster'); - // namespace from state takes precedence over the cluster's ns - if (state?.includes(',ns=')) { - [state, namespace] = state.split(',ns='); - } - // some SSO providers do not return a url-encoded state param - // check for namespace using URLSearchParams instead of paramsFor - const queryString = decodeURIComponent(window.location.search); - const urlParams = new URLSearchParams(queryString); - const checkState = urlParams.get('state'); - if (checkState?.includes(',ns=')) { - [state, namespace] = checkState.split(',ns='); - } - path = window.decodeURIComponent(path); - const source = 'oidc-callback'; // required by event listener in auth-jwt component - const queryParams = { source, path: path || '', code: code || '', state: state || '' }; - if (namespace) { - queryParams.namespace = namespace; - } - window.opener.postMessage(queryParams, window.origin); + const { auth_path: path, code, state } = this.paramsFor(this.routeName); + const { namespaceQueryParam: namespace } = this.paramsFor('vault.cluster'); + const queryString = window.location.search; + const payload = getParamsForCallback({ path, code, state, namespace }, queryString); + window.opener.postMessage(payload, window.origin); }, setupController(controller) { this._super(...arguments); diff --git a/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js b/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js index 3f942f221..6d9b6e903 100644 --- a/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js +++ b/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js @@ -3,9 +3,10 @@ * SPDX-License-Identifier: MPL-2.0 */ -import { module, skip, test } from 'qunit'; +import { module, test } from 'qunit'; import { setupTest } from 'ember-qunit'; import sinon from 'sinon'; +import { getParamsForCallback } from 'vault/routes/vault/cluster/oidc-callback'; module('Unit | Route | vault/cluster/oidc-callback', function (hooks) { setupTest(hooks); @@ -52,159 +53,132 @@ module('Unit | Route | vault/cluster/oidc-callback', function (hooks) { assert.ok(this.route); }); - skip('it uses namespace param from state instead of cluster, with custom oidc path', function (assert) { - this.routeName = 'vault.cluster.oidc-callback'; - this.callbackUrlQueryParams(encodeURIComponent(`${this.state},ns=test-ns`)); - this.route.paramsFor = (path) => { - if (path === 'vault.cluster') return { namespaceQueryParam: 'admin' }; - return { - auth_path: 'oidc-dev', - code: this.code, + module('getParamsForCallback helper fn', function () { + test('it parses params correctly with regular inputs and no namespace', function (assert) { + const qp = { + state: 'my-state', + code: 'my-code', + path: 'oidc-path', }; - }; - this.route.afterModel(); - assert.propEqual( - this.windowStub.lastCall.args[0], - { - code: this.code, - path: 'oidc-dev', - namespace: 'test-ns', - state: this.state, - source: 'oidc-callback', - }, - 'ns from state not cluster' - ); - }); + const searchString = `?code=my-code&state=my-state`; + const results = getParamsForCallback(qp, searchString); + assert.deepEqual(results, { source: 'oidc-callback', ...qp }); + }); - skip('it uses namespace from cluster when state does not include ns param', function (assert) { - this.routeName = 'vault.cluster.oidc-callback'; - this.callbackUrlQueryParams(encodeURIComponent(this.state)); - this.route.paramsFor = (path) => { - if (path === 'vault.cluster') return { namespaceQueryParam: 'admin' }; - return { - auth_path: this.path, - code: this.code, + test('it parses params correctly regular inputs and namespace param', function (assert) { + const params = { + state: 'my-state', + code: 'my-code', + path: 'oidc-path', + namespace: 'my-namespace', }; - }; - this.route.afterModel(); - assert.propEqual( - this.windowStub.lastCall.args[0], - { - code: this.code, - path: this.path, + const results = getParamsForCallback(params, '?code=my-code&state=my-state&namespace=my-namespace'); + assert.deepEqual(results, { source: 'oidc-callback', ...params }); + }); + + /* + If authenticating to a namespace, most SSO providers return a callback url + with a 'state' query param that includes a URI encoded namespace, example: + '?code=BZBDVPMz0By2JTqulEMWX5-6rflW3A20UAusJYHEeFygJ&state=sst_yOarDguU848w5YZuotLs%2Cns%3Dadmin' + + Active Directory Federation Service (AD FS), instead, decodes the namespace portion: + '?code=BZBDVPMz0By2JTqulEMWX5-6rflW3A20UAusJYHEeFygJ&state=st_yOarDguU848w5YZuotLs,ns=admin' + + 'ns' isn't recognized as a separate param because there is no ampersand, so using this.paramsFor() returns + a namespace-less state and authentication fails + { state: 'st_yOarDguU848w5YZuotLs,ns' } + */ + test('it parses params correctly with regular inputs and namespace in state (unencoded)', function (assert) { + const searchString = '?code=my-code&state=my-state,ns=foo/bar'; + const params = { + state: 'my-state,ns', // Ember parses the QP incorrectly if unencoded + code: 'my-code', + path: 'oidc-path', + }; + const results = getParamsForCallback(params, searchString); + assert.deepEqual(results, { + source: 'oidc-callback', + ...params, + state: 'my-state', + namespace: 'foo/bar', + }); + }); + + test('it parses params correctly with regular inputs and namespace in state (encoded)', function (assert) { + const qp = { + state: 'my-state,ns=foo/bar', // Ember parses the QP correctly when encoded + code: 'my-code', + path: 'oidc-path', + }; + const searchString = `?code=my-code&state=${encodeURIComponent(qp.state)}`; + const results = getParamsForCallback(qp, searchString); + assert.deepEqual(results, { source: 'oidc-callback', ...qp, state: 'my-state', namespace: 'foo/bar' }); + }); + + test('namespace in state takes precedence over namespace in route (encoded)', function (assert) { + const qp = { + state: 'my-state,ns=foo/bar', + code: 'my-code', + path: 'oidc-path', + namespace: 'other/ns', + }; + const searchString = `?code=my-code&state=${encodeURIComponent( + qp.state + )}&namespace=${encodeURIComponent(qp.namespace)}`; + const results = getParamsForCallback(qp, searchString); + assert.deepEqual(results, { + source: 'oidc-callback', + ...qp, + state: 'my-state', + namespace: 'foo/bar', + }); + }); + + test('namespace in state takes precedence over namespace in route (unencoded)', function (assert) { + const qp = { + state: 'my-state,ns', + code: 'my-code', + path: 'oidc-path', + namespace: 'other/ns', + }; + const searchString = `?code=${qp.code}&state=my-state,ns=foo/bar&namespace=${qp.namespace}`; + const results = getParamsForCallback(qp, searchString); + assert.deepEqual(results, { + source: 'oidc-callback', + ...qp, + state: 'my-state', + namespace: 'foo/bar', + }); + }); + + test('it parses params correctly when window.location.search is empty (HCP scenario)', function (assert) { + const params = { + state: 'some-state,ns=admin/child-ns', + code: 'my-code', namespace: 'admin', - state: this.state, - source: 'oidc-callback', - }, - `namespace is from cluster's namespaceQueryParam` - ); - }); - - skip('it correctly parses encoded, nested ns param from state', function (assert) { - this.callbackUrlQueryParams(encodeURIComponent(`${this.state},ns=parent-ns/child-ns`)); - this.route.afterModel(); - assert.propEqual( - this.windowStub.lastCall.args[0], - { - code: this.code, - path: this.path, - namespace: 'parent-ns/child-ns', - state: this.state, - source: 'oidc-callback', - }, - 'it has correct nested ns from state and sets as namespace param' - ); - }); - - skip('the afterModel hook returns when both cluster and route params are empty strings', function (assert) { - this.routeName = 'vault.cluster.oidc-callback'; - this.callbackUrlQueryParams(''); - this.route.paramsFor = (path) => { - if (path === 'vault.cluster') return { namespaceQueryParam: '' }; - return { - auth_path: '', - code: '', + path: 'oidc-path', }; - }; - this.route.afterModel(); - assert.propEqual( - this.windowStub.lastCall.args[0], - { - path: '', - state: '', - code: '', + const results = getParamsForCallback(params, ''); + assert.deepEqual(results, { source: 'oidc-callback', - }, - 'model hook returns with empty params' - ); - }); + code: 'my-code', + path: 'oidc-path', + state: 'some-state', + namespace: 'admin/child-ns', + }); + }); - skip('the afterModel hook returns when state param does not exist', function (assert) { - this.routeName = 'vault.cluster.oidc-callback'; - this.callbackUrlQueryParams('stateless'); - this.route.afterModel(); - assert.propEqual( - this.windowStub.lastCall.args[0], - { - code: this.code, - path: 'oidc', - state: '', - source: 'oidc-callback', - }, - 'model hook returns empty string when state param nonexistent' - ); - }); - - skip('the afterModel hook returns when cluster ns exists and all route params are empty strings', function (assert) { - this.routeName = 'vault.cluster.oidc-callback'; - this.callbackUrlQueryParams(''); - this.route.paramsFor = (path) => { - if (path === 'vault.cluster') return { namespaceQueryParam: 'ns1' }; - return { - auth_path: '', - code: '', + test('it defaults to reasonable values if query params are missing', function (assert) { + const params = { + path: 'oidc-path', }; - }; - this.route.afterModel(); - assert.propEqual( - this.windowStub.lastCall.args[0], - { + const results = getParamsForCallback(params, ''); + assert.deepEqual(results, { + source: 'oidc-callback', code: '', - namespace: 'ns1', - path: '', - source: 'oidc-callback', + path: 'oidc-path', state: '', - }, - 'model hook returns with empty parameters' - ); - }); - - /* - If authenticating to a namespace, most SSO providers return a callback url - with a 'state' query param that includes a URI encoded namespace, example: - '?code=BZBDVPMz0By2JTqulEMWX5-6rflW3A20UAusJYHEeFygJ&state=sst_yOarDguU848w5YZuotLs%2Cns%3Dadmin' - - Active Directory Federation Service (AD FS), instead, decodes the namespace portion: - '?code=BZBDVPMz0By2JTqulEMWX5-6rflW3A20UAusJYHEeFygJ&state=st_yOarDguU848w5YZuotLs,ns=admin' - - 'ns' isn't recognized as a separate param because there is no ampersand, so using this.paramsFor() returns - a namespace-less state and authentication fails - { state: 'st_yOarDguU848w5YZuotLs,ns' } - */ - skip('it uses namespace when state param is not uri encoded', async function (assert) { - this.routeName = 'vault.cluster.oidc-callback'; - this.callbackUrlQueryParams(`${this.state},ns=admin`); - this.route.afterModel(); - assert.propEqual( - this.windowStub.lastCall.args[0], - { - code: this.code, - namespace: 'admin', - path: this.path, - source: 'oidc-callback', - state: this.state, - }, - 'namespace is parsed correctly' - ); + }); + }); }); });