OIDC/JWT Role Fetch Error Handling Updates

changelog/23908.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Updates OIDC/JWT login error handling to surface all role related errors
+```

ui/app/components/auth-jwt.js

@@ -3,13 +3,11 @@
  * SPDX-License-Identifier: BUSL-1.1
  */
 
-import Ember from 'ember';
 import { inject as service } from '@ember/service';
 // ARG NOTE: Once you remove outer-html after glimmerizing you can remove the outer-html component
 import Component from './outer-html';
 import { task, timeout, waitForEvent } from 'ember-concurrency';
-import { computed } from '@ember/object';
-import { waitFor } from '@ember/test-waiters';
+import { debounce } from '@ember/runloop';
 
 const WAIT_TIME = 500;
 const ERROR_WINDOW_CLOSED =
@@ -28,57 +26,50 @@ export default Component.extend({
   roleName: null,
   role: null,
   errorMessage: null,
+  isOIDC: true,
+
   onRoleName() {},
   onLoading() {},
   onError() {},
   onNamespace() {},
 
   didReceiveAttrs() {
-    this._super();
-    const debounce = !this.oldSelectedAuthPath && !this.selectedAuthPath;
-
-    if (this.oldSelectedAuthPath !== this.selectedAuthPath || debounce) {
-      this.fetchRole.perform(this.roleName, { debounce });
+    this._super(...arguments);
+    // if mount path or type changes we need to check again for JWT configuration
+    const didChangePath = this._authPath !== this.selectedAuthPath;
+    const didChangeType = this._authType !== this.selectedAuthType;
+
+    if (didChangePath || didChangeType) {
+      // path updates as the user types so we need to debounce that event
+      const wait = didChangePath ? 500 : 0;
+      debounce(this, 'fetchRole', wait);
     }
-
-    this.set('errorMessage', null);
-    this.set('oldSelectedAuthPath', this.selectedAuthPath);
+    this._authPath = this.selectedAuthPath;
+    this._authType = this.selectedAuthType;
   },
 
-  // 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 () {
-    return this.errorMessage !== ERROR_JWT_LOGIN;
-  }),
-
   getWindow() {
     return this.window || window;
   },
 
-  fetchRole: task(
-    waitFor(function* (roleName, options = { debounce: true }) {
-      if (options.debounce) {
-        this.onRoleName(roleName);
-        // debounce
-        yield timeout(Ember.testing ? 0 : WAIT_TIME);
-      }
-      const path = this.selectedAuthPath || this.selectedAuthType;
-      const id = JSON.stringify([path, roleName]);
-      let role = null;
-      try {
-        role = yield this.store.findRecord('role-jwt', id, { adapterOptions: { namespace: this.namespace } });
-      } catch (e) {
-        // throwing here causes failures in tests
-        if ((!e.httpStatus || e.httpStatus !== 400) && !Ember.testing) {
-          throw e;
-        }
-        if (e.errors && e.errors.length > 0) {
-          this.set('errorMessage', e.errors[0]);
-        }
-      }
+  async fetchRole() {
+    const path = this.selectedAuthPath || this.selectedAuthType;
+    const id = JSON.stringify([path, this.roleName]);
+    this.setProperties({ role: null, errorMessage: null, isOIDC: true });
+
+    try {
+      const role = await this.store.findRecord('role-jwt', id, {
+        adapterOptions: { namespace: this.namespace },
+      });
       this.set('role', role);
-    })
-  ).restartable(),
+    } catch (e) {
+      const error = (e.errors || [])[0];
+      const errorMessage =
+        e.httpStatus === 400 ? 'Invalid role. Please try again.' : `Error fetching role: ${error}`;
+      // assume OIDC until it's known that the mount is configured for JWT authentication via static keys, JWKS, or OIDC discovery.
+      this.setProperties({ isOIDC: error !== ERROR_JWT_LOGIN, errorMessage });
+    }
+  },
 
   cancelLogin(oidcWindow, errorMessage) {
     this.closeWindow(oidcWindow);
@@ -170,33 +161,20 @@ export default Component.extend({
     yield this.onSubmit(null, null, resp.auth.client_token);
   }),
 
-  actions: {
-    async startOIDCAuth(data, e) {
-      this.onError(null);
-      if (e && e.preventDefault) {
-        e.preventDefault();
-      }
-      try {
-        await this.fetchRole.perform(this.roleName, { debounce: false });
-      } catch (error) {
-        // this task could be cancelled if the instances in didReceiveAttrs resolve after this was started
-        if (error?.name !== 'TaskCancelation') {
-          throw error;
-        }
-      }
-      if (!this.isOIDC || !this.role || !this.role.authUrl) {
-        let message = this.errorMessage;
-        if (!this.role) {
-          message = 'Invalid role. Please try again.';
-        } else if (!this.role.authUrl) {
-          message =
-            'Missing auth_url. Please check that allowed_redirect_uris for the role include this mount path.';
-        }
-        this.onError(message);
-        return;
-      }
-      const win = this.getWindow();
+  async startOIDCAuth() {
+    this.onError(null);
 
+    await this.fetchRole();
+
+    const error =
+      this.role && !this.role.authUrl
+        ? 'Missing auth_url. Please check that allowed_redirect_uris for the role include this mount path.'
+        : this.errorMessage || null;
+
+    if (error) {
+      this.onError(error);
+    } else {
+      const win = this.getWindow();
       const POPUP_WIDTH = 500;
       const POPUP_HEIGHT = 600;
       const left = win.screen.width / 2 - POPUP_WIDTH / 2;
@@ -208,6 +186,23 @@ export default Component.extend({
       );
 
       this.prepareForOIDC.perform(oidcWindow);
+    }
+  },
+
+  actions: {
+    onRoleChange(event) {
+      this.onRoleName(event.target.value);
+      debounce(this, 'fetchRole', 500);
+    },
+    signIn(event) {
+      event.preventDefault();
+
+      if (this.isOIDC) {
+        this.startOIDCAuth();
+      } else {
+        const { jwt, roleName: role } = this;
+        this.onSubmit({ role, jwt });
+      }
     },
   },
 });

ui/app/templates/components/auth-jwt.hbs

@@ -3,20 +3,20 @@
   SPDX-License-Identifier: BUSL-1.1
 ~}}
 
-<form id="auth-form" onsubmit={{action (if this.isOIDC "startOIDCAuth" @onSubmit) (hash role=this.roleName jwt=this.jwt)}}>
+<form id="auth-form" {{on "submit" (action "signIn")}}>
   <div class="field">
     <label for="role" class="is-label">Role</label>
     <div class="control">
       <input
-        value={{@roleName}}
+        value={{this.roleName}}
         placeholder="Default"
-        oninput={{perform this.fetchRole value="target.value"}}
         autocomplete="off"
         spellcheck="false"
         name="role"
         id="role"
         class="input"
         type="text"
+        {{on "input" (action "onRoleChange")}}
         data-test-role
       />
     </div>

ui/tests/acceptance/oidc-auth-method-test.js

@@ -11,22 +11,49 @@ import { setupMirage } from 'ember-cli-mirage/test-support';
 import { fakeWindow, buildMessage } from '../helpers/oidc-window-stub';
 import sinon from 'sinon';
 import { later, _cancelTimers as cancelTimers } from '@ember/runloop';
+import { Response } from 'miragejs';
 
 module('Acceptance | oidc auth method', function (hooks) {
   setupApplicationTest(hooks);
   setupMirage(hooks);
 
   hooks.beforeEach(function () {
     this.openStub = sinon.stub(window, 'open').callsFake(() => fakeWindow.create());
-    // OIDC test fails when using fake timestamps, we use the real timestamp.now here
-    this.server.post('/auth/oidc/oidc/auth_url', () => ({
-      data: { auth_url: 'http://example.com' },
-    }));
+
+    this.setupMocks = (assert) => {
+      this.server.post('/auth/oidc/oidc/auth_url', () => ({
+        data: { auth_url: 'http://example.com' },
+      }));
+      // there was a bug that would result in the /auth/:path/login endpoint hit with an empty payload rather than lookup-self
+      // ensure that the correct endpoint is hit after the oidc callback
+      if (assert) {
+        this.server.get('/auth/token/lookup-self', (schema, req) => {
+          assert.ok(true, 'request made to auth/token/lookup-self after oidc callback');
+          req.passthrough();
+        });
+      }
+    };
+
     this.server.get('/auth/foo/oidc/callback', () => ({
       auth: { client_token: 'root' },
     }));
+
+    // select method from dropdown or click auth path tab
+    this.selectMethod = async (method, useLink) => {
+      const methodSelector = useLink
+        ? `[data-test-auth-method-link="${method}"]`
+        : '[data-test-select="auth-method"]';
+      await waitUntil(() => find(methodSelector));
+      if (useLink) {
+        await click(`[data-test-auth-method-link="${method}"]`);
+      } else {
+        await fillIn('[data-test-select="auth-method"]', method);
+      }
+    };
+
     // ensure clean state
     localStorage.removeItem('selectedAuth');
+    authPage.logout();
   });
 
   hooks.afterEach(function () {
@@ -36,14 +63,9 @@ module('Acceptance | oidc auth method', function (hooks) {
   test('it should login with oidc when selected from auth methods dropdown', async function (assert) {
     assert.expect(1);
 
-    this.server.get('/auth/token/lookup-self', (schema, req) => {
-      assert.ok(true, 'request made to auth/token/lookup-self after oidc callback');
-      req.passthrough();
-    });
-    authPage.logout();
-    // select from dropdown or click auth path tab
-    await waitUntil(() => find('[data-test-select="auth-method"]'));
-    await fillIn('[data-test-select="auth-method"]', 'oidc');
+    this.setupMocks(assert);
+
+    await this.selectMethod('oidc');
     later(() => {
       window.postMessage(buildMessage().data, window.origin);
       cancelTimers();
@@ -54,6 +76,8 @@ module('Acceptance | oidc auth method', function (hooks) {
   test('it should login with oidc from listed auth mount tab', async function (assert) {
     assert.expect(3);
 
+    this.setupMocks(assert);
+
     this.server.get('/sys/internal/ui/mounts', () => ({
       data: {
         auth: {
@@ -68,17 +92,8 @@ module('Acceptance | oidc auth method', function (hooks) {
       assert.ok(true, 'auth_url request made to correct non-standard mount path');
       return { data: { auth_url: 'http://example.com' } };
     });
-    // there was a bug that would result in the /auth/:path/login endpoint hit with an empty payload rather than lookup-self
-    // ensure that the correct endpoint is hit after the oidc callback
-    this.server.get('/auth/token/lookup-self', (schema, req) => {
-      assert.ok(true, 'request made to auth/token/lookup-self after oidc callback');
-      req.passthrough();
-    });
 
-    authPage.logout();
-    // select from dropdown or click auth path tab
-    await waitUntil(() => find('[data-test-auth-method-link="oidc"]'));
-    await click('[data-test-auth-method-link="oidc"]');
+    await this.selectMethod('oidc', true);
     later(() => {
       window.postMessage(buildMessage().data, window.origin);
       cancelTimers();
@@ -88,10 +103,8 @@ module('Acceptance | oidc auth method', function (hooks) {
 
   // 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) {
-    authPage.logout();
-    // select from dropdown or click auth path tab
-    await waitUntil(() => find('[data-test-select="auth-method"]'));
-    await fillIn('[data-test-select="auth-method"]', 'oidc');
+    this.setupMocks();
+    await this.selectMethod('oidc');
     later(() => {
       window.postMessage(buildMessage().data, window.origin);
       cancelTimers();
@@ -104,4 +117,39 @@ module('Acceptance | oidc auth method', function (hooks) {
       .dom('[data-test-select="auth-method"]')
       .hasValue('oidc', 'Previous auth method selected on logout');
   });
+
+  test('it should fetch role when switching between oidc/jwt auth methods and changing the mount path', async function (assert) {
+    let reqCount = 0;
+    this.server.post('/auth/:method/oidc/auth_url', (schema, req) => {
+      reqCount++;
+      const errors =
+        req.params.method === 'jwt' ? ['OIDC login is not configured for this mount'] : ['missing role'];
+      return new Response(400, {}, { errors });
+    });
+
+    await this.selectMethod('oidc');
+    assert.dom('[data-test-jwt]').doesNotExist('JWT Token input hidden for OIDC');
+    await this.selectMethod('jwt');
+    assert.dom('[data-test-jwt]').exists('JWT Token input renders for JWT configured method');
+    await click('[data-test-auth-form-options-toggle]');
+    await fillIn('[data-test-auth-form-mount-path]', 'foo');
+    assert.strictEqual(reqCount, 3, 'Role is fetched when dependant values are changed');
+  });
+
+  test('it should display role fetch errors when signing in with OIDC', async function (assert) {
+    this.server.post('/auth/:method/oidc/auth_url', (schema, req) => {
+      const { role } = JSON.parse(req.requestBody);
+      const status = role ? 403 : 400;
+      const errors = role ? ['permission denied'] : ['missing role'];
+      return new Response(status, {}, { errors });
+    });
+
+    await this.selectMethod('oidc');
+    await click('[data-test-auth-submit]');
+    assert.dom('[data-test-message-error-description]').hasText('Invalid role. Please try again.');
+
+    await fillIn('[data-test-role]', 'test');
+    await click('[data-test-auth-submit]');
+    assert.dom('[data-test-message-error-description]').hasText('Error fetching role: permission denied');
+  });
 });

ui/tests/integration/components/auth-jwt-test.js

@@ -138,7 +138,7 @@ module('Integration | Component | auth jwt', function (hooks) {
 
     await component.role('okta');
     // 1 for initial render, 1 for each time role changed = 3
-    assert.strictEqual(this.server.handledRequests.length, 4, 'fetches the auth_url when the path changes');
+    assert.strictEqual(this.server.handledRequests.length, 3, 'fetches the auth_url when the path changes');
     assert.strictEqual(
       component.loginButtonText,
       'Sign in with Okta',