feat(users): Permissions tab (legacy iframe embed) (#36719) - #37083
feat(users): Permissions tab (legacy iframe embed) (#36719)#37083AP2300 wants to merge 2 commits into
Conversation
Ships the Users portlet Create/Edit dialog shell with the Profile tab fully wired and placeholders for the three sibling tabs (delivered by #36718, #36719, #36720). Dialog / Profile tab: - 4-tab strip with Profile as the only functional tab; Roles, Permissions, and API Tokens render "Coming soon" placeholders - Header with avatar + name + Active status chip - Account section: first/last name, email, password + confirm, Active - Additional Info section: prefix/suffix/title/company/website - Access section: disabled (values informational only), shows admin / backend / frontend / hasConsoleAccess pulled from the loaded user - Delete User section (edit mode) with required replacement-user picker and email-typed confirmation List CRUD: - DotUsersService gains getUser/createUser/updateUser using POST/PUT/GET /api/v1/users; roles field intentionally omitted on update so backend preserves role membership (see UserResource#processRoles) - DotUsersListStore gains createUser / updateUser / deleteSingleUser - Bulk delete on the list toolbar now shows the same replacement picker instead of the old p-confirmDialog - /users route now resolves to the new users-beta portlet id Shared: - DotUsersReplacementPickerComponent — server-backed autocomplete used by both delete flows; excludes the users being deleted from suggestions client-side Test coverage: 56 tests in the portlet + 46 in data-access all pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the Permissions tab to the Create/Edit User dialog. Renders the legacy Dojo role-permissions UI inside an iframe as the interim solution until a native Angular permissions surface lands. Changes: - New JSP bridge at html/portlet/ext/useradmin/permissions.jsp that loads view_role_permissions_js_inc.jsp / _inc.jsp for the user's implicit role. Declares its own dojo.require + norm() helper so the include page does not throw when embedded standalone. Bumps the accordion container to 720px so more permission rows fit before the iframe scrollbar kicks in. - New presentational component `dot-permissions-iframe` split out of the existing dialog wrapper so the tab can embed it inline (dialog wrapper now composes it instead of duplicating markup). - Users Permissions tab uses the new component with the sandbox JSP URL parameterized by userId. In create mode, shows a "save the user first" hint since the JSP requires a persisted user. - Shell now imports the real Permissions tab (replacing the placeholder from #36717) and re-adds the Save button disable + tooltip on the Permissions tab (that tab has its own Save inside the iframe). Test coverage: 56 portlet tests + 14 UI tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
Area : Backend
| // Only allow same-origin relative paths; reject absolute URLs, | ||
| // protocol-relative URLs, and dangerous schemes (javascript:, | ||
| // data:, http:, etc.) | ||
| if (!url.startsWith('/') || url.startsWith('//')) return null; |
There was a problem hiding this comment.
This guard came over from the dialog unchanged, so it isn't something the PR introduced — but it's worth tightening now that it's the shared entry point. /\evil.com passes both checks and browsers resolve it as protocol-relative: the URL parser treats \ as / in the relative-slash state, so the host becomes evil.com and the frame loads cross-origin. Since bypassSecurityTrustResourceUrl turns Angular's sanitizer off, this line is the only thing standing there — rejecting a second character of / or \ closes it.
| @let src = $iframeSrc(); | ||
|
|
||
| @if (src) { | ||
| <iframe |
There was a problem hiding this comment.
The iframe renders with no load or error handling, and the JSP behind it emits an empty body when the viewer lacks users portlet access or the id doesn't resolve — so a permission failure and a slow load look identical from here: a blank white pane. A (load) handler switching off a skeleton, plus a fallback message when nothing arrives, would make the difference visible.
Small aside on the same element: title="Permissions" is hardcoded English in a shared lib, so it reaches screen readers untranslated. An input or an i18n key travels better.
| * [url]="/html/portlet/ext/useradmin/permissions.jsp?userId=…&popup=true" | ||
| * minHeight="34rem" /> | ||
| * | ||
| * The dialog variant `DotPermissionsIframeDialogComponent` is a thin |
There was a problem hiding this comment.
Good extraction — the presentational/dialog split is what lets the tab embed this without dragging in the dialog machinery. Worth one more step while it's fresh: dot-roles/dot-role-permissions-iframe is a fourth hand-rolled copy of this same bypassSecurityTrustResourceUrl dance, and its own comment says it follows the dialog's pattern. It could become a thin wrapper passing [url] from the store, which would also mean the guard fix above covers it.
| standalone: true, | ||
| imports: [DotMessagePipe, DotPermissionsIframeComponent], | ||
| templateUrl: './dot-users-permissions-tab.component.html', | ||
| changeDetection: ChangeDetectionStrategy.OnPush, |
There was a problem hiding this comment.
changeDetection can go (OnPush is the framework default now) and standalone: true is implied — the two libs/ui components in this same PR already omit both and use #config / $url, so this file ends up the odd one out. userId and permissionsUrl want the $ prefix for the same reason. Also host: 'flex h-full min-h-0 flex-col block' sets display twice; dropping block removes the ambiguity.
| changeDetection: ChangeDetectionStrategy.OnPush, | ||
| host: { class: 'flex h-full min-h-0 flex-col block' } | ||
| }) | ||
| export class DotUsersPermissionsTabComponent { |
There was a problem hiding this comment.
No spec for this one, while both libs/ui components in the PR got theirs. Two cases would cover it: the create-mode empty state, and that the built URL carries the userId plus popup=true — your comment says that second param is load-bearing, which is exactly what a test should pin down.
| <div | ||
| class="flex h-full min-h-96 flex-col items-center justify-center gap-2 py-8" | ||
| data-testid="users-permissions-tab-empty"> | ||
| <span class="material-symbols-rounded text-5xl! text-surface-400" aria-hidden="true"> |
There was a problem hiding this comment.
text-5xl! is an !important with no reason beside it. If the Material Symbols sizing really needs it a short comment would do; otherwise plain text-5xl should hold.
2 participants
Closes #36719
Depends on #36990 (Profile tab shell). This PR's diff currently includes the shell commits; once #36990 is merged to main, this branch will be rebased onto the new main and the diff will show only the Permissions tab changes.
Summary
Ships the Permissions tab of the Users dialog as an iframe embed of the legacy permissions surface. Full-native reimplementation is deferred — the legacy UI already covers the acceptance criteria and reimplementing it would multiply the scope of the users portlet migration.
/html/portlet/ext/useradmin/*permissions view scoped to the currently-edited user id.Notable non-obvious calls
Test plan
🤖 Generated with Claude Code