Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI/OIDC auth bug for cloud ui HCP namespace flag #16886

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/16886.txt
@@ -0,0 +1,3 @@
```release-note:bug
ui: Fix OIDC callback to accept namespace flag in different formats
hellobontempo marked this conversation as resolved.
Show resolved Hide resolved
```
21 changes: 5 additions & 16 deletions ui/app/routes/vault/cluster/oidc-callback.js
Expand Up @@ -6,28 +6,17 @@ export default Route.extend({
// left blank so we render the template immediately
},
afterModel() {
const queryString = decodeURIComponent(window.location.search);
// Since state param can also contain namespace, fetch the values using native url api.
// For instance, state params value can be state=st_123456,ns=d4fq
// Ember paramsFor used to strip out the value after the "=" sign. In short ns value was not being passed along.
hellobontempo marked this conversation as resolved.
Show resolved Hide resolved
let urlParams = new URLSearchParams(queryString);
let state = urlParams.get('state'),
code = urlParams.get('code'),
ns;
if (state.includes(',ns=')) {
let { auth_path: path, code, state } = this.paramsFor(this.routeName);
let { namespaceQueryParam: namespace } = this.paramsFor('vault.cluster');
// only replace namespace param from cluster if state has a namespace
if (state?.includes(',ns=')) {
let arrayParams = state.split(',ns=');
state = arrayParams[0];
ns = arrayParams[1];
namespace = arrayParams[1];
}
let { auth_path: path } = this.paramsFor(this.routeName);
let { namespaceQueryParam: namespace } = this.paramsFor('vault.cluster');
path = window.decodeURIComponent(path);
const source = 'oidc-callback'; // required by event listener in auth-jwt component
let queryParams = { source, namespace, path, code, state };
Copy link
Collaborator

@hashishaw hashishaw Aug 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more cleanup task (follow-on) is, if namespace doesn't exist don't include it. So remove from queryParams here and then below do
if (namespace) queryParams.namespace = namespace

// If state had ns value, send it as part of namespace param
if (ns) {
queryParams.namespace = ns;
}
window.opener.postMessage(queryParams, window.origin);
},
setupController(controller) {
Expand Down
111 changes: 111 additions & 0 deletions ui/tests/unit/routes/vault/cluster/oidc-callback-test.js
@@ -0,0 +1,111 @@
import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';
import sinon from 'sinon';

module('Unit | Route | vault/cluster/oidc-callback', function (hooks) {
setupTest(hooks);
const parentNs = 'admin';
const childNs = 'admin/child-ns';
const path = 'oidc';
const customPath = 'oidc-dev';
const code = 'lTazRXEwKfyGKBUCo5TyLJzdIt39YniBJOXPABiRMkL0T';
const state = (ns) => {
ns ? 'st_91ji6vR2sQ2zBiZSQkqJ' + `,ns=${ns}` : 'st_91ji6vR2sQ2zBiZSQkqJ';
};

hooks.beforeEach(function () {
hellobontempo marked this conversation as resolved.
Show resolved Hide resolved
this.router = this.owner.lookup('service:router');
this.route = this.owner.lookup('route:vault/cluster/oidc-callback');
this.windowStub = sinon.stub(window.opener, 'postMessage');
});

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: parentNs };
return {
auth_path: path,
state: state(childNs),
code,
};
};
assert.ok(this.windowStub.calledWith, 'test');
hellobontempo marked this conversation as resolved.
Show resolved Hide resolved
assert.propContains(
this.route.afterModel(),
{
path,
namespace: childNs,
state: state(),
},
'state and namespace queryParams are correct'
);
});

test('it uses namespace param from state not namespaceQueryParam from cluster with custom path', function (assert) {
this.routeName = 'vault.cluster.oidc-callback';
this.route.paramsFor = (path) => {
if (path === 'vault.cluster') return { namespaceQueryParam: parentNs };
return {
auth_path: customPath,
state: state(childNs),
code,
};
};
assert.propContains(
this.route.afterModel(),
{
path: customPath,
namespace: childNs,
state: state(),
},
'state ns takes precedence, state no longer has ns query'
);
});

test('it uses namespace from namespaceQueryParam when no ns param from state', function (assert) {
this.routeName = 'vault.cluster.oidc-callback';
this.route.paramsFor = (path) => {
if (path === 'vault.cluster') return { namespaceQueryParam: parentNs };
return {
auth_path: path,
state: state(),
code,
};
};
assert.propContains(
this.route.afterModel(),
{
path,
namespace: parentNs,
state: state(),
},
'namespace is from cluster namespaceQueryParam'
);
});

test('it uses ns param from state when no namespaceQueryParam from cluster', function (assert) {
hashishaw marked this conversation as resolved.
Show resolved Hide resolved
this.routeName = 'vault.cluster.oidc-callback';
this.route.paramsFor = (path) => {
if (path === 'vault.cluster') return { namespaceQueryParam: '' };
return {
auth_path: path,
state: state('ns1'),
code,
};
};
assert.propContains(
this.route.afterModel(),
{
path,
namespace: 'ns1',
state: state(),
},
'it strips ns from state and uses as namespace param'
);
});
});