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

Add directory paths to KV capabilities checks #24404

Merged
merged 8 commits into from
Dec 7, 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/24404.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
ui: fix issue where kv v2 capabilities checks were not passing in the full secret path if secret was inside a directory.
```
18 changes: 13 additions & 5 deletions ui/app/models/kv/data.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,20 @@ export default class KvSecretDataModel extends Model {
return isDeleted(this.deletionTime);
}

get permissionsPath() {
return this.fullSecretPath || this.path;
Monkeychip marked this conversation as resolved.
Show resolved Hide resolved
}

// Permissions
@lazyCapabilities(apiPath`${'backend'}/data/${'path'}`, 'backend', 'path') dataPath;
@lazyCapabilities(apiPath`${'backend'}/metadata/${'path'}`, 'backend', 'path') metadataPath;
@lazyCapabilities(apiPath`${'backend'}/delete/${'path'}`, 'backend', 'path') deletePath;
@lazyCapabilities(apiPath`${'backend'}/destroy/${'path'}`, 'backend', 'path') destroyPath;
@lazyCapabilities(apiPath`${'backend'}/undelete/${'path'}`, 'backend', 'path') undeletePath;
@lazyCapabilities(apiPath`${'backend'}/data/${'permissionsPath'}`, 'backend', 'permissionsPath') dataPath;
@lazyCapabilities(apiPath`${'backend'}/metadata/${'permissionsPath'}`, 'backend', 'permissionsPath')
metadataPath;
@lazyCapabilities(apiPath`${'backend'}/delete/${'permissionsPath'}`, 'backend', 'permissionsPath')
deletePath;
@lazyCapabilities(apiPath`${'backend'}/destroy/${'permissionsPath'}`, 'backend', 'permissionsPath')
destroyPath;
@lazyCapabilities(apiPath`${'backend'}/undelete/${'permissionsPath'}`, 'backend', 'permissionsPath')
undeletePath;

get canDeleteLatestVersion() {
return this.dataPath.get('canDelete') !== false;
Expand Down
9 changes: 7 additions & 2 deletions ui/app/models/kv/metadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,14 @@ export default class KvSecretMetadataModel extends Model {
};
}

get permissionsPath() {
return this.fullSecretPath || this.path;
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why fullSecretPath wouldn't always be the value to use if it represents the full path to the secret.

Copy link
Contributor Author

@Monkeychip Monkeychip Dec 7, 2023

Choose a reason for hiding this comment

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

Because it's generated by the serializer (not easy to see from the diff). And the payload does not always return data.keys. See here.

An example of when the serializer wouldn't return this.fullSecretPath is when we're looking at a secret's details.

}

// permissions needed for the list view where kv/data has not yet been called. Allows us to conditionally show action items in the LinkedBlock popups.
@lazyCapabilities(apiPath`${'backend'}/data/${'path'}`, 'backend', 'path') dataPath;
@lazyCapabilities(apiPath`${'backend'}/metadata/${'path'}`, 'backend', 'path') metadataPath;
@lazyCapabilities(apiPath`${'backend'}/data/${'permissionsPath'}`, 'backend', 'permissionsPath') dataPath;
@lazyCapabilities(apiPath`${'backend'}/metadata/${'permissionsPath'}`, 'backend', 'permissionsPath')
metadataPath;

get canDeleteMetadata() {
return this.metadataPath.get('canDelete') !== false;
Expand Down
7 changes: 6 additions & 1 deletion ui/lib/kv/addon/components/page/list.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,11 @@
{{/if}}
{{#if metadata.canCreateVersionData}}
<li>
<LinkTo @route="secret.details.edit" @model={{metadata.fullSecretPath}}>
<LinkTo
@route="secret.details.edit"
@model={{metadata.fullSecretPath}}
data-test-popup-create-new-version
>
Create new version
</LinkTo>
</li>
Expand All @@ -106,6 +110,7 @@
@isInDropdown={{true}}
@onConfirmAction={{fn this.onDelete metadata}}
@confirmMessage="This will permanently delete this secret and all its versions."
data-test-popup-metadata-delete
/>
{{/if}}
{{/if}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1003,6 +1003,26 @@ module('Acceptance | kv-v2 workflow | secret and version create', function (hook
});
});

module('secret-nested-creator persona', function (hooks) {
hooks.beforeEach(async function () {
const token = await runCmd(
tokenWithPolicyCmd('secret-nested-creator', personas.secretNestedCreator(this.backend))
);
await authPage.login(token);
clearRecords(this.store);
return;
});
test('can create a secret from the nested list view (snc)', async function (assert) {
assert.expect(1);
// go to nested secret directory list view
await visit(`/vault/secrets/${this.backend}/kv/list/app/`);
// correct popup menu items appear on list view
const popupSelector = `${PAGE.list.item('first')} ${PAGE.popup}`;
await click(popupSelector);
assert.dom(PAGE.list.listMenuCreate).exists('shows the option to create new version');
});
});

module('enterprise controlled access persona', function (hooks) {
hooks.beforeEach(async function () {
this.controlGroup = this.owner.lookup('service:control-group');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,11 @@ module('Acceptance | kv-v2 workflow | delete, undelete, destroy', function (hook
this.store = this.owner.lookup('service:store');
this.backend = `kv-delete-${uuidv4()}`;
this.secretPath = 'bad-secret';
this.nestedSecretPath = 'app/nested/bad-secret';
await authPage.login();
await runCmd(mountEngineCmd('kv-v2', this.backend), false);
await writeVersionedSecret(this.backend, this.secretPath, 'foo', 'bar', 4);
await writeVersionedSecret(this.backend, this.nestedSecretPath, 'foo', 'bar', 1);
await writeVersionedSecret(this.backend, 'nuke', 'foo', 'bar', 2);
// Delete latest version for testing undelete for users that can't delete
await runCmd(deleteLatestCmd(this.backend, 'nuke'));
Expand Down Expand Up @@ -353,6 +355,33 @@ module('Acceptance | kv-v2 workflow | delete, undelete, destroy', function (hook
});
});

module('secret-nested-creator persona', function (hooks) {
hooks.beforeEach(async function () {
const token = await runCmd(
tokenWithPolicyCmd('secret-nested-creator', personas.secretNestedCreator(this.backend))
);
await authPage.login(token);
clearRecords(this.store);
return;
});
test('can delete all secret versions from the nested list view (snc)', async function (assert) {
assert.expect(1);
// go to nested secret directory list view
await visit(`/vault/secrets/${this.backend}/kv/list/app/nested`);
// correct popup menu items appear on list view
const popupSelector = `${PAGE.list.item('bad-secret')} ${PAGE.popup}`;
await click(popupSelector);
assert.dom(PAGE.list.listMenuDelete).exists('shows the option to permanently delete');
});
test('can not delete all secret versions from root list view (snc)', async function (assert) {
assert.expect(1);
// go to root secret directory list view
await visit(`/vault/secrets/${this.backend}/kv/list`);
// shows overview card and not list view
assert.dom(PAGE.list.overviewCard).exists('renders overview card');
});
});

module('secret-creator persona', function (hooks) {
hooks.beforeEach(async function () {
const token = await runCmd(tokenWithPolicyCmd('secret-creator', personas.secretCreator(this.backend)));
Expand Down
2 changes: 2 additions & 0 deletions ui/tests/helpers/kv/kv-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ export const PAGE = {
createSecret: '[data-test-toolbar-create-secret]',
item: (secret) => (!secret ? '[data-test-list-item]' : `[data-test-list-item="${secret}"]`),
filter: `[data-test-kv-list-filter]`,
listMenuDelete: `[data-test-popup-metadata-delete]`,
listMenuCreate: `[data-test-popup-create-new-version]`,
overviewCard: '[data-test-overview-card-container="View secret"]',
overviewInput: '[data-test-view-secret] input',
overviewButton: '[data-test-get-secret-detail]',
Expand Down
20 changes: 19 additions & 1 deletion ui/tests/helpers/policy-generator/kv.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ export const dataPolicy = ({ backend, secretPath = '*', capabilities = root }) =
`;
};

export const dataNestedPolicy = ({ backend, secretPath = '*', capabilities = root }) => {
return `
path "${backend}/data/app/${secretPath}" {
capabilities = [${format(capabilities)}]
}
`;
};

export const metadataPolicy = ({ backend, secretPath = '*', capabilities = root }) => {
// "delete" capability on this path can destroy all versions
return `
Expand All @@ -34,6 +42,14 @@ export const metadataPolicy = ({ backend, secretPath = '*', capabilities = root
`;
};

export const metadataNestedPolicy = ({ backend, secretPath = '*', capabilities = root }) => {
return `
path "${backend}/metadata/app/${secretPath}" {
capabilities = [${format(capabilities)}]
}
`;
};

export const metadataListPolicy = (backend) => {
return `
path "${backend}/metadata" {
Expand Down Expand Up @@ -71,11 +87,13 @@ export const personas = {
dataListReader: (backend) =>
dataPolicy({ backend, capabilities: ['read', 'delete'] }) + metadataListPolicy(backend),
metadataMaintainer: (backend) =>
metadataListPolicy(backend) +
metadataPolicy({ backend, capabilities: ['create', 'read', 'update', 'list'] }) +
deleteVersionsPolicy({ backend }) +
undeleteVersionsPolicy({ backend }) +
destroyVersionsPolicy({ backend }),
secretNestedCreator: (backend) =>
dataNestedPolicy({ backend, capabilities: ['create', 'update'] }) +
metadataNestedPolicy({ backend, capabilities: ['list', 'delete'] }),
secretCreator: (backend) =>
dataPolicy({ backend, capabilities: ['create', 'update'] }) +
metadataPolicy({ backend, capabilities: ['delete'] }),
Expand Down
Loading