From b6a811cbbd8bc5b38a9297d736084dd82f8ff15a Mon Sep 17 00:00:00 2001 From: Dharini Jeeva Date: Mon, 18 Jul 2022 09:08:09 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20=F0=9F=90=9B=20filter=20out=20scopes=20w?= =?UTF-8?q?ith=20no=20auth=20methods=20(#1115)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: 🐛 filter out scopes with no authenticatable auth methods --- addons/api/mirage/config.js | 19 ++++++++-- addons/api/mirage/factories/auth-method.js | 1 + .../controllers/scopes/scope/authenticate.js | 1 + .../app/routes/scopes/scope/authenticate.js | 36 ++++++++++++++++--- .../templates/scopes/scope/authenticate.hbs | 16 ++------- .../acceptance/accounts/set-password-test.js | 6 +++- .../tests/acceptance/authentication-test.js | 10 ------ .../credential-library/update-test.js | 5 ++- .../credential-store/update-test.js | 5 ++- .../acceptance/managed-groups/create-test.js | 5 ++- .../acceptance/managed-groups/delete-test.js | 5 ++- .../tests/acceptance/roles/grants-test.js | 25 ++++++++++--- 12 files changed, 94 insertions(+), 40 deletions(-) diff --git a/addons/api/mirage/config.js b/addons/api/mirage/config.js index bb969a04c..eb949a1a8 100644 --- a/addons/api/mirage/config.js +++ b/addons/api/mirage/config.js @@ -57,8 +57,23 @@ export default function () { this.get( '/auth-methods', - ({ authMethods }, { queryParams: { scope_id: scopeId, filter } }) => { - const resultSet = authMethods.where({ scopeId }); + ({ authMethods }, { queryParams: { scope_id, recursive, filter } }) => { + let resultSet; + if (recursive && scope_id === 'global') { + resultSet = authMethods.all(); + } else if (recursive) { + resultSet = authMethods.where((authMethod) => { + const authMethodModel = authMethods.find(authMethod.id); + return ( + authMethod.scopeId === scope_id || + authMethodModel?.scope?.scope?.id === scope_id + ); + }); + } else { + resultSet = authMethods.where( + (authMethod) => authMethod.scopeId === scope_id + ); + } return resultSet.filter(makeBooleanFilter(filter)); } ); diff --git a/addons/api/mirage/factories/auth-method.js b/addons/api/mirage/factories/auth-method.js index f89e752e3..7057ef708 100644 --- a/addons/api/mirage/factories/auth-method.js +++ b/addons/api/mirage/factories/auth-method.js @@ -12,6 +12,7 @@ export default factory.extend({ 'read', 'update', 'delete', + 'authenticate', ], authorized_collection_actions: () => { return { diff --git a/ui/admin/app/controllers/scopes/scope/authenticate.js b/ui/admin/app/controllers/scopes/scope/authenticate.js index 25ebcd1df..4e6070cb0 100644 --- a/ui/admin/app/controllers/scopes/scope/authenticate.js +++ b/ui/admin/app/controllers/scopes/scope/authenticate.js @@ -7,6 +7,7 @@ export default class ScopesScopeAuthenticateController extends Controller { * Moves the global scope to index 0. * @type {Array} */ + get sortedScopes() { return [ ...this.model.scopes.filter((scope) => scope.id === 'global'), diff --git a/ui/admin/app/routes/scopes/scope/authenticate.js b/ui/admin/app/routes/scopes/scope/authenticate.js index 9f1130ce6..58fe2ae70 100644 --- a/ui/admin/app/routes/scopes/scope/authenticate.js +++ b/ui/admin/app/routes/scopes/scope/authenticate.js @@ -7,7 +7,7 @@ export default class ScopesScopeAuthenticateRoute extends Route { @service session; @service router; - + @service resourceFilterStore; // =methods beforeModel() { @@ -20,13 +20,39 @@ export default class ScopesScopeAuthenticateRoute extends Route { * scope and all scopes (for org navigation). * @return {Promise} `{scope, scopes, authMethods}` */ - model() { + async model() { const { id: scope_id } = this.modelFor('scopes.scope'); + // Fetch auth methods for the current scope + const authMethods = await this.resourceFilterStore.queryBy( + 'auth-method', + { + authorized_actions: [{ contains: 'authenticate' }], + }, + { + scope_id, + } + ); + // Preload all authenticatable auth methods into the store + const authMethodsForAllScopes = await this.resourceFilterStore.queryBy( + 'auth-method', + { + authorized_actions: [{ contains: 'authenticate' }], + }, + { + scope_id: 'global', + recursive: true, + } + ); + // Fetch org scopes + // and filter out any that have no auth methods + const scopes = this.modelFor('scopes').filter( + ({ id: scope_id, isOrg }) => + isOrg && authMethodsForAllScopes.filterBy('scopeID', scope_id).length + ); return hash({ scope: this.modelFor('scopes.scope'), - scopes: this.modelFor('scopes').filter((scope) => scope.isOrg), - authMethods: this.store.query('auth-method', { scope_id }), - + scopes, + authMethods, // for integration testing: // authMethods: A([{ // id: 'am_1234567890', diff --git a/ui/admin/app/templates/scopes/scope/authenticate.hbs b/ui/admin/app/templates/scopes/scope/authenticate.hbs index e7179d3e1..0a73f858e 100644 --- a/ui/admin/app/templates/scopes/scope/authenticate.hbs +++ b/ui/admin/app/templates/scopes/scope/authenticate.hbs @@ -14,6 +14,7 @@ 'auth.descriptions.authenticating-with-org' name=@model.scope.displayName }}

+ {{t 'titles.global'}} + {{#each this.sortedScopes as |scope|}} {{scope.displayName}} + {{/each}} @@ -75,19 +78,6 @@ {{#if @model.authMethods}} {{outlet}} {{/if}} - {{#unless @model.authMethods}} - - - - {{t 'resources.auth-method.messages.none.description'}} - - - - {{/unless}} diff --git a/ui/admin/tests/acceptance/accounts/set-password-test.js b/ui/admin/tests/acceptance/accounts/set-password-test.js index daded2a75..c233d4c16 100644 --- a/ui/admin/tests/acceptance/accounts/set-password-test.js +++ b/ui/admin/tests/acceptance/accounts/set-password-test.js @@ -86,7 +86,11 @@ module('Acceptance | accounts | set password', function (hooks) { '/accounts/:idMethod', (_, { params: { idMethod }, requestBody }) => { const attrs = JSON.parse(requestBody); - assert.strictEqual(attrs.password, 'update password', 'new password is set'); + assert.strictEqual( + attrs.password, + 'update password', + 'new password is set' + ); const id = idMethod.split(':')[0]; return { id }; } diff --git a/ui/admin/tests/acceptance/authentication-test.js b/ui/admin/tests/acceptance/authentication-test.js index 9e8b28872..df8b7ad9b 100644 --- a/ui/admin/tests/acceptance/authentication-test.js +++ b/ui/admin/tests/acceptance/authentication-test.js @@ -113,16 +113,6 @@ module('Acceptance | authentication', function (hooks) { assert.strictEqual(currentURL(), authMethodAuthenticateURL); }); - 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.strictEqual(currentURL(), authenticateURL); - assert.ok(find('.rose-message')); - }); - test('visiting auth method when the scope cannot be loaded is still allowed', async function (assert) { assert.expect(1); this.server.get('/scopes', () => { diff --git a/ui/admin/tests/acceptance/credential-library/update-test.js b/ui/admin/tests/acceptance/credential-library/update-test.js index b1abad358..b608128b7 100644 --- a/ui/admin/tests/acceptance/credential-library/update-test.js +++ b/ui/admin/tests/acceptance/credential-library/update-test.js @@ -94,7 +94,10 @@ module('Acceptance | credential-libraries | update', function (hooks) { this.server.schema.credentialLibraries.all().models[0].name, 'random string' ); - assert.strictEqual(find('[name="name"]').value, instances.credentialLibrary.name); + assert.strictEqual( + find('[name="name"]').value, + instances.credentialLibrary.name + ); }); test('saving an existing credential library with invalid fields displays error messages', async function (assert) { diff --git a/ui/admin/tests/acceptance/credential-store/update-test.js b/ui/admin/tests/acceptance/credential-store/update-test.js index ff5c00b90..1aea5038a 100644 --- a/ui/admin/tests/acceptance/credential-store/update-test.js +++ b/ui/admin/tests/acceptance/credential-store/update-test.js @@ -83,7 +83,10 @@ module('Acceptance | credential-stores | update', function (hooks) { await fillIn('[name="name"]', 'random string'); await click('.rose-form-actions [type="button"]'); assert.notEqual(instances.credentialStore.name, 'random string'); - assert.strictEqual(find('[name="name"]').value, instances.credentialStore.name); + assert.strictEqual( + find('[name="name"]').value, + instances.credentialStore.name + ); }); test('saving an existing credential store with invalid fields displays error messages', async function (assert) { diff --git a/ui/admin/tests/acceptance/managed-groups/create-test.js b/ui/admin/tests/acceptance/managed-groups/create-test.js index dac708c21..5926062d5 100644 --- a/ui/admin/tests/acceptance/managed-groups/create-test.js +++ b/ui/admin/tests/acceptance/managed-groups/create-test.js @@ -71,7 +71,10 @@ module('Acceptance | managed-groups | create', function (hooks) { await fillIn('[name="name"]', 'Managed group name'); await fillIn('[name="description"]', 'description'); await click('form [type="submit"]:not(:disabled)'); - assert.strictEqual(this.server.db.managedGroups.length, managedGroupsCount + 1); + assert.strictEqual( + this.server.db.managedGroups.length, + managedGroupsCount + 1 + ); }); test('User can not create a new managed group without proper authorization', async function (assert) { diff --git a/ui/admin/tests/acceptance/managed-groups/delete-test.js b/ui/admin/tests/acceptance/managed-groups/delete-test.js index 99e3bdb9b..15ec28ce1 100644 --- a/ui/admin/tests/acceptance/managed-groups/delete-test.js +++ b/ui/admin/tests/acceptance/managed-groups/delete-test.js @@ -63,7 +63,10 @@ module('Acceptance | managed-groups | delete', function (hooks) { const managedGroupsCount = this.server.db.managedGroups.length; await visit(urls.managedGroup); await click('.rose-layout-page-actions .rose-dropdown-button-danger'); - assert.strictEqual(this.server.db.managedGroups.length, managedGroupsCount - 1); + assert.strictEqual( + this.server.db.managedGroups.length, + managedGroupsCount - 1 + ); }); test('User cannot delete a managed-group without proper authorization', async function (assert) { diff --git a/ui/admin/tests/acceptance/roles/grants-test.js b/ui/admin/tests/acceptance/roles/grants-test.js index 2161e1844..d323f86b3 100644 --- a/ui/admin/tests/acceptance/roles/grants-test.js +++ b/ui/admin/tests/acceptance/roles/grants-test.js @@ -60,7 +60,10 @@ module('Acceptance | roles | grants', function (hooks) { await visit(urls.grants); await a11yAudit(); assert.strictEqual(currentURL(), urls.grants); - assert.strictEqual(findAll(`${grantsForm} [name="grant"]`).length, grantsCount); + assert.strictEqual( + findAll(`${grantsForm} [name="grant"]`).length, + grantsCount + ); }); test('cannot set grants without proper authorization', async function (assert) { @@ -129,7 +132,10 @@ module('Acceptance | roles | grants', function (hooks) { ); }); await visit(urls.grants); - assert.strictEqual(findAll(`${grantsForm} [name="grant"]`).length, grantsCount); + assert.strictEqual( + findAll(`${grantsForm} [name="grant"]`).length, + grantsCount + ); await fillIn(`${grantsForm} [name="grant"]`, 'id=123,action=delete'); await click('.rose-form-actions [type="submit"]:not(:disabled)'); assert.ok(find('[role="alert"]')); @@ -180,7 +186,10 @@ module('Acceptance | roles | grants', function (hooks) { ); }); await visit(urls.grants); - assert.strictEqual(findAll(`${grantsForm} [name="grant"]`).length, grantsCount); + assert.strictEqual( + findAll(`${grantsForm} [name="grant"]`).length, + grantsCount + ); await fillIn(`${newGrantForm} [name="grant"]`, 'id=123,action=delete'); await click(`${newGrantForm} [type="submit"]:not(:disabled)`); await click('.rose-form-actions [type="submit"]:not(:disabled)'); @@ -203,7 +212,10 @@ module('Acceptance | roles | grants', function (hooks) { await visit(urls.grants); await click(`${grantsForm} button`); await click('.rose-form-actions button:not([type="submit"])'); - assert.strictEqual(findAll(`${grantsForm} [name="grant"]`).length, grantsCount); + assert.strictEqual( + findAll(`${grantsForm} [name="grant"]`).length, + grantsCount + ); }); test('shows error message on grant remove', async function (assert) { @@ -221,7 +233,10 @@ module('Acceptance | roles | grants', function (hooks) { ); }); await visit(urls.grants); - assert.strictEqual(findAll(`${grantsForm} [name="grant"]`).length, grantsCount); + assert.strictEqual( + findAll(`${grantsForm} [name="grant"]`).length, + grantsCount + ); await click(`${grantsForm} button:not([type="submit"])`); await click('.rose-form-actions [type="submit"]:not(:disabled)'); assert.ok(find('[role="alert"]'));