diff --git a/changelog/28698.txt b/changelog/28698.txt new file mode 100644 index 0000000000..7a9eb05292 --- /dev/null +++ b/changelog/28698.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: No longer running decodeURIComponent on KVv2 list view allowing percent encoded data-octets in path name. +``` diff --git a/ui/app/models/kv/data.js b/ui/app/models/kv/data.js index 19b07fac95..37ab01e3aa 100644 --- a/ui/app/models/kv/data.js +++ b/ui/app/models/kv/data.js @@ -52,7 +52,11 @@ const validations = { @withFormFields() export default class KvSecretDataModel extends Model { @attr('string') backend; // dynamic path of secret -- set on response from value passed to queryRecord. - @attr('string', { label: 'Path for this secret' }) path; + @attr('string', { + label: 'Path for this secret', + subText: 'Names with forward slashes define hierarchical path structures.', + }) + path; @attr('object') secretData; // { key: value } data of the secret version // Params returned on the GET response. diff --git a/ui/lib/kv/addon/routes/list-directory.js b/ui/lib/kv/addon/routes/list-directory.js index 6190bd6844..3ead4324d8 100644 --- a/ui/lib/kv/addon/routes/list-directory.js +++ b/ui/lib/kv/addon/routes/list-directory.js @@ -6,9 +6,7 @@ import Route from '@ember/routing/route'; import { service } from '@ember/service'; import { hash } from 'rsvp'; -import { normalizePath } from 'vault/utils/path-encoding-helpers'; -import { breadcrumbsForSecret } from 'kv/utils/kv-breadcrumbs'; -import { pathIsDirectory } from 'kv/utils/kv-breadcrumbs'; +import { pathIsDirectory, breadcrumbsForSecret } from 'kv/utils/kv-breadcrumbs'; export default class KvSecretsListRoute extends Route { @service store; @@ -48,7 +46,9 @@ export default class KvSecretsListRoute extends Route { getPathToSecret(pathParam) { if (!pathParam) return ''; // links and routing assumes pathToParam includes trailing slash - return pathIsDirectory(pathParam) ? normalizePath(pathParam) : normalizePath(`${pathParam}/`); + // users may want to include a percent-encoded octet like %2f in their path. Example: 'foo%2fbar' or non-data octets like 'foo%bar'. + // we are assuming the user intended to include these characters in their path and we should not decode them. + return pathIsDirectory(pathParam) ? pathParam : `${pathParam}/`; } model(params) { diff --git a/ui/tests/acceptance/secrets/backend/kv/kv-v2-workflow-navigation-test.js b/ui/tests/acceptance/secrets/backend/kv/kv-v2-workflow-navigation-test.js index 854fb173b7..e1ac5dc932 100644 --- a/ui/tests/acceptance/secrets/backend/kv/kv-v2-workflow-navigation-test.js +++ b/ui/tests/acceptance/secrets/backend/kv/kv-v2-workflow-navigation-test.js @@ -125,6 +125,117 @@ module('Acceptance | kv-v2 workflow | navigation', function (hooks) { return; }); + test('KVv2 handles secret with % and space in path correctly', async function (assert) { + // To check this bug no longer happens: https://github.com/hashicorp/vault/issues/11616 + await navToBackend(this.backend); + await click(PAGE.list.createSecret); + const pathWithSpace = 'per%centfu ll'; + await typeIn(GENERAL.inputByAttr('path'), pathWithSpace); + await fillIn(FORM.keyInput(), 'someKey'); + await fillIn(FORM.maskedValueInput(), 'someValue'); + await click(FORM.saveBtn); + assert.dom(PAGE.title).hasText(pathWithSpace, 'title is full path without any encoding/decoding.'); + assert + .dom(PAGE.breadcrumbAtIdx(1)) + .hasText(this.backend, 'breadcrumb before secret path is backend path'); + assert + .dom(PAGE.breadcrumbCurrentAtIdx(2)) + .hasText('per%centfu ll', 'the current breadcrumb is value of the secret path'); + + await click(PAGE.breadcrumbAtIdx(1)); + assert + .dom(`${PAGE.list.item(pathWithSpace)} [data-test-path]`) + .hasText(pathWithSpace, 'the list item is shown correctly'); + + await typeIn(PAGE.list.filter, 'per%'); + await click('[data-test-kv-list-filter-submit]'); + assert + .dom(`${PAGE.list.item(pathWithSpace)} [data-test-path]`) + .hasText(pathWithSpace, 'the list item is shown correctly after filtering'); + + await click(PAGE.list.item(pathWithSpace)); + assert.strictEqual( + currentURL(), + `/vault/secrets/${this.backend}/kv/${encodeURIComponent(pathWithSpace)}`, + 'Path is encoded in the URL' + ); + }); + + test('KVv2 handles nested secret with % and space in path correctly', async function (assert) { + await navToBackend(this.backend); + await click(PAGE.list.createSecret); + const nestedPathWithSpace = 'per%/centfu ll'; + await typeIn(GENERAL.inputByAttr('path'), nestedPathWithSpace); + await fillIn(FORM.keyInput(), 'someKey'); + await fillIn(FORM.maskedValueInput(), 'someValue'); + await click(FORM.saveBtn); + assert + .dom(PAGE.title) + .hasText( + nestedPathWithSpace, + 'title is of the full nested path (directory included) without any encoding/decoding.' + ); + assert.dom(PAGE.breadcrumbAtIdx(2)).hasText('per%'); + assert + .dom(PAGE.breadcrumbCurrentAtIdx(3)) + .hasText('centfu ll', 'the current breadcrumb is value centfu ll'); + + await click(PAGE.breadcrumbAtIdx(1)); + assert + .dom(`${PAGE.list.item('per%/')} [data-test-path]`) + .hasText('per%/', 'the directory item is shown correctly'); + + await typeIn(PAGE.list.filter, 'per%/'); + await click('[data-test-kv-list-filter-submit]'); + assert + .dom(`${PAGE.list.item('centfu ll')} [data-test-path]`) + .hasText('centfu ll', 'the list item is shown correctly after filtering'); + + await click(PAGE.list.item('centfu ll')); + assert.strictEqual( + currentURL(), + `/vault/secrets/${this.backend}/kv/${encodeURIComponent(nestedPathWithSpace)}`, + 'Path is encoded in the URL' + ); + }); + + test('KVv2 handles nested secret with a percent-encoded data octet in path correctly', async function (assert) { + // To check this bug no longer happens: https://github.com/hashicorp/vault/issues/25905 + await navToBackend(this.backend); + await click(PAGE.list.createSecret); + const pathDataOctet = 'hello/foo%2fbar/world'; + await typeIn(GENERAL.inputByAttr('path'), pathDataOctet); + await fillIn(FORM.keyInput(), 'someKey'); + await fillIn(FORM.maskedValueInput(), 'someValue'); + await click(FORM.saveBtn); + assert + .dom(PAGE.title) + .hasText( + pathDataOctet, + 'title is of the full nested path (directory included) without any encoding/decoding.' + ); + assert + .dom(PAGE.breadcrumbAtIdx(2)) + .hasText('hello', 'hello is the first directory and shows up as a separate breadcrumb'); + assert + .dom(PAGE.breadcrumbAtIdx(3)) + .hasText('foo%2fbar', 'foo%2fbar is the second directory and shows up as a separate breadcrumb'); + assert.dom(PAGE.breadcrumbCurrentAtIdx(4)).hasText('world', 'the current breadcrumb is value world'); + + await click(PAGE.breadcrumbAtIdx(2)); + assert + .dom(`${PAGE.list.item('foo%2fbar/')} [data-test-path]`) + .hasText('foo%2fbar/', 'the directory item is shown correctly'); + + await click(PAGE.list.item('foo%2fbar/')); + await click(PAGE.list.item('world')); + assert.strictEqual( + currentURL(), + `/vault/secrets/${this.backend}/kv/${encodeURIComponent(pathDataOctet)}`, + 'Path is encoded in the URL' + ); + }); + module('admin persona', function (hooks) { hooks.beforeEach(async function () { const token = await runCmd( diff --git a/ui/tests/acceptance/secrets/backend/kv/secret-test.js b/ui/tests/acceptance/secrets/backend/kv/secret-test.js index f4ca3bb2f7..184cfd83e1 100644 --- a/ui/tests/acceptance/secrets/backend/kv/secret-test.js +++ b/ui/tests/acceptance/secrets/backend/kv/secret-test.js @@ -256,7 +256,7 @@ module('Acceptance | secrets/secret/create, read, delete', function (hooks) { await deleteEngine(backend, assert); }); - test('UI handles secret with % in path correctly', async function (assert) { + test('KVv1 handles secret with % in path correctly', async function (assert) { const enginePath = this.backend; const secretPath = 'per%cent/%fu ll'; const [firstPath, secondPath] = secretPath.split('/'); diff --git a/ui/tests/helpers/kv/kv-selectors.js b/ui/tests/helpers/kv/kv-selectors.js index 77e7b91416..3c0b4af42c 100644 --- a/ui/tests/helpers/kv/kv-selectors.js +++ b/ui/tests/helpers/kv/kv-selectors.js @@ -9,6 +9,8 @@ export const PAGE = { breadcrumbs: '[data-test-breadcrumbs]', breadcrumb: '[data-test-breadcrumbs] li', breadcrumbAtIdx: (idx) => `[data-test-breadcrumbs] li:nth-child(${idx + 1}) a`, + breadcrumbCurrentAtIdx: (idx) => + `[data-test-breadcrumbs] li:nth-child(${idx + 1}) .hds-breadcrumb__current`, infoRow: '[data-test-component="info-table-row"]', infoRowValue: (label) => `[data-test-value-div="${label}"]`, infoRowToggleMasked: (label) => `[data-test-value-div="${label}"] [data-test-button="toggle-masked"]`, diff --git a/ui/tests/unit/decorators/model-form-fields-test.js b/ui/tests/unit/decorators/model-form-fields-test.js index 6d824ce9d2..89080bbc34 100644 --- a/ui/tests/unit/decorators/model-form-fields-test.js +++ b/ui/tests/unit/decorators/model-form-fields-test.js @@ -43,6 +43,7 @@ module('Unit | Decorators | ModelFormFields', function (hooks) { name: 'path', options: { label: 'Path for this secret', + subText: 'Names with forward slashes define hierarchical path structures.', }, type: 'string', },