From bfb3e256ff5aea7ff1f8a4d39d0ed7d050ec469e Mon Sep 17 00:00:00 2001 From: claire bontempo Date: Mon, 6 Jan 2025 08:54:58 -0600 Subject: [PATCH 01/13] refactor crumbs --- ui/lib/ldap/addon/routes/roles/role/credentials.ts | 7 +++++-- ui/lib/ldap/addon/routes/roles/role/details.ts | 8 ++++++-- ui/lib/ldap/addon/routes/roles/role/edit.ts | 8 ++++++-- ui/lib/ldap/addon/routes/roles/subdirectory.ts | 10 ++++++++-- ui/lib/ldap/addon/utils/ldap-breadcrumbs.ts | 11 ++++++----- ui/tests/unit/utils/ldap-breadcrumbs-test.js | 7 +++++-- 6 files changed, 36 insertions(+), 15 deletions(-) diff --git a/ui/lib/ldap/addon/routes/roles/role/credentials.ts b/ui/lib/ldap/addon/routes/roles/role/credentials.ts index a3082de8431d..70c68c813568 100644 --- a/ui/lib/ldap/addon/routes/roles/role/credentials.ts +++ b/ui/lib/ldap/addon/routes/roles/role/credentials.ts @@ -5,7 +5,7 @@ import Route from '@ember/routing/route'; import { service } from '@ember/service'; -import { ldapBreadcrumbs } from 'ldap/utils/ldap-breadcrumbs'; +import { ldapBreadcrumbs, roleRoutes } from 'ldap/utils/ldap-breadcrumbs'; import type Store from '@ember-data/store'; import type LdapRoleModel from 'vault/models/ldap/role'; @@ -58,11 +58,14 @@ export default class LdapRoleCredentialsRoute extends Route { super.setupController(controller, resolvedModel, transition); const role = this.modelFor('roles.role') as LdapRoleModel; + const routeParams = (childResource: string) => { + return [role.backend, role.type, childResource]; + }; controller.breadcrumbs = [ { label: 'Secrets', route: 'secrets', linkExternal: true }, { label: role.backend, route: 'overview' }, { label: 'Roles', route: 'roles' }, - ...ldapBreadcrumbs(role.name, role.type, role.backend), + ...ldapBreadcrumbs(role.name, routeParams, roleRoutes), { label: 'Credentials' }, ]; } diff --git a/ui/lib/ldap/addon/routes/roles/role/details.ts b/ui/lib/ldap/addon/routes/roles/role/details.ts index 0b6ee9afa490..c8c7ecb65236 100644 --- a/ui/lib/ldap/addon/routes/roles/role/details.ts +++ b/ui/lib/ldap/addon/routes/roles/role/details.ts @@ -5,7 +5,7 @@ import Route from '@ember/routing/route'; import { service } from '@ember/service'; -import { ldapBreadcrumbs } from 'ldap/utils/ldap-breadcrumbs'; +import { ldapBreadcrumbs, roleRoutes } from 'ldap/utils/ldap-breadcrumbs'; import type { Breadcrumb } from 'vault/vault/app-types'; import type Controller from '@ember/controller'; @@ -24,11 +24,15 @@ export default class LdapRolesRoleDetailsRoute extends Route { setupController(controller: RouteController, resolvedModel: LdapRoleModel, transition: Transition) { super.setupController(controller, resolvedModel, transition); + const routeParams = (childResource: string) => { + return [this.secretMountPath.currentPath, resolvedModel.type, childResource]; + }; + controller.breadcrumbs = [ { label: 'Secrets', route: 'secrets', linkExternal: true }, { label: resolvedModel.backend, route: 'overview' }, { label: 'Roles', route: 'roles' }, - ...ldapBreadcrumbs(resolvedModel.name, resolvedModel.type, this.secretMountPath.currentPath, true), + ...ldapBreadcrumbs(resolvedModel.name, routeParams, roleRoutes, true), ]; } } diff --git a/ui/lib/ldap/addon/routes/roles/role/edit.ts b/ui/lib/ldap/addon/routes/roles/role/edit.ts index c12ad20e3a3f..df383ad03cad 100644 --- a/ui/lib/ldap/addon/routes/roles/role/edit.ts +++ b/ui/lib/ldap/addon/routes/roles/role/edit.ts @@ -4,7 +4,7 @@ */ import Route from '@ember/routing/route'; -import { ldapBreadcrumbs } from 'ldap/utils/ldap-breadcrumbs'; +import { ldapBreadcrumbs, roleRoutes } from 'ldap/utils/ldap-breadcrumbs'; import type LdapRoleModel from 'vault/models/ldap/role'; import type Controller from '@ember/controller'; @@ -20,11 +20,15 @@ export default class LdapRoleEditRoute extends Route { setupController(controller: RouteController, resolvedModel: LdapRoleModel, transition: Transition) { super.setupController(controller, resolvedModel, transition); + const routeParams = (childResource: string) => { + return [resolvedModel.backend, resolvedModel.type, childResource]; + }; + controller.breadcrumbs = [ { label: 'Secrets', route: 'secrets', linkExternal: true }, { label: resolvedModel.backend, route: 'overview' }, { label: 'Roles', route: 'roles' }, - ...ldapBreadcrumbs(resolvedModel.name, resolvedModel.type, resolvedModel.backend), + ...ldapBreadcrumbs(resolvedModel.name, routeParams, roleRoutes), { label: 'Edit' }, ]; } diff --git a/ui/lib/ldap/addon/routes/roles/subdirectory.ts b/ui/lib/ldap/addon/routes/roles/subdirectory.ts index cb84d232566b..053d996d2cdf 100644 --- a/ui/lib/ldap/addon/routes/roles/subdirectory.ts +++ b/ui/lib/ldap/addon/routes/roles/subdirectory.ts @@ -5,7 +5,7 @@ import LdapRolesRoute from '../roles'; import { hash } from 'rsvp'; -import { ldapBreadcrumbs } from 'ldap/utils/ldap-breadcrumbs'; +import { ldapBreadcrumbs, roleRoutes } from 'ldap/utils/ldap-breadcrumbs'; import type { Breadcrumb } from 'vault/vault/app-types'; import type Controller from '@ember/controller'; @@ -55,11 +55,17 @@ export default class LdapRolesSubdirectoryRoute extends LdapRolesRoute { setupController(controller: RouteController, resolvedModel: RouteModel, transition: Transition) { super.setupController(controller, resolvedModel, transition); const { backendModel, roleAncestry } = resolvedModel; + + const routeParams = (childResource: string) => { + return [backendModel.id, roleAncestry.type, childResource]; + }; + const crumbs = [ { label: 'Secrets', route: 'secrets', linkExternal: true }, { label: backendModel.id, route: 'overview' }, { label: 'Roles', route: 'roles' }, - ...ldapBreadcrumbs(roleAncestry.path_to_role, roleAncestry.type, backendModel.id, true), + + ...ldapBreadcrumbs(roleAncestry.path_to_role, routeParams, roleRoutes, true), ]; // must call 'set' so breadcrumbs update as we navigate through directories diff --git a/ui/lib/ldap/addon/utils/ldap-breadcrumbs.ts b/ui/lib/ldap/addon/utils/ldap-breadcrumbs.ts index cbc21b50564b..039bede32ce0 100644 --- a/ui/lib/ldap/addon/utils/ldap-breadcrumbs.ts +++ b/ui/lib/ldap/addon/utils/ldap-breadcrumbs.ts @@ -4,10 +4,12 @@ */ import type { Breadcrumb } from 'vault/vault/app-types'; +export const roleRoutes = { details: 'roles.role.details', subdirectory: 'roles.subdirectory' }; + export const ldapBreadcrumbs = ( fullPath: string | undefined, // i.e. path/to/item - roleType: string, - mountPath: string, + routeParams: Function, + routes: { details: string; subdirectory: string }, lastItemCurrent = false // this array of objects can be spread anywhere within the crumbs array ): Breadcrumb[] => { if (!fullPath) return []; @@ -26,11 +28,10 @@ export const ldapBreadcrumbs = ( const segment = ancestry.slice(0, idx + 1).join('/'); const itemPath = isLast && !isDirectory ? segment : `${segment}/`; - const routeParams = [mountPath, roleType, itemPath]; return { label: name, - route: isLast && !isDirectory ? 'roles.role.details' : 'roles.subdirectory', - models: routeParams, + route: isLast && !isDirectory ? routes.details : routes.subdirectory, + models: routeParams(itemPath), }; }); }; diff --git a/ui/tests/unit/utils/ldap-breadcrumbs-test.js b/ui/tests/unit/utils/ldap-breadcrumbs-test.js index 8c1318b40c7c..5ad510594b2a 100644 --- a/ui/tests/unit/utils/ldap-breadcrumbs-test.js +++ b/ui/tests/unit/utils/ldap-breadcrumbs-test.js @@ -3,15 +3,18 @@ * SPDX-License-Identifier: BUSL-1.1 */ -import { ldapBreadcrumbs } from 'ldap/utils/ldap-breadcrumbs'; +import { ldapBreadcrumbs, roleRoutes } from 'ldap/utils/ldap-breadcrumbs'; import { module, test } from 'qunit'; module('Unit | Utility | ldap breadcrumbs', function (hooks) { hooks.beforeEach(async function () { this.mountPath = 'my-engine'; this.roleType = 'static'; + const routeParams = (childResource) => { + return [this.mountPath, this.roleType, childResource]; + }; this.testCrumbs = (path, { lastItemCurrent }) => { - return ldapBreadcrumbs(path, this.roleType, this.mountPath, lastItemCurrent); + return ldapBreadcrumbs(path, routeParams, roleRoutes, lastItemCurrent); }; }); From 45fbb606ddf10e3eab8f26327b2bfcdffd859e33 Mon Sep 17 00:00:00 2001 From: claire bontempo Date: Mon, 6 Jan 2025 09:59:47 -0600 Subject: [PATCH 02/13] add subdirectory library route and hierarchical nav --- ui/app/adapters/ldap/library.js | 25 ++++---- ui/app/models/ldap/library.js | 1 + .../ldap/addon/components/page/libraries.hbs | 44 +++++++++----- .../ldap/addon/components/page/libraries.ts | 10 ++++ ui/lib/ldap/addon/routes.js | 2 + .../addon/routes/libraries/subdirectory.ts | 60 +++++++++++++++++++ .../templates/libraries/subdirectory.hbs | 11 ++++ ui/lib/ldap/addon/utils/ldap-breadcrumbs.ts | 4 ++ ui/types/vault/models/ldap/library.d.ts | 1 + 9 files changed, 131 insertions(+), 27 deletions(-) create mode 100644 ui/lib/ldap/addon/routes/libraries/subdirectory.ts create mode 100644 ui/lib/ldap/addon/templates/libraries/subdirectory.hbs diff --git a/ui/app/adapters/ldap/library.js b/ui/app/adapters/ldap/library.js index 66dca68e4f81..f493564fefc4 100644 --- a/ui/app/adapters/ldap/library.js +++ b/ui/app/adapters/ldap/library.js @@ -7,23 +7,26 @@ import NamedPathAdapter from 'vault/adapters/named-path'; import { encodePath } from 'vault/utils/path-encoding-helpers'; export default class LdapLibraryAdapter extends NamedPathAdapter { - getURL(backend, name) { + // path could be the library name (full path) or just part of the path i.e. west-account/ + _getURL(backend, path) { const base = `${this.buildURL()}/${encodePath(backend)}/library`; - return name ? `${base}/${name}` : base; + return path ? `${base}/${path}` : base; } urlForUpdateRecord(name, modelName, snapshot) { - return this.getURL(snapshot.attr('backend'), name); + return this._getURL(snapshot.attr('backend'), name); } urlForDeleteRecord(name, modelName, snapshot) { - return this.getURL(snapshot.attr('backend'), name); + return this._getURL(snapshot.attr('backend'), name); } query(store, type, query) { - const { backend } = query; - return this.ajax(this.getURL(backend), 'GET', { data: { list: true } }) + const { backend, path_to_library } = query; + // if we have a path_to_library then we're listing subdirectories at a hierarchical library path (i.e west-account/my-library) + const url = this._getURL(backend, path_to_library); + return this.ajax(url, 'GET', { data: { list: true } }) .then((resp) => { - return resp.data.keys.map((name) => ({ name, backend })); + return resp.data.keys.map((name) => ({ name, backend, path_to_library })); }) .catch((error) => { if (error.httpStatus === 404) { @@ -34,11 +37,11 @@ export default class LdapLibraryAdapter extends NamedPathAdapter { } queryRecord(store, type, query) { const { backend, name } = query; - return this.ajax(this.getURL(backend, name), 'GET').then((resp) => ({ ...resp.data, backend, name })); + return this.ajax(this._getURL(backend, name), 'GET').then((resp) => ({ ...resp.data, backend, name })); } fetchStatus(backend, name) { - const url = `${this.getURL(backend, name)}/status`; + const url = `${this._getURL(backend, name)}/status`; return this.ajax(url, 'GET').then((resp) => { const statuses = []; for (const key in resp.data) { @@ -53,7 +56,7 @@ export default class LdapLibraryAdapter extends NamedPathAdapter { }); } checkOutAccount(backend, name, ttl) { - const url = `${this.getURL(backend, name)}/check-out`; + const url = `${this._getURL(backend, name)}/check-out`; return this.ajax(url, 'POST', { data: { ttl } }).then((resp) => { const { lease_id, lease_duration, renewable } = resp; const { service_account_name: account, password } = resp.data; @@ -61,7 +64,7 @@ export default class LdapLibraryAdapter extends NamedPathAdapter { }); } checkInAccount(backend, name, service_account_names) { - const url = `${this.getURL(backend, name)}/check-in`; + const url = `${this._getURL(backend, name)}/check-in`; return this.ajax(url, 'POST', { data: { service_account_names } }).then((resp) => resp.data); } } diff --git a/ui/app/models/ldap/library.js b/ui/app/models/ldap/library.js index e66789a6eb1f..8ca61c6c37d9 100644 --- a/ui/app/models/ldap/library.js +++ b/ui/app/models/ldap/library.js @@ -18,6 +18,7 @@ const formFields = ['name', 'service_account_names', 'ttl', 'max_ttl', 'disable_ @withFormFields(formFields) export default class LdapLibraryModel extends Model { @attr('string') backend; // dynamic path of secret -- set on response from value passed to queryRecord + @attr('string') path_to_library; // ancestral path to the library added in the adapter (only exists for nested libraries) @attr('string', { label: 'Library name', diff --git a/ui/lib/ldap/addon/components/page/libraries.hbs b/ui/lib/ldap/addon/components/page/libraries.hbs index b881dff58a4a..69d8abc761b0 100644 --- a/ui/lib/ldap/addon/components/page/libraries.hbs +++ b/ui/lib/ldap/addon/components/page/libraries.hbs @@ -43,7 +43,7 @@ {{else}}
{{#each this.filteredLibraries as |library|}} - + {{library.name}} @@ -57,22 +57,34 @@ @hasChevron={{false}} data-test-popup-menu-trigger /> - {{#if library.canEdit}} - Edit - {{/if}} - {{#if library.canRead}} - Details - {{/if}} - {{#if library.canDelete}} + {{#if (this.isHierarchical library.name)}} Delete + data-test-subdirectory + @route="libraries.subdirectory" + @model={{concat library.path_to_library library.name}} + >Content + {{else}} + {{#if library.canEdit}} + Edit + {{/if}} + {{#if library.canRead}} + Details + {{/if}} + {{#if library.canDelete}} + Delete + {{/if}} {{/if}} {{/if}} diff --git a/ui/lib/ldap/addon/components/page/libraries.ts b/ui/lib/ldap/addon/components/page/libraries.ts index 06d7bb35b320..5ac9b5545d5f 100644 --- a/ui/lib/ldap/addon/components/page/libraries.ts +++ b/ui/lib/ldap/addon/components/page/libraries.ts @@ -28,6 +28,16 @@ export default class LdapLibrariesPageComponent extends Component { @tracked filterValue = ''; @tracked libraryToDelete: LdapLibraryModel | null = null; + isHierarchical = (name: string) => name.endsWith('/'); + + linkParams = (library: LdapLibraryModel) => { + const route = this.isHierarchical(library.name) ? 'libraries.subdirectory' : 'libraries.library.details'; + // if there is a path_to_library we're in a subdirectory + // and must concat the ancestors with the leaf name to get the full library path + const libraryName = library.path_to_library ? library.path_to_library + library.name : library.name; + return [route, libraryName]; + }; + get mountPoint(): string { const owner = getOwner(this) as EngineOwner; return owner.mountPoint; diff --git a/ui/lib/ldap/addon/routes.js b/ui/lib/ldap/addon/routes.js index 593128f47abe..f11c0ceb4cc3 100644 --- a/ui/lib/ldap/addon/routes.js +++ b/ui/lib/ldap/addon/routes.js @@ -19,6 +19,8 @@ export default buildRoutes(function () { }); this.route('libraries', function () { this.route('create'); + // wildcard route so we can traverse hierarchical libraries i.e. prod/admin/my-library + this.route('subdirectory', { path: '/subdirectory/*path_to_library' }); this.route('library', { path: '/:name' }, function () { this.route('details', function () { this.route('accounts'); diff --git a/ui/lib/ldap/addon/routes/libraries/subdirectory.ts b/ui/lib/ldap/addon/routes/libraries/subdirectory.ts new file mode 100644 index 000000000000..d7a33e005188 --- /dev/null +++ b/ui/lib/ldap/addon/routes/libraries/subdirectory.ts @@ -0,0 +1,60 @@ +/** + * Copyright (c) HashiCorp, Inc. + * SPDX-License-Identifier: BUSL-1.1 + */ + +import Route from '@ember/routing/route'; +import { service } from '@ember/service'; +import { hash } from 'rsvp'; + +import type Store from '@ember-data/store'; +import type SecretMountPath from 'vault/services/secret-mount-path'; +import type Transition from '@ember/routing/transition'; +import type LdapLibraryModel from 'vault/models/ldap/library'; +import type SecretEngineModel from 'vault/models/secret-engine'; +import type Controller from '@ember/controller'; +import type { Breadcrumb } from 'vault/vault/app-types'; +import { ldapBreadcrumbs, libraryRoutes } from 'ldap/utils/ldap-breadcrumbs'; + +interface RouteModel { + backendModel: SecretEngineModel; + path_to_library: string; + libraries: Array; +} +interface RouteController extends Controller { + breadcrumbs: Array; + model: RouteModel; +} +interface RouteParams { + path_to_library?: string; +} + +export default class LdapLibrariesSubdirectoryRoute extends Route { + @service declare readonly store: Store; + @service declare readonly secretMountPath: SecretMountPath; + + model(params: RouteParams) { + const backendModel = this.modelFor('application') as SecretEngineModel; + const { path_to_library } = params; + return hash({ + backendModel, + path_to_library, + libraries: this.store.query('ldap/library', { backend: backendModel.id, path_to_library }), + }); + } + + setupController(controller: RouteController, resolvedModel: RouteModel, transition: Transition) { + super.setupController(controller, resolvedModel, transition); + + const routeParams = (childResource: string) => { + return [resolvedModel.backendModel.id, , childResource]; + }; + + controller.breadcrumbs = [ + { label: 'Secrets', route: 'secrets', linkExternal: true }, + { label: resolvedModel.backendModel.id, route: 'overview' }, + { label: 'Libraries', route: 'libraries' }, + ...ldapBreadcrumbs(resolvedModel.path_to_library, routeParams, libraryRoutes, true), + ]; + } +} diff --git a/ui/lib/ldap/addon/templates/libraries/subdirectory.hbs b/ui/lib/ldap/addon/templates/libraries/subdirectory.hbs new file mode 100644 index 000000000000..0cc729408812 --- /dev/null +++ b/ui/lib/ldap/addon/templates/libraries/subdirectory.hbs @@ -0,0 +1,11 @@ +{{! + Copyright (c) HashiCorp, Inc. + SPDX-License-Identifier: BUSL-1.1 +~}} + + \ No newline at end of file diff --git a/ui/lib/ldap/addon/utils/ldap-breadcrumbs.ts b/ui/lib/ldap/addon/utils/ldap-breadcrumbs.ts index 039bede32ce0..f8ac465a877f 100644 --- a/ui/lib/ldap/addon/utils/ldap-breadcrumbs.ts +++ b/ui/lib/ldap/addon/utils/ldap-breadcrumbs.ts @@ -5,6 +5,10 @@ import type { Breadcrumb } from 'vault/vault/app-types'; export const roleRoutes = { details: 'roles.role.details', subdirectory: 'roles.subdirectory' }; +export const libraryRoutes = { + details: 'libraries.library.details', + subdirectory: 'libraries.subdirectory', +}; export const ldapBreadcrumbs = ( fullPath: string | undefined, // i.e. path/to/item diff --git a/ui/types/vault/models/ldap/library.d.ts b/ui/types/vault/models/ldap/library.d.ts index 1def4cc4f6f1..359cdc67bb95 100644 --- a/ui/types/vault/models/ldap/library.d.ts +++ b/ui/types/vault/models/ldap/library.d.ts @@ -13,6 +13,7 @@ import type { export default interface LdapLibraryModel extends WithFormFieldsAndValidationsModel { backend: string; name: string; + path_to_library: string; service_account_names: string; default_ttl: number; max_ttl: number; From 13429f94fa667f76c646e3eb397fdcbe2ab85e2c Mon Sep 17 00:00:00 2001 From: claire bontempo Date: Mon, 6 Jan 2025 10:00:25 -0600 Subject: [PATCH 03/13] update library breadcrumbs; --- ui/lib/ldap/addon/routes/libraries/library/check-out.ts | 7 +++++-- ui/lib/ldap/addon/routes/libraries/library/details.ts | 7 ++++++- ui/lib/ldap/addon/routes/libraries/library/edit.ts | 6 +++++- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/ui/lib/ldap/addon/routes/libraries/library/check-out.ts b/ui/lib/ldap/addon/routes/libraries/library/check-out.ts index 23025f96892b..c8f71936a25f 100644 --- a/ui/lib/ldap/addon/routes/libraries/library/check-out.ts +++ b/ui/lib/ldap/addon/routes/libraries/library/check-out.ts @@ -16,6 +16,7 @@ import type Transition from '@ember/routing/transition'; import type { Breadcrumb } from 'vault/vault/app-types'; import { LdapLibraryCheckOutCredentials } from 'vault/vault/adapters/ldap/library'; import type AdapterError from 'ember-data/adapter'; // eslint-disable-line ember/use-ember-data-rfc-395-imports +import { ldapBreadcrumbs, libraryRoutes } from 'ldap/utils/ldap-breadcrumbs'; interface LdapLibraryCheckOutController extends Controller { breadcrumbs: Array; @@ -45,12 +46,14 @@ export default class LdapLibraryCheckOutRoute extends Route { transition: Transition ) { super.setupController(controller, resolvedModel, transition); - const library = this.modelFor('libraries.library') as LdapLibraryModel; + const routeParams = (childResource: string) => { + return [library.backend, childResource]; + }; controller.breadcrumbs = [ { label: library.backend, route: 'overview' }, { label: 'Libraries', route: 'libraries' }, - { label: library.name, route: 'libraries.library' }, + ...ldapBreadcrumbs(library.name, routeParams, libraryRoutes), { label: 'Check-Out' }, ]; } diff --git a/ui/lib/ldap/addon/routes/libraries/library/details.ts b/ui/lib/ldap/addon/routes/libraries/library/details.ts index 233835b602c8..e39ab6e1c346 100644 --- a/ui/lib/ldap/addon/routes/libraries/library/details.ts +++ b/ui/lib/ldap/addon/routes/libraries/library/details.ts @@ -9,6 +9,7 @@ import type LdapLibraryModel from 'vault/models/ldap/library'; import type Controller from '@ember/controller'; import type Transition from '@ember/routing/transition'; import type { Breadcrumb } from 'vault/vault/app-types'; +import { ldapBreadcrumbs, libraryRoutes } from 'ldap/utils/ldap-breadcrumbs'; interface LdapLibraryDetailsController extends Controller { breadcrumbs: Array; @@ -23,10 +24,14 @@ export default class LdapLibraryDetailsRoute extends Route { ) { super.setupController(controller, resolvedModel, transition); + const routeParams = (childResource: string) => { + return [resolvedModel.backend, childResource]; + }; + controller.breadcrumbs = [ { label: resolvedModel.backend, route: 'overview' }, { label: 'Libraries', route: 'libraries' }, - { label: resolvedModel.name }, + ...ldapBreadcrumbs(resolvedModel.name, routeParams, libraryRoutes, true), ]; } } diff --git a/ui/lib/ldap/addon/routes/libraries/library/edit.ts b/ui/lib/ldap/addon/routes/libraries/library/edit.ts index 191190cee140..5a3059ab6e41 100644 --- a/ui/lib/ldap/addon/routes/libraries/library/edit.ts +++ b/ui/lib/ldap/addon/routes/libraries/library/edit.ts @@ -9,6 +9,7 @@ import type LdapLibraryModel from 'vault/models/ldap/library'; import type Controller from '@ember/controller'; import type Transition from '@ember/routing/transition'; import type { Breadcrumb } from 'vault/vault/app-types'; +import { ldapBreadcrumbs, libraryRoutes } from 'ldap/utils/ldap-breadcrumbs'; interface LdapLibraryEditController extends Controller { breadcrumbs: Array; @@ -23,10 +24,13 @@ export default class LdapLibraryEditRoute extends Route { ) { super.setupController(controller, resolvedModel, transition); + const routeParams = (childResource: string) => { + return [resolvedModel.backend, childResource]; + }; controller.breadcrumbs = [ { label: resolvedModel.backend, route: 'overview' }, { label: 'Libraries', route: 'libraries' }, - { label: resolvedModel.name, route: 'libraries.library.details' }, + ...ldapBreadcrumbs(resolvedModel.name, routeParams, libraryRoutes), { label: 'Edit' }, ]; } From 613b61dfef4eb59516ca4aee2acc9c4db8f27fb3 Mon Sep 17 00:00:00 2001 From: claire bontempo Date: Mon, 6 Jan 2025 10:40:42 -0600 Subject: [PATCH 04/13] fix role popup menus --- ui/app/adapters/ldap/role.js | 4 ++-- ui/app/models/ldap/role.js | 9 +++++++-- ui/lib/ldap/addon/components/page/roles.hbs | 10 +++++++--- ui/lib/ldap/addon/components/page/roles.ts | 4 ++-- ui/lib/ldap/addon/routes/roles/subdirectory.ts | 1 - ui/types/vault/models/ldap/role.d.ts | 1 + 6 files changed, 19 insertions(+), 10 deletions(-) diff --git a/ui/app/adapters/ldap/role.js b/ui/app/adapters/ldap/role.js index 93bb67db371a..f02d3b79112d 100644 --- a/ui/app/adapters/ldap/role.js +++ b/ui/app/adapters/ldap/role.js @@ -56,8 +56,8 @@ export default class LdapRoleAdapter extends ApplicationAdapter { } urlForDeleteRecord(id, modelName, snapshot) { - const { backend, type, name } = snapshot.record; - return this._getURL(backend, this._pathForRoleType(type), name); + const { backend, type, completeRoleName } = snapshot.record; + return this._getURL(backend, this._pathForRoleType(type), completeRoleName); } /* diff --git a/ui/app/models/ldap/role.js b/ui/app/models/ldap/role.js index 4c2dc68c2f1f..b2e4b8fdf795 100644 --- a/ui/app/models/ldap/role.js +++ b/ui/app/models/ldap/role.js @@ -201,6 +201,9 @@ export default class LdapRoleModel extends Model { @lazyCapabilities(apiPath`${'backend'}/${'credsUri'}/${'name'}`, 'backend', 'credsUri', 'name') credsPath; @lazyCapabilities(apiPath`${'backend'}/rotate-role/${'name'}`, 'backend', 'name') staticRotateCredsPath; + get completeRoleName() { + return this.path_to_role ? `${this.path_to_role}${this.name}` : this.name; + } get canCreate() { return this.rolePath.get('canCreate') !== false; } @@ -224,9 +227,11 @@ export default class LdapRoleModel extends Model { } fetchCredentials() { - return this.store.adapterFor('ldap/role').fetchCredentials(this.backend, this.type, this.name); + return this.store + .adapterFor('ldap/role') + .fetchCredentials(this.backend, this.type, this.completeRoleName); } rotateStaticPassword() { - return this.store.adapterFor('ldap/role').rotateStaticPassword(this.backend, this.name); + return this.store.adapterFor('ldap/role').rotateStaticPassword(this.backend, this.completeRoleName); } } diff --git a/ui/lib/ldap/addon/components/page/roles.hbs b/ui/lib/ldap/addon/components/page/roles.hbs index cf9747a5dff4..946adeb39c42 100644 --- a/ui/lib/ldap/addon/components/page/roles.hbs +++ b/ui/lib/ldap/addon/components/page/roles.hbs @@ -61,7 +61,7 @@ Content {{else}} {{#if role.canEdit}} @@ -72,7 +72,11 @@ >Edit {{/if}} {{#if role.canReadCreds}} - + Get credentials {{/if}} @@ -87,7 +91,7 @@ data-test-details @route="roles.role.details" {{! this will force the roles.role model hook to fire since we may only have a partial model loaded in the list view }} - @models={{array role.type role.name}} + @models={{array role.type role.completeRoleName}} >Details {{#if role.canDelete}} { @action async onRotate(model: LdapRoleModel) { try { - const message = `Successfully rotated credentials for ${model.name}.`; + const message = `Successfully rotated credentials for ${model.completeRoleName}.`; await model.rotateStaticPassword(); this.flashMessages.success(message); } catch (error) { @@ -74,7 +74,7 @@ export default class LdapRolesPageComponent extends Component { @action async onDelete(model: LdapRoleModel) { try { - const message = `Successfully deleted role ${model.name}.`; + const message = `Successfully deleted role ${model.completeRoleName}.`; await model.destroyRecord(); this.pagination.clearDataset('ldap/role'); this.router.transitionTo('vault.cluster.secrets.backend.ldap.roles'); diff --git a/ui/lib/ldap/addon/routes/roles/subdirectory.ts b/ui/lib/ldap/addon/routes/roles/subdirectory.ts index 053d996d2cdf..5b83c72deca4 100644 --- a/ui/lib/ldap/addon/routes/roles/subdirectory.ts +++ b/ui/lib/ldap/addon/routes/roles/subdirectory.ts @@ -64,7 +64,6 @@ export default class LdapRolesSubdirectoryRoute extends LdapRolesRoute { { label: 'Secrets', route: 'secrets', linkExternal: true }, { label: backendModel.id, route: 'overview' }, { label: 'Roles', route: 'roles' }, - ...ldapBreadcrumbs(roleAncestry.path_to_role, routeParams, roleRoutes, true), ]; diff --git a/ui/types/vault/models/ldap/role.d.ts b/ui/types/vault/models/ldap/role.d.ts index 6464e3686496..8f3936396c4a 100644 --- a/ui/types/vault/models/ldap/role.d.ts +++ b/ui/types/vault/models/ldap/role.d.ts @@ -26,6 +26,7 @@ export default interface LdapRoleModel extends WithFormFieldsAndValidationsModel get displayFields(): Array; get roleUri(): string; get credsUri(): string; + get completeRoleName(): string; rolePath: CapabilitiesModel; credsPath: CapabilitiesModel; staticRotateCredsPath: CapabilitiesModel; From 92cb7e3c5eaef99a62c21163f5147a44eebea5f4 Mon Sep 17 00:00:00 2001 From: claire bontempo Date: Mon, 6 Jan 2025 10:58:25 -0600 Subject: [PATCH 05/13] add getter to library model for full path --- ui/app/adapters/ldap/library.js | 4 +++- ui/app/models/ldap/library.js | 6 ++++++ ui/lib/ldap/addon/components/page/libraries.hbs | 6 +++--- ui/lib/ldap/addon/components/page/libraries.ts | 7 ++----- ui/types/vault/models/ldap/library.d.ts | 1 + 5 files changed, 15 insertions(+), 9 deletions(-) diff --git a/ui/app/adapters/ldap/library.js b/ui/app/adapters/ldap/library.js index f493564fefc4..3120b505a74c 100644 --- a/ui/app/adapters/ldap/library.js +++ b/ui/app/adapters/ldap/library.js @@ -14,10 +14,12 @@ export default class LdapLibraryAdapter extends NamedPathAdapter { } urlForUpdateRecord(name, modelName, snapshot) { + // when editing the name IS the full path so we can use "name" instead of "completeLibraryName" here return this._getURL(snapshot.attr('backend'), name); } urlForDeleteRecord(name, modelName, snapshot) { - return this._getURL(snapshot.attr('backend'), name); + const { backend, completeLibraryName } = snapshot.record; + return this._getURL(backend, completeLibraryName); } query(store, type, query) { diff --git a/ui/app/models/ldap/library.js b/ui/app/models/ldap/library.js index 8ca61c6c37d9..89034b465195 100644 --- a/ui/app/models/ldap/library.js +++ b/ui/app/models/ldap/library.js @@ -65,6 +65,12 @@ export default class LdapLibraryModel extends Model { }) disable_check_in_enforcement; + get completeLibraryName() { + // if there is a path_to_library then the name is hierarchical + // and we must concat the ancestors with the leaf name to get the full library path + return this.path_to_library ? `${this.path_to_library}${this.name}` : this.name; + } + get displayFields() { return this.formFields.filter((field) => field.name !== 'service_account_names'); } diff --git a/ui/lib/ldap/addon/components/page/libraries.hbs b/ui/lib/ldap/addon/components/page/libraries.hbs index 69d8abc761b0..63308095cecf 100644 --- a/ui/lib/ldap/addon/components/page/libraries.hbs +++ b/ui/lib/ldap/addon/components/page/libraries.hbs @@ -61,21 +61,21 @@ Content {{else}} {{#if library.canEdit}} Edit {{/if}} {{#if library.canRead}} Details {{/if}} {{#if library.canDelete}} diff --git a/ui/lib/ldap/addon/components/page/libraries.ts b/ui/lib/ldap/addon/components/page/libraries.ts index 5ac9b5545d5f..f004b419fdb7 100644 --- a/ui/lib/ldap/addon/components/page/libraries.ts +++ b/ui/lib/ldap/addon/components/page/libraries.ts @@ -32,10 +32,7 @@ export default class LdapLibrariesPageComponent extends Component { linkParams = (library: LdapLibraryModel) => { const route = this.isHierarchical(library.name) ? 'libraries.subdirectory' : 'libraries.library.details'; - // if there is a path_to_library we're in a subdirectory - // and must concat the ancestors with the leaf name to get the full library path - const libraryName = library.path_to_library ? library.path_to_library + library.name : library.name; - return [route, libraryName]; + return [route, library.completeLibraryName]; }; get mountPoint(): string { @@ -53,7 +50,7 @@ export default class LdapLibrariesPageComponent extends Component { @action async onDelete(model: LdapLibraryModel) { try { - const message = `Successfully deleted library ${model.name}.`; + const message = `Successfully deleted library ${model.completeLibraryName}.`; await model.destroyRecord(); this.flashMessages.success(message); } catch (error) { diff --git a/ui/types/vault/models/ldap/library.d.ts b/ui/types/vault/models/ldap/library.d.ts index 359cdc67bb95..969538ba951e 100644 --- a/ui/types/vault/models/ldap/library.d.ts +++ b/ui/types/vault/models/ldap/library.d.ts @@ -18,6 +18,7 @@ export default interface LdapLibraryModel extends WithFormFieldsAndValidationsMo default_ttl: number; max_ttl: number; disable_check_in_enforcement: string; + get completeLibraryName(): string; get displayFields(): Array; libraryPath: CapabilitiesModel; statusPath: CapabilitiesModel; From 191b391b35973e5717de11c48c297cdf7ba19056 Mon Sep 17 00:00:00 2001 From: claire bontempo Date: Mon, 6 Jan 2025 11:01:05 -0600 Subject: [PATCH 06/13] cleanup model getters --- ui/app/models/ldap/library.js | 2 +- ui/app/models/ldap/role.js | 9 ++++++--- ui/lib/ldap/addon/components/page/roles.ts | 5 +---- ui/types/vault/models/ldap/role.d.ts | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/ui/app/models/ldap/library.js b/ui/app/models/ldap/library.js index 89034b465195..972d91569bfe 100644 --- a/ui/app/models/ldap/library.js +++ b/ui/app/models/ldap/library.js @@ -68,7 +68,7 @@ export default class LdapLibraryModel extends Model { get completeLibraryName() { // if there is a path_to_library then the name is hierarchical // and we must concat the ancestors with the leaf name to get the full library path - return this.path_to_library ? `${this.path_to_library}${this.name}` : this.name; + return this.path_to_library ? this.path_to_library + this.name : this.name; } get displayFields() { diff --git a/ui/app/models/ldap/role.js b/ui/app/models/ldap/role.js index b2e4b8fdf795..74d0aed000ab 100644 --- a/ui/app/models/ldap/role.js +++ b/ui/app/models/ldap/role.js @@ -163,6 +163,12 @@ export default class LdapRoleModel extends Model { }) rollback_ldif; + get completeRoleName() { + // if there is a path_to_role then the name is hierarchical + // and we must concat the ancestors with the leaf name to get the full role path + return this.path_to_role ? this.path_to_role + this.name : this.name; + } + get isStatic() { return this.type === 'static'; } @@ -201,9 +207,6 @@ export default class LdapRoleModel extends Model { @lazyCapabilities(apiPath`${'backend'}/${'credsUri'}/${'name'}`, 'backend', 'credsUri', 'name') credsPath; @lazyCapabilities(apiPath`${'backend'}/rotate-role/${'name'}`, 'backend', 'name') staticRotateCredsPath; - get completeRoleName() { - return this.path_to_role ? `${this.path_to_role}${this.name}` : this.name; - } get canCreate() { return this.rolePath.get('canCreate') !== false; } diff --git a/ui/lib/ldap/addon/components/page/roles.ts b/ui/lib/ldap/addon/components/page/roles.ts index d5bd2c054a15..609ac0a35455 100644 --- a/ui/lib/ldap/addon/components/page/roles.ts +++ b/ui/lib/ldap/addon/components/page/roles.ts @@ -37,10 +37,7 @@ export default class LdapRolesPageComponent extends Component { linkParams = (role: LdapRoleModel) => { const route = this.isHierarchical(role.name) ? 'roles.subdirectory' : 'roles.role.details'; - // if there is a path_to_role we're in a subdirectory - // and must concat the ancestors with the leaf name to get the full role path - const roleName = role.path_to_role ? role.path_to_role + role.name : role.name; - return [route, role.type, roleName]; + return [route, role.type, role.completeRoleName]; }; get mountPoint(): string { diff --git a/ui/types/vault/models/ldap/role.d.ts b/ui/types/vault/models/ldap/role.d.ts index 8f3936396c4a..cb7dce38e879 100644 --- a/ui/types/vault/models/ldap/role.d.ts +++ b/ui/types/vault/models/ldap/role.d.ts @@ -20,13 +20,13 @@ export default interface LdapRoleModel extends WithFormFieldsAndValidationsModel username_template: string; creation_ldif: string; rollback_ldif: string; + get completeRoleName(): string; get isStatic(): string; get isDynamic(): string; get fieldsForType(): Array; get displayFields(): Array; get roleUri(): string; get credsUri(): string; - get completeRoleName(): string; rolePath: CapabilitiesModel; credsPath: CapabilitiesModel; staticRotateCredsPath: CapabilitiesModel; From 03e1b4b3a54a5c3d7a2d894bff8042df7ce1d651 Mon Sep 17 00:00:00 2001 From: claire bontempo Date: Mon, 6 Jan 2025 11:06:02 -0600 Subject: [PATCH 07/13] add changelog --- changelog/29293.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/29293.txt diff --git a/changelog/29293.txt b/changelog/29293.txt new file mode 100644 index 000000000000..e05d90d2dea2 --- /dev/null +++ b/changelog/29293.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: Adds navigation for LDAP hierarchical libraries +``` \ No newline at end of file From 244b9748e33ebb1b6073a44ba22c822bde94dfbd Mon Sep 17 00:00:00 2001 From: claire bontempo Date: Mon, 6 Jan 2025 11:18:15 -0600 Subject: [PATCH 08/13] add bug fix note --- changelog/29293.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/changelog/29293.txt b/changelog/29293.txt index e05d90d2dea2..056b7cf96892 100644 --- a/changelog/29293.txt +++ b/changelog/29293.txt @@ -1,3 +1,6 @@ ```release-note:improvement ui: Adds navigation for LDAP hierarchical libraries +``` +```release-note:bug +ui: Fixes navigation for quick actions in LDAP roles' popup menu ``` \ No newline at end of file From 530dad679ad563e1368164c52b354fac30f32370 Mon Sep 17 00:00:00 2001 From: claire bontempo Date: Mon, 6 Jan 2025 11:27:37 -0600 Subject: [PATCH 09/13] add transition after deleting --- ui/lib/ldap/addon/components/page/libraries.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/lib/ldap/addon/components/page/libraries.ts b/ui/lib/ldap/addon/components/page/libraries.ts index f004b419fdb7..28a4fc688d24 100644 --- a/ui/lib/ldap/addon/components/page/libraries.ts +++ b/ui/lib/ldap/addon/components/page/libraries.ts @@ -14,6 +14,7 @@ import type LdapLibraryModel from 'vault/models/ldap/library'; import type SecretEngineModel from 'vault/models/secret-engine'; import type FlashMessageService from 'vault/services/flash-messages'; import type { Breadcrumb, EngineOwner } from 'vault/vault/app-types'; +import type RouterService from '@ember/routing/router-service'; interface Args { libraries: Array; @@ -24,6 +25,7 @@ interface Args { export default class LdapLibrariesPageComponent extends Component { @service declare readonly flashMessages: FlashMessageService; + @service('app-router') declare readonly router: RouterService; @tracked filterValue = ''; @tracked libraryToDelete: LdapLibraryModel | null = null; @@ -52,6 +54,7 @@ export default class LdapLibrariesPageComponent extends Component { try { const message = `Successfully deleted library ${model.completeLibraryName}.`; await model.destroyRecord(); + this.router.transitionTo('vault.cluster.secrets.backend.ldap.libraries'); this.flashMessages.success(message); } catch (error) { this.flashMessages.danger(`Error deleting library \n ${errorMessage(error)}`); From ceb6c3c617aae8f84b6ddd740c2dce5332442943 Mon Sep 17 00:00:00 2001 From: claire bontempo Date: Mon, 6 Jan 2025 14:03:08 -0600 Subject: [PATCH 10/13] fix function definition --- ui/lib/ldap/addon/utils/ldap-breadcrumbs.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/lib/ldap/addon/utils/ldap-breadcrumbs.ts b/ui/lib/ldap/addon/utils/ldap-breadcrumbs.ts index f8ac465a877f..cdeda3a12a7f 100644 --- a/ui/lib/ldap/addon/utils/ldap-breadcrumbs.ts +++ b/ui/lib/ldap/addon/utils/ldap-breadcrumbs.ts @@ -12,7 +12,7 @@ export const libraryRoutes = { export const ldapBreadcrumbs = ( fullPath: string | undefined, // i.e. path/to/item - routeParams: Function, + routeParams: (childResource: string) => string[], // array of route param strings routes: { details: string; subdirectory: string }, lastItemCurrent = false // this array of objects can be spread anywhere within the crumbs array ): Breadcrumb[] => { From 325f8b63d8f4fc569c62bbfec16a8525b2e83e17 Mon Sep 17 00:00:00 2001 From: claire bontempo Date: Mon, 6 Jan 2025 14:39:41 -0600 Subject: [PATCH 11/13] update adapter test --- ui/tests/unit/adapters/ldap/role-test.js | 404 ++++++++++++++--------- 1 file changed, 243 insertions(+), 161 deletions(-) diff --git a/ui/tests/unit/adapters/ldap/role-test.js b/ui/tests/unit/adapters/ldap/role-test.js index 78bda11cbabb..aa61b2f3ea2e 100644 --- a/ui/tests/unit/adapters/ldap/role-test.js +++ b/ui/tests/unit/adapters/ldap/role-test.js @@ -19,8 +19,8 @@ module('Unit | Adapter | ldap/role', function (hooks) { this.adapter = this.store.adapterFor('ldap/role'); this.path = 'role'; - this.getModel = (type) => { - const name = 'test-role'; + this.getModel = (type, roleName) => { + const name = roleName || 'test-role'; this.store.pushPayload('ldap/role', { modelName: 'ldap/role', backend: 'ldap-test', @@ -32,214 +32,296 @@ module('Unit | Adapter | ldap/role', function (hooks) { }; }); - test('it should make request to correct endpoints when listing records', async function (assert) { - assert.expect(6); + module('happy paths', function () { + test('it should make request to correct endpoints when listing records', async function (assert) { + assert.expect(6); - const assertRequest = (schema, req) => { - assert.ok(req.queryParams.list, 'list query param sent when listing roles'); - const name = req.url.includes('static-role') ? 'static-test' : 'dynamic-test'; - return { data: { keys: [name] } }; - }; - - this.server.get('/ldap-test/static-role', assertRequest); - this.server.get('/ldap-test/role', assertRequest); + const assertRequest = (schema, req) => { + assert.ok(req.queryParams.list, 'list query param sent when listing roles'); + const name = req.url.includes('static-role') ? 'static-test' : 'dynamic-test'; + return { data: { keys: [name] } }; + }; - this.models = await this.store.query('ldap/role', { backend: 'ldap-test' }); + this.server.get('/ldap-test/static-role', assertRequest); + this.server.get('/ldap-test/role', assertRequest); - const model = this.models[0]; - assert.strictEqual(this.models.length, 2, 'Returns responses from both endpoints'); - assert.strictEqual(model.backend, 'ldap-test', 'Backend value is set on records returned from query'); - // sorted alphabetically by name so dynamic should be first - assert.strictEqual(model.type, 'dynamic', 'Type value is set on records returned from query'); - assert.strictEqual(model.name, 'dynamic-test', 'Name value is set on records returned from query'); - }); + this.models = await this.store.query('ldap/role', { backend: 'ldap-test' }); - test('it should conditionally trigger info level flash message for single endpoint error from query', async function (assert) { - const flashMessages = this.owner.lookup('service:flashMessages'); - const flashSpy = sinon.spy(flashMessages, 'info'); - - this.server.get('/ldap-test/static-role', () => { - return new Response(403, {}, { errors: ['permission denied'] }); + const model = this.models[0]; + assert.strictEqual(this.models.length, 2, 'Returns responses from both endpoints'); + assert.strictEqual(model.backend, 'ldap-test', 'Backend value is set on records returned from query'); + // sorted alphabetically by name so dynamic should be first + assert.strictEqual(model.type, 'dynamic', 'Type value is set on records returned from query'); + assert.strictEqual(model.name, 'dynamic-test', 'Name value is set on records returned from query'); }); - this.server.get('/ldap-test/role', () => ({ data: { keys: ['dynamic-test'] } })); - - await this.store.query('ldap/role', { backend: 'ldap-test' }); - await this.store.query( - 'ldap/role', - { backend: 'ldap-test' }, - { adapterOptions: { showPartialError: true } } - ); - - assert.true( - flashSpy.calledOnceWith('Error fetching roles from /v1/ldap-test/static-role: permission denied'), - 'Partial error info only displays when adapter option is passed' - ); - }); - test('it should throw error for query when requests to both endpoints fail', async function (assert) { - assert.expect(2); + test('it should conditionally trigger info level flash message for single endpoint error from query', async function (assert) { + const flashMessages = this.owner.lookup('service:flashMessages'); + const flashSpy = sinon.spy(flashMessages, 'info'); - this.server.get('/ldap-test/:path', (schema, req) => { - const errors = { - 'static-role': ['permission denied'], - role: ['server error'], - }[req.params.path]; - return new Response(req.params.path === 'static-role' ? 403 : 500, {}, { errors }); - }); + this.server.get('/ldap-test/static-role', () => { + return new Response(403, {}, { errors: ['permission denied'] }); + }); + this.server.get('/ldap-test/role', () => ({ data: { keys: ['dynamic-test'] } })); - try { await this.store.query('ldap/role', { backend: 'ldap-test' }); - } catch (error) { - assert.deepEqual( - error.errors, - ['/v1/ldap-test/static-role: permission denied', '/v1/ldap-test/role: server error'], - 'Error messages is thrown with correct payload from query.' + await this.store.query( + 'ldap/role', + { backend: 'ldap-test' }, + { adapterOptions: { showPartialError: true } } ); - assert.strictEqual( - error.message, - 'Error fetching roles:', - 'Error message is thrown with correct payload from query.' - ); - } - }); - - test('it should make request to correct endpoints when querying record', async function (assert) { - assert.expect(5); - this.server.get('/ldap-test/:path/test-role', (schema, req) => { - assert.strictEqual( - req.params.path, - this.path, - 'GET request made to correct endpoint when querying record' + assert.true( + flashSpy.calledOnceWith('Error fetching roles from /v1/ldap-test/static-role: permission denied'), + 'Partial error info only displays when adapter option is passed' ); }); - for (const type of ['dynamic', 'static']) { - this.model = await this.store.queryRecord('ldap/role', { - backend: 'ldap-test', - type, - name: 'test-role', + test('it should throw error for query when requests to both endpoints fail', async function (assert) { + assert.expect(2); + + this.server.get('/ldap-test/:path', (schema, req) => { + const errors = { + 'static-role': ['permission denied'], + role: ['server error'], + }[req.params.path]; + return new Response(req.params.path === 'static-role' ? 403 : 500, {}, { errors }); }); - this.path = 'static-role'; - } - assert.strictEqual( - this.model.backend, - 'ldap-test', - 'Backend value is set on records returned from query' - ); - assert.strictEqual(this.model.type, 'static', 'Type value is set on records returned from query'); - assert.strictEqual(this.model.name, 'test-role', 'Name value is set on records returned from query'); - }); + try { + await this.store.query('ldap/role', { backend: 'ldap-test' }); + } catch (error) { + assert.deepEqual( + error.errors, + ['/v1/ldap-test/static-role: permission denied', '/v1/ldap-test/role: server error'], + 'Error messages is thrown with correct payload from query.' + ); + assert.strictEqual( + error.message, + 'Error fetching roles:', + 'Error message is thrown with correct payload from query.' + ); + } + }); - test('it should make request to correct endpoints when creating new dynamic role record', async function (assert) { - assert.expect(1); + test('it should make request to correct endpoints when querying record', async function (assert) { + assert.expect(5); + + this.server.get('/ldap-test/:path/test-role', (schema, req) => { + assert.strictEqual( + req.params.path, + this.path, + 'GET request made to correct endpoint when querying record' + ); + }); + + for (const type of ['dynamic', 'static']) { + this.model = await this.store.queryRecord('ldap/role', { + backend: 'ldap-test', + type, + name: 'test-role', + }); + this.path = 'static-role'; + } - this.server.post('/ldap-test/:path/:name', (schema, req) => { assert.strictEqual( - req.params.path, - this.path, - 'POST request made to correct endpoint when creating new record for a dynamic role' + this.model.backend, + 'ldap-test', + 'Backend value is set on records returned from query' ); + assert.strictEqual(this.model.type, 'static', 'Type value is set on records returned from query'); + assert.strictEqual(this.model.name, 'test-role', 'Name value is set on records returned from query'); }); - const getModel = (type, name) => { - return this.store.createRecord('ldap/role', { - backend: 'ldap-test', - name, - type, + test('it should make request to correct endpoints when creating new dynamic role record', async function (assert) { + assert.expect(1); + + this.server.post('/ldap-test/:path/:name', (schema, req) => { + assert.strictEqual( + req.params.path, + this.path, + 'POST request made to correct endpoint when creating new record for a dynamic role' + ); }); - }; - const model = getModel('dynamic-role', 'dynamic-role-name'); - await model.save(); - }); + const getModel = (type, name) => { + return this.store.createRecord('ldap/role', { + backend: 'ldap-test', + name, + type, + }); + }; - test('it should make request to correct endpoints when creating new static role record', async function (assert) { - assert.expect(1); + const model = getModel('dynamic-role', 'dynamic-role-name'); + await model.save(); + }); - this.server.post('/ldap-test/:path/:name', (schema, req) => { - assert.strictEqual( - req.params.path, - this.path, - 'POST request made to correct endpoint when creating new record for a static role' - ); + test('it should make request to correct endpoints when creating new static role record', async function (assert) { + assert.expect(1); + + this.server.post('/ldap-test/:path/:name', (schema, req) => { + assert.strictEqual( + req.params.path, + this.path, + 'POST request made to correct endpoint when creating new record for a static role' + ); + }); + + const getModel = (type, name) => { + return this.store.createRecord('ldap/role', { + backend: 'ldap-test', + name, + type, + }); + }; + + const model = getModel('static-role', 'static-role-name'); + await model.save(); }); - const getModel = (type, name) => { - return this.store.createRecord('ldap/role', { - backend: 'ldap-test', - name, - type, + test('it should make request to correct endpoints when updating record', async function (assert) { + assert.expect(2); + + this.server.post('/ldap-test/:path/test-role', (schema, req) => { + assert.strictEqual( + req.params.path, + this.path, + 'POST request made to correct endpoint when updating record' + ); }); - }; - const model = getModel('static-role', 'static-role-name'); - await model.save(); - }); + for (const type of ['dynamic', 'static']) { + const record = this.getModel(type); + await record.save(); + this.path = 'static-role'; + } + }); - test('it should make request to correct endpoints when updating record', async function (assert) { - assert.expect(2); + test('it should make request to correct endpoints when deleting record', async function (assert) { + assert.expect(2); - this.server.post('/ldap-test/:path/test-role', (schema, req) => { - assert.strictEqual( - req.params.path, - this.path, - 'POST request made to correct endpoint when updating record' - ); + this.server.delete('/ldap-test/:path/test-role', (schema, req) => { + assert.strictEqual( + req.params.path, + this.path, + 'DELETE request made to correct endpoint when deleting record' + ); + }); + + for (const type of ['dynamic', 'static']) { + const record = this.getModel(type); + await record.destroyRecord(); + this.path = 'static-role'; + } }); - for (const type of ['dynamic', 'static']) { - const record = this.getModel(type); - await record.save(); - this.path = 'static-role'; - } - }); + test('it should make request to correct endpoints when fetching credentials', async function (assert) { + assert.expect(2); - test('it should make request to correct endpoints when deleting record', async function (assert) { - assert.expect(2); + this.path = 'creds'; - this.server.delete('/ldap-test/:path/test-role', (schema, req) => { - assert.strictEqual( - req.params.path, - this.path, - 'DELETE request made to correct endpoint when deleting record' - ); + this.server.get('/ldap-test/:path/test-role', (schema, req) => { + assert.strictEqual( + req.params.path, + this.path, + 'GET request made to correct endpoint when fetching credentials' + ); + }); + + for (const type of ['dynamic', 'static']) { + await this.adapter.fetchCredentials('ldap-test', type, 'test-role'); + this.path = 'static-cred'; + } }); - for (const type of ['dynamic', 'static']) { - const record = this.getModel(type); - await record.destroyRecord(); - this.path = 'static-role'; - } + test('it should make request to correct endpoint when rotating static role password', async function (assert) { + assert.expect(1); + + this.server.post('/ldap-test/rotate-role/test-role', () => { + assert.ok('GET request made to correct endpoint when rotating static role password'); + }); + + await this.adapter.rotateStaticPassword('ldap-test', 'test-role'); + }); }); - test('it should make request to correct endpoints when fetching credentials', async function (assert) { - assert.expect(2); + module('hierarchical paths', function () { + test('it should make request to correct endpoint when listing hierarchical records', async function (assert) { + assert.expect(2); - this.path = 'creds'; + const staticAncestry = { path_to_role: 'static-admin/', type: 'static' }; + const dynamicAncestry = { path_to_role: 'dynamic-admin/', type: 'dynamic' }; - this.server.get('/ldap-test/:path/test-role', (schema, req) => { - assert.strictEqual( - req.params.path, - this.path, - 'GET request made to correct endpoint when fetching credentials' + this.server.get(`/ldap-test/static-role/${staticAncestry.path_to_role}`, (schema, req) => { + assert.strictEqual( + req.queryParams.list, + 'true', + `query request lists roles of type: ${staticAncestry.type}` + ); + return { data: { keys: ['my-static-role'] } }; + }); + this.server.get(`/ldap-test/role/${dynamicAncestry.path_to_role}`, (schema, req) => { + assert.strictEqual( + req.queryParams.list, + 'true', + `query request lists roles of type: ${dynamicAncestry.type}` + ); + return { data: { keys: ['my-dynamic-role'] } }; + }); + + await this.store.query( + 'ldap/role', + { backend: 'ldap-test' }, + { adapterOptions: { roleAncestry: staticAncestry } } + ); + await this.store.query( + 'ldap/role', + { backend: 'ldap-test' }, + { adapterOptions: { roleAncestry: dynamicAncestry } } ); }); for (const type of ['dynamic', 'static']) { - await this.adapter.fetchCredentials('ldap-test', type, 'test-role'); - this.path = 'static-cred'; + test(`it should make request to correct endpoint when deleting a role for type: ${type}`, async function (assert) { + assert.expect(1); + + const url = + type === 'static' + ? '/ldap-test/static-role/admin/my-static-role' + : '/ldap-test/role/admin/my-dynamic-role'; + + this.server.delete(url, () => { + assert.true(true, `DELETE request made to delete hierarchical role of type: ${type}`); + }); + + const record = this.getModel(type, `admin/my-${type}-role`); + await record.destroyRecord(); + }); + + test(`it should make request to correct endpoints when fetching credentials for type: ${type}`, async function (assert) { + assert.expect(1); + + const url = + type === 'static' + ? '/ldap-test/static-cred/admin/my-static-role' + : '/ldap-test/creds/admin/my-dynamic-role'; + + this.server.get(url, () => { + assert.true(true, `request made to fetch credentials for role type: ${type}`); + }); + + await this.adapter.fetchCredentials('ldap-test', type, `admin/my-${type}-role`); + }); } - }); - test('it should make request to correct endpoint when rotating static role password', async function (assert) { - assert.expect(1); + test('it should make request to correct endpoint when rotating static role password', async function (assert) { + assert.expect(1); - this.server.post('/ldap-test/rotate-role/test-role', () => { - assert.ok('GET request made to correct endpoint when rotating static role password'); - }); + this.server.post('/ldap-test/rotate-role/admin/test-role', () => { + assert.ok('GET request made to correct endpoint when rotating static role password'); + }); - await this.adapter.rotateStaticPassword('ldap-test', 'test-role'); + await this.adapter.rotateStaticPassword('ldap-test', 'admin/test-role'); + }); }); }); From 5a5518428c608d3bc24ad7912483c4490c2a5210 Mon Sep 17 00:00:00 2001 From: claire bontempo Date: Mon, 6 Jan 2025 15:25:42 -0600 Subject: [PATCH 12/13] add test coverage --- .../ldap/addon/components/page/libraries.hbs | 4 +-- ui/mirage/handlers/ldap.js | 18 ++++++----- ui/mirage/scenarios/ldap.js | 2 ++ .../secrets/backend/ldap/libraries-test.js | 30 +++++++++++++++---- ui/tests/helpers/ldap/ldap-selectors.ts | 2 ++ .../components/ldap/page/libraries-test.js | 18 +++++++---- 6 files changed, 54 insertions(+), 20 deletions(-) diff --git a/ui/lib/ldap/addon/components/page/libraries.hbs b/ui/lib/ldap/addon/components/page/libraries.hbs index 63308095cecf..55f31e266038 100644 --- a/ui/lib/ldap/addon/components/page/libraries.hbs +++ b/ui/lib/ldap/addon/components/page/libraries.hbs @@ -46,7 +46,7 @@ - {{library.name}} + {{library.name}} {{#if (or library.canRead library.canEdit library.canDelete)}} @@ -55,7 +55,7 @@ @icon="more-horizontal" @text="More options" @hasChevron={{false}} - data-test-popup-menu-trigger + data-test-popup-menu-trigger={{library.completeLibraryName}} /> {{#if (this.isHierarchical library.name)}} { - // if the param name is admin, we want to LIST admin/ roles + const dbKey = type ? 'ldapRoles' : 'ldapLibraries'; + const query = type ? { type, name: `admin/child-${type}-role` } : { name: 'admin/test-library' }; if (req.queryParams.list) { - // passing a query with specific name is not flexible - // but we only seeded the mirage db with one hierarchical role for each type - return listRecords(schema, 'ldapRoles', { type, name: `admin/child-${type}-role` }); + // the mirage database has setup all hierarchical names to be prefixed with "admin/" + // while passing a query with specific name is not flexible, for simplicity + // we only seeded the mirage db with one hierarchical resource for each role and a library + return listRecords(schema, dbKey, query); } - // otherwise we want to view details for a specific role - return getRecord(schema, req, 'ldapRoles', type); + // otherwise we want to view details for a specific resource + return getRecord(schema, req, dbKey); }; // config @@ -77,9 +79,9 @@ export default function (server) { })); // libraries 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/*name', (schema, req) => listOrGetRecord(schema, req)); server.get('/:backend/library', (schema) => listRecords(schema, 'ldapLibraries')); - server.get('/:backend/library/:name/status', (schema) => { + server.get('/:backend/library/*name/status', (schema) => { const data = schema.db['ldapAccountStatuses'].reduce((prev, curr) => { prev[curr.account] = { available: curr.available, diff --git a/ui/mirage/scenarios/ldap.js b/ui/mirage/scenarios/ldap.js index cff48bd84df1..66df01ff37ef 100644 --- a/ui/mirage/scenarios/ldap.js +++ b/ui/mirage/scenarios/ldap.js @@ -14,6 +14,8 @@ export default function (server) { server.create('ldap-role', 'static', { name: 'my-role' }); server.create('ldap-role', 'dynamic', { name: 'my-role' }); server.create('ldap-library', { name: 'test-library' }); + // mirage handler is hardcoded to accommodate hierarchical paths starting with 'admin/' + server.create('ldap-library', { name: 'admin/test-library' }); server.create('ldap-account-status', { id: 'bob.johnson', account: 'bob.johnson', diff --git a/ui/tests/acceptance/secrets/backend/ldap/libraries-test.js b/ui/tests/acceptance/secrets/backend/ldap/libraries-test.js index fee09ad7f859..d04b3718846a 100644 --- a/ui/tests/acceptance/secrets/backend/ldap/libraries-test.js +++ b/ui/tests/acceptance/secrets/backend/ldap/libraries-test.js @@ -10,9 +10,10 @@ 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 { click, currentURL } from '@ember/test-helpers'; import { isURL, visitURL } from 'vault/tests/helpers/ldap/ldap-helpers'; import { deleteEngineCmd, mountEngineCmd, runCmd } from 'vault/tests/helpers/commands'; +import { LDAP_SELECTORS } from 'vault/tests/helpers/ldap/ldap-selectors'; module('Acceptance | ldap | libraries', function (hooks) { setupApplicationTest(hooks); @@ -37,7 +38,7 @@ module('Acceptance | ldap | libraries', function (hooks) { test('it should show libraries on overview page', async function (assert) { await visitURL('overview', this.backend); - assert.dom('[data-test-libraries-count]').hasText('1'); + assert.dom('[data-test-libraries-count]').hasText('2'); }); test('it should transition to create library route on toolbar link click', async function (assert) { @@ -49,15 +50,34 @@ module('Acceptance | ldap | libraries', function (hooks) { }); 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', this.backend), + await click(LDAP_SELECTORS.libraryItem('test-library')); + assert.strictEqual( + currentURL(), + `/vault/secrets/${this.backend}/ldap/libraries/test-library/details/accounts`, '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 library details for hierarchical list items', async function (assert) { + await click(LDAP_SELECTORS.libraryItem('admin/')); + assert.strictEqual( + currentURL(), + `/vault/secrets/${this.backend}/ldap/libraries/subdirectory/admin/`, + 'Transitions to subdirectory list view' + ); + + await click(LDAP_SELECTORS.libraryItem('admin/test-library')); + assert.strictEqual( + currentURL(), + `/vault/secrets/${this.backend}/ldap/libraries/admin%2Ftest-library/details/accounts`, + 'Transitions to child library details accounts' + ); + 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) { assert.expect(2); diff --git a/ui/tests/helpers/ldap/ldap-selectors.ts b/ui/tests/helpers/ldap/ldap-selectors.ts index 73933bcaae8c..b99b77d0dd77 100644 --- a/ui/tests/helpers/ldap/ldap-selectors.ts +++ b/ui/tests/helpers/ldap/ldap-selectors.ts @@ -5,6 +5,8 @@ export const LDAP_SELECTORS = { roleItem: (type: string, name: string) => `[data-test-role="${type} ${name}"]`, + libraryItem: (name: string) => `[data-test-library="${name}"]`, roleMenu: (type: string, name: string) => `[data-test-popup-menu-trigger="${type} ${name}"]`, + libraryMenu: (name: string) => `[data-test-popup-menu-trigger="${name}"]`, action: (action: string) => `[data-test-${action}]`, }; diff --git a/ui/tests/integration/components/ldap/page/libraries-test.js b/ui/tests/integration/components/ldap/page/libraries-test.js index b9b3be13cd3c..09cd90945db6 100644 --- a/ui/tests/integration/components/ldap/page/libraries-test.js +++ b/ui/tests/integration/components/ldap/page/libraries-test.js @@ -12,6 +12,7 @@ import hbs from 'htmlbars-inline-precompile'; import { allowAllCapabilitiesStub } from 'vault/tests/helpers/stubs'; import { createSecretsEngine, generateBreadcrumbs } from 'vault/tests/helpers/ldap/ldap-helpers'; import { setRunOptions } from 'ember-a11y-testing/test-support'; +import { LDAP_SELECTORS } from 'vault/tests/helpers/ldap/ldap-selectors'; module('Integration | Component | ldap | Page::Libraries', function (hooks) { setupRenderingTest(hooks); @@ -25,7 +26,7 @@ module('Integration | Component | ldap | Page::Libraries', function (hooks) { this.backend = createSecretsEngine(this.store); this.breadcrumbs = generateBreadcrumbs(this.backend.id); - for (const name of ['foo', 'bar']) { + for (const name of ['foo', 'bar', 'foo/']) { this.store.pushPayload('ldap/library', { modelName: 'ldap/library', backend: 'ldap-test', @@ -93,12 +94,19 @@ module('Integration | Component | ldap | Page::Libraries', function (hooks) { await this.renderComponent(); assert.dom('[data-test-list-item-content] svg').hasClass('hds-icon-folder', 'List item icon renders'); - assert.dom('[data-test-library]').hasText(this.libraries[0].name, 'List item name renders'); + assert.dom('[data-test-library="foo"]').hasText('foo', 'List item name renders'); - await click('[data-test-popup-menu-trigger]'); + await click(LDAP_SELECTORS.libraryMenu('foo')); + assert.dom('[data-test-subdirectory]').doesNotExist(); assert.dom('[data-test-edit]').hasText('Edit', 'Edit link renders in menu'); assert.dom('[data-test-details]').hasText('Details', 'Details link renders in menu'); assert.dom('[data-test-delete]').hasText('Delete', 'Details link renders in menu'); + + await click(LDAP_SELECTORS.libraryMenu('foo/')); + assert.dom('[data-test-subdirectory]').hasText('Content', 'Content link renders in menu'); + assert.dom('[data-test-edit]').doesNotExist(); + assert.dom('[data-test-details]').doesNotExist(); + assert.dom('[data-test-delete]').doesNotExist(); }); test('it should filter libraries', async function (assert) { @@ -110,11 +118,11 @@ module('Integration | Component | ldap | Page::Libraries', function (hooks) { .hasText('There are no libraries matching "baz"', 'Filter message renders'); await fillIn('[data-test-filter-input]', 'foo'); - assert.dom('[data-test-list-item-content]').exists({ count: 1 }, 'List is filtered with correct results'); + assert.dom('[data-test-list-item-content]').exists({ count: 2 }, 'List is filtered with correct results'); await fillIn('[data-test-filter-input]', ''); assert .dom('[data-test-list-item-content]') - .exists({ count: 2 }, 'All libraries are displayed when filter is cleared'); + .exists({ count: 3 }, 'All libraries are displayed when filter is cleared'); }); }); From cc1a9b85155ce902a56ceb46d3a5f1f0dac1e418 Mon Sep 17 00:00:00 2001 From: claire bontempo Date: Mon, 6 Jan 2025 15:28:16 -0600 Subject: [PATCH 13/13] fix crumb typo --- ui/lib/ldap/addon/routes/libraries/subdirectory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/lib/ldap/addon/routes/libraries/subdirectory.ts b/ui/lib/ldap/addon/routes/libraries/subdirectory.ts index d7a33e005188..136bb435d529 100644 --- a/ui/lib/ldap/addon/routes/libraries/subdirectory.ts +++ b/ui/lib/ldap/addon/routes/libraries/subdirectory.ts @@ -47,7 +47,7 @@ export default class LdapLibrariesSubdirectoryRoute extends Route { super.setupController(controller, resolvedModel, transition); const routeParams = (childResource: string) => { - return [resolvedModel.backendModel.id, , childResource]; + return [resolvedModel.backendModel.id, childResource]; }; controller.breadcrumbs = [