Skip to content

feat: add Zammad ticket integration - #238

Open
Optic00 wants to merge 10 commits into
Windshiftapp:mainfrom
Optic00:codex/zammad-integration-pr
Open

feat: add Zammad ticket integration#238
Optic00 wants to merge 10 commits into
Windshiftapp:mainfrom
Optic00:codex/zammad-integration-pr

Conversation

@Optic00

@Optic00 Optic00 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add administrator-managed Zammad connections using encrypted API tokens or the native OAuth flow.
  • Reuse Windshift's integration-provider, action-credential, workspace-scope, OAuth, and item-link infrastructure. Zammad-specific ticket policy and synchronization remain typed provider data.
  • Create new tickets or link existing ones from Windshift items, with durable mappings, group scoping, correlation-based duplicate protection, polling, manual refresh, owner/group/state updates, and optional closed-state workflow mapping.
  • Persist the administrator-approved Zammad group catalog as {id, name} pairs. Runtime operations verify live ticket and group IDs without calling Zammad's admin-only groups endpoint.
  • Add permission-checked return links from Zammad to the current Windshift item, plus setup and operating documentation.

Safety and recovery

  • Harden outbound requests with the existing public-address SSRF policy, redirect checks, bounded responses, and sanitized errors.
  • Preserve existing Zammad configuration during upgrade and backfill canonical workspace scopes only for managed Zammad credentials.
  • Guard item, workspace, and user lifecycle operations through provider-neutral integration-link checks.
  • Provide an audited, administrator-only local detach path for recovery when a remote Zammad ticket is unavailable.

Scope

This implements the core integration proposed in #234. Comment and attachment synchronization, arbitrary field synchronization, webhooks, and the optional workspace dashboard remain out of scope.

Validation

  • go test ./...
  • Frontend: npm run check, npm run typecheck, npm run test:run, npm run build
  • Container build with the repository-pinned Node and Go toolchains
  • Upgrade and manual end-to-end use in an isolated Zammad/Windshift lab: connection test, single-ticket refresh, system-wide refresh, and links in both directions

The integration has only been tested in the lab, not in production. The existing OAuth connection remained operational after the upgrade, but OAuth authorization itself was not repeated in the final pass. Zammad is not yet part of the automated end-to-end suite.

@stefan-ernst

Copy link
Copy Markdown
Contributor

Thank you!

One issue:

The setup guide says to grant only ticket.agent with
read/create/change group access and explicitly says not to grant
group administration: zammad-integration.md
(

## Zammad service account and permissions
).

However, virtually every operation calls Client.Groups(), which
requests GET /api/v1/groups: client.go
(

func (c *Client) Metadata(ctx context.Context) (*models.ZammadConnectionMetadata, error) {
).
Zammad officially documents that endpoint as requiring admin.group:
Zammad Group API
(https://docs.zammad.org/en/latest/api/group.html).

Generally: Since we are mapping an entire zammad admin credential to the windshift integration, that means the integration is a bit unbalanced from a scaling perspective. I believe it will work for small teams, so we can merge it with this architecture with one caveat: We should extend the existing integration system to support this instead of building a parallel one

@Optic00

Optic00 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, you are right on both points. The /groups dependency is a real authorization bug, not a documentation issue. It is currently reached by connection testing, ticket creation, linking, updates and every polling cycle, while Zammad requires admin.group. I do not want to solve that by broadening the credential permissions.

I propose removing /api/v1/groups from all runtime paths. Configured groups will be stored as {id, name}, validated against /api/v1/users/me and its group_ids, and ticket synchronization will validate the ticket's own group_id. I will verify group_id support during ticket creation and /users/search access with the restricted service account in the lab.

I also agree with extending the existing integration system. The PR already reuses integration_providers, action_credentials and item_integration_links, but connection ownership, OAuth, workspace scoping and core link guards still diverge.

My proposed boundary is a common provider/connection layer supporting both user-owned and system-owned credentials, API tokens and OAuth, workspace scopes and provider capabilities. Zammad-specific ticket policy and synchronization remain a typed provider sidecar. I would also replace the direct Zammad dependencies in deletion, move and offboarding paths with a generic link policy.

Since none of this schema has been released yet, I would prefer correcting that structure in this PR instead of shipping parallel lifecycle tables. Does that match what you had in mind?

@Optic00
Optic00 marked this pull request as draft August 31, 2026 21:00
@Optic00
Optic00 marked this pull request as ready for review September 1, 2026 14:57
@Optic00
Optic00 force-pushed the codex/zammad-integration-pr branch from 2aeb692 to eb1410f Compare September 1, 2026 14:57
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

This PR has merge conflicts that need to be resolved before it can be merged. Please rebase on the latest main branch.

@Optic00
Optic00 force-pushed the codex/zammad-integration-pr branch from eb1410f to 0d7a68b Compare September 1, 2026 15:28
@github-actions github-actions Bot removed the conflict label Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

Merge conflicts have been resolved.

@Optic00

Optic00 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Quick update: both review points are addressed.

The runtime dependency on /api/v1/groups is gone. Connections now store approved groups as ID/name pairs, and ticket operations enforce that policy.

Connection ownership and lifecycle reuse the existing integration providers, managed action credentials, workspace scopes, OAuth, and item integration links. Item deletion, workspace moves and deletion, and personal-workspace offboarding all go through generic, provider-neutral link guards.

The branch sits directly on current main, all 22 checks pass, and the PR body lists the remaining lab-only validation limits.

Ready for another look. Thanks for the direction.

@stefan-ernst

Copy link
Copy Markdown
Contributor

Hey @Optic00 thanks for putting so much work into this!

one thing came out of my review:

Owner lookup decodes the wrong Zammad response shape — client.go (

func (c *Client) Owners(ctx context.Context, groupID int) ([]Owner, error) {
const pageSize = 100
owners := make([]Owner, 0)
for page := 1; ; page++ {
query := url.Values{}
query.Set("query", "*")
query.Add("permissions[]", "ticket.agent")
query.Set(fmt.Sprintf("group_ids[%d]", groupID), "full")
query.Set("page", strconv.Itoa(page))
query.Set("per_page", strconv.Itoa(pageSize))
var rawUsers []map[string]json.RawMessage
if err := c.getJSON(ctx, "/api/v1/users/search?"+query.Encode(), &rawUsers); err != nil {
return nil, err
}
for _, raw := range rawUsers {
var id int
var active bool
var first, last, login string
if json.Unmarshal(raw["id"], &id) != nil || id <= 0 {
continue
}
if value, ok := raw["active"]; ok {
if err := json.Unmarshal(value, &active); err != nil || !active {
continue
}
}
var groups map[string][]string
groupValue, ok := raw["group_ids"]
if !ok || json.Unmarshal(groupValue, &groups) != nil || !containsAccess(groups[strconv.Itoa(groupID)], "full") {
continue
)

 /api/v1/users/search defaults to compact {id,label,value} results; full=true instead returns {user_ids, assets}. The implementation expects an array of complete users containing active, group_ids, names, and login, so every real user is discarded at
 lines 265–268. Consequently, owner selection exposes only “Unassigned,” and assigning an owner fails validation.

 The regression test mocks a response Zammad does not provide (https://github.com/Windshiftapp/core/blob/e0c85bb1771fe0bc1b326471c2c770b85fade82e/internal/integrations/zammad/client_test.go#L149-L177). Align the client and test with the official
 Zammad users-search contract (https://github.com/zammad/zammad/blob/develop/app/controllers/users_controller.rb).

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

This PR has merge conflicts that need to be resolved before it can be merged. Please rebase on the latest main branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants