Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filter Auth methods by name or type #20747

Merged
merged 9 commits into from
May 26, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/20747.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui: Add filtering by auth type and auth name to the Authentication Method list view.
```
76 changes: 63 additions & 13 deletions ui/app/controllers/vault/cluster/access/methods.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,72 @@
*/

import Controller from '@ember/controller';
import { task } from 'ember-concurrency';
import { dropTask } from 'ember-concurrency';
import { inject as service } from '@ember/service';
import { action } from '@ember/object';
import { tracked } from '@glimmer/tracking';

export default Controller.extend({
flashMessages: service(),
export default class VaultClusterAccessMethodsController extends Controller {
@service flashMessages;

queryParams: {
page: 'page',
pageFilter: 'pageFilter',
},
@tracked authMethodOptions = [];
@tracked selectedAuthType = null;
@tracked selectedAuthName = null;

page: 1,
pageFilter: null,
filter: null,
queryParams = ['page, pageFilter'];

disableMethod: task(function* (method) {
page = 1;
pageFilter = null;
filter = null;

get authMethodList() {
// return an options list to filter by engine type, ex: 'kv'
if (this.selectedAuthType) {
// check first if the user has also filtered by name.
if (this.selectedAuthName) {
return this.model.filter((method) => this.selectedAuthName === method.id);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious if we should be filtering by both type and name if they are both set? I think there could be a case where the same name is used for different types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For secret engines you can't have the same name as used by another secret engine. We do some validation work on that and say as much. And if they passed validation, then the API would return the same error.

But great question for auth methods. I had to check if this was the same. Turns out it is. We don't do any validation but we do return the API error.
image

So names are individualized across all auth methods. Which I believes answers your question?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok good to know thanks! Maybe nice to add a comment in there stating as much so it's clear why type is ignored if name is set.

// otherwise filter by auth type
return this.model.filter((method) => this.selectedAuthType === method.type);
}
// return an options list to filter by auth name, ex: 'my-userpass'
if (this.selectedAuthName) {
return this.model.filter((method) => this.selectedAuthName === method.id);
}
// no filters, return full sorted list.
return this.model;
}

get authMethodArrayByType() {
const arrayOfAllAuthTypes = this.authMethodList.map((modelObject) => modelObject.type);
// filter out repeated auth types (e.g. [userpass, userpass] => [userpass])
const arrayOfUniqueAuthTypes = [...new Set(arrayOfAllAuthTypes)];

return arrayOfUniqueAuthTypes.map((authType) => ({
name: authType,
id: authType,
}));
}

get authMethodArrayByName() {
return this.authMethodList.map((modelObject) => ({
name: modelObject.id,
id: modelObject.id,
}));
}

@action
filterAuthType([type]) {
this.selectedAuthType = type;
}

@action
filterAuthName([name]) {
this.selectedAuthName = name;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker but it seems like some of this filtering logic could be moved to a decorator or utility so that it can be shared since this is the second instance of this functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thought. The only issue is that the sorted list that is passed around is pretty unique. For KV we need to combine and filter two lists (supported and unsupported) and here we're just using the this.model and filtering there. With KV the unique list situation meant a couple of extra lines of code not seen here. But it's a great consideration. I'll think about this with the KV work as the new models may allow us to standardize the list.


@dropTask
*disableMethod(method) {
const { type, path } = method;
try {
yield method.destroyRecord();
Expand All @@ -29,5 +79,5 @@ export default Controller.extend({
`There was an error disabling Auth Method at ${path}: ${err.errors.join(' ')}.`
);
}
}).drop(),
});
}
}
12 changes: 6 additions & 6 deletions ui/app/routes/vault/cluster/access/methods.js
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldn't help myself. I opened the file and had to glimmerize.

Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@
import Route from '@ember/routing/route';
import { inject as service } from '@ember/service';

export default Route.extend({
store: service(),
export default class VaultClusterAccessMethodsRoute extends Route {
@service store;

queryParams: {
queryParams = {
page: {
refreshModel: true,
},
pageFilter: {
refreshModel: true,
},
},
};

model() {
return this.store.findAll('auth-method');
},
});
}
}
29 changes: 28 additions & 1 deletion ui/app/templates/vault/cluster/access/methods.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,41 @@
</PageHeader>

<Toolbar>
<ToolbarFilters>
<SearchSelect
@id="filter-by-auth-type"
@options={{this.authMethodArrayByType}}
@selectLimit="1"
@disallowNewItems={{true}}
@fallbackComponent="input-search"
@onChange={{this.filterAuthType}}
@placeholder={{"Filter by auth type"}}
@displayInherit={{true}}
@inputValue={{if this.selectedAuthType (array this.selectedAuthType)}}
@disabled={{if this.selectedAuthName true false}}
class="is-marginless"
/>
<SearchSelect
@id="filter-by-auth-name"
@options={{this.authMethodArrayByName}}
@selectLimit="1"
@disallowNewItems={{true}}
@fallbackComponent="input-search"
@onChange={{this.filterAuthName}}
@placeholder={{"Filter by auth name"}}
@displayInherit={{true}}
@inputValue={{if this.selectedAuthName (array this.selectedAuthName)}}
class="is-marginless has-left-padding-s"
/>
</ToolbarFilters>
<ToolbarActions>
<ToolbarLink @route="vault.cluster.settings.auth.enable" @type="add" data-test-auth-enable>
Enable new method
</ToolbarLink>
</ToolbarActions>
</Toolbar>

{{#each (sort-by "path" this.model) as |method|}}
{{#each (sort-by "path" this.authMethodList) as |method|}}
<LinkedBlock
@params={{array "vault.cluster.access.method" method.id}}
class="list-item-row"
Expand Down
53 changes: 51 additions & 2 deletions ui/tests/acceptance/access/methods-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,71 @@
*/

import { currentRouteName } from '@ember/test-helpers';
import { clickTrigger } from 'ember-power-select/test-support/helpers';
import { module, test } from 'qunit';
import { setupApplicationTest } from 'ember-qunit';
import { create } from 'ember-cli-page-object';
import page from 'vault/tests/pages/access/methods';
import authEnable from 'vault/tests/pages/settings/auth/enable';
import authPage from 'vault/tests/pages/auth';
import ss from 'vault/tests/pages/components/search-select';
import consoleClass from 'vault/tests/pages/components/console/ui-panel';

module('Acceptance | /access/', function (hooks) {
import { v4 as uuidv4 } from 'uuid';

const consoleComponent = create(consoleClass);
const searchSelect = create(ss);

module('Acceptance | auth-methods list view', function (hooks) {
setupApplicationTest(hooks);

hooks.beforeEach(function () {
this.uid = uuidv4();
return authPage.login();
});

test('it navigates', async function (assert) {
test('it navigates to auth method', async function (assert) {
await page.visit();
assert.strictEqual(currentRouteName(), 'vault.cluster.access.methods', 'navigates to the correct route');
assert.ok(page.methodsLink.isActive, 'the first link is active');
assert.strictEqual(page.methodsLink.text, 'Authentication methods');
});

test('it filters by name and auth type', async function (assert) {
assert.expect(4);
const authPath1 = `userpass-1-${this.uid}`;
const authPath2 = `userpass-2-${this.uid}`;
const type = 'userpass';
await authEnable.visit();
await authEnable.enable(type, authPath1);
await authEnable.visit();
await authEnable.enable(type, authPath2);
await page.visit();
// filter by auth type

await clickTrigger('#filter-by-auth-type');
await searchSelect.options.objectAt(0).click();

const rows = document.querySelectorAll('[data-test-auth-backend-link]');
const rowsUserpass = Array.from(rows).filter((row) => row.innerText.includes('userpass'));

assert.strictEqual(rows.length, rowsUserpass.length, 'all rows returned are userpass');

// filter by name
await clickTrigger('#filter-by-auth-name');
const firstItemToSelect = searchSelect.options.objectAt(0).text;
await searchSelect.options.objectAt(0).click();
const singleRow = document.querySelectorAll('[data-test-auth-backend-link]');

assert.strictEqual(singleRow.length, 1, 'returns only one row');
assert.dom(singleRow[0]).includesText(firstItemToSelect, 'shows the filtered by auth name');
// clear filter by engine name
await searchSelect.deleteButtons.objectAt(1).click();
const rowsAgain = document.querySelectorAll('[data-test-auth-backend-link]');
assert.ok(rowsAgain.length > 1, 'filter has been removed');

// cleanup
await consoleComponent.runCommands([`delete sys/auth/${authPath1}`]);
await consoleComponent.runCommands([`delete sys/auth/${authPath2}`]);
});
});
9 changes: 5 additions & 4 deletions ui/tests/acceptance/secrets/backend/engines-test.js
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed I needed to clean this up from my last test writing for the secret engine filtering.

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { currentRouteName, settled } from '@ember/test-helpers';
import { clickTrigger } from 'ember-power-select/test-support/helpers';
import { create } from 'ember-cli-page-object';
import { module, test } from 'qunit';
import { runCommands } from 'vault/tests/helpers/pki/pki-run-commands';
import consoleClass from 'vault/tests/pages/components/console/ui-panel';
import { setupApplicationTest } from 'ember-qunit';
import { v4 as uuidv4 } from 'uuid';

Expand All @@ -16,6 +16,7 @@ import backendsPage from 'vault/tests/pages/secrets/backends';
import authPage from 'vault/tests/pages/auth';
import ss from 'vault/tests/pages/components/search-select';

const consoleComponent = create(consoleClass);
const searchSelect = create(ss);

module('Acceptance | secret-engine list view', function (hooks) {
Expand Down Expand Up @@ -76,7 +77,7 @@ module('Acceptance | secret-engine list view', function (hooks) {
assert.dom(rowSupported[0]).hasClass('linked-block', `linked-block class is added to supported engines.`);

// cleanup
await runCommands([`delete sys/mounts/${enginePath}`]);
await consoleComponent.runCommands([`delete sys/mounts/${enginePath}`]);
});

test('it filters by name and engine type', async function (assert) {
Expand Down Expand Up @@ -109,7 +110,7 @@ module('Acceptance | secret-engine list view', function (hooks) {
assert.ok(rowsAgain.length > 1, 'filter has been removed');

// cleanup
await runCommands([`delete sys/mounts/${enginePath1}`]);
await runCommands([`delete sys/mounts/${enginePath2}`]);
await consoleComponent.runCommands([`delete sys/mounts/${enginePath1}`]);
await consoleComponent.runCommands([`delete sys/mounts/${enginePath2}`]);
});
});