UI/OIDC auth bug for hcp namespace flag (#16886)

* revert to using paramsFor but add check for state having ns=

* revert to using paramsFor but add check for state having ns=

* cleanup hook

* add tests

* add changelog

* Test troubleshooting

* cleanup tests, use window stub correctly!

* add test for state param not existing at all

Co-authored-by: hashishaw <cshaw@hashicorp.com>

changelog/16886.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Fix OIDC callback to accept namespace flag in different formats
+```

ui/app/routes/vault/cluster/oidc-callback.js

@@ -6,28 +6,15 @@ 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.
-    let urlParams = new URLSearchParams(queryString);
-    let state = urlParams.get('state'),
-      code = urlParams.get('code'),
-      ns;
-    if (state.includes(',ns=')) {
-      let arrayParams = state.split(',ns=');
-      state = arrayParams[0];
-      ns = arrayParams[1];
-    }
-    let { auth_path: path } = this.paramsFor(this.routeName);
+    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=')) {
+      [state, namespace] = state.split(',ns=');
+    }
     path = window.decodeURIComponent(path);
     const source = 'oidc-callback'; // required by event listener in auth-jwt component
     let queryParams = { source, namespace, path, code, state };
-    // 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) {

ui/tests/unit/routes/vault/cluster/oidc-callback-test.js

@@ -0,0 +1,184 @@
+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);
+
+  hooks.beforeEach(function () {
+    this.originalOpener = window.opener;
+    window.opener = {
+      postMessage: () => {},
+    };
+    this.route = this.owner.lookup('route:vault/cluster/oidc-callback');
+    this.windowStub = sinon.stub(window.opener, 'postMessage');
+    this.path = 'oidc';
+    this.code = 'lTazRXEwKfyGKBUCo5TyLJzdIt39YniBJOXPABiRMkL0T';
+    this.state = (ns) => {
+      return ns ? 'st_91ji6vR2sQ2zBiZSQkqJ' + `,ns=${ns}` : 'st_91ji6vR2sQ2zBiZSQkqJ';
+    };
+  });
+
+  hooks.afterEach(function () {
+    this.windowStub.restore();
+    window.opener = this.originalOpener;
+  });
+
+  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.ok(this.windowStub.calledOnce, 'it is called');
+    assert.propContains(
+      this.windowStub.lastCall.args[0],
+      {
+        code: 'lTazRXEwKfyGKBUCo5TyLJzdIt39YniBJOXPABiRMkL0T',
+        namespace: 'admin/child-ns',
+        path: 'oidc',
+      },
+      '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) {
+    this.routeName = 'vault.cluster.oidc-callback';
+    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,
+      };
+    };
+    this.route.afterModel();
+    assert.propContains(
+      this.windowStub.lastCall.args[0],
+      {
+        path: 'oidc-dev',
+        namespace: 'admin/child-ns',
+        state: this.state(),
+      },
+      'state ns takes precedence, state no longer has ns query'
+    );
+  });
+
+  test(`it uses namespace from namespaceQueryParam when state does not include: ',ns=some-namespace'`, 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(),
+        code: this.code,
+      };
+    };
+    this.route.afterModel();
+    assert.propContains(
+      this.windowStub.lastCall.args[0],
+      {
+        path: this.path,
+        namespace: 'admin',
+        state: this.state(),
+      },
+      'namespace is from cluster 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,
+      };
+    };
+    this.route.afterModel();
+    assert.propContains(
+      this.windowStub.lastCall.args[0],
+      {
+        path: this.path,
+        namespace: 'ns1',
+        state: this.state(),
+      },
+      'it strips ns from state and uses 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.route.paramsFor = (path) => {
+      if (path === 'vault.cluster') return { namespaceQueryParam: '' };
+      return {
+        auth_path: '',
+        state: '',
+        code: '',
+      };
+    };
+    this.route.afterModel();
+    assert.propContains(
+      this.windowStub.lastCall.args[0],
+      {
+        path: '',
+        state: '',
+        code: '',
+      },
+      'model hook returns with empty params'
+    );
+  });
+
+  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.route.afterModel();
+    assert.propContains(
+      this.windowStub.lastCall.args[0],
+      {
+        code: undefined,
+        path: 'oidc',
+        state: undefined,
+      },
+      'model hook returns non-existent state param'
+    );
+  });
+
+  test('the afterModel hook returns when cluster namespaceQueryParam exists and all route params are empty strings', function (assert) {
+    this.routeName = 'vault.cluster.oidc-callback';
+    this.route.paramsFor = (path) => {
+      if (path === 'vault.cluster') return { namespaceQueryParam: 'ns1' };
+      return {
+        auth_path: '',
+        state: '',
+        code: '',
+      };
+    };
+    this.route.afterModel();
+    assert.propContains(
+      this.windowStub.lastCall.args[0],
+      {
+        path: '',
+        state: '',
+        code: '',
+      },
+      'model hook returns with empty parameters'
+    );
+  });
+});