From 586a4dbe23e4d5624e03eb36ac7b2aacafb34d8f Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Mon, 12 Apr 2021 18:10:08 -0400 Subject: [PATCH] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20roll=20out=20OIDC=20to?= =?UTF-8?q?=20desktop?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../scopes/scope/authenticate/method/oidc.js | 18 ++++ ui/desktop/app/router.js | 4 +- .../scopes/scope/authenticate/method.js | 45 ++++++++-- .../scopes/scope/authenticate/method/index.js | 3 + .../scopes/scope/authenticate/method/oidc.js | 82 +++++++++++++++++++ .../scopes/scope/authenticate/method.hbs | 18 +--- .../scope/authenticate/method/index.hbs | 17 ++++ .../scopes/scope/authenticate/method/oidc.hbs | 23 ++++++ ui/desktop/config/environment.js | 1 + ui/desktop/electron-app/src/handlers.js | 6 +- ui/desktop/package.json | 3 + .../tests/acceptance/authentication-test.js | 2 + .../scope/authenticate/method/oidc-test.js | 17 ++++ .../scope/authenticate/method/index-test.js | 16 ++++ .../scope/authenticate/method/oidc-test.js | 16 ++++ yarn.lock | 27 ++---- 16 files changed, 251 insertions(+), 47 deletions(-) create mode 100644 ui/desktop/app/controllers/scopes/scope/authenticate/method/oidc.js create mode 100644 ui/desktop/app/routes/scopes/scope/authenticate/method/index.js create mode 100644 ui/desktop/app/routes/scopes/scope/authenticate/method/oidc.js create mode 100644 ui/desktop/app/templates/scopes/scope/authenticate/method/index.hbs create mode 100644 ui/desktop/app/templates/scopes/scope/authenticate/method/oidc.hbs create mode 100644 ui/desktop/tests/unit/controllers/scopes/scope/authenticate/method/oidc-test.js create mode 100644 ui/desktop/tests/unit/routes/scopes/scope/authenticate/method/index-test.js create mode 100644 ui/desktop/tests/unit/routes/scopes/scope/authenticate/method/oidc-test.js diff --git a/ui/desktop/app/controllers/scopes/scope/authenticate/method/oidc.js b/ui/desktop/app/controllers/scopes/scope/authenticate/method/oidc.js new file mode 100644 index 000000000..119f54ed0 --- /dev/null +++ b/ui/desktop/app/controllers/scopes/scope/authenticate/method/oidc.js @@ -0,0 +1,18 @@ +import Controller from '@ember/controller'; +import { inject as service } from '@ember/service'; + +export default class ScopesScopeAuthenticateMethodOidcController extends Controller { + // =services + + @service session; + + // =attributes + + /** + * Authetnication URL for the pending OIDC flow, if any. + * @type {?string} + */ + get authURL() { + return this.session.data.pending.oidc.attributes.auth_url; + } +} diff --git a/ui/desktop/app/router.js b/ui/desktop/app/router.js index 041868e34..71ea1f191 100644 --- a/ui/desktop/app/router.js +++ b/ui/desktop/app/router.js @@ -10,7 +10,9 @@ Router.map(function () { this.route('scopes', function () { this.route('scope', { path: ':scope_id' }, function () { this.route('authenticate', function () { - this.route('method', { path: ':auth_method_id' }); + this.route('method', { path: ':auth_method_id' }, function () { + this.route('oidc'); + }); }); this.route('projects', function () { this.route('targets', function () { diff --git a/ui/desktop/app/routes/scopes/scope/authenticate/method.js b/ui/desktop/app/routes/scopes/scope/authenticate/method.js index 33b7fd324..66fc52286 100644 --- a/ui/desktop/app/routes/scopes/scope/authenticate/method.js +++ b/ui/desktop/app/routes/scopes/scope/authenticate/method.js @@ -1,5 +1,6 @@ import Route from '@ember/routing/route'; import { inject as service } from '@ember/service'; +import { getOwner } from '@ember/application'; import { action } from '@ember/object'; import loading from 'ember-loading/decorator'; import { notifyError } from 'core/decorators/notify'; @@ -9,6 +10,7 @@ export default class ScopesScopeAuthenticateMethodRoute extends Route { @service session; @service notify; + @service ipc; @service intl; // =methods @@ -23,6 +25,26 @@ export default class ScopesScopeAuthenticateMethodRoute extends Route { return this.store.findRecord('auth-method', id, adapterOptions); } + /** + * + */ + 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. By default this uses + * `window.open`, but may be overriden. + * @param {string} url + */ + async openExternalOIDCFlow(url) { + await this.ipc.invoke('openExternal', url); + } + // =actions /** @@ -37,10 +59,23 @@ export default class ScopesScopeAuthenticateMethodRoute extends Route { const authMethod = this.modelFor('scopes.scope.authenticate.method'); const authenticatorName = `authenticator:${authMethod.type}`; const requestCookies = false; - await this.session.authenticate(authenticatorName, creds, requestCookies, { - scope, - authMethod, - }); - this.replaceWith('index'); + switch (authMethod.type) { + case 'password': + await this.session.authenticate( + authenticatorName, + creds, + requestCookies, + { scope, authMethod } + ); + this.transitionTo('index'); + break; + case 'oidc': + await this.startOIDCAuthentication(authenticatorName, { + scope, + authMethod, + }); + this.transitionTo('scopes.scope.authenticate.method.oidc'); + break; + } } } diff --git a/ui/desktop/app/routes/scopes/scope/authenticate/method/index.js b/ui/desktop/app/routes/scopes/scope/authenticate/method/index.js new file mode 100644 index 000000000..fc9b10575 --- /dev/null +++ b/ui/desktop/app/routes/scopes/scope/authenticate/method/index.js @@ -0,0 +1,3 @@ +import Route from '@ember/routing/route'; + +export default class ScopesScopeAuthenticateMethodIndexRoute extends Route {} diff --git a/ui/desktop/app/routes/scopes/scope/authenticate/method/oidc.js b/ui/desktop/app/routes/scopes/scope/authenticate/method/oidc.js new file mode 100644 index 000000000..b6cb354c4 --- /dev/null +++ b/ui/desktop/app/routes/scopes/scope/authenticate/method/oidc.js @@ -0,0 +1,82 @@ +import Route from '@ember/routing/route'; +import { getOwner } from '@ember/application'; +import { inject as service } from '@ember/service'; +import { task, timeout } from 'ember-concurrency'; +import { action } from '@ember/object'; +import { notifyError } from 'core/decorators/notify'; +import config from '../../../../../config/environment'; + +const POLL_TIMEOUT_SECONDS = config.oidcPollingTimeoutSeconds; + +export default class ScopesScopeAuthenticateMethodOidcRoute extends Route { + + // =services + + @service session; + + // =attributes + + /** + * A simple Ember Concurrency-based polling task that refreshes the route + * every POLL_TIMEOUT_SECONDS seconds. This is necessary to display changes + * to session `status` that may occur. + * + * NOTE: tasks are sort of attributes and sort of methods, but they are not + * language-level constructs. Thus we annotate this task as if it + * is an attribute. + * @type {Task} + */ + @task(function * () { + while(!this.session.isAuthenticated) { + yield timeout(POLL_TIMEOUT_SECONDS * 1000); + yield this.refresh(); + } + /* eslint-disable-next-line prettier/prettier */ + }).drop() poller; + + // =methods + + model() { + const scope = this.modelFor('scopes.scope'); + const authMethod = this.modelFor('scopes.scope.authenticate.method'); + const authenticatorName = `authenticator:${authMethod.type}`; + 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 + return oidc.attemptFetchToken({ scope, authMethod }); + } + + /** + * If authenticated, redirects to index for further processing. + */ + redirect() { + if (this.session.isAuthenticated) this.replaceWith('index'); + } + + /** + * When this route is activated (entered), begin polling for changes. + */ + activate() { + this.poller.perform(); + } + + /** + * When this route is deactivated (exited), stop polling for changes. + */ + deactivate() { + this.poller.cancelAll(); + } + + /** + * An error in this route indicates authentication failed. The user is + * notified and returned to the index. + */ + @action + @notifyError(() => 'errors.authentication-failed.title', { catch: true }) + error(e) { + this.transitionTo('scopes.scope.authenticate.method.index'); + // rethrow the error to activate the notifyError decorator + throw e; + } + +} diff --git a/ui/desktop/app/templates/scopes/scope/authenticate/method.hbs b/ui/desktop/app/templates/scopes/scope/authenticate/method.hbs index e05057cee..c24cd6895 100644 --- a/ui/desktop/app/templates/scopes/scope/authenticate/method.hbs +++ b/ui/desktop/app/templates/scopes/scope/authenticate/method.hbs @@ -1,17 +1 @@ -{{#if (feature-flag 'oidc')}} - {{! OIDC enabled: show form for any auth method}} - -{{else}} - {{! OIDC disabled: hide OIDC auth form}} - {{#unless @model.isOIDC}} - - {{/unless}} -{{/if}} +{{outlet}} diff --git a/ui/desktop/app/templates/scopes/scope/authenticate/method/index.hbs b/ui/desktop/app/templates/scopes/scope/authenticate/method/index.hbs new file mode 100644 index 000000000..e05057cee --- /dev/null +++ b/ui/desktop/app/templates/scopes/scope/authenticate/method/index.hbs @@ -0,0 +1,17 @@ +{{#if (feature-flag 'oidc')}} + {{! OIDC enabled: show form for any auth method}} + +{{else}} + {{! OIDC disabled: hide OIDC auth form}} + {{#unless @model.isOIDC}} + + {{/unless}} +{{/if}} diff --git a/ui/desktop/app/templates/scopes/scope/authenticate/method/oidc.hbs b/ui/desktop/app/templates/scopes/scope/authenticate/method/oidc.hbs new file mode 100644 index 000000000..c6e38e882 --- /dev/null +++ b/ui/desktop/app/templates/scopes/scope/authenticate/method/oidc.hbs @@ -0,0 +1,23 @@ + + <:header> +

{{t 'resources.auth-method.messages.pending.title'}}

+ + <:body> +

{{t 'resources.auth-method.messages.pending.description'}}

+

+ {{t 'resources.auth-method.questions.no-see-window'}} + + {{t 'actions.retry'}} + +

+

+ + {{t 'actions.cancel'}} + +

+ + diff --git a/ui/desktop/config/environment.js b/ui/desktop/config/environment.js index 50394aa0e..3f6883324 100644 --- a/ui/desktop/config/environment.js +++ b/ui/desktop/config/environment.js @@ -44,6 +44,7 @@ module.exports = function (environment) { notifyTimeout: 4000, sessionPollingTimeoutSeconds: 2.5, + oidcPollingTimeoutSeconds: 1, documentation: { baseURL: 'https://boundaryproject.io/help/desktop', diff --git a/ui/desktop/electron-app/src/handlers.js b/ui/desktop/electron-app/src/handlers.js index 6a251d742..d3b1d4d30 100644 --- a/ui/desktop/electron-app/src/handlers.js +++ b/ui/desktop/electron-app/src/handlers.js @@ -1,4 +1,5 @@ const { shell } = require('electron'); +const isDev = require('electron-is-dev'); const handle = require('./ipc-handler'); const boundaryCli = require('./boundary-cli'); const origin = require('./origin.js'); @@ -26,10 +27,11 @@ handle('resetOrigin', async (requestOrigin) => { }); /** - * Opens the specified URL in an external browser. Only HTTPS URLs are allowed. + * Opens the specified URL in an external browser. Only HTTPS URLs are allowed, + * except in development where all protocols are allowed. */ handle('openExternal', async (href) => { - if (href.startsWith('https://')) { + if (href.startsWith('https://') || isDev) { // openExternal is necessary in order to display documentation and to // support arbitrary OIDC flows. The protocol is validated. shell.openExternal(href); /* eng-disable OPEN_EXTERNAL_JS_CHECK */ diff --git a/ui/desktop/package.json b/ui/desktop/package.json index a34495516..aa2e7152b 100644 --- a/ui/desktop/package.json +++ b/ui/desktop/package.json @@ -36,6 +36,9 @@ "postinstall": "cd electron-app && yarn install", "posttest": "ember coverage-merge && yarn clean:coverage" }, + "dependencies": { + "ember-named-blocks-polyfill": "^0.2.4" + }, "devDependencies": { "@doyensec/electronegativity": "^1.8.1", "@ember/jquery": "^1.1.0", diff --git a/ui/desktop/tests/acceptance/authentication-test.js b/ui/desktop/tests/acceptance/authentication-test.js index fa34de896..514d1d2b7 100644 --- a/ui/desktop/tests/acceptance/authentication-test.js +++ b/ui/desktop/tests/acceptance/authentication-test.js @@ -87,9 +87,11 @@ module('Acceptance | authentication', function (hooks) { // create other resources instances.authMethods.global = this.server.create('auth-method', { scope: instances.scopes.global, + type: 'password', }); instances.authMethods.org = this.server.create('auth-method', { scope: instances.scopes.org, + type: 'password', }); urls.scopes.global = `/scopes/${instances.scopes.global.id}`; diff --git a/ui/desktop/tests/unit/controllers/scopes/scope/authenticate/method/oidc-test.js b/ui/desktop/tests/unit/controllers/scopes/scope/authenticate/method/oidc-test.js new file mode 100644 index 000000000..0a98d8794 --- /dev/null +++ b/ui/desktop/tests/unit/controllers/scopes/scope/authenticate/method/oidc-test.js @@ -0,0 +1,17 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module( + 'Unit | Controller | scopes/scope/authenticate/method/oidc', + function (hooks) { + setupTest(hooks); + + // TODO: Replace this with your real tests. + test('it exists', function (assert) { + let controller = this.owner.lookup( + 'controller:scopes/scope/authenticate/method/oidc' + ); + assert.ok(controller); + }); + } +); diff --git a/ui/desktop/tests/unit/routes/scopes/scope/authenticate/method/index-test.js b/ui/desktop/tests/unit/routes/scopes/scope/authenticate/method/index-test.js new file mode 100644 index 000000000..10a6a561c --- /dev/null +++ b/ui/desktop/tests/unit/routes/scopes/scope/authenticate/method/index-test.js @@ -0,0 +1,16 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module( + 'Unit | Route | scopes/scope/authenticate/method/index', + function (hooks) { + setupTest(hooks); + + test('it exists', function (assert) { + let route = this.owner.lookup( + 'route:scopes/scope/authenticate/method/index' + ); + assert.ok(route); + }); + } +); diff --git a/ui/desktop/tests/unit/routes/scopes/scope/authenticate/method/oidc-test.js b/ui/desktop/tests/unit/routes/scopes/scope/authenticate/method/oidc-test.js new file mode 100644 index 000000000..2c2764470 --- /dev/null +++ b/ui/desktop/tests/unit/routes/scopes/scope/authenticate/method/oidc-test.js @@ -0,0 +1,16 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module( + 'Unit | Route | scopes/scope/authenticate/method/oidc', + function (hooks) { + setupTest(hooks); + + test('it exists', function (assert) { + let route = this.owner.lookup( + 'route:scopes/scope/authenticate/method/oidc' + ); + assert.ok(route); + }); + } +); diff --git a/yarn.lock b/yarn.lock index 3851d6409..13ab98965 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7571,7 +7571,7 @@ debug@~4.1.0: dependencies: ms "^2.1.1" -debuglog@*, debuglog@^1.0.1: +debuglog@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/debuglog/-/debuglog-1.0.1.tgz#aa24ffb9ac3df9a2351837cfb2d279360cd78492" integrity sha1-qiT/uaw9+aI1GDfPstJ5NgzXhJI= @@ -12336,7 +12336,7 @@ import-lazy@^4.0.0: resolved "https://registry.yarnpkg.com/import-lazy/-/import-lazy-4.0.0.tgz#e8eb627483a0a43da3c03f3e35548be5cb0cc153" integrity sha512-rKtvo6a868b5Hu3heneU+L4yEQ4jYKLtjpnPeUdK7h0yzXGmyBTypknlkCvHFBqfX9YlorEiMM6Dnq/5atfHkw== -imurmurhash@*, imurmurhash@^0.1.4: +imurmurhash@^0.1.4: version "0.1.4" resolved "https://registry.yarnpkg.com/imurmurhash/-/imurmurhash-0.1.4.tgz#9218b9b2b928a238b13dc4fb6b6d576f231453ea" integrity sha1-khi5srkoojixPcT7a21XbyMUU+o= @@ -14034,11 +14034,6 @@ lodash._baseflatten@^3.0.0: lodash.isarguments "^3.0.0" lodash.isarray "^3.0.0" -lodash._baseindexof@*: - version "3.1.0" - resolved "https://registry.yarnpkg.com/lodash._baseindexof/-/lodash._baseindexof-3.1.0.tgz#fe52b53a1c6761e42618d654e4a25789ed61822c" - integrity sha1-/lK1OhxnYeQmGNZU5KJXie1hgiw= - lodash._baseuniq@~4.6.0: version "4.6.0" resolved "https://registry.yarnpkg.com/lodash._baseuniq/-/lodash._baseuniq-4.6.0.tgz#0ebb44e456814af7905c6212fa2c9b2d51b841e8" @@ -14047,16 +14042,11 @@ lodash._baseuniq@~4.6.0: lodash._createset "~4.0.0" lodash._root "~3.0.0" -lodash._bindcallback@*, lodash._bindcallback@^3.0.0: +lodash._bindcallback@^3.0.0: version "3.0.1" resolved "https://registry.yarnpkg.com/lodash._bindcallback/-/lodash._bindcallback-3.0.1.tgz#e531c27644cf8b57a99e17ed95b35c748789392e" integrity sha1-5THCdkTPi1epnhftlbNcdIeJOS4= -lodash._cacheindexof@*: - version "3.0.2" - resolved "https://registry.yarnpkg.com/lodash._cacheindexof/-/lodash._cacheindexof-3.0.2.tgz#3dc69ac82498d2ee5e3ce56091bafd2adc7bde92" - integrity sha1-PcaayCSY0u5ePOVgkbr9Ktx73pI= - lodash._createassigner@^3.0.0: version "3.1.1" resolved "https://registry.yarnpkg.com/lodash._createassigner/-/lodash._createassigner-3.1.1.tgz#838a5bae2fdaca63ac22dee8e19fa4e6d6970b11" @@ -14066,19 +14056,12 @@ lodash._createassigner@^3.0.0: lodash._isiterateecall "^3.0.0" lodash.restparam "^3.0.0" -lodash._createcache@*: - version "3.1.2" - resolved "https://registry.yarnpkg.com/lodash._createcache/-/lodash._createcache-3.1.2.tgz#56d6a064017625e79ebca6b8018e17440bdcf093" - integrity sha1-VtagZAF2JeeevKa4AY4XRAvc8JM= - dependencies: - lodash._getnative "^3.0.0" - lodash._createset@~4.0.0: version "4.0.3" resolved "https://registry.yarnpkg.com/lodash._createset/-/lodash._createset-4.0.3.tgz#0f4659fbb09d75194fa9e2b88a6644d363c9fe26" integrity sha1-D0ZZ+7CddRlPqeK4imZE02PJ/iY= -lodash._getnative@*, lodash._getnative@^3.0.0: +lodash._getnative@^3.0.0: version "3.9.1" resolved "https://registry.yarnpkg.com/lodash._getnative/-/lodash._getnative-3.9.1.tgz#570bc7dede46d61cdcde687d65d3eecbaa3aaff5" integrity sha1-VwvH3t5G1hzc3mh9ZdPuy6o6r/U= @@ -14296,7 +14279,7 @@ lodash.pick@^4.4.0: resolved "https://registry.yarnpkg.com/lodash.pick/-/lodash.pick-4.4.0.tgz#52f05610fff9ded422611441ed1fc123a03001b3" integrity sha1-UvBWEP/53tQiYRRB7R/BI6AwAbM= -lodash.restparam@*, lodash.restparam@^3.0.0: +lodash.restparam@^3.0.0: version "3.6.1" resolved "https://registry.yarnpkg.com/lodash.restparam/-/lodash.restparam-3.6.1.tgz#936a4e309ef330a7645ed4145986c85ae5b20805" integrity sha1-k2pOMJ7zMKdkXtQUWYbIWuWyCAU=