Add support for roles by Echoloot · Pull Request #167 · zammad/zammad-api-client-php

Echoloot · 2026-09-30T12:37:57Z

Wanted to add a user to Zammad with certain role_ids but couldn't get the defined roles.

This adds support for /api/v1/roles and presents them in $client->role()

Summary by CodeRabbit

New Features
- Added support for retrieving and managing roles through the API, including role names, notes, active status, permissions, and group access.
- Added role lookup for assigning roles to users.

$client->role() If you create a user you can specify role_ids but there's no way to get those ids.

coderabbitai · 2026-09-30T12:38:10Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 02ee4818-fdaf-49cb-940b-628c9a21486f

📥 Commits

Reviewing files that changed from the base of the PR and between 4fd4280 and 2daad26.

📒 Files selected for processing (9)

CHANGELOG.md
README.md
docs/migration-v3-examples.md
src/Endpoints/Roles/RoleDTO.php
src/Endpoints/Roles/RoleRepository.php
test/Integration/RoleIntegrationTest.php
test/Unit/DTOs/DTOTest.php
test/Unit/Repositories/RoleRepositoryTest.php
test/Unit/RepositoryAccessorsTest.php

🚧 Files skipped from review as they are similar to previous changes (1)

src/Endpoints/Roles/RoleRepository.php

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

The change adds RoleDTO and RoleRepository for the roles endpoint. It registers the repository with the roles API path and RoleDTO, then adds a typed role() accessor that resolves the repository. It also adds unit and integration tests and documents role fields, permissions, access requirements, and deletion behavior.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 2daad

No actionable issue is established; the role support is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 2daad

Role operations reuse the existing authenticated connection rather than granting additional privileges locally. No authorization bypass was established. Remaining uncertainty concerns server-enforced permissions, recovery after interrupted mutations, and the complete comparison with behavior before this PR.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — Authorized role mutations can affect permission grants, group access, and signup defaults on the configured Zammad instance, with downstream effects on users receiving those roles. Effective scope depends on the credential and server policy; an additional tenant or asset restriction was not established.

Trust Boundaries and Controls

observed — Caller-supplied role payloads cross into Zammad through the same handler used by other client repositories. The standard factory configures the authorization header and disables redirects. Role payload fields do not themselves select credentials; mutation authority remains delegated to the remote API.

Resilience and Maintainability Implications

observed — The integration user test resolves Customer by name and deletes the returned user in finally. Creation occurs before that block, leaving no inspected recovery for an unknown-result POST or failed deletion. This lifecycle exists in the immediate parent; its PR-wide introduction and the configured Customer role's effective privileges remain unresolved.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 65.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 8 files. (3 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adding role support, including the roles endpoint, DTO, repository, accessor, documentation, and tests.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 65.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 8 files. (3 skipped: 3 unsupported.)

Fix all pre-merge checks with AI

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

@extends

Covers the roles support added in PR zammad#167 with unit and integration tests and documents the endpoint: - RoleRepositoryTest: all(), find(), create(), patch(), search() and the BadMethodCallException from the unsupported delete() - RoleDTO added to the shared DTO provider, role accessor to the repository accessor provider - RoleIntegrationTest: listing, default roles, find(), pagination and the motivating use case (resolve a role name to the id used in role_ids) - Restore the house-style docblocks on RoleDTO/RoleRepository, including the @extends annotation phpstan needs - README: role accessor, Roles section, RoleDTO reference, delete table entry, admin permission note for integration tests; CHANGELOG and migration examples Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Verified against the Zammad server source instead of assuming: - No DELETE route exists for roles (config/routes/role.rb maps index, search, show, create and update only), so the non-deletable documentation holds. - /api/v1/roles/search does exist and supports with_total_count, so totalCount() works — covered by an integration test. - index and show are permitted to ticket.agent, admin.role and ticket.customer, not admin-only as documented before. A customer sees a reduced role whose name Zammad masks as "Role_<id>" (Role::Assets#filter_unauthorized_attributes), so name resolution needs an agent or admin token; search, create and patch need admin.role. - permission_ids and group_ids are applied on create and update via associations_from_param, and group_ids is a group-ID-to-access map rather than a flat list. RoleDTO therefore gains default_at_signup, permission_ids and group_ids, with unit tests for hydration of the map shape, the reduced customer view and payload construction, plus the corrected README and docblocks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

derpixler · 2026-10-02T14:40:54Z

Thanks a lot for this — really useful addition. role_ids on user creation was a dead end without a way to look the IDs up.

I've added unit and integration tests plus the docs on top of your commits, so this is good to go from my side.

Thanks again!

@extends

Covers the roles support added in PR #167 with unit and integration tests and documents the endpoint: - RoleRepositoryTest: all(), find(), create(), patch(), search() and the BadMethodCallException from the unsupported delete() - RoleDTO added to the shared DTO provider, role accessor to the repository accessor provider - RoleIntegrationTest: listing, default roles, find(), pagination and the motivating use case (resolve a role name to the id used in role_ids) - Restore the house-style docblocks on RoleDTO/RoleRepository, including the @extends annotation phpstan needs - README: role accessor, Roles section, RoleDTO reference, delete table entry, admin permission note for integration tests; CHANGELOG and migration examples Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Echoloot added 2 commits September 30, 2026 14:20

Enable access to roles (Endpoint roles)

30f4cd3

$client->role() If you create a user you can specify role_ids but there's no way to get those ids.

reduced comments | was copy & paste from TicketPriorities

4fd4280

derpixler and others added 2 commits October 2, 2026 16:27

derpixler merged commit 0c70fa8 into zammad:master Oct 2, 2026
4 checks passed

Navigation Menu

Add support for roles - #167

Add support for roles#167
derpixler merged 4 commits into
zammad:masterfrom
Echoloot:master

Echoloot commented Sep 30, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 30, 2026 •

edited

Loading

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

derpixler commented Oct 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Echoloot commented Sep 30, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Security Architecture Review

❌ Failed checks (1 warning)

Uh oh!

derpixler commented Oct 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Echoloot commented Sep 30, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 30, 2026 •

edited

Loading