feat: runtime User Management screen with RBAC - #953
Merged
Conversation
Add a "User Management" screen under the project tree's Device folder, shown only while connected to a runtime. Lets an admin list, create, edit, and delete runtime accounts; a normal user can edit only its own account. - Shared frontend (editor + web, byte-identical): new `plc-user-management` editor variant, gated Device-tree leaf + Users icon, and a single-table screen with per-row edit/delete icons (OPC-UA style). - Reusable RuntimeUserModal (create | edit | bootstrap). The first-user dialog is refactored to use it. Edit mode shows a masked password placeholder and only sends a password when the field is actually edited (dirty-tracked) — an untouched form never resets a password. Editing your own password requires the current password. - RuntimePort gains listUsers / whoAmI / updateUser / deleteUser and a role on createUser; editor adapter + IPC channels implement them. create-user is sent unauthenticated for first-user bootstrap and authenticated (admin) afterwards. - UI role-gates actions (admin sees create/delete + role selector); the runtime remains the real authorization boundary. - Tests: adapter methods (100%), tabs factory, and the modal's dirty-password / current-password rules. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds runtime user-management support across IPC, middleware contracts, frontend navigation, a dedicated editor, shared modal flows, role-aware account operations, runtime capability gating, and validation tests. ChangesRuntime user management
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant UserManagementEditor
participant RuntimePort
participant rendererProcessBridge
participant IPCHandlers
participant RuntimeAPI
UserManagementEditor->>RuntimePort: Request user data or mutation
RuntimePort->>rendererProcessBridge: Invoke typed runtime bridge method
rendererProcessBridge->>IPCHandlers: Send runtime IPC request
IPCHandlers->>RuntimeAPI: Authenticate and call runtime endpoint
RuntimeAPI-->>UserManagementEditor: Return user data or operation result
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/frontend/components/_organisms/modals/runtime-user-modal.tsx (1)
199-208: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: avoid the
as RuntimeUserRolecast.Guidelines forbid type assertions other than
as const. Since the<select>only ever emits'admin'or'user', you can narrow instead of casting:♻️ Narrow instead of assert
- onChange={(e) => setRole(e.target.value as RuntimeUserRole)} + onChange={(e) => { + if (e.target.value === 'admin' || e.target.value === 'user') setRole(e.target.value) + }}As per coding guidelines: "Do not use type assertions, except
as const".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/components/_organisms/modals/runtime-user-modal.tsx` around lines 199 - 208, Remove the RuntimeUserRole type assertion from the select onChange handler. Narrow e.target.value to the supported 'admin' and 'user' values before passing it to setRole, preserving the existing role behavior without using any non-const type assertion.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/frontend/assets/icons/project/Users.tsx`:
- Around line 18-24: Update the SVG element in the Users icon component to
remove its role='button' semantics and mark it as decorative for assistive
technologies; preserve button behavior on the parent tree item or other
interactive leaf control.
In `@src/frontend/components/_molecules/project-tree/index.tsx`:
- Around line 769-770: Disable the double-click rename behavior for the
userManagement leaf in the tree component, alongside the existing devPin and
devConfig exclusions. Update the onDoubleClick path to avoid calling
setIsEditing(true) for userManagement while preserving the current rename
behavior for other editable leaves.
In `@src/frontend/store/slices/tabs/utils.ts`:
- Around line 185-186: Update CreateEditorObjectFromTab to add a default branch
that assigns the unmatched tab value to never, enforcing exhaustive handling of
all TabsProps variants before returning or throwing. Preserve the existing
user-management branch and editor creation behavior.
In `@src/main/modules/ipc/main.ts`:
- Around line 231-234: Update handleRuntimeCreateUser’s authenticated branch to
use a request path that treats all 2xx responses, including 201 Created, as
success instead of relying on makeRuntimeApiPostRequest’s 200-only behavior.
Preserve propagation of the response error for non-2xx responses.
---
Nitpick comments:
In `@src/frontend/components/_organisms/modals/runtime-user-modal.tsx`:
- Around line 199-208: Remove the RuntimeUserRole type assertion from the select
onChange handler. Narrow e.target.value to the supported 'admin' and 'user'
values before passing it to setRole, preserving the existing role behavior
without using any non-const type assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 57fd1acc-ecf8-4c0a-8c8d-eae3a0892c6f
📒 Files selected for processing (18)
src/frontend/assets/icons/project/Users.tsxsrc/frontend/components/_features/[workspace]/editor/user-management/index.tsxsrc/frontend/components/_molecules/project-tree/index.tsxsrc/frontend/components/_organisms/explorer/project.tsxsrc/frontend/components/_organisms/modals/__tests__/runtime-user-modal.test.tsxsrc/frontend/components/_organisms/modals/runtime-create-user-modal.tsxsrc/frontend/components/_organisms/modals/runtime-user-modal.tsxsrc/frontend/screens/workspace-screen.tsxsrc/frontend/store/__tests__/tabs-utils.test.tssrc/frontend/store/slices/editor/types.tssrc/frontend/store/slices/tabs/types.tssrc/frontend/store/slices/tabs/utils.tssrc/frontend/store/slices/workspace/types.tssrc/main/modules/ipc/main.tssrc/main/modules/ipc/renderer.tssrc/middleware/adapters/editor/__tests__/runtime-adapter.test.tssrc/middleware/adapters/editor/runtime-adapter.tssrc/middleware/shared/ports/runtime-port.ts
Comment on lines
+18
to
+24
| <svg | ||
| role='button' | ||
| viewBox='0 0 28 28' | ||
| fill='none' | ||
| xmlns='http://www.w3.org/2000/svg' | ||
| className={cn(`${sizeClasses[size]}`, className)} | ||
| {...res} |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not expose the decorative SVG as a button.
role='button' gives this SVG button semantics, but it has no accessible name, focusability, or keyboard handler; activation is handled by the parent tree item. Remove the role and mark the icon decorative, or move button semantics to the interactive leaf control.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/frontend/assets/icons/project/Users.tsx` around lines 18 - 24, Update the
SVG element in the Users icon component to remove its role='button' semantics
and mark it as decorative for assistive technologies; preserve button behavior
on the parent tree item or other interactive leaf control.
Comment on lines
+769
to
770
| {leafLang === 'devPin' || leafLang === 'devConfig' || leafLang === 'userManagement' ? null : ( | ||
| <Popover.Root open={isPopoverOpen && !isDebuggerVisible} onOpenChange={setPopoverOpen}> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Also disable the double-click rename path for userManagement.
Hiding the popover does not prevent onDoubleClick from calling setIsEditing(true) for this leaf. Double-clicking it opens a rename input and then shows the “cannot be renamed” error on blur.
Proposed fix
- onDoubleClick={() => !isDebuggerVisible && setIsEditing(true)}
+ onDoubleClick={() => !isDebuggerVisible && leafLang !== 'userManagement' && setIsEditing(true)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {leafLang === 'devPin' || leafLang === 'devConfig' || leafLang === 'userManagement' ? null : ( | |
| <Popover.Root open={isPopoverOpen && !isDebuggerVisible} onOpenChange={setPopoverOpen}> | |
| onDoubleClick={() => !isDebuggerVisible && leafLang !== 'userManagement' && setIsEditing(true)} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/frontend/components/_molecules/project-tree/index.tsx` around lines 769 -
770, Disable the double-click rename behavior for the userManagement leaf in the
tree component, alongside the existing devPin and devConfig exclusions. Update
the onDoubleClick path to avoid calling setIsEditing(true) for userManagement
while preserving the current rename behavior for other editable leaves.
Comment on lines
+185
to
+186
| case 'user-management': | ||
| return CreateUserManagementEditor(name) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add the required exhaustive check to CreateEditorObjectFromTab.
The new branch is correct, but the discriminated-union switch still has no never fallback. A future TabsProps variant could therefore return undefined silently.
Proposed fix
case 'diff-viewer':
return CreateDiffViewerEditor(name, elementType.filePath)
+ default: {
+ const exhaustiveCheck: never = elementType
+ return exhaustiveCheck
+ }As per coding guidelines, switches over discriminated unions must be exhaustive with a never check.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/frontend/store/slices/tabs/utils.ts` around lines 185 - 186, Update
CreateEditorObjectFromTab to add a default branch that assigns the unmatched tab
value to never, enforcing exhaustive handling of all TabsProps variants before
returning or throwing. Preserve the existing user-management branch and editor
creation behavior.
Source: Coding guidelines
Addresses review feedback on the User Management feature: 1. First-user setup now stays connected — the bootstrap dialog no longer tears down the just-established session when it closes on success. 4. Authenticated create-user accepts the runtime's 201 (was treated as an error, so the dialog showed the raw response and stayed open). 5. The self-edit "Current password" field now renders last (after Confirm password) and only once the password is actually being changed, so it doesn't shove the field the user just clicked. 7. Gate the User Management tree leaf on runtime version ≥ v4.1.9 (isUserManagementCapableRuntime); the connected runtime version is now stored in runtimeConnection and set on connect. Also: edit-icon tooltip now reads "Edit user" (icon no longer swallows the button title), "New User" label centered, and changing your own password signs you out to force a fresh login with the new credentials. Tests: version-gate helper, device-slice runtimeVersion, existing modal and adapter suites updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/backend/shared/firmware/runtime-version-gate.ts (1)
90-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the exported minimum version as the comparison source.
Lines [93-95] duplicate
4.1.9instead of usingMIN_USER_MANAGEMENT_RUNTIME_VERSION. If the public minimum changes, the constant and predicate can silently diverge and gate unsupported runtimes incorrectly. Parse/use the constant as the single source of truth while preserving the documented prerelease policy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/firmware/runtime-version-gate.ts` around lines 90 - 96, Update isUserManagementCapableRuntime to compare the parsed runtime against MIN_USER_MANAGEMENT_RUNTIME_VERSION rather than hardcoding 4.1.9. Parse or otherwise reuse the exported minimum-version constant as the single comparison source, while preserving the existing documented prerelease policy.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/frontend/components/_features/`[workspace]/editor/device/configuration/board.tsx:
- Around line 369-371: Clear the runtime version during all abandoned connection
paths in the handler containing setRuntimeVersion, including disconnect, error,
and cancel teardown. Consolidate cleanup where possible so status, token, and
runtimeVersion are reset together, ensuring RuntimeConnection.runtimeVersion is
null whenever the connection is disconnected or failed.
In `@src/frontend/store/slices/device/types.ts`:
- Line 151: Update the action-key contract test in device-types.test.ts to
include setRuntimeVersion in the expected keys and change the expected action
count from 19 to 20, preserving coverage of the complete action surface.
---
Nitpick comments:
In `@src/backend/shared/firmware/runtime-version-gate.ts`:
- Around line 90-96: Update isUserManagementCapableRuntime to compare the parsed
runtime against MIN_USER_MANAGEMENT_RUNTIME_VERSION rather than hardcoding
4.1.9. Parse or otherwise reuse the exported minimum-version constant as the
single comparison source, while preserving the existing documented prerelease
policy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9a1e96b7-7570-4f02-bb20-9c1801f46178
📒 Files selected for processing (13)
src/backend/shared/firmware/__tests__/runtime-version-gate.test.tssrc/backend/shared/firmware/runtime-version-gate.tssrc/frontend/components/_features/[workspace]/editor/device/configuration/board.tsxsrc/frontend/components/_features/[workspace]/editor/user-management/index.tsxsrc/frontend/components/_organisms/explorer/project.tsxsrc/frontend/components/_organisms/modals/runtime-create-user-modal.tsxsrc/frontend/components/_organisms/modals/runtime-user-modal.tsxsrc/frontend/store/__tests__/device-slice.test.tssrc/frontend/store/__tests__/device-types.test.tssrc/frontend/store/slices/device/slice.tssrc/frontend/store/slices/device/types.tssrc/frontend/utils/device.tssrc/main/modules/ipc/main.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/frontend/components/_organisms/explorer/project.tsx
- src/frontend/components/_features/[workspace]/editor/user-management/index.tsx
- src/frontend/components/_organisms/modals/runtime-user-modal.tsx
- src/frontend/components/_organisms/modals/runtime-create-user-modal.tsx
- src/main/modules/ipc/main.ts
Comment on lines
+369
to
+371
| // Remember the runtime version so version-gated UI (e.g. User | ||
| // Management) can react to it for the lifetime of the connection. | ||
| setRuntimeVersion(result.runtimeVersion ?? null) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear runtimeVersion when the connection attempt is abandoned.
Line [371] persists a version before login/connection validation completes. The handler’s disconnect, error, and cancel paths only update status/token, so the store can retain a non-null version while disconnected or errored, contrary to RuntimeConnection.runtimeVersion’s documented contract in src/frontend/store/slices/device/types.ts Lines [67-70]. Reset it on teardown/failure, ideally through one cleanup path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/frontend/components/_features/`[workspace]/editor/device/configuration/board.tsx
around lines 369 - 371, Clear the runtime version during all abandoned
connection paths in the handler containing setRuntimeVersion, including
disconnect, error, and cancel teardown. Consolidate cleanup where possible so
status, token, and runtimeVersion are reset together, ensuring
RuntimeConnection.runtimeVersion is null whenever the connection is disconnected
or failed.
| setRuntimeIpAddress: (ipAddress: string) => void | ||
| setRuntimeJwtToken: (token: string | null) => void | ||
| setRuntimeConnectionStatus: (status: ConnectionStatus) => void | ||
| setRuntimeVersion: (version: string | null) => void |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the action-key contract test for setRuntimeVersion.
Adding the action at Line [151] leaves src/frontend/store/__tests__/device-types.test.ts Lines [243-264] with 19 keys and no setRuntimeVersion. Add the key and update the expected length to 20; otherwise this test no longer checks the complete action surface.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/frontend/store/slices/device/types.ts` at line 151, Update the action-key
contract test in device-types.test.ts to include setRuntimeVersion in the
expected keys and change the expected action count from 19 to 20, preserving
coverage of the complete action surface.
The bootstrap dialog was still disconnecting on the success close. Reading runtimeConnection.connectionStatus in the close handler was stale: handleSubmit sets it to 'connected', but RuntimeUserModal then calls onOpenChange(false) synchronously before a re-render, so the handler saw the old 'connecting' value and reverted to 'disconnected'. Track success with a ref instead (set in handleSubmit, reset when the dialog opens) so the close handler reliably distinguishes a successful connect from a genuine cancel. Restores the pre-refactor behavior where the success path closed via a controlled prop change that never triggered the cancel logic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bug 3: the PlusIcon strokes `inherit` with no stroke color set, so it
rendered invisibly — its empty box pushed the "New User" label off-center.
Give it `stroke-current` (white on the brand button) so it shows as a
proper "+ New User".
Bug 6 follow-up: changing your own password signs you out, but the User
Management tab stayed open and any action then crashed with
"users.map is not a function". Two guards:
- When not connected, render a neutral placeholder instead of the table
and actions (which would hit the runtime unauthenticated).
- listUsers coerces a non-array payload to [] (the runtime returns an
existence-only {"msg":"Users found"} object when the token is invalid),
in the editor adapter and the screen, so the table can never receive a
non-array.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The tab atom maps fileDerivation.type to an icon and defaults to the IL icon when the type isn't handled — so the User Management tab showed the IL glyph. Add a 'user-management' case using the same UsersIcon as the project-tree leaf. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a User Management screen under the project tree's Device folder, shown only while connected to a runtime. An admin can list, create, edit, and delete runtime accounts; a normal user can edit only their own account. Pairs with the runtime API PR (Autonomy-Logic/openplc-runtime#155) and the web parity PR.
Changes
(you)marker, and a+ New Userbutton — actions role-gated in the UI (the runtime is the real authorization boundary).RuntimeUserModal(create | edit | bootstrap). The existing first-user dialog is refactored to use it. In edit mode the password field shows a masked placeholder and a password is only sent when actually edited (dirty-tracked) — an untouched form never resets a password; editing your own password requires the current password.RuntimePortgainslistUsers/whoAmI/updateUser/deleteUserand aroleoncreateUser; editor adapter + newruntime:*IPC channels implement them.create-usergoes unauthenticated for first-user bootstrap, authenticated (admin) afterwards.plc-user-managementeditor variant + gated Device-tree leaf + Users icon.Cross-repo / parity
frontend/**,middleware/shared/**) are byte-identical with openplc-web (compare-surfaces gate passes, 0 diffs).Testing
RuntimeUserModaldirty-password / current-password / validation rules. Typecheck, ESLint, andvalidate:archclean; full suite green.🤖 Generated with Claude Code
Summary by CodeRabbit