[DNM] feat: add chat-access site-wide role to gate chat creation#23724
[DNM] feat: add chat-access site-wide role to gate chat creation#23724
Conversation
Members no longer get chat CRUD by default. A new built-in 'chat-access' role must be assigned to grant chat create, read, update, and delete permissions on the user's own chats. Owners retain full access via site-level permissions. The system, owner, and user-admin roles can assign chat-access. A migration auto-assigns the role to every user who has ever created a chat, so existing users are not disrupted. Also excludes ResourceChat from org member and org service account allPermsExcept calls to prevent permission leaks.
Documentation CheckUpdates Needed
New Documentation Needed
Automated review via Coder Tasks |
There was a problem hiding this comment.
Pull request overview
Adds a new built-in chat-access site role to gate chat CRUD/creation, with backend RBAC updates, migration to backfill existing chat creators, and frontend role UI/mocks updates.
Changes:
- Introduce
chat-accessbuilt-in role (Go + TS constants) and update RBAC permission matrices/tests. - Remove
ResourceChatfrom default member/org-member/org-service-account permission sets to prevent implicit access. - Add DB migration to grant
chat-accessto users who have created chats; update frontend role display/description and story mocks.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| site/src/testHelpers/entities.ts | Adds mock chat-access role and includes it in mock role lists. |
| site/src/pages/OrganizationSettingsPage/UserTable/UserRoleCell.tsx | Adds chat-access to the role sort/display ordering. |
| site/src/pages/OrganizationSettingsPage/UserTable/EditRolesButton.tsx | Adds user-facing description for the new role. |
| site/src/pages/OrganizationSettingsPage/UserTable/EditRolesButton.stories.tsx | Updates story to include chat-access in selected roles. |
| site/src/api/typesGenerated.ts | Adds RoleChatAccess constant for frontend use. |
| codersdk/rbacroles.go | Adds RoleChatAccess constant to the SDK. |
| coderd/rbac/roles_test.go | Updates RBAC test matrix to require chat-access for chat CRUD at user scope. |
| coderd/rbac/roles.go | Defines chat-access role; removes ResourceChat from default member/org-member/service-account permission sets; allows assignment by system/owner/user-admin. |
| coderd/database/migrations/000455_chat_access_role.up.sql | Backfills chat-access to users who have created chats. |
| coderd/database/migrations/000455_chat_access_role.down.sql | Removes chat-access from all users on rollback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ResourceUser, | ||
| ResourceOrganizationMember, | ||
| ResourceAibridgeInterception, | ||
| ResourceChat, |
There was a problem hiding this comment.
Adding ResourceChat to the OrgMemberPermissions exclusion list removes all org-member-level permissions for the chat resource. However database.ChatFile.RBACObject() uses rbac.ResourceChat...InOrg(c.OrganizationID) (coderd/database/modelmethods.go:181-183), which means chat file CRUD is evaluated under org-member rules. After this change, regular org members (even with the new site-wide chat-access role, which only grants user-scope perms when org_owner == "") will be denied access to chat files.
Consider either (a) authorizing chat file operations as user-scope (don’t set org_owner for chat files / don’t use .InOrg(...)), or (b) introducing a dedicated resource type/permission model for chat files that matches the intended access control.
| ResourceChat, |
| ResourceUser, | ||
| ResourceOrganizationMember, | ||
| ResourceAibridgeInterception, | ||
| ResourceChat, |
There was a problem hiding this comment.
Same concern as OrgMemberPermissions: excluding ResourceChat here means org service accounts lose all org-member-level access to chat resources. Since ChatFile is modeled as ResourceChat with org_owner set, this will also block service-account interactions with chat files unless chat files are moved to user-scope or otherwise reworked.
| ResourceChat, |
| ), | ||
| User: append( | ||
| allPermsExcept(ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceWorkspace, ResourceUser, ResourceOrganizationMember, ResourceOrganizationMember, ResourceBoundaryUsage, ResourceAibridgeInterception), | ||
| allPermsExcept(ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceWorkspace, ResourceUser, ResourceOrganizationMember, ResourceOrganizationMember, ResourceBoundaryUsage, ResourceAibridgeInterception, ResourceChat), |
There was a problem hiding this comment.
allPermsExcept is called with ResourceOrganizationMember twice here, which is redundant and makes the exclusion list harder to reason about. Remove the duplicate argument to keep the role definition clean.
| allPermsExcept(ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceWorkspace, ResourceUser, ResourceOrganizationMember, ResourceOrganizationMember, ResourceBoundaryUsage, ResourceAibridgeInterception, ResourceChat), | |
| allPermsExcept(ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceWorkspace, ResourceUser, ResourceOrganizationMember, ResourceBoundaryUsage, ResourceAibridgeInterception, ResourceChat), |
| /** | ||
| * Ideally this roles would be generated from the rbac/roles.go package. | ||
| */ |
There was a problem hiding this comment.
The JSDoc comment has a grammatical error: "Ideally this roles" → "Ideally these roles".
chat-accessbuilt-in role granting chat CRUD at User scopeResourceChatfrom member, org member, and org service accountallPermsExceptcallsmemberMedenied,chatAccessUserallowedImplementation plan & decisions
Approach: New built-in role (like
auditor,template-admin) rather than permission flag onmember.Key decisions:
chatAccessUserNOT added torequiredSubjectsin test (followsauditor/orgAdminBanWorkspacepattern)ResourceChatexcluded to prevent permission leakuserAdmincan assign to stay consistent with user access managementBreaking change: Members without
chat-accesslose chat creation ability. Migration covers existing chat creators.