From 2c7b7ec3d7a9bd1e99878937e6dac152395a3b4b Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Thu, 23 Mar 2023 14:52:46 -0500 Subject: [PATCH 1/7] Strip out business logic of oidc-callback into testable method --- ui/app/routes/vault/cluster/oidc-callback.js | 49 +++++---- .../vault/cluster/oidc-callback-test.js | 101 +++++++++++++++++- 2 files changed, 127 insertions(+), 23 deletions(-) diff --git a/ui/app/routes/vault/cluster/oidc-callback.js b/ui/app/routes/vault/cluster/oidc-callback.js index a9eecc5e7ccb..2aea967739e5 100644 --- a/ui/app/routes/vault/cluster/oidc-callback.js +++ b/ui/app/routes/vault/cluster/oidc-callback.js @@ -5,33 +5,40 @@ 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=')) { + // TODO: does it ever get here, since Ember params strips out `=` and after? + [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 3f942f221456..ddcf0ec4fd3a 100644 --- a/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js +++ b/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js @@ -6,6 +6,7 @@ import { module, skip, 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); @@ -48,6 +49,102 @@ module('Unit | Route | vault/cluster/oidc-callback', function (hooks) { this.callbackUrlQueryParams(''); }); + module('getParamsForCallback helper fn', function () { + test('it parses params correctly with regular inputs no namespace', function (assert) { + const params = { + state: 'my-state', + code: 'my-code', + path: 'oidc-path', + }; + const results = getParamsForCallback(params, '?code=my-code&state=my-state'); + assert.deepEqual(results, { source: 'oidc-callback', ...params }); + }); + + 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/nested', + }; + const results = getParamsForCallback(params, '?code=my-code&state=my-state'); + assert.deepEqual(results, { source: 'oidc-callback', ...params }); + }); + + test('it parses params correctly regular inputs and namespace in state', function (assert) { + const queryString = '?code=my-code&state=my-state,ns=blah'; + const params = { + state: 'my-state,ns', // mock what Ember does with the state QP + code: 'my-code', + path: 'oidc-path', + }; + const results = getParamsForCallback(params, queryString); + assert.deepEqual(results, { source: 'oidc-callback', ...params, state: 'my-state', namespace: 'blah' }); + }); + + test('namespace in state takes precedence over namespace in route', function (assert) { + const queryString = '?code=my-code&state=my-state,ns=some-ns/foo'; + const params = { + state: 'my-state,ns', // mock what Ember does with the state QP + code: 'my-code', + path: 'oidc-path', + namespace: 'some-ns', + }; + const results = getParamsForCallback(params, queryString); + assert.deepEqual(results, { + source: 'oidc-callback', + ...params, + state: 'my-state', + namespace: 'some-ns/foo', + }); + }); + + test('correctly decodes path and state', function (assert) { + const searchString = '?code=my-code&state=spaces%20state,ns=some.ns%2Fchild'; + const params = { + state: 'spaces state,ns', + code: 'my-code', + path: 'oidc-path', + }; + const results = getParamsForCallback(params, searchString); + assert.deepEqual(results, { + source: 'oidc-callback', + state: 'spaces state', + code: 'my-code', + path: 'oidc-path', + namespace: 'some.ns/child', + }); + }); + + test('correctly decodes namespace in state', function (assert) { + const queryString = '?code=my-code&state=spaces%20state,ns=spaces%20ns'; + const params = { + state: 'spaces state,ns', // mock what Ember does with the state QP + code: 'my-code', + path: 'oidc-path', + }; + const results = getParamsForCallback(params, queryString); + assert.deepEqual(results, { + source: 'oidc-callback', + state: 'spaces state', + code: 'my-code', + path: 'oidc-path', + namespace: 'spaces ns', + }); + }); + + test('it parses params correctly when window.location.search is empty', function (assert) { + const params = { + state: 'my-state', + code: 'my-code', + path: 'oidc-path', + namespace: 'ns1', + }; + const results = getParamsForCallback(params, ''); + assert.deepEqual(results, { source: 'oidc-callback', ...params, state: 'my-state' }); + }); + }); + test('it calls route', function (assert) { assert.ok(this.route); }); @@ -182,11 +279,11 @@ module('Unit | Route | vault/cluster/oidc-callback', function (hooks) { /* 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' + '?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' } From 3550a13f74fa00a1f1844813d685294953cecbf5 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Thu, 23 Mar 2023 15:25:44 -0500 Subject: [PATCH 2/7] Update empty searchString test with values from live testing HCP --- .../routes/vault/cluster/oidc-callback-test.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) 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 ddcf0ec4fd3a..8ae406b96ec3 100644 --- a/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js +++ b/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js @@ -133,15 +133,21 @@ module('Unit | Route | vault/cluster/oidc-callback', function (hooks) { }); }); - test('it parses params correctly when window.location.search is empty', function (assert) { + test('it parses params correctly when window.location.search is empty (HCP scenario)', function (assert) { const params = { - state: 'my-state', + state: 'some-state,ns=admin/child-ns', code: 'my-code', + namespace: 'admin', path: 'oidc-path', - namespace: 'ns1', }; const results = getParamsForCallback(params, ''); - assert.deepEqual(results, { source: 'oidc-callback', ...params, state: 'my-state' }); + assert.deepEqual(results, { + source: 'oidc-callback', + code: 'my-code', + path: 'oidc-path', + state: 'some-state', + namespace: 'admin/child-ns', + }); }); }); From f6f1b9a1b409cd247af7e3c65d49f44431654123 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Thu, 23 Mar 2023 15:26:50 -0500 Subject: [PATCH 3/7] cleanup --- ui/app/routes/vault/cluster/oidc-callback.js | 1 - 1 file changed, 1 deletion(-) diff --git a/ui/app/routes/vault/cluster/oidc-callback.js b/ui/app/routes/vault/cluster/oidc-callback.js index 2aea967739e5..39c4f3d55fe3 100644 --- a/ui/app/routes/vault/cluster/oidc-callback.js +++ b/ui/app/routes/vault/cluster/oidc-callback.js @@ -10,7 +10,6 @@ export function getParamsForCallback(qp, searchString) { let { path, code, state, namespace } = qp; // namespace from state takes precedence over the cluster's ns if (state?.includes(',ns=')) { - // TODO: does it ever get here, since Ember params strips out `=` and after? [state, namespace] = state.split(',ns='); } // some SSO providers do not return a url-encoded state param From c03f14ff35397081da98531e218bc993ca82b9d4 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com> Date: Thu, 23 Mar 2023 15:38:41 -0500 Subject: [PATCH 4/7] update test name Co-authored-by: claire bontempo <68122737+hellobontempo@users.noreply.github.com> --- ui/tests/unit/routes/vault/cluster/oidc-callback-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 8ae406b96ec3..b6a980e7d9e7 100644 --- a/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js +++ b/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js @@ -71,7 +71,7 @@ module('Unit | Route | vault/cluster/oidc-callback', function (hooks) { assert.deepEqual(results, { source: 'oidc-callback', ...params }); }); - test('it parses params correctly regular inputs and namespace in state', function (assert) { + test('it parses params correctly regular inputs and unencoded namespace in state', function (assert) { const queryString = '?code=my-code&state=my-state,ns=blah'; const params = { state: 'my-state,ns', // mock what Ember does with the state QP From 4fe88c4f0fa26b429456e9a4453f4361be1df834 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Thu, 23 Mar 2023 16:06:12 -0500 Subject: [PATCH 5/7] Clarify test language --- .../vault/cluster/oidc-callback-test.js | 206 +++++++++--------- 1 file changed, 104 insertions(+), 102 deletions(-) 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 b6a980e7d9e7..fe5786c35a4a 100644 --- a/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js +++ b/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js @@ -49,108 +49,6 @@ module('Unit | Route | vault/cluster/oidc-callback', function (hooks) { this.callbackUrlQueryParams(''); }); - module('getParamsForCallback helper fn', function () { - test('it parses params correctly with regular inputs no namespace', function (assert) { - const params = { - state: 'my-state', - code: 'my-code', - path: 'oidc-path', - }; - const results = getParamsForCallback(params, '?code=my-code&state=my-state'); - assert.deepEqual(results, { source: 'oidc-callback', ...params }); - }); - - 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/nested', - }; - const results = getParamsForCallback(params, '?code=my-code&state=my-state'); - assert.deepEqual(results, { source: 'oidc-callback', ...params }); - }); - - test('it parses params correctly regular inputs and unencoded namespace in state', function (assert) { - const queryString = '?code=my-code&state=my-state,ns=blah'; - const params = { - state: 'my-state,ns', // mock what Ember does with the state QP - code: 'my-code', - path: 'oidc-path', - }; - const results = getParamsForCallback(params, queryString); - assert.deepEqual(results, { source: 'oidc-callback', ...params, state: 'my-state', namespace: 'blah' }); - }); - - test('namespace in state takes precedence over namespace in route', function (assert) { - const queryString = '?code=my-code&state=my-state,ns=some-ns/foo'; - const params = { - state: 'my-state,ns', // mock what Ember does with the state QP - code: 'my-code', - path: 'oidc-path', - namespace: 'some-ns', - }; - const results = getParamsForCallback(params, queryString); - assert.deepEqual(results, { - source: 'oidc-callback', - ...params, - state: 'my-state', - namespace: 'some-ns/foo', - }); - }); - - test('correctly decodes path and state', function (assert) { - const searchString = '?code=my-code&state=spaces%20state,ns=some.ns%2Fchild'; - const params = { - state: 'spaces state,ns', - code: 'my-code', - path: 'oidc-path', - }; - const results = getParamsForCallback(params, searchString); - assert.deepEqual(results, { - source: 'oidc-callback', - state: 'spaces state', - code: 'my-code', - path: 'oidc-path', - namespace: 'some.ns/child', - }); - }); - - test('correctly decodes namespace in state', function (assert) { - const queryString = '?code=my-code&state=spaces%20state,ns=spaces%20ns'; - const params = { - state: 'spaces state,ns', // mock what Ember does with the state QP - code: 'my-code', - path: 'oidc-path', - }; - const results = getParamsForCallback(params, queryString); - assert.deepEqual(results, { - source: 'oidc-callback', - state: 'spaces state', - code: 'my-code', - path: 'oidc-path', - namespace: 'spaces ns', - }); - }); - - 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', - path: 'oidc-path', - }; - const results = getParamsForCallback(params, ''); - assert.deepEqual(results, { - source: 'oidc-callback', - code: 'my-code', - path: 'oidc-path', - state: 'some-state', - namespace: 'admin/child-ns', - }); - }); - }); - test('it calls route', function (assert) { assert.ok(this.route); }); @@ -310,4 +208,108 @@ module('Unit | Route | vault/cluster/oidc-callback', function (hooks) { 'namespace is parsed correctly' ); }); + + 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', + }; + const searchString = `?code=my-code&state=my-state`; + const results = getParamsForCallback(qp, searchString); + assert.deepEqual(results, { source: 'oidc-callback', ...qp }); + }); + + 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', + }; + const results = getParamsForCallback(params, '?code=my-code&state=my-state&namespace=my-namespace'); + assert.deepEqual(results, { source: 'oidc-callback', ...params }); + }); + + 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', + path: 'oidc-path', + }; + const results = getParamsForCallback(params, ''); + assert.deepEqual(results, { + source: 'oidc-callback', + code: 'my-code', + path: 'oidc-path', + state: 'some-state', + namespace: 'admin/child-ns', + }); + }); + }); }); From bad457e5f3e8be5fb0abf44194a34479279cc27a Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Thu, 23 Mar 2023 16:16:30 -0500 Subject: [PATCH 6/7] Add test case for missing query params --- .../unit/routes/vault/cluster/oidc-callback-test.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) 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 fe5786c35a4a..a03619074a57 100644 --- a/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js +++ b/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js @@ -311,5 +311,18 @@ module('Unit | Route | vault/cluster/oidc-callback', function (hooks) { namespace: 'admin/child-ns', }); }); + + test('it defaults to reasonable values if query params are missing', function (assert) { + const params = { + path: 'oidc-path', + }; + const results = getParamsForCallback(params, ''); + assert.deepEqual(results, { + source: 'oidc-callback', + code: '', + path: 'oidc-path', + state: '', + }); + }); }); }); From 6e4647c289ad3e7a5610a188f96d9eb5cb6c2e3a Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Mon, 27 Mar 2023 12:56:15 -0500 Subject: [PATCH 7/7] remove skipped tests --- .../vault/cluster/oidc-callback-test.js | 170 ++---------------- 1 file changed, 13 insertions(+), 157 deletions(-) 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 a03619074a57..6d9b6e903b72 100644 --- a/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js +++ b/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js @@ -3,7 +3,7 @@ * 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'; @@ -53,162 +53,6 @@ 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, - }; - }; - 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' - ); - }); - - 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, - }; - }; - this.route.afterModel(); - assert.propEqual( - this.windowStub.lastCall.args[0], - { - code: this.code, - path: this.path, - 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: '', - }; - }; - this.route.afterModel(); - assert.propEqual( - this.windowStub.lastCall.args[0], - { - path: '', - state: '', - code: '', - source: 'oidc-callback', - }, - 'model hook returns with empty params' - ); - }); - - 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: '', - }; - }; - this.route.afterModel(); - assert.propEqual( - this.windowStub.lastCall.args[0], - { - code: '', - namespace: 'ns1', - path: '', - source: 'oidc-callback', - 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' - ); - }); - module('getParamsForCallback helper fn', function () { test('it parses params correctly with regular inputs and no namespace', function (assert) { const qp = { @@ -232,6 +76,18 @@ module('Unit | Route | vault/cluster/oidc-callback', function (hooks) { 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 = {