diff --git a/ui/app/routes/vault/cluster/oidc-callback.js b/ui/app/routes/vault/cluster/oidc-callback.js index 61219c658d09..cd3185202b8b 100644 --- a/ui/app/routes/vault/cluster/oidc-callback.js +++ b/ui/app/routes/vault/cluster/oidc-callback.js @@ -6,20 +6,18 @@ export default Route.extend({ // left blank so we render the template immediately }, afterModel() { - let { auth_path: path, code, state } = this.paramsFor(this.routeName); + let { auth_path: path, code } = this.paramsFor(this.routeName); + // some SSO providers do not return a url-encoded state param + // parse state using URLSearchParams instead of paramsFor + const queryString = decodeURIComponent(window.location.search); + const urlParams = new URLSearchParams(queryString); + let state = urlParams.get('state'); + // check cluster for a namespace, i.e. HCP namespace flag 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 || '' }; 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 449801b21111..b805fdaaffeb 100644 --- a/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js +++ b/ui/tests/unit/routes/vault/cluster/oidc-callback-test.js @@ -12,58 +12,48 @@ module('Unit | Route | vault/cluster/oidc-callback', function (hooks) { }; this.route = this.owner.lookup('route:vault/cluster/oidc-callback'); this.windowStub = sinon.stub(window.opener, 'postMessage'); + this.state = 'st_yOarDguU848w5YZuotLs'; this.path = 'oidc'; this.code = 'lTazRXEwKfyGKBUCo5TyLJzdIt39YniBJOXPABiRMkL0T'; - this.state = (ns) => { - return ns ? 'st_91ji6vR2sQ2zBiZSQkqJ' + `,ns=${ns}` : 'st_91ji6vR2sQ2zBiZSQkqJ'; + this.route.paramsFor = (path) => { + if (path === 'vault.cluster') return { namespaceQueryParam: '' }; + return { + auth_path: this.path, + code: this.code, + }; }; - this.pushQueryParam = (queryString) => { - window.history.pushState({}, '', '?' + queryString); + this.callbackUrlQueryParams = (stateParam) => { + switch (stateParam) { + case '': + window.history.pushState({}, ''); + break; + case 'stateless': + window.history.pushState({}, '', '?' + `code=${this.code}`); + break; + default: + window.history.pushState({}, '', '?' + `code=${this.code}&state=${stateParam}`); + break; + } }; }); hooks.afterEach(function () { this.windowStub.restore(); window.opener = this.originalOpener; - this.pushQueryParam(''); + this.callbackUrlQueryParams(''); }); test('it calls route', function (assert) { assert.ok(this.route); }); - test('it uses namespace param from state not namespaceQueryParam from cluster with default path', function (assert) { - this.routeName = 'vault.cluster.oidc-callback'; - this.route.paramsFor = (path) => { - if (path === 'vault.cluster') return { namespaceQueryParam: 'admin' }; - return { - auth_path: this.path, - state: this.state('admin/child-ns'), - code: this.code, - }; - }; - this.route.afterModel(); - - assert.propEqual( - this.windowStub.lastCall.args[0], - { - code: 'lTazRXEwKfyGKBUCo5TyLJzdIt39YniBJOXPABiRMkL0T', - namespace: 'admin/child-ns', - path: 'oidc', - source: 'oidc-callback', - state: this.state(), - }, - 'namespace param is from state, ns=admin/child-ns' - ); - }); - - test('it uses namespace param from state not namespaceQueryParam from cluster with custom path', function (assert) { + test('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', - state: this.state('admin/child-ns'), code: this.code, }; }; @@ -73,21 +63,21 @@ module('Unit | Route | vault/cluster/oidc-callback', function (hooks) { { code: this.code, path: 'oidc-dev', - namespace: 'admin/child-ns', - state: this.state(), + namespace: 'test-ns', + state: this.state, source: 'oidc-callback', }, - 'state ns takes precedence, state no longer has ns query' + 'ns from state not cluster' ); }); - test(`it uses namespace from namespaceQueryParam when state does not include: ',ns=some-namespace'`, function (assert) { + test('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, - state: this.state(), code: this.code, }; }; @@ -98,44 +88,36 @@ module('Unit | Route | vault/cluster/oidc-callback', function (hooks) { code: this.code, path: this.path, namespace: 'admin', - state: this.state(), + state: this.state, source: 'oidc-callback', }, - 'namespace is from cluster namespaceQueryParam' + `namespace is from cluster's namespaceQueryParam` ); }); - test('it uses ns param from state when no namespaceQueryParam from cluster', function (assert) { - this.routeName = 'vault.cluster.oidc-callback'; - this.route.paramsFor = (path) => { - if (path === 'vault.cluster') return { namespaceQueryParam: '' }; - return { - auth_path: this.path, - state: this.state('ns1'), - code: this.code, - }; - }; + test('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: 'ns1', - state: this.state(), + namespace: 'parent-ns/child-ns', + state: this.state, source: 'oidc-callback', }, - 'it strips ns from state and uses as namespace param' + 'it has correct nested ns from state and sets as namespace param' ); }); test('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: '', - state: '', code: '', }; }; @@ -154,17 +136,12 @@ module('Unit | Route | vault/cluster/oidc-callback', function (hooks) { test('the afterModel hook returns when state param does not exist', function (assert) { this.routeName = 'vault.cluster.oidc-callback'; - this.route.paramsFor = (path) => { - if (path === 'vault.cluster') return { namespaceQueryParam: '' }; - return { - auth_path: this.path, - }; - }; + this.callbackUrlQueryParams('stateless'); this.route.afterModel(); assert.propEqual( this.windowStub.lastCall.args[0], { - code: '', + code: this.code, path: 'oidc', state: '', source: 'oidc-callback', @@ -173,13 +150,13 @@ module('Unit | Route | vault/cluster/oidc-callback', function (hooks) { ); }); - test('the afterModel hook returns when cluster namespaceQueryParam exists and all route params are empty strings', function (assert) { + test('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: '', - state: '', code: '', }; }; @@ -200,53 +177,18 @@ 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=st_EC8PbzZ7XUQ0ClEgssS9%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_gVRGT4TJe2RpvHNX5HV0,ns=admin' + '?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_91ji6vR2sQ2zBiZSQkqJ,ns' } + { state: 'st_yOarDguU848w5YZuotLs,ns' } */ - test('is parses the namespace from a uri with decoded state param', async function (assert) { - this.pushQueryParam(`?code=${this.code}&state=st_gVRGT4TJe2RpvHNX5HV0,ns=admin`); - this.routeName = 'vault.cluster.oidc-callback'; - this.route.paramsFor = (path) => { - if (path === 'vault.cluster') return { namespaceQueryParam: '' }; - return { - auth_path: this.path, - state: 'st_91ji6vR2sQ2zBiZSQkqJ,ns', - code: this.code, - }; - }; - - this.route.afterModel(); - assert.propEqual( - this.windowStub.lastCall.args[0], - { - code: this.code, - namespace: 'admin', - path: this.path, - source: 'oidc-callback', - state: 'st_gVRGT4TJe2RpvHNX5HV0', - }, - 'namespace is passed to window queryParams' - ); - }); - - test('is parses the namespace from a uri with encoded state param', async function (assert) { - this.pushQueryParam(`?code=${this.code}&state=st_EC8PbzZ7XUQ0ClEgssS9%2Cns%3Dadmin`); + test('it uses namespace when state param is not uri encoded', async function (assert) { this.routeName = 'vault.cluster.oidc-callback'; - this.route.paramsFor = (path) => { - if (path === 'vault.cluster') return { namespaceQueryParam: '' }; - return { - auth_path: this.path, - state: 'st_EC8PbzZ7XUQ0ClEgssS9,ns=admin', - code: this.code, - }; - }; - + this.callbackUrlQueryParams(`${this.state},ns=admin`); this.route.afterModel(); assert.propEqual( this.windowStub.lastCall.args[0], @@ -255,9 +197,9 @@ module('Unit | Route | vault/cluster/oidc-callback', function (hooks) { namespace: 'admin', path: this.path, source: 'oidc-callback', - state: 'st_EC8PbzZ7XUQ0ClEgssS9', + state: this.state, }, - 'namespace is passed to window queryParams' + 'namespace is parsed correctly' ); }); });