From 2ce68778e4af8619b5a0ddba4473fe316debcb5e Mon Sep 17 00:00:00 2001 From: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com> Date: Fri, 20 Sep 2024 13:44:29 -0500 Subject: [PATCH 1/8] UI: Fix LDAP Mirage Handler (#28432) * update ldap mirage scenario to allow check-in/check-out action * update libraries test to mount engine * update mirage, fix tests * update lease renew CLI command * fix test * update tests --- .../page/library/details/accounts.ts | 2 +- ui/mirage/handlers/ldap.js | 71 +++++++++++++++---- ui/mirage/models/ldap-account-status.js | 13 ++++ ui/mirage/scenarios/ldap.js | 13 +++- .../secrets/backend/ldap/libraries-test.js | 39 ++++++++-- .../secrets/backend/ldap/overview-test.js | 62 +++++++++++----- .../secrets/backend/ldap/roles-test.js | 36 +++++++--- ui/tests/helpers/ldap/ldap-helpers.js | 10 +-- .../page/library/details/accounts-test.js | 5 +- 9 files changed, 195 insertions(+), 56 deletions(-) create mode 100644 ui/mirage/models/ldap-account-status.js diff --git a/ui/lib/ldap/addon/components/page/library/details/accounts.ts b/ui/lib/ldap/addon/components/page/library/details/accounts.ts index 6e039f56893b..7e119375dbc0 100644 --- a/ui/lib/ldap/addon/components/page/library/details/accounts.ts +++ b/ui/lib/ldap/addon/components/page/library/details/accounts.ts @@ -27,7 +27,7 @@ export default class LdapLibraryDetailsAccountsPageComponent extends Component new Response(204)); - server.get('/sys/internal/ui/mounts/:path', () => ({ - data: { - accessor: 'ldap_ade94329', - type: 'ldap', - path: 'ldap-test/', - uuid: '35e9119d-5708-4b6b-58d2-f913e27f242d', - config: {}, - }, - })); // config server.post('/:backend/config', (schema, req) => createOrUpdateRecord(schema, req, 'ldapConfigs')); server.get('/:backend/config', (schema, req) => getRecord(schema, req, 'ldapConfigs')); @@ -67,8 +56,60 @@ export default function (server) { server.post('/:backend/library/:name', (schema, req) => createOrUpdateRecord(schema, req, 'ldapLibraries')); server.get('/:backend/library/:name', (schema, req) => getRecord(schema, req, 'ldapLibraries')); server.get('/:backend/library', (schema) => listRecords(schema, 'ldapLibraries')); - server.get('/:backend/library/:name/status', () => ({ - 'bob.johnson': { available: false, borrower_client_token: '8b80c305eb3a7dbd161ef98f10ea60a116ce0910' }, - 'mary.smith': { available: true }, - })); + server.get('/:backend/library/:name/status', (schema) => { + const data = schema.db['ldapAccountStatuses'].reduce((prev, curr) => { + prev[curr.account] = { + available: curr.available, + borrower_client_token: curr.borrower_client_token, + }; + return prev; + }, {}); + return { data }; + }); + // check-out / check-in + server.post('/:backend/library/:set_name/check-in', (schema, req) => { + // Check-in makes an unavailable account available again + const { service_account_names } = JSON.parse(req.requestBody); + const dbCollection = schema.db['ldapAccountStatuses']; + const updated = dbCollection.find(service_account_names).map((f) => ({ + ...f, + available: true, + borrower_client_token: undefined, + })); + updated.forEach((u) => { + dbCollection.update(u.id, u); + }); + return { + data: { + check_ins: service_account_names, + }, + }; + }); + server.post('/:backend/library/:set_name/check-out', (schema, req) => { + const { set_name, backend } = req.params; + const dbCollection = schema.db['ldapAccountStatuses']; + const available = dbCollection.where({ available: true }); + if (available) { + return Response(404, {}, { errors: ['no accounts available to check out'] }); + } + const checkOut = { + ...available[0], + available: false, + borrower_client_token: crypto.randomUUID(), + }; + dbCollection.update(checkOut.id, checkOut); + return { + request_id: '364a17d4-e5ab-998b-ceee-b49929229e0c', + lease_id: `${backend}/library/${set_name}/check-out/aoBsaBEI4PK96VnukubvYDlZ`, + renewable: true, + lease_duration: 36000, + data: { + password: crypto.randomUUID(), + service_account_name: checkOut.account, + }, + wrap_info: null, + warnings: null, + auth: null, + }; + }); } diff --git a/ui/mirage/models/ldap-account-status.js b/ui/mirage/models/ldap-account-status.js new file mode 100644 index 000000000000..557261f6cd78 --- /dev/null +++ b/ui/mirage/models/ldap-account-status.js @@ -0,0 +1,13 @@ +/** + * Copyright (c) HashiCorp, Inc. + * SPDX-License-Identifier: BUSL-1.1 + */ + +import { Model } from 'miragejs'; + +export default Model.extend({ + account: '', // should match ID + library: '', + available: false, + borrower_client_token: undefined, +}); diff --git a/ui/mirage/scenarios/ldap.js b/ui/mirage/scenarios/ldap.js index 19b6b61f4c58..d946dc68529d 100644 --- a/ui/mirage/scenarios/ldap.js +++ b/ui/mirage/scenarios/ldap.js @@ -4,8 +4,19 @@ */ export default function (server) { - server.create('ldap-config', { path: 'kubernetes' }); + server.create('ldap-config', { path: 'kubernetes', backend: 'ldap-test' }); server.create('ldap-role', 'static', { name: 'static-role' }); server.create('ldap-role', 'dynamic', { name: 'dynamic-role' }); server.create('ldap-library', { name: 'test-library' }); + server.create('ldap-account-status', { + id: 'bob.johnson', + account: 'bob.johnson', + available: false, + borrower_client_token: '8b80c305eb3a7dbd161ef98f10ea60a116ce0910', + }); + server.create('ldap-account-status', { + id: 'mary.smith', + account: 'mary.smith', + available: true, + }); } diff --git a/ui/tests/acceptance/secrets/backend/ldap/libraries-test.js b/ui/tests/acceptance/secrets/backend/ldap/libraries-test.js index dd3d0e0725a6..4e9cc5a57059 100644 --- a/ui/tests/acceptance/secrets/backend/ldap/libraries-test.js +++ b/ui/tests/acceptance/secrets/backend/ldap/libraries-test.js @@ -6,11 +6,13 @@ import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; import { setupMirage } from 'ember-cli-mirage/test-support'; +import { v4 as uuidv4 } from 'uuid'; import ldapMirageScenario from 'vault/mirage/scenarios/ldap'; import ldapHandlers from 'vault/mirage/handlers/ldap'; import authPage from 'vault/tests/pages/auth'; import { click } from '@ember/test-helpers'; import { isURL, visitURL } from 'vault/tests/helpers/ldap/ldap-helpers'; +import { deleteEngineCmd, mountEngineCmd, runCmd } from 'vault/tests/helpers/commands'; module('Acceptance | ldap | libraries', function (hooks) { setupApplicationTest(hooks); @@ -19,21 +21,41 @@ module('Acceptance | ldap | libraries', function (hooks) { hooks.beforeEach(async function () { ldapHandlers(this.server); ldapMirageScenario(this.server); + this.backend = `ldap-test-${uuidv4()}`; await authPage.login(); - return visitURL('libraries'); + // mount & configure + await runCmd([ + mountEngineCmd('ldap', this.backend), + `write ${this.backend}/config binddn=foo bindpass=bar url=http://localhost:8208`, + ]); + return visitURL('libraries', this.backend); + }); + + hooks.afterEach(async function () { + await runCmd(deleteEngineCmd(this.backend)); + }); + + test('it should show libraries on overview page', async function (assert) { + await visitURL('overview', this.backend); + assert.dom('[data-test-libraries-count]').hasText('1'); }); test('it should transition to create library route on toolbar link click', async function (assert) { await click('[data-test-toolbar-action="library"]'); - assert.true(isURL('libraries/create'), 'Transitions to library create route on toolbar link click'); + assert.true( + isURL('libraries/create', this.backend), + 'Transitions to library create route on toolbar link click' + ); }); test('it should transition to library details route on list item click', async function (assert) { await click('[data-test-list-item-link] a'); assert.true( - isURL('libraries/test-library/details/accounts'), + isURL('libraries/test-library/details/accounts', this.backend), 'Transitions to library details accounts route on list item click' ); + assert.dom('[data-test-account-name]').exists({ count: 2 }, 'lists the accounts'); + assert.dom('[data-test-checked-out-account]').exists({ count: 1 }, 'lists the checked out accounts'); }); test('it should transition to routes from list item action menu', async function (assert) { @@ -44,7 +66,7 @@ module('Acceptance | ldap | libraries', function (hooks) { await click(`[data-test-${action}]`); const uri = action === 'details' ? 'details/accounts' : action; assert.true( - isURL(`libraries/test-library/${uri}`), + isURL(`libraries/test-library/${uri}`, this.backend), `Transitions to ${action} route on list item action menu click` ); await click('[data-test-breadcrumb="libraries"] a'); @@ -55,13 +77,13 @@ module('Acceptance | ldap | libraries', function (hooks) { await click('[data-test-list-item-link] a'); await click('[data-test-tab="config"]'); assert.true( - isURL('libraries/test-library/details/configuration'), + isURL('libraries/test-library/details/configuration', this.backend), 'Transitions to configuration route on tab click' ); await click('[data-test-tab="accounts"]'); assert.true( - isURL('libraries/test-library/details/accounts'), + isURL('libraries/test-library/details/accounts', this.backend), 'Transitions to accounts route on tab click' ); }); @@ -69,6 +91,9 @@ module('Acceptance | ldap | libraries', function (hooks) { test('it should transition to routes from library details toolbar links', async function (assert) { await click('[data-test-list-item-link] a'); await click('[data-test-edit]'); - assert.true(isURL('libraries/test-library/edit'), 'Transitions to credentials route from toolbar link'); + assert.true( + isURL('libraries/test-library/edit', this.backend), + 'Transitions to credentials route from toolbar link' + ); }); }); diff --git a/ui/tests/acceptance/secrets/backend/ldap/overview-test.js b/ui/tests/acceptance/secrets/backend/ldap/overview-test.js index 8fe90cccf2e1..1490e40e00f3 100644 --- a/ui/tests/acceptance/secrets/backend/ldap/overview-test.js +++ b/ui/tests/acceptance/secrets/backend/ldap/overview-test.js @@ -6,12 +6,14 @@ import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; import { setupMirage } from 'ember-cli-mirage/test-support'; +import { v4 as uuidv4 } from 'uuid'; import ldapMirageScenario from 'vault/mirage/scenarios/ldap'; import ldapHandlers from 'vault/mirage/handlers/ldap'; import authPage from 'vault/tests/pages/auth'; import { click, fillIn, visit } from '@ember/test-helpers'; import { selectChoose } from 'ember-power-select/test-support'; import { isURL, visitURL } from 'vault/tests/helpers/ldap/ldap-helpers'; +import { deleteEngineCmd, mountEngineCmd, runCmd } from 'vault/tests/helpers/commands'; module('Acceptance | ldap | overview', function (hooks) { setupApplicationTest(hooks); @@ -19,77 +21,101 @@ module('Acceptance | ldap | overview', function (hooks) { hooks.beforeEach(async function () { ldapHandlers(this.server); + this.backend = `ldap-test-${uuidv4()}`; + this.mountAndConfig = (backend) => { + return runCmd([ + mountEngineCmd('ldap', backend), + `write ${backend}/config binddn=foo bindpass=bar url=http://localhost:8208`, + ]); + }; return authPage.login(); }); test('it should transition to ldap overview on mount success', async function (assert) { + const backend = 'ldap-test-mount'; await visit('/vault/secrets'); await click('[data-test-enable-engine]'); await click('[data-test-mount-type="ldap"]'); - await fillIn('[data-test-input="path"]', 'ldap-test'); + await fillIn('[data-test-input="path"]', backend); await click('[data-test-mount-submit]'); - assert.true(isURL('overview'), 'Transitions to ldap overview route on mount success'); + assert.true(isURL('overview', backend), 'Transitions to ldap overview route on mount success'); + assert.dom('[data-test-header-title]').hasText(backend); + // cleanup mounted engine + await visit('/vault/secrets'); + await runCmd(deleteEngineCmd(backend)); }); test('it should transition to routes on tab link click', async function (assert) { assert.expect(4); + await this.mountAndConfig(this.backend); - await visitURL('overview'); + await visitURL('overview', this.backend); for (const tab of ['roles', 'libraries', 'config', 'overview']) { await click(`[data-test-tab="${tab}"]`); const route = tab === 'config' ? 'configuration' : tab; - assert.true(isURL(route), `Transitions to ${route} route on tab link click`); + assert.true(isURL(route, this.backend), `Transitions to ${route} route on tab link click`); } }); test('it should transition to configuration route when engine is not configured', async function (assert) { - await visitURL('overview'); + await runCmd(mountEngineCmd('ldap', this.backend)); + await visitURL('overview', this.backend); await click('[data-test-config-cta] a'); - assert.true(isURL('configure'), 'Transitions to configure route on cta link click'); + assert.true(isURL('configure', this.backend), 'Transitions to configure route on cta link click'); - await click('[data-test-breadcrumb="ldap-test"] a'); + await click(`[data-test-breadcrumb="${this.backend}"] a`); await click('[data-test-toolbar-action="config"]'); - assert.true(isURL('configure'), 'Transitions to configure route on toolbar link click'); + assert.true(isURL('configure', this.backend), 'Transitions to configure route on toolbar link click'); }); // including a test for the configuration route here since it is the only one needed test('it should transition to configuration edit on toolbar link click', async function (assert) { ldapMirageScenario(this.server); - await visitURL('overview'); + await this.mountAndConfig(this.backend); + await visitURL('overview', this.backend); await click('[data-test-tab="config"]'); await click('[data-test-toolbar-config-action]'); - assert.true(isURL('configure'), 'Transitions to configure route on toolbar link click'); + assert.true(isURL('configure', this.backend), 'Transitions to configure route on toolbar link click'); }); test('it should transition to create role route on card action link click', async function (assert) { ldapMirageScenario(this.server); - await visitURL('overview'); + await this.mountAndConfig(this.backend); + await visitURL('overview', this.backend); await click('[data-test-overview-card="Roles"] a'); - assert.true(isURL('roles/create'), 'Transitions to role create route on card action link click'); + assert.true( + isURL('roles/create', this.backend), + 'Transitions to role create route on card action link click' + ); }); test('it should transition to create library route on card action link click', async function (assert) { ldapMirageScenario(this.server); - await visitURL('overview'); + await this.mountAndConfig(this.backend); + await visitURL('overview', this.backend); await click('[data-test-overview-card="Libraries"] a'); - assert.true(isURL('libraries/create'), 'Transitions to library create route on card action link click'); + assert.true( + isURL('libraries/create', this.backend), + 'Transitions to library create route on card action link click' + ); }); test('it should transition to role credentials route on generate credentials action', async function (assert) { ldapMirageScenario(this.server); - await visitURL('overview'); + await this.mountAndConfig(this.backend); + await visitURL('overview', this.backend); await selectChoose('.search-select', 'static-role'); await click('[data-test-generate-credential-button]'); assert.true( - isURL('roles/static/static-role/credentials'), + isURL('roles/static/static-role/credentials', this.backend), 'Transitions to role credentials route on generate credentials action' ); - await click('[data-test-breadcrumb="ldap-test"] a'); + await click(`[data-test-breadcrumb="${this.backend}"] a`); await selectChoose('.search-select', 'dynamic-role'); await click('[data-test-generate-credential-button]'); assert.true( - isURL('roles/dynamic/dynamic-role/credentials'), + isURL('roles/dynamic/dynamic-role/credentials', this.backend), 'Transitions to role credentials route on generate credentials action' ); }); diff --git a/ui/tests/acceptance/secrets/backend/ldap/roles-test.js b/ui/tests/acceptance/secrets/backend/ldap/roles-test.js index 15d1df74df2a..739b604dfc33 100644 --- a/ui/tests/acceptance/secrets/backend/ldap/roles-test.js +++ b/ui/tests/acceptance/secrets/backend/ldap/roles-test.js @@ -6,11 +6,14 @@ import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; import { setupMirage } from 'ember-cli-mirage/test-support'; +import { v4 as uuidv4 } from 'uuid'; import ldapMirageScenario from 'vault/mirage/scenarios/ldap'; import ldapHandlers from 'vault/mirage/handlers/ldap'; import authPage from 'vault/tests/pages/auth'; -import { click, fillIn } from '@ember/test-helpers'; +import { click, fillIn, waitFor } from '@ember/test-helpers'; import { isURL, visitURL } from 'vault/tests/helpers/ldap/ldap-helpers'; +import { GENERAL } from 'vault/tests/helpers/general-selectors'; +import { deleteEngineCmd, mountEngineCmd, runCmd } from 'vault/tests/helpers/commands'; module('Acceptance | ldap | roles', function (hooks) { setupApplicationTest(hooks); @@ -19,26 +22,39 @@ module('Acceptance | ldap | roles', function (hooks) { hooks.beforeEach(async function () { ldapHandlers(this.server); ldapMirageScenario(this.server); + this.backend = `ldap-test-${uuidv4()}`; await authPage.login(); - return visitURL('roles'); + // mount & configure + await runCmd([ + mountEngineCmd('ldap', this.backend), + `write ${this.backend}/config binddn=foo bindpass=bar url=http://localhost:8208`, + ]); + return visitURL('roles', this.backend); + }); + + hooks.afterEach(async function () { + await runCmd(deleteEngineCmd(this.backend)); }); test('it should transition to create role route on toolbar link click', async function (assert) { await click('[data-test-toolbar-action="role"]'); - assert.true(isURL('roles/create'), 'Transitions to role create route on toolbar link click'); + assert.true( + isURL('roles/create', this.backend), + 'Transitions to role create route on toolbar link click' + ); }); test('it should transition to role details route on list item click', async function (assert) { await click('[data-test-list-item-link]:nth-of-type(1) a'); assert.true( - isURL('roles/dynamic/dynamic-role/details'), + isURL('roles/dynamic/dynamic-role/details', this.backend), 'Transitions to role details route on list item click' ); await click('[data-test-breadcrumb="roles"] a'); await click('[data-test-list-item-link]:nth-of-type(2) a'); assert.true( - isURL('roles/static/static-role/details'), + isURL('roles/static/static-role/details', this.backend), 'Transitions to role details route on list item click' ); }); @@ -51,7 +67,7 @@ module('Acceptance | ldap | roles', function (hooks) { await click(`[data-test-${action}]`); const uri = action === 'get-creds' ? 'credentials' : action; assert.true( - isURL(`roles/dynamic/dynamic-role/${uri}`), + isURL(`roles/dynamic/dynamic-role/${uri}`, this.backend), `Transitions to ${uri} route on list item action menu click` ); await click('[data-test-breadcrumb="roles"] a'); @@ -62,13 +78,16 @@ module('Acceptance | ldap | roles', function (hooks) { await click('[data-test-list-item-link]:nth-of-type(1) a'); await click('[data-test-get-credentials]'); assert.true( - isURL('roles/dynamic/dynamic-role/credentials'), + isURL('roles/dynamic/dynamic-role/credentials', this.backend), 'Transitions to credentials route from toolbar link' ); await click('[data-test-breadcrumb="dynamic-role"] a'); await click('[data-test-edit]'); - assert.true(isURL('roles/dynamic/dynamic-role/edit'), 'Transitions to edit route from toolbar link'); + assert.true( + isURL('roles/dynamic/dynamic-role/edit', this.backend), + 'Transitions to edit route from toolbar link' + ); }); test('it should clear roles page filter value on route exit', async function (assert) { @@ -76,6 +95,7 @@ module('Acceptance | ldap | roles', function (hooks) { assert .dom('[data-test-filter-input]') .hasValue('foo', 'Roles page filter value set after model refresh and rerender'); + await waitFor(GENERAL.emptyStateTitle); await click('[data-test-tab="libraries"]'); await click('[data-test-tab="roles"]'); assert.dom('[data-test-filter-input]').hasNoValue('Roles page filter value cleared on route exit'); diff --git a/ui/tests/helpers/ldap/ldap-helpers.js b/ui/tests/helpers/ldap/ldap-helpers.js index a08e02275361..b184878a0ff6 100644 --- a/ui/tests/helpers/ldap/ldap-helpers.js +++ b/ui/tests/helpers/ldap/ldap-helpers.js @@ -28,13 +28,13 @@ export const generateBreadcrumbs = (backend, childRoute) => { return breadcrumbs; }; -const baseURL = '/vault/secrets/ldap-test/ldap/'; +const baseURL = (backend) => `/vault/secrets/${backend}/ldap/`; const stripLeadingSlash = (uri) => (uri.charAt(0) === '/' ? uri.slice(1) : uri); -export const isURL = (uri) => { - return currentURL() === `${baseURL}${stripLeadingSlash(uri)}`; +export const isURL = (uri, backend = 'ldap-test') => { + return currentURL() === `${baseURL(backend)}${stripLeadingSlash(uri)}`; }; -export const visitURL = (uri) => { - return visit(`${baseURL}${stripLeadingSlash(uri)}`); +export const visitURL = (uri, backend = 'ldap-test') => { + return visit(`${baseURL(backend)}${stripLeadingSlash(uri)}`); }; diff --git a/ui/tests/integration/components/ldap/page/library/details/accounts-test.js b/ui/tests/integration/components/ldap/page/library/details/accounts-test.js index 94b0048cc512..8bb1ab696921 100644 --- a/ui/tests/integration/components/ldap/page/library/details/accounts-test.js +++ b/ui/tests/integration/components/ldap/page/library/details/accounts-test.js @@ -77,6 +77,9 @@ module('Integration | Component | ldap | Page::Library::Details::Accounts', func assert .dom('[data-test-accounts-code-block] code') - .hasText('vault lease renew ad/library/test-library/check-out/:lease_id', 'Renew cli command renders'); + .hasText( + 'vault lease renew ldap-test/library/test-library/check-out/:lease_id', + 'Renew cli command renders with backend path' + ); }); }); From 520f1416082b86ae4ee93b5048dc20ba0aa18543 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com> Date: Fri, 20 Sep 2024 13:52:59 -0500 Subject: [PATCH 2/8] UI: refactor KMIP role model (#28418) * update kmip/role model and adapter * New KMIP role form component * cleanup on kmip role adapter/model * fix role details view * update tests to check for kmip role form and details validity * cleanup * Add kmip-role-fields test * add headers, remove old component * Address PR comments --- ui/app/adapters/kmip/role.js | 33 +++--- ui/app/models/kmip/role.js | 75 +++++------- ui/app/styles/components/kmip-role-edit.scss | 5 +- ui/app/utils/kmip-role-fields.js | 27 +++++ .../addon/components/edit-form-kmip-role.js | 55 --------- .../kmip/addon/components/kmip/role-form.hbs | 82 ++++++++++++++ .../kmip/addon/components/kmip/role-form.js | 107 ++++++++++++++++++ .../kmip/addon/routes/scope/roles/create.js | 1 + .../components/edit-form-kmip-role.hbs | 107 ------------------ ui/lib/kmip/addon/templates/role/edit.hbs | 5 +- .../addon/templates/scope/roles/create.hbs | 6 +- ui/lib/kmip/index.js | 4 + ui/tests/acceptance/enterprise-kmip-test.js | 46 ++++---- ui/tests/pages/secrets/backend/kmip/roles.js | 2 +- ui/tests/unit/adapters/kmip/role-test.js | 10 +- ui/tests/unit/utils/kmip-role-fields-test.js | 52 +++++++++ 16 files changed, 360 insertions(+), 257 deletions(-) create mode 100644 ui/app/utils/kmip-role-fields.js delete mode 100644 ui/lib/kmip/addon/components/edit-form-kmip-role.js create mode 100644 ui/lib/kmip/addon/components/kmip/role-form.hbs create mode 100644 ui/lib/kmip/addon/components/kmip/role-form.js delete mode 100644 ui/lib/kmip/addon/templates/components/edit-form-kmip-role.hbs create mode 100644 ui/tests/unit/utils/kmip-role-fields-test.js diff --git a/ui/app/adapters/kmip/role.js b/ui/app/adapters/kmip/role.js index 9779ad67a564..693ae6327e11 100644 --- a/ui/app/adapters/kmip/role.js +++ b/ui/app/adapters/kmip/role.js @@ -6,10 +6,11 @@ import BaseAdapter from './base'; import { decamelize } from '@ember/string'; import { getProperties } from '@ember/object'; +import { nonOperationFields } from 'vault/utils/kmip-role-fields'; export default BaseAdapter.extend({ createRecord(store, type, snapshot) { - const name = snapshot.id || snapshot.attr('name'); + const name = snapshot.id || snapshot.record.role; const url = this._url( type.modelName, { @@ -18,10 +19,11 @@ export default BaseAdapter.extend({ }, name ); - return this.ajax(url, 'POST', { data: this.serialize(snapshot) }).then(() => { + const data = this.serialize(snapshot); + return this.ajax(url, 'POST', { data }).then(() => { return { id: name, - name, + role: name, backend: snapshot.record.backend, scope: snapshot.record.scope, }; @@ -29,7 +31,8 @@ export default BaseAdapter.extend({ }, deleteRecord(store, type, snapshot) { - const name = snapshot.id || snapshot.attr('name'); + // records must always have IDs + const name = snapshot.id; const url = this._url( type.modelName, { @@ -41,35 +44,35 @@ export default BaseAdapter.extend({ return this.ajax(url, 'DELETE'); }, + updateRecord() { + return this.createRecord(...arguments); + }, + serialize(snapshot) { // the endpoint here won't allow sending `operation_all` and `operation_none` at the same time or with // other operation_ values, so we manually check for them and send an abbreviated object const json = snapshot.serialize(); - const keys = snapshot.record.nonOperationFields.map(decamelize); - const nonOperationFields = getProperties(json, keys); - for (const field in nonOperationFields) { - if (nonOperationFields[field] == null) { - delete nonOperationFields[field]; + const keys = nonOperationFields(snapshot.record.editableFields).map(decamelize); + const nonOp = getProperties(json, keys); + for (const field in nonOp) { + if (nonOp[field] == null) { + delete nonOp[field]; } } if (json.operation_all) { return { operation_all: true, - ...nonOperationFields, + ...nonOp, }; } if (json.operation_none) { return { operation_none: true, - ...nonOperationFields, + ...nonOp, }; } delete json.operation_none; delete json.operation_all; return json; }, - - updateRecord() { - return this.createRecord(...arguments); - }, }); diff --git a/ui/app/models/kmip/role.js b/ui/app/models/kmip/role.js index 8083acd7b331..520efcf1b517 100644 --- a/ui/app/models/kmip/role.js +++ b/ui/app/models/kmip/role.js @@ -4,52 +4,35 @@ */ import Model, { attr } from '@ember-data/model'; -import { computed } from '@ember/object'; -import fieldToAttrs, { expandAttributeMeta } from 'vault/utils/field-to-attrs'; import apiPath from 'vault/utils/api-path'; import lazyCapabilities from 'vault/macros/lazy-capabilities'; +import { withExpandedAttributes } from 'vault/decorators/model-expanded-attributes'; +import { operationFields, operationFieldsWithoutSpecial, tlsFields } from 'vault/utils/kmip-role-fields'; import { removeManyFromArray } from 'vault/helpers/remove-from-array'; -const COMPUTEDS = { - operationFields: computed('newFields', function () { - return this.newFields.filter((key) => key.startsWith('operation')); - }), +@withExpandedAttributes() +export default class KmipRoleModel extends Model { + @attr({ readOnly: true }) backend; + @attr({ readOnly: true }) scope; - operationFieldsWithoutSpecial: computed('operationFields', function () { - return removeManyFromArray(this.operationFields, ['operationAll', 'operationNone']); - }), + get editableFields() { + return Object.keys(this.allByKey).filter((k) => !['backend', 'scope', 'role'].includes(k)); + } - tlsFields: computed(function () { - return ['tlsClientKeyBits', 'tlsClientKeyType', 'tlsClientTtl']; - }), - - // For rendering on the create/edit pages - defaultFields: computed('newFields', 'operationFields', 'tlsFields', function () { - const excludeFields = ['role'].concat(this.operationFields, this.tlsFields); - return removeManyFromArray(this.newFields, excludeFields); - }), - - // For adapter/serializer - nonOperationFields: computed('newFields', 'operationFields', function () { - return removeManyFromArray(this.newFields, this.operationFields); - }), -}; - -export default Model.extend(COMPUTEDS, { - backend: attr({ readOnly: true }), - scope: attr({ readOnly: true }), - name: attr({ readOnly: true }), - - fieldGroups: computed('fields', 'defaultFields.length', 'tlsFields', function () { - const groups = [{ TLS: this.tlsFields }]; - if (this.defaultFields.length) { - groups.unshift({ default: this.defaultFields }); + get fieldGroups() { + const tls = tlsFields(); + const groups = [{ TLS: tls }]; + // op fields are shown in OperationFieldDisplay + const opFields = operationFields(this.editableFields); + // not op fields, tls fields, or role/backend/scope + const defaultFields = this.editableFields.filter((f) => ![...opFields, ...tls].includes(f)); + if (defaultFields.length) { + groups.unshift({ default: defaultFields }); } - const ret = fieldToAttrs(this, groups); - return ret; - }), + return this._expandGroups(groups); + } - operationFormFields: computed('operationFieldsWithoutSpecial', function () { + get operationFormFields() { const objects = [ 'operationCreate', 'operationActivate', @@ -62,7 +45,7 @@ export default Model.extend(COMPUTEDS, { const attributes = ['operationAddAttribute', 'operationGetAttributes']; const server = ['operationDiscoverVersions']; - const others = removeManyFromArray(this.operationFieldsWithoutSpecial, [ + const others = removeManyFromArray(operationFieldsWithoutSpecial(this.editableFields), [ ...objects, ...attributes, ...server, @@ -77,14 +60,8 @@ export default Model.extend(COMPUTEDS, { Other: others, }); } - return fieldToAttrs(this, groups); - }), - tlsFormFields: computed('tlsFields', function () { - return expandAttributeMeta(this, this.tlsFields); - }), - fields: computed('defaultFields', function () { - return expandAttributeMeta(this, this.defaultFields); - }), + return this._expandGroups(groups); + } - updatePath: lazyCapabilities(apiPath`${'backend'}/scope/${'scope'}/role/${'id'}`, 'backend', 'scope', 'id'), -}); + @lazyCapabilities(apiPath`${'backend'}/scope/${'scope'}/role/${'id'}`, 'backend', 'scope', 'id') updatePath; +} diff --git a/ui/app/styles/components/kmip-role-edit.scss b/ui/app/styles/components/kmip-role-edit.scss index a517c1a54313..f4fc884be41f 100644 --- a/ui/app/styles/components/kmip-role-edit.scss +++ b/ui/app/styles/components/kmip-role-edit.scss @@ -3,11 +3,14 @@ * SPDX-License-Identifier: BUSL-1.1 */ +.kmip-role-operations { + column-count: 2; +} .kmip-role-allowed-operations { @extend .box; flex: 1 1 auto; box-shadow: none; - padding: 0; + padding: $spacing-4 0; } .kmip-role-allowed-operations .field { margin-bottom: $spacing-4; diff --git a/ui/app/utils/kmip-role-fields.js b/ui/app/utils/kmip-role-fields.js new file mode 100644 index 000000000000..3503ad58f449 --- /dev/null +++ b/ui/app/utils/kmip-role-fields.js @@ -0,0 +1,27 @@ +/** + * Copyright (c) HashiCorp, Inc. + * SPDX-License-Identifier: BUSL-1.1 + */ + +import { removeManyFromArray } from 'vault/helpers/remove-from-array'; + +export const operationFields = (fieldNames) => { + if (!Array.isArray(fieldNames)) { + throw new Error('fieldNames must be an array'); + } + return fieldNames.filter((key) => key.startsWith('operation')); +}; + +export const operationFieldsWithoutSpecial = (fieldNames) => { + const opFields = operationFields(fieldNames); + return removeManyFromArray(opFields, ['operationAll', 'operationNone']); +}; + +export const nonOperationFields = (fieldNames) => { + const opFields = operationFields(fieldNames); + return removeManyFromArray(fieldNames, opFields); +}; + +export const tlsFields = () => { + return ['tlsClientKeyBits', 'tlsClientKeyType', 'tlsClientTtl']; +}; diff --git a/ui/lib/kmip/addon/components/edit-form-kmip-role.js b/ui/lib/kmip/addon/components/edit-form-kmip-role.js deleted file mode 100644 index 43a230b80b19..000000000000 --- a/ui/lib/kmip/addon/components/edit-form-kmip-role.js +++ /dev/null @@ -1,55 +0,0 @@ -/** - * Copyright (c) HashiCorp, Inc. - * SPDX-License-Identifier: BUSL-1.1 - */ - -import EditForm from 'core/components/edit-form'; -import { computed } from '@ember/object'; -import layout from '../templates/components/edit-form-kmip-role'; - -export default EditForm.extend({ - layout, - model: null, - - cancelLink: computed('cancelLinkParams.[]', function () { - if (!Array.isArray(this.cancelLinkParams) || !this.cancelLinkParams.length) return; - const [route, ...models] = this.cancelLinkParams; - return { route, models }; - }), - - init() { - this._super(...arguments); - - if (this.model.isNew) { - this.model.operationAll = true; - } - }, - - actions: { - toggleOperationSpecial(checked) { - this.model.operationNone = !checked; - this.model.operationAll = checked; - }, - - // when operationAll is true, we want all of the items - // to appear checked, but we don't want to override what items - // a user has selected - so this action creates an object that we - // pass to the FormField component as the model instead of the real model - placeholderOrModel(isOperationAll, attr) { - return isOperationAll ? { [attr.name]: true } : this.model; - }, - - preSave(model) { - // if we have operationAll or operationNone, we want to clear - // out the others so that display shows the right data - if (model.operationAll || model.operationNone) { - model.operationFieldsWithoutSpecial.forEach((field) => model.set(field, null)); - } - // set operationNone if user unchecks 'operationAll' instead of toggling the 'operationNone' input - // doing here instead of on the 'operationNone' input because a user might deselect all, then reselect some options - // and immediately setting operationNone will hide all of the checkboxes in the UI - this.model.operationNone = - model.operationFieldsWithoutSpecial.every((attr) => !model[attr]) && !this.model.operationAll; - }, - }, -}); diff --git a/ui/lib/kmip/addon/components/kmip/role-form.hbs b/ui/lib/kmip/addon/components/kmip/role-form.hbs new file mode 100644 index 000000000000..d10fb57add42 --- /dev/null +++ b/ui/lib/kmip/addon/components/kmip/role-form.hbs @@ -0,0 +1,82 @@ +{{! + Copyright (c) HashiCorp, Inc. + SPDX-License-Identifier: BUSL-1.1 +~}} + +
+ +
+ + {{#if @model.isNew}} + {{! Show role name only in create mode }} + + {{/if}} +
+ + +
+ {{#unless @model.operationNone}} + +

+ Allowed Operations +

+ +
+ +
+
+ {{#each this.operationFormGroups as |group|}} +
+

{{group.name}}

+ {{#each group.fields as |attr|}} + + {{/each}} +
+ {{/each}} +
+
+ {{/unless}} +
+

+ TLS +

+ {{#each this.tlsFormFields as |attr|}} + + {{/each}} +
+ {{#each @model.fields as |attr|}} + + {{/each}} +
+ +
+ + + + +
+ \ No newline at end of file diff --git a/ui/lib/kmip/addon/components/kmip/role-form.js b/ui/lib/kmip/addon/components/kmip/role-form.js new file mode 100644 index 000000000000..496dde4106a0 --- /dev/null +++ b/ui/lib/kmip/addon/components/kmip/role-form.js @@ -0,0 +1,107 @@ +/** + * Copyright (c) HashiCorp, Inc. + * SPDX-License-Identifier: BUSL-1.1 + */ + +import AdapterError from '@ember-data/adapter/error'; +import { action } from '@ember/object'; +import { service } from '@ember/service'; +import Component from '@glimmer/component'; +import { task } from 'ember-concurrency'; +import { removeManyFromArray } from 'vault/helpers/remove-from-array'; +import { operationFieldsWithoutSpecial, tlsFields } from 'vault/utils/kmip-role-fields'; + +export default class KmipRoleFormComponent extends Component { + @service flashMessages; + @service store; + + // Actual attribute fields + get tlsFormFields() { + return tlsFields().map((attr) => this.args.model.allByKey[attr]); + } + get operationFormGroups() { + const objects = [ + 'operationCreate', + 'operationActivate', + 'operationGet', + 'operationLocate', + 'operationRekey', + 'operationRevoke', + 'operationDestroy', + ]; + const attributes = ['operationAddAttribute', 'operationGetAttributes']; + const server = ['operationDiscoverVersions']; + const others = removeManyFromArray(operationFieldsWithoutSpecial(this.args.model.editableFields), [ + ...objects, + ...attributes, + ...server, + ]); + const groups = [ + { name: 'Managed Cryptographic Objects', fields: objects }, + { name: 'Object Attributes', fields: attributes }, + { name: 'Server', fields: server }, + ]; + if (others.length) { + groups.push({ + name: 'Other', + fields: others, + }); + } + // expand field names to attributes + return groups.map((group) => ({ + ...group, + fields: group.fields.map((attr) => this.args.model.allByKey[attr]), + })); + } + + placeholderOrModel = (model, attrName) => { + return model.operationAll ? { [attrName]: true } : model; + }; + + preSave() { + const opFieldsWithoutSpecial = operationFieldsWithoutSpecial(this.args.model.editableFields); + // if we have operationAll or operationNone, we want to clear + // out the others so that display shows the right data + if (this.args.model.operationAll || this.args.model.operationNone) { + opFieldsWithoutSpecial.forEach((field) => (this.args.model[field] = null)); + } + // set operationNone if user unchecks 'operationAll' instead of toggling the 'operationNone' input + // doing here instead of on the 'operationNone' input because a user might deselect all, then reselect some options + // and immediately setting operationNone will hide all of the checkboxes in the UI + this.args.model.operationNone = + opFieldsWithoutSpecial.every((attr) => this.args.model[attr] !== true) && !this.args.model.operationAll; + return this.args.model; + } + + @action toggleOperationSpecial(evt) { + const { checked } = evt.target; + this.args.model.operationNone = !checked; + this.args.model.operationAll = checked; + } + + save = task(async (evt) => { + evt.preventDefault(); + const model = this.preSave(); + try { + await model.save(); + this.flashMessages.success(`Saved role ${model.role}`); + } catch (err) { + // err will display via model state + // AdapterErrors are handled by the error-message component + if (err instanceof AdapterError === false) { + throw err; + } + return; + } + this.args.onSave(); + }); + + willDestroy() { + // components are torn down after store is unloaded and will cause an error if attempt to unload record + const noTeardown = this.store && !this.store.isDestroying; + if (noTeardown && this.args?.model?.isDirty) { + this.args.model.rollbackAttributes(); + } + super.willDestroy(); + } +} diff --git a/ui/lib/kmip/addon/routes/scope/roles/create.js b/ui/lib/kmip/addon/routes/scope/roles/create.js index 82984cddcb63..7070b4c27408 100644 --- a/ui/lib/kmip/addon/routes/scope/roles/create.js +++ b/ui/lib/kmip/addon/routes/scope/roles/create.js @@ -21,6 +21,7 @@ export default Route.extend({ const model = this.store.createRecord('kmip/role', { backend: this.secretMountPath.currentPath, scope: this.scope(), + operationAll: true, }); return model; }, diff --git a/ui/lib/kmip/addon/templates/components/edit-form-kmip-role.hbs b/ui/lib/kmip/addon/templates/components/edit-form-kmip-role.hbs deleted file mode 100644 index 3aea47a1d8df..000000000000 --- a/ui/lib/kmip/addon/templates/components/edit-form-kmip-role.hbs +++ /dev/null @@ -1,107 +0,0 @@ -{{! - Copyright (c) HashiCorp, Inc. - SPDX-License-Identifier: BUSL-1.1 -~}} - -
- -
- - {{#if (eq @mode "create")}} - - {{/if}} -
- - -
- {{#unless this.model.operationNone}} - -

- Allowed Operations -

- -
- -
-
-
- {{#each-in (get this.model.operationFormFields 0) as |groupName fieldsInGroup|}} -

{{groupName}}

- {{#each fieldsInGroup as |attr|}} - - {{/each}} - {{/each-in}} -
-
- {{#each (drop 1 (or this.model.operationFormFields (array))) as |group|}} -
- {{#each-in group as |groupName fieldsInGroup|}} -

{{groupName}}

- {{#each fieldsInGroup as |attr|}} - - {{/each}} - {{/each-in}} -
- {{/each}} -
-
-
- {{/unless}} -
-

- TLS -

- {{#each this.model.tlsFormFields as |attr|}} - - {{/each}} -
- {{#each this.model.fields as |attr|}} - - {{/each}} -
- -
- - - {{#if this.cancelLink}} - - {{/if}} - -
- \ No newline at end of file diff --git a/ui/lib/kmip/addon/templates/role/edit.hbs b/ui/lib/kmip/addon/templates/role/edit.hbs index ab2003d85783..482083290567 100644 --- a/ui/lib/kmip/addon/templates/role/edit.hbs +++ b/ui/lib/kmip/addon/templates/role/edit.hbs @@ -13,8 +13,9 @@ - \ No newline at end of file diff --git a/ui/lib/kmip/addon/templates/scope/roles/create.hbs b/ui/lib/kmip/addon/templates/scope/roles/create.hbs index 42fcc4bf4074..71af81aa1bad 100644 --- a/ui/lib/kmip/addon/templates/scope/roles/create.hbs +++ b/ui/lib/kmip/addon/templates/scope/roles/create.hbs @@ -13,9 +13,9 @@ - \ No newline at end of file diff --git a/ui/lib/kmip/index.js b/ui/lib/kmip/index.js index 2d2ddc66e948..635b19df758a 100644 --- a/ui/lib/kmip/index.js +++ b/ui/lib/kmip/index.js @@ -17,6 +17,10 @@ module.exports = EngineAddon.extend({ enabled: true, }, + babel: { + plugins: [require.resolve('ember-concurrency/async-arrow-task-transform')], + }, + isDevelopingAddon() { return true; }, diff --git a/ui/tests/acceptance/enterprise-kmip-test.js b/ui/tests/acceptance/enterprise-kmip-test.js index a2f743227640..916cc31da051 100644 --- a/ui/tests/acceptance/enterprise-kmip-test.js +++ b/ui/tests/acceptance/enterprise-kmip-test.js @@ -16,7 +16,7 @@ import { import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; -import authPage from 'vault/tests/pages/auth'; +import { login } from 'vault/tests/helpers/auth/auth-helpers'; import scopesPage from 'vault/tests/pages/secrets/backend/kmip/scopes'; import rolesPage from 'vault/tests/pages/secrets/backend/kmip/roles'; import credentialsPage from 'vault/tests/pages/secrets/backend/kmip/credentials'; @@ -91,7 +91,7 @@ module('Acceptance | Enterprise | KMIP secrets', function (hooks) { hooks.beforeEach(async function () { this.backend = `kmip-${uuidv4()}`; - await authPage.login(); + await login(); return; }); @@ -221,16 +221,11 @@ module('Acceptance | Enterprise | KMIP secrets', function (hooks) { test('it can create a role', async function (assert) { // moving create scope here to help with flaky test - const backend = await mountWithConfig(this.backend); - await settled(); const scope = `scope-for-can-create-role`; - await settled(); - const res = await runCmd([`write ${backend}/scope/${scope} -force`]); - await settled(); - if (res.includes('Error')) { - throw new Error(`Error creating scope: ${res}`); - } const role = `role-new-role`; + const backend = await mountWithConfig(this.backend); + await settled(); + await runCmd([`write ${backend}/scope/${scope} -force`], true); await rolesPage.visit({ backend, scope }); await settled(); assert.ok(rolesPage.isEmpty, 'renders the empty role page'); @@ -241,11 +236,15 @@ module('Acceptance | Enterprise | KMIP secrets', function (hooks) { `/vault/secrets/${backend}/kmip/scopes/${scope}/roles/create`, 'links to the role create form' ); + // check that the role form looks right + assert.dom(GENERAL.inputByAttr('operationNone')).isChecked('allows role to perform roles by default'); + assert.dom(GENERAL.inputByAttr('operationAll')).isChecked('operationAll is checked by default'); + assert.dom('[data-test-kmip-section]').exists({ count: 2 }); + assert.dom('[data-test-kmip-operations]').exists({ count: 4 }); await rolesPage.roleName(role); await settled(); - await rolesPage.submit(); - await settled(); + await click(GENERAL.saveButton); assert.strictEqual( currentURL(), `/vault/secrets/${backend}/kmip/scopes/${scope}/roles`, @@ -253,6 +252,13 @@ module('Acceptance | Enterprise | KMIP secrets', function (hooks) { ); assert.strictEqual(rolesPage.listItemLinks.length, 1, 'renders a single role'); + await rolesPage.visitDetail({ backend, scope, role }); + // check that the role details looks right + assert.dom('h2').exists({ count: 2 }, 'renders correct section headings'); + assert.dom('[data-test-inline-error-message]').hasText('This role allows all KMIP operations'); + ['Managed Cryptographic Objects', 'Object Attributes', 'Server', 'Other'].forEach((title) => { + assert.dom(`[data-test-row-label="${title}"]`).exists(`Renders allowed operations row for: ${title}`); + }); }); test('it navigates to kmip roles view using breadcrumbs', async function (assert) { @@ -304,8 +310,7 @@ module('Acceptance | Enterprise | KMIP secrets', function (hooks) { `/vault/secrets/${backend}/kmip/scopes/${scope}/roles/${role}/edit`, 'navigates to role edit' ); - await rolesPage.cancelLink(); - await settled(); + await click(GENERAL.cancelButton); assert.strictEqual( currentURL(), `/vault/secrets/${backend}/kmip/scopes/${scope}/roles/${role}`, @@ -364,12 +369,12 @@ module('Acceptance | Enterprise | KMIP secrets', function (hooks) { this.store = this.owner.lookup('service:store'); this.scope = 'my-scope'; this.name = 'my-role'; - await authPage.login(); + await login(); await runCmd(mountEngineCmd('kmip', this.backend), false); await runCmd([`write ${this.backend}/scope/${this.scope} -force`]); await rolesPage.visit({ backend: this.backend, scope: this.scope }); this.setModel = async () => { - await click('[data-test-edit-form-submit]'); + await click(GENERAL.saveButton); await visit(`/vault/secrets/${this.backend}/kmip/scopes/${this.scope}/roles/${this.name}`); this.model = this.store.peekRecord('kmip/role', this.name); }; @@ -382,7 +387,7 @@ module('Acceptance | Enterprise | KMIP secrets', function (hooks) { assert.expect(3); await click('[data-test-role-create]'); - await fillIn(GENERAL.inputByAttr('name'), this.name); + await fillIn(GENERAL.inputByAttr('role'), this.name); assert.dom(GENERAL.inputByAttr('operationAll')).isChecked('operationAll is checked by default'); await this.setModel(); assert.true(this.model.operationAll, 'operationAll is true'); @@ -393,7 +398,7 @@ module('Acceptance | Enterprise | KMIP secrets', function (hooks) { assert.expect(4); await click('[data-test-role-create]'); - await fillIn(GENERAL.inputByAttr('name'), this.name); + await fillIn(GENERAL.inputByAttr('role'), this.name); await click(GENERAL.inputByAttr('operationNone')); assert .dom(GENERAL.inputByAttr('operationNone')) @@ -410,9 +415,10 @@ module('Acceptance | Enterprise | KMIP secrets', function (hooks) { assert.expect(2); await click('[data-test-role-create]'); - await fillIn(GENERAL.inputByAttr('name'), this.name); + await fillIn(GENERAL.inputByAttr('role'), this.name); await click(GENERAL.inputByAttr('operationAll')); await this.setModel(); + assert.strictEqual(this.model.operationAll, undefined, 'operationAll is unset'); assert.true(this.model.operationNone, 'operationNone is true'); }); @@ -421,7 +427,7 @@ module('Acceptance | Enterprise | KMIP secrets', function (hooks) { assert.expect(6); await click('[data-test-role-create]'); - await fillIn(GENERAL.inputByAttr('name'), this.name); + await fillIn(GENERAL.inputByAttr('role'), this.name); await click(GENERAL.inputByAttr('operationAll')); await click(GENERAL.inputByAttr('operationGet')); await click(GENERAL.inputByAttr('operationGetAttributes')); diff --git a/ui/tests/pages/secrets/backend/kmip/roles.js b/ui/tests/pages/secrets/backend/kmip/roles.js index aafaeba988a0..bc06acb4a584 100644 --- a/ui/tests/pages/secrets/backend/kmip/roles.js +++ b/ui/tests/pages/secrets/backend/kmip/roles.js @@ -11,7 +11,7 @@ export default create({ visit: visitable('/vault/secrets/:backend/kmip/scopes/:scope/roles'), visitDetail: visitable('/vault/secrets/:backend/kmip/scopes/:scope/roles/:role'), create: clickable('[data-test-role-create]'), - roleName: fillable('[data-test-input="name"]'), + roleName: fillable('[data-test-input="role"]'), submit: clickable('[data-test-edit-form-submit]'), detailEditLink: clickable('[data-test-kmip-link-edit-role]'), cancelLink: clickable('[data-test-edit-form-cancel]'), diff --git a/ui/tests/unit/adapters/kmip/role-test.js b/ui/tests/unit/adapters/kmip/role-test.js index a9d20d900459..36dcf9a883de 100644 --- a/ui/tests/unit/adapters/kmip/role-test.js +++ b/ui/tests/unit/adapters/kmip/role-test.js @@ -9,6 +9,8 @@ import { setupTest } from 'ember-qunit'; module('Unit | Adapter | kmip/role', function (hooks) { setupTest(hooks); + // these are only some of the actual editable fields + const editableFields = ['tlsTtl', 'operationAll', 'operationNone', 'operationGet', 'operationCreate']; const serializeTests = [ [ 'operation_all is the only operation item present after serialization', @@ -17,7 +19,7 @@ module('Unit | Adapter | kmip/role', function (hooks) { return { operation_all: true, operation_get: true, operation_create: true, tls_ttl: '10s' }; }, record: { - nonOperationFields: ['tlsTtl'], + editableFields, }, }, { @@ -32,7 +34,7 @@ module('Unit | Adapter | kmip/role', function (hooks) { return { operation_all: true, operation_get: true, operation_create: true }; }, record: { - nonOperationFields: ['tlsTtl'], + editableFields, }, }, { @@ -46,7 +48,7 @@ module('Unit | Adapter | kmip/role', function (hooks) { return { operation_none: true, operation_get: true, operation_add_attribute: true, tls_ttl: '10s' }; }, record: { - nonOperationFields: ['tlsTtl'], + editableFields, }, }, { @@ -67,7 +69,7 @@ module('Unit | Adapter | kmip/role', function (hooks) { }; }, record: { - nonOperationFields: ['tlsTtl'], + editableFields, }, }, { diff --git a/ui/tests/unit/utils/kmip-role-fields-test.js b/ui/tests/unit/utils/kmip-role-fields-test.js new file mode 100644 index 000000000000..effbda02fb5c --- /dev/null +++ b/ui/tests/unit/utils/kmip-role-fields-test.js @@ -0,0 +1,52 @@ +/** + * Copyright (c) HashiCorp, Inc. + * SPDX-License-Identifier: BUSL-1.1 + */ + +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; +import { + nonOperationFields, + operationFields, + operationFieldsWithoutSpecial, +} from 'vault/utils/kmip-role-fields'; + +module('Unit | Util | kmip role fields', function (hooks) { + setupTest(hooks); + + [ + { + name: 'when fields is empty', + fields: [], + opFields: [], + nonOpFields: [], + opWithoutSpecial: [], + }, + { + name: 'when no op fields', + fields: ['foo', 'bar'], + opFields: [], + nonOpFields: ['foo', 'bar'], + opWithoutSpecial: [], + }, + { + name: 'when op fields', + fields: ['foo', 'bar', 'operationFoo', 'operationBar', 'operationAll'], + opFields: ['operationFoo', 'operationBar', 'operationAll'], + nonOpFields: ['foo', 'bar'], + opWithoutSpecial: ['operationFoo', 'operationBar'], + }, + ].forEach(({ name, fields, opFields, nonOpFields, opWithoutSpecial }) => { + test(`${name}`, function (assert) { + const originalFields = JSON.parse(JSON.stringify(fields)); + assert.deepEqual(operationFields(fields), opFields, 'operation fields correct'); + assert.deepEqual(nonOperationFields(fields), nonOpFields, 'non operation fields'); + assert.deepEqual( + operationFieldsWithoutSpecial(fields), + opWithoutSpecial, + 'operation fields without special' + ); + assert.deepEqual(fields, originalFields, 'does not mutate the original'); + }); + }); +}); From 0c986fc660b816bb87db13338ee318495701e450 Mon Sep 17 00:00:00 2001 From: rajesht-source Date: Sat, 21 Sep 2024 02:23:38 +0530 Subject: [PATCH 3/8] updated vault helm chart doc with usecase of nlb (#27690) * updated vault helm chart doc with usecase of nlb Signed-off-by: Kumar, Rajesh (XINBM1A) * Update index.mdx - changes as per the pr comment Signed-off-by: Kumar, Rajesh (XINBM1A) --------- Signed-off-by: Kumar, Rajesh (XINBM1A) --- website/content/docs/platform/k8s/helm/index.mdx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/website/content/docs/platform/k8s/helm/index.mdx b/website/content/docs/platform/k8s/helm/index.mdx index 53f410103b93..6cc6c6bb1ed9 100644 --- a/website/content/docs/platform/k8s/helm/index.mdx +++ b/website/content/docs/platform/k8s/helm/index.mdx @@ -59,6 +59,15 @@ cluster](https://kubernetes.io/docs/tasks/administer-cluster/securing-a-cluster/ options](/vault/docs/platform/k8s/helm/configuration), and read the [production deployment checklist](/vault/docs/platform/k8s/helm/run#architecture). + + + + If you use AWS features (e.g, AWS PrivateLink) that require a network load + balancer (NLB), you must provision your NLB **before** your application load + balancer (ALB). + + + ## Tutorial Refer to the [Kubernetes](/vault/tutorials/kubernetes) From 2fc8e35ec355f59f7b8669f1a089c997245e2859 Mon Sep 17 00:00:00 2001 From: Andrew Talbot Date: Fri, 20 Sep 2024 17:39:16 -0400 Subject: [PATCH 4/8] docs(lambda-extension): update distributed tracing headers warning; (#28319) * docs(lambda-extension): update cache header warning; * docs(lambda-extension): tweak language; * docs(lambda-extension): accept pr comment Co-authored-by: Sarah Chavis <62406755+schavis@users.noreply.github.com> --------- Co-authored-by: Sarah Chavis <62406755+schavis@users.noreply.github.com> --- .../docs/platform/aws/lambda-extension.mdx | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/website/content/docs/platform/aws/lambda-extension.mdx b/website/content/docs/platform/aws/lambda-extension.mdx index 94b6b8b8d99e..1df42ecf0cee 100644 --- a/website/content/docs/platform/aws/lambda-extension.mdx +++ b/website/content/docs/platform/aws/lambda-extension.mdx @@ -243,12 +243,16 @@ request will be forwarded to Vault and the response returned and cached. Currently, the cache key is a hash of the request URL path, headers, body, and token. - - - The Vault Lambda Extension cache key includes headers from proxy requests. As - a result, distributed tracing headers like `traceparent` can lead to - individualized hashes that make repeated requests appear unique and cause cache - misses. + + + The Vault Lambda Extension cache key includes headers from proxy requests, but + excludes the standard distributed tracing headers `traceparent` and + `tracestate` because trace IDs are unique per request and would lead to unique + hashes for repeated requests. + + Some distributed tracing tools may add nonstandard tracing headers, which can + also lead to individualized hashes that make repeated requests unique and + cause cache misses. From fc5ed22fd140300bfca7e1f1087227b6cfc03046 Mon Sep 17 00:00:00 2001 From: Brian Howe <30811239+bhowe34@users.noreply.github.com> Date: Mon, 23 Sep 2024 07:46:15 -0500 Subject: [PATCH 5/8] pass context to S3 and dynamoDB storage calls (#27927) * pass context to S3 and dynamoDB storage calls * add changelog * fix changelog --- changelog/27927.txt | 6 ++++++ physical/dynamodb/dynamodb.go | 18 +++++++++--------- physical/s3/s3.go | 8 ++++---- 3 files changed, 19 insertions(+), 13 deletions(-) create mode 100644 changelog/27927.txt diff --git a/changelog/27927.txt b/changelog/27927.txt new file mode 100644 index 000000000000..afc37a7acbd3 --- /dev/null +++ b/changelog/27927.txt @@ -0,0 +1,6 @@ +```release-note:improvement +storage/s3: Pass context to AWS SDK calls +``` +```release-note:improvement +storage/dynamodb: Pass context to AWS SDK calls +``` diff --git a/physical/dynamodb/dynamodb.go b/physical/dynamodb/dynamodb.go index c4484d20d446..bc27def0c987 100644 --- a/physical/dynamodb/dynamodb.go +++ b/physical/dynamodb/dynamodb.go @@ -294,7 +294,7 @@ func (d *DynamoDBBackend) Put(ctx context.Context, entry *physical.Entry) error }) } - return d.batchWriteRequests(requests) + return d.batchWriteRequests(ctx, requests) } // Get is used to fetch an entry @@ -304,7 +304,7 @@ func (d *DynamoDBBackend) Get(ctx context.Context, key string) (*physical.Entry, d.permitPool.Acquire() defer d.permitPool.Release() - resp, err := d.client.GetItem(&dynamodb.GetItemInput{ + resp, err := d.client.GetItemWithContext(ctx, &dynamodb.GetItemInput{ TableName: aws.String(d.table), ConsistentRead: aws.Bool(true), Key: map[string]*dynamodb.AttributeValue{ @@ -363,7 +363,7 @@ func (d *DynamoDBBackend) Delete(ctx context.Context, key string) error { excluded = append(excluded, recordKeyForVaultKey(prefixes[index-1])) } - hasChildren, err := d.hasChildren(prefix, excluded) + hasChildren, err := d.hasChildren(ctx, prefix, excluded) if err != nil { return err } @@ -387,7 +387,7 @@ func (d *DynamoDBBackend) Delete(ctx context.Context, key string) error { } } - return d.batchWriteRequests(requests) + return d.batchWriteRequests(ctx, requests) } // List is used to list all the keys under a given @@ -420,7 +420,7 @@ func (d *DynamoDBBackend) List(ctx context.Context, prefix string) ([]string, er d.permitPool.Acquire() defer d.permitPool.Release() - err := d.client.QueryPages(queryInput, func(out *dynamodb.QueryOutput, lastPage bool) bool { + err := d.client.QueryPagesWithContext(ctx, queryInput, func(out *dynamodb.QueryOutput, lastPage bool) bool { var record DynamoDBRecord for _, item := range out.Items { dynamodbattribute.UnmarshalMap(item, &record) @@ -443,7 +443,7 @@ func (d *DynamoDBBackend) List(ctx context.Context, prefix string) ([]string, er // before any deletes take place. To account for that hasChildren accepts a slice of // strings representing values we expect to find that should NOT be counted as children // because they are going to be deleted. -func (d *DynamoDBBackend) hasChildren(prefix string, exclude []string) (bool, error) { +func (d *DynamoDBBackend) hasChildren(ctx context.Context, prefix string, exclude []string) (bool, error) { prefix = strings.TrimSuffix(prefix, "/") prefix = escapeEmptyPath(prefix) @@ -473,7 +473,7 @@ func (d *DynamoDBBackend) hasChildren(prefix string, exclude []string) (bool, er d.permitPool.Acquire() defer d.permitPool.Release() - out, err := d.client.Query(queryInput) + out, err := d.client.QueryWithContext(ctx, queryInput) if err != nil { return false, err } @@ -519,7 +519,7 @@ func (d *DynamoDBBackend) HAEnabled() bool { // batchWriteRequests takes a list of write requests and executes them in badges // with a maximum size of 25 (which is the limit of BatchWriteItem requests). -func (d *DynamoDBBackend) batchWriteRequests(requests []*dynamodb.WriteRequest) error { +func (d *DynamoDBBackend) batchWriteRequests(ctx context.Context, requests []*dynamodb.WriteRequest) error { for len(requests) > 0 { batchSize := int(math.Min(float64(len(requests)), 25)) batch := map[string][]*dynamodb.WriteRequest{d.table: requests[:batchSize]} @@ -534,7 +534,7 @@ func (d *DynamoDBBackend) batchWriteRequests(requests []*dynamodb.WriteRequest) for len(batch) > 0 { var output *dynamodb.BatchWriteItemOutput - output, err = d.client.BatchWriteItem(&dynamodb.BatchWriteItemInput{ + output, err = d.client.BatchWriteItemWithContext(ctx, &dynamodb.BatchWriteItemInput{ RequestItems: batch, }) if err != nil { diff --git a/physical/s3/s3.go b/physical/s3/s3.go index da82acccd3ca..b1687a91622e 100644 --- a/physical/s3/s3.go +++ b/physical/s3/s3.go @@ -183,7 +183,7 @@ func (s *S3Backend) Put(ctx context.Context, entry *physical.Entry) error { putObjectInput.SSEKMSKeyId = aws.String(s.kmsKeyId) } - _, err := s.client.PutObject(putObjectInput) + _, err := s.client.PutObjectWithContext(ctx, putObjectInput) if err != nil { return err } @@ -201,7 +201,7 @@ func (s *S3Backend) Get(ctx context.Context, key string) (*physical.Entry, error // Setup key key = path.Join(s.path, key) - resp, err := s.client.GetObject(&s3.GetObjectInput{ + resp, err := s.client.GetObjectWithContext(ctx, &s3.GetObjectInput{ Bucket: aws.String(s.bucket), Key: aws.String(key), }) @@ -254,7 +254,7 @@ func (s *S3Backend) Delete(ctx context.Context, key string) error { // Setup key key = path.Join(s.path, key) - _, err := s.client.DeleteObject(&s3.DeleteObjectInput{ + _, err := s.client.DeleteObjectWithContext(ctx, &s3.DeleteObjectInput{ Bucket: aws.String(s.bucket), Key: aws.String(key), }) @@ -289,7 +289,7 @@ func (s *S3Backend) List(ctx context.Context, prefix string) ([]string, error) { keys := []string{} - err := s.client.ListObjectsV2Pages(params, + err := s.client.ListObjectsV2PagesWithContext(ctx, params, func(page *s3.ListObjectsV2Output, lastPage bool) bool { if page != nil { // Add truncated 'folder' paths From efd2fb2ae4ed725a1c53809828a666fef3d218df Mon Sep 17 00:00:00 2001 From: framsouza Date: Mon, 23 Sep 2024 16:09:05 +0200 Subject: [PATCH 6/8] [docs] Updating Kubernetes upgrade instruction (#25286) * [docs] Updating kubernetes upgrade instruction * Fixing code block * Update website/content/docs/platform/k8s/helm/run.mdx Co-authored-by: Sarah Chavis <62406755+schavis@users.noreply.github.com> * Update website/content/docs/platform/k8s/helm/run.mdx Co-authored-by: Sarah Chavis <62406755+schavis@users.noreply.github.com> * Update website/content/docs/platform/k8s/helm/run.mdx Co-authored-by: Sarah Chavis <62406755+schavis@users.noreply.github.com> * Update website/content/docs/platform/k8s/helm/run.mdx Co-authored-by: Sarah Chavis <62406755+schavis@users.noreply.github.com> * Update website/content/docs/platform/k8s/helm/run.mdx Co-authored-by: Sarah Chavis <62406755+schavis@users.noreply.github.com> --------- Co-authored-by: Peter Wilson Co-authored-by: Sarah Chavis <62406755+schavis@users.noreply.github.com> Co-authored-by: divyaac Co-authored-by: Violet Hynes --- .../content/docs/platform/k8s/helm/run.mdx | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/website/content/docs/platform/k8s/helm/run.mdx b/website/content/docs/platform/k8s/helm/run.mdx index 95184caaf18b..261d227eda9e 100644 --- a/website/content/docs/platform/k8s/helm/run.mdx +++ b/website/content/docs/platform/k8s/helm/run.mdx @@ -436,7 +436,25 @@ running: $ kubectl delete pod ``` -If Vault is deployed using `ha` mode, the standby pods must be upgraded first. + +If you deployed Vault in high availability (`ha`) mode, you must upgrade your +standby pods before upgrading the active pod: + +1. Before deleting the standby pod, remove the associated node from the raft + with `vault operator raft remove-peer `. +1. Confirm Vault removed the node successfully from Raft with + `vault operator raft list-peers`. +1. Once you confirm the removal, delete the pod. + + + +Removing a pod without first deleting the node from its cluster means that +Raft will not be aware of the correct number of nodes in the cluster. Not knowing +the correct number of nodes can trigger a leader election, which can potentially +cause unneeded downtime. + + + Vault has K8s service discovery built in (when enabled in the server configuration) and will automatically change the labels of the pod with its current leader status. These labels can be used to filter the pods. From 7c1a83422b906654682441a3af57ab684cfbb623 Mon Sep 17 00:00:00 2001 From: "Luis (LT) Carbonell" Date: Mon, 23 Sep 2024 10:55:20 -0400 Subject: [PATCH 7/8] Improve Error Handling for Missing Credentials in AppRole and UserPass (#28441) * Return invalid credentials for missing login parameters (400 vs 500) * Add changelog * Update test --- builtin/credential/approle/path_login.go | 2 +- builtin/credential/userpass/path_login.go | 2 +- changelog/28441.txt | 3 +++ vault/external_tests/delegated_auth/delegated_auth_test.go | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) create mode 100644 changelog/28441.txt diff --git a/builtin/credential/approle/path_login.go b/builtin/credential/approle/path_login.go index 72b7ac352f06..ed1bc2ff0c36 100644 --- a/builtin/credential/approle/path_login.go +++ b/builtin/credential/approle/path_login.go @@ -125,7 +125,7 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat // RoleID must be supplied during every login roleID := strings.TrimSpace(data.Get("role_id").(string)) if roleID == "" { - return logical.ErrorResponse("missing role_id"), nil + return nil, logical.ErrInvalidCredentials } // Look for the storage entry that maps the roleID to role diff --git a/builtin/credential/userpass/path_login.go b/builtin/credential/userpass/path_login.go index b53953ee837a..37fc7fbde570 100644 --- a/builtin/credential/userpass/path_login.go +++ b/builtin/credential/userpass/path_login.go @@ -67,7 +67,7 @@ func (b *backend) pathLogin(ctx context.Context, req *logical.Request, d *framew password := d.Get("password").(string) if password == "" { - return nil, fmt.Errorf("missing password") + return nil, logical.ErrInvalidCredentials } // Get the user and validate auth diff --git a/changelog/28441.txt b/changelog/28441.txt new file mode 100644 index 000000000000..e78ed504af22 --- /dev/null +++ b/changelog/28441.txt @@ -0,0 +1,3 @@ +```release-note:bug +auth: Updated error handling for missing login credentials in AppRole and UserPass auth methods to return a 400 error instead of a 500 error. +``` diff --git a/vault/external_tests/delegated_auth/delegated_auth_test.go b/vault/external_tests/delegated_auth/delegated_auth_test.go index c50077ffe5f9..6eea412ecfea 100644 --- a/vault/external_tests/delegated_auth/delegated_auth_test.go +++ b/vault/external_tests/delegated_auth/delegated_auth_test.go @@ -327,7 +327,7 @@ func TestDelegatedAuth(t *testing.T) { path: "login", username: "allowed-est", password: "", - errorContains: "missing password", + errorContains: "invalid credentials", }, { name: "bad-path-within-delegated-auth-error", From 13de05393507d55a7a3313ddb4e454131e40c39b Mon Sep 17 00:00:00 2001 From: Steven Clark Date: Mon, 23 Sep 2024 12:22:11 -0400 Subject: [PATCH 8/8] Do not shadown err within MSSQL test container intialization (#28468) - Get better test failure error messages by not shadowing the errors when we are attempting to start the MSSQL docker container, so we can fail the tests with the proper error message that is occuring instead of mssqlhelper.go:60: Could not start docker MSSQL: %!s() --- helper/testhelpers/mssql/mssqlhelper.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/helper/testhelpers/mssql/mssqlhelper.go b/helper/testhelpers/mssql/mssqlhelper.go index 94b34a4f3d85..71d867de260d 100644 --- a/helper/testhelpers/mssql/mssqlhelper.go +++ b/helper/testhelpers/mssql/mssqlhelper.go @@ -35,7 +35,8 @@ func PrepareMSSQLTestContainer(t *testing.T) (cleanup func(), retURL string) { var err error for i := 0; i < numRetries; i++ { var svc *docker.Service - runner, err := docker.NewServiceRunner(docker.RunOptions{ + var runner *docker.Runner + runner, err = docker.NewServiceRunner(docker.RunOptions{ ContainerName: "sqlserver", ImageRepo: "mcr.microsoft.com/mssql/server", ImageTag: "2017-latest-ubuntu", @@ -48,7 +49,8 @@ func PrepareMSSQLTestContainer(t *testing.T) (cleanup func(), retURL string) { }, }) if err != nil { - t.Fatalf("Could not start docker MSSQL: %s", err) + t.Logf("Could not start docker MSSQL: %v", err) + continue } svc, err = runner.StartService(context.Background(), connectMSSQL) @@ -57,7 +59,7 @@ func PrepareMSSQLTestContainer(t *testing.T) (cleanup func(), retURL string) { } } - t.Fatalf("Could not start docker MSSQL: %s", err) + t.Fatalf("Could not start docker MSSQL: %v", err) return nil, "" }