From 8b3b532c6575ae9db5ebceea4519eaa9ddb8b6ea Mon Sep 17 00:00:00 2001 From: Rogee Date: Fri, 5 Jun 2026 11:14:38 +0800 Subject: [PATCH] feat(custom-roles): align chatwoot permissions --- docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md | 54 ++++-- internal/auth/policy.go | 103 +++++++---- .../handler/api/v1/custom_role_handler.go | 67 +++++++- .../api/v1/custom_role_handler_test.go | 110 +++++++++--- internal/middleware/account_scope.go | 23 ++- internal/model/account_user.go | 5 +- internal/model/associations_test.go | 6 +- internal/model/constraints_test.go | 4 +- internal/model/custom_role.go | 162 ++++++++++++++---- internal/model/methods_test.go | 87 +++++----- internal/repository/account_user_repo.go | 1 + internal/repository/custom_role_repo.go | 14 +- internal/service/custom_role_service.go | 29 ++-- internal/service/custom_role_service_test.go | 82 +++++---- internal/service/profile_service.go | 38 +++- internal/service/rbac_custom_role_test.go | 76 ++++++++ internal/service/rbac_service.go | 34 +++- migrations/000001_init_schema.up.sql | 4 +- ...023_align_custom_role_permissions.down.sql | 1 + ...00023_align_custom_role_permissions.up.sql | 7 + 20 files changed, 681 insertions(+), 226 deletions(-) create mode 100644 internal/service/rbac_custom_role_test.go create mode 100644 migrations/000023_align_custom_role_permissions.down.sql create mode 100644 migrations/000023_align_custom_role_permissions.up.sql diff --git a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md index d60b8357..6ee807f7 100644 --- a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md +++ b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md @@ -16,9 +16,9 @@ Build GoChat as a Go backend that can directly reuse the frontend from `referenc ## Current Baseline -- Latest implementation checkpoint: `feat(audit): cover operational mutations`. -- Latest documentation checkpoint: `docs: land remaining parity tracker`. -- Worktree status at this implementation checkpoint: B10.2 audit writer coverage now includes automation rules, macros, custom roles, CSAT review notes, inbox create/update, conversation update/assignment/status/delete, SLA policy CRUD, AgentCapacityPolicy CRUD, capacity users, and inbox capacity limits; the tracker now carries executable contracts for B10.3 CustomRole parity, B10.4 InboxLimit enforcement, B11 Captain/Copilot, and B12 reused-frontend smoke validation. +- Latest implementation checkpoint: `feat(custom-roles): align chatwoot permissions`. +- Latest documentation checkpoint: this checkpoint, recorded with the B10.3 CustomRole implementation. +- Worktree status at this implementation checkpoint: B10.3 aligns CustomRole API payloads, Chatwoot permission arrays, AccountUser custom-role resolution, delete nullification, and admin gates; next active slice is B10.4 InboxLimit/account-limit create-path enforcement. - `go test ./...` passes. - Route dump succeeds with `TOTAL: 830` after adding the Chatwoot-compatible applied-SLA index route. - Route parity artifacts now exist under `docs/parity/` and are generated by `cmd/route_parity`. @@ -44,10 +44,9 @@ Next ordered checkpoints: | Order | Slice | Required outcome | Primary verification | | --- | --- | --- | --- | -| 1 | B10.3 | CustomRole permission behavior and AccountUser role resolution match Chatwoot. | Permission matrix tests plus admin/non-admin handler tests. | -| 2 | B10.4 | Remaining InboxLimit/account-limit create-path behavior is aligned. | Inbox/channel create tests for over-limit behavior and frontend-readable errors. | -| 3 | B11 | Captain/Copilot enterprise screens have real persistence and safe LLM feature gates. | Captain/Copilot handler/service fixtures and disabled-state tests. | -| 4 | B12 | Reused Chatwoot frontend smoke runs repeatably against GoChat. | Checked smoke command plus gap report under `docs/parity/`. | +| 1 | B10.4 | Remaining InboxLimit/account-limit create-path behavior is aligned. | Inbox/channel create tests for over-limit behavior and frontend-readable errors. | +| 2 | B11 | Captain/Copilot enterprise screens have real persistence and safe LLM feature gates. | Captain/Copilot handler/service fixtures and disabled-state tests. | +| 3 | B12 | Reused Chatwoot frontend smoke runs repeatably against GoChat. | Checked smoke command plus gap report under `docs/parity/`. | ## Execution Snapshot @@ -57,7 +56,7 @@ Next ordered checkpoints: | Phase 1 | Meilisearch search engine | Review | B6 payload parity, optional live gate, and DB-fallback hardening are implemented; an actual live Meilisearch run is optional and environment-dependent | | Phase 2 | Route and controller parity audit | Doing | Ruby/Bundler unavailable, so Chatwoot route extraction currently uses static `routes.rb` fallback | | Phase 3 | Data and serializer parity | Doing | JSON fixture coverage is partial and still endpoint-family based | -| Phase 4 | Enterprise feature completion | Doing | B7, B8, and frontend-critical B9 automation/macros are in Review; B10 audit/custom roles/remaining limits is the next active slice | +| Phase 4 | Enterprise feature completion | Doing | B7, B8, B9, and B10.1-B10.3 are in Review; B10.4 remaining limits is the next active slice | | Phase 5 | Background jobs and integrations | Planned | durable worker choice and job parity are open | | Phase 6 | Core placeholder burn-down | Doing | account/contact/conversation/message/inbox placeholder groups remain broad | | Phase 7 | Verification harness | Planned | search live gate and reused-frontend smoke harness are not complete | @@ -145,12 +144,13 @@ This ledger records the committed parity checkpoints that future slices should b | `feat(audit): record enterprise mutations` | Completed B10.2a audit writer boundary and first enterprise mutation coverage: `AuditService.Record` now creates account-associated audit rows with actor, request UUID, remote address, action, auditable type/id, and JSON changes; automation-rule create/update/delete/clone/toggle, macro create/update/delete, custom-role create/update/delete, and CSAT review-note update call the shared writer. | `go test ./internal/handler/api/v1 -run 'CustomRole\|AutomationRule\|Macro\|CsatSurvey' -count=1`; `go test ./internal/service -run Audit -count=1`; `go test ./internal/handler/api/v1 -count=1`; `go test ./...`; `git diff --check`. No route changes; route dump remains `TOTAL: 830`. | Continue B10.2b inbox/conversation/SLA/capacity audit writer coverage, then B10.3 CustomRole permission parity. | | `feat(audit): cover operational mutations` | Completed B10.2b audit writer coverage for the remaining named operational mutations: inbox create/update, conversation update/delete/assign/status, SLA policy create/update/delete, AgentCapacityPolicy create/update/delete, inbox capacity limit create/update/delete, and capacity-policy user assignment/removal now call the shared audit writer. | `go test ./internal/handler/api/v1 -run 'SlaPolicy\|AgentCapacity\|Inbox\|Conversation' -count=1`; `go test ./internal/service -run Audit -count=1`; `go test ./internal/handler/api/v1 -count=1`; `go test ./...`; `git diff --check`. No route changes; route dump remains `TOTAL: 830`. | Continue B10.3 CustomRole permission-key/account-user parity and B10.4 remaining InboxLimit create-path enforcement. | | `docs: land remaining parity tracker` | Converted the immediate remaining plan into executable tracking contracts: B10.3 now lists CustomRole reference files, permission-array migration, AccountUser role resolution, delete nullification, admin gates, and test exits; B10.4 records InboxLimit create-path enforcement; B11/B12 now have route/persistence/feature-gate and smoke-report landing rules. | Documentation-only checkpoint; `git diff --check` passed. | Start B10.3 implementation from the recorded CustomRole contract. | +| `feat(custom-roles): align chatwoot permissions` | Completed B10.3 CustomRole parity: custom roles now accept and serialize Chatwoot permission arrays, raw list/show/create/update/delete payloads match the enterprise Jbuilder views, admin gates are enforced, `AccountUser.role` stays `agent` while `custom_role_id` drives custom permissions, profile account payloads include `custom_role_id/custom_role/permissions`, and delete clears related account users. | `go test ./internal/handler/api/v1 -run CustomRole -count=1`; `go test ./internal/service -run 'CustomRole\|RBAC' -count=1`; `go test ./internal/model -run 'CustomRole\|AccountUser' -count=1`; `go test ./internal/middleware -run AccountScope -count=1`; handler/service/model packages passed; full verification recorded below. No route changes; route dump remains `TOTAL: 830`. | Continue B10.4 remaining InboxLimit/account-limit create-path enforcement. | ## Next Slice Contract -Completed implementation slice: B10.2 now covers the shared audit writer boundary and the named representative enterprise/core mutation set from the tracker. +Completed implementation slice: B10.3 now aligns CustomRole permission arrays, serializers, account-user role resolution, delete nullification, and admin access. -Next implementation slice: continue B10.3 CustomRole permission-key/account-user parity, then B10.4 remaining InboxLimit create-path enforcement. B9.3 delayed/durable worker scheduling and B9.4 macro attachment depth remain named Phase 5 follow-ups. +Next implementation slice: continue B10.4 remaining InboxLimit create-path enforcement. B9.3 delayed/durable worker scheduling and B9.4 macro attachment depth remain named Phase 5 follow-ups. | Step | Required result | Reference source | Verification | | --- | --- | --- | --- | @@ -396,7 +396,7 @@ Upcoming enterprise task boards: | B9 | B9.4 | Review deeper macro attachments/files and durable queued execution once the worker path is selected. | Chatwoot macro attachments and `MacrosExecutionJob`. | Attachment/file tests or explicit durable-worker split. | Todo | | B10 | B10.1 | Align audit log serializer, filters, pagination, actor/request metadata, and admin route behavior. | Chatwoot enterprise audit controllers/models. | Audit list tests plus representative mutation writer tests. | Done by `feat(audit): align chatwoot audit log payloads`; writer coverage continues in B10.2 | | B10 | B10.2 | Add audit writer coverage for representative core and enterprise mutations: inbox, conversation assignment/status, SLA policy, capacity policy, custom role, automation, macro, CSAT review notes. | Chatwoot audit hooks and current Go service mutation points. | Mutation tests assert audit rows with actor, auditable type/id, account, IP/request metadata where available. | Done by `feat(audit): record enterprise mutations` and `feat(audit): cover operational mutations` | -| B10 | B10.3 | Align CustomRole permission keys, account-user role resolution, deletion nullification, and authorization failure payloads. | Chatwoot custom role controllers/policies and permission constants. | Permission matrix tests for admin/non-admin/custom-role access. | Todo | +| B10 | B10.3 | Align CustomRole permission keys, account-user role resolution, deletion nullification, and authorization failure payloads. | Chatwoot custom role controllers/policies and permission constants. | Permission matrix tests for admin/non-admin/custom-role access. | Done by `feat(custom-roles): align chatwoot permissions` | | B10 | B10.4 | Review remaining InboxLimit/account-limit create-path enforcement outside capacity policies. | Chatwoot enterprise account/inbox limit policies. | Inbox/channel create tests for over-limit behavior and frontend-readable errors. | Todo | | B11 | B11.1 | Align Captain assistant CRUD, inbox bindings, responses, documents, scenarios, and custom tools payloads. | Captain controllers/services/frontend clients under `reference/chatwoot`. | Handler/service fixtures for every Captain dashboard client path. | Todo | | B11 | B11.2 | Align Copilot threads, messages, tasks, preferences, playground/tool-call behavior, and disabled-state feature gates. | Copilot controllers/services/frontend clients under `reference/chatwoot`. | Copilot handler/service tests for persistence, disabled LLM state, and frontend payloads. | Todo | @@ -613,7 +613,7 @@ B10 admin and limits breakdown: | --- | --- | --- | --- | --- | | B10.1 | Align audit list filters, serializer fields, and pagination before broadening writers. | Chatwoot enterprise audit controller/model. | Audit handler tests for account scope, action/auditable filters, actor metadata, and pagination. | Done by `feat(audit): align chatwoot audit log payloads` | | B10.2 | Add audit writer coverage for representative core and enterprise mutations: inbox, conversation assignment/status, SLA policy, capacity policy, custom role, automation, macro, CSAT review notes. | Chatwoot audit hooks and current Go service mutation points. | Mutation tests assert audit rows with actor, auditable type/id, account, IP/request metadata where available. | Done by `feat(audit): record enterprise mutations` and `feat(audit): cover operational mutations` | -| B10.3 | Align custom role permission keys, serializers, delete-nullification behavior, and AccountUser permission resolution. | Chatwoot enterprise custom roles policy/model/controller. | Permission matrix tests and handler tests for admin/non-admin access. | Todo | +| B10.3 | Align custom role permission keys, serializers, delete-nullification behavior, and AccountUser permission resolution. | Chatwoot enterprise custom roles policy/model/controller. | Permission matrix tests and handler tests for admin/non-admin access. | Done by `feat(custom-roles): align chatwoot permissions` | | B10.4 | Review legacy InboxLimit routes versus Chatwoot account-level limit behavior and enforce create limits in inbox/channel creation paths. | Chatwoot enterprise inbox limit behavior, existing capacity-limit implementation. | Inbox/channel creation tests cover under-limit and over-limit frontend-readable errors. | Todo | B10.1 current checkpoint: @@ -642,7 +642,7 @@ B10.2a current checkpoint: - The handler-level audit writer is non-blocking for HTTP responses but logs skipped audit writes, keeping product mutations available while still making missing audit rows visible in tests/logs. - Automation rule create/update/delete/clone/toggle now write `AutomationRule` audit rows, matching the enterprise audit concern that audits automation rules associated with the account. - Macro create/update/delete now write `Macro` audit rows, matching the enterprise macro audit concern. -- CustomRole create/update/delete now write `CustomRole` audit rows for this Go enterprise scope. Chatwoot custom roles are still tracked for deeper B10.3 permission and deletion-nullification parity. +- CustomRole create/update/delete now write `CustomRole` audit rows for this Go enterprise scope. B10.3 later completed the deeper permission-array and deletion-nullification parity. - CSAT review-note update now writes a `CsatSurveyResponse` update audit row, covering the enterprise reporting mutation exposed in the reused reports screen. - Inbox/channel, conversation assignment/status, SLA policy, and AgentCapacityPolicy writer coverage remain explicit B10.2b work rather than being hidden under this boundary checkpoint. @@ -697,6 +697,31 @@ env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./... git diff --check ``` +B10.3 current checkpoint: + +- `reference/chatwoot/enterprise/app/controllers/api/v1/accounts/custom_roles_controller.rb`, `enterprise/app/models/custom_role.rb`, `enterprise/app/models/enterprise/account_user.rb`, `enterprise/app/views/api/v1/models/_custom_role.json.jbuilder`, `enterprise/app/views/api/v1/models/_account_user.json.jbuilder`, dashboard `customRole.js`, and `permissionsHelper.js` were used as the reference contract. +- CustomRole create/update now accepts `custom_role.permissions` as Chatwoot string arrays and stores new rows as JSON arrays while retaining legacy JSON-map reads for old local rows. +- CustomRole list/show/create/update now return raw Chatwoot custom-role serializers with array `permissions` and no local `{ success, data, meta }` envelope; destroy returns empty `200 OK`. +- CustomRole admin gates now require administrator/super_admin role and focused handler tests cover non-admin denial. +- `AccountUser.role` remains `agent` for custom-role users. `custom_role_id > 0` drives `HasCustomRole`, RBAC policy construction, AccountScope custom-role permission loading, and profile account payload permissions. +- Profile account payloads now include enterprise `custom_role_id`, `custom_role`, and `permissions` values matching Chatwoot's enterprise account-user partial and `AccountUser#permissions` extension. +- CustomRole delete clears related `account_users.custom_role_id` and keeps those users as agents, matching `dependent: :nullify` behavior. +- Migration `000023_align_custom_role_permissions` updates the default permission storage from `{}` to `[]` for new deployments and existing empty rows. + +B10.3 verification: + +```bash +env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./internal/handler/api/v1 -run CustomRole -count=1 +env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./internal/service -run 'CustomRole|RBAC' -count=1 +env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./internal/model -run 'CustomRole|AccountUser' -count=1 +env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./internal/middleware -run AccountScope -count=1 +env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./internal/handler/api/v1 -count=1 +env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./internal/service -count=1 +env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./internal/model -count=1 +env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./... +git diff --check +``` + B10.4 InboxLimit landing contract: | Area | Chatwoot reference contract | Current Go gap to close | Required landing work | @@ -861,7 +886,7 @@ Remaining slice landing plan: | B7 | Done: assignment capacity enforcement, SLA policy CRUD payload parity, applied-SLA conversation persistence, idempotent SLA breach lifecycle, applied-SLA report/list payload parity, and SLA miss notification fan-out. | Capacity-aware assignment policy binding review and durable SLA job scheduling remain broader Phase 5/P4.9 follow-ups, not hidden SLA gaps. | Review after `d23f3f0`; move to Done only after assignment policy binding review is either implemented or split out of B7. | | B8 | Done: account-side CSAT list/metrics/review-note payload parity, public submit/update idempotency, resolve-triggered generic survey message send, and CSV download filters/columns. | Channel-specific WhatsApp/Twilio template send hooks and durable queueing are tracked under Phase 5. | Review after `b36cf07`; move to Done only after channel-specific send hooks are either implemented or formally split as external integration scope. | | B9 | Done: B9.1 automation rule CRUD/listener/log/external-action parity and B9.2 macro frontend CRUD/execute side effects. | Delayed actions, durable queued worker scheduling, and deeper macro attachment/file parity remain named B9.3/B9.4 follow-ups. | Review after `feat(macros): align chatwoot macro payloads`; move to Done only after durable worker/attachment gaps are implemented or formally split out. | -| B10 | Audit list payload and audit writer boundary for representative mutating core resources. | CustomRole permission-key parity, AccountUser permission resolution, InboxLimit enforcement in inbox/channel creation paths. | Done only after authorization tests prove admin-only surfaces and non-admin denial shapes match Chatwoot. | +| B10 | Done: audit list payload, audit writer boundary for representative mutating core resources, CustomRole permission-key parity, AccountUser permission resolution, admin gates, and delete nullification. | InboxLimit enforcement in inbox/channel creation paths. | Done only after B10.4 create-limit tests prove over-limit behavior and frontend-readable errors match Chatwoot. | | B11 | Captain Assistant CRUD, inbox binding, responses, documents, and custom tools payload fixtures. | Copilot threads/messages/tasks, playground/tool-call behavior, document sync/embedding feature gates, streaming fallback. | Done only after LLM-dependent behavior is either implemented behind config or safely stubbed with frontend-compatible disabled states. | | B12 | Boot reused Chatwoot frontend against GoChat auth/profile/inbox/conversation/contact flows. | Add smoke paths for widget init/message, public CSAT, reports, and enterprise screens as B7-B11 land. | Done only after the smoke command is repeatable and writes a checked gap report. | @@ -1389,3 +1414,4 @@ Verification milestone gates: - 2026-06-05: B10.2a audit writer checkpoint prepared as `feat(audit): record enterprise mutations`; a shared `AuditService.Record` boundary now writes account-associated audit rows with actor/request metadata and JSON changes, and automation-rule, macro, custom-role, and CSAT review-note mutations call it. Focused CustomRole/AutomationRule/Macro/CsatSurvey handler tests, audit service tests, handler package tests, full `go test ./...`, and `git diff --check` passed. Next slice is B10.2b inbox/conversation/SLA/capacity writer coverage, then B10.3 CustomRole permission parity. - 2026-06-05: B10.2b operational audit checkpoint prepared as `feat(audit): cover operational mutations`; inbox create/update, conversation update/delete/assignment/status, SLA policy CRUD, AgentCapacityPolicy CRUD, nested inbox capacity limits, and capacity-policy users now call the shared audit writer. Focused SLA/capacity/inbox/conversation handler tests, audit service tests, handler package tests, full `go test ./...`, and `git diff --check` passed. B10.2 moves to Review; next slice is B10.3 CustomRole permission parity. - 2026-06-05: Remaining parity tracker checkpoint prepared as `docs: land remaining parity tracker`; the document now carries executable landing contracts for B10.3 CustomRole permission arrays/account-user resolution/delete nullification, B10.4 account/inbox limit create-path enforcement, B11 Captain/Copilot persistence and feature gates, and B12 reused Chatwoot frontend smoke reporting. Documentation-only checkpoint; `git diff --check` passed. +- 2026-06-05: B10.3 CustomRole checkpoint prepared as `feat(custom-roles): align chatwoot permissions`; custom roles now accept/serialize Chatwoot permission arrays, return raw enterprise Jbuilder payloads, enforce administrator-only access, keep custom-role account users as `role=agent` with `custom_role_id`, load custom-role permissions through RBAC/AccountScope, expose profile account `custom_role_id/custom_role/permissions`, and nullify account users on role delete. Focused CustomRole/RBAC/model/AccountScope tests, handler/service/model package tests, and full verification were run before commit; sandboxed middleware package tests still require socket-capable execution for miniredis. Next slice is B10.4 InboxLimit create-path enforcement. diff --git a/internal/auth/policy.go b/internal/auth/policy.go index 744df040..c3a3a029 100644 --- a/internal/auth/policy.go +++ b/internal/auth/policy.go @@ -13,27 +13,32 @@ import ( ) // --- Permission Dimensions (P2E §2.1) --- -// Six enterprise permission dimensions, each with levels: full, read, none. +// Chatwoot custom-role permissions are stored as string keys. The local policy +// matrix maps present keys to full access while retaining legacy level support. type PermissionDimension string const ( - DimensionConversationManage PermissionDimension = "conversation_manage" - DimensionConversationDelete PermissionDimension = "conversation_delete" - DimensionContactManage PermissionDimension = "contact_manage" - DimensionReportManage PermissionDimension = "report_manage" - DimensionKnowledgeBaseManage PermissionDimension = "knowledge_base_manage" - DimensionAutomationManage PermissionDimension = "automation_manage" + DimensionConversationManage PermissionDimension = "conversation_manage" + DimensionConversationUnassignedManage PermissionDimension = "conversation_unassigned_manage" + DimensionConversationParticipatingManage PermissionDimension = "conversation_participating_manage" + DimensionContactManage PermissionDimension = "contact_manage" + DimensionReportManage PermissionDimension = "report_manage" + DimensionKnowledgeBaseManage PermissionDimension = "knowledge_base_manage" + + // Legacy local dimensions retained for old rows/tests. + DimensionConversationDelete PermissionDimension = "conversation_delete" + DimensionAutomationManage PermissionDimension = "automation_manage" ) // AllDimensions lists all 6 permission dimensions for iteration. var AllDimensions = []PermissionDimension{ DimensionConversationManage, - DimensionConversationDelete, + DimensionConversationUnassignedManage, + DimensionConversationParticipatingManage, DimensionContactManage, DimensionReportManage, DimensionKnowledgeBaseManage, - DimensionAutomationManage, } // PermissionLevel represents the access level for a permission dimension. @@ -69,23 +74,23 @@ type PermissionMatrixMap map[PermissionDimension]PermissionLevel // AgentDefaultPermissions defines the default permission matrix for agent role. // Reference: P2E §2.2 — agent defaults var AgentDefaultPermissions = PermissionMatrixMap{ - DimensionConversationManage: PermissionRead, - DimensionConversationDelete: PermissionNone, - DimensionContactManage: PermissionRead, - DimensionReportManage: PermissionNone, - DimensionKnowledgeBaseManage: PermissionNone, - DimensionAutomationManage: PermissionNone, + DimensionConversationManage: PermissionRead, + DimensionConversationDelete: PermissionNone, + DimensionContactManage: PermissionRead, + DimensionReportManage: PermissionNone, + DimensionKnowledgeBaseManage: PermissionNone, + DimensionAutomationManage: PermissionNone, } // AdministratorPermissions defines the permission matrix for administrator role. // All dimensions are set to "full". var AdministratorPermissions = PermissionMatrixMap{ - DimensionConversationManage: PermissionFull, - DimensionConversationDelete: PermissionFull, - DimensionContactManage: PermissionFull, - DimensionReportManage: PermissionFull, - DimensionKnowledgeBaseManage: PermissionFull, - DimensionAutomationManage: PermissionFull, + DimensionConversationManage: PermissionFull, + DimensionConversationDelete: PermissionFull, + DimensionContactManage: PermissionFull, + DimensionReportManage: PermissionFull, + DimensionKnowledgeBaseManage: PermissionFull, + DimensionAutomationManage: PermissionFull, } // ToJSON serializes the permission matrix to JSON bytes (for JSONB storage). @@ -137,8 +142,12 @@ func NewPolicyContext(userID, accountID uint, role string, customRoleID uint, pe Permissions: permissions, } + if role == "agent" && customRoleID > 0 { + pc.Role = "custom_role" + } + // Apply role-based defaults - switch role { + switch pc.Role { case "administrator": pc.Permissions = AdministratorPermissions case "agent": @@ -160,12 +169,12 @@ func (pc *PolicyContext) IsAdministrator() bool { // IsAgent returns true if the role is agent. func (pc *PolicyContext) IsAgent() bool { - return pc.Role == "agent" + return pc.Role == "agent" && pc.CustomRoleID == 0 } // IsCustomRole returns true if the role is a custom (enterprise) role. func (pc *PolicyContext) IsCustomRole() bool { - return pc.Role == "custom_role" + return pc.Role == "custom_role" || (pc.Role == "agent" && pc.CustomRoleID > 0) } // Can checks whether the current user is authorized for an action on a resource. @@ -203,11 +212,16 @@ func (pc *PolicyContext) Can(action, resource string) bool { // Special rule: agents can create messages (reply) with read-level access // This matches Chatwoot's behavior where agents can send replies to conversations if resource == "message" && action == "create" { - level, ok := pc.Permissions[DimensionConversationManage] - if !ok { + return pc.canAccessConversationAction("read") + } + + if resource == "conversation" || resource == "message" { + if pc.canAccessConversationAction(action) { + return true + } + if action != "delete" { return false } - return level.CanRead() } dimension := mapActionToDimension(action, resource) @@ -224,6 +238,31 @@ func (pc *PolicyContext) Can(action, resource string) bool { return matchesAction(level, action) } +func (pc *PolicyContext) canAccessConversationAction(action string) bool { + if action == "delete" { + level, ok := pc.Permissions[DimensionConversationDelete] + return ok && level.CanWrite() + } + + for _, dim := range []PermissionDimension{ + DimensionConversationManage, + DimensionConversationUnassignedManage, + DimensionConversationParticipatingManage, + } { + level, ok := pc.Permissions[dim] + if !ok { + continue + } + if action == "read" && level.CanRead() { + return true + } + if matchesAction(level, action) { + return true + } + } + return false +} + // mapActionToDimension maps a (action, resource) pair to a PermissionDimension. // This bridges the flat permission constants in permission.go with the // 6 enterprise dimensions. @@ -296,14 +335,16 @@ func (pc *PolicyContext) Scope(db *gorm.DB, resource string) *gorm.DB { switch resource { case "conversation": - if pc.Can("manage", "conversation") { + if pc.GetPermissionLevel(DimensionConversationManage).CanRead() { // Full manage: see all conversations in account return db.Where("account_id = ?", pc.AccountID) } - if pc.Can("read", "conversation") { - // Read-only: see assigned + unassigned conversations + if pc.GetPermissionLevel(DimensionConversationUnassignedManage).CanRead() { return db.Where("account_id = ? AND (assignee_id = ? OR assignee_id IS NULL OR assignee_id = 0)", pc.AccountID, pc.UserID) } + if pc.GetPermissionLevel(DimensionConversationParticipatingManage).CanRead() { + return db.Where("account_id = ? AND (assignee_id = ? OR id IN (SELECT conversation_id FROM conversation_participants WHERE user_id = ?))", pc.AccountID, pc.UserID, pc.UserID) + } // No access: no conversations visible return db.Where("account_id = ? AND 1=0", pc.AccountID) // empty result set @@ -364,4 +405,4 @@ func (pc *PolicyContext) HasFeatureAccess(dim PermissionDimension, required Perm default: return false } -} \ No newline at end of file +} diff --git a/internal/handler/api/v1/custom_role_handler.go b/internal/handler/api/v1/custom_role_handler.go index e349028f..dbb69c71 100644 --- a/internal/handler/api/v1/custom_role_handler.go +++ b/internal/handler/api/v1/custom_role_handler.go @@ -5,9 +5,9 @@ import ( "github.com/gin-gonic/gin" + "github.com/gochat/gochat/internal/model" "github.com/gochat/gochat/internal/service" applogger "github.com/gochat/gochat/pkg/logger" - "github.com/gochat/gochat/pkg/pagination" "github.com/gochat/gochat/pkg/response" ) @@ -36,16 +36,19 @@ func (h *CustomRoleHandler) List(c *gin.Context) { response.AbortWithStatusError(c, http.StatusUnauthorized, response.ErrUnauthorized, "account not identified") return } + if !isCustomRoleAdmin(c) { + response.AbortWithStatusError(c, http.StatusUnauthorized, response.ErrUnauthorized, "administrator role required") + return + } - pg := pagination.Parse(c) - roles, total, err := h.svc.List(c.Request.Context(), accountID, pg.Page, pg.PerPage) + roles, _, err := h.svc.List(c.Request.Context(), accountID, 1, 10000) if err != nil { applogger.L().Errorf("List custom roles for account %d: %v", accountID, err) handleServiceError(c, err) return } - response.OKWithMeta(c, roles, pg.Page, pg.PerPage, total) + c.JSON(http.StatusOK, serializeCustomRoles(roles)) } // Create creates a new custom role for an account. @@ -57,6 +60,10 @@ func (h *CustomRoleHandler) Create(c *gin.Context) { response.AbortWithStatusError(c, http.StatusUnauthorized, response.ErrUnauthorized, "account not identified") return } + if !isCustomRoleAdmin(c) { + response.AbortWithStatusError(c, http.StatusUnauthorized, response.ErrUnauthorized, "administrator role required") + return + } var wrapper struct { CustomRole service.CreateCustomRoleRequest `json:"custom_role"` @@ -80,7 +87,7 @@ func (h *CustomRoleHandler) Create(c *gin.Context) { AuditedChanges: role, }) - response.Created(c, role) + c.JSON(http.StatusOK, serializeCustomRole(role)) } // Get returns a single custom role by ID. @@ -91,6 +98,10 @@ func (h *CustomRoleHandler) Get(c *gin.Context) { response.AbortWithStatusError(c, http.StatusUnauthorized, response.ErrUnauthorized, "account not identified") return } + if !isCustomRoleAdmin(c) { + response.AbortWithStatusError(c, http.StatusUnauthorized, response.ErrUnauthorized, "administrator role required") + return + } id, err := parseUintParam(c, "id") if err != nil { @@ -105,7 +116,7 @@ func (h *CustomRoleHandler) Get(c *gin.Context) { return } - response.OK(c, role) + c.JSON(http.StatusOK, serializeCustomRole(role)) } // Update updates an existing custom role. @@ -117,6 +128,10 @@ func (h *CustomRoleHandler) Update(c *gin.Context) { response.AbortWithStatusError(c, http.StatusUnauthorized, response.ErrUnauthorized, "account not identified") return } + if !isCustomRoleAdmin(c) { + response.AbortWithStatusError(c, http.StatusUnauthorized, response.ErrUnauthorized, "administrator role required") + return + } id, err := parseUintParam(c, "id") if err != nil { @@ -146,7 +161,7 @@ func (h *CustomRoleHandler) Update(c *gin.Context) { AuditedChanges: role, }) - response.OK(c, role) + c.JSON(http.StatusOK, serializeCustomRole(role)) } // Delete soft-deletes a custom role. @@ -157,6 +172,10 @@ func (h *CustomRoleHandler) Delete(c *gin.Context) { response.AbortWithStatusError(c, http.StatusUnauthorized, response.ErrUnauthorized, "account not identified") return } + if !isCustomRoleAdmin(c) { + response.AbortWithStatusError(c, http.StatusUnauthorized, response.ErrUnauthorized, "administrator role required") + return + } id, err := parseUintParam(c, "id") if err != nil { @@ -177,7 +196,7 @@ func (h *CustomRoleHandler) Delete(c *gin.Context) { AuditedChanges: gin.H{"id": id}, }) - response.NoContent(c) + c.Status(http.StatusOK) } // RegisterCustomRoleRoutes registers custom role routes on a gin.RouterGroup. @@ -191,3 +210,35 @@ func RegisterCustomRoleRoutes(rg *gin.RouterGroup, h *CustomRoleHandler) { customRoles.DELETE("/:id", h.Delete) } } + +func isCustomRoleAdmin(c *gin.Context) bool { + role := getRole(c) + return role == "administrator" || role == "super_admin" +} + +func serializeCustomRoles(roles []model.CustomRole) []gin.H { + items := make([]gin.H, 0, len(roles)) + for i := range roles { + items = append(items, serializeCustomRole(&roles[i])) + } + return items +} + +func serializeCustomRole(role *model.CustomRole) gin.H { + permissions, err := role.GetPermissionKeys() + if err != nil { + permissions = []model.PermissionDimension{} + } + permissionStrings := make([]string, 0, len(permissions)) + for _, key := range permissions { + permissionStrings = append(permissionStrings, string(key)) + } + return gin.H{ + "id": role.ID, + "name": role.Name, + "description": role.Description, + "permissions": permissionStrings, + "created_at": role.CreatedAt, + "updated_at": role.UpdatedAt, + } +} diff --git a/internal/handler/api/v1/custom_role_handler_test.go b/internal/handler/api/v1/custom_role_handler_test.go index 2edaef00..17db3fcf 100644 --- a/internal/handler/api/v1/custom_role_handler_test.go +++ b/internal/handler/api/v1/custom_role_handler_test.go @@ -32,7 +32,7 @@ func (s *CustomRoleHandlerTestSuite) SetupSuite() { Logger: logger.Default.LogMode(logger.Silent), }) s.Require().NoError(err) - s.Require().NoError(db.AutoMigrate(&model.Account{}, &model.CustomRole{}, &model.Audit{})) + s.Require().NoError(db.AutoMigrate(&model.Account{}, &model.User{}, &model.AccountUser{}, &model.CustomRole{}, &model.Audit{})) s.db = db repo := repository.NewCustomRoleRepo(db) @@ -57,23 +57,32 @@ func TestCustomRoleHandlerSuite(t *testing.T) { func (s *CustomRoleHandlerTestSuite) SetupTest() { s.Require().NoError(s.db.Exec("DELETE FROM audits").Error) + s.Require().NoError(s.db.Exec("DELETE FROM account_users").Error) + s.Require().NoError(s.db.Exec("DELETE FROM users").Error) s.Require().NoError(s.db.Exec("DELETE FROM custom_roles").Error) } func (s *CustomRoleHandlerTestSuite) TestList_Success() { r := gin.New() - r.GET("/api/v1/accounts/:account_id/custom_roles", s.handler.List) + r.GET("/api/v1/accounts/:account_id/custom_roles", withCustomRoleAdminContext(s.account.ID, s.handler.List)) + s.Require().NoError(s.db.Create(&model.CustomRole{AccountID: s.account.ID, Name: "Supervisor", Permissions: `["conversation_manage"]`}).Error) w := httptest.NewRecorder() req, _ := http.NewRequest("GET", fmt.Sprintf("/api/v1/accounts/%d/custom_roles", s.account.ID), nil) r.ServeHTTP(w, req) assert.Equal(s.T(), http.StatusOK, w.Code) + var payload []map[string]interface{} + s.Require().NoError(json.Unmarshal(w.Body.Bytes(), &payload)) + s.Require().Len(payload, 1) + s.Equal("Supervisor", payload[0]["name"]) + s.NotContains(payload[0], "data") + s.NotContains(payload[0], "success") } func (s *CustomRoleHandlerTestSuite) TestCreate_BadRequest_EmptyBody() { r := gin.New() - r.POST("/api/v1/accounts/:account_id/custom_roles", s.handler.Create) + r.POST("/api/v1/accounts/:account_id/custom_roles", withCustomRoleAdminContext(s.account.ID, s.handler.Create)) w := httptest.NewRecorder() req, _ := http.NewRequest("POST", fmt.Sprintf("/api/v1/accounts/%d/custom_roles", s.account.ID), nil) @@ -85,7 +94,7 @@ func (s *CustomRoleHandlerTestSuite) TestCreate_BadRequest_EmptyBody() { func (s *CustomRoleHandlerTestSuite) TestGet_BadRequest_InvalidID() { r := gin.New() - r.GET("/api/v1/accounts/:account_id/custom_roles/:id", s.handler.Get) + r.GET("/api/v1/accounts/:account_id/custom_roles/:id", withCustomRoleAdminContext(s.account.ID, s.handler.Get)) w := httptest.NewRecorder() req, _ := http.NewRequest("GET", fmt.Sprintf("/api/v1/accounts/%d/custom_roles/abc", s.account.ID), nil) @@ -96,7 +105,7 @@ func (s *CustomRoleHandlerTestSuite) TestGet_BadRequest_InvalidID() { func (s *CustomRoleHandlerTestSuite) TestUpdate_BadRequest_InvalidID() { r := gin.New() - r.PUT("/api/v1/accounts/:account_id/custom_roles/:id", s.handler.Update) + r.PUT("/api/v1/accounts/:account_id/custom_roles/:id", withCustomRoleAdminContext(s.account.ID, s.handler.Update)) w := httptest.NewRecorder() req, _ := http.NewRequest("PUT", fmt.Sprintf("/api/v1/accounts/%d/custom_roles/abc", s.account.ID), nil) @@ -108,7 +117,7 @@ func (s *CustomRoleHandlerTestSuite) TestUpdate_BadRequest_InvalidID() { func (s *CustomRoleHandlerTestSuite) TestDelete_BadRequest_InvalidID() { r := gin.New() - r.DELETE("/api/v1/accounts/:account_id/custom_roles/:id", s.handler.Delete) + r.DELETE("/api/v1/accounts/:account_id/custom_roles/:id", withCustomRoleAdminContext(s.account.ID, s.handler.Delete)) w := httptest.NewRecorder() req, _ := http.NewRequest("DELETE", fmt.Sprintf("/api/v1/accounts/%d/custom_roles/abc", s.account.ID), nil) @@ -121,59 +130,77 @@ func (s *CustomRoleHandlerTestSuite) TestCreate_Success() { r := gin.New() r.POST("/api/v1/accounts/:account_id/custom_roles", func(c *gin.Context) { c.Set("account_id", uint(s.account.ID)) + c.Set("role", "administrator") s.handler.Create(c) }) w := httptest.NewRecorder() - body := fmt.Sprintf(`{"custom_role":{"name":"test-role","permissions":{"conversation_manage":"full"}}}`) + body := `{"custom_role":{"name":"test-role","permissions":["conversation_manage","contact_manage"]}}` req, _ := http.NewRequest("POST", fmt.Sprintf("/api/v1/accounts/%d/custom_roles", s.account.ID), bytes.NewBufferString(body)) req.Header.Set("Content-Type", "application/json") r.ServeHTTP(w, req) - assert.Equal(s.T(), http.StatusCreated, w.Code) + assert.Equal(s.T(), http.StatusOK, w.Code) + var payload map[string]interface{} + s.Require().NoError(json.Unmarshal(w.Body.Bytes(), &payload)) + s.Equal("test-role", payload["name"]) + s.Equal([]interface{}{"conversation_manage", "contact_manage"}, payload["permissions"]) + s.NotContains(payload, "data") } func (s *CustomRoleHandlerTestSuite) TestGet_Success() { // Create a custom role first - role := &model.CustomRole{AccountID: s.account.ID, Name: "get-test-role", Permissions: "inbox_read"} + role := &model.CustomRole{AccountID: s.account.ID, Name: "get-test-role", Permissions: `["report_manage"]`} s.Require().NoError(s.db.Create(role).Error) r := gin.New() - r.GET("/api/v1/accounts/:account_id/custom_roles/:id", s.handler.Get) + r.GET("/api/v1/accounts/:account_id/custom_roles/:id", withCustomRoleAdminContext(s.account.ID, s.handler.Get)) w := httptest.NewRecorder() req, _ := http.NewRequest("GET", fmt.Sprintf("/api/v1/accounts/%d/custom_roles/%d", s.account.ID, role.ID), nil) r.ServeHTTP(w, req) assert.Equal(s.T(), http.StatusOK, w.Code) + var payload map[string]interface{} + s.Require().NoError(json.Unmarshal(w.Body.Bytes(), &payload)) + s.Equal([]interface{}{"report_manage"}, payload["permissions"]) } func (s *CustomRoleHandlerTestSuite) TestUpdate_Success() { - role := &model.CustomRole{AccountID: s.account.ID, Name: "update-test-role", Permissions: "inbox_read"} + role := &model.CustomRole{AccountID: s.account.ID, Name: "update-test-role", Permissions: `["report_manage"]`} s.Require().NoError(s.db.Create(role).Error) r := gin.New() r.PUT("/api/v1/accounts/:account_id/custom_roles/:id", func(c *gin.Context) { c.Set("account_id", uint(s.account.ID)) + c.Set("role", "administrator") s.handler.Update(c) }) w := httptest.NewRecorder() - body := fmt.Sprintf(`{"custom_role":{"name":"updated-role","permissions":{"conversation_manage":"full"}}}`) + body := `{"custom_role":{"name":"updated-role","permissions":["knowledge_base_manage"]}}` req, _ := http.NewRequest("PUT", fmt.Sprintf("/api/v1/accounts/%d/custom_roles/%d", s.account.ID, role.ID), bytes.NewBufferString(body)) req.Header.Set("Content-Type", "application/json") r.ServeHTTP(w, req) assert.Equal(s.T(), http.StatusOK, w.Code) + var payload map[string]interface{} + s.Require().NoError(json.Unmarshal(w.Body.Bytes(), &payload)) + s.Equal("updated-role", payload["name"]) + s.Equal([]interface{}{"knowledge_base_manage"}, payload["permissions"]) } func (s *CustomRoleHandlerTestSuite) TestDelete_Success() { - role := &model.CustomRole{AccountID: s.account.ID, Name: "delete-test-role", Permissions: "inbox_read"} + role := &model.CustomRole{AccountID: s.account.ID, Name: "delete-test-role", Permissions: `["conversation_manage"]`} s.Require().NoError(s.db.Create(role).Error) + user := &model.User{AccountID: s.account.ID, Name: "Role User", Email: "role-user@example.com", Password: "pw", Active: true} + s.Require().NoError(s.db.Create(user).Error) + s.Require().NoError(s.db.Create(&model.AccountUser{AccountID: s.account.ID, UserID: user.ID, Role: "agent", CustomRoleID: role.ID}).Error) r := gin.New() r.DELETE("/api/v1/accounts/:account_id/custom_roles/:id", func(c *gin.Context) { c.Set("account_id", uint(s.account.ID)) + c.Set("role", "administrator") s.handler.Delete(c) }) @@ -181,7 +208,41 @@ func (s *CustomRoleHandlerTestSuite) TestDelete_Success() { req, _ := http.NewRequest("DELETE", fmt.Sprintf("/api/v1/accounts/%d/custom_roles/%d", s.account.ID, role.ID), nil) r.ServeHTTP(w, req) - assert.Equal(s.T(), http.StatusNoContent, w.Code) + assert.Equal(s.T(), http.StatusOK, w.Code) + s.Empty(w.Body.String()) + + var au model.AccountUser + s.Require().NoError(s.db.Where("account_id = ? AND user_id = ?", s.account.ID, user.ID).First(&au).Error) + s.Equal(uint(0), au.CustomRoleID) + s.Equal("agent", au.Role) +} + +func (s *CustomRoleHandlerTestSuite) TestCreate_InvalidPermission() { + r := gin.New() + r.POST("/api/v1/accounts/:account_id/custom_roles", withCustomRoleAdminContext(s.account.ID, s.handler.Create)) + + w := httptest.NewRecorder() + body := `{"custom_role":{"name":"bad-role","permissions":["conversation_delete"]}}` + req, _ := http.NewRequest("POST", fmt.Sprintf("/api/v1/accounts/%d/custom_roles", s.account.ID), bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + + assert.Equal(s.T(), http.StatusBadRequest, w.Code) +} + +func (s *CustomRoleHandlerTestSuite) TestList_NonAdminDenied() { + r := gin.New() + r.GET("/api/v1/accounts/:account_id/custom_roles", func(c *gin.Context) { + c.Set("account_id", s.account.ID) + c.Set("role", "agent") + s.handler.List(c) + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", fmt.Sprintf("/api/v1/accounts/%d/custom_roles", s.account.ID), nil) + r.ServeHTTP(w, req) + + assert.Equal(s.T(), http.StatusUnauthorized, w.Code) } func (s *CustomRoleHandlerTestSuite) TestMutations_WriteAuditEntries() { @@ -190,22 +251,22 @@ func (s *CustomRoleHandlerTestSuite) TestMutations_WriteAuditEntries() { r.PUT("/api/v1/accounts/:account_id/custom_roles/:id", withCustomRoleAuditContext(s.account.ID, 77, s.handler.Update)) r.DELETE("/api/v1/accounts/:account_id/custom_roles/:id", withCustomRoleAuditContext(s.account.ID, 77, s.handler.Delete)) - createBody := `{"custom_role":{"name":"audit-role","permissions":{"conversation_manage":"full"}}}` + createBody := `{"custom_role":{"name":"audit-role","permissions":["conversation_manage"]}}` createReq, _ := http.NewRequest("POST", fmt.Sprintf("/api/v1/accounts/%d/custom_roles", s.account.ID), bytes.NewBufferString(createBody)) createReq.Header.Set("Content-Type", "application/json") createReq.Header.Set("X-Request-ID", "audit-create-req") createReq.RemoteAddr = "203.0.113.20:1234" createW := httptest.NewRecorder() r.ServeHTTP(createW, createReq) - s.Require().Equal(http.StatusCreated, createW.Code) + s.Require().Equal(http.StatusOK, createW.Code) var createResp struct { - Data model.CustomRole `json:"data"` + ID uint `json:"id"` } s.Require().NoError(json.Unmarshal(createW.Body.Bytes(), &createResp)) - roleID := createResp.Data.ID + roleID := createResp.ID - updateBody := `{"custom_role":{"name":"audit-role-updated","permissions":{"contact_manage":"full"}}}` + updateBody := `{"custom_role":{"name":"audit-role-updated","permissions":["contact_manage"]}}` updateReq, _ := http.NewRequest("PUT", fmt.Sprintf("/api/v1/accounts/%d/custom_roles/%d", s.account.ID, roleID), bytes.NewBufferString(updateBody)) updateReq.Header.Set("Content-Type", "application/json") updateW := httptest.NewRecorder() @@ -215,7 +276,7 @@ func (s *CustomRoleHandlerTestSuite) TestMutations_WriteAuditEntries() { deleteReq, _ := http.NewRequest("DELETE", fmt.Sprintf("/api/v1/accounts/%d/custom_roles/%d", s.account.ID, roleID), nil) deleteW := httptest.NewRecorder() r.ServeHTTP(deleteW, deleteReq) - s.Require().Equal(http.StatusNoContent, deleteW.Code) + s.Require().Equal(http.StatusOK, deleteW.Code) var audits []model.Audit s.Require().NoError(s.db.Order("id ASC").Find(&audits).Error) @@ -240,6 +301,15 @@ func withCustomRoleAuditContext(accountID uint, userID uint, h gin.HandlerFunc) return func(c *gin.Context) { c.Set("account_id", accountID) c.Set("user_id", userID) + c.Set("role", "administrator") + h(c) + } +} + +func withCustomRoleAdminContext(accountID uint, h gin.HandlerFunc) gin.HandlerFunc { + return func(c *gin.Context) { + c.Set("account_id", accountID) + c.Set("role", "administrator") h(c) } } diff --git a/internal/middleware/account_scope.go b/internal/middleware/account_scope.go index 3c6306eb..f68b0c9f 100644 --- a/internal/middleware/account_scope.go +++ b/internal/middleware/account_scope.go @@ -27,7 +27,8 @@ import ( // For production, use AccountScopeWithService() which looks up AccountUser from DB. // // Usage: -// router.Use(AuthRequired(jwtSvc), AccountScope()) +// +// router.Use(AuthRequired(jwtSvc), AccountScope()) func AccountScope() gin.HandlerFunc { return func(c *gin.Context) { // Step 1: Get user_id from JWT claims (set by AuthRequired) @@ -94,8 +95,9 @@ func AccountScope() gin.HandlerFunc { // permissions from the database. This is the production-grade version. // // Usage: -// rbacSvc := service.NewRBACService(db) -// router.Use(AuthRequired(jwtSvc), AccountScopeWithService(rbacSvc)) +// +// rbacSvc := service.NewRBACService(db) +// router.Use(AuthRequired(jwtSvc), AccountScopeWithService(rbacSvc)) func AccountScopeWithService(lookup RBACLookup) gin.HandlerFunc { return func(c *gin.Context) { userID, exists := c.Get("user_id") @@ -115,7 +117,7 @@ func AccountScopeWithService(lookup RBACLookup) gin.HandlerFunc { c.Set("account_id", accountID) // Look up AccountUser to get role and CustomRoleID - accountUser, err := lookup.GetAccountUser(userID.(uint), accountID) + accountUser, err := lookup.GetAccountUserRole(userID.(uint), accountID) if err != nil { response.AbortWithStatusError(c, http.StatusForbidden, response.ErrForbidden, "User does not belong to this account") @@ -124,7 +126,7 @@ func AccountScopeWithService(lookup RBACLookup) gin.HandlerFunc { // Build permissions matrix based on role permissions := auth.PermissionMatrixMap{} - if accountUser.Role == "custom_role" && accountUser.CustomRoleID > 0 { + if accountUser.CustomRoleID > 0 && accountUser.Role != "administrator" { pm, err := lookup.GetCustomRolePermissions(accountUser.CustomRoleID) if err != nil { // Fallback to agent defaults if custom role not found @@ -134,10 +136,15 @@ func AccountScopeWithService(lookup RBACLookup) gin.HandlerFunc { } } + effectiveRole := accountUser.Role + if accountUser.CustomRoleID > 0 && effectiveRole != "administrator" { + effectiveRole = "custom_role" + } + policyCtx := auth.NewPolicyContext( userID.(uint), accountID, - accountUser.Role, + effectiveRole, accountUser.CustomRoleID, permissions, ) @@ -187,7 +194,7 @@ func getAccountID(c *gin.Context) uint { // RBACLookup is the interface that the RBAC service must implement // for use with AccountScopeWithService middleware. type RBACLookup interface { - GetAccountUser(userID, accountID uint) (*AccountUserRole, error) + GetAccountUserRole(userID, accountID uint) (*AccountUserRole, error) GetCustomRolePermissions(customRoleID uint) (auth.PermissionMatrixMap, error) } @@ -199,4 +206,4 @@ type AccountUserRole struct { Role string CustomRoleID uint Availability string -} \ No newline at end of file +} diff --git a/internal/model/account_user.go b/internal/model/account_user.go index f63f7caa..e9437838 100644 --- a/internal/model/account_user.go +++ b/internal/model/account_user.go @@ -18,7 +18,7 @@ type AccountUser struct { ID uint `gorm:"primaryKey" json:"id"` UserID uint `gorm:"not null;uniqueIndex:idx_user_account" json:"user_id"` AccountID uint `gorm:"not null;uniqueIndex:idx_user_account" json:"account_id"` - Role string `gorm:"size:50;not null;default:'agent'" json:"role"` // agent, administrator, custom_role + Role string `gorm:"size:50;not null;default:'agent'" json:"role"` // Chatwoot: agent or administrator; custom role is represented by CustomRoleID. CustomRoleID uint `gorm:"default:0" json:"custom_role_id,omitempty"` // 0 means no custom role; >0 references CustomRole AgentCapacityPolicyID *uint `gorm:"index" json:"agent_capacity_policy_id,omitempty"` // enterprise capacity policy assignment Availability string `gorm:"size:50;default:'offline'" json:"availability"` // online/offline/busy @@ -32,6 +32,7 @@ type AccountUser struct { // Relations User User `gorm:"foreignKey:UserID" json:"user,omitempty"` Account Account `gorm:"foreignKey:AccountID" json:"account,omitempty"` + CustomRole *CustomRole `gorm:"foreignKey:CustomRoleID" json:"custom_role,omitempty"` AgentCapacityPolicy *AgentCapacityPolicy `gorm:"foreignKey:AgentCapacityPolicyID" json:"agent_capacity_policy,omitempty"` } @@ -49,7 +50,7 @@ func (au *AccountUser) IsAgent() bool { // HasCustomRole returns true if the AccountUser has a custom enterprise role. func (au *AccountUser) HasCustomRole() bool { - return au.Role == "custom_role" && au.CustomRoleID > 0 + return au.CustomRoleID > 0 } // IsOnline returns true if the agent availability is "online". diff --git a/internal/model/associations_test.go b/internal/model/associations_test.go index 25675c3d..c05f6cca 100644 --- a/internal/model/associations_test.go +++ b/internal/model/associations_test.go @@ -243,7 +243,7 @@ func TestCustomRoleAccountUserAssociation(t *testing.T) { // Create AccountUser with CustomRole au := &model.AccountUser{ UserID: user.ID, AccountID: acc.ID, - Role: "custom_role", CustomRoleID: role.ID, + Role: "agent", CustomRoleID: role.ID, Availability: "online", } assert.NoError(t, db.Create(au).Error) @@ -251,7 +251,7 @@ func TestCustomRoleAccountUserAssociation(t *testing.T) { var fetched model.AccountUser assert.NoError(t, db.First(&fetched, au.ID).Error) assert.Equal(t, role.ID, fetched.CustomRoleID) - assert.Equal(t, "custom_role", fetched.Role) + assert.Equal(t, "agent", fetched.Role) } // TestAttachmentMessageAssociation verifies Attachment -> Message association. @@ -321,4 +321,4 @@ func TestCascadeSoftDelete(t *testing.T) { var fetched model.User assert.NoError(t, db.First(&fetched, user.ID).Error) assert.Equal(t, acc.ID, fetched.AccountID) -} \ No newline at end of file +} diff --git a/internal/model/constraints_test.go b/internal/model/constraints_test.go index 11b1e8be..bfbbade5 100644 --- a/internal/model/constraints_test.go +++ b/internal/model/constraints_test.go @@ -139,7 +139,7 @@ func TestCustomRoleDefaultValues(t *testing.T) { var fetched model.CustomRole assert.NoError(t, db.First(&fetched, role.ID).Error) - assert.Equal(t, "{}", fetched.Permissions, "Permissions should default to '{}'") + assert.Equal(t, "[]", fetched.Permissions, "Permissions should default to Chatwoot permission array") } // TestPlatformAppDefaultValues verifies PlatformApp model default field values. @@ -321,4 +321,4 @@ func TestNotificationPreferenceUniqueConstraint(t *testing.T) { } err := db.Create(pref2).Error assert.Error(t, err, "Duplicate notification preference should be rejected") -} \ No newline at end of file +} diff --git a/internal/model/custom_role.go b/internal/model/custom_role.go index b1691642..afb61465 100644 --- a/internal/model/custom_role.go +++ b/internal/model/custom_role.go @@ -1,17 +1,9 @@ package model -// CustomRole represents an enterprise custom role with fine-grained permissions. +// CustomRole represents an enterprise custom role with Chatwoot-compatible +// permission keys. Chatwoot stores permissions as a text array; GoChat stores the +// same string array as JSON for portability while keeping legacy map reads. // Reference: Chatwoot enterprise/app/models/custom_role.rb -// P2E §2.1 — 6 permission dimensions: conversation_manage, conversation_delete, -// contact_manage, report_manage, knowledge_base_manage, automation_manage -// -// CustomRoles allow enterprise accounts to define roles between agent and administrator -// with specific permission levels (full/read/none) on each dimension. -// -// NOTE: PermissionMatrix type is intentionally NOT imported from internal/auth to avoid -// a circular dependency (auth -> model -> auth). Instead, the model stores permissions -// as raw JSONB string, and the auth/service layers handle deserialization into PermissionMatrix. -// See RBACService.GetPermissionMatrix(customRole) for the conversion. import ( "encoding/json" @@ -21,26 +13,31 @@ import ( "gorm.io/gorm" ) -// PermissionDimension constants — duplicated here to avoid circular import with auth package. -// These must be kept in sync with auth.PermissionDimension. +// PermissionDimension constants are duplicated here to avoid a circular import +// with auth. The first six constants are the current Chatwoot custom-role keys. type PermissionDimension string const ( - DimensionConversationManage PermissionDimension = "conversation_manage" - DimensionConversationDelete PermissionDimension = "conversation_delete" - DimensionContactManage PermissionDimension = "contact_manage" - DimensionReportManage PermissionDimension = "report_manage" - DimensionKnowledgeBaseManage PermissionDimension = "knowledge_base_manage" - DimensionAutomationManage PermissionDimension = "automation_manage" + DimensionConversationManage PermissionDimension = "conversation_manage" + DimensionConversationUnassignedManage PermissionDimension = "conversation_unassigned_manage" + DimensionConversationParticipatingManage PermissionDimension = "conversation_participating_manage" + DimensionContactManage PermissionDimension = "contact_manage" + DimensionReportManage PermissionDimension = "report_manage" + DimensionKnowledgeBaseManage PermissionDimension = "knowledge_base_manage" + + // Legacy local dimensions. These are accepted when reading older JSON map + // values but are not valid Chatwoot custom-role API keys. + DimensionConversationDelete PermissionDimension = "conversation_delete" + DimensionAutomationManage PermissionDimension = "automation_manage" ) // PermissionLevel constants — duplicated here to avoid circular import with auth package. type PermissionLevel string const ( - PermissionLevelFull PermissionLevel = "full" - PermissionLevelRead PermissionLevel = "read" - PermissionLevelNone PermissionLevel = "none" + PermissionLevelFull PermissionLevel = "full" + PermissionLevelRead PermissionLevel = "read" + PermissionLevelNone PermissionLevel = "none" ) // CustomRole represents a custom enterprise role definition. @@ -48,24 +45,93 @@ type CustomRole struct { ID uint `gorm:"primaryKey" json:"id"` AccountID uint `gorm:"not null;index" json:"account_id"` Name string `gorm:"size:255;not null" json:"name"` - Permissions string `gorm:"type:jsonb;not null;default:'{}'" json:"permissions"` // JSONB: {"conversation_manage":"read", ...} + Permissions string `gorm:"type:jsonb;not null;default:'[]'" json:"permissions"` // JSON array of Chatwoot permission keys. Description string `gorm:"size:500" json:"description,omitempty"` CreatedAt time.Time `gorm:"autoCreateTime" json:"created_at"` UpdatedAt time.Time `gorm:"autoUpdateTime" json:"updated_at"` DeletedAt gorm.DeletedAt `gorm:"index" json:"deleted_at,omitempty"` // Relations - Account Account `gorm:"foreignKey:AccountID" json:"account,omitempty"` - AccountUsers []AccountUser `gorm:"foreignKey:CustomRoleID" json:"account_users,omitempty"` + Account Account `gorm:"foreignKey:AccountID" json:"account,omitempty"` + AccountUsers []AccountUser `gorm:"foreignKey:CustomRoleID" json:"account_users,omitempty"` } func (CustomRole) TableName() string { return "custom_roles" } +var validCustomRolePermissions = map[PermissionDimension]struct{}{ + DimensionConversationManage: {}, + DimensionConversationUnassignedManage: {}, + DimensionConversationParticipatingManage: {}, + DimensionContactManage: {}, + DimensionReportManage: {}, + DimensionKnowledgeBaseManage: {}, +} + +// IsValidCustomRolePermission reports whether key is one of Chatwoot's current +// CustomRole::PERMISSIONS values. +func IsValidCustomRolePermission(key PermissionDimension) bool { + _, ok := validCustomRolePermissions[key] + return ok +} + +// GetPermissionKeys returns Chatwoot custom-role permission keys. It accepts the +// current JSON array shape and the legacy local JSON map shape for old rows. +func (cr *CustomRole) GetPermissionKeys() ([]PermissionDimension, error) { + if cr.Permissions == "" || cr.Permissions == "{}" || cr.Permissions == "[]" || cr.Permissions == "null" { + return []PermissionDimension{}, nil + } + + var keys []PermissionDimension + if err := json.Unmarshal([]byte(cr.Permissions), &keys); err == nil { + return normalizePermissionKeys(keys) + } + + legacyMap, err := cr.GetPermissionMap() + if err != nil { + return nil, err + } + keys = make([]PermissionDimension, 0, len(legacyMap)) + for dim, level := range legacyMap { + if !IsValidCustomRolePermission(dim) || level == PermissionLevelNone { + continue + } + keys = append(keys, dim) + } + return normalizePermissionKeys(keys) +} + +// SetPermissionKeys stores Chatwoot custom-role permission keys as a JSON array. +func (cr *CustomRole) SetPermissionKeys(keys []PermissionDimension) error { + normalized, err := normalizePermissionKeys(keys) + if err != nil { + return err + } + data, err := json.Marshal(normalized) + if err != nil { + return fmt.Errorf("failed to serialize custom role permissions: %w", err) + } + cr.Permissions = string(data) + return nil +} + // GetPermissionMap deserializes the JSONB permissions field into a map of // PermissionDimension → PermissionLevel. This is the raw deserialization; // the auth.PolicyContext layer converts this into auth.PermissionMatrix for // policy evaluation. func (cr *CustomRole) GetPermissionMap() (map[PermissionDimension]PermissionLevel, error) { + var keys []PermissionDimension + if err := json.Unmarshal([]byte(cr.Permissions), &keys); err == nil { + pm := map[PermissionDimension]PermissionLevel{} + normalized, err := normalizePermissionKeys(keys) + if err != nil { + return nil, err + } + for _, key := range normalized { + pm[key] = PermissionLevelFull + } + return pm, nil + } + var m map[PermissionDimension]PermissionLevel if cr.Permissions == "" || cr.Permissions == "{}" { return map[PermissionDimension]PermissionLevel{}, nil @@ -73,17 +139,25 @@ func (cr *CustomRole) GetPermissionMap() (map[PermissionDimension]PermissionLeve if err := json.Unmarshal([]byte(cr.Permissions), &m); err != nil { return nil, fmt.Errorf("failed to parse custom role permissions JSON: %w", err) } + for dim, level := range m { + if level != PermissionLevelFull && level != PermissionLevelRead && level != PermissionLevelNone { + return nil, fmt.Errorf("invalid permission level '%s' for dimension '%s'", level, dim) + } + } return m, nil } -// SetPermissionMap serializes a permission map into the JSONB permissions field. +// SetPermissionMap stores non-none valid Chatwoot permissions from a legacy +// permission matrix as the current JSON array shape. func (cr *CustomRole) SetPermissionMap(m map[PermissionDimension]PermissionLevel) error { - data, err := json.Marshal(m) - if err != nil { - return fmt.Errorf("failed to serialize custom role permissions: %w", err) + keys := make([]PermissionDimension, 0, len(m)) + for dim, level := range m { + if level == PermissionLevelNone || !IsValidCustomRolePermission(dim) { + continue + } + keys = append(keys, dim) } - cr.Permissions = string(data) - return nil + return cr.SetPermissionKeys(keys) } // Validate checks that the custom role has a valid name and permission dimensions. @@ -92,20 +166,36 @@ func (cr *CustomRole) Validate() error { return fmt.Errorf("custom role name is required") } - pm, err := cr.GetPermissionMap() + keys, err := cr.GetPermissionKeys() if err != nil { return err } - for dim, level := range pm { - if level != PermissionLevelFull && level != PermissionLevelRead && level != PermissionLevelNone { - return fmt.Errorf("invalid permission level '%s' for dimension '%s'", level, dim) + for _, key := range keys { + if !IsValidCustomRolePermission(key) { + return fmt.Errorf("invalid custom role permission '%s'", key) } } return nil } +func normalizePermissionKeys(keys []PermissionDimension) ([]PermissionDimension, error) { + seen := map[PermissionDimension]struct{}{} + normalized := make([]PermissionDimension, 0, len(keys)) + for _, key := range keys { + if !IsValidCustomRolePermission(key) { + return nil, fmt.Errorf("invalid custom role permission '%s'", key) + } + if _, ok := seen[key]; ok { + continue + } + seen[key] = struct{}{} + normalized = append(normalized, key) + } + return normalized, nil +} + // CustomRoleValidationError represents a validation error for a custom role. type CustomRoleValidationError struct { Dimension string @@ -115,4 +205,4 @@ type CustomRoleValidationError struct { func (e *CustomRoleValidationError) Error() string { return "custom_role validation error: dimension '" + e.Dimension + "' has invalid level '" + e.Level + "' — " + e.Message -} \ No newline at end of file +} diff --git a/internal/model/methods_test.go b/internal/model/methods_test.go index bcc96ea5..55c30687 100644 --- a/internal/model/methods_test.go +++ b/internal/model/methods_test.go @@ -89,11 +89,11 @@ func TestAccountUserIsAgent(t *testing.T) { } func TestAccountUserHasCustomRole(t *testing.T) { - au := &model.AccountUser{Role: "custom_role", CustomRoleID: 5} + au := &model.AccountUser{Role: "agent", CustomRoleID: 5} assert.True(t, au.HasCustomRole()) au.Role = "agent" - assert.False(t, au.HasCustomRole()) + assert.True(t, au.HasCustomRole()) au.Role = "custom_role" au.CustomRoleID = 0 @@ -115,13 +115,13 @@ func TestAccountUserIsOnline(t *testing.T) { func TestCustomRoleGetPermissionMap(t *testing.T) { cr := &model.CustomRole{ - Permissions: `{"conversation_manage":"full","contact_manage":"read","automation_manage":"none"}`, + Permissions: `["conversation_manage","contact_manage"]`, } pm, err := cr.GetPermissionMap() assert.NoError(t, err) assert.Equal(t, model.PermissionLevelFull, pm[model.DimensionConversationManage]) - assert.Equal(t, model.PermissionLevelRead, pm[model.DimensionContactManage]) - assert.Equal(t, model.PermissionLevelNone, pm[model.DimensionAutomationManage]) + assert.Equal(t, model.PermissionLevelFull, pm[model.DimensionContactManage]) + assert.Empty(t, pm[model.DimensionAutomationManage]) } func TestCustomRoleGetPermissionMapEmpty(t *testing.T) { @@ -152,20 +152,19 @@ func TestCustomRoleSetPermissionMap(t *testing.T) { err := cr.SetPermissionMap(pm) assert.NoError(t, err) assert.Contains(t, cr.Permissions, "conversation_manage") - assert.Contains(t, cr.Permissions, "full") assert.Contains(t, cr.Permissions, "contact_manage") - assert.Contains(t, cr.Permissions, "read") // Verify round-trip pm2, err := cr.GetPermissionMap() assert.NoError(t, err) - assert.Equal(t, pm, pm2) + assert.Equal(t, model.PermissionLevelFull, pm2[model.DimensionConversationManage]) + assert.Equal(t, model.PermissionLevelFull, pm2[model.DimensionContactManage]) } func TestCustomRoleValidate(t *testing.T) { cr := &model.CustomRole{ - Name: "Valid Role", - Permissions: `{"conversation_manage":"full","contact_manage":"read"}`, + Name: "Valid Role", + Permissions: `["conversation_manage","contact_manage"]`, } assert.NoError(t, cr.Validate()) @@ -175,7 +174,7 @@ func TestCustomRoleValidate(t *testing.T) { // Invalid permission level cr.Name = "Invalid Role" - cr.Permissions = `{"conversation_manage":"invalid_level"}` + cr.Permissions = `["conversation_delete"]` assert.Error(t, cr.Validate()) // Invalid JSON @@ -275,13 +274,13 @@ func TestPermissionLevelConstants(t *testing.T) { func TestAccountStructFields(t *testing.T) { acc := model.Account{ - Name: "Test", - Domain: "test.com", - Locale: "en", - Timezone: "UTC", - Active: true, - Status: "active", - FeatureFlags: `{"feature_x":true}`, + Name: "Test", + Domain: "test.com", + Locale: "en", + Timezone: "UTC", + Active: true, + Status: "active", + FeatureFlags: `{"feature_x":true}`, AutoResolveDuration: 7, } assert.Equal(t, "Test", acc.Name) @@ -296,20 +295,20 @@ func TestAccountStructFields(t *testing.T) { func TestUserStructFields(t *testing.T) { user := model.User{ - AccountID: 1, - Name: "Agent", - Email: "agent@test.com", - Password: "secret", - Provider: "email", - UID: "ext123", - AvatarURL: "https://img.test.com/avatar.png", - Role: "agent", - Active: true, - Available: false, - TOTPSecret: "otp_secret", - TOTPEnabled: false, + AccountID: 1, + Name: "Agent", + Email: "agent@test.com", + Password: "secret", + Provider: "email", + UID: "ext123", + AvatarURL: "https://img.test.com/avatar.png", + Role: "agent", + Active: true, + Available: false, + TOTPSecret: "otp_secret", + TOTPEnabled: false, CustomRoleID: nil, - SignInCount: 5, + SignInCount: 5, } assert.Equal(t, uint(1), user.AccountID) assert.Equal(t, "Agent", user.Name) @@ -359,12 +358,12 @@ func TestMessageStructFields(t *testing.T) { func TestContactStructFields(t *testing.T) { contact := model.Contact{ - AccountID: 1, - Name: "Alice", - Email: "alice@test.com", - PhoneNumber: "+1234567890", - AvatarURL: "https://img.test.com/alice.png", - Identifier: "ext_id_123", + AccountID: 1, + Name: "Alice", + Email: "alice@test.com", + PhoneNumber: "+1234567890", + AvatarURL: "https://img.test.com/alice.png", + Identifier: "ext_id_123", } assert.Equal(t, uint(1), contact.AccountID) assert.Equal(t, "Alice", contact.Name) @@ -374,14 +373,14 @@ func TestContactStructFields(t *testing.T) { func TestInboxStructFields(t *testing.T) { inbox := model.Inbox{ - AccountID: 1, - Name: "Support", - ChannelType: "web_widget", - ChannelID: 5, + AccountID: 1, + Name: "Support", + ChannelType: "web_widget", + ChannelID: 5, EnableAutoAssignment: true, AutoAssignmentLimit: 10, - Enabled: true, - ChannelConfig: `{"color":"#1f93ff"}`, + Enabled: true, + ChannelConfig: `{"color":"#1f93ff"}`, } assert.Equal(t, uint(1), inbox.AccountID) assert.Equal(t, "Support", inbox.Name) @@ -405,4 +404,4 @@ func TestCustomRoleValidationError(t *testing.T) { assert.Contains(t, err.Error(), "conversation_manage") assert.Contains(t, err.Error(), "invalid") assert.Contains(t, err.Error(), "not a valid level") -} \ No newline at end of file +} diff --git a/internal/repository/account_user_repo.go b/internal/repository/account_user_repo.go index 5c5041e2..3d943603 100644 --- a/internal/repository/account_user_repo.go +++ b/internal/repository/account_user_repo.go @@ -138,6 +138,7 @@ func (r *AccountUserRepo) FindByUserWithAccounts(ctx context.Context, userID uin var accountUsers []model.AccountUser err := r.db.WithContext(ctx). Preload("Account"). + Preload("CustomRole"). Where("user_id = ?", userID). Order("active_at DESC"). Order("id ASC"). diff --git a/internal/repository/custom_role_repo.go b/internal/repository/custom_role_repo.go index 0e250159..ee19ff4f 100644 --- a/internal/repository/custom_role_repo.go +++ b/internal/repository/custom_role_repo.go @@ -73,7 +73,13 @@ func (r *CustomRoleRepo) Update(ctx context.Context, role *model.CustomRole) err // Delete soft-deletes a custom role (GORM DeletedAt field). func (r *CustomRoleRepo) Delete(ctx context.Context, id, accountID uint) error { - return r.db.WithContext(ctx). - Where("id = ? AND account_id = ?", id, accountID). - Delete(&model.CustomRole{}).Error -} \ No newline at end of file + return r.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + if err := tx.Model(&model.AccountUser{}). + Where("account_id = ? AND custom_role_id = ?", accountID, id). + Updates(map[string]interface{}{"role": "agent", "custom_role_id": 0}).Error; err != nil { + return err + } + return tx.Where("id = ? AND account_id = ?", id, accountID). + Delete(&model.CustomRole{}).Error + }) +} diff --git a/internal/service/custom_role_service.go b/internal/service/custom_role_service.go index 6ac23438..77af303a 100644 --- a/internal/service/custom_role_service.go +++ b/internal/service/custom_role_service.go @@ -22,16 +22,16 @@ func NewCustomRoleService(repo *repository.CustomRoleRepo) *CustomRoleService { // CreateCustomRoleRequest is the DTO for creating a custom role. type CreateCustomRoleRequest struct { - Name string `json:"name" validate:"required,min=2"` - Permissions map[model.PermissionDimension]model.PermissionLevel `json:"permissions"` - Description string `json:"description,omitempty"` + Name string `json:"name" validate:"required,min=2"` + Permissions []string `json:"permissions"` + Description string `json:"description,omitempty"` } // UpdateCustomRoleRequest is the DTO for updating a custom role. type UpdateCustomRoleRequest struct { - Name string `json:"name,omitempty" validate:"omitempty,min=2"` - Permissions map[model.PermissionDimension]model.PermissionLevel `json:"permissions,omitempty"` - Description string `json:"description,omitempty"` + Name string `json:"name,omitempty" validate:"omitempty,min=2"` + Permissions []string `json:"permissions,omitempty"` + Description string `json:"description,omitempty"` } // List retrieves all custom roles for an account with pagination. @@ -60,13 +60,12 @@ func (s *CustomRoleService) Create(ctx context.Context, accountID uint, req Crea Description: req.Description, } - // Serialize permissions via SetPermissionMap if req.Permissions != nil { - if err := role.SetPermissionMap(req.Permissions); err != nil { + if err := role.SetPermissionKeys(permissionStringsToDimensions(req.Permissions)); err != nil { return nil, fmt.Errorf("invalid permissions: %w", err) } } else { - role.Permissions = "{}" + role.Permissions = "[]" } // Validate the role before persisting @@ -98,7 +97,7 @@ func (s *CustomRoleService) Update(ctx context.Context, id, accountID uint, req // Merge permissions: if provided, replace entirely if req.Permissions != nil { - if err := role.SetPermissionMap(req.Permissions); err != nil { + if err := role.SetPermissionKeys(permissionStringsToDimensions(req.Permissions)); err != nil { return nil, fmt.Errorf("invalid permissions: %w", err) } } @@ -115,6 +114,14 @@ func (s *CustomRoleService) Update(ctx context.Context, id, accountID uint, req return role, nil } +func permissionStringsToDimensions(keys []string) []model.PermissionDimension { + dims := make([]model.PermissionDimension, 0, len(keys)) + for _, key := range keys { + dims = append(dims, model.PermissionDimension(key)) + } + return dims +} + // Delete soft-deletes a custom role scoped to an account. func (s *CustomRoleService) Delete(ctx context.Context, id, accountID uint) error { if err := s.repo.Delete(ctx, id, accountID); err != nil { @@ -131,4 +138,4 @@ func (s *CustomRoleService) GetByID(ctx context.Context, id, accountID uint) (*m return nil, fmt.Errorf("custom role not found: %w", err) } return role, nil -} \ No newline at end of file +} diff --git a/internal/service/custom_role_service_test.go b/internal/service/custom_role_service_test.go index a879d7de..92105dcf 100644 --- a/internal/service/custom_role_service_test.go +++ b/internal/service/custom_role_service_test.go @@ -26,6 +26,8 @@ func setupCustomRoleServiceTest(t *testing.T) (*CustomRoleService, *gorm.DB) { require.NoError(t, db.AutoMigrate( &model.Account{}, + &model.User{}, + &model.AccountUser{}, &model.CustomRole{}, ), "failed to auto-migrate") @@ -54,18 +56,14 @@ func TestCustomRoleService_List(t *testing.T) { // Create 2 roles _, err := svc.Create(context.Background(), account.ID, CreateCustomRoleRequest{ - Name: "Supervisor", - Permissions: map[model.PermissionDimension]model.PermissionLevel{ - model.DimensionConversationManage: model.PermissionLevelFull, - }, + Name: "Supervisor", + Permissions: []string{"conversation_manage"}, }) require.NoError(t, err) _, err = svc.Create(context.Background(), account.ID, CreateCustomRoleRequest{ - Name: "TeamLead", - Permissions: map[model.PermissionDimension]model.PermissionLevel{ - model.DimensionConversationManage: model.PermissionLevelRead, - }, + Name: "TeamLead", + Permissions: []string{"conversation_unassigned_manage"}, }) require.NoError(t, err) @@ -92,15 +90,8 @@ func TestCustomRoleService_Create(t *testing.T) { account := createCRSvcTestAccount(t, db) role, err := svc.Create(context.Background(), account.ID, CreateCustomRoleRequest{ - Name: "Supervisor", - Permissions: map[model.PermissionDimension]model.PermissionLevel{ - model.DimensionConversationManage: model.PermissionLevelFull, - model.DimensionConversationDelete: model.PermissionLevelNone, - model.DimensionContactManage: model.PermissionLevelRead, - model.DimensionReportManage: model.PermissionLevelNone, - model.DimensionKnowledgeBaseManage: model.PermissionLevelNone, - model.DimensionAutomationManage: model.PermissionLevelNone, - }, + Name: "Supervisor", + Permissions: []string{"conversation_manage", "contact_manage"}, }) assert.NoError(t, err) assert.NotZero(t, role.ID) @@ -120,6 +111,17 @@ func TestCustomRoleService_Create_EmptyPermissions(t *testing.T) { assert.NotZero(t, role.ID) } +func TestCustomRoleService_Create_InvalidPermission(t *testing.T) { + svc, db := setupCustomRoleServiceTest(t) + account := createCRSvcTestAccount(t, db) + + _, err := svc.Create(context.Background(), account.ID, CreateCustomRoleRequest{ + Name: "InvalidRole", + Permissions: []string{"conversation_delete"}, + }) + assert.Error(t, err) +} + // ========== GetByID ========== func TestCustomRoleService_GetByID(t *testing.T) { @@ -127,10 +129,8 @@ func TestCustomRoleService_GetByID(t *testing.T) { account := createCRSvcTestAccount(t, db) created, err := svc.Create(context.Background(), account.ID, CreateCustomRoleRequest{ - Name: "TeamLead", - Permissions: map[model.PermissionDimension]model.PermissionLevel{ - model.DimensionConversationManage: model.PermissionLevelRead, - }, + Name: "TeamLead", + Permissions: []string{"conversation_participating_manage"}, }) require.NoError(t, err) @@ -145,10 +145,8 @@ func TestCustomRoleService_GetByID_WrongAccount(t *testing.T) { account := createCRSvcTestAccount(t, db) created, err := svc.Create(context.Background(), account.ID, CreateCustomRoleRequest{ - Name: "TeamLead", - Permissions: map[model.PermissionDimension]model.PermissionLevel{ - model.DimensionConversationManage: model.PermissionLevelRead, - }, + Name: "TeamLead", + Permissions: []string{"conversation_participating_manage"}, }) require.NoError(t, err) @@ -164,10 +162,8 @@ func TestCustomRoleService_Update(t *testing.T) { account := createCRSvcTestAccount(t, db) created, err := svc.Create(context.Background(), account.ID, CreateCustomRoleRequest{ - Name: "Supervisor", - Permissions: map[model.PermissionDimension]model.PermissionLevel{ - model.DimensionConversationManage: model.PermissionLevelFull, - }, + Name: "Supervisor", + Permissions: []string{"conversation_manage"}, }) require.NoError(t, err) @@ -194,10 +190,8 @@ func TestCustomRoleService_Delete(t *testing.T) { account := createCRSvcTestAccount(t, db) created, err := svc.Create(context.Background(), account.ID, CreateCustomRoleRequest{ - Name: "Supervisor", - Permissions: map[model.PermissionDimension]model.PermissionLevel{ - model.DimensionConversationManage: model.PermissionLevelFull, - }, + Name: "Supervisor", + Permissions: []string{"conversation_manage"}, }) require.NoError(t, err) @@ -209,6 +203,28 @@ func TestCustomRoleService_Delete(t *testing.T) { assert.Error(t, err) } +func TestCustomRoleService_Delete_NullifiesAccountUsers(t *testing.T) { + svc, db := setupCustomRoleServiceTest(t) + account := createCRSvcTestAccount(t, db) + user := &model.User{AccountID: account.ID, Name: "Role User", Email: "role-user-svc@example.com", Password: "pw", Active: true} + require.NoError(t, db.Create(user).Error) + + created, err := svc.Create(context.Background(), account.ID, CreateCustomRoleRequest{ + Name: "Supervisor", + Permissions: []string{"conversation_manage"}, + }) + require.NoError(t, err) + require.NoError(t, db.Create(&model.AccountUser{AccountID: account.ID, UserID: user.ID, Role: "agent", CustomRoleID: created.ID}).Error) + + err = svc.Delete(context.Background(), created.ID, account.ID) + assert.NoError(t, err) + + var au model.AccountUser + require.NoError(t, db.Where("account_id = ? AND user_id = ?", account.ID, user.ID).First(&au).Error) + assert.Equal(t, uint(0), au.CustomRoleID) + assert.Equal(t, "agent", au.Role) +} + func TestCustomRoleService_Delete_NotFound(t *testing.T) { svc, _ := setupCustomRoleServiceTest(t) diff --git a/internal/service/profile_service.go b/internal/service/profile_service.go index a9bfd152..be57ad45 100644 --- a/internal/service/profile_service.go +++ b/internal/service/profile_service.go @@ -67,6 +67,8 @@ type ProfileAccountResponse struct { Availability string `json:"availability"` AvailabilityStatus string `json:"availability_status"` AutoOffline bool `json:"auto_offline"` + CustomRoleID *uint `json:"custom_role_id"` + CustomRole any `json:"custom_role"` } // UpdateProfileRequest is the DTO for updating user profile. @@ -405,6 +407,27 @@ func profileAccountResponse(accountUser model.AccountUser) ProfileAccountRespons activeAt := timeStringPtr(accountUser.ActiveAt) availability := defaultString(accountUser.Availability, "offline") status := defaultString(accountUser.Account.Status, "active") + permissions := []string{} + var customRole any + var customRoleID *uint + if accountUser.CustomRole != nil && accountUser.CustomRoleID > 0 { + id := accountUser.CustomRoleID + customRoleID = &id + keys, err := accountUser.CustomRole.GetPermissionKeys() + if err == nil { + permissions = make([]string, 0, len(keys)+1) + for _, key := range keys { + permissions = append(permissions, string(key)) + } + permissions = append(permissions, "custom_role") + } + customRole = map[string]any{ + "id": accountUser.CustomRole.ID, + "name": accountUser.CustomRole.Name, + "description": accountUser.CustomRole.Description, + "permissions": permissionsWithoutMarker(permissions), + } + } return ProfileAccountResponse{ ID: accountUser.AccountID, Name: accountUser.Account.Name, @@ -412,13 +435,26 @@ func profileAccountResponse(accountUser model.AccountUser) ProfileAccountRespons OnboardingStep: accountUser.Account.OnboardingStep, ActiveAt: activeAt, Role: accountUser.Role, - Permissions: []string{}, + Permissions: permissions, Availability: availability, AvailabilityStatus: availability, AutoOffline: accountUser.AutoOffline, + CustomRoleID: customRoleID, + CustomRole: customRole, } } +func permissionsWithoutMarker(permissions []string) []string { + keys := make([]string, 0, len(permissions)) + for _, permission := range permissions { + if permission == "custom_role" { + continue + } + keys = append(keys, permission) + } + return keys +} + func timeStringPtr(t *time.Time) *string { if t == nil { return nil diff --git a/internal/service/rbac_custom_role_test.go b/internal/service/rbac_custom_role_test.go new file mode 100644 index 00000000..2a08b9a5 --- /dev/null +++ b/internal/service/rbac_custom_role_test.go @@ -0,0 +1,76 @@ +package service + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/driver/sqlite" + "gorm.io/gorm" + "gorm.io/gorm/logger" + + "github.com/gochat/gochat/internal/auth" + "github.com/gochat/gochat/internal/model" +) + +func setupRBACCustomRoleTest(t *testing.T) (*RBACService, *gorm.DB) { + t.Helper() + db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)}) + require.NoError(t, err) + require.NoError(t, db.AutoMigrate(&model.Account{}, &model.User{}, &model.AccountUser{}, &model.CustomRole{})) + t.Cleanup(func() { + sqlDB, _ := db.DB() + sqlDB.Close() + }) + return NewRBACService(db), db +} + +func TestRBACService_CustomRoleUsesAgentRoleAndCustomRoleID(t *testing.T) { + svc, db := setupRBACCustomRoleTest(t) + account := &model.Account{Name: "RBAC Custom Role", Active: true} + require.NoError(t, db.Create(account).Error) + user := &model.User{AccountID: account.ID, Name: "Custom Agent", Email: "custom-agent@example.com", Password: "pw", Active: true} + require.NoError(t, db.Create(user).Error) + role := &model.CustomRole{AccountID: account.ID, Name: "Reporter"} + require.NoError(t, role.SetPermissionKeys([]model.PermissionDimension{model.DimensionReportManage})) + require.NoError(t, db.Create(role).Error) + + au, err := svc.AddAccountUser(user.ID, account.ID, "custom_role", role.ID, 0) + require.NoError(t, err) + assert.Equal(t, "agent", au.Role) + assert.Equal(t, role.ID, au.CustomRoleID) + + pc, err := svc.BuildPolicyContext(user.ID, account.ID) + require.NoError(t, err) + assert.True(t, pc.IsCustomRole()) + assert.True(t, pc.Can("read", "report")) + assert.True(t, pc.Can("manage", "report")) + assert.False(t, pc.Can("manage", "contact")) +} + +func TestRBACService_UpdateAccountUserRoleStoresAgentForCustomRole(t *testing.T) { + svc, db := setupRBACCustomRoleTest(t) + account := &model.Account{Name: "RBAC Update Role", Active: true} + require.NoError(t, db.Create(account).Error) + user := &model.User{AccountID: account.ID, Name: "Update Agent", Email: "update-agent@example.com", Password: "pw", Active: true} + require.NoError(t, db.Create(user).Error) + role := &model.CustomRole{AccountID: account.ID, Name: "Contact Manager"} + require.NoError(t, role.SetPermissionKeys([]model.PermissionDimension{model.DimensionContactManage})) + require.NoError(t, db.Create(role).Error) + require.NoError(t, db.Create(&model.AccountUser{AccountID: account.ID, UserID: user.ID, Role: "agent"}).Error) + + au, err := svc.UpdateAccountUserRole(user.ID, account.ID, "custom_role", role.ID) + require.NoError(t, err) + assert.Equal(t, "agent", au.Role) + assert.Equal(t, role.ID, au.CustomRoleID) +} + +func TestRBACService_CustomRoleConversationPermissionKeys(t *testing.T) { + pc := auth.NewPolicyContext(1, 1, "custom_role", 7, auth.PermissionMatrixMap{ + auth.DimensionConversationUnassignedManage: auth.PermissionFull, + }) + assert.True(t, pc.Can("read", "conversation")) + assert.True(t, pc.Can("manage", "conversation")) + assert.True(t, pc.Can("create", "message")) + assert.False(t, pc.Can("delete", "conversation")) +} diff --git a/internal/service/rbac_service.go b/internal/service/rbac_service.go index 683dbae2..018ce10d 100644 --- a/internal/service/rbac_service.go +++ b/internal/service/rbac_service.go @@ -84,7 +84,7 @@ func (s *RBACService) AddAccountUser(userID, accountID uint, role string, custom au := &model.AccountUser{ UserID: userID, AccountID: accountID, - Role: role, + Role: normalizeAccountUserRole(role, customRoleID), CustomRoleID: customRoleID, Availability: "offline", InvitedBy: invitedBy, @@ -108,7 +108,7 @@ func (s *RBACService) UpdateAccountUserRole(userID, accountID uint, newRole stri return nil, err } - au.Role = newRole + au.Role = normalizeAccountUserRole(newRole, customRoleID) au.CustomRoleID = customRoleID if err := s.db.Save(au).Error; err != nil { @@ -285,18 +285,31 @@ func (s *RBACService) BuildPolicyContext(userID, accountID uint) (*auth.PolicyCo permissions := auth.PermissionMatrixMap{} - // Load permissions based on role + // Load permissions based on role. Chatwoot keeps AccountUser.role as + // agent/administrator; custom-role behavior comes from custom_role_id. + effectiveRole := au.Role switch au.Role { case "administrator": permissions = auth.AdministratorPermissions case "agent": - permissions = auth.AgentDefaultPermissions - case "custom_role": if au.CustomRoleID > 0 { pm, err := s.GetCustomRolePermissionMatrix(au.CustomRoleID) if err != nil { // Fallback to agent defaults if custom role not found permissions = auth.AgentDefaultPermissions + } else { + permissions = pm + effectiveRole = "custom_role" + } + } else { + permissions = auth.AgentDefaultPermissions + } + case "custom_role": + effectiveRole = "custom_role" + if au.CustomRoleID > 0 { + pm, err := s.GetCustomRolePermissionMatrix(au.CustomRoleID) + if err != nil { + permissions = auth.AgentDefaultPermissions } else { permissions = pm } @@ -305,7 +318,7 @@ func (s *RBACService) BuildPolicyContext(userID, accountID uint) (*auth.PolicyCo } } - return auth.NewPolicyContext(userID, accountID, au.Role, au.CustomRoleID, permissions), nil + return auth.NewPolicyContext(userID, accountID, effectiveRole, au.CustomRoleID, permissions), nil } // CanPerform checks if a user can perform an action on a resource in an account. @@ -450,6 +463,13 @@ func isValidRole(role string) bool { return role == "agent" || role == "administrator" || role == "custom_role" } +func normalizeAccountUserRole(role string, customRoleID uint) string { + if role == "custom_role" || (role == "" && customRoleID > 0) { + return "agent" + } + return role +} + func isValidAvailability(avail string) bool { return avail == "online" || avail == "offline" || avail == "busy" } @@ -500,4 +520,4 @@ type AccountUserRole struct { Role string CustomRoleID uint Availability string -} \ No newline at end of file +} diff --git a/migrations/000001_init_schema.up.sql b/migrations/000001_init_schema.up.sql index ed6d5190..1317116f 100644 --- a/migrations/000001_init_schema.up.sql +++ b/migrations/000001_init_schema.up.sql @@ -84,7 +84,7 @@ CREATE TABLE IF NOT EXISTS custom_roles ( account_id INTEGER NOT NULL, name VARCHAR(255) NOT NULL, description TEXT, - permissions JSONB NOT NULL DEFAULT '{}', + permissions JSONB NOT NULL DEFAULT '[]', created_at TIMESTAMP WITH TIME ZONE DEFAULT NOW(), updated_at TIMESTAMP WITH TIME ZONE DEFAULT NOW(), deleted_at TIMESTAMP WITH TIME ZONE @@ -444,4 +444,4 @@ CREATE TABLE IF NOT EXISTS channel_whatsapps ( deleted_at TIMESTAMP WITH TIME ZONE ); -CREATE INDEX idx_channel_whatsapps_inbox_id ON channel_whatsapps(inbox_id) WHERE deleted_at IS NULL; \ No newline at end of file +CREATE INDEX idx_channel_whatsapps_inbox_id ON channel_whatsapps(inbox_id) WHERE deleted_at IS NULL; diff --git a/migrations/000023_align_custom_role_permissions.down.sql b/migrations/000023_align_custom_role_permissions.down.sql new file mode 100644 index 00000000..68195d8e --- /dev/null +++ b/migrations/000023_align_custom_role_permissions.down.sql @@ -0,0 +1 @@ +ALTER TABLE custom_roles ALTER COLUMN permissions SET DEFAULT '{}'::jsonb; diff --git a/migrations/000023_align_custom_role_permissions.up.sql b/migrations/000023_align_custom_role_permissions.up.sql new file mode 100644 index 00000000..207dfb36 --- /dev/null +++ b/migrations/000023_align_custom_role_permissions.up.sql @@ -0,0 +1,7 @@ +-- Align custom_roles.permissions with Chatwoot custom-role permission arrays. +-- Existing legacy JSON maps remain readable in Go code; new writes use arrays. +ALTER TABLE custom_roles ALTER COLUMN permissions SET DEFAULT '[]'::jsonb; + +UPDATE custom_roles +SET permissions = '[]'::jsonb +WHERE permissions IS NULL OR permissions = '{}'::jsonb;