From 3d13d8f412ee60f01f7d7ceeff825eca58201e25 Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Thu, 1 Apr 2021 17:41:30 -0400 Subject: [PATCH] =?UTF-8?q?refactor:=20=F0=9F=92=A1=20auth=20payloads=20no?= =?UTF-8?q?w=20have=20"attributes"=20field?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- addons/api/mirage/route-handlers/auth.js | 58 ++++++----- addons/auth/addon/authenticators/base.js | 11 ++- addons/auth/addon/authenticators/oidc.js | 8 +- addons/auth/addon/authenticators/password.js | 2 +- .../tests/unit/authenticators/base-test.js | 42 ++++---- .../unit/authenticators/password-test.js | 2 +- addons/core/addon/authenticators/password.js | 4 +- .../unit/authenticators/password-test.js | 12 +-- .../scopes/scope/authenticate/method.js | 2 +- ui/desktop/app/authenticators/password.js | 98 ------------------- .../unit/authenticators/password-test.js | 53 ---------- 11 files changed, 74 insertions(+), 218 deletions(-) delete mode 100644 ui/desktop/app/authenticators/password.js delete mode 100644 ui/desktop/tests/unit/authenticators/password-test.js diff --git a/addons/api/mirage/route-handlers/auth.js b/addons/api/mirage/route-handlers/auth.js index b96411ee3..4c7c3f1d0 100644 --- a/addons/api/mirage/route-handlers/auth.js +++ b/addons/api/mirage/route-handlers/auth.js @@ -12,7 +12,7 @@ import { Response } from 'miragejs'; // */ // export default function authenticateHandler(schema, request) { // const payload = JSON.parse(request.requestBody); -// if (payload.credentials.login_name === 'error') { +// if (payload.attributes.login_name === 'error') { // return new Response(400); // } else { // const cookieName = config.auth.passwordCookieName; @@ -32,23 +32,25 @@ const oidcRequiredAttempts = 3; const commandHandlers = { password: { login: (payload, scopeAttrs) => { - if (payload.credentials.login_name === 'error') { + if (payload.attributes.login_name === 'error') { return new Response(400); } else { return new Response( 200, {}, { - scope: scopeAttrs, - id: 'token123', - token: 'thetokenstring', - account_id: '1', - user_id: 'user123', - auth_method_id: 'authmethod123', - created_time: '', - updated_time: '', - last_used_time: '', - expiration_time: '', + attributes: { + scope: scopeAttrs, + id: 'token123', + token: 'thetokenstring', + account_id: '1', + user_id: 'user123', + auth_method_id: 'authmethod123', + created_time: '', + updated_time: '', + last_used_time: '', + expiration_time: '', + }, } ); } @@ -71,10 +73,12 @@ const commandHandlers = { 200, {}, { - authorization_request_url: 'https://www.duckduckgo.com', - retreival_url: '', - token_request_id: 'token_request_1234', - state: 'base_58_encoded_string', + attributes: { + authorization_request_url: 'https://www.duckduckgo.com', + retreival_url: '', + token_request_id: 'token_request_1234', + state: 'base_58_encoded_string', + }, } ), token: (_, scopeAttrs) => { @@ -86,16 +90,18 @@ const commandHandlers = { 200, {}, { - scope: scopeAttrs, - id: 'token123', - token: 'thetokenstring', - account_id: '1', - user_id: 'user123', - auth_method_id: 'authmethod123', - created_time: '', - updated_time: '', - last_used_time: '', - expiration_time: '', + attributes: { + scope: scopeAttrs, + id: 'token123', + token: 'thetokenstring', + account_id: '1', + user_id: 'user123', + auth_method_id: 'authmethod123', + created_time: '', + updated_time: '', + last_used_time: '', + expiration_time: '', + }, } ); } diff --git a/addons/auth/addon/authenticators/base.js b/addons/auth/addon/authenticators/base.js index 1611dd6ec..bdc69d504 100644 --- a/addons/auth/addon/authenticators/base.js +++ b/addons/auth/addon/authenticators/base.js @@ -72,9 +72,10 @@ export default class BaseAuthenticator extends SimpleAuthBaseAuthenticator { */ async restore(data) { if (!data) return reject(); - return this.validateToken(data.token, data.id).then(() => - this.normalizeData(data) - ); + return this.validateToken( + data.attributes.token, + data.attributes.id + ).then(() => this.normalizeData(data)); } /** @@ -86,6 +87,10 @@ export default class BaseAuthenticator extends SimpleAuthBaseAuthenticator { * @return {object} */ normalizeData(data, username) { + // Pull fields up from `data.attributes` for easier access in JavaScript. + // The `attributes` field exists on the Go side for its convenience but is + // unnecessary here. + Object.assign(data, data.attributes); // Add booleans indicated the scope type data.isGlobal = data?.scope?.type === 'global'; data.isOrg = data?.scope?.type === 'org'; diff --git a/addons/auth/addon/authenticators/oidc.js b/addons/auth/addon/authenticators/oidc.js index 28c6213d9..87c2ce198 100644 --- a/addons/auth/addon/authenticators/oidc.js +++ b/addons/auth/addon/authenticators/oidc.js @@ -83,8 +83,12 @@ export default class OIDCAuthenticator extends BaseAuthenticator { const url = this.buildAuthEndpointURL(options); const body = JSON.stringify({ command: 'token', - state: this.session.get('data.pending.oidc.state'), - token_request_id: this.session.get('data.pending.oidc.token_request_id'), + attributes: { + state: this.session.get('data.pending.oidc.state'), + token_request_id: this.session.get( + 'data.pending.oidc.token_request_id' + ), + }, }); // Fetch the endpoint and get the response JSON const response = await fetch(url, { method: 'post', body }); diff --git a/addons/auth/addon/authenticators/password.js b/addons/auth/addon/authenticators/password.js index 6b22ee61b..3e0eb53e4 100644 --- a/addons/auth/addon/authenticators/password.js +++ b/addons/auth/addon/authenticators/password.js @@ -58,7 +58,7 @@ export default class PasswordAuthenticator extends BaseAuthenticator { const body = JSON.stringify({ command: 'login', token_type: requestCookies ? 'cookie' : null, - credentials: { login_name, password }, + attributes: { login_name, password }, }); const authEndpointURL = this.buildAuthEndpointURL(options); const response = await fetch(authEndpointURL, { method: 'post', body }); diff --git a/addons/auth/tests/unit/authenticators/base-test.js b/addons/auth/tests/unit/authenticators/base-test.js index 726e76162..31d2b3603 100644 --- a/addons/auth/tests/unit/authenticators/base-test.js +++ b/addons/auth/tests/unit/authenticators/base-test.js @@ -32,29 +32,25 @@ module('Unit | Authenticator | base', function (hooks) { test('it passes token validation on restoration if receiving 200', async function (assert) { assert.expect(1); - const mockData = { id: 'token_123' }; + const id = 'token_123'; + const mockData = { attributes: { id } }; const authenticator = this.owner.lookup('authenticator:base'); - this.server.get( - authenticator.buildTokenValidationEndpointURL(mockData.id), - () => { - assert.ok(true, 'token validation was requested'); - return new Response(200); - } - ); + this.server.get(authenticator.buildTokenValidationEndpointURL(id), () => { + assert.ok(true, 'token validation was requested'); + return new Response(200); + }); await authenticator.restore(mockData); }); test('it fails token validation on restoration if receiving 401', async function (assert) { assert.expect(2); - const mockData = { id: 'token_123' }; + const id = 'token_123'; + const mockData = { attributes: { id } }; const authenticator = this.owner.lookup('authenticator:base'); - this.server.get( - authenticator.buildTokenValidationEndpointURL(mockData.id), - () => { - assert.ok(true, 'token validation was requested'); - return new Response(401); - } - ); + this.server.get(authenticator.buildTokenValidationEndpointURL(id), () => { + assert.ok(true, 'token validation was requested'); + return new Response(401); + }); try { await authenticator.restore(mockData); } catch (e) { @@ -64,15 +60,13 @@ module('Unit | Authenticator | base', function (hooks) { test('it fails token validation on restoration if receiving 404', async function (assert) { assert.expect(2); - const mockData = { id: 'token_123' }; + const id = 'token_123'; + const mockData = { attributes: { id } }; const authenticator = this.owner.lookup('authenticator:base'); - this.server.get( - authenticator.buildTokenValidationEndpointURL(mockData.id), - () => { - assert.ok(true, 'token validation was requested'); - return new Response(404); - } - ); + this.server.get(authenticator.buildTokenValidationEndpointURL(id), () => { + assert.ok(true, 'token validation was requested'); + return new Response(404); + }); try { await authenticator.restore(mockData); } catch (e) { diff --git a/addons/auth/tests/unit/authenticators/password-test.js b/addons/auth/tests/unit/authenticators/password-test.js index a444702bb..9799101e7 100644 --- a/addons/auth/tests/unit/authenticators/password-test.js +++ b/addons/auth/tests/unit/authenticators/password-test.js @@ -49,7 +49,7 @@ module('Unit | Authenticator | password', function (hooks) { assert.deepEqual(json, { command: 'login', token_type: 'cookie', - credentials: { + attributes: { login_name: 'foo', password: 'bar', }, diff --git a/addons/core/addon/authenticators/password.js b/addons/core/addon/authenticators/password.js index c76874ce9..186731a7d 100644 --- a/addons/core/addon/authenticators/password.js +++ b/addons/core/addon/authenticators/password.js @@ -62,7 +62,7 @@ export default class PasswordAuthenticator extends BasePasswordAuthenticator { */ authenticate() { return super.authenticate(...arguments).then((data) => { - const token = data?.token; + const token = data?.attributes?.token; this.addTokenToAuthorization(token); return data; }); @@ -76,7 +76,7 @@ export default class PasswordAuthenticator extends BasePasswordAuthenticator { * @return {object} */ restore(data) { - const token = data?.token; + const token = data?.attributes?.token; this.addTokenToAuthorization(token); return super.restore(data); } diff --git a/addons/core/tests/unit/authenticators/password-test.js b/addons/core/tests/unit/authenticators/password-test.js index a7580ef7d..2542aed56 100644 --- a/addons/core/tests/unit/authenticators/password-test.js +++ b/addons/core/tests/unit/authenticators/password-test.js @@ -26,13 +26,11 @@ module('Unit | Authenticator | password', function (hooks) { test('it adds an authorization header to application adapter on restore', async function (assert) { assert.expect(1); - const mockData = { id: 'token1234', token: 'token1234' }; - this.server.get( - authenticator.buildTokenValidationEndpointURL(mockData.id), - () => { - return new Response(200); - } - ); + const id = 'token1234'; + const mockData = { attributes: { id, token: 'token1234' } }; + this.server.get(authenticator.buildTokenValidationEndpointURL(id), () => { + return new Response(200); + }); authenticator.restore(mockData); assert.equal(applicationAdapter.headers.Authorization, 'Bearer token1234'); }); diff --git a/ui/admin/app/routes/scopes/scope/authenticate/method.js b/ui/admin/app/routes/scopes/scope/authenticate/method.js index 7adf58b98..302f405a1 100644 --- a/ui/admin/app/routes/scopes/scope/authenticate/method.js +++ b/ui/admin/app/routes/scopes/scope/authenticate/method.js @@ -66,7 +66,7 @@ export default class ScopesScopeAuthenticateMethodRoute extends Route { // 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.authorization_request_url); + await this.openExternalOIDCFlow(json.attributes.authorization_request_url); } /** diff --git a/ui/desktop/app/authenticators/password.js b/ui/desktop/app/authenticators/password.js deleted file mode 100644 index f9d97033e..000000000 --- a/ui/desktop/app/authenticators/password.js +++ /dev/null @@ -1,98 +0,0 @@ -import BasePasswordAuthenticator from 'auth/authenticators/password'; -import { inject as service } from '@ember/service'; - -/** - * A username/password authenticator that authenticates with - * a scope/auth-method endpoint. Authentication occurs on an auth method - * endpoint, while deauthentication occurs on a scope endpoint. - */ -export default class PasswordAuthenticator extends BasePasswordAuthenticator { - // =services - - @service store; - - // =attributes - - /** - * Generates an auth method URL with which to authenticate. - * @override - * @param {object} options - * @param {string} options.scope.scope_id - * @param {string} options.authMethod.id - * @return {string} - */ - buildAuthEndpointURL({ - scope: { id: scopeID }, - authMethod: { id: authMethodID }, - }) { - const adapter = this.store.adapterFor('application'); - const options = { adapterOptions: { scopeID, method: 'authenticate' } }; - return adapter.buildURL('auth-method', authMethodID, options, 'findRecord'); - } - - /** - * Generates a scope URL with which to deauthenticate. - * @override - * @param {object} options - * @param {string} scopeID - * @return {string} - */ - buildDeauthEndpointURL({ scope: { id: scopeID } }) { - const adapter = this.store.adapterFor('application'); - const options = { adapterOptions: { method: 'deauthenticate' } }; - return adapter.buildURL('scope', scopeID, options, 'findRecord'); - } - - /** - * Generates an auth token validation URL used to check tokens on restoration. - * @override - * @param {string} tokenID - * @return {string} - */ - buildTokenValidationEndpointURL(tokenID) { - const adapter = this.store.adapterFor('auth-token'); - return adapter.buildURL('auth-token', tokenID, {}, 'findRecord'); - } - - /** - * Intercepts the authenticate response, if any, and assigns the returned - * token to all future requests via `addTokenToAuthorization`. - * Returns the response data as normal. - * @override - */ - authenticate() { - return super.authenticate(...arguments).then((data) => { - const token = data?.token; - this.addTokenToAuthorization(token); - return data; - }); - } - - /** - * When restoring the session (say, on load), assigns the session token - * to all future requests via `addTokenToAuthorization`. - * @override - * @param {object} data - * @return {object} - */ - restore(data) { - const token = data?.token; - this.addTokenToAuthorization(token); - return super.restore(data); - } - - /** - * Assigns a token string all future requests via the `Authorization` header - * on the application adapter prototype. Only application and adapters that - * extend the application adapter receive this header (which is most). - * @param {string} token - */ - addTokenToAuthorization(token) { - const adapterPrototype = this.store.adapterFor('application').constructor - .prototype; - const headers = adapterPrototype?.headers; - if (!headers) adapterPrototype.headers = {}; - adapterPrototype.headers.Authorization = null; - adapterPrototype.headers.Authorization = `Bearer ${token}`; - } -} diff --git a/ui/desktop/tests/unit/authenticators/password-test.js b/ui/desktop/tests/unit/authenticators/password-test.js deleted file mode 100644 index 35e872b48..000000000 --- a/ui/desktop/tests/unit/authenticators/password-test.js +++ /dev/null @@ -1,53 +0,0 @@ -import { module, test } from 'qunit'; -import { setupTest } from 'ember-qunit'; -import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; -import { Response } from 'miragejs'; - -module('Unit | Authenticator | password', function (hooks) { - setupTest(hooks); - setupMirage(hooks); - - let store; - let applicationAdapter; - let session; - let authenticator; - - hooks.beforeEach(function () { - store = this.owner.lookup('service:store'); - applicationAdapter = store.adapterFor('application'); - session = this.owner.lookup('service:session'); - authenticator = this.owner.lookup('authenticator:password'); - session.invalidate(); - }); - - hooks.afterEach(function () { - session.invalidate(); - }); - - test('it adds an authorization header to application adapter on restore', async function (assert) { - assert.expect(1); - const mockData = { id: 'token1234', token: 'token1234' }; - this.server.get( - authenticator.buildTokenValidationEndpointURL(mockData.id), - () => { - return new Response(200); - } - ); - authenticator.restore(mockData); - assert.equal(applicationAdapter.headers.Authorization, 'Bearer token1234'); - }); - - test('it adds an authorization header to application adapter on authenticate', async function (assert) { - assert.expect(1); - const creds = { identification: 'foo', password: 'bar' }; - const scope = { id: 'global' }; - const authMethod = { id: 'paum_1234' }; - this.server.create('scope', { id: 'global', type: 'global' }); - this.server.create('auth-method', { id: authMethod.id, scopeId: 'global' }); - await authenticator.authenticate(creds, null, { scope, authMethod }); - assert.equal( - applicationAdapter.headers.Authorization, - 'Bearer thetokenstring' - ); - }); -});