UI: LDAP hierarchal roles #28763
UI: LDAP hierarchal roles #28763
Conversation
github-actions
bot
added
the
hashicorp-contributed-pr
|
CI Results: failed ❌ |
| @@ -73,12 +74,6 @@ export interface TtlEvent { | |||
| goSafeTimeString: string; | |||
| } | |||
|
|
|||
| export interface Breadcrumb { | |||
|
|
||
| // LIST request for children of a hierarchal role | ||
| async _querySubdirectory(backend, roleAncestry) { | ||
| // path_to_role is the ancestral path |
There was a problem hiding this comment.
maybe an example in the comment of what an ancestral path could look like? or a link to the documentation. https://developer.hashicorp.com/vault/docs/secrets/ldap#hierarchical-paths
| @@ -33,8 +33,18 @@ export default class LdapRoleAdapter extends NamedPathAdapter { | |||
| } | |||
|
|
|||
| async query(store, type, query, recordArray, options) { | |||
| const { showPartialError } = options.adapterOptions || {}; | |||
| const { showPartialError, roleAncestry } = options.adapterOptions || {}; | |||
There was a problem hiding this comment.
I like the name, but it's unclear if roleAncestry is a boolean, string, or an object. Without making this file typescript, maybe adding a default value would clarify.
| @@ -33,8 +33,18 @@ export default class LdapRoleAdapter extends NamedPathAdapter { | |||
| } | |||
|
|
|||
| async query(store, type, query, recordArray, options) { | |||
| const { showPartialError } = options.adapterOptions || {}; | |||
| const { showPartialError, roleAncestry } = options.adapterOptions || {}; | |||
There was a problem hiding this comment.
| const { showPartialError, roleAncestry } = options.adapterOptions || {}; | |
| const { showPartialError, roleAncestry={} } = options.adapterOptions || {}; |
| linkParams = (role: LdapRoleModel) => { | ||
| const route = role.name.endsWith('/') ? 'roles.subdirectory' : 'roles.role.details'; | ||
| // remove trailing forward slash from URL | ||
| const sanitizedName = sanitizePath(role.name); |
There was a problem hiding this comment.
SanitizePath does a little more than we're needing it to here. In addition to removing a trailing forward slash it will remove a leading forward slash and whitespace. One idea, might be to create another function in the addon/utils directory to remove only the trailing forward slash so we don't have any unexpected behavior. Similar to how we've narrowed down a function to just remove the leading slash called sanitizeStart.
| @@ -9,6 +9,7 @@ export default buildRoutes(function () { | |||
| this.route('overview'); | |||
| this.route('roles', function () { | |||
| this.route('create'); | |||
| this.route('subdirectory', { path: '/:type/subdirectory/*path_to_role' }); | |||
There was a problem hiding this comment.
Great naming and route structure.
Maybe add the comment here similar to the one included in KV routes noting why we're using wildcard vs dynamic.
| }, | ||
| { adapterOptions: { showPartialError: true } } | ||
| ), | ||
| roles: this.lazyQuery({ showPartialError: true }, backendModel.id, params), |
There was a problem hiding this comment.
I like this, creative and clean.
|
closing in favor of #28824 |
Description
Hierarchical paths for roles in the LDAP secrets engine were added in 1.17 #27203. This PR updates the UI to handle listing roles at the hierarchical level. Originally we were just going to disable roles ending in a forward slash and role out the listing feature later. However, we were able to carve out some time during hackweek to get this done. We followed a similar pattern to the kv v2 secrets engine for UX.
TODO only if you're a HashiCorp employee
to N, N-1, and N-2, using the
backport/ent/x.x.x+entlabels. If this PR is in the CE repo, you should only backport to N, using thebackport/x.x.xlabel, not the enterprise labels.of a public function, even if that change is in a CE file, double check that
applying the patch for this PR to the ENT repo and running tests doesn't
break any tests. Sometimes ENT only tests rely on public functions in CE
files.
in the PR description, commit message, or branch name.
description. Also, make sure the changelog is in this PR, not in your ENT PR.