OIDC Logout Bug (#14545)

* fixes issue with token auth selected after logging out from oidc or jwt methods * adds changelog entry * reverts backendType var name change in auth-form authenticate method
hashicorp · Mar 18, 2022 · d8128ea · d8128ea
1 parent 3fdb221
commit d8128ea
Show file tree

Hide file tree

Showing 8 changed files with 119 additions and 75 deletions.
diff --git a/changelog/14545.txt b/changelog/14545.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Fixes issue with correct auth method not selected when logging out from OIDC or JWT methods
+```
diff --git a/ui/app/components/auth-form.js b/ui/app/components/auth-form.js
@@ -111,6 +111,18 @@ export default Component.extend(DEFAULTS, {
     this.setProperties(DEFAULTS);
   },
 
+  getAuthBackend(type) {
+    const { wrappedToken, methods, selectedAuth, selectedAuthIsPath: keyIsPath } = this;
+    const selected = type || selectedAuth;
+    if (!methods && !wrappedToken) {
+      return {};
+    }
+    if (keyIsPath) {
+      return methods.findBy('path', selected);
+    }
+    return BACKENDS.findBy('type', selected);
+  },
+
   selectedAuthIsPath: match('selectedAuth', /\/$/),
   selectedAuthBackend: computed(
     'wrappedToken',
@@ -119,14 +131,7 @@ export default Component.extend(DEFAULTS, {
     'selectedAuth',
     'selectedAuthIsPath',
     function () {
-      let { wrappedToken, methods, selectedAuth, selectedAuthIsPath: keyIsPath } = this;
-      if (!methods && !wrappedToken) {
-        return {};
-      }
-      if (keyIsPath) {
-        return methods.findBy('path', selectedAuth);
-      }
-      return BACKENDS.findBy('type', selectedAuth);
+      return this.getAuthBackend();
     }
   ),
 
@@ -208,10 +213,18 @@ export default Component.extend(DEFAULTS, {
 
   authenticate: task(
     waitFor(function* (backendType, data) {
-      let clusterId = this.cluster.id;
+      const {
+        selectedAuth,
+        cluster: { id: clusterId },
+      } = this;
       try {
         this.delayAuthMessageReminder.perform();
-        const authResponse = yield this.auth.authenticate({ clusterId, backend: backendType, data });
+        const authResponse = yield this.auth.authenticate({
+          clusterId,
+          backend: backendType,
+          data,
+          selectedAuth,
+        });
         this.onSuccess(authResponse, backendType, data);
       } catch (e) {
         this.set('loading', false);
@@ -246,7 +259,10 @@ export default Component.extend(DEFAULTS, {
       this.setProperties({
         error: null,
       });
-      let backend = this.selectedAuthBackend || {};
+      // if callback from oidc or jwt we have a token at this point
+      let backend = ['oidc', 'jwt'].includes(this.selectedAuth)
+        ? this.getAuthBackend('token')
+        : this.selectedAuthBackend || {};
       let backendMeta = BACKENDS.find(
         (b) => (b.type || '').toLowerCase() === (backend.type || '').toLowerCase()
       );

diff --git a/ui/app/components/auth-jwt.js b/ui/app/components/auth-jwt.js
@@ -165,7 +165,6 @@ export default Component.extend({
       return this.handleOIDCError(e);
     }
     let token = resp.auth.client_token;
-    this.onSelectedAuth('token');
     this.onToken(token);
     yield this.onSubmit();
   }),

diff --git a/ui/app/services/auth.js b/ui/app/services/auth.js
@@ -358,7 +358,7 @@ export default Service.extend({
     return {};
   },
 
-  async authenticate(/*{clusterId, backend, data}*/) {
+  async authenticate(/*{clusterId, backend, data, selectedAuth}*/) {
     const [options] = arguments;
     const adapter = this.clusterAdapter();
 
@@ -389,6 +389,8 @@ export default Service.extend({
   },
 
   async authSuccess(options, response) {
+    // persist selectedAuth to sessionStorage to rehydrate auth form on logout
+    sessionStorage.setItem('selectedAuth', options.selectedAuth);
     const authData = await this.persistAuthData(options, response, this.namespaceService.path);
     await this.permissions.getPaths.perform();
     return authData;
@@ -407,8 +409,11 @@ export default Service.extend({
   },
 
   getAuthType() {
-    if (!this.authData) return;
-    return this.authData.backend.type;
+    // check sessionStorage first
+    const selectedAuth = sessionStorage.getItem('selectedAuth');
+    if (selectedAuth) return selectedAuth;
+    // fallback to authData which discerns backend type from token
+    return this.authData ? this.authData.backend.type : null;
   },
 
   deleteCurrentToken() {

diff --git a/ui/app/templates/components/auth-form.hbs b/ui/app/templates/components/auth-form.hbs
@@ -68,7 +68,6 @@
         @onToken={{action (mut this.token)}}
         @namespace={{this.namespace}}
         @onNamespace={{action (mut this.namespace)}}
-        @onSelectedAuth={{action (mut this.selectedAuth)}}
         @onSubmit={{action "doSubmit"}}
         @onRoleName={{action (mut this.roleName)}}
         @roleName={{this.roleName}}

diff --git a/ui/tests/acceptance/logout-auth-method-test.js b/ui/tests/acceptance/logout-auth-method-test.js
@@ -0,0 +1,42 @@
+import { module, test } from 'qunit';
+import { setupApplicationTest } from 'ember-qunit';
+import { click, visit, fillIn, settled } from '@ember/test-helpers';
+import { setupMirage } from 'ember-cli-mirage/test-support';
+import { fakeWindow, buildMessage } from '../helpers/oidc-window-stub';
+import sinon from 'sinon';
+import { later, run } from '@ember/runloop';
+
+module('Acceptance | logout auth method', function (hooks) {
+  setupApplicationTest(hooks);
+  setupMirage(hooks);
+
+  hooks.beforeEach(function () {
+    this.openStub = sinon.stub(window, 'open').callsFake(() => fakeWindow.create());
+  });
+  hooks.afterEach(function () {
+    this.openStub.restore();
+  });
+
+  // coverage for bug where token was selected as auth method for oidc and jwt
+  test('it should populate oidc auth method on logout', async function (assert) {
+    this.server.post('/auth/oidc/oidc/auth_url', () => ({
+      data: { auth_url: 'http://example.com' },
+    }));
+    this.server.get('/auth/foo/oidc/callback', () => ({
+      auth: { client_token: 'root' },
+    }));
+    // ensure clean state
+    sessionStorage.removeItem('selectedAuth');
+    await visit('/vault/auth');
+    await fillIn('[data-test-select="auth-method"]', 'oidc');
+    later(() => run.cancelTimers(), 50);
+    await click('[data-test-auth-submit]');
+    window.postMessage(buildMessage().data, window.origin);
+    await settled();
+    await click('.nav-user-button button');
+    await click('#logout');
+    assert
+      .dom('[data-test-select="auth-method"]')
+      .hasValue('oidc', 'Previous auth method selected on logout');
+  });
+});
diff --git a/ui/tests/helpers/oidc-window-stub.js b/ui/tests/helpers/oidc-window-stub.js
@@ -0,0 +1,33 @@
+import EmberObject, { computed } from '@ember/object';
+import Evented from '@ember/object/evented';
+
+export const fakeWindow = EmberObject.extend(Evented, {
+  init() {
+    this._super(...arguments);
+    this.on('close', () => {
+      this.set('closed', true);
+    });
+  },
+  screen: computed(function () {
+    return {
+      height: 600,
+      width: 500,
+    };
+  }),
+  origin: 'https://my-vault.com',
+  closed: false,
+  open() {},
+  close() {},
+});
+
+export const buildMessage = (opts) => ({
+  isTrusted: true,
+  origin: 'https://my-vault.com',
+  data: {
+    source: 'oidc-callback',
+    path: 'foo',
+    state: 'state',
+    code: 'code',
+  },
+  ...opts,
+});
diff --git a/ui/tests/integration/components/auth-jwt-test.js b/ui/tests/integration/components/auth-jwt-test.js
@@ -1,6 +1,4 @@
 import { run } from '@ember/runloop';
-import EmberObject, { computed } from '@ember/object';
-import Evented from '@ember/object/evented';
 import Service from '@ember/service';
 import { module, test } from 'qunit';
 import { setupRenderingTest } from 'ember-qunit';
@@ -12,38 +10,19 @@ import { resolve } from 'rsvp';
 import { create } from 'ember-cli-page-object';
 import form from '../../pages/components/auth-jwt';
 import { ERROR_WINDOW_CLOSED, ERROR_MISSING_PARAMS, ERROR_JWT_LOGIN } from 'vault/components/auth-jwt';
+import { fakeWindow, buildMessage } from '../../helpers/oidc-window-stub';
 
 const component = create(form);
 const windows = [];
-const buildMessage = (opts) => ({
-  isTrusted: true,
-  origin: 'https://my-vault.com',
-  data: {},
-  ...opts,
-});
-const fakeWindow = EmberObject.extend(Evented, {
+
+fakeWindow.reopen({
   init() {
     this._super(...arguments);
-    this.on('close', () => {
-      this.set('closed', true);
-    });
     windows.push(this);
   },
-  screen: computed(function () {
-    return {
-      height: 600,
-      width: 500,
-    };
-  }),
-  origin: 'https://my-vault.com',
-  closed: false,
-});
-
-fakeWindow.reopen({
   open() {
     return fakeWindow.create();
   },
-
   close() {
     windows.forEach((w) => w.trigger('close'));
   },
@@ -227,17 +206,7 @@ module('Integration | Component | auth jwt', function (hooks) {
     await waitUntil(() => {
       return this.openSpy.calledOnce;
     });
-    this.window.trigger(
-      'message',
-      buildMessage({
-        data: {
-          source: 'oidc-callback',
-          path: 'foo',
-          state: 'state',
-          code: 'code',
-        },
-      })
-    );
+    this.window.trigger('message', buildMessage());
     await settled();
     assert.equal(this.selectedAuth, 'token', 'calls onSelectedAuth with token');
     assert.equal(this.token, 'token', 'calls onToken with token');
@@ -252,18 +221,7 @@ module('Integration | Component | auth jwt', function (hooks) {
     await waitUntil(() => {
       return this.openSpy.calledOnce;
     });
-    this.window.trigger(
-      'message',
-      buildMessage({
-        origin: 'http://hackerz.com',
-        data: {
-          source: 'oidc-callback',
-          path: 'foo',
-          state: 'state',
-          code: 'code',
-        },
-      })
-    );
+    this.window.trigger('message', buildMessage({ origin: 'http://hackerz.com' }));
     run.cancelTimers();
     await settled();
     assert.notOk(this.handler.called, 'should not call the submit handler');
@@ -277,18 +235,7 @@ module('Integration | Component | auth jwt', function (hooks) {
     await waitUntil(() => {
       return this.openSpy.calledOnce;
     });
-    this.window.trigger(
-      'message',
-      buildMessage({
-        isTrusted: false,
-        data: {
-          source: 'oidc-callback',
-          path: 'foo',
-          state: 'state',
-          code: 'code',
-        },
-      })
-    );
+    this.window.trigger('message', buildMessage({ isTrusted: false }));
     run.cancelTimers();
     await settled();
     assert.notOk(this.handler.called, 'should not call the submit handler');