OIDC Logout Bug (#14545) (#14577)

* 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 · ee78ae4 · ee78ae4
1 parent 5383be0
commit ee78ae4
Show file tree

Hide file tree

Showing 8 changed files with 172 additions and 125 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
@@ -109,26 +109,31 @@ 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',
     'methods',
     'methods.[]',
     '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);
+    function () {
+      return this.getAuthBackend();
     }
   ),
 
-  providerName: computed('selectedAuthBackend.type', function() {
+  providerName: computed('selectedAuthBackend.type', function () {
     if (!this.selectedAuthBackend) {
       return;
     }
@@ -142,22 +147,24 @@ export default Component.extend(DEFAULTS, {
 
   cspErrorText: `This is a standby Vault node but can't communicate with the active node via request forwarding. Sign in at the active node to use the Vault UI.`,
 
-  allSupportedMethods: computed('methodsToShow', 'hasMethodsWithPath', function() {
+  allSupportedMethods: computed('methodsToShow', 'hasMethodsWithPath', function () {
     let hasMethodsWithPath = this.hasMethodsWithPath;
     let methodsToShow = this.methodsToShow;
     return hasMethodsWithPath ? methodsToShow.concat(BACKENDS) : methodsToShow;
   }),
 
-  hasMethodsWithPath: computed('methodsToShow', function() {
+  hasMethodsWithPath: computed('methodsToShow', function () {
     return this.methodsToShow.isAny('path');
   }),
-  methodsToShow: computed('methods', function() {
+  methodsToShow: computed('methods', function () {
     let methods = this.methods || [];
-    let shownMethods = methods.filter(m => BACKENDS.find(b => b.type.toLowerCase() === m.type.toLowerCase()));
+    let shownMethods = methods.filter((m) =>
+      BACKENDS.find((b) => b.type.toLowerCase() === m.type.toLowerCase())
+    );
     return shownMethods.length ? shownMethods : BACKENDS;
   }),
 
-  unwrapToken: task(function*(token) {
+  unwrapToken: task(function* (token) {
     // will be using the Token Auth Method, so set it here
     this.set('selectedAuth', 'token');
     let adapter = this.store.adapterFor('tools');
@@ -170,7 +177,7 @@ export default Component.extend(DEFAULTS, {
     }
   }).withTestWaiter(),
 
-  fetchMethods: task(function*() {
+  fetchMethods: task(function* () {
     let store = this.store;
     try {
       let methods = yield store.findAll('auth-method', {
@@ -180,7 +187,7 @@ export default Component.extend(DEFAULTS, {
       });
       this.set(
         'methods',
-        methods.map(m => {
+        methods.map((m) => {
           const method = m.serialize({ includeId: true });
           return {
             ...method,
@@ -202,7 +209,7 @@ export default Component.extend(DEFAULTS, {
     this.set('loading', false);
     let errors;
     if (e.errors) {
-      errors = e.errors.map(error => {
+      errors = e.errors.map((error) => {
         if (error.detail) {
           return error.detail;
         }
@@ -215,13 +222,21 @@ export default Component.extend(DEFAULTS, {
     this.set('error', `${message}${errors.join('.')}`);
   },
 
-  authenticate: task(function*(backendType, data) {
-    let clusterId = this.cluster.id;
+  authenticate: task(function* (backendType, data) {
+    const {
+      selectedAuth,
+      cluster: { id: clusterId },
+    } = this;
     try {
       if (backendType === 'okta') {
         this.delayAuthMessageReminder.perform();
       }
-      let authResponse = yield this.auth.authenticate({ clusterId, backend: backendType, data });
+      let authResponse = yield this.auth.authenticate({
+        clusterId,
+        backend: backendType,
+        data,
+        selectedAuth,
+      });
 
       let { isRoot, namespace } = authResponse;
       let transition;
@@ -248,7 +263,7 @@ export default Component.extend(DEFAULTS, {
     }
   }).withTestWaiter(),
 
-  delayAuthMessageReminder: task(function*() {
+  delayAuthMessageReminder: task(function* () {
     if (Ember.testing) {
       this.showLoading = true;
       yield timeout(0);
@@ -272,9 +287,12 @@ 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()
+        (b) => (b.type || '').toLowerCase() === (backend.type || '').toLowerCase()
       );
       let attributes = (backendMeta || {}).formAttributes || [];
 

diff --git a/ui/app/components/auth-jwt.js b/ui/app/components/auth-jwt.js
@@ -29,6 +29,7 @@ export default Component.extend({
   onNamespace() {},
 
   didReceiveAttrs() {
+this._super();
     let { oldSelectedAuthPath, selectedAuthPath } = this;
     let shouldDebounce = !oldSelectedAuthPath && !selectedAuthPath;
     if (oldSelectedAuthPath !== selectedAuthPath) {
@@ -44,15 +45,15 @@ export default Component.extend({
 
   // Assumes authentication using OIDC until it's known that the mount is
   // configured for JWT authentication via static keys, JWKS, or OIDC discovery.
-  isOIDC: computed('errorMessage', function() {
+  isOIDC: computed('errorMessage', function () {
     return this.errorMessage !== ERROR_JWT_LOGIN;
   }),
 
   getWindow() {
     return this.window || window;
   },
 
-  fetchRole: task(function*(roleName, options = { debounce: true }) {
+  fetchRole: task(function* (roleName, options = { debounce: true }) {
     if (options.debounce) {
       this.onRoleName(roleName);
       // debounce
@@ -82,7 +83,7 @@ export default Component.extend({
     this.onError(err);
   },
 
-  prepareForOIDC: task(function*(oidcWindow) {
+  prepareForOIDC: task(function* (oidcWindow) {
     const thisWindow = this.getWindow();
     // show the loading animation in the parent
     this.onLoading(true);
@@ -104,7 +105,7 @@ export default Component.extend({
     }
   }),
 
-  watchPopup: task(function*(oidcWindow) {
+  watchPopup: task(function* (oidcWindow) {
     while (true) {
       yield timeout(WAIT_TIME);
       if (!oidcWindow || oidcWindow.closed) {
@@ -113,7 +114,7 @@ export default Component.extend({
     }
   }),
 
-  watchCurrent: task(function*(oidcWindow) {
+  watchCurrent: task(function* (oidcWindow) {
     // when user is about to change pages, close the popup window
     yield waitForEvent(this.getWindow(), 'beforeunload');
     oidcWindow.close();
@@ -125,7 +126,7 @@ export default Component.extend({
     oidcWindow.close();
   },
 
-  exchangeOIDC: task(function*(oidcState, oidcWindow) {
+  exchangeOIDC: task(function* (oidcState, oidcWindow) {
     if (oidcState === null || oidcState === undefined) {
       return;
     }
@@ -163,7 +164,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
@@ -32,7 +32,7 @@ export default Service.extend({
     return getOwner(this).lookup('adapter:cluster');
   },
 
-  tokens: computed(function() {
+  tokens: computed(function () {
     return this.getTokensFromStorage() || [];
   }),
 
@@ -91,7 +91,7 @@ export default Service.extend({
     return fetch(url, {
       method: opts.method || 'GET',
       headers: opts.headers || {},
-    }).then(response => {
+    }).then((response) => {
       if (response.status === 204) {
         return resolve();
       } else if (response.status >= 200 && response.status < 300) {
@@ -143,7 +143,7 @@ export default Service.extend({
     let currentBackend = BACKENDS.findBy('type', backend);
     let displayName;
     if (isArray(currentBackend.displayNamePath)) {
-      displayName = currentBackend.displayNamePath.map(name => get(resp, name)).join('/');
+      displayName = currentBackend.displayNamePath.map((name) => get(resp, name)).join('/');
     } else {
       displayName = get(resp, currentBackend.displayNamePath);
     }
@@ -216,7 +216,7 @@ export default Service.extend({
     return this.storage(token).removeItem(token);
   },
 
-  tokenExpirationDate: computed('currentTokenName', 'expirationCalcTS', function() {
+  tokenExpirationDate: computed('currentTokenName', 'expirationCalcTS', function () {
     const tokenName = this.currentTokenName;
     if (!tokenName) {
       return;
@@ -231,7 +231,7 @@ export default Service.extend({
     return expiration ? this.now() >= expiration : null;
   },
 
-  renewAfterEpoch: computed('currentTokenName', 'expirationCalcTS', function() {
+  renewAfterEpoch: computed('currentTokenName', 'expirationCalcTS', function () {
     const tokenName = this.currentTokenName;
     let { expirationCalcTS } = this;
     const data = this.getTokenData(tokenName);
@@ -251,18 +251,18 @@ export default Service.extend({
     }
     this.set('isRenewing', true);
     return this.renewCurrentToken().then(
-      resp => {
+      (resp) => {
         this.set('isRenewing', false);
         return this.persistAuthData(tokenName, resp.data || resp.auth);
       },
-      e => {
+      (e) => {
         this.set('isRenewing', false);
         throw e;
       }
     );
   },
 
-  checkShouldRenew: task(function*() {
+  checkShouldRenew: task(function* () {
     while (true) {
       if (Ember.testing) {
         return;
@@ -303,7 +303,7 @@ export default Service.extend({
   getTokensFromStorage(filterFn) {
     return this.storage()
       .keys()
-      .reject(key => {
+      .reject((key) => {
         return key.indexOf(TOKEN_PREFIX) !== 0 || (filterFn && filterFn(key));
       });
   },
@@ -313,27 +313,32 @@ export default Service.extend({
       return;
     }
 
-    this.getTokensFromStorage().forEach(key => {
+    this.getTokensFromStorage().forEach((key) => {
       const data = this.getTokenData(key);
       if (data && data.policies && data.policies.includes('root')) {
         this.removeTokenData(key);
       }
     });
   },
 
-  async authenticate(/*{clusterId, backend, data}*/) {
+  async authenticate(/*{clusterId, backend, data, selectedAuth}*/) {
     const [options] = arguments;
     const adapter = this.clusterAdapter();
 
     let resp = await adapter.authenticate(options);
+    // persist selectedAuth to sessionStorage to rehydrate auth form on logout
+    sessionStorage.setItem('selectedAuth', options.selectedAuth);
     let authData = await this.persistAuthData(options, resp.auth || resp.data, this.namespaceService.path);
     await this.permissions.getPaths.perform();
     return authData;
   },
 
   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() {
@@ -349,19 +354,19 @@ export default Service.extend({
   },
 
   // returns the key for the token to use
-  currentTokenName: computed('activeCluster', 'tokens', 'tokens.[]', function() {
+  currentTokenName: computed('activeCluster', 'tokens', 'tokens.[]', function () {
     const regex = new RegExp(this.activeCluster);
-    return this.tokens.find(key => regex.test(key));
+    return this.tokens.find((key) => regex.test(key));
   }),
 
-  currentToken: computed('currentTokenName', function() {
+  currentToken: computed('currentTokenName', function () {
     const name = this.currentTokenName;
     const data = name && this.getTokenData(name);
     // data.token is undefined so that's why it returns current token undefined
     return name && data ? data.token : null;
   }),
 
-  authData: computed('currentTokenName', function() {
+  authData: computed('currentTokenName', function () {
     const token = this.currentTokenName;
     if (!token) {
       return;

diff --git a/ui/app/templates/components/auth-form.hbs b/ui/app/templates/components/auth-form.hbs
@@ -49,7 +49,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}}