Skip to content

feat: add Zammad ticket integration - #238

Merged
stefan-ernst merged 17 commits into
Windshiftapp:mainfrom
Optic00:codex/zammad-integration-pr
Sep 11, 2026
Merged

feat: add Zammad ticket integration#238
stefan-ernst merged 17 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.

@Optic00
Optic00 force-pushed the codex/zammad-integration-pr branch from e0c85bb to 94e2bff Compare September 1, 2026 23:15
@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

Thanks, the core concern was right: our owner test was not grounded in a real Zammad response.

I traced UsersController#search and the shared renderer, then verified the behavior in Zammad 7.1.1 with a restricted ticket.agent account. The relevant response branches are:

  • label or term: compact results
  • full=true: assets envelope
  • expand=true: array of expanded user records

The client now uses expand=true, keeps the permission and group-access filters, and validates active status and group access locally. The regression test uses an anonymized lab fixture reduced to the fields the client reads.

I also removed the runtime object-manager request, and ticket creation now sends the numeric group_id.

The branch is rebased on current main. Post-rebase internal Go tests and go vet pass; the full repository checks, including 53 frontend tests, lint, OpenAPI validation, and the build, passed immediately before the rebase. CI is now rerunning on the updated branch. Validation remains lab-only, not production-tested.

@github-actions

github-actions Bot commented Sep 3, 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.

@stefan-ernst

Copy link
Copy Markdown
Contributor

I am currently moving a lot of frontend and backend routes to the cleaner v2 api, I will merge this into this branch when all tests are green 🚀

The v2 item and workspace delete routes replaced the internal handlers that
translated the protected-link guard into HTTP 409. Without a mapping the guard
surfaced as a 500, so both error funnels now return the same conflict response
as the v1 handlers.
@Optic00
Optic00 force-pushed the codex/zammad-integration-pr branch from dea568c to fe48d21 Compare September 3, 2026 19:12
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

Merge conflicts have been resolved.

@github-actions github-actions Bot removed the conflict label Sep 3, 2026
@Optic00

Optic00 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main, including the v2 route migration - all checks are green and it is mergeable again. One thing the migration surfaced: the internal delete handlers that were removed carried the 409 mapping for protected integration links, so the v2 routes would have returned 500 for that case. Mapped in itemError and workspaceMutationError in the same branch.

Move PostgreSQL sequence cleanup after the workspace deletion commits so auxiliary DDL failures cannot roll back valid deletions. Remove the unused transactional helper and pin the historical Zammad migration checksums to prevent accidental edits.
@stefan-ernst

Copy link
Copy Markdown
Contributor

all good, one last observation: the polling interval can be a little bit more aggressive, for example every minute / 100 tickets or something, to make reconciliation a bit quicker

@stefan-ernst

Copy link
Copy Markdown
Contributor

Hey @Optic00 - 0.8.8 has finally shipped so I can merge this in now. Do you want to make the polling interval more aggressive?

@github-actions

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 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Yes, I've updated it to poll up to 100 due ticket links per run, with a one-minute wait after each run finishes. That keeps slow runs from overlapping and preserves the existing retry backoff. Thanks for the suggestion and for reviewing this, Stefan!

@github-actions github-actions Bot removed the conflict label Sep 11, 2026
@github-actions

Copy link
Copy Markdown

Merge conflicts have been resolved.

@stefan-ernst

Copy link
Copy Markdown
Contributor

Great, merging this now. Will release with 0.8.9 probably next week, as there not many other workitems scheduled

@stefan-ernst
stefan-ernst merged commit 3f9326b into Windshiftapp:main Sep 11, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants