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

UI: Align auth method ttl with tune value #26663

Merged
Merged
Show file tree
Hide file tree
Changes from all 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/26663.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui: Show computed values from `sys/internal/ui/mounts` endpoint for auth mount configuration view
```
18 changes: 12 additions & 6 deletions ui/app/adapters/auth-method.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,26 @@ export default ApplicationAdapter.extend({

findAll(store, type, sinceToken, snapshotRecordArray) {
const isUnauthenticated = snapshotRecordArray?.adapterOptions?.unauthenticated;
if (isUnauthenticated) {
// sys/internal/ui/mounts returns the actual value of the system TTL
// instead of '0' which just indicates the mount is using system defaults
const useMountsEndpoint = snapshotRecordArray?.adapterOptions?.useMountsEndpoint;
if (isUnauthenticated || useMountsEndpoint) {
const url = `/${this.urlPrefix()}/internal/ui/mounts`;
return this.ajax(url, 'GET', {
unauthenticated: true,
unauthenticated: isUnauthenticated,
})
.then((result) => {
return {
data: result.data.auth,
};
})
.catch(() => {
return {
data: {},
};
.catch((e) => {
if (isUnauthenticated) return { data: {} };

if (e instanceof AdapterError) {
set(e, 'policyPath', 'sys/internal/ui/mounts');
}
throw e;
});
}
return this.ajax(this.url(), 'GET').catch((e) => {
Expand Down
40 changes: 21 additions & 19 deletions ui/app/routes/vault/cluster/access/method.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,28 @@ export default Route.extend({

model(params) {
const { path } = params;
return this.store.findAll('auth-method').then((modelArray) => {
const model = modelArray.find((m) => m.id === path);
if (!model) {
const error = new AdapterError();
set(error, 'httpStatus', 404);
throw error;
}
const supportManaged = supportedManagedAuthBackends();
if (!supportManaged.includes(model.methodType)) {
// do not fetch path-help for unmanaged auth types
model.set('paths', {
apiPath: model.apiPath,
paths: [],
return this.store
Copy link
Contributor

Choose a reason for hiding this comment

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

cleanup suggestion: this might read a little cleaner as async/await

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer that syntax as well, but I think I'll leave it is to minimize changes in this PR. Which reminds me that I wanted to ask Finn if this should be backported.

.findAll('auth-method', { adapterOptions: { useMountsEndpoint: true } })
.then((modelArray) => {
const model = modelArray.find((m) => m.id === path);
if (!model) {
const error = new AdapterError();
set(error, 'httpStatus', 404);
throw error;
}
const supportManaged = supportedManagedAuthBackends();
if (!supportManaged.includes(model.methodType)) {
// do not fetch path-help for unmanaged auth types
model.set('paths', {
apiPath: model.apiPath,
paths: [],
});
return model;
}
return this.pathHelp.getPaths(model.apiPath, path).then((paths) => {
model.set('paths', paths);
return model;
});
return model;
}
return this.pathHelp.getPaths(model.apiPath, path).then((paths) => {
model.set('paths', paths);
return model;
});
});
},
});
2 changes: 2 additions & 0 deletions ui/app/templates/components/auth-method/configuration.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@
@alwaysRender={{not (is-empty-value (get @model attr.name))}}
@label={{or attr.options.label (to-label attr.name)}}
@value={{stringify (get @model attr.name)}}
@formatTtl={{eq attr.options.editType "ttl"}}
/>
{{else}}
<InfoTableRow
@alwaysRender={{not (is-empty-value (get @model attr.name))}}
@label={{or attr.options.label (to-label attr.name)}}
@value={{get @model attr.name}}
@formatTtl={{eq attr.options.editType "ttl"}}
/>
{{/if}}
{{/each}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<PageHeader as |p|>
<p.top>
<Hds::Breadcrumb>
<Hds::Breadcrumb data-test-breadcrumbs>
<Hds::Breadcrumb::Item @text="Auth Methods" @route="vault.cluster.access.methods" />
<Hds::Breadcrumb::Item @text={{this.model.id}} @route="vault.cluster.access.method" @model={{this.model.id}} />
<Hds::Breadcrumb::Item @text="Configure" @current={{true}} />
Expand Down
25 changes: 24 additions & 1 deletion ui/tests/acceptance/settings/auth/enable-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
* SPDX-License-Identifier: BUSL-1.1
*/

import { currentRouteName, settled } from '@ember/test-helpers';
import { click, currentRouteName, settled } from '@ember/test-helpers';
import { module, test } from 'qunit';
import { setupApplicationTest } from 'ember-qunit';
import { v4 as uuidv4 } from 'uuid';

import { GENERAL } from 'vault/tests/helpers/general-selectors';
import page from 'vault/tests/pages/settings/auth/enable';
import listPage from 'vault/tests/pages/access/methods';
import authPage from 'vault/tests/pages/auth';
Expand Down Expand Up @@ -42,4 +43,26 @@ module('Acceptance | settings/auth/enable', function (hooks) {
await listPage.visit();
assert.ok(listPage.findLinkById(path), 'mount is present in the list');
});

test('it renders default config details', async function (assert) {
const path = `approle-config-${this.uid}`;
const type = 'approle';
await page.visit();
await page.enable(type, path);
// the config details is updated to query mount details from sys/internal/ui/mounts
// but we still want these forms to continue using sys/auth which returns 0 for default ttl values
// check tune form (right after enabling)
assert.dom(GENERAL.toggleInput('Default Lease TTL')).isNotChecked('default lease ttl is unset');
assert.dom(GENERAL.toggleInput('Max Lease TTL')).isNotChecked('max lease ttl is unset');
await click(GENERAL.breadcrumbAtIdx(1));
assert
.dom(GENERAL.infoRowValue('Default Lease TTL'))
.hasText('1 month 1 day', 'shows system default TTL');
assert.dom(GENERAL.infoRowValue('Max Lease TTL')).hasText('1 month 1 day', 'shows the proper max TTL');

// check edit form TTL values
await click('[data-test-configure-link]');
assert.dom(GENERAL.toggleInput('Default Lease TTL')).isNotChecked('default lease ttl is still unset');
assert.dom(GENERAL.toggleInput('Max Lease TTL')).isNotChecked('max lease ttl is still unset');
});
});
7 changes: 4 additions & 3 deletions ui/tests/helpers/general-selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@ export const GENERAL = {
menuTrigger: '[data-test-popup-menu-trigger]',
listItem: '[data-test-list-item-link]',
// FORMS
checkboxByAttr: (attr: string) => `[data-test-checkbox="${attr}"]`,
enableField: (attr: string) => `[data-test-enable-field="${attr}"] button`,
fieldByAttr: (attr: string) => `[data-test-field="${attr}"]`,
infoRowLabel: (label: string) => `[data-test-row-label="${label}"]`,
infoRowValue: (label: string) => `[data-test-value-div="${label}"]`,
inputByAttr: (attr: string) => `[data-test-input="${attr}"]`,
selectByAttr: (attr: string) => `[data-test-select="${attr}"]`,
checkboxByAttr: (attr: string) => `[data-test-checkbox="${attr}"]`,
fieldByAttr: (attr: string) => `[data-test-field="${attr}"]`,
enableField: (attr: string) => `[data-test-enable-field="${attr}"] button`,
toggleInput: (attr: string) => `[data-test-toggle-input="${attr}"]`,
ttl: {
toggle: (attr: string) => `[data-test-toggle-label="${attr}"]`,
input: (attr: string) => `[data-test-ttl-value="${attr}"]`,
Expand Down
67 changes: 67 additions & 0 deletions ui/tests/unit/adapters/auth-method-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/**
* Copyright (c) HashiCorp, Inc.
* SPDX-License-Identifier: BUSL-1.1
*/

import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';
import { setupMirage } from 'ember-cli-mirage/test-support';

module('Unit | Adapter | auth method', function (hooks) {
setupTest(hooks);
setupMirage(hooks);

hooks.beforeEach(async function () {
this.store = this.owner.lookup('service:store');
this.mockResponse = {
data: {
auth: {
'approle/': {
accessor: 'auth_approle_43e5a627',
config: {
default_lease_ttl: 2764800,
force_no_cache: false,
listing_visibility: 'hidden',
max_lease_ttl: 2764800,
token_type: 'default-service',
},
uuid: '7a8bc146-76d0-3a9c-9feb-47a6713a85b3',
},
},
},
};
});

test('findAll makes request to correct endpoint with no adapterOptions', async function (assert) {
assert.expect(1);

this.server.get('sys/auth', () => {
assert.ok(true, 'request made to sys/auth when no options are passed to findAll');
return { data: this.mockResponse.data.auth };
});

await this.store.findAll('auth-method');
});

test('findAll makes request to correct endpoint when unauthenticated is true', async function (assert) {
assert.expect(1);

this.server.get('sys/internal/ui/mounts', () => {
assert.ok(true, 'request made to correct endpoint when unauthenticated');
return this.mockResponse;
});

await this.store.findAll('auth-method', { adapterOptions: { unauthenticated: true } });
});

test('findAll makes request to correct endpoint when useMountsEndpoint is true', async function (assert) {
assert.expect(1);

this.server.get('sys/internal/ui/mounts', () => {
assert.ok(true, 'request made to correct endpoint when useMountsEndpoint');
return this.mockResponse;
});

await this.store.findAll('auth-method', { adapterOptions: { useMountsEndpoint: true } });
});
});
Loading