diff --git a/ui/admin/app/routes/scopes/scope/authenticate/method.js b/ui/admin/app/routes/scopes/scope/authenticate/method.js index a7f6be207..9c42ca274 100644 --- a/ui/admin/app/routes/scopes/scope/authenticate/method.js +++ b/ui/admin/app/routes/scopes/scope/authenticate/method.js @@ -11,7 +11,7 @@ export default class ScopesScopeAuthenticateMethodRoute extends Route { @service session; @service notify; @service intl; - @service('browser/window') window; + @service windowManager; // =methods @@ -26,22 +26,16 @@ export default class ScopesScopeAuthenticateMethodRoute extends Route { } /** - * + * Kicks off the OIDC auth flow by opening a new window to the provider. + * @param {string} authenticatorName + * @param {object} options */ async startOIDCAuthentication(authenticatorName, options) { const oidc = getOwner(this).lookup(authenticatorName); // TODO: delegate this call from the session service so that we don't have // to look up the authenticator directly const json = await oidc.startAuthentication(options); - await this.openExternalOIDCFlow(json.attributes.auth_url); - } - - /** - * Opens the specified URL in a new tab or window. - * @param {string} url - */ - openExternalOIDCFlow(url) { - this.window.open(url); + await this.windowManager.open(json.attributes.auth_url); } // =actions diff --git a/ui/admin/app/routes/scopes/scope/authenticate/method/oidc.js b/ui/admin/app/routes/scopes/scope/authenticate/method/oidc.js index b6cb354c4..db24f3bbd 100644 --- a/ui/admin/app/routes/scopes/scope/authenticate/method/oidc.js +++ b/ui/admin/app/routes/scopes/scope/authenticate/method/oidc.js @@ -13,6 +13,7 @@ export default class ScopesScopeAuthenticateMethodOidcRoute extends Route { // =services @service session; + @service windowManager; // =attributes @@ -61,10 +62,12 @@ export default class ScopesScopeAuthenticateMethodOidcRoute extends Route { } /** - * When this route is deactivated (exited), stop polling for changes. + * When this route is deactivated (exited), stop polling for changes and close + * any windows opened via the window manager service. */ deactivate() { this.poller.cancelAll(); + this.windowManager.closeAll(); } /** diff --git a/ui/admin/app/services/window-manager.js b/ui/admin/app/services/window-manager.js index 8c54be361..b9cdacf04 100644 --- a/ui/admin/app/services/window-manager.js +++ b/ui/admin/app/services/window-manager.js @@ -28,7 +28,7 @@ export default class WindowManagerService extends Service { */ open(url) { const newWindow = this.window.open(url); - this.add(newWindow); + if (newWindow) this.add(newWindow); } /** diff --git a/ui/admin/app/styles/app.scss b/ui/admin/app/styles/app.scss index 4647973af..420258319 100644 --- a/ui/admin/app/styles/app.scss +++ b/ui/admin/app/styles/app.scss @@ -152,6 +152,12 @@ } } +.rose-dialog-cover { + svg { + max-width: 20rem; + } +} + .copyable { white-space: nowrap; diff --git a/ui/admin/app/templates/scopes/scope/authenticate/method/oidc.hbs b/ui/admin/app/templates/scopes/scope/authenticate/method/oidc.hbs index dedca3ab3..651b0a350 100644 --- a/ui/admin/app/templates/scopes/scope/authenticate/method/oidc.hbs +++ b/ui/admin/app/templates/scopes/scope/authenticate/method/oidc.hbs @@ -8,9 +8,9 @@

{{t 'resources.auth-method.questions.no-see-window'}} {{t 'actions.retry'}} diff --git a/ui/admin/config/environment.js b/ui/admin/config/environment.js index a0350eae9..14982afa1 100644 --- a/ui/admin/config/environment.js +++ b/ui/admin/config/environment.js @@ -149,8 +149,9 @@ module.exports = function (environment) { // Notification timeout should be 0 for fast tests ENV.notifyTimeout = 0; - // Session polling timeout should be short + // Polling timeouts should be short ENV.sessionPollingTimeoutSeconds = 0.25; + ENV.oidcPollingTimeoutSeconds = 0; ENV.enableConfirmService = false; } diff --git a/ui/admin/tests/acceptance/authentication-test.js b/ui/admin/tests/acceptance/authentication-test.js index 3578ae935..f42540f4f 100644 --- a/ui/admin/tests/acceptance/authentication-test.js +++ b/ui/admin/tests/acceptance/authentication-test.js @@ -12,12 +12,14 @@ import { import { setupApplicationTest } from 'ember-qunit'; import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; import { Response } from 'miragejs'; +//import { run, later } from '@ember/runloop'; import a11yAudit from 'ember-a11y-testing/test-support/audit'; import { currentSession, authenticateSession, invalidateSession, } from 'ember-simple-auth/test-support'; +import Service from '@ember/service'; module('Acceptance | authentication', function (hooks) { setupApplicationTest(hooks); @@ -31,12 +33,15 @@ module('Acceptance | authentication', function (hooks) { let orgScopeURL; let scope; let globalAuthMethod; - let authMethod; let globalAuthMethodID; + let authMethod; let authMethodID; + let authMethodOIDC; + let authMethodOIDCID; let globalAuthenticateURL; let authenticateURL; let authMethodAuthenticateURL; + let authMethodOIDCAuthenticateURL; let authMethodGlobalAuthenticateURL; let changePasswordURL; let orgsURL; @@ -72,15 +77,21 @@ module('Acceptance | authentication', function (hooks) { scope: orgScope, type: 'password', }); + authMethodOIDC = this.server.create('auth-method', { + scope: orgScope, + type: 'oidc', + }); orgScopeID = orgScope.id; globalAuthMethodID = globalAuthMethod.id; authMethodID = authMethod.id; + authMethodOIDCID = authMethodOIDC.id; scopesURL = `/scopes`; orgScopeURL = `/scopes/${orgScopeID}`; globalAuthenticateURL = `/scopes/global/authenticate`; authenticateURL = `/scopes/${orgScopeID}/authenticate`; authMethodGlobalAuthenticateURL = `/scopes/global/authenticate/${globalAuthMethodID}`; authMethodAuthenticateURL = `/scopes/${orgScopeID}/authenticate/${authMethodID}`; + authMethodOIDCAuthenticateURL = `/scopes/${orgScopeID}/authenticate/${authMethodOIDCID}`; changePasswordURL = `/account/change-password`; orgsURL = `/scopes/global/scopes`; orgEditURL = `/scopes/${orgScopeID}/edit`; @@ -105,6 +116,7 @@ module('Acceptance | authentication', function (hooks) { test('visiting auth methods authenticate route when there no methods shows a message', async function (assert) { assert.expect(2); authMethod.destroy(); + authMethodOIDC.destroy(); await visit(authenticateURL); await a11yAudit(); assert.equal(currentURL(), authenticateURL); @@ -350,4 +362,28 @@ module('Acceptance | authentication', function (hooks) { assert.notOk(getRootElement().classList.contains('rose-theme-light')); assert.notOk(getRootElement().classList.contains('rose-theme-dark')); }); + + // TODO: figure out a sustainable test strategy for polling routes + test('OIDC authentication opens a new window and closes it upon completion', async function (assert) { + assert.expect(3); + // Register mock window service + this.owner.register( + 'service:browser/window', + class extends Service { + open(/* url */) { + assert.ok(true, 'New window was opened.'); + return { + close() { + assert.ok(true, 'New window was closed.'); + assert.ok(currentSession().isAuthenticated); + }, + }; + } + } + ); + await visit(authMethodOIDCAuthenticateURL); + await click('form [type="submit"]'); + }); + + // TODO: test OIDC retry and cancel });