Skip to content

Commit

Permalink
remove state from paramsFor completely, update tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@hellobontempo
hellobontempo committed Mar 22, 2023
1 parent 34ec5b2 commit bea042f
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 113 deletions.
16 changes: 7 additions & 9 deletions ui/app/routes/vault/cluster/oidc-callback.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 || '' };
Expand Down
150 changes: 46 additions & 104 deletions ui/tests/unit/routes/vault/cluster/oidc-callback-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
};
Expand All @@ -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,
};
};
Expand All @@ -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: '',
};
};
Expand All @@ -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',
Expand All @@ -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: '',
};
};
Expand All @@ -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],
Expand All @@ -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'
);
});
});

0 comments on commit bea042f

Please sign in to comment.