diff --git a/core.ref b/core.ref index 0cb0abe..86850e2 100644 --- a/core.ref +++ b/core.ref @@ -1 +1 @@ -ca5674382cb750b9d226b70067496f548f4937a0 +ea4ea1d83f8c5f61da91bc7c72c8cde0018cd569 diff --git a/frontend/src/lib/api/core.test.js b/frontend/src/lib/api/core.test.js index 14bfb4e..c307c26 100644 --- a/frontend/src/lib/api/core.test.js +++ b/frontend/src/lib/api/core.test.js @@ -137,7 +137,11 @@ describe('fetchAPI — error mapping', () => { status: 400, statusText: 'Bad Request', body: JSON.stringify({ - error: { code: 'invalid_request', message: 'page is out of range', details: { field: 'page' } }, + error: { + code: 'invalid_request', + message: 'page is out of range', + details: { field: 'page' }, + }, request_id: 'req-v2', }), }) @@ -467,7 +471,7 @@ describe('fetchAPI — network and timeout errors', () => { await expect(request).rejects.toMatchObject({ name: 'AbortError' }); }); - test('treats a hidden-document TypeError as navigation cancellation', async () => { + test('preserves network errors in a hidden live document', async () => { const originalVisibilityState = document.visibilityState; Object.defineProperty(document, 'visibilityState', { configurable: true, @@ -476,7 +480,10 @@ describe('fetchAPI — network and timeout errors', () => { global.fetch = vi.fn(() => Promise.reject(new TypeError('Failed to fetch'))); try { - await expect(fetchAPI('/items')).rejects.toMatchObject({ name: 'AbortError' }); + await expect(fetchAPI('/items')).rejects.toMatchObject({ + code: 'NETWORK_ERROR', + status: 0, + }); } finally { Object.defineProperty(document, 'visibilityState', { configurable: true, @@ -555,9 +562,7 @@ describe('fetchAPI — administration UI refresh signaling', () => { window.history.replaceState({}, '', '/'); } - expect(events).toEqual([ - { endpoint: '/admin/llm-connections', method: 'POST' }, - ]); + expect(events).toEqual([{ endpoint: '/admin/llm-connections', method: 'POST' }]); }); test('supports admin routes below a configured context path', async () => { diff --git a/frontend/src/lib/api/zammadObservability.test.js b/frontend/src/lib/api/zammadObservability.test.js new file mode 100644 index 0000000..afd2466 --- /dev/null +++ b/frontend/src/lib/api/zammadObservability.test.js @@ -0,0 +1,21 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { zammadTickets } from './integrations.js'; + +afterEach(() => vi.unstubAllGlobals()); + +describe('Zammad observation API', () => { + it.each([ + ['item timeline', () => zammadTickets.history(99, { limit: 6 }), '/api/items/99/zammad-history?limit=6'], + ['workspace overview', () => zammadTickets.workspaceOverview(23, { limit: 5 }), '/api/workspaces/23/zammad-overview?limit=5'], + ])('loads the %s endpoint', async (_label, request, path) => { + const response = { recent_changes: [] }; + const fetchMock = vi.fn().mockResolvedValue(new Response(JSON.stringify(response), { + status: 200, headers: { 'content-type': 'application/json' }, + })); + vi.stubGlobal('fetch', fetchMock); + + await expect(request()).resolves.toEqual(response); + expect(fetchMock).toHaveBeenCalledOnce(); + expect(fetchMock.mock.calls[0][0]).toBe(path); + }); +}); diff --git a/frontend/src/lib/composables/useItemEventStream.test.js b/frontend/src/lib/composables/useItemEventStream.test.js index 82b24af..3cd6ffd 100644 --- a/frontend/src/lib/composables/useItemEventStream.test.js +++ b/frontend/src/lib/composables/useItemEventStream.test.js @@ -17,10 +17,10 @@ describe('normalizeItemEventStreamID', () => { }); describe('createConnectionReconcileTracker', () => { - it('does not reconcile the initial healthy connection', () => { + it('reconciles the initial connection to cover changes before subscription', () => { const tracker = createConnectionReconcileTracker(); - expect(tracker.markConnected()).toBe(false); + expect(tracker.markConnected()).toBe(true); }); it('reconciles after a connected stream disconnects', () => { diff --git a/frontend/src/lib/features/items/ZammadItemPanel.test.js b/frontend/src/lib/features/items/ZammadItemPanel.test.js new file mode 100644 index 0000000..ab6fec7 --- /dev/null +++ b/frontend/src/lib/features/items/ZammadItemPanel.test.js @@ -0,0 +1,156 @@ +/** @vitest-environment jsdom */ + +import '@testing-library/jest-dom/vitest'; +import { cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/svelte'; +import { tick } from 'svelte'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import ZammadItemPanel from './ZammadItemPanel.svelte'; + +const mocks = vi.hoisted(() => ({ + connections: vi.fn(), + links: vi.fn(), + history: vi.fn(), + refresh: vi.fn(), + successToast: vi.fn(), + errorToast: vi.fn(), +})); +vi.mock('../../api.js', () => ({ + api: { + zammadConnections: { forWorkspace: mocks.connections }, + zammadTickets: { forItem: mocks.links, history: mocks.history, refresh: mocks.refresh }, + }, +})); +vi.mock('../../stores', () => ({ authStore: { currentUser: { timezone: 'UTC' } } })); +vi.mock('../../stores/toasts.svelte.js', () => ({ + successToast: mocks.successToast, + errorToast: mocks.errorToast, +})); +vi.mock('../../stores/i18n.svelte.js', () => ({ + t: (key, params = {}) => { + if (key === 'zammad.ticketNumber') return `Zammad #${params.number}`; + if (key === 'zammad.overview.statusIdFallback') return `Status ID ${params.id}`; + if (key === 'zammad.timeline.change') return `${params.field}: ${params.from} → ${params.to}`; + return key; + }, +})); + +const ticket = (overrides = {}) => ({ + id: 'link-1', + connection_id: 'dev', + connection_name: 'Helpdesk', + ticket_id: 42, + ticket_number: '123', + ticket_title: 'VPN access', + ticket_url: 'https://zammad.example.test/#ticket/zoom/42', + group_name: 'Network', + owner_name: 'Ada', + sync_state: 'linked', + last_status_id: 4, + last_status_name: 'closed', + closed: true, + ...overrides, +}); + +beforeEach(() => { + vi.clearAllMocks(); + mocks.connections.mockResolvedValue([ + { id: 'dev', name: 'Helpdesk', auth_method: 'api_token', ready: true }, + ]); + mocks.links.mockResolvedValue([ticket()]); + mocks.history.mockResolvedValue({ events: [] }); + mocks.refresh.mockResolvedValue(ticket()); +}); +afterEach(cleanup); + +describe('Zammad item panel observations', () => { + it('keeps the current sidebar actions while showing ticket title, status and observed changes', async () => { + mocks.history.mockResolvedValue({ + events: [ + { + id: 'change-1', + field: 'owner', + ticket_number: '123', + old_value: { id: 1, name: 'Unassigned' }, + new_value: { id: 9, name: 'Ada' }, + observed_at: '2026-09-30T10:00:00Z', + }, + { id: 'ignored', field: 'priority' }, + ], + }); + render(ZammadItemPanel, { itemId: 5, workspaceId: 7, canEdit: true }); + + const card = await screen.findByTestId('zammad-ticket-card-link-1'); + expect(within(card).getByRole('link', { name: 'VPN access' })).toHaveAttribute( + 'href', + 'https://zammad.example.test/#ticket/zoom/42' + ); + expect(within(card).getByText('closed')).toBeInTheDocument(); + expect(within(card).getByRole('button', { name: 'zammad.refreshTicket' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'zammad.createTicket' })).toBeInTheDocument(); + await fireEvent.click(screen.getByRole('button', { name: 'zammad.timeline.toggle' })); + expect( + await screen.findByText('zammad.timeline.field.owner: zammad.unassignedOwner → Ada') + ).toBeInTheDocument(); + expect(mocks.history).toHaveBeenCalledWith(5, { limit: 6 }); + expect(screen.queryByText('priority')).not.toBeInTheDocument(); + }); + + it('reloads hidden history after a link change invalidates an in-flight response', async () => { + let resolveOldHistory; + const oldHistory = new Promise((resolve) => { + resolveOldHistory = resolve; + }); + mocks.history.mockReturnValueOnce(oldHistory).mockResolvedValueOnce({ + events: [ + { + id: 'fresh-change', + field: 'status', + ticket_number: '123', + old_value: { id: 2, name: 'open' }, + new_value: { id: 4, name: 'freshly closed' }, + observed_at: '2026-10-04T10:00:00Z', + }, + ], + }); + render(ZammadItemPanel, { itemId: 5, workspaceId: 7 }); + await screen.findByTestId('zammad-ticket-card-link-1'); + await fireEvent.click(screen.getByRole('button', { name: 'zammad.timeline.toggle' })); + expect(mocks.history).toHaveBeenCalledTimes(1); + await fireEvent.click(screen.getByRole('button', { name: 'zammad.timeline.toggle' })); + window.dispatchEvent(new CustomEvent('item-zammad-links-changed', { detail: { itemId: 5 } })); + await waitFor(() => expect(mocks.links).toHaveBeenCalledTimes(2)); + await screen.findByTestId('zammad-ticket-card-link-1'); + resolveOldHistory({ events: [] }); + await oldHistory; + await tick(); + await fireEvent.click(screen.getByRole('button', { name: 'zammad.timeline.toggle' })); + await waitFor(() => expect(mocks.history).toHaveBeenCalledTimes(2)); + expect( + await screen.findByText('zammad.timeline.field.status: open → freshly closed') + ).toBeInTheDocument(); + }); + + it('shows a nameless observed status by ID', async () => { + mocks.links.mockResolvedValue([ + ticket({ last_status_id: 9, last_status_name: '', closed: false }), + ]); + render(ZammadItemPanel, { itemId: 5, workspaceId: 7 }); + const card = await screen.findByTestId('zammad-ticket-card-link-1'); + expect(within(card).getByText('Status ID 9')).toBeInTheDocument(); + }); + + it('retains the sidebar card if reloading after a successful refresh fails', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + mocks.links.mockResolvedValueOnce([ticket()]).mockRejectedValueOnce(new Error('reload failed')); + render(ZammadItemPanel, { itemId: 5, workspaceId: 7, canEdit: true }); + const card = await screen.findByTestId('zammad-ticket-card-link-1'); + await fireEvent.click(within(card).getByRole('button', { name: 'zammad.refreshTicket' })); + await waitFor(() => + expect(mocks.errorToast).toHaveBeenCalledWith('zammad.ticketReloadAfterChangeFailed') + ); + expect(screen.getByTestId('zammad-ticket-card-link-1')).toBeInTheDocument(); + expect(screen.queryByText('zammad.loadLinksFailed')).not.toBeInTheDocument(); + expect(mocks.successToast).not.toHaveBeenCalled(); + consoleError.mockRestore(); + }); +}); diff --git a/frontend/src/lib/features/support/CannedResponsePicker.test.js b/frontend/src/lib/features/support/CannedResponsePicker.test.js index 216b4e3..96dc125 100644 --- a/frontend/src/lib/features/support/CannedResponsePicker.test.js +++ b/frontend/src/lib/features/support/CannedResponsePicker.test.js @@ -78,11 +78,37 @@ describe("CannedResponsePicker", () => { }); }); - it("loads nothing and shows no options without a workspace", async () => { + it("loads nothing and renders no picker without a workspace", async () => { render(CannedResponsePicker, { props: { workspaceId: null, isOpen: true, onSelect: vi.fn() } }); await waitFor(() => { expect(mocks.getAll).not.toHaveBeenCalled(); }); - expect(screen.getByText("common.loading")).toBeTruthy(); + expect(screen.queryByTestId("canned-response-picker")).toBeNull(); + }); + + it("renders no picker when the workspace has no canned responses", async () => { + mocks.getAll.mockResolvedValue([]); + render(CannedResponsePicker, { props: { workspaceId: 5, isOpen: true, onSelect: vi.fn() } }); + + await waitFor(() => { + expect(mocks.getAll).toHaveBeenCalledWith(5); + }); + expect(screen.queryByTestId("canned-response-picker")).toBeNull(); + expect(screen.queryByTestId("canned-response-search")).toBeNull(); + }); + + it("does not render the picker until responses load", async () => { + let resolveLoad; + mocks.getAll.mockReturnValue( + new Promise((resolve) => { + resolveLoad = resolve; + }) + ); + render(CannedResponsePicker, { props: { workspaceId: 5, isOpen: true, onSelect: vi.fn() } }); + + expect(screen.queryByTestId("canned-response-picker")).toBeNull(); + + resolveLoad(responses); + expect(await screen.findByTestId("canned-response-picker")).toBeTruthy(); }); }); diff --git a/frontend/src/lib/services/authenticatedShellUI.test.js b/frontend/src/lib/services/authenticatedShellUI.test.js index b6c2a31..96a4afb 100644 --- a/frontend/src/lib/services/authenticatedShellUI.test.js +++ b/frontend/src/lib/services/authenticatedShellUI.test.js @@ -14,7 +14,7 @@ vi.mock('../stores/aiStore.svelte.js', () => ({ aiStore: { hydrate: vi.fn() }, })); vi.mock('../stores/capabilities.svelte.js', () => ({ - capabilitiesStore: { hydrate: vi.fn() }, + capabilitiesStore: { hydrate: vi.fn(), failHydration: vi.fn() }, })); vi.mock('../stores/logbook.svelte.js', () => ({ logbookStore: { hydrateAvailability: vi.fn() }, @@ -30,6 +30,7 @@ vi.mock('../stores/permissions.svelte.js', () => ({ setLogbookAvailable: vi.fn(), setHasAssetSets: vi.fn(), setHasActivePortals: vi.fn(), + setHasPortals: vi.fn(), setManagesChannels: vi.fn(), }, })); @@ -56,7 +57,9 @@ import { workspaceDataStore } from '../stores/workspaceDataStore.svelte.js'; import { workspacesStore } from '../stores/workspaces.svelte.js'; import { hydrateAuthenticatedShellUI, + loadAuthenticatedShellUI, refreshAuthenticatedShellUI, + resetAuthenticatedShellUILoad, } from './authenticatedShellUI.js'; const snapshot = { @@ -68,6 +71,7 @@ const snapshot = { logbook_available: true, }, has_asset_sets: true, + has_portals: true, has_active_portals: false, manages_channels: true, work_item_staleness: { stale_after_days: 30 }, @@ -93,6 +97,7 @@ describe('authenticated shell UI refresh', () => { expect(workItemStalenessSettings.hydrate).toHaveBeenCalledWith(snapshot.work_item_staleness); expect(permissionStore.setLogbookAvailable).toHaveBeenCalledWith(true); expect(permissionStore.setHasAssetSets).toHaveBeenCalledWith(true); + expect(permissionStore.setHasPortals).toHaveBeenCalledWith(true); expect(permissionStore.setHasActivePortals).toHaveBeenCalledWith(false); expect(permissionStore.setManagesChannels).toHaveBeenCalledWith(true); }); @@ -108,3 +113,81 @@ describe('authenticated shell UI refresh', () => { expect(themeStore.setActiveTheme).toHaveBeenCalledWith({ id: 9, name: 'Night' }); }); }); + +describe('authenticated shell UI loading deadline (WI-1611)', () => { + beforeEach(() => { + resetAuthenticatedShellUILoad(); + }); + + test('threads the caller deadline into the shell bootstrap request', async () => { + await loadAuthenticatedShellUI(7, { timeout: 1234 }); + + expect(api.shellBootstrap.get).toHaveBeenCalledWith({ timeout: 1234 }); + }); + + test('hydrates when the bootstrap resolves before the deadline', async () => { + let resolveBootstrap; + api.shellBootstrap.get.mockImplementation( + () => + new Promise((resolve) => { + resolveBootstrap = resolve; + }) + ); + + const request = loadAuthenticatedShellUI(7, { timeout: 50 }); + resolveBootstrap(snapshot); + + await expect(request).resolves.toBe(true); + expect(capabilitiesStore.hydrate).toHaveBeenCalledWith(snapshot.features); + }); + + test('reports failure and marks capabilities failed when the request rejects', async () => { + api.shellBootstrap.get.mockRejectedValue( + Object.assign(new Error('The server took too long to respond.'), { + code: 'REQUEST_TIMEOUT', + }) + ); + + await expect(loadAuthenticatedShellUI(7, { timeout: 50 })).resolves.toBe(false); + expect(capabilitiesStore.failHydration).toHaveBeenCalledTimes(1); + }); + + test('a forced retry after a failure re-requests and can hydrate', async () => { + api.shellBootstrap.get.mockRejectedValueOnce(new Error('offline')); + await expect(loadAuthenticatedShellUI(7, { timeout: 50 })).resolves.toBe(false); + + api.shellBootstrap.get.mockResolvedValueOnce(snapshot); + await expect(loadAuthenticatedShellUI(7, { force: true, timeout: 50 })).resolves.toBe(true); + + expect(api.shellBootstrap.get).toHaveBeenCalledTimes(2); + expect(capabilitiesStore.hydrate).toHaveBeenCalledWith(snapshot.features); + }); + + test('a non-forced call after a settled failure does not re-request', async () => { + api.shellBootstrap.get.mockRejectedValueOnce(new Error('offline')); + await loadAuthenticatedShellUI(7, { timeout: 50 }); + + await expect(loadAuthenticatedShellUI(7, { timeout: 50 })).resolves.toBe(false); + expect(api.shellBootstrap.get).toHaveBeenCalledTimes(1); + }); + + test('an account switch discards the previous audience result', async () => { + let resolveFirst; + api.shellBootstrap.get + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve; + }) + ) + .mockResolvedValueOnce(snapshot); + + const first = loadAuthenticatedShellUI(1, { timeout: 50 }); + const second = loadAuthenticatedShellUI(2, { timeout: 50 }); + await expect(second).resolves.toBe(true); + + resolveFirst(snapshot); + await expect(first).resolves.toBe(false); + expect(capabilitiesStore.hydrate).toHaveBeenCalledTimes(1); + }); +}); diff --git a/frontend/src/lib/stores/permissions.test.js b/frontend/src/lib/stores/permissions.test.js index 71fa348..2163b73 100644 --- a/frontend/src/lib/stores/permissions.test.js +++ b/frontend/src/lib/stores/permissions.test.js @@ -40,6 +40,7 @@ beforeEach(() => { authStore.set({ currentUser: null }); permissionStore.clear(); permissionStore.setHasAssetSets(false); + permissionStore.setHasPortals(false); permissionStore.setHasActivePortals(false); permissionStore.setManagesChannels(false); permissionStore.setLogbookAvailable(false); @@ -90,32 +91,39 @@ describe('canAccessAdmin', () => { describe('canAccessCustomers', () => { test('false when no user', () => { - permissionStore.setHasActivePortals(true); + permissionStore.setHasPortals(true); expect(get(permissionStore).canAccessCustomers).toBe(false); }); - test('false when no active portals (even for system admin)', () => { + test('false when no portal exists at all (even for system admin)', () => { authStore.set({ currentUser: { id: 1, is_system_admin: true } }); - permissionStore.setHasActivePortals(false); + permissionStore.setHasPortals(false); expect(get(permissionStore).canAccessCustomers).toBe(false); }); - test('true for system admin when portals are active', () => { + test('true for system admin when a portal exists', () => { authStore.set({ currentUser: { id: 1, is_system_admin: true } }); - permissionStore.setHasActivePortals(true); + permissionStore.setHasPortals(true); + expect(get(permissionStore).canAccessCustomers).toBe(true); + }); + + test('true for system admin when every portal is disabled but one exists', () => { + authStore.set({ currentUser: { id: 1, is_system_admin: true } }); + permissionStore.setHasPortals(true); + permissionStore.setHasActivePortals(false); expect(get(permissionStore).canAccessCustomers).toBe(true); }); - test('false for non-admin without customers.manage even with portals active', async () => { + test('false for non-admin without customers.manage even when a portal exists', async () => { authStore.set({ currentUser: { id: 1, is_system_admin: false } }); - permissionStore.setHasActivePortals(true); + permissionStore.setHasPortals(true); // No permission keys loaded. expect(get(permissionStore).canAccessCustomers).toBe(false); }); - test('true for non-admin with customers.manage when portals are active', async () => { + test('true for non-admin with customers.manage when a portal exists', async () => { authStore.set({ currentUser: { id: 1, is_system_admin: false } }); - permissionStore.setHasActivePortals(true); + permissionStore.setHasPortals(true); api.permissions.getUserPermissions.mockResolvedValueOnce({ global_permissions: ['customers.manage'], }); diff --git a/frontend/src/lib/utils/zammadObservations.test.js b/frontend/src/lib/utils/zammadObservations.test.js new file mode 100644 index 0000000..0b4ced7 --- /dev/null +++ b/frontend/src/lib/utils/zammadObservations.test.js @@ -0,0 +1,40 @@ +import { describe, expect, it } from 'vitest'; +import { + getZammadObservedValueLabel, + getZammadStatusAppearance, + getZammadStatusBucketDisplayLabel, + getZammadStatusBucketLabel, + isCurrentZammadWorkspaceOverviewRequest, +} from './zammadObservations.js'; + +const translate = (key, params = {}) => `${key}:${params.id ?? ''}`; + +describe('Zammad observation formatting', () => { + it('distinguishes missing names and IDs from the initial value', () => { + expect(getZammadObservedValueLabel({ id: 17, name: ' ' }, translate)).toBe('zammad.timeline.valueIdFallback:17'); + expect(getZammadObservedValueLabel({ id: 0, name: '' }, translate)).toBe('zammad.timeline.initialValue:'); + expect(getZammadStatusBucketLabel({ id: 9, name: '' }, translate)).toBe('zammad.overview.statusIdFallback:9'); + expect(getZammadStatusBucketLabel({ id: null, name: '' }, translate)).toBe('zammad.overview.unknownStatusBucket:'); + }); + + it('localizes only the explicit unassigned owner sentinel in historical changes', () => { + expect(getZammadObservedValueLabel({ id: 1, name: 'Unassigned' }, translate, 'owner')).toBe('zammad.unassignedOwner:'); + expect(getZammadObservedValueLabel({ id: 0, name: '' }, translate, 'owner')).toBe('zammad.timeline.initialValue:'); + expect(getZammadObservedValueLabel({ id: 1, name: 'new' }, translate, 'status')).toBe('new'); + expect(getZammadObservedValueLabel({ id: 1, name: 'Unassigned' }, translate, 'group')).toBe('Unassigned'); + }); + + it('shows each status bucket with its connection and appearance', () => { + const render = (key, params = {}) => key === 'zammad.overview.statusWithConnection' + ? `${params.connection} · ${params.status}` : translate(key, params); + expect(getZammadStatusBucketDisplayLabel({ id: 2, name: 'open', connection_name: 'Helpdesk' }, render)).toBe('Helpdesk · open'); + expect(getZammadStatusAppearance({ id: 3, name: 'pending reminder' }, false)).toBe('warning'); + expect(getZammadStatusAppearance({ id: 4, name: 'closed' }, true)).toBe('success'); + }); + + it('rejects an overview response from an older workspace', () => { + expect(isCurrentZammadWorkspaceOverviewRequest(1, 2, 7, 8)).toBe(false); + expect(isCurrentZammadWorkspaceOverviewRequest(2, 2, 7, 8)).toBe(false); + expect(isCurrentZammadWorkspaceOverviewRequest(2, 2, 8, 8)).toBe(true); + }); +}); diff --git a/frontend/src/lib/widgets/ZammadSupportWidget.test.js b/frontend/src/lib/widgets/ZammadSupportWidget.test.js new file mode 100644 index 0000000..973694d --- /dev/null +++ b/frontend/src/lib/widgets/ZammadSupportWidget.test.js @@ -0,0 +1,86 @@ +/** @vitest-environment jsdom */ + +import '@testing-library/jest-dom/vitest'; +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/svelte'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import ZammadSupportWidget from './ZammadSupportWidget.svelte'; + +const mocks = vi.hoisted(() => ({ overview: vi.fn() })); +vi.mock('../api.js', () => ({ api: { zammadTickets: { workspaceOverview: mocks.overview } } })); +vi.mock('../stores', () => ({ authStore: { currentUser: { timezone: 'UTC' } } })); +vi.mock('../stores/i18n.svelte.js', () => ({ + t: (key, params = {}) => { + if (key === 'zammad.ticketNumber') return `Zammad #${params.number}`; + if (key === 'zammad.timeline.change') return `${params.field}: ${params.from} → ${params.to}`; + if (key === 'zammad.overview.statusWithConnection') return `${params.connection} · ${params.status}`; + return key; + }, +})); + +const deferred = () => { + let resolve; + let reject; + const promise = new Promise((res, rej) => { resolve = res; reject = rej; }); + return { promise, resolve, reject }; +}; + +const overview = (title) => ({ + total: 1, + active: 1, + by_status: [{ connection_name: 'Helpdesk', id: 2, name: 'open', count: 1 }], + tickets: [{ id: title, item_id: 42, item_key: 'OPS-42', ticket_title: title, ticket_number: '123', status: { id: 2, name: 'open' }, group: { id: 7, name: 'Network' }, owner: { id: 9, name: 'Ada' } }], + recent_changes: [ + { id: `${title}-status`, item_id: 42, item_key: 'OPS-42', ticket_title: title, ticket_number: '123', field: 'status', old_value: { id: 1, name: 'new' }, new_value: { id: 2, name: 'open' }, current_group: { id: 7, name: 'Network' }, current_owner: { id: 9, name: 'Ada' }, observed_at: '2026-09-30T10:00:00Z' }, + { id: `${title}-owner`, item_id: 42, item_key: 'OPS-42', ticket_title: title, ticket_number: '123', field: 'owner', old_value: { id: 1, name: 'Unassigned' }, new_value: { id: 9, name: 'Ada' }, current_group: { id: 7, name: 'Network' }, current_owner: { id: 9, name: 'Ada' }, observed_at: '2026-09-30T10:00:00Z' }, + { id: `${title}-priority`, item_id: 42, field: 'priority' }, + ], +}); + +beforeEach(() => mocks.overview.mockReset()); +afterEach(() => { cleanup(); vi.restoreAllMocks(); }); + +describe('Zammad workspace support widget', () => { + it('renders linked tickets and observed changes with workspace navigation', async () => { + mocks.overview.mockResolvedValue(overview('VPN access')); + render(ZammadSupportWidget, { workspaceId: 7 }); + + expect(await screen.findByTestId('zammad-support-overview')).toBeInTheDocument(); + expect(mocks.overview).toHaveBeenCalledWith(7, { limit: 5 }); + expect(screen.getByRole('link', { name: 'OPS-42' })).toHaveAttribute('href', '/workspaces/7/items/42'); + expect(screen.getByText('Helpdesk · open: 1')).toBeInTheDocument(); + expect(screen.getAllByText('Network').length).toBeGreaterThan(0); + expect(screen.getAllByText('Ada').length).toBeGreaterThan(0); + expect(screen.getByText('zammad.timeline.field.status: new → open')).toBeInTheDocument(); + expect(screen.getByText('zammad.timeline.field.owner: zammad.unassignedOwner → Ada')).toBeInTheDocument(); + expect(screen.queryByText('priority')).not.toBeInTheDocument(); + }); + + it('shows an API permission error and recovers on retry', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + mocks.overview.mockRejectedValueOnce(new Error('403')).mockResolvedValueOnce(overview('Recovered')); + render(ZammadSupportWidget, { workspaceId: 7 }); + + expect(await screen.findByText('zammad.overview.loadFailed')).toBeInTheDocument(); + expect(screen.queryByTestId('zammad-support-overview')).not.toBeInTheDocument(); + await fireEvent.click(screen.getByRole('button', { name: 'common.retry' })); + expect((await screen.findAllByText('Recovered')).length).toBeGreaterThan(0); + expect(mocks.overview).toHaveBeenCalledTimes(2); + expect(consoleError).toHaveBeenCalledOnce(); + }); + + it('ignores a stale error from a previous workspace', async () => { + const old = deferred(); + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + mocks.overview.mockReturnValueOnce(old.promise).mockResolvedValueOnce(overview('Current workspace')); + const view = render(ZammadSupportWidget, { workspaceId: 7 }); + await waitFor(() => expect(mocks.overview).toHaveBeenCalledWith(7, { limit: 5 })); + + await view.rerender({ workspaceId: 8 }); + expect((await screen.findAllByText('Current workspace')).length).toBeGreaterThan(0); + old.reject(new Error('403')); + await waitFor(() => expect(mocks.overview).toHaveBeenCalledTimes(2)); + expect(screen.getAllByText('Current workspace').length).toBeGreaterThan(0); + expect(screen.queryByText('zammad.overview.loadFailed')).not.toBeInTheDocument(); + expect(consoleError).not.toHaveBeenCalled(); + }); +}); diff --git a/internal/database/zammad_history_migration_test.go b/internal/database/zammad_history_migration_test.go new file mode 100644 index 0000000..dfbfde4 --- /dev/null +++ b/internal/database/zammad_history_migration_test.go @@ -0,0 +1,44 @@ +//go:build test + +package database_test + +import ( + "testing" + + "windshift/internal/testutils" +) + +func TestZammadHistoryMigrationReappliesOnExistingDatabase(t *testing.T) { + tdb := testutils.CreateTestDB(t, true) + defer tdb.Close() + + const version = "20261002_zammad_ticket_change_history" + assertHistoryMigration := func(stage string) { + t.Helper() + var stampCount int + if err := tdb.QueryRow("SELECT COUNT(*) FROM schema_migrations WHERE version = ?", version).Scan(&stampCount); err != nil { + t.Fatalf("%s: read migration stamp: %v", stage, err) + } + if stampCount != 1 { + t.Fatalf("%s: expected one migration stamp, got %d", stage, stampCount) + } + rows, err := tdb.Query("SELECT ticket_link_id, field_name, old_value_name, new_value_name, observed_at FROM zammad_ticket_changes WHERE 1=0") + if err != nil { + t.Fatalf("%s: history table missing expected columns: %v", stage, err) + } + _ = rows.Close() + } + assertHistoryMigration("fresh schema") + + // Model an existing installation at the previous migration version. + if _, err := tdb.Exec("DROP TABLE zammad_ticket_changes"); err != nil { + t.Fatalf("drop history table for upgrade fixture: %v", err) + } + if _, err := tdb.Exec("DELETE FROM schema_migrations WHERE version = ?", version); err != nil { + t.Fatalf("remove history migration stamp: %v", err) + } + if err := tdb.GetDatabase().Initialize(); err != nil { + t.Fatalf("run history migration on existing database: %v", err) + } + assertHistoryMigration("upgraded schema") +} diff --git a/internal/database/zammad_schema_test.go b/internal/database/zammad_schema_test.go new file mode 100644 index 0000000..d20ed72 --- /dev/null +++ b/internal/database/zammad_schema_test.go @@ -0,0 +1,376 @@ +package database + +import ( + "encoding/json" + "path/filepath" + "strings" + "testing" +) + +func TestZammadSchemaInitializesOnSQLite(t *testing.T) { + db, err := NewSQLiteDB(filepath.Join(t.TempDir(), "windshift-test.db")) + if err != nil { + t.Fatal(err) + } + defer func() { _ = db.Close() }() + if err := db.Initialize(); err != nil { + t.Fatalf("initialize SQLite schema: %v", err) + } + + for _, table := range []string{"zammad_connections", "zammad_ticket_links", "zammad_ticket_changes", "zammad_oauth_tokens", "zammad_oauth_state"} { + var count int + if err := db.QueryRow("SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name=?", table).Scan(&count); err != nil { + t.Fatal(err) + } + if count != 1 { + t.Fatalf("expected table %s", table) + } + } + + for _, version := range []string{"20260829_zammad_integration", "20260830_zammad_oauth_connections", "20260830_zammad_oauth_generation", "20260830_zammad_ticket_link_metadata", "20260830_zammad_ticket_link_completion_postgres", "20260831_zammad_connection_config_revision", "20260831_zammad_ticket_sync_lock_owner", "20260831_zammad_ticket_link_item_restrict", "20260831_zammad_persisted_group_catalog", "20260901_zammad_canonical_workspace_scope", "20261002_zammad_ticket_change_history"} { + var migrationCount int + if err := db.QueryRow("SELECT COUNT(*) FROM schema_migrations WHERE version=?", version).Scan(&migrationCount); err != nil { + t.Fatal(err) + } + if migrationCount != 1 { + t.Fatalf("Zammad migration %s was not stamped", version) + } + } + rows, err := db.Query("PRAGMA table_info(zammad_connections)") + if err != nil { + t.Fatal(err) + } + defer func() { _ = rows.Close() }() + var requiredCredential, authMethod, oauthGeneration, configRevision, oauthAttempt bool + for rows.Next() { + var cid int + var name, columnType string + var notNull, primaryKey int + var defaultValue any + if err := rows.Scan(&cid, &name, &columnType, ¬Null, &defaultValue, &primaryKey); err != nil { + t.Fatal(err) + } + if name == "credential_id" { + requiredCredential = notNull == 1 + } + if name == "auth_method" { + authMethod = true + } + if name == "oauth_generation" { + oauthGeneration = notNull == 1 + } + if name == "config_revision" { + configRevision = notNull == 1 + } + if name == "oauth_attempt_id" { + oauthAttempt = true + } + } + if !requiredCredential || !authMethod || !oauthGeneration || !configRevision || !oauthAttempt { + t.Fatal("Zammad OAuth connection schema must retain the legacy managed-credential constraint") + } + for table, columns := range map[string][]string{ + "zammad_oauth_tokens": {"oauth_generation", "refresh_claim_owner"}, + "zammad_oauth_state": {"oauth_generation"}, + "zammad_ticket_links": {"sync_lock_owner"}, + } { + for _, column := range columns { + var count int + if err := db.QueryRow("SELECT COUNT(*) FROM pragma_table_info(?) WHERE name = ?", table, column).Scan(&count); err != nil { + t.Fatal(err) + } + if count != 1 { + t.Fatalf("expected %s.%s", table, column) + } + } + } + var stateProviderIndex int + if err := db.QueryRow("SELECT COUNT(*) FROM sqlite_master WHERE type='index' AND name='idx_zammad_oauth_state_provider'").Scan(&stateProviderIndex); err != nil || stateProviderIndex != 1 { + t.Fatalf("expected unique per-connection OAuth state index: count=%d err=%v", stateProviderIndex, err) + } +} + +func TestZammadTicketLinkMetadataSQLiteUpgrade(t *testing.T) { + db, err := NewSQLiteDB(filepath.Join(t.TempDir(), "legacy-zammad-links.db")) + if err != nil { + t.Fatal(err) + } + defer func() { _ = db.Close() }() + if err := db.Initialize(); err != nil { + t.Fatal(err) + } + + // Recreate the stamped baseline table without the metadata columns. This + // models an existing install whose 20260829 migration already ran. + if _, err := db.Exec("PRAGMA foreign_keys=OFF"); err != nil { + t.Fatal(err) + } + _, err = db.Exec(` + CREATE TABLE zammad_ticket_links_legacy ( + id TEXT PRIMARY KEY, item_id INTEGER NOT NULL, provider_id TEXT NOT NULL, + item_integration_link_id TEXT, ticket_id INTEGER, ticket_number TEXT DEFAULT '', + ticket_url TEXT DEFAULT '', group_id INTEGER, group_name TEXT DEFAULT '', + correlation_key TEXT NOT NULL, sync_state TEXT NOT NULL DEFAULT 'pending', + creating_started_at DATETIME, last_status_id INTEGER, last_status_name TEXT DEFAULT '', + last_synced_at DATETIME, last_error TEXT DEFAULT '', + completion_applied BOOLEAN NOT NULL DEFAULT false, sync_lock_until DATETIME, + created_by INTEGER, created_at DATETIME DEFAULT CURRENT_TIMESTAMP, + updated_at DATETIME DEFAULT CURRENT_TIMESTAMP, + UNIQUE(item_id, provider_id), UNIQUE(provider_id, ticket_id), + UNIQUE(provider_id, correlation_key) + ); + DROP TABLE zammad_ticket_links; + ALTER TABLE zammad_ticket_links_legacy RENAME TO zammad_ticket_links; + DELETE FROM schema_migrations + WHERE version IN ( + '20260830_zammad_ticket_link_metadata', + '20260831_zammad_ticket_sync_lock_owner', + '20260831_zammad_ticket_link_item_restrict' + ); + `) + if err != nil { + t.Fatal(err) + } + if _, err := db.Exec("PRAGMA foreign_keys=ON"); err != nil { + t.Fatal(err) + } + if err := runPendingMigrations(db, Catalog); err != nil { + t.Fatalf("run additive ticket-link metadata migration: %v", err) + } + + for _, column := range []string{"owner_id", "owner_name", "last_attempt_at", "next_attempt_at", "completion_applied", "sync_lock_owner"} { + var count int + if err := db.QueryRow("SELECT COUNT(*) FROM pragma_table_info('zammad_ticket_links') WHERE name = ?", column).Scan(&count); err != nil { + t.Fatal(err) + } + if count != 1 { + t.Fatalf("expected migrated zammad_ticket_links.%s", column) + } + } + var deleteRule string + if err := db.QueryRow(`SELECT on_delete FROM pragma_foreign_key_list('zammad_ticket_links') WHERE "table" = 'items' AND "from" = 'item_id'`).Scan(&deleteRule); err != nil { + t.Fatal(err) + } + if deleteRule != "RESTRICT" { + t.Fatalf("expected migrated item foreign key to restrict deletion, got %q", deleteRule) + } +} + +func TestZammadTicketLinkMetadataMigrationBackendParity(t *testing.T) { + var migration *Migration + for i := range Catalog { + if Catalog[i].Version == "20260830_zammad_ticket_link_metadata" { + migration = &Catalog[i] + break + } + } + if migration == nil { + t.Fatal("Zammad ticket-link metadata migration is missing") + } + for _, column := range []string{"owner_id", "owner_name", "last_attempt_at", "next_attempt_at"} { + if !strings.Contains(migration.CheckSQLite, column) || !strings.Contains(migration.CheckPostgres, column) { + t.Fatalf("metadata column %s is missing from one of the backend checks", column) + } + if !strings.Contains(migration.SQLite, "ADD COLUMN "+column) || !strings.Contains(migration.Postgres, "ADD COLUMN IF NOT EXISTS "+column) { + t.Fatalf("metadata column %s is missing from one of the backend migration bodies", column) + } + } + if !strings.Contains(zammadSchemaMigrationSQLite, "completion_applied") || !strings.Contains(zammadSchemaMigrationPostgres, "completion_applied") { + t.Fatal("baseline Zammad migrations must include completion_applied") + } + var completionMigration *Migration + for i := range Catalog { + if Catalog[i].Version == "20260830_zammad_ticket_link_completion_postgres" { + completionMigration = &Catalog[i] + break + } + } + if completionMigration == nil || !strings.Contains(completionMigration.Postgres, "ADD COLUMN IF NOT EXISTS completion_applied") { + t.Fatal("PostgreSQL completion_applied backfill migration is missing") + } +} + +func TestZammadCanonicalWorkspaceScopeMigrationChecksumStability(t *testing.T) { + var migration *Migration + for i := range Catalog { + if Catalog[i].Version == "20260901_zammad_canonical_workspace_scope" { + migration = &Catalog[i] + break + } + } + if migration == nil { + t.Fatal("Zammad canonical workspace scope migration is missing") + } + + for driver, want := range map[string]string{ + driverSQLite: "164fb8cb79ee0f5705aa5891540576995bc030260effc89c75a6de470e45a33f", + driverPostgres: "4d1561e94cff763531c5c68a749f5f32bc3f3aa2feb2493ac7911d697bd450b4", + } { + if got := migration.checksum(driver); got != want { + t.Fatalf("historical %s migration checksum changed: got %s, want %s", driver, got, want) + } + } +} + +func TestZammadTicketSyncLockOwnerMigrationBackendParity(t *testing.T) { + var migration *Migration + for i := range Catalog { + if Catalog[i].Version == "20260831_zammad_ticket_sync_lock_owner" { + migration = &Catalog[i] + break + } + } + if migration == nil { + t.Fatal("Zammad sync lock owner migration is missing") + } + if !strings.Contains(migration.CheckSQLite, "sync_lock_owner") || !strings.Contains(migration.CheckPostgres, "sync_lock_owner") { + t.Fatal("sync lock owner migration checks must cover both backends") + } + if !strings.Contains(migration.SQLite, "ADD COLUMN sync_lock_owner TEXT") || !strings.Contains(migration.Postgres, "ADD COLUMN IF NOT EXISTS sync_lock_owner TEXT") { + t.Fatal("sync lock owner migration bodies must cover both backends") + } + if strings.Contains(zammadSchemaMigrationSQLite, "sync_lock_owner TEXT") || strings.Contains(zammadSchemaMigrationPostgres, "sync_lock_owner TEXT") { + t.Fatal("historical Zammad migration must remain checksum-stable; sync_lock_owner belongs only in the additive migration") + } +} + +func TestZammadTicketChangeHistoryMigrationBackendParity(t *testing.T) { + var migration *Migration + for i := range Catalog { + if Catalog[i].Version == "20261002_zammad_ticket_change_history" { + migration = &Catalog[i] + break + } + } + if migration == nil { + t.Fatal("Zammad ticket change history migration is missing") + } + for _, body := range []string{migration.CheckSQLite, migration.CheckPostgres, migration.SQLite, migration.Postgres} { + if !strings.Contains(body, "zammad_ticket_changes") { + t.Fatal("ticket change history migration must cover both backends") + } + } + if strings.Contains(zammadSchemaMigrationSQLite, "zammad_ticket_changes") || strings.Contains(zammadSchemaMigrationPostgres, "zammad_ticket_changes") { + t.Fatal("historical Zammad migrations must remain checksum-stable") + } +} + +func TestZammadTicketLinkItemRestrictMigrationBackendParity(t *testing.T) { + var migration *Migration + for i := range Catalog { + if Catalog[i].Version == "20260831_zammad_ticket_link_item_restrict" { + migration = &Catalog[i] + break + } + } + if migration == nil { + t.Fatal("Zammad item deletion restriction migration is missing") + } + if !strings.Contains(migration.CheckSQLite, "on_delete = 'RESTRICT'") || !strings.Contains(migration.CheckPostgres, "delete_rule = 'RESTRICT'") { + t.Fatal("item deletion restriction migration checks must cover both backends") + } + if !strings.Contains(migration.SQLite, "REFERENCES items(id) ON DELETE RESTRICT") || !strings.Contains(migration.Postgres, "REFERENCES items(id) ON DELETE RESTRICT") { + t.Fatal("item deletion restriction migration bodies must cover both backends") + } + if !strings.Contains(zammadSchemaMigrationSQLite, "REFERENCES items(id) ON DELETE CASCADE") || !strings.Contains(zammadSchemaMigrationPostgres, "REFERENCES items(id) ON DELETE CASCADE") { + t.Fatal("historical Zammad migrations must remain checksum-stable") + } +} + +// This starts from the deployed 20260829 table shape. The OAuth migration is +// additive: it preserves an API-token connection and permits a new OAuth row +// with its mandatory managed-credential container. +func TestZammadOAuthSQLiteUpgradePreservesLegacyConnection(t *testing.T) { + db, err := NewSQLiteDB(filepath.Join(t.TempDir(), "legacy-zammad.db")) + if err != nil { + t.Fatal(err) + } + defer func() { _ = db.Close() }() + if err := db.Initialize(); err != nil { + t.Fatal(err) + } + for _, statement := range []string{ + `INSERT INTO users(id, email, username, first_name, last_name) VALUES (9001, 'zammad-upgrade@example.test', 'zammad-upgrade', 'Zammad', 'Upgrade')`, + `INSERT INTO workspaces(id, name, key) VALUES (9100, 'Legacy scope', 'LEGACY')`, + `INSERT INTO action_credentials(id, name, credential_type, applies_to_all_workspaces, encrypted_secret, secret_metadata, is_enabled) VALUES (9001, 'legacy API', 'custom_header', true, 'opaque-ciphertext', '{"_windshift_managed_credential":"v1","managed_by":"zammad","owner_id":"legacy-zammad"}', true)`, + `INSERT INTO integration_providers(id, slug, name, provider_type, enabled, provider_config) VALUES ('legacy-zammad', 'legacy-zammad', 'Legacy Zammad', 'zammad', true, '{}')`, + `INSERT INTO zammad_connections(provider_id, credential_id, auth_method, base_url, default_group_id, default_group_name, allowed_groups, default_customer, closed_state_ids, created_by) VALUES ('legacy-zammad', 9001, 'api_token', 'https://legacy.example.test', 7, 'Support', '[]', 'robot@example.test', '[]', 9001)`, + } { + if _, err := db.ExecWrite(statement); err != nil { + t.Fatal(err) + } + } + if _, err := db.Exec("PRAGMA foreign_keys=OFF"); err != nil { + t.Fatal(err) + } + _, err = db.Exec(` + CREATE TABLE zammad_connections_legacy ( + provider_id TEXT PRIMARY KEY, credential_id INTEGER NOT NULL, base_url TEXT NOT NULL, default_group_id INTEGER, + default_group_name TEXT DEFAULT '', allowed_group_ids TEXT NOT NULL DEFAULT '[]', default_customer TEXT NOT NULL, + correlation_field TEXT NOT NULL DEFAULT 'windshift_item_key', closed_state_ids TEXT NOT NULL DEFAULT '[]', completion_status_id INTEGER, + applies_to_all_workspaces BOOLEAN NOT NULL DEFAULT false, last_tested_at DATETIME, last_test_error TEXT DEFAULT '', created_by INTEGER, + created_at DATETIME DEFAULT CURRENT_TIMESTAMP, updated_at DATETIME DEFAULT CURRENT_TIMESTAMP + ); + INSERT INTO zammad_connections_legacy(provider_id, credential_id, base_url, default_group_id, default_group_name, allowed_group_ids, default_customer, correlation_field, closed_state_ids, completion_status_id, applies_to_all_workspaces, last_tested_at, last_test_error, created_by, created_at, updated_at) + SELECT provider_id, credential_id, base_url, default_group_id, default_group_name, allowed_groups, default_customer, correlation_field, closed_state_ids, completion_status_id, false, last_tested_at, last_test_error, created_by, created_at, updated_at FROM zammad_connections; + DROP TABLE zammad_connections; + ALTER TABLE zammad_connections_legacy RENAME TO zammad_connections; + DROP TABLE IF EXISTS zammad_connection_workspaces; + CREATE TABLE zammad_connection_workspaces ( + provider_id TEXT NOT NULL, workspace_id INTEGER NOT NULL, + PRIMARY KEY (provider_id, workspace_id) + ); + INSERT INTO zammad_connection_workspaces(provider_id, workspace_id) VALUES ('legacy-zammad', 9100); + UPDATE zammad_connections SET allowed_group_ids = '[7,"bad",8,0]'; + DROP TABLE zammad_oauth_state; + DROP TABLE zammad_oauth_tokens; + DELETE FROM schema_migrations WHERE version = '20260830_zammad_oauth_connections'; + DELETE FROM schema_migrations WHERE version = '20260830_zammad_oauth_generation'; + DELETE FROM schema_migrations WHERE version = '20260831_zammad_connection_config_revision'; + DELETE FROM schema_migrations WHERE version = '20260831_zammad_persisted_group_catalog'; + DELETE FROM schema_migrations WHERE version = '20260901_zammad_canonical_workspace_scope'; + `) + if _, onErr := db.Exec("PRAGMA foreign_keys=ON"); onErr != nil { + t.Fatal(onErr) + } + if err != nil { + t.Fatal(err) + } + if err := runPendingMigrations(db, Catalog); err != nil { + t.Fatalf("run additive OAuth migration: %v", err) + } + var credentialID int + var configRevision int64 + var authMethod string + if err := db.QueryRow(`SELECT credential_id, auth_method, config_revision FROM zammad_connections WHERE provider_id='legacy-zammad'`).Scan(&credentialID, &authMethod, &configRevision); err != nil || credentialID != 9001 || authMethod != "api_token" || configRevision != 1 { + t.Fatalf("legacy API connection was not preserved: credential=%d method=%q revision=%d err=%v", credentialID, authMethod, configRevision, err) + } + var migratedGroupsJSON string + if err := db.QueryRow(`SELECT allowed_groups FROM zammad_connections WHERE provider_id='legacy-zammad'`).Scan(&migratedGroupsJSON); err != nil { + t.Fatal(err) + } + var migratedGroups []struct { + ID int `json:"id"` + Name string `json:"name"` + } + if err := json.Unmarshal([]byte(migratedGroupsJSON), &migratedGroups); err != nil || len(migratedGroups) != 2 || migratedGroups[0].ID != 7 || migratedGroups[0].Name != "Support" || migratedGroups[1].ID != 8 || migratedGroups[1].Name != "" { + t.Fatalf("legacy group IDs were not migrated safely: groups=%#v raw=%q err=%v", migratedGroups, migratedGroupsJSON, err) + } + var appliesToAll bool + if err := db.QueryRow(`SELECT applies_to_all_workspaces FROM action_credentials WHERE id=9001`).Scan(&appliesToAll); err != nil || appliesToAll { + t.Fatalf("legacy restricted scope was not copied to the managed credential: applies_all=%v err=%v", appliesToAll, err) + } + var scopeRows int + if err := db.QueryRow(`SELECT COUNT(*) FROM action_credential_workspaces WHERE credential_id=9001 AND workspace_id=9100`).Scan(&scopeRows); err != nil || scopeRows != 1 { + t.Fatalf("legacy workspace allowlist was not copied to the managed credential: count=%d err=%v", scopeRows, err) + } + for _, statement := range []string{ + `INSERT INTO action_credentials(id, name, credential_type, applies_to_all_workspaces, encrypted_secret, is_enabled) VALUES (9002, 'OAuth pending', 'custom_header', true, 'opaque-pending-ciphertext', true)`, + `INSERT INTO integration_providers(id, slug, name, provider_type, enabled, oauth_client_id, oauth_client_secret_encrypted, provider_config) VALUES ('oauth-zammad', 'oauth-zammad', 'OAuth Zammad', 'zammad', true, 'client', 'opaque-ciphertext', '{}')`, + `INSERT INTO zammad_connections(provider_id, credential_id, auth_method, base_url, allowed_groups, default_customer, closed_state_ids, applies_to_all_workspaces, created_by) VALUES ('oauth-zammad', 9002, 'oauth', 'https://oauth.example.test', '[]', 'robot@example.test', '[]', true, 9001)`, + } { + if _, err := db.ExecWrite(statement); err != nil { + t.Fatalf("upgraded schema rejected OAuth pending managed credential: %v", err) + } + } +} diff --git a/internal/email/processor_core_regression_test.go b/internal/email/processor_core_regression_test.go index 55b4760..34102f6 100644 --- a/internal/email/processor_core_regression_test.go +++ b/internal/email/processor_core_regression_test.go @@ -2,6 +2,7 @@ package email import ( "context" + "fmt" "path/filepath" "testing" "time" @@ -105,7 +106,11 @@ func TestTrackingPreclaimRecoversOnlyStaleIncompleteClaims(t *testing.T) { } } -func TestSenderThreadParticipationIsScopedToEmailChannel(t *testing.T) { +// TestSenderThreadParticipationIsItemScoped pins the WI-1546 contract: the +// thread belongs to the item, not to a channel, so a prior participant on the +// item's thread authorizes regardless of which channel their messages used, +// while a participant on some other item's thread does not. +func TestSenderThreadParticipationIsItemScoped(t *testing.T) { db := newProcessorRegressionTestDB(t) insertChannel := func(name string) int { t.Helper() @@ -121,7 +126,7 @@ func TestSenderThreadParticipationIsScopedToEmailChannel(t *testing.T) { firstChannelID := insertChannel("First mailbox") secondChannelID := insertChannel("Second mailbox") - var workspaceID, itemID int + var workspaceID, itemID, otherItemID int if err := db.QueryRow(`INSERT INTO workspaces (name, key) VALUES ('Thread', 'THR') RETURNING id`).Scan(&workspaceID); err != nil { t.Fatalf("insert workspace: %v", err) } @@ -131,27 +136,146 @@ func TestSenderThreadParticipationIsScopedToEmailChannel(t *testing.T) { `, workspaceID, firstChannelID, testutils.NextTestFracIndex()).Scan(&itemID); err != nil { t.Fatalf("insert item: %v", err) } + if err := db.QueryRow(` + INSERT INTO items (workspace_id, workspace_item_number, title, channel_id, frac_index) + VALUES (?, 2, 'Other item', ?, ?) RETURNING id + `, workspaceID, secondChannelID, testutils.NextTestFracIndex()).Scan(&otherItemID); err != nil { + t.Fatalf("insert other item: %v", err) + } + // Sender last participated on a different email channel than the item's. if _, err := db.ExecWrite(` INSERT INTO email_message_tracking (channel_id, message_id, dedup_key, from_email, item_id) VALUES (?, '', '', 'sender@example.com', ?) `, secondChannelID, itemID); err != nil { - t.Fatalf("insert other-channel participant: %v", err) + t.Fatalf("insert cross-channel participant: %v", err) + } + + processor := NewProcessor(db, "") + if !processor.senderIsThreadParticipant(context.Background(), itemID, "sender@example.com") { + t.Fatal("participant on the item's thread (other channel) was not authorized") + } + // Participation on a different item authorizes nothing on this one. + if processor.senderIsThreadParticipant(context.Background(), otherItemID, "sender@example.com") { + t.Fatal("participation on another item's thread authorized this item") + } + // Unknown senders stay rejected. + if processor.senderIsThreadParticipant(context.Background(), itemID, "stranger@example.com") { + t.Fatal("unknown sender was authorized") + } +} + +// TestSenderThreadParticipationAcceptsExternalParticipant proves WI-1136: an +// explicitly added external participant can reply by email to the item's +// thread, and only to that item. +func TestSenderThreadParticipationAcceptsExternalParticipant(t *testing.T) { + db := newProcessorRegressionTestDB(t) + var workspaceID, itemID, otherItemID int + if err := db.QueryRow(`INSERT INTO workspaces (name, key) VALUES ('Participants', 'PRT') RETURNING id`).Scan(&workspaceID); err != nil { + t.Fatalf("insert workspace: %v", err) + } + if err := db.QueryRow(` + INSERT INTO items (workspace_id, workspace_item_number, title, frac_index) + VALUES (?, 1, 'Participant item', ?) RETURNING id + `, workspaceID, testutils.NextTestFracIndex()).Scan(&itemID); err != nil { + t.Fatalf("insert item: %v", err) + } + if err := db.QueryRow(` + INSERT INTO items (workspace_id, workspace_item_number, title, frac_index) + VALUES (?, 2, 'Other item', ?) RETURNING id + `, workspaceID, testutils.NextTestFracIndex()).Scan(&otherItemID); err != nil { + t.Fatalf("insert other item: %v", err) + } + var customerID int + if err := db.QueryRow(` + INSERT INTO portal_customers (name, email, created_via) + VALUES ('Participant', 'participant@example.com', 'agent') RETURNING id + `).Scan(&customerID); err != nil { + t.Fatalf("insert customer: %v", err) + } + if _, err := db.ExecWrite(`INSERT INTO item_participants (item_id, portal_customer_id) VALUES (?, ?)`, itemID, customerID); err != nil { + t.Fatalf("insert participant: %v", err) } processor := NewProcessor(db, "") - if processor.senderIsThreadParticipant(context.Background(), itemID, firstChannelID, "sender@example.com") { - t.Fatal("participation in a different email channel authorized this thread") + if !processor.senderIsThreadParticipant(context.Background(), itemID, "PARTICIPANT@example.com") { + t.Fatal("external participant was not authorized to reply") } + if processor.senderIsThreadParticipant(context.Background(), otherItemID, "participant@example.com") { + t.Fatal("participant on another item authorized this item") + } + if processor.senderIsThreadParticipant(context.Background(), itemID, "stranger@example.com") { + t.Fatal("unknown sender was authorized") + } +} + +// TestFindParentItemRoutesPortalReplyAcrossChannels is the WI-1546 loop: a +// portal ticket's synthetic anchor was minted on the portal channel; the +// customer replies by email to the notification, so the message arrives on the +// email intake channel quoting the anchor. Matching must find the portal item, +// and the creator must be recognized without any prior tracked message. +func TestFindParentItemRoutesPortalReplyAcrossChannels(t *testing.T) { + db := newProcessorRegressionTestDB(t) + var workspaceID int + if err := db.QueryRow(`INSERT INTO workspaces (name, key) VALUES ('Portal loop', 'PLP') RETURNING id`).Scan(&workspaceID); err != nil { + t.Fatalf("insert workspace: %v", err) + } + var portalChannelID, emailChannelID int + if err := db.QueryRow(`INSERT INTO channels (name, type, direction) VALUES ('Portal', 'portal', 'inbound') RETURNING id`).Scan(&portalChannelID); err != nil { + t.Fatalf("insert portal channel: %v", err) + } + if err := db.QueryRow(`INSERT INTO channels (name, type, direction) VALUES ('Mailbox', 'email', 'inbound') RETURNING id`).Scan(&emailChannelID); err != nil { + t.Fatalf("insert email channel: %v", err) + } + var customerID int + if err := db.QueryRow(`INSERT INTO portal_customers (name, email) VALUES ('Customer', 'customer@example.com') RETURNING id`).Scan(&customerID); err != nil { + t.Fatalf("insert customer: %v", err) + } + var itemID int + if err := db.QueryRow(` + INSERT INTO items (workspace_id, workspace_item_number, title, channel_id, creator_portal_customer_id, frac_index) + VALUES (?, 1, 'Portal request', ?, ?, ?) RETURNING id + `, workspaceID, portalChannelID, customerID, testutils.NextTestFracIndex()).Scan(&itemID); err != nil { + t.Fatalf("insert portal item: %v", err) + } + // The synthetic thread anchor minted at submission (slice 1). + anchorID := fmt.Sprintf("", itemID) if _, err := db.ExecWrite(` INSERT INTO email_message_tracking - (channel_id, message_id, dedup_key, from_email, item_id) - VALUES (?, '', '', 'sender@example.com', ?) - `, firstChannelID, itemID); err != nil { - t.Fatalf("insert same-channel participant: %v", err) + (channel_id, message_id, dedup_key, from_email, item_id, direction) + VALUES (?, ?, ?, '', ?, 'outbound') + `, portalChannelID, anchorID, anchorID, itemID); err != nil { + t.Fatalf("insert anchor: %v", err) + } + + processor := NewProcessor(db, "") + parent := processor.findParentItem(context.Background(), &ParsedEmail{ + InReplyTo: anchorID, + From: EmailAddress{Address: "customer@example.com"}, + }) + if parent == nil || *parent != itemID { + t.Fatalf("portal reply parent = %v, want %d", parent, itemID) + } + + // A stranger quoting the leaked anchor must not land on the ticket. + if strangerParent := processor.findParentItem(context.Background(), &ParsedEmail{ + InReplyTo: anchorID, + From: EmailAddress{Address: "stranger@example.com"}, + }); strangerParent != nil { + t.Fatalf("stranger reply matched item %d; hijack guard failed", *strangerParent) + } + + // A participant of the email conversation (an outbound notice recipient is + // recorded under their from_email) may also append. + if _, err := db.ExecWrite(` + INSERT INTO email_message_tracking + (channel_id, message_id, dedup_key, from_email, item_id, direction) + VALUES (?, '', '', 'customer@example.com', ?, 'outbound') + `, emailChannelID, itemID); err != nil { + t.Fatalf("insert participant row: %v", err) } - if !processor.senderIsThreadParticipant(context.Background(), itemID, firstChannelID, "sender@example.com") { - t.Fatal("same-channel participant was not authorized") + if !processor.senderIsThreadParticipant(context.Background(), itemID, "customer@example.com") { + t.Fatal("thread participant lost authorization after channel change") } } @@ -178,7 +302,7 @@ func TestFindParentItemAcceptsLegacyBareMessageID(t *testing.T) { t.Fatalf("insert legacy tracking: %v", err) } - parent := NewProcessor(db, "").findParentItem(context.Background(), channelID, &ParsedEmail{ + parent := NewProcessor(db, "").findParentItem(context.Background(), &ParsedEmail{ InReplyTo: "", From: EmailAddress{Address: "sender@example.com"}, }) diff --git a/internal/handlers/workspaces_settings_test.go b/internal/handlers/workspaces_settings_test.go index 232d2ea..d67696e 100644 --- a/internal/handlers/workspaces_settings_test.go +++ b/internal/handlers/workspaces_settings_test.go @@ -357,3 +357,9 @@ func TestWorkspaceHandler_UpdateHomepageLayout_AcceptsStoryPointsByAssigneeType( t.Fatalf("saved widgets = %+v, want one story-points-by-assignee", saved.Widgets) } } + +func TestValidWorkspaceWidgetTypesIncludesZammadSupportOverview(t *testing.T) { + if !validWorkspaceWidgetTypes["zammad-support-overview"] { + t.Fatal("zammad support overview must be accepted by homepage layout validation") + } +} diff --git a/internal/handlers/zammad_observability_read_test.go b/internal/handlers/zammad_observability_read_test.go new file mode 100644 index 0000000..db5227f --- /dev/null +++ b/internal/handlers/zammad_observability_read_test.go @@ -0,0 +1,78 @@ +//go:build test + +package handlers + +import ( + "net/http" + "net/http/httptest" + "testing" + + "windshift/internal/logger" + "windshift/internal/repository" + "windshift/internal/services" + "windshift/internal/testutils" +) + +func TestZammadObservabilityReadsRequireViewAndBoundLimit(t *testing.T) { + tdb := testutils.CreateTestDB(t, true) + defer tdb.Close() + seed := tdb.SeedTestData(t) + if _, err := tdb.Exec(`INSERT INTO users (id, email, username, first_name, last_name, is_active) + VALUES (2, 'outsider@example.test', 'outsider', 'Outside', 'User', true)`); err != nil { + t.Fatal(err) + } + if _, err := tdb.Exec(`INSERT INTO items + (id, workspace_id, workspace_item_number, title, description, frac_index, status_id, creator_id, last_active_at) + VALUES (1, 1, 1, 'Observed item', '', 'a0', ?, 1, CURRENT_TIMESTAMP)`, seed.StatusID); err != nil { + t.Fatal(err) + } + // An explicit Viewer assignment closes the implicit Everyone view grant. + if _, err := tdb.Exec(`INSERT INTO user_workspace_roles (user_id, workspace_id, role_id, granted_at) + SELECT 1, 1, id, CURRENT_TIMESTAMP FROM workspace_roles WHERE name = 'Viewer'`); err != nil { + t.Fatal(err) + } + permission, _, _ := createTestServices(t, *tdb) + zammadRepo := repository.NewZammadRepository(tdb.GetDatabase()) + handler := NewZammadHandler( + repository.NewItemRepository(tdb.GetDatabase()), + services.NewZammadService(tdb.GetDatabase(), zammadRepo, nil, nil, nil, nil, nil), + permission, + logger.NewAuditor(tdb.GetDatabase()), + ) + + for _, endpoint := range []struct { + name, path, param string + serve http.HandlerFunc + }{ + {"workspace overview", "/api/workspaces/1/zammad-overview", "workspaceId", handler.GetWorkspaceOverview}, + {"item history", "/api/items/1/zammad-history", "id", handler.GetItemHistory}, + } { + t.Run(endpoint.name, func(t *testing.T) { + call := func(userID int, limit string) int { + t.Helper() + url := endpoint.path + if limit != "" { + url += "?limit=" + limit + } + req := httptest.NewRequest(http.MethodGet, url, nil) + req.SetPathValue(endpoint.param, "1") + rec := httptest.NewRecorder() + endpoint.serve(rec, testutils.WithAuthContext(req, testutils.TestUserWithID(userID))) + return rec.Code + } + if got := call(2, "invalid"); got != http.StatusNotFound { + t.Fatalf("outsider must receive 404 before limit parsing; got %d", got) + } + for _, value := range []string{"0", "101", "invalid"} { + if got := call(1, value); got != http.StatusBadRequest { + t.Errorf("limit %q: expected 400, got %d", value, got) + } + } + for _, value := range []string{"", "1", "100"} { + if got := call(1, value); got != http.StatusOK { + t.Errorf("limit %q: expected 200, got %d", value, got) + } + } + }) + } +} diff --git a/internal/integrations/zammad/client_test.go b/internal/integrations/zammad/client_test.go new file mode 100644 index 0000000..590a07d --- /dev/null +++ b/internal/integrations/zammad/client_test.go @@ -0,0 +1,267 @@ +package zammad + +import ( + "bytes" + "context" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + "time" +) + +type httpTestTransport struct{ client *http.Client } + +func (t httpTestTransport) Do(ctx context.Context, method, targetURL string, body []byte, headers map[string]string) (*Response, error) { + req, err := http.NewRequestWithContext(ctx, method, targetURL, bytes.NewReader(body)) + if err != nil { + return nil, err + } + for key, value := range headers { + req.Header.Set(key, value) + } + resp, err := t.client.Do(req) + if err != nil { + return nil, err + } + defer func() { _ = resp.Body.Close() }() + responseBody, err := io.ReadAll(resp.Body) + if err != nil { + return nil, err + } + return &Response{StatusCode: resp.StatusCode, Body: responseBody}, nil +} + +func TestClientStatesCorrelationSearchAndTicketCreation(t *testing.T) { + t.Parallel() + const token = "synthetic-secret-token" + postCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("Authorization"); got != "Token token="+token { + t.Fatalf("unexpected authorization header: %q", got) + } + w.Header().Set("Content-Type", "application/json") + switch { + case r.URL.Path == "/api/v1/ticket_states": + _, _ = w.Write([]byte(`[{"id":4,"name":"closed","state_type_id":5,"active":true},{"id":5,"name":"inactive","active":false}]`)) + case r.URL.Path == "/api/v1/tickets/search": + _, _ = w.Write([]byte(`[{"id":9,"number":"42009","title":"Existing support request","group_id":1,"state_id":2,"state":"open","windshift_item_key":"windshift:abc:ITEM-49"}]`)) + case r.URL.Path == "/api/v1/tickets" && r.Method == http.MethodPost: + postCount++ + body, _ := io.ReadAll(r.Body) + if !strings.Contains(string(body), `"windshift_item_key":"windshift:abc:ITEM-50"`) || + !strings.Contains(string(body), `"group_id":7`) || strings.Contains(string(body), `"group":`) { + t.Fatalf("ticket payload lacks correlation field: %s", body) + } + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"id":10,"number":"42010","title":"ITEM-50","group_id":1,"state_id":2,"state":"open"}`)) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + client := NewClient(server.URL, token, httpTestTransport{client: server.Client()}) + states, err := client.States(context.Background()) + if err != nil { + t.Fatal(err) + } + if len(states) != 1 || states[0].Name != "closed" { + t.Fatalf("unexpected states: %#v", states) + } + found, err := client.FindByCorrelation(context.Background(), "windshift_item_key", "windshift:abc:ITEM-49") + if err != nil || found == nil || found.ID != 9 || found.Title != "Existing support request" { + t.Fatalf("unexpected search result: ticket=%#v err=%v", found, err) + } + created, err := client.CreateTicket(context.Background(), "ITEM-50", "Synthetic body", "robot@example.test", 7, "windshift_item_key", "windshift:abc:ITEM-50") + if err != nil || created.ID != 10 || created.Title != "ITEM-50" || postCount != 1 { + t.Fatalf("unexpected create result: ticket=%#v posts=%d err=%v", created, postCount, err) + } +} + +func TestClientRedactsAPIErrorBodyAndToken(t *testing.T) { + t.Parallel() + const token = "must-not-appear" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"error":"token must-not-appear is invalid"}`)) + })) + defer server.Close() + + client := NewClient(server.URL, token, httpTestTransport{client: server.Client()}) + _, err := client.States(context.Background()) + if err == nil { + t.Fatal("expected authentication error") + } + if strings.Contains(err.Error(), token) || strings.Contains(err.Error(), "invalid") { + t.Fatalf("API error disclosed response content: %v", err) + } +} + +func TestClientHonorsContextTimeout(t *testing.T) { + t.Parallel() + server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + <-r.Context().Done() + })) + defer server.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Millisecond) + defer cancel() + client := NewClient(server.URL, "token", httpTestTransport{client: server.Client()}) + _, err := client.States(ctx) + if err == nil { + t.Fatal("expected timeout") + } +} + +func TestClientFindByNumberRequiresExactMatch(t *testing.T) { + t.Parallel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/tickets/search" { + http.NotFound(w, r) + return + } + if got := r.URL.Query().Get("query"); got != `number:"42009"` { + t.Fatalf("unexpected ticket-number query: %q", got) + } + _, _ = w.Write([]byte(`[{"id":9,"number":"420090"},{"id":10,"number":"42008"}]`)) + })) + defer server.Close() + + client := NewClient(server.URL, "token", httpTestTransport{client: server.Client()}) + found, err := client.FindByNumber(context.Background(), "42009") + if err != nil { + t.Fatal(err) + } + if found != nil { + t.Fatalf("non-exact search result was accepted: %#v", found) + } +} + +func TestClientOwnersUsesGroupPermissionQueryAndFiltersIneligibleUsers(t *testing.T) { + t.Parallel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/users/search" { + http.NotFound(w, r) + return + } + query := r.URL.Query() + if query.Get("query") != "*" || query.Get("expand") != "true" || query.Get("permissions[]") != "ticket.agent" || query.Get("group_ids[7]") != "full" || query.Get("page") != "1" || query.Get("per_page") != "100" { + t.Fatalf("unexpected owner query: %s", r.URL.RawQuery) + } + _, _ = w.Write([]byte(`[ + {"id":2,"active":true,"firstname":"Ada","lastname":"Lovelace","group_ids":{"7":["full"]}}, + {"id":6,"active":true,"login":"change-only","group_ids":{"7":["change"]}}, + {"id":3,"active":true,"login":"read-only","group_ids":{"7":["read"]}}, + {"id":4,"active":false,"login":"inactive","group_ids":{"7":["full"]}}, + {"id":5,"active":true,"login":"fallback","group_ids":{"8":["full"]}} + ]`)) + })) + defer server.Close() + + client := NewClient(server.URL, "token", httpTestTransport{client: server.Client()}) + owners, err := client.Owners(context.Background(), 7) + if err != nil { + t.Fatal(err) + } + if len(owners) != 1 || owners[0] != (Owner{ID: 2, Name: "Ada Lovelace"}) { + t.Fatalf("unexpected eligible owners: %#v", owners) + } +} + +func TestClientOwnersDecodesZammad711ExpandedSearchFixture(t *testing.T) { + t.Parallel() + fixture, err := os.ReadFile("testdata/users_search_expand_zammad_7_1_1.json") + if err != nil { + t.Fatal(err) + } + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/users/search" || r.URL.Query().Get("expand") != "true" { + t.Fatalf("unexpected owner request: %s?%s", r.URL.Path, r.URL.RawQuery) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write(fixture) + })) + defer server.Close() + + client := NewClient(server.URL, "token", httpTestTransport{client: server.Client()}) + owners, err := client.Owners(context.Background(), 7) + if err != nil { + t.Fatal(err) + } + if len(owners) != 1 || owners[0] != (Owner{ID: 42, Name: "Ada Lovelace"}) { + t.Fatalf("unexpected owners from Zammad 7.1.1 fixture: %#v", owners) + } +} + +func TestClientOwnersPagesUntilShortPage(t *testing.T) { + t.Parallel() + pageOne := make([]string, 100) + for i := range pageOne { + pageOne[i] = fmt.Sprintf(`{"id":%d,"active":true,"login":"user-%d","group_ids":{"7":["full"]}}`, i+1, i+1) + } + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/users/search" { + http.NotFound(w, r) + return + } + query := r.URL.Query() + if query.Get("query") != "*" || query.Get("expand") != "true" || query.Get("permissions[]") != "ticket.agent" || query.Get("group_ids[7]") != "full" || query.Get("per_page") != "100" { + t.Fatalf("unexpected owner query: %s", r.URL.RawQuery) + } + switch query.Get("page") { + case "1": + _, _ = fmt.Fprintf(w, "[%s]", strings.Join(pageOne, ",")) + case "2": + _, _ = w.Write([]byte(`[{"id":101,"active":true,"firstname":"Grace","lastname":"Hopper","group_ids":{"7":["full"]}}]`)) + default: + t.Fatalf("unexpected page request: %s", query.Get("page")) + } + })) + defer server.Close() + + client := NewClient(server.URL, "token", httpTestTransport{client: server.Client()}) + owners, err := client.Owners(context.Background(), 7) + if err != nil { + t.Fatal(err) + } + if len(owners) != 101 || owners[0] != (Owner{ID: 1, Name: "user-1"}) || owners[100] != (Owner{ID: 101, Name: "Grace Hopper"}) { + t.Fatalf("unexpected paged owners: len=%d first=%#v last=%#v", len(owners), owners[0], owners[len(owners)-1]) + } +} + +func TestClientOwnersPagingHonorsContext(t *testing.T) { + t.Parallel() + pageOne := make([]string, 100) + for i := range pageOne { + pageOne[i] = fmt.Sprintf(`{"id":%d,"active":true,"login":"user-%d","group_ids":{"7":["full"]}}`, i+1, i+1) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + requests := 0 + transport := TransportFunc(func(requestCtx context.Context, _ string, _ string, _ []byte, _ map[string]string) (*Response, error) { + requests++ + if requests == 1 { + cancel() + body := fmt.Sprintf("[%s]", strings.Join(pageOne, ",")) + return &Response{StatusCode: http.StatusOK, Body: []byte(body)}, nil + } + if err := requestCtx.Err(); err != nil { + return nil, err + } + return &Response{StatusCode: http.StatusOK, Body: []byte(`[]`)}, nil + }) + + client := NewClient("https://zammad.example.test", "token", transport) + _, err := client.Owners(ctx, 7) + if err == nil || !errors.Is(err, context.Canceled) { + t.Fatalf("expected context cancellation, got %v", err) + } + if requests != 2 { + t.Fatalf("expected second page request to receive canceled context, got %d requests", requests) + } +} diff --git a/internal/repository/item_list_pagination_test.go b/internal/repository/item_list_pagination_test.go index 0e09342..48cf73e 100644 --- a/internal/repository/item_list_pagination_test.go +++ b/internal/repository/item_list_pagination_test.go @@ -418,7 +418,13 @@ func TestItemListPagePlanKeepsCollectionAndFilteredListsOnSharedPlan(t *testing. if plan.workspaceCountID != workspaceID { t.Fatalf("plain workspace count ID = %d, want %d", plan.workspaceCountID, workspaceID) } - if plan.pageFromClause != "FROM items i " || plan.pageWhereClause != "WHERE i.workspace_id = ?" { + // WI-1528: merged duplicates are redirects, so the direct workspace count + // and page plan must both exclude them. A page that still listed a merged + // row would disagree with the total. + if plan.countQuery != "SELECT COUNT(*) FROM items WHERE workspace_id = ? AND merged_into_item_id IS NULL" { + t.Fatalf("plain workspace count query = %q, want merged-filtered count", plan.countQuery) + } + if plan.pageFromClause != "FROM items i " || plan.pageWhereClause != "WHERE i.workspace_id = ? AND i.merged_into_item_id IS NULL" { t.Fatalf("plain workspace page plan = %#v, want direct items filter", plan) } diff --git a/internal/repository/zammad_observability_scope_test.go b/internal/repository/zammad_observability_scope_test.go new file mode 100644 index 0000000..f60d8b2 --- /dev/null +++ b/internal/repository/zammad_observability_scope_test.go @@ -0,0 +1,72 @@ +//go:build test + +package repository + +import ( + "testing" + + "windshift/internal/models" + "windshift/internal/testutils" +) + +func TestZammadObservabilityFollowsCredentialWorkspaceScope(t *testing.T) { + tdb := testutils.CreateTestDB(t, true) + t.Cleanup(func() { _ = tdb.Close() }) + db := tdb.GetDatabase() + userID := testutils.InsertID(t, db, `INSERT INTO users (email, username, first_name, last_name) VALUES ('zammad-observation@example.test', 'zammad-observation', 'Zammad', 'Observation')`) + workspaceID := testutils.InsertID(t, db, `INSERT INTO workspaces (name, key) VALUES ('Observed workspace', 'ZOBS')`) + itemID := testutils.InsertID(t, db, `INSERT INTO items (workspace_id, workspace_item_number, title, frac_index, creator_id) VALUES (?, 1, 'Observed item', 'a', ?)`, workspaceID, userID) + credentialID := testutils.InsertID(t, db, `INSERT INTO action_credentials (name, credential_type, applies_to_all_workspaces, encrypted_secret, is_enabled) VALUES ('Zammad observation credential', 'custom_header', false, 'synthetic', true)`) + if _, err := db.ExecWrite(`INSERT INTO integration_providers (id, slug, name, provider_type, enabled) VALUES ('zammad-observation', 'zammad-observation', 'Zammad observation', 'zammad', true)`); err != nil { + t.Fatal(err) + } + if _, err := db.ExecWrite(`INSERT INTO zammad_connections (provider_id, credential_id, base_url, default_customer) VALUES ('zammad-observation', ?, 'https://zammad.example.test', 'robot@example.test')`, credentialID); err != nil { + t.Fatal(err) + } + if _, err := db.ExecWrite(`INSERT INTO item_integration_links (id, item_id, integration_provider_id, external_id, external_url, title, link_type, linked_by) VALUES ('zammad-observation-external', ?, 'zammad-observation', '901', 'https://zammad.example.test/#ticket/zoom/901', 'Observed ticket', 'ticket', ?)`, itemID, userID); err != nil { + t.Fatal(err) + } + if _, err := db.ExecWrite(`INSERT INTO zammad_ticket_links (id, item_id, provider_id, item_integration_link_id, ticket_id, ticket_number, correlation_key, sync_state, created_by) VALUES ('zammad-observation-link', ?, 'zammad-observation', 'zammad-observation-external', 901, '901', 'ZOBS-1', ?, ?)`, itemID, models.ZammadSyncFailed, userID); err != nil { + t.Fatal(err) + } + if _, err := db.ExecWrite(`INSERT INTO zammad_ticket_changes (id, ticket_link_id, field_name, new_value_id, new_value_name) VALUES ('zammad-observation-change', 'zammad-observation-link', 'status', 2, 'open')`); err != nil { + t.Fatal(err) + } + + repo := NewZammadRepository(db) + assertVisible := func(want bool) { + t.Helper() + changes, err := repo.ListTicketChangesForItem(itemID, 10) + if err != nil || (len(changes) == 1) != want { + t.Fatalf("item changes: visible=%v rows=%d err=%v", want, len(changes), err) + } + recent, err := repo.ListRecentTicketChangesForWorkspace(workspaceID, 10) + if err != nil || (len(recent) == 1) != want { + t.Fatalf("recent changes: visible=%v rows=%d err=%v", want, len(recent), err) + } + tickets, err := repo.ListOverviewTicketsForWorkspace(workspaceID, 10) + if err != nil || (len(tickets) == 1) != want { + t.Fatalf("overview tickets: visible=%v rows=%d err=%v", want, len(tickets), err) + } + links, err := repo.ListTicketLinksForWorkspace(workspaceID) + if err != nil || (len(links) == 1) != want { + t.Fatalf("overview links: visible=%v rows=%d err=%v", want, len(links), err) + } + failed, uncertain, err := repo.CountProblemTicketLinksForWorkspace(workspaceID) + if err != nil { + t.Fatal(err) + } + wantedFailed := 0 + if want { + wantedFailed = 1 + } + if failed != wantedFailed || uncertain != 0 { + t.Fatalf("problem counts: failed=%d uncertain=%d want failed=%d", failed, uncertain, wantedFailed) + } + } + assertVisible(false) + if _, err := db.ExecWrite(`INSERT INTO action_credential_workspaces (credential_id, workspace_id) VALUES (?, ?)`, credentialID, workspaceID); err != nil { + t.Fatal(err) + } + assertVisible(true) +} diff --git a/internal/repository/zammad_repository_test.go b/internal/repository/zammad_repository_test.go new file mode 100644 index 0000000..9295f49 --- /dev/null +++ b/internal/repository/zammad_repository_test.go @@ -0,0 +1,414 @@ +package repository + +import ( + "errors" + "path/filepath" + "testing" + "time" + + "windshift/internal/database" + "windshift/internal/models" +) + +type zammadLeaseFixture struct { + db database.Database + repo *ZammadRepository + linkID string +} + +func newZammadLeaseFixture(t *testing.T) *zammadLeaseFixture { + t.Helper() + db, err := database.NewSQLiteDB(filepath.Join(t.TempDir(), "zammad-repository.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := db.Initialize(); err != nil { + t.Fatal(err) + } + if _, err := db.ExecWrite(`INSERT INTO users (id, email, username, first_name, last_name) + VALUES (1, 'zammad-repository@example.test', 'zammad-repository', 'Zammad', 'Repository')`); err != nil { + t.Fatal(err) + } + if _, err := db.ExecWrite(`INSERT INTO workspaces (id, name, key) VALUES (1, 'Zammad tests', 'ZRT')`); err != nil { + t.Fatal(err) + } + var statusID int + if err := db.QueryRow(`SELECT id FROM statuses ORDER BY id LIMIT 1`).Scan(&statusID); err != nil { + t.Fatal(err) + } + if _, err := db.ExecWrite(`INSERT INTO items + (id, workspace_id, workspace_item_number, title, description, frac_index, status_id, creator_id, last_active_at) + VALUES (1, 1, 1, 'Zammad repository lease test', '', 'a0', ?, 1, CURRENT_TIMESTAMP)`, statusID); err != nil { + t.Fatal(err) + } + if _, err := db.ExecWrite(`INSERT INTO action_credentials + (id, name, credential_type, applies_to_all_workspaces, encrypted_secret, is_enabled) + VALUES (1, 'Zammad test credential', 'custom_header', true, 'test-ciphertext', true)`); err != nil { + t.Fatal(err) + } + if _, err := db.ExecWrite(`INSERT INTO integration_providers + (id, slug, name, provider_type, enabled) VALUES ('zammad-test', 'zammad-test', 'Zammad test', 'zammad', true)`); err != nil { + t.Fatal(err) + } + if _, err := db.ExecWrite(`INSERT INTO zammad_connections + (provider_id, credential_id, base_url, default_customer) VALUES ('zammad-test', 1, 'https://zammad.example.test', 'robot@example.test')`); err != nil { + t.Fatal(err) + } + if _, err := db.ExecWrite(`INSERT INTO zammad_ticket_links + (id, item_id, provider_id, correlation_key, sync_state, created_by) + VALUES ('zammad-link', 1, 'zammad-test', 'ZRT-1', ?, 1)`, models.ZammadSyncPending); err != nil { + t.Fatal(err) + } + return &zammadLeaseFixture{db: db, repo: NewZammadRepository(db), linkID: "zammad-link"} +} + +func (f *zammadLeaseFixture) makeComplete(t *testing.T) { + t.Helper() + if _, err := f.db.ExecWrite(`INSERT INTO item_integration_links + (id, item_id, integration_provider_id, external_id, external_url, title, link_type, linked_by) + VALUES ('zammad-link-external', '1', 'zammad-test', '901', 'https://zammad.example.test/#ticket/zoom/901', 'Zammad #901', 'ticket', '1')`); err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite(`UPDATE zammad_ticket_links + SET item_integration_link_id = 'zammad-link-external', ticket_id = 901, ticket_number = '901', + sync_state = ? WHERE id = ?`, models.ZammadSyncLinked, f.linkID); err != nil { + t.Fatal(err) + } +} + +func TestZammadCorrelationKeyResolvesCurrentItem(t *testing.T) { + f := newZammadLeaseFixture(t) + + itemID, workspaceID, err := f.repo.GetItemDestinationByCorrelationKey("zammad-test", "ZRT-1") + if err != nil || itemID != 1 || workspaceID != 1 { + t.Fatalf("resolve correlation key: item=%d workspace=%d err=%v", itemID, workspaceID, err) + } + if _, _, err := f.repo.GetItemDestinationByCorrelationKey("zammad-test", "missing"); !errors.Is(err, ErrNotFound) { + t.Fatalf("missing correlation key must fail closed: %v", err) + } +} + +func TestZammadItemTicketLinksIncludeRemoteTitleAndConfiguredClosedState(t *testing.T) { + f := newZammadLeaseFixture(t) + f.makeComplete(t) + if _, err := f.db.ExecWrite(`UPDATE zammad_connections + SET closed_state_ids = '[4]' + WHERE provider_id = 'zammad-test'`); err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite(`UPDATE item_integration_links SET title = 'VPN access unavailable' + WHERE id = 'zammad-link-external'`); err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite(`UPDATE zammad_ticket_links + SET last_status_id = 4, last_status_name = 'closed' + WHERE id = ?`, f.linkID); err != nil { + t.Fatal(err) + } + + links, err := f.repo.GetTicketLinksForItem(1) + if err != nil { + t.Fatal(err) + } + if len(links) != 1 || links[0].TicketTitle != "VPN access unavailable" || !links[0].Closed { + t.Fatalf("item ticket projection is incomplete: %#v", links) + } + response := links[0].ItemResponse() + if response.TicketTitle != "VPN access unavailable" || !response.Closed { + t.Fatalf("item ticket response lost display metadata: %#v", response) + } +} + +func TestZammadOverviewTicketsAreCurrentOrderedAndWorkspaceScoped(t *testing.T) { + f := newZammadLeaseFixture(t) + f.makeComplete(t) + if _, err := f.db.ExecWrite(`UPDATE item_integration_links SET title = 'Persisted remote title' + WHERE id = 'zammad-link-external'`); err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite(`UPDATE zammad_ticket_links SET + last_status_id = 2, last_status_name = 'open', group_id = 7, group_name = 'Support', + owner_id = 9, owner_name = 'Ada Agent', last_synced_at = '2026-08-30 10:00:00' + WHERE id = ?`, f.linkID); err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite(`INSERT INTO items + (id, workspace_id, workspace_item_number, title, description, frac_index, status_id, creator_id, last_active_at) + SELECT 2, 1, 2, 'Newer item', '', 'a1', status_id, 1, CURRENT_TIMESTAMP FROM items WHERE id = 1`); err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite(`INSERT INTO item_integration_links + (id, item_id, integration_provider_id, external_id, external_url, title, link_type, linked_by) + VALUES ('overview-newer-external', '2', 'zammad-test', '902', 'https://zammad.example.test/#ticket/zoom/902', '', 'ticket', '1')`); err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite(`INSERT INTO zammad_ticket_links + (id, item_id, provider_id, item_integration_link_id, ticket_id, ticket_number, ticket_url, + group_id, group_name, owner_id, owner_name, correlation_key, sync_state, + last_status_id, last_status_name, last_synced_at, created_by) + VALUES ('overview-newer', 2, 'zammad-test', 'overview-newer-external', 902, '902', + 'https://zammad.example.test/#ticket/zoom/902', 8, 'Escalations', 1, 'Unassigned', + 'ZRT-2', ?, 3, 'pending', '2026-08-31 10:00:00', 1)`, models.ZammadSyncLinked); err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite(`INSERT INTO workspaces (id, name, key) VALUES (2, 'Other workspace', 'OTH')`); err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite(`INSERT INTO items + (id, workspace_id, workspace_item_number, title, description, frac_index, status_id, creator_id, last_active_at) + SELECT 3, 2, 1, 'Other item', '', 'a2', status_id, 1, CURRENT_TIMESTAMP FROM items WHERE id = 1`); err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite(`INSERT INTO item_integration_links + (id, item_id, integration_provider_id, external_id, external_url, title, link_type, linked_by) + VALUES ('overview-other-external', '3', 'zammad-test', '903', 'https://zammad.example.test/#ticket/zoom/903', 'Other title', 'ticket', '1')`); err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite(`INSERT INTO zammad_ticket_links + (id, item_id, provider_id, item_integration_link_id, ticket_id, ticket_number, correlation_key, sync_state, created_by) + VALUES ('overview-other', 3, 'zammad-test', 'overview-other-external', 903, '903', 'OTH-1', ?, 1)`, models.ZammadSyncLinked); err != nil { + t.Fatal(err) + } + + tickets, err := f.repo.ListOverviewTicketsForWorkspace(1, 100) + if err != nil { + t.Fatal(err) + } + if len(tickets) != 2 { + t.Fatalf("overview tickets crossed workspace scope or omitted a complete link: %#v", tickets) + } + if tickets[0].ID != "overview-newer" || tickets[0].ItemID != 2 || tickets[0].ItemKey != "ZRT-2" || tickets[0].TicketTitle != "Zammad #902" || + tickets[0].Status.ID != 3 || tickets[0].Status.Name != "pending" || + tickets[0].Group.ID != 8 || tickets[0].Group.Name != "Escalations" || + tickets[0].Owner.ID != 1 || tickets[0].Owner.Name != "Unassigned" { + t.Fatalf("newest overview ticket projection is incomplete: %#v", tickets[0]) + } + if tickets[1].ItemID != 1 || tickets[1].TicketTitle != "Persisted remote title" || + tickets[1].TicketNumber != "901" || tickets[1].Owner.Name != "Ada Agent" { + t.Fatalf("persisted remote title or current snapshot was lost: %#v", tickets[1]) + } +} + +func TestZammadSyncLeaseOwnerTakeoverProtectsSnapshotReleaseAndDelete(t *testing.T) { + f := newZammadLeaseFixture(t) + f.makeComplete(t) + now := time.Now().UTC() + + claimed, err := f.repo.ClaimSync(f.linkID, "owner-a", now.Add(-time.Minute)) + if err != nil || !claimed { + t.Fatalf("claim owner A: claimed=%v err=%v", claimed, err) + } + claimed, err = f.repo.ClaimSync(f.linkID, "owner-b", now.Add(time.Minute)) + if err != nil || !claimed { + t.Fatalf("take over expired lease as owner B: claimed=%v err=%v", claimed, err) + } + + if err := f.repo.ReleaseSyncClaim(f.linkID, "owner-a"); !errors.Is(err, ErrConcurrentUpdate) { + t.Fatalf("owner A must not release owner B lease: %v", err) + } + if err := f.repo.UpdateTicketLinkSync(f.linkID, "owner-a", 4, "closed", 7, "Support", 0, "", "", "", now, true, true); !errors.Is(err, ErrConcurrentUpdate) { + t.Fatalf("owner A must not persist a snapshot after takeover: %v", err) + } + if err := f.repo.DeleteTicketLinkClaimed(f.linkID, "owner-a"); !errors.Is(err, ErrConcurrentUpdate) { + t.Fatalf("owner A must not delete owner B link: %v", err) + } + renewed, err := f.repo.RenewSyncClaim(f.linkID, "owner-a", now.Add(2*time.Minute)) + if err != nil || renewed { + t.Fatalf("owner A must not renew owner B lease: renewed=%v err=%v", renewed, err) + } + + var lockOwner string + var ticketID, statusID int + if err := f.db.QueryRow(`SELECT sync_lock_owner, ticket_id, COALESCE(last_status_id, 0) + FROM zammad_ticket_links WHERE id = ?`, f.linkID).Scan(&lockOwner, &ticketID, &statusID); err != nil { + t.Fatal(err) + } + if lockOwner != "owner-b" || ticketID != 901 || statusID != 0 { + t.Fatalf("stale owner changed stored link: owner=%q ticket=%d status=%d", lockOwner, ticketID, statusID) + } + var genericCount int + if err := f.db.QueryRow(`SELECT COUNT(*) FROM item_integration_links WHERE id = 'zammad-link-external'`).Scan(&genericCount); err != nil { + t.Fatal(err) + } + if genericCount != 1 { + t.Fatalf("stale owner deleted generic link: count=%d", genericCount) + } + if err := f.repo.ReleaseSyncClaim(f.linkID, "owner-b"); err != nil { + t.Fatalf("release current lease owner: %v", err) + } +} + +func TestZammadSQLiteTimePredicatesNormalizeNonUTCTimestamps(t *testing.T) { + f := newZammadLeaseFixture(t) + f.makeComplete(t) + localNow := time.Now().In(time.FixedZone("UTC+14", 14*60*60)) + + if _, err := f.db.ExecWrite(`UPDATE zammad_ticket_links SET next_attempt_at = ? WHERE id = ?`, localNow.Add(-time.Minute), f.linkID); err != nil { + t.Fatal(err) + } + due, err := f.repo.ListDueTicketLinks(localNow.Add(time.Minute), 10) + if err != nil || len(due) != 1 || due[0].ID != f.linkID { + t.Fatalf("non-UTC retry timestamp was not compared in UTC: links=%#v err=%v", due, err) + } + + claimed, err := f.repo.ClaimSync(f.linkID, "expired-owner", localNow.Add(-time.Minute)) + if err != nil || !claimed { + t.Fatalf("claim expired non-UTC lease: claimed=%v err=%v", claimed, err) + } + claimed, err = f.repo.ClaimSync(f.linkID, "replacement-owner", localNow.Add(time.Minute)) + if err != nil || !claimed { + t.Fatalf("take over expired non-UTC lease: claimed=%v err=%v", claimed, err) + } + + if err := f.repo.CreateOAuthState("non-utc-state", "zammad-test", 1, 1, localNow.Add(time.Minute)); err != nil { + t.Fatal(err) + } + state, err := f.repo.ConsumeOAuthState("non-utc-state") + if err != nil || state.ProviderID != "zammad-test" || state.InitiatedBy != 1 { + t.Fatalf("non-UTC OAuth expiry was not compared in UTC: state=%#v err=%v", state, err) + } + + if err := f.repo.UpsertOAuthToken(ZammadOAuthToken{ + ProviderID: "zammad-test", OAuthGeneration: 1, ExpiresAt: localNow.Add(time.Hour), + }); err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite(`UPDATE zammad_oauth_tokens + SET refresh_lock_until = ?, refresh_claim_owner = ? WHERE provider_id = ?`, + localNow.Add(-time.Minute), "expired-oauth-owner", "zammad-test"); err != nil { + t.Fatal(err) + } + claimed, err = f.repo.ClaimOAuthRefresh("zammad-test", 1, "replacement-oauth-owner", localNow.Add(time.Minute)) + if err != nil || !claimed { + t.Fatalf("take over expired non-UTC OAuth lease: claimed=%v err=%v", claimed, err) + } + if _, err := f.repo.GetOAuthTokenForRefreshClaim("zammad-test", 1, "replacement-oauth-owner"); err != nil { + t.Fatalf("read active non-UTC OAuth lease: %v", err) + } + if err := database.WithTx(f.db, func(tx database.Tx) error { + guarded, err := f.repo.GuardOAuthRefreshClaimTx(tx, "zammad-test", 1, "replacement-oauth-owner") + if err != nil { + return err + } + if !guarded { + t.Fatal("active non-UTC OAuth lease was not guarded") + } + return nil + }); err != nil { + t.Fatal(err) + } +} + +func TestZammadTicketCompletionRequiresCurrentLeaseOwner(t *testing.T) { + f := newZammadLeaseFixture(t) + now := time.Now().UTC() + + claimed, wasUncertain, err := f.repo.ClaimTicketCreation(f.linkID, "owner-a", now, now.Add(-time.Minute)) + if err != nil || !claimed { + t.Fatalf("claim ticket creation as owner A: claimed=%v err=%v", claimed, err) + } + if wasUncertain { + t.Fatal("pending ticket creation must not report uncertain state") + } + claimed, err = f.repo.ClaimSync(f.linkID, "owner-b", now.Add(time.Minute)) + if err != nil || !claimed { + t.Fatalf("take over ticket creation lease as owner B: claimed=%v err=%v", claimed, err) + } + + if err := f.repo.CompleteTicketCreation(f.linkID, "owner-a", 902, "902", "Synthetic ticket", "https://zammad.example.test/#ticket/zoom/902", 2, "open", 7, "Support", 0, "", 1); !errors.Is(err, ErrConcurrentUpdate) { + t.Fatalf("stale owner must not complete ticket creation: %v", err) + } + var genericCount int + if err := f.db.QueryRow(`SELECT COUNT(*) FROM item_integration_links WHERE id = 'zammad-link-external'`).Scan(&genericCount); err != nil { + t.Fatal(err) + } + if genericCount != 0 { + t.Fatalf("stale owner created generic item link: count=%d", genericCount) + } + + if err := f.repo.CompleteTicketCreation(f.linkID, "owner-b", 902, "902", "Synthetic ticket", "https://zammad.example.test/#ticket/zoom/902", 2, "open", 7, "Support", 0, "", 1); err != nil { + t.Fatalf("current owner completes ticket creation: %v", err) + } + var ticketID int + var lockOwner any + if err := f.db.QueryRow(`SELECT ticket_id, sync_lock_owner FROM zammad_ticket_links WHERE id = ?`, f.linkID).Scan(&ticketID, &lockOwner); err != nil { + t.Fatal(err) + } + if ticketID != 902 || lockOwner != nil { + t.Fatalf("completion did not atomically store ticket and clear lease: ticket=%d owner=%v", ticketID, lockOwner) + } +} + +func TestZammadExistingTicketCompletionRequiresCurrentLeaseOwner(t *testing.T) { + f := newZammadLeaseFixture(t) + now := time.Now().UTC() + claimed, _, err := f.repo.ClaimTicketCreation(f.linkID, "owner-a", now, now.Add(-time.Minute)) + if err != nil || !claimed { + t.Fatalf("claim existing-ticket reservation as owner A: claimed=%v err=%v", claimed, err) + } + claimed, err = f.repo.ClaimSync(f.linkID, "owner-b", now.Add(time.Minute)) + if err != nil || !claimed { + t.Fatalf("take over existing-ticket reservation as owner B: claimed=%v err=%v", claimed, err) + } + ticket := &models.ZammadTicketLink{ + TicketID: 903, TicketNumber: "903", TicketURL: "https://zammad.example.test/#ticket/zoom/903", + GroupID: 7, GroupName: "Support", LastStatusID: 2, LastStatusName: "open", + } + if err := f.repo.CompleteExistingTicketLink(f.linkID, "owner-a", f.linkID+"-external", ticket, "Synthetic ticket", 1); !errors.Is(err, ErrConcurrentUpdate) { + t.Fatalf("stale owner must not complete existing ticket link: %v", err) + } + if err := f.repo.CompleteExistingTicketLink(f.linkID, "owner-b", f.linkID+"-external", ticket, "Synthetic ticket", 1); err != nil { + t.Fatalf("current owner completes existing ticket link: %v", err) + } +} + +func TestZammadExistingTicketCompletionReusesPreexistingGenericLinkID(t *testing.T) { + f := newZammadLeaseFixture(t) + if _, err := f.db.ExecWrite(`INSERT INTO item_integration_links + (id, item_id, integration_provider_id, external_id, external_url, title, link_type, linked_by) + VALUES ('preexisting-generic', '1', 'zammad-test', '903', 'https://old.example.test', 'Old title', 'ticket', '1')`); err != nil { + t.Fatal(err) + } + claimed, err := f.repo.ClaimSync(f.linkID, "owner", time.Now().Add(time.Minute)) + if err != nil || !claimed { + t.Fatalf("claim existing-ticket reservation: claimed=%v err=%v", claimed, err) + } + ticket := &models.ZammadTicketLink{ + TicketID: 903, TicketNumber: "903", TicketURL: "https://zammad.example.test/#ticket/zoom/903", + GroupID: 7, GroupName: "Support", LastStatusID: 2, LastStatusName: "open", + } + if err := f.repo.CompleteExistingTicketLink(f.linkID, "owner", f.linkID+"-external", ticket, "Synthetic ticket", 1); err != nil { + t.Fatalf("complete against preexisting generic link: %v", err) + } + var genericID string + if err := f.db.QueryRow(`SELECT item_integration_link_id FROM zammad_ticket_links WHERE id = ?`, f.linkID).Scan(&genericID); err != nil { + t.Fatal(err) + } + if genericID != "preexisting-generic" { + t.Fatalf("completion stored generated generic ID %q instead of existing row", genericID) + } +} + +func TestZammadTicketCreationClaimReturnsAtomicUncertainState(t *testing.T) { + for _, tt := range []struct { + name string + state models.ZammadSyncState + startedAt any + }{ + {name: "explicit uncertain", state: models.ZammadSyncUncertain}, + {name: "stale in-flight create", state: models.ZammadSyncCreating, startedAt: time.Now().UTC().Add(-3 * time.Minute)}, + } { + t.Run(tt.name, func(t *testing.T) { + f := newZammadLeaseFixture(t) + if _, err := f.db.ExecWrite(`UPDATE zammad_ticket_links SET sync_state = ?, creating_started_at = ? WHERE id = ?`, tt.state, tt.startedAt, f.linkID); err != nil { + t.Fatal(err) + } + now := time.Now().UTC() + claimed, wasUncertain, err := f.repo.ClaimTicketCreation(f.linkID, "owner-a", now, now.Add(time.Minute)) + if err != nil || !claimed || !wasUncertain { + t.Fatalf("unsafe retry claim must report uncertain state: claimed=%v uncertain=%v err=%v", claimed, wasUncertain, err) + } + }) + } +} diff --git a/internal/services/email_reply_service_test.go b/internal/services/email_reply_service_test.go index 765cc99..90134f5 100644 --- a/internal/services/email_reply_service_test.go +++ b/internal/services/email_reply_service_test.go @@ -277,13 +277,16 @@ func TestEmailReplyService_SkipsItemWithoutPortalCustomer(t *testing.T) { } } -func TestEmailReplyService_SkipsNonEmailChannel(t *testing.T) { +// TestEmailReplyService_SendsForPortalChannelTicket is the WI-1546 core +// behavior: an agent comment on a portal-originated ticket emails the +// customer, threading from the anchor (lazily ensured for tickets created +// before anchors existed), with the outbox row on the ticket's origin channel. +func TestEmailReplyService_SendsForPortalChannelTicket(t *testing.T) { db := createCommentTestDB(t) env := setupCommentTestEnv(t, db) mock := &mockSMTPSender{configured: true} svc := services.NewEmailReplyService(db, mock) - // Create a portal channel (not email type) chID := testutils.InsertID(t, db, ` INSERT INTO channels (name, type, direction, status) VALUES ('Portal', 'portal', 'inbound', 'enabled') @@ -304,22 +307,68 @@ func TestEmailReplyService_SkipsNonEmailChannel(t *testing.T) { t.Fatalf("Failed to load portal customer id: %v", err) } - _, err = db.Exec("UPDATE items SET channel_id = ?, creator_portal_customer_id = ? WHERE id = ?", chID, pcID, env.ItemID) + _, err = db.Exec("UPDATE items SET channel_id = ?, creator_portal_customer_id = ?, title = 'Printer on fire' WHERE id = ?", chID, pcID, env.ItemID) if err != nil { t.Fatalf("Failed to update item: %v", err) } + insertTestComment(t, db, 1, env.ItemID, env.UserID) err = svc.HandleCommentCreated(services.HandleCommentParams{ CommentID: 1, ItemID: env.ItemID, AuthorID: env.UserID, - Content: "test", + Content: "On it — checking the fuser.", }) if err != nil { t.Fatalf("Expected nil error, got: %v", err) } - if len(mock.sent) != 0 { - t.Error("Expected no email sent for non-email channel") + if len(mock.sent) != 1 { + t.Fatalf("Expected 1 email sent for portal-channel ticket, got %d", len(mock.sent)) + } + + // Threading: In-Reply-To is the lazily ensured anchor; the subject comes + // from the ticket title (the portal ticket has no inbound email subject). + anchorID := fmt.Sprintf("", env.ItemID) + if mock.sent[0].InReplyTo != anchorID { + t.Errorf("InReplyTo = %q, want anchor %q", mock.sent[0].InReplyTo, anchorID) + } + if mock.sent[0].Subject != "Re: Printer on fire" { + t.Errorf("Subject = %q, want %q", mock.sent[0].Subject, "Re: Printer on fire") + } + + // Exactly two tracking rows for the item now: the anchor and the recorded + // outbound reply, both on the origin (portal) channel. + var rows int + var channels int + if err := db.QueryRow(` + SELECT COUNT(*), COUNT(DISTINCT channel_id) FROM email_message_tracking WHERE item_id = ? + `, env.ItemID).Scan(&rows, &channels); err != nil { + t.Fatalf("count tracking rows: %v", err) + } + if rows != 2 || channels != 1 { + t.Fatalf("tracking rows = %d across %d channel(s), want 2 rows on 1 (portal) channel", rows, channels) + } + var outboundChannel int + if err := db.QueryRow(` + SELECT channel_id FROM email_message_tracking + WHERE item_id = ? AND comment_id IS NOT NULL + `, env.ItemID).Scan(&outboundChannel); err != nil { + t.Fatalf("load outbound tracking: %v", err) + } + if outboundChannel != chID { + t.Fatalf("outbound tracking channel = %d, want origin portal channel %d", outboundChannel, chID) + } + + // The anchor must not grant thread participation to anyone. + var anchoredFrom string + if err := db.QueryRow(` + SELECT from_email FROM email_message_tracking + WHERE item_id = ? AND comment_id IS NULL + `, env.ItemID).Scan(&anchoredFrom); err != nil { + t.Fatalf("load anchor: %v", err) + } + if anchoredFrom != "" { + t.Fatalf("anchor from_email = %q, want empty", anchoredFrom) } } @@ -392,9 +441,9 @@ func TestEmailReplyService_SendsEmailForInternalUserComment(t *testing.T) { t.Errorf("Expected References to contain original message ID, got %v", sent.References) } - // Verify Message-ID format - if !strings.HasPrefix(sent.MessageID, "") { t.Errorf("Expected MessageID to end with '>', got '%s'", sent.MessageID) @@ -450,8 +499,8 @@ func TestEmailReplyService_RecordsOutboundTracking(t *testing.T) { if err != nil { t.Fatalf("Failed to query outbound message_id: %v", err) } - if !strings.HasPrefix(msgID, " 0 { + f.ticket["group_id"] = f.putRerouteGroupID + } + } + return jsonResponse(http.StatusOK, f.ticket), nil + case strings.HasPrefix(parsed.Path, "/api/v1/tickets/"): + if f.getStatus != 0 { + return jsonResponse(f.getStatus, map[string]string{"error": "synthetic remote detail"}), nil + } + if f.getTicket != nil { + return jsonResponse(http.StatusOK, f.getTicket), nil + } + return jsonResponse(http.StatusOK, f.ticket), nil + case parsed.Path == "/api/v1/ticket_states": + if f.states != nil { + return jsonResponse(http.StatusOK, f.states), nil + } + return jsonResponse(http.StatusOK, []map[string]any{{"id": 2, "name": "open", "active": true}, {"id": 4, "name": "closed", "active": true}}), nil + case parsed.Path == "/api/v1/users/search": + return jsonResponse(http.StatusOK, f.users), nil + default: + return jsonResponse(http.StatusNotFound, map[string]string{"error": "not found"}), nil + } +} + +func (f *fakeZammadTransport) counts() (requests, posts int) { + f.mu.Lock() + defer f.mu.Unlock() + return f.requests, f.posts +} + +func (f *fakeZammadTransport) putCount() int { + f.mu.Lock() + defer f.mu.Unlock() + return f.puts +} + +func (f *fakeZammadTransport) lastPutPayload() map[string]any { + f.mu.Lock() + defer f.mu.Unlock() + if len(f.putPayloads) == 0 { + return nil + } + return f.putPayloads[len(f.putPayloads)-1] +} + +func (f *fakeZammadTransport) resetRequests() { + f.mu.Lock() + defer f.mu.Unlock() + f.requests = 0 +} + +func jsonResponse(status int, value any) *zammad.Response { + body, _ := json.Marshal(value) + return &zammad.Response{StatusCode: status, Body: body} +} + +type allowZammadPermission struct{} + +func (allowZammadPermission) HasWorkspacePermission(_, _ int, _ string) (bool, error) { + return true, nil +} + +type recordingItemChangePublisher struct { + itemID int + kind ItemChangeKind +} + +func (p *recordingItemChangePublisher) PublishItemChange(itemID int, kind ItemChangeKind) { + p.itemID = itemID + p.kind = kind +} + +type fakeZammadWorkflow struct { + db database.Database + writes int + postCommitErr error +} + +func (f *fakeZammadWorkflow) PerformTransition(_ context.Context, req PerformTransitionRequest, itemRepo *repository.ItemRepository, _ *ConditionService, _ transitionApprovalService) (*PerformTransitionResult, error) { + item, err := itemRepo.FindByIDWithDetails(req.ItemID) + if err != nil { + return nil, err + } + if item.StatusID != nil && *item.StatusID == req.ToStatusID { + return &PerformTransitionResult{Item: item, OldStatusID: item.StatusID, NewStatusID: item.StatusID, NoOp: true}, nil + } + oldStatusID := item.StatusID + if _, err := f.db.ExecWrite("UPDATE items SET status_id = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?", req.ToStatusID, req.ItemID); err != nil { + return nil, err + } + f.writes++ + item, err = itemRepo.FindByIDWithDetails(req.ItemID) + if err != nil { + return nil, err + } + newStatusID := req.ToStatusID + result := &PerformTransitionResult{Item: item, OldStatusID: oldStatusID, NewStatusID: &newStatusID} + if f.postCommitErr != nil { + return result, f.postCommitErr + } + return result, nil +} + +type zammadServiceFixture struct { + t *testing.T + db database.Database + service *ZammadService + credentials *ActionCredentialService + transport *fakeZammadTransport + connection *models.ZammadConnection + workspace1 int + workspace2 int + item1 int + item2 int + actorID int + openStatus int + doneStatus int +} + +func newZammadServiceFixture(t *testing.T, workflow zammadWorkflowTransitioner) *zammadServiceFixture { + t.Helper() + db, err := database.NewSQLiteDB(t.TempDir() + "/windshift.db") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := db.Initialize(); err != nil { + t.Fatal(err) + } + f := &zammadServiceFixture{t: t, db: db, transport: &fakeZammadTransport{}} + f.actorID = mustInsertID(t, db, `INSERT INTO users (email, username, first_name, last_name) VALUES (?, ?, ?, ?)`, "agent@example.test", "zammad-agent", "Zammad", "Agent") + f.workspace1 = mustInsertID(t, db, `INSERT INTO workspaces (name, key) VALUES (?, ?)`, "Primary", "PRI") + f.workspace2 = mustInsertID(t, db, `INSERT INTO workspaces (name, key) VALUES (?, ?)`, "Other", "OTH") + if err := db.QueryRow("SELECT id FROM statuses ORDER BY id LIMIT 1").Scan(&f.openStatus); err != nil { + t.Fatal(err) + } + if err := db.QueryRow("SELECT id FROM statuses ORDER BY id DESC LIMIT 1").Scan(&f.doneStatus); err != nil { + t.Fatal(err) + } + f.item1 = mustInsertID(t, db, `INSERT INTO items (workspace_id, workspace_item_number, title, description, frac_index, status_id, creator_id, last_active_at) VALUES (?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP)`, f.workspace1, 49, "Synthetic ticket source", "Synthetic description", "a0", f.openStatus, f.actorID) + f.item2 = mustInsertID(t, db, `INSERT INTO items (workspace_id, workspace_item_number, title, description, frac_index, status_id, creator_id, last_active_at) VALUES (?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP)`, f.workspace2, 1, "Out of scope", "Synthetic description", "a1", f.openStatus, f.actorID) + credentialService := NewActionCredentialService(repository.NewActionCredentialRepository(db), "synthetic-server-secret-for-zammad-tests") + f.credentials = credentialService + if workflow == nil { + workflow = &fakeZammadWorkflow{db: db} + } + f.service = NewZammadService(db, repository.NewZammadRepository(db), credentialService, allowZammadPermission{}, workflow, nil, nil) + f.service.SetTransportForTesting(f.transport) + f.connection, err = f.service.CreateConnection(models.CreateZammadConnectionRequest{ + Slug: "helpdesk", Name: "Synthetic helpdesk", BaseURL: "https://zammad.example.test", + APIToken: "synthetic-zammad-token", DefaultGroupID: 7, DefaultGroupName: "Support", + DefaultCustomer: "robot@example.test", CorrelationField: "windshift_item_key", + WorkspaceIDs: []int{f.workspace1}, + }, f.actorID) + if err != nil { + t.Fatal(err) + } + return f +} + +func mustInsertID(t *testing.T, db database.Database, query string, args ...any) int { + t.Helper() + result, err := db.ExecWrite(query, args...) + if err != nil { + t.Fatal(err) + } + id, err := result.LastInsertId() + if err != nil { + t.Fatal(err) + } + return int(id) +} + +func TestZammadCreateTicketIsIdempotentAndPersistsGenericLink(t *testing.T) { + f := newZammadServiceFixture(t, nil) + first, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + second, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + _, posts := f.transport.counts() + if posts != 1 || first.TicketID != 901 || second.ID != first.ID { + t.Fatalf("unexpected idempotency result: first=%#v second=%#v posts=%d", first, second, posts) + } + var genericID string + if err := f.db.QueryRow("SELECT item_integration_link_id FROM zammad_ticket_links WHERE id = ?", first.ID).Scan(&genericID); err != nil { + t.Fatal(err) + } + var count int + var title string + if err := f.db.QueryRow("SELECT COUNT(*), MAX(title) FROM item_integration_links WHERE id = ? AND external_id = ?", genericID, strconv.Itoa(first.TicketID)).Scan(&count, &title); err != nil || count != 1 { + t.Fatalf("generic link was not persisted: count=%d err=%v", count, err) + } + if title != "[PRI-49] Synthetic ticket source" { + t.Fatalf("generic link did not retain the Zammad ticket title: %q", title) + } +} + +func TestZammadLinkExistingTicketPinsExactTicketAndIsIdempotent(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.ticket = map[string]any{ + "id": 711, "number": "420711", "title": "Existing VPN request", "group_id": 7, "group": "Support", + "state_id": 2, "state": "open", "owner_id": 99, + } + f.transport.users = []map[string]any{{"id": 99, "active": true, "firstname": "Grace", "lastname": "Hopper", "group_ids": map[string][]string{"7": {"full"}}}} + + first, err := f.service.LinkExistingTicket(context.Background(), f.item1, f.actorID, models.LinkZammadTicketRequest{ + ConnectionID: f.connection.ProviderID, TicketNumber: "420711", + }) + if err != nil { + t.Fatal(err) + } + second, err := f.service.LinkExistingTicket(context.Background(), f.item1, f.actorID, models.LinkZammadTicketRequest{ + ConnectionID: f.connection.ProviderID, TicketNumber: "420711", + }) + if err != nil { + t.Fatal(err) + } + if first.ID != second.ID || first.TicketID != 711 || first.SyncState != models.ZammadSyncLinked || f.transport.putCount() != 1 { + t.Fatalf("existing-ticket link was not idempotent: first=%#v second=%#v puts=%d", first, second, f.transport.putCount()) + } + if first.OwnerID != 99 || first.OwnerName != "Grace Hopper" { + t.Fatalf("existing-ticket owner was not resolved: %#v", first) + } + if got := f.transport.lastPutPayload()["windshift_item_key"]; got != first.CorrelationKey { + t.Fatalf("remote correlation was not pinned to the local link: %#v", f.transport.lastPutPayload()) + } +} + +func TestZammadLinkExistingTicketSetupFailureCannotBePromotedBySync(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.ticket = map[string]any{ + "id": 715, "number": "420715", "group_id": 7, "group": "Support", + "state_id": 2, "state": "open", "owner_id": 1, + } + f.transport.putError = context.DeadlineExceeded + + _, err := f.service.LinkExistingTicket(context.Background(), f.item1, f.actorID, models.LinkZammadTicketRequest{ + ConnectionID: f.connection.ProviderID, TicketNumber: "420715", + }) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected correlation update failure, got %v", err) + } + reserved, err := f.service.TicketLinksForItem(f.item1) + if err != nil || len(reserved) != 1 { + t.Fatalf("failed setup did not retain exactly one reservation: links=%#v err=%v", reserved, err) + } + link := reserved[0] + if link.SyncState != models.ZammadSyncCreating || link.ItemIntegrationLinkID != "" || link.LastError == "" { + t.Fatalf("failed setup looked complete or lost its safe error: %#v", link) + } + var genericLinks int + if err := f.db.QueryRow("SELECT COUNT(*) FROM item_integration_links WHERE integration_provider_id = ?", f.connection.ProviderID).Scan(&genericLinks); err != nil || genericLinks != 0 { + t.Fatalf("failed setup created a generic integration link: count=%d err=%v", genericLinks, err) + } + + f.transport.resetRequests() + if err := f.service.SyncDue(context.Background(), 10); err != nil { + t.Fatal(err) + } + requests, _ := f.transport.counts() + if requests != 0 { + t.Fatalf("scheduler attempted to sync incomplete reservation: requests=%d", requests) + } + if _, err := f.service.SyncTicketLink(context.Background(), link.ID); err == nil { + t.Fatal("manual refresh promoted an incomplete reservation") + } + closedStateID := 4 + if _, err := f.service.UpdateTicketLink(context.Background(), link.ID, models.UpdateZammadTicketLinkRequest{StateID: &closedStateID}); err == nil { + t.Fatal("ticket edit was allowed before link setup completed") + } + stored, err := f.service.GetTicketLink(link.ID) + if err != nil || stored.SyncState != models.ZammadSyncCreating || stored.ItemIntegrationLinkID != "" { + t.Fatalf("sync attempts changed incomplete reservation: link=%#v err=%v", stored, err) + } + + f.transport.putError = nil + completed, err := f.service.LinkExistingTicket(context.Background(), f.item1, f.actorID, models.LinkZammadTicketRequest{ + ConnectionID: f.connection.ProviderID, TicketNumber: "420715", + }) + if err != nil { + t.Fatal(err) + } + if completed.ID != link.ID || completed.SyncState != models.ZammadSyncLinked || completed.ItemIntegrationLinkID == "" || completed.LastError != "" { + t.Fatalf("retry did not finish the retained reservation: %#v", completed) + } +} + +func TestZammadLinkExistingTicketRejectsDisallowedGroupAndConflictingCorrelation(t *testing.T) { + t.Run("disallowed group", func(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.ticket = map[string]any{"id": 712, "number": "420712", "group_id": 99, "state_id": 2, "state": "open"} + _, err := f.service.LinkExistingTicket(context.Background(), f.item1, f.actorID, models.LinkZammadTicketRequest{ConnectionID: f.connection.ProviderID, TicketNumber: "420712"}) + var validationErr *ZammadValidationError + if !errors.As(err, &validationErr) || f.transport.putCount() != 0 { + t.Fatalf("disallowed ticket group was accepted: err=%v puts=%d", err, f.transport.putCount()) + } + }) + t.Run("conflicting correlation", func(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.ticket = map[string]any{ + "id": 713, "number": "420713", "group_id": 7, "state_id": 2, "state": "open", + "windshift_item_key": "windshift:other-connection:PRI-49", + } + _, err := f.service.LinkExistingTicket(context.Background(), f.item1, f.actorID, models.LinkZammadTicketRequest{ConnectionID: f.connection.ProviderID, TicketNumber: "420713"}) + var validationErr *ZammadValidationError + if !errors.As(err, &validationErr) || f.transport.putCount() != 0 { + t.Fatalf("conflicting remote correlation was accepted: err=%v puts=%d", err, f.transport.putCount()) + } + var links int + if err := f.db.QueryRow("SELECT COUNT(*) FROM zammad_ticket_links").Scan(&links); err != nil || links != 0 { + t.Fatalf("conflicting ticket left a local link behind: links=%d err=%v", links, err) + } + }) +} + +func TestZammadLinkExistingTicketDoesNotStealAlreadyReservedRemoteTicket(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.ticket = map[string]any{"id": 714, "number": "420714", "group_id": 7, "state_id": 2, "state": "open"} + first, err := f.service.LinkExistingTicket(context.Background(), f.item1, f.actorID, models.LinkZammadTicketRequest{ConnectionID: f.connection.ProviderID, TicketNumber: "420714"}) + if err != nil { + t.Fatal(err) + } + secondItem := mustInsertID(t, f.db, `INSERT INTO items (workspace_id, workspace_item_number, title, description, frac_index, status_id, creator_id, last_active_at) VALUES (?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP)`, f.workspace1, 50, "Second ticket source", "Synthetic description", "a2", f.openStatus, f.actorID) + // Simulate a remote-side correlation edit between attempts. The local unique + // provider/ticket reservation must still prevent the second item from taking + // over this ticket. + f.transport.ticket["windshift_item_key"] = "" + puts := f.transport.putCount() + _, err = f.service.LinkExistingTicket(context.Background(), secondItem, f.actorID, models.LinkZammadTicketRequest{ConnectionID: f.connection.ProviderID, TicketNumber: "420714"}) + if !errors.Is(err, repository.ErrDuplicateEntry) || f.transport.putCount() != puts { + t.Fatalf("second item stole an already reserved ticket: err=%v puts=%d", err, f.transport.putCount()) + } + stored, err := f.service.GetTicketLink(first.ID) + if err != nil || stored.ItemID != f.item1 { + t.Fatalf("original local reservation changed: link=%#v err=%v", stored, err) + } +} + +func TestZammadUpdateTicketLinkValidatesOwnerForEffectiveGroupAndPersistsSnapshot(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + f.transport.ticket = map[string]any{"id": 901, "number": "420901", "group_id": 7, "state_id": 2, "owner_id": 1} + f.transport.groups = []map[string]any{{"id": 7, "name": "Support", "active": true}, {"id": 8, "name": "Escalations", "active": true}} + f.transport.states = []map[string]any{{"id": 2, "name": "open", "active": true}, {"id": 4, "name": "closed", "active": true}} + f.transport.users = []map[string]any{{"id": 99, "active": true, "firstname": "Grace", "lastname": "Hopper", "group_ids": map[string][]string{"8": {"full"}}}} + allowedGroups := zammadTestGroupRefs(7, 8) + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{AllowedGroups: &allowedGroups}); err != nil { + t.Fatal(err) + } + stateID, groupID, ownerID := 4, 8, 99 + updated, err := f.service.UpdateTicketLink(context.Background(), link.ID, models.UpdateZammadTicketLinkRequest{StateID: &stateID, GroupID: &groupID, OwnerID: &ownerID}) + if err != nil { + t.Fatal(err) + } + if updated.LastStatusID != 4 || updated.LastStatusName != "closed" || updated.GroupID != 8 || updated.GroupName != "Escalations" || updated.OwnerID != 99 || updated.OwnerName != "Grace Hopper" { + t.Fatalf("remote update snapshot was not persisted: %#v", updated) + } + puts := f.transport.putCount() + invalidOwner := 100 + _, err = f.service.UpdateTicketLink(context.Background(), link.ID, models.UpdateZammadTicketLinkRequest{OwnerID: &invalidOwner}) + var validationErr *ZammadValidationError + if !errors.As(err, &validationErr) || f.transport.putCount() != puts { + t.Fatalf("owner without group access reached remote update: err=%v puts=%d", err, f.transport.putCount()) + } +} + +func TestZammadTicketSnapshotBlocksConcurrentGroupPolicyNarrowing(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + f.transport.groups = []map[string]any{{"id": 7, "name": "Support", "active": true}, {"id": 8, "name": "Escalations", "active": true}} + initialAllowedGroups := zammadTestGroupRefs(7, 8) + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{AllowedGroups: &initialAllowedGroups}); err != nil { + t.Fatal(err) + } + + arrived := make(chan struct{}, 1) + release := make(chan struct{}) + f.service.SetPersistBeforeConnectionLockForTesting(func() { + arrived <- struct{}{} + <-release + }) + defer f.service.SetPersistBeforeConnectionLockForTesting(nil) + + newGroupID := 8 + updateErr := make(chan error, 1) + go func() { + _, err := f.service.UpdateTicketLink(context.Background(), link.ID, models.UpdateZammadTicketLinkRequest{GroupID: &newGroupID}) + updateErr <- err + }() + <-arrived + narrowedAllowedGroups := zammadTestGroupRefs(7) + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{AllowedGroups: &narrowedAllowedGroups}); !errors.Is(err, ErrZammadConnectionBusy) { + t.Fatalf("group policy changed while remote update was pending: %v", err) + } + close(release) + if err := <-updateErr; err != nil { + t.Fatalf("remote group move failed after conflicting policy update was rejected: %v", err) + } + + stored, err := f.service.GetTicketLink(link.ID) + if err != nil { + t.Fatal(err) + } + if stored.GroupID != 8 || stored.SyncState != models.ZammadSyncLinked || stored.LastError != "" { + t.Fatalf("rejected policy race prevented the valid snapshot: %#v", stored) + } + if got := int(f.transport.ticket["group_id"].(float64)); got != 8 { + t.Fatalf("test did not move the remote ticket before narrowing: group=%d", got) + } +} + +func TestZammadExpiredUpdateLeaseReloadsGroupPolicyBeforeRemoteWrite(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + f.transport.groups = []map[string]any{{"id": 7, "name": "Support", "active": true}, {"id": 8, "name": "Escalations", "active": true}} + initialAllowed := zammadTestGroupRefs(7, 8) + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{AllowedGroups: &initialAllowed}); err != nil { + t.Fatal(err) + } + arrived := make(chan struct{}, 1) + release := make(chan struct{}) + f.service.SetUpdateBeforeRemoteWriteForTesting(func() { + arrived <- struct{}{} + <-release + }) + defer f.service.SetUpdateBeforeRemoteWriteForTesting(nil) + + groupID := 8 + updateErr := make(chan error, 1) + go func() { + _, err := f.service.UpdateTicketLink(context.Background(), link.ID, models.UpdateZammadTicketLinkRequest{GroupID: &groupID}) + updateErr <- err + }() + <-arrived + if _, err := f.db.ExecWrite("UPDATE zammad_ticket_links SET sync_lock_until = ? WHERE id = ?", time.Now().Add(-time.Minute), link.ID); err != nil { + t.Fatal(err) + } + narrowed := zammadTestGroupRefs(7) + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{AllowedGroups: &narrowed}); err != nil { + t.Fatalf("group policy could not change after synthetic lease expiry: %v", err) + } + puts := f.transport.putCount() + close(release) + if err := <-updateErr; !errors.Is(err, ErrZammadTicketGroupPolicyChanged) { + t.Fatalf("expired update worker did not honor current group policy: %v", err) + } + if f.transport.putCount() != puts { + t.Fatalf("expired update worker wrote a group removed by current policy") + } +} + +func TestZammadTicketCreationBlocksGroupNarrowingAcrossRemoteReroute(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.groups = []map[string]any{{"id": 7, "name": "Support", "active": true}, {"id": 8, "name": "Escalations", "active": true}} + f.transport.createGroupID = 8 + initialAllowedGroups := zammadTestGroupRefs(7, 8) + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{AllowedGroups: &initialAllowedGroups}); err != nil { + t.Fatal(err) + } + arrived := make(chan struct{}, 1) + release := make(chan struct{}) + f.service.SetPersistBeforeConnectionLockForTesting(func() { + arrived <- struct{}{} + <-release + }) + defer f.service.SetPersistBeforeConnectionLockForTesting(nil) + + type creationResult struct { + link *models.ZammadTicketLink + err error + } + result := make(chan creationResult, 1) + go func() { + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID, GroupID: 7}) + result <- creationResult{link: link, err: err} + }() + <-arrived + narrowedAllowedGroups := zammadTestGroupRefs(7) + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{AllowedGroups: &narrowedAllowedGroups}); !errors.Is(err, ErrZammadConnectionBusy) { + t.Fatalf("group policy changed while rerouted ticket creation was incomplete: %v", err) + } + close(release) + created := <-result + if created.err != nil || created.link == nil || created.link.GroupID != 8 { + t.Fatalf("ticket creation did not finish under the preserved policy: link=%#v err=%v", created.link, created.err) + } +} + +func TestZammadIncompleteReservationAllowsPolicyExpansionAndCorrelationRecovery(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.groups = []map[string]any{{"id": 7, "name": "Support", "active": true}, {"id": 8, "name": "Escalations", "active": true}} + f.transport.createGroupID = 8 + initialAllowedGroups := zammadTestGroupRefs(7) + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{AllowedGroups: &initialAllowedGroups}); err != nil { + t.Fatal(err) + } + + _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + var validationErr *ZammadValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("rerouted ticket unexpectedly completed under the old group policy: %v", err) + } + links, err := f.service.TicketLinksForItem(f.item1) + if err != nil || len(links) != 1 || links[0].SyncState != models.ZammadSyncUncertain { + t.Fatalf("rerouted ticket was not retained for correlation recovery: links=%#v err=%v", links, err) + } + + expandedAllowedGroups := zammadTestGroupRefs(7, 8) + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{AllowedGroups: &expandedAllowedGroups}); err != nil { + t.Fatalf("safe group policy expansion was blocked by incomplete reservation: %v", err) + } + narrowedAllowedGroups := zammadTestGroupRefs(7) + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{AllowedGroups: &narrowedAllowedGroups}); !errors.Is(err, ErrZammadConnectionBusy) { + t.Fatalf("group policy narrowing was allowed while reservation was incomplete: %v", err) + } + + recovered, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatalf("correlation retry did not recover after policy expansion: %v", err) + } + if recovered.TicketID != 901 || recovered.GroupID != 8 || recovered.SyncState != models.ZammadSyncLinked { + t.Fatalf("correlation retry did not complete the rerouted ticket: %#v", recovered) + } + _, posts := f.transport.counts() + if posts != 1 { + t.Fatalf("correlation retry created a duplicate ticket: posts=%d", posts) + } +} + +func TestZammadGroupPolicyNarrows(t *testing.T) { + currentNumeric := &repository.ZammadConnectionMutationSnapshot{DefaultGroupID: 7, AllowedGroups: zammadTestGroupRefs(7, 8)} + currentNameOnly := &repository.ZammadConnectionMutationSnapshot{DefaultGroupName: "Support"} + tests := []struct { + name string + current *repository.ZammadConnectionMutationSnapshot + proposed *models.ZammadConnection + narrows bool + }{ + { + name: "numeric expansion", current: currentNumeric, + proposed: &models.ZammadConnection{DefaultGroupID: 7, AllowedGroups: zammadTestGroupRefs(7, 8, 9)}, + }, + { + name: "numeric expansion preserves name-only default through allowlist", + current: &repository.ZammadConnectionMutationSnapshot{ + DefaultGroupName: "Support", AllowedGroups: zammadTestGroupRefs(7, 8), + }, + proposed: &models.ZammadConnection{DefaultGroupName: "Support", AllowedGroups: zammadTestGroupRefs(7, 8, 9)}, + }, + { + name: "numeric narrowing", current: currentNumeric, + proposed: &models.ZammadConnection{DefaultGroupID: 7, AllowedGroups: zammadTestGroupRefs(7)}, narrows: true, + }, + { + name: "old default remains allowed", current: &repository.ZammadConnectionMutationSnapshot{DefaultGroupID: 7}, + proposed: &models.ZammadConnection{DefaultGroupID: 8, AllowedGroups: zammadTestGroupRefs(7, 8)}, + }, + { + name: "old default excluded", current: &repository.ZammadConnectionMutationSnapshot{DefaultGroupID: 7}, + proposed: &models.ZammadConnection{DefaultGroupID: 8, AllowedGroups: zammadTestGroupRefs(8)}, narrows: true, + }, + { + name: "same name-only policy", current: currentNameOnly, + proposed: &models.ZammadConnection{DefaultGroupName: "Support"}, + }, + { + name: "name-only changed to IDs", current: currentNameOnly, + proposed: &models.ZammadConnection{DefaultGroupID: 7, AllowedGroups: zammadTestGroupRefs(7)}, narrows: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := zammadGroupPolicyNarrows(tt.current, tt.proposed); got != tt.narrows { + t.Fatalf("zammadGroupPolicyNarrows() = %v, want %v", got, tt.narrows) + } + }) + } +} + +func TestZammadExistingTicketLinkBlocksGroupNarrowingAcrossRemoteReroute(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.groups = []map[string]any{{"id": 7, "name": "Support", "active": true}, {"id": 8, "name": "Escalations", "active": true}} + f.transport.ticket = map[string]any{"id": 714, "number": "420714", "group_id": 7, "state_id": 2, "state": "open"} + f.transport.putRerouteGroupID = 8 + initialAllowedGroups := zammadTestGroupRefs(7, 8) + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{AllowedGroups: &initialAllowedGroups}); err != nil { + t.Fatal(err) + } + arrived := make(chan struct{}, 1) + release := make(chan struct{}) + f.service.SetPersistBeforeConnectionLockForTesting(func() { + arrived <- struct{}{} + <-release + }) + defer f.service.SetPersistBeforeConnectionLockForTesting(nil) + + type linkResult struct { + link *models.ZammadTicketLink + err error + } + result := make(chan linkResult, 1) + go func() { + link, err := f.service.LinkExistingTicket(context.Background(), f.item1, f.actorID, models.LinkZammadTicketRequest{ConnectionID: f.connection.ProviderID, TicketNumber: "420714"}) + result <- linkResult{link: link, err: err} + }() + <-arrived + narrowedAllowedGroups := zammadTestGroupRefs(7) + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{AllowedGroups: &narrowedAllowedGroups}); !errors.Is(err, ErrZammadConnectionBusy) { + t.Fatalf("group policy changed while rerouted existing-ticket link was incomplete: %v", err) + } + close(release) + linked := <-result + if linked.err != nil || linked.link == nil || linked.link.GroupID != 8 { + t.Fatalf("existing ticket link did not finish under the preserved policy: link=%#v err=%v", linked.link, linked.err) + } +} + +func TestZammadUnlinkClearsOnlyExactRemoteCorrelationWithoutDeletingTicket(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + if _, err := f.service.UnlinkTicket(context.Background(), link.ID); err != nil { + t.Fatal(err) + } + if f.transport.ticket == nil || f.transport.ticket["id"] != 901 || f.transport.ticket["windshift_item_key"] != "" { + t.Fatalf("unlink changed or deleted the remote ticket: %#v", f.transport.ticket) + } + if _, err := f.service.GetTicketLink(link.ID); !errors.Is(err, repository.ErrNotFound) { + t.Fatalf("unlink retained local association: %v", err) + } +} + +func TestZammadLocalDetachRemovesLinksWithoutContactingUnavailableUpstream(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + f.transport.resetRequests() + f.transport.getStatus = http.StatusServiceUnavailable + f.transport.putError = errors.New("synthetic upstream outage") + + detached, err := f.service.DetachTicketLinkLocally(link.ID) + if err != nil { + t.Fatalf("local detach failed while upstream was unavailable: %v", err) + } + if detached.ID != link.ID { + t.Fatalf("local detach returned the wrong link: got %q want %q", detached.ID, link.ID) + } + requests, _ := f.transport.counts() + if requests != 0 { + t.Fatalf("local detach contacted Zammad %d times", requests) + } + if f.transport.ticket["windshift_item_key"] != link.CorrelationKey { + t.Fatalf("local detach unexpectedly changed remote correlation: %#v", f.transport.ticket) + } + if _, err := f.service.GetTicketLink(link.ID); !errors.Is(err, repository.ErrNotFound) { + t.Fatalf("local detach retained typed link: %v", err) + } + var genericLinkCount int + if err := f.db.QueryRow("SELECT COUNT(*) FROM item_integration_links WHERE id = ?", link.ItemIntegrationLinkID).Scan(&genericLinkCount); err != nil || genericLinkCount != 0 { + t.Fatalf("local detach retained generic link: count=%d err=%v", genericLinkCount, err) + } + if _, err := NewItemCRUDService(f.db).Delete(f.item1); err != nil { + t.Fatalf("item remained protected after local detach: %v", err) + } +} + +func TestZammadDisabledConnectionCanBeUnlinkedAndDeleted(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + disabled := false + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{Enabled: &disabled}); err != nil { + t.Fatal(err) + } + if _, err := f.service.UnlinkTicket(context.Background(), link.ID); err != nil { + t.Fatalf("unlink through disabled connection failed: %v", err) + } + if f.transport.ticket["windshift_item_key"] != "" { + t.Fatalf("disabled-connection unlink left remote correlation: %#v", f.transport.ticket) + } + if _, err := f.service.GetTicketLink(link.ID); !errors.Is(err, repository.ErrNotFound) { + t.Fatalf("disabled-connection unlink retained local link: %v", err) + } + if err := f.service.DeleteConnection(f.connection.ProviderID); err != nil { + t.Fatalf("connection remained undeletable after unlink: %v", err) + } +} + +func TestZammadUnlinkStillRequiresWorkspaceScope(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite("DELETE FROM action_credential_workspaces WHERE credential_id = ?", f.connection.CredentialID); err != nil { + t.Fatal(err) + } + if _, err := f.service.UnlinkTicket(context.Background(), link.ID); !errors.Is(err, ErrCredentialScopeMismatch) { + t.Fatalf("out-of-scope unlink returned %v", err) + } + if _, err := f.service.GetTicketLink(link.ID); err != nil { + t.Fatalf("out-of-scope unlink removed local link: %v", err) + } + if f.transport.ticket["windshift_item_key"] != link.CorrelationKey { + t.Fatalf("out-of-scope unlink changed remote correlation: %#v", f.transport.ticket) + } +} + +func TestZammadUnlinkStillRequiresEnabledCredential(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite("UPDATE action_credentials SET is_enabled = false WHERE id = ?", f.connection.CredentialID); err != nil { + t.Fatal(err) + } + if _, err := f.service.UnlinkTicket(context.Background(), link.ID); !errors.Is(err, ErrCredentialDisabled) { + t.Fatalf("disabled-credential unlink returned %v", err) + } + if _, err := f.service.GetTicketLink(link.ID); err != nil { + t.Fatalf("disabled-credential unlink removed local link: %v", err) + } + if f.transport.ticket["windshift_item_key"] != link.CorrelationKey { + t.Fatalf("disabled-credential unlink changed remote correlation: %#v", f.transport.ticket) + } +} + +func TestZammadLinkedItemDeletionRequiresExplicitUnlink(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + + if _, err := f.db.ExecWrite("DELETE FROM items WHERE id = ?", f.item1); err == nil { + t.Fatal("database allowed direct deletion of a Zammad-linked item") + } + crud := NewItemCRUDService(f.db) + if _, err := crud.Delete(f.item1); !errors.Is(err, ErrItemHasProtectedIntegrationLinks) { + t.Fatalf("linked item deletion returned %v", err) + } + if _, err := repository.NewItemRepository(f.db).FindByID(f.item1); err != nil { + t.Fatalf("rejected deletion removed item: %v", err) + } + if _, err := f.service.GetTicketLink(link.ID); err != nil { + t.Fatalf("rejected deletion removed Zammad link: %v", err) + } + var genericLinkCount int + if err := f.db.QueryRow("SELECT COUNT(*) FROM item_integration_links WHERE id = ?", link.ItemIntegrationLinkID).Scan(&genericLinkCount); err != nil || genericLinkCount != 1 { + t.Fatalf("rejected deletion removed generic item link: count=%d err=%v", genericLinkCount, err) + } + + if _, err := f.service.UnlinkTicket(context.Background(), link.ID); err != nil { + t.Fatal(err) + } + if _, err := crud.Delete(f.item1); err != nil { + t.Fatalf("item remained undeletable after explicit unlink: %v", err) + } + if _, err := repository.NewItemRepository(f.db).FindByID(f.item1); !errors.Is(err, repository.ErrNotFound) { + t.Fatalf("item remained after unlink and delete: %v", err) + } +} + +func TestZammadCascadeDeletionIsBlockedByLinkedDescendant(t *testing.T) { + f := newZammadServiceFixture(t, nil) + childID := mustInsertID(t, f.db, `INSERT INTO items + (workspace_id, workspace_item_number, parent_id, title, description, frac_index, status_id, creator_id, last_active_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP)`, + f.workspace1, 50, f.item1, "Linked child", "Synthetic child", "a0V", f.openStatus, f.actorID) + if _, err := f.service.CreateTicket(context.Background(), childID, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}); err != nil { + t.Fatal(err) + } + + if _, err := NewItemCRUDService(f.db).Delete(f.item1); !errors.Is(err, ErrItemHasProtectedIntegrationLinks) { + t.Fatalf("cascade with linked descendant returned %v", err) + } + itemRepo := repository.NewItemRepository(f.db) + if _, err := itemRepo.FindByID(f.item1); err != nil { + t.Fatalf("rejected cascade removed root item: %v", err) + } + if _, err := itemRepo.FindByID(childID); err != nil { + t.Fatalf("rejected cascade removed linked descendant: %v", err) + } +} + +func TestZammadLinkedItemsBlockWorkspaceDeletion(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + + workspaceService := NewWorkspaceService(f.db) + if err := workspaceService.Delete(f.workspace1); !errors.Is(err, ErrWorkspaceHasProtectedIntegrationLinks) { + t.Fatalf("workspace deletion with linked item returned %v", err) + } + if exists, err := repository.NewWorkspaceRepository(f.db).Exists(f.workspace1); err != nil || !exists { + t.Fatalf("rejected workspace deletion removed workspace: exists=%t err=%v", exists, err) + } + if _, err := repository.NewItemRepository(f.db).FindByID(f.item1); err != nil { + t.Fatalf("rejected workspace deletion removed item: %v", err) + } + if _, err := f.service.GetTicketLink(link.ID); err != nil { + t.Fatalf("rejected workspace deletion removed Zammad link: %v", err) + } + + if _, err := f.service.UnlinkTicket(context.Background(), link.ID); err != nil { + t.Fatal(err) + } + if err := workspaceService.Delete(f.workspace1); err != nil { + t.Fatalf("workspace remained undeletable after explicit unlink: %v", err) + } + if exists, err := repository.NewWorkspaceRepository(f.db).Exists(f.workspace1); err != nil || exists { + t.Fatalf("workspace remained after unlink and delete: exists=%t err=%v", exists, err) + } +} + +func TestZammadLinkedPersonalWorkspaceBlocksUserOffboardingBeforeMutation(t *testing.T) { + f := newZammadServiceFixture(t, nil) + if _, err := f.db.ExecWrite("UPDATE workspaces SET is_personal = true, owner_id = ? WHERE id = ?", f.actorID, f.workspace1); err != nil { + t.Fatal(err) + } + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + var beforeEmail, beforeUsername string + var beforeActive bool + if err := f.db.QueryRow("SELECT email, username, is_active FROM users WHERE id = ?", f.actorID).Scan(&beforeEmail, &beforeUsername, &beforeActive); err != nil { + t.Fatal(err) + } + + if _, err := OffboardUser(f.db, f.actorID, nil); !errors.Is(err, ErrUserOffboardingHasProtectedIntegrationLinks) { + t.Fatalf("offboarding with linked personal item returned %v", err) + } + var afterEmail, afterUsername string + var afterActive bool + if err := f.db.QueryRow("SELECT email, username, is_active FROM users WHERE id = ?", f.actorID).Scan(&afterEmail, &afterUsername, &afterActive); err != nil { + t.Fatal(err) + } + if beforeEmail != afterEmail || beforeUsername != afterUsername || beforeActive != afterActive { + t.Fatalf("blocked offboarding mutated user: before=(%q,%q,%t) after=(%q,%q,%t)", beforeEmail, beforeUsername, beforeActive, afterEmail, afterUsername, afterActive) + } + if exists, err := repository.NewWorkspaceRepository(f.db).Exists(f.workspace1); err != nil || !exists { + t.Fatalf("blocked offboarding removed personal workspace: exists=%t err=%v", exists, err) + } + if _, err := repository.NewItemRepository(f.db).FindByID(f.item1); err != nil { + t.Fatalf("blocked offboarding removed item: %v", err) + } + if _, err := f.service.GetTicketLink(link.ID); err != nil { + t.Fatalf("blocked offboarding removed Zammad link: %v", err) + } + var offboarded bool + if err := f.db.QueryRow("SELECT offboarded_at IS NOT NULL FROM users WHERE id = ?", f.actorID).Scan(&offboarded); err != nil || offboarded { + t.Fatalf("blocked offboarding marked user offboarded: value=%t err=%v", offboarded, err) + } + if _, err := f.service.UnlinkTicket(context.Background(), link.ID); err != nil { + t.Fatal(err) + } + if _, err := OffboardUser(f.db, f.actorID, nil); err != nil { + t.Fatalf("offboarding after explicit unlink failed: %v", err) + } + if err := f.db.QueryRow("SELECT offboarded_at IS NOT NULL FROM users WHERE id = ?", f.actorID).Scan(&offboarded); err != nil || !offboarded { + t.Fatalf("successful offboarding did not mark user offboarded: value=%t err=%v", offboarded, err) + } + if exists, err := repository.NewWorkspaceRepository(f.db).Exists(f.workspace1); err != nil || exists { + t.Fatalf("successful offboarding retained personal workspace: exists=%t err=%v", exists, err) + } +} + +func TestZammadUnlinkUpstreamFailurePreservesLocalLink(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + f.transport.putError = context.DeadlineExceeded + if _, err := f.service.UnlinkTicket(context.Background(), link.ID); !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected remote unlink failure, got %v", err) + } + if stored, err := f.service.GetTicketLink(link.ID); err != nil || stored.CorrelationKey != link.CorrelationKey { + t.Fatalf("unlink failure removed or corrupted the local link: link=%#v err=%v", stored, err) + } +} + +func TestZammadUnlinkDoesNotClearAnotherCorrelation(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + f.transport.ticket["windshift_item_key"] = "windshift:another-link:OTHER-8" + puts := f.transport.putCount() + if _, err := f.service.UnlinkTicket(context.Background(), link.ID); err != nil { + t.Fatal(err) + } + if f.transport.putCount() != puts || f.transport.ticket["windshift_item_key"] != "windshift:another-link:OTHER-8" { + t.Fatalf("unlink cleared a foreign correlation: %#v", f.transport.ticket) + } + if _, err := f.service.GetTicketLink(link.ID); !errors.Is(err, repository.ErrNotFound) { + t.Fatalf("stale local link was not removed: %v", err) + } +} + +func TestZammadUnlinkUncertainCreationResolvesExactCorrelationBeforeDelete(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.postErrorAfterCreate = context.DeadlineExceeded + _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected ambiguous create timeout, got %v", err) + } + links, err := f.service.TicketLinksForItem(f.item1) + if err != nil || len(links) != 1 || links[0].TicketID != 0 || links[0].SyncState != models.ZammadSyncUncertain { + t.Fatalf("ambiguous create reservation missing: links=%#v err=%v", links, err) + } + if _, err := f.service.UnlinkTicket(context.Background(), links[0].ID); err != nil { + t.Fatalf("uncertain unlink did not resolve the exact remote correlation: %v", err) + } + if f.transport.ticket["windshift_item_key"] != "" { + t.Fatalf("uncertain unlink left the resolved remote correlation: ticket=%#v", f.transport.ticket) + } + if _, err := f.service.GetTicketLink(links[0].ID); !errors.Is(err, repository.ErrNotFound) { + t.Fatalf("resolved uncertain reservation remained local: %v", err) + } +} + +func TestZammadUnlinkUncertainCreationPreservesReservationWhenSearchIsEmpty(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.postErrorAfterCreate = context.DeadlineExceeded + _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected ambiguous create timeout, got %v", err) + } + links, err := f.service.TicketLinksForItem(f.item1) + if err != nil || len(links) != 1 { + t.Fatalf("ambiguous create reservation missing: links=%#v err=%v", links, err) + } + correlation := links[0].CorrelationKey + f.transport.hideSearch = true + if _, err := f.service.UnlinkTicket(context.Background(), links[0].ID); err == nil { + t.Fatal("uncertain unlink discarded an unresolved correlation") + } + stored, err := f.service.GetTicketLink(links[0].ID) + if err != nil || stored.SyncState != models.ZammadSyncUncertain { + t.Fatalf("empty correlation search removed uncertain reservation: link=%#v err=%v", stored, err) + } + if f.transport.ticket["windshift_item_key"] != correlation { + t.Fatalf("empty search changed remote correlation: ticket=%#v", f.transport.ticket) + } +} + +func TestZammadCreateTicketMarksLocalCompletionFailureUncertainAndKeepsCorrelationPinned(t *testing.T) { + f := newZammadServiceFixture(t, nil) + if _, err := f.db.ExecWrite(`CREATE TRIGGER fail_zammad_ticket_completion + BEFORE UPDATE OF ticket_id ON zammad_ticket_links + WHEN NEW.ticket_id IS NOT NULL + BEGIN SELECT RAISE(ABORT, 'synthetic completion failure'); END`); err != nil { + t.Fatal(err) + } + _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err == nil { + t.Fatal("expected local completion failure") + } + link, err := f.service.TicketLinksForItem(f.item1) + if err != nil || len(link) != 1 || link[0].SyncState != models.ZammadSyncUncertain || f.transport.ticket["windshift_item_key"] != link[0].CorrelationKey { + t.Fatalf("known remote ticket was not preserved as uncertain: links=%#v ticket=%#v err=%v", link, f.transport.ticket, err) + } + if _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}); err == nil { + t.Fatal("expected retry to surface the retained local completion failure") + } + _, posts := f.transport.counts() + if posts != 1 { + t.Fatalf("retry created a second ticket after local completion failure: posts=%d", posts) + } +} + +func TestZammadCreateTicketMarksAmbiguousHTTPFailuresUncertainWithoutRetryingPOST(t *testing.T) { + for _, status := range []int{http.StatusInternalServerError, http.StatusRequestTimeout, http.StatusTooManyRequests} { + t.Run(http.StatusText(status), func(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.hideSearch = true + f.transport.postStatusAfterCreate = status + + _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + var apiErr *zammad.APIError + if !errors.As(err, &apiErr) || apiErr.StatusCode != status { + t.Fatalf("expected HTTP %d create failure, got %v", status, err) + } + links, err := f.service.TicketLinksForItem(f.item1) + if err != nil || len(links) != 1 || links[0].SyncState != models.ZammadSyncUncertain { + t.Fatalf("ambiguous HTTP %d result was not retained as uncertain: links=%#v err=%v", status, links, err) + } + + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatalf("normal retry after ambiguous HTTP %d failed: %v", status, err) + } + _, posts := f.transport.counts() + if posts != 1 || link.SyncState != models.ZammadSyncUncertain { + t.Fatalf("normal retry after ambiguous HTTP %d sent another POST: link=%#v posts=%d", status, link, posts) + } + }) + } +} + +func TestZammadCreateTicketMarksRejectingHTTP400Failed(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.hideSearch = true + f.transport.postStatusAfterCreate = http.StatusBadRequest + + _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + var apiErr *zammad.APIError + if !errors.As(err, &apiErr) || apiErr.StatusCode != http.StatusBadRequest { + t.Fatalf("expected HTTP 400 create failure, got %v", err) + } + links, err := f.service.TicketLinksForItem(f.item1) + if err != nil || len(links) != 1 || links[0].SyncState != models.ZammadSyncFailed { + t.Fatalf("rejecting HTTP 400 result was not retained as failed: links=%#v err=%v", links, err) + } +} + +func TestZammadCreateTicketRejectsCorrelationMatchInDisallowedGroup(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.ticket = map[string]any{ + "id": 715, "number": "420715", "group_id": 99, "state_id": 2, "state": "open", + "windshift_item_key": "windshift:" + f.connection.ProviderID + ":PRI-49", + } + + _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + var validationErr *ZammadValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("correlation match in a disallowed group was accepted: %v", err) + } + links, err := f.service.TicketLinksForItem(f.item1) + if err != nil || len(links) != 1 || links[0].TicketID != 0 || links[0].SyncState == models.ZammadSyncLinked { + t.Fatalf("disallowed correlation match completed a local link: links=%#v err=%v", links, err) + } + var genericLinks int + if err := f.db.QueryRow("SELECT COUNT(*) FROM item_integration_links").Scan(&genericLinks); err != nil || genericLinks != 0 { + t.Fatalf("disallowed correlation match created a generic link: count=%d err=%v", genericLinks, err) + } +} + +func TestZammadSyncPersistsRemoteGroupAndOwner(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + f.transport.getTicket = map[string]any{"id": 901, "number": "420901", "title": "Escalated VPN problem", "group_id": 8, "state_id": 3, "state": "pending", "owner_id": 99} + f.transport.groups = []map[string]any{{"id": 7, "name": "Support", "active": true}, {"id": 8, "name": "Escalations", "active": true}} + f.transport.users = []map[string]any{{"id": 99, "active": true, "firstname": "Grace", "lastname": "Hopper", "group_ids": map[string][]string{"8": {"full"}}}} + allowedGroups := zammadTestGroupRefs(7, 8) + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{AllowedGroups: &allowedGroups}); err != nil { + t.Fatal(err) + } + synced, err := f.service.SyncTicketLink(context.Background(), link.ID) + if err != nil { + t.Fatal(err) + } + if synced.GroupID != 8 || synced.GroupName != "Escalations" || synced.OwnerID != 99 || synced.OwnerName != "Grace Hopper" || synced.LastSyncedAt == nil { + t.Fatalf("sync did not retain remote group/owner state: %#v", synced) + } + history, err := f.service.TicketHistoryForItem(f.item1, 10) + if err != nil || len(history) != 3 { + t.Fatalf("expected one event per observed status/group/owner change: history=%#v err=%v", history, err) + } + for _, change := range history { + if change.TicketTitle != "Escalated VPN problem" || change.TicketURL != "https://zammad.example.test/#ticket/zoom/901" || + change.CurrentGroup.ID != 8 || change.CurrentGroup.Name != "Escalations" || + change.CurrentOwner.ID != 99 || change.CurrentOwner.Name != "Grace Hopper" { + t.Fatalf("observed change omitted current ticket context: %#v", change) + } + } + if _, err := f.service.SyncTicketLink(context.Background(), link.ID); err != nil { + t.Fatal(err) + } + history, err = f.service.TicketHistoryForItem(f.item1, 10) + if err != nil || len(history) != 3 { + t.Fatalf("identical snapshot duplicated observed changes: history=%#v err=%v", history, err) + } + if _, err := f.db.ExecWrite(`UPDATE items SET workspace_id = ? WHERE id = ?`, f.workspace2, f.item1); err != nil { + t.Fatal(err) + } + history, err = f.service.TicketHistoryForItem(f.item1, 10) + if err != nil || len(history) != 0 { + t.Fatalf("history remained visible after the item moved outside the connection scope: history=%#v err=%v", history, err) + } + overview, err := f.service.WorkspaceOverview(f.workspace2, 10) + if err != nil { + t.Fatal(err) + } + if overview.Total != 0 || len(overview.RecentChanges) != 0 { + t.Fatalf("workspace overview disclosed a connection that is unavailable there: %#v", overview) + } +} + +func TestZammadWorkspaceOverviewCountsOnlyCompleteTicketLinks(t *testing.T) { + f := newZammadServiceFixture(t, nil) + if _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}); err != nil { + t.Fatal(err) + } + if _, err := f.service.CreateTicket(context.Background(), f.item2, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}); err == nil { + t.Fatal("out-of-scope ticket creation unexpectedly succeeded") + } + overview, err := f.service.WorkspaceOverview(f.workspace1, 5) + if err != nil { + t.Fatal(err) + } + if overview.Total != 1 || overview.Active != 1 || overview.Closed != 0 || overview.UnknownStatus != 0 || overview.Unassigned != 1 { + t.Fatalf("unexpected overview for complete link: %#v", overview) + } + if len(overview.ByStatus) != 1 || overview.ByStatus[0].Count != 1 { + t.Fatalf("overview omitted linked ticket status: %#v", overview.ByStatus) + } +} + +func TestZammadWorkspaceOverviewIncludesCurrentTicketAndSyncedRemoteTitle(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + overview, err := f.service.WorkspaceOverview(f.workspace1, 5) + if err != nil { + t.Fatal(err) + } + if len(overview.Tickets) != 1 { + t.Fatalf("newly linked ticket missing from overview: %#v", overview.Tickets) + } + ticket := overview.Tickets[0] + if ticket.ID != link.ID || ticket.ItemID != f.item1 || ticket.ItemKey != "PRI-49" || ticket.TicketNumber != "420901" || + ticket.TicketTitle != "[PRI-49] Synthetic ticket source" || + ticket.TicketURL != "https://zammad.example.test/#ticket/zoom/901" || + ticket.Closed || + ticket.Status.ID != 2 || ticket.Status.Name != "open" || + ticket.Group.ID != 7 || ticket.Group.Name != "Support" { + t.Fatalf("newly linked ticket snapshot is incomplete: %#v", ticket) + } + + f.transport.getTicket = map[string]any{ + "id": 901, "number": "420901", "title": "Renamed in Zammad", + "group_id": 7, "group": "Support", "state_id": 2, "state": "open", + } + if _, err := f.service.SyncTicketLink(context.Background(), link.ID); err != nil { + t.Fatal(err) + } + overview, err = f.service.WorkspaceOverview(f.workspace1, 5) + if err != nil { + t.Fatal(err) + } + if len(overview.Tickets) != 1 || overview.Tickets[0].TicketTitle != "Renamed in Zammad" { + t.Fatalf("synchronized Zammad title was not persisted into the overview: %#v", overview.Tickets) + } + + closedStates := []int{2} + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{ + ClosedStateIDs: &closedStates, + }); err != nil { + t.Fatal(err) + } + overview, err = f.service.WorkspaceOverview(f.workspace1, 5) + if err != nil { + t.Fatal(err) + } + if len(overview.Tickets) != 1 || !overview.Tickets[0].Closed { + t.Fatalf("overview ticket ignored configured closed-state IDs: %#v", overview.Tickets) + } +} + +func TestZammadWorkspaceOverviewSeparatesSameStatusAcrossConnections(t *testing.T) { + f := newZammadServiceFixture(t, nil) + if _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}); err != nil { + t.Fatal(err) + } + second, err := f.service.CreateConnection(models.CreateZammadConnectionRequest{ + Slug: "second-helpdesk", Name: "Second helpdesk", BaseURL: "https://second-zammad.example.test", + APIToken: "synthetic-zammad-token", DefaultGroupID: 7, DefaultGroupName: "Support", + DefaultCustomer: "robot@example.test", WorkspaceIDs: []int{f.workspace1}, + }, f.actorID) + if err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite(`INSERT INTO item_integration_links + (id, item_id, integration_provider_id, external_id, external_url, title, link_type, linked_by) + VALUES (?, ?, ?, ?, ?, ?, 'ticket', ?)`, "second-link-external", strconv.Itoa(f.item1), second.ProviderID, "902", "https://second-zammad.example.test/#ticket/zoom/902", "Zammad #902", strconv.Itoa(f.actorID)); err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite(`INSERT INTO zammad_ticket_links + (id, item_id, provider_id, item_integration_link_id, ticket_id, ticket_number, group_id, group_name, correlation_key, sync_state, last_status_id, last_status_name, created_by) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, "second-link", f.item1, second.ProviderID, "second-link-external", 902, "902", 7, "Support", "windshift:second-helpdesk:PRI-49", models.ZammadSyncLinked, 2, "open", f.actorID); err != nil { + t.Fatal(err) + } + overview, err := f.service.WorkspaceOverview(f.workspace1, 5) + if err != nil { + t.Fatal(err) + } + if overview.Total != 2 || len(overview.ByStatus) != 2 { + t.Fatalf("same status IDs from distinct connections were merged: %#v", overview) + } + seenConnections := map[string]bool{} + for _, bucket := range overview.ByStatus { + if bucket.ID != 2 || bucket.Count != 1 { + t.Fatalf("unexpected bucket: %#v", bucket) + } + if bucket.ConnectionName == "" { + t.Fatalf("bucket response omitted the readable connection name: %#v", bucket) + } + seenConnections[bucket.ConnectionID] = true + } + if !seenConnections[f.connection.ProviderID] || !seenConnections[second.ProviderID] { + t.Fatalf("bucket response omitted connection identity: %#v", overview.ByStatus) + } +} + +func TestZammadSyncRejectsTicketMovedToDisallowedGroupBeforeSnapshotOrCompletion(t *testing.T) { + workflow := &fakeZammadWorkflow{} + f := newZammadServiceFixture(t, workflow) + workflow.db = f.db + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + allowedGroups := zammadTestGroupRefs(7) + closedStates := []int{4} + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{ + AllowedGroups: &allowedGroups, + ClosedStateIDs: &closedStates, + CompletionStatusID: &f.doneStatus, + }); err != nil { + t.Fatal(err) + } + f.transport.getTicket = map[string]any{"id": 901, "number": "420901", "group_id": 99, "state_id": 4, "state": "closed"} + f.transport.groups = []map[string]any{{"id": 7, "name": "Support", "active": true}, {"id": 99, "name": "Restricted", "active": true}} + + if _, err := f.service.SyncTicketLink(context.Background(), link.ID); err == nil { + t.Fatal("sync accepted a ticket moved to a disallowed group") + } + stored, err := f.service.GetTicketLink(link.ID) + if err != nil { + t.Fatal(err) + } + if stored.LastStatusID != link.LastStatusID || stored.GroupID != link.GroupID || stored.CompletionApplied || workflow.writes != 0 { + t.Fatalf("disallowed remote group changed local snapshot or completed item: link=%#v writes=%d", stored, workflow.writes) + } + var statusID int + if err := f.db.QueryRow("SELECT status_id FROM items WHERE id = ?", f.item1).Scan(&statusID); err != nil { + t.Fatal(err) + } + if statusID != f.openStatus { + t.Fatalf("disallowed remote group completed the item: status=%d", statusID) + } +} + +func TestZammadDueLinksRespectRetryDelayAndOAuthReauthorization(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + oldSync := time.Now().Add(-10 * time.Minute) + if _, err := f.db.ExecWrite("UPDATE zammad_ticket_links SET last_synced_at = ? WHERE id = ?", oldSync, link.ID); err != nil { + t.Fatal(err) + } + attemptedAt := time.Now() + syncOwner := "retry-delay-test" + claimed, err := f.service.repo.ClaimSync(link.ID, syncOwner, time.Now().Add(time.Minute)) + if err != nil || !claimed { + t.Fatalf("could not claim retry-delay test link: claimed=%v err=%v", claimed, err) + } + if err := f.service.repo.UpdateTicketLinkSync(link.ID, syncOwner, link.LastStatusID, link.LastStatusName, + link.GroupID, link.GroupName, link.OwnerID, link.OwnerName, + "", "synthetic safe failure", attemptedAt, false, false); err != nil { + t.Fatal(err) + } + due, err := f.service.repo.ListDueTicketLinks(time.Now().Add(-2*time.Minute), 10) + if err != nil { + t.Fatal(err) + } + if len(due) != 0 { + t.Fatalf("failed link ignored retry delay: %#v", due) + } + if _, err := f.db.ExecWrite("UPDATE zammad_ticket_links SET next_attempt_at = ? WHERE id = ?", time.Now().Add(-time.Minute), link.ID); err != nil { + t.Fatal(err) + } + due, err = f.service.repo.ListDueTicketLinks(time.Now().Add(-2*time.Minute), 10) + if err != nil || len(due) != 1 || due[0].ID != link.ID { + t.Fatalf("eligible retry was not scheduled: due=%#v err=%v", due, err) + } + if _, err := f.db.ExecWrite("UPDATE zammad_connections SET auth_method = 'oauth' WHERE provider_id = ?", f.connection.ProviderID); err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite(`INSERT INTO zammad_oauth_tokens(provider_id, oauth_generation, expires_at, reauthorization_required) + VALUES (?, 1, ?, true)`, f.connection.ProviderID, time.Now().Add(time.Hour)); err != nil { + t.Fatal(err) + } + due, err = f.service.repo.ListDueTicketLinks(time.Now().Add(-2*time.Minute), 10) + if err != nil { + t.Fatal(err) + } + if len(due) != 0 { + t.Fatalf("reauthorization-required OAuth connection remained due: %#v", due) + } +} + +func TestZammadRetryAfterAmbiguousTimeoutFindsExistingTicket(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.postErrorAfterCreate = context.DeadlineExceeded + _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected timeout, got %v", err) + } + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + _, posts := f.transport.counts() + if posts != 1 || link.TicketID != 901 || link.SyncState != models.ZammadSyncLinked { + t.Fatalf("retry created a duplicate or failed to recover: link=%#v posts=%d", link, posts) + } +} + +func TestZammadAmbiguousCreateNeverPostsAgainWhileSearchIsEmpty(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.hideSearch = true + f.transport.postErrorAfterCreate = context.DeadlineExceeded + _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected timeout, got %v", err) + } + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + _, posts := f.transport.counts() + if posts != 1 || link.SyncState != models.ZammadSyncUncertain { + t.Fatalf("uncertain retry sent another POST: state=%s posts=%d", link.SyncState, posts) + } + f.transport.postErrorAfterCreate = nil + link, err = f.service.RetryUncertainTicketCreation(context.Background(), link.ID, f.actorID) + if err != nil { + t.Fatal(err) + } + _, posts = f.transport.counts() + if posts != 2 || link.TicketID != 901 { + t.Fatalf("explicit administrator override did not retry creation: ticket=%d posts=%d", link.TicketID, posts) + } +} + +func TestZammadUncertainCreateSurvivesTransientCorrelationSearchFailure(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.postErrorAfterCreate = context.DeadlineExceeded + _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected ambiguous initial timeout, got %v", err) + } + f.transport.searchError = context.DeadlineExceeded + if _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}); !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected transient correlation-search failure, got %v", err) + } + links, err := f.service.TicketLinksForItem(f.item1) + if err != nil || len(links) != 1 || links[0].SyncState != models.ZammadSyncUncertain { + t.Fatalf("search failure downgraded uncertain creation: links=%#v err=%v", links, err) + } + f.transport.hideSearch = true + retried, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + _, posts := f.transport.counts() + if posts != 1 || retried.SyncState != models.ZammadSyncUncertain { + t.Fatalf("search failure enabled a duplicate POST: posts=%d link=%#v", posts, retried) + } +} + +func TestZammadStaleCreatingLeaseRetriesSearchOnly(t *testing.T) { + f := newZammadServiceFixture(t, nil) + item, err := repository.NewItemRepository(f.db).FindByIDWithDetails(f.item1) + if err != nil { + t.Fatal(err) + } + link := &models.ZammadTicketLink{ + ID: "stale-create-link", ItemID: item.ID, ProviderID: f.connection.ProviderID, + GroupID: 7, GroupName: "Support", CorrelationKey: "windshift:stale-create:PRI-49", + SyncState: models.ZammadSyncPending, CreatedBy: &f.actorID, + } + if err := f.service.reserveZammadTicketLink(f.connection, item, link, false); err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite(`UPDATE zammad_ticket_links + SET sync_state = ?, creating_started_at = ?, sync_lock_until = ?, sync_lock_owner = ? + WHERE id = ?`, models.ZammadSyncCreating, time.Now().Add(-3*time.Minute), time.Now().Add(-time.Minute), "crashed-worker", link.ID); err != nil { + t.Fatal(err) + } + f.transport.hideSearch = true + retried, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + _, posts := f.transport.counts() + if posts != 0 || retried.SyncState != models.ZammadSyncUncertain { + t.Fatalf("stale in-flight creation retried POST: posts=%d link=%#v", posts, retried) + } +} + +func TestZammadCreateVerifiesGroupFromTicketDetailAfterPartialResponse(t *testing.T) { + for _, tt := range []struct { + name string + getTicket map[string]any + wantGroup int + }{ + { + name: "remote reroute is rejected", + getTicket: map[string]any{"id": 901, "number": "420901", "group_id": 8, "state_id": 2, "state": "open"}, + wantGroup: 8, + }, + { + name: "missing detail group is rejected", + getTicket: map[string]any{"id": 901, "number": "420901", "state_id": 2, "state": "open"}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.groups = []map[string]any{{"id": 7, "name": "Support", "active": true}, {"id": 8, "name": "Escalations", "active": true}} + f.transport.createGroupID = tt.wantGroup + f.transport.postResponse = map[string]any{"id": 901, "number": "420901", "state_id": 2, "state": "open"} + f.transport.getTicket = tt.getTicket + + _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + var validationErr *ZammadValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("partial create response completed without a verified allowed group: %v", err) + } + links, err := f.service.TicketLinksForItem(f.item1) + if err != nil || len(links) != 1 || links[0].SyncState != models.ZammadSyncUncertain || links[0].ItemIntegrationLinkID != "" { + t.Fatalf("unverified create was not retained as uncertain: links=%#v err=%v", links, err) + } + if got, ok := f.transport.ticket["group_id"].(int); tt.wantGroup > 0 && (!ok || got != tt.wantGroup) { + t.Fatalf("test did not model remote reroute: ticket=%#v", f.transport.ticket) + } + }) + } +} + +func TestZammadRejectsUnconfiguredGroupBeforeCreate(t *testing.T) { + f := newZammadServiceFixture(t, nil) + _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID, GroupID: 99}) + var validationErr *ZammadValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("expected group validation error, got %v", err) + } + _, posts := f.transport.counts() + if posts != 0 { + t.Fatalf("unconfigured group reached ticket create endpoint %d times", posts) + } +} + +func TestZammadWorkspaceScopeBlocksHTTPAndSecretIsEncrypted(t *testing.T) { + f := newZammadServiceFixture(t, nil) + _, err := f.service.CreateTicket(context.Background(), f.item2, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if !errors.Is(err, ErrCredentialScopeMismatch) { + t.Fatalf("expected scope mismatch, got %v", err) + } + requests, _ := f.transport.counts() + if requests != 0 { + t.Fatalf("out-of-scope request reached transport %d times", requests) + } + encoded, err := json.Marshal(f.connection) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(encoded), "synthetic-zammad-token") || strings.Contains(string(encoded), "credential_id") { + t.Fatalf("connection API model disclosed credential material: %s", encoded) + } + var encrypted string + if err := f.db.QueryRow("SELECT encrypted_secret FROM action_credentials WHERE id = ?", f.connection.CredentialID).Scan(&encrypted); err != nil { + t.Fatal(err) + } + if encrypted == "synthetic-zammad-token" || strings.Contains(encrypted, "synthetic-zammad-token") { + t.Fatal("Zammad token was stored in plaintext") + } + listed, err := f.credentials.ListForWorkspace(f.workspace1) + if err != nil { + t.Fatal(err) + } + if len(listed) != 0 { + t.Fatalf("managed Zammad credential leaked into generic list: %#v", listed) + } + if _, err := f.credentials.Get(f.connection.CredentialID); !errors.Is(err, repository.ErrNotFound) { + t.Fatalf("managed Zammad credential was addressable through generic CRUD: %v", err) + } + if _, _, err := f.credentials.Resolve(context.Background(), f.connection.CredentialID, f.workspace1); !errors.Is(err, ErrCredentialPurposeMismatch) { + t.Fatalf("managed Zammad credential resolved through generic action path: %v", err) + } +} + +func TestZammadConnectionScopeCannotStrandExistingTicketLinks(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + + excludedScope := []int{f.workspace2} + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{WorkspaceIDs: &excludedScope}); err == nil { + t.Fatal("connection scope excluded a workspace containing a linked ticket") + } + stored, err := f.service.GetTicketLink(link.ID) + if err != nil || stored.ItemIntegrationLinkID == "" { + t.Fatalf("rejected scope update changed the link: link=%#v err=%v", stored, err) + } + if _, _, err := f.credentials.ResolveManaged(context.Background(), f.connection.CredentialID, f.workspace1, string(models.IntegrationProviderZammad), f.connection.ProviderID); err != nil { + t.Fatalf("rejected scope update changed credential scope: %v", err) + } + + expandedScope := []int{f.workspace1, f.workspace2} + updated, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{WorkspaceIDs: &expandedScope}) + if err != nil || !slices.Contains(updated.WorkspaceIDs, f.workspace2) { + t.Fatalf("scope expansion with existing link failed: connection=%#v err=%v", updated, err) + } +} + +func TestZammadConnectionRoutingAndGroupsCannotStrandExistingTicketLinks(t *testing.T) { + f := newZammadServiceFixture(t, nil) + if _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}); err != nil { + t.Fatal(err) + } + newBaseURL := "https://replacement.example.test" + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{BaseURL: &newBaseURL}); err == nil { + t.Fatal("base URL changed with an existing ticket link") + } + newCorrelationField := "replacement_item_key" + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{CorrelationField: &newCorrelationField}); err == nil { + t.Fatal("correlation field changed with an existing ticket link") + } + newDefaultGroupID := 8 + newDefaultGroupName := "Escalations" + newAllowedGroups := zammadTestGroupRefs(8) + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{ + DefaultGroupID: &newDefaultGroupID, DefaultGroupName: &newDefaultGroupName, AllowedGroups: &newAllowedGroups, + }); err == nil { + t.Fatal("group scope excluded an existing ticket link") + } + stored, err := f.service.GetConnection(f.connection.ProviderID) + if err != nil || stored.BaseURL != f.connection.BaseURL || stored.CorrelationField != f.connection.CorrelationField || stored.DefaultGroupID != 7 { + t.Fatalf("rejected routing/group updates changed connection: connection=%#v err=%v", stored, err) + } +} + +func TestZammadReservationRejectsStaleConnectionAndItemSnapshots(t *testing.T) { + t.Run("connection routing", func(t *testing.T) { + f := newZammadServiceFixture(t, nil) + item, err := repository.NewItemRepository(f.db).FindByIDWithDetails(f.item1) + if err != nil { + t.Fatal(err) + } + staleConnection := *f.connection + newBaseURL := "https://new-routing.example.test" + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{BaseURL: &newBaseURL}); err != nil { + t.Fatal(err) + } + link := &models.ZammadTicketLink{ID: "stale-routing", ItemID: item.ID, ProviderID: f.connection.ProviderID, GroupID: 7, GroupName: "Support", CorrelationKey: "stale", SyncState: models.ZammadSyncPending, CreatedBy: &f.actorID} + if err := f.service.reserveZammadTicketLink(&staleConnection, item, link, false); !errors.Is(err, ErrZammadLinkReservationConflict) { + t.Fatalf("stale connection snapshot reserved a link: %v", err) + } + }) + + t.Run("connection group policy", func(t *testing.T) { + f := newZammadServiceFixture(t, nil) + item, err := repository.NewItemRepository(f.db).FindByIDWithDetails(f.item1) + if err != nil { + t.Fatal(err) + } + staleConnection := *f.connection + newDefaultGroupID := 8 + newDefaultGroupName := "Escalations" + newAllowedGroups := zammadTestGroupRefs(8) + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{ + DefaultGroupID: &newDefaultGroupID, DefaultGroupName: &newDefaultGroupName, AllowedGroups: &newAllowedGroups, + }); err != nil { + t.Fatal(err) + } + link := &models.ZammadTicketLink{ID: "stale-group", ItemID: item.ID, ProviderID: f.connection.ProviderID, GroupID: 7, GroupName: "Support", CorrelationKey: "stale", SyncState: models.ZammadSyncPending, CreatedBy: &f.actorID} + if err := f.service.reserveZammadTicketLink(&staleConnection, item, link, false); !errors.Is(err, ErrZammadLinkReservationConflict) { + t.Fatalf("stale group policy reserved a link: %v", err) + } + }) + + t.Run("connection default customer", func(t *testing.T) { + f := newZammadServiceFixture(t, nil) + item, err := repository.NewItemRepository(f.db).FindByIDWithDetails(f.item1) + if err != nil { + t.Fatal(err) + } + staleConnection := *f.connection + newDefaultCustomer := "new-customer@example.test" + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{DefaultCustomer: &newDefaultCustomer}); err != nil { + t.Fatal(err) + } + link := &models.ZammadTicketLink{ID: "stale-customer", ItemID: item.ID, ProviderID: f.connection.ProviderID, GroupID: 7, GroupName: "Support", CorrelationKey: "stale", SyncState: models.ZammadSyncPending, CreatedBy: &f.actorID} + if err := f.service.reserveZammadTicketLink(&staleConnection, item, link, false); !errors.Is(err, ErrZammadLinkReservationConflict) { + t.Fatalf("stale default customer reserved a link: %v", err) + } + }) + + t.Run("item workspace and key", func(t *testing.T) { + f := newZammadServiceFixture(t, nil) + item, err := repository.NewItemRepository(f.db).FindByIDWithDetails(f.item1) + if err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite("UPDATE items SET workspace_id = ?, workspace_item_number = 2 WHERE id = ?", f.workspace2, f.item1); err != nil { + t.Fatal(err) + } + link := &models.ZammadTicketLink{ID: "stale-item", ItemID: item.ID, ProviderID: f.connection.ProviderID, GroupID: 7, GroupName: "Support", CorrelationKey: "stale", SyncState: models.ZammadSyncPending, CreatedBy: &f.actorID} + if err := f.service.reserveZammadTicketLink(f.connection, item, link, false); !errors.Is(err, ErrZammadLinkReservationConflict) { + t.Fatalf("stale item snapshot reserved a link: %v", err) + } + }) +} + +func TestZammadOutOfScopeLegacyLinkIsNotExposed(t *testing.T) { + f := newZammadServiceFixture(t, nil) + if _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}); err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite("UPDATE items SET workspace_id = ?, workspace_item_number = 2 WHERE id = ?", f.workspace2, f.item1); err != nil { + t.Fatal(err) + } + links, err := f.service.TicketLinksForItem(f.item1) + if err != nil || len(links) != 0 { + t.Fatalf("out-of-scope legacy link was exposed: links=%#v err=%v", links, err) + } +} + +func TestZammadLinkedItemMoveRequiresDestinationConnectionScope(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + var itemTypeID int + if err := f.db.QueryRow("SELECT id FROM item_types WHERE hierarchy_level >= 0 ORDER BY id LIMIT 1").Scan(&itemTypeID); err != nil { + t.Fatal(err) + } + input := ItemWorkspaceMoveInput{DestinationWorkspaceID: f.workspace2, TargetItemTypeID: itemTypeID, TargetStatusID: f.openStatus} + moveService := NewItemWorkspaceMoveService(f.db) + if _, err := moveService.Move(f.item1, f.actorID, input); err == nil { + t.Fatal("item moved outside its linked Zammad connection scope") + } + + expandedScope := []int{f.workspace1, f.workspace2} + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{WorkspaceIDs: &expandedScope}); err != nil { + t.Fatal(err) + } + result, err := moveService.Move(f.item1, f.actorID, input) + if err != nil { + t.Fatalf("item move inside linked Zammad connection scope failed: %v", err) + } + if result.Item.WorkspaceID != f.workspace2 { + t.Fatalf("item remained in workspace %d", result.Item.WorkspaceID) + } + stored, err := f.service.GetTicketLink(link.ID) + if err != nil || stored.CorrelationKey != link.CorrelationKey { + t.Fatalf("allowed item move changed linked ticket identity: link=%#v err=%v", stored, err) + } +} + +func TestZammadDeleteConnectionRejectsExistingTicketLinks(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + if err := f.service.DeleteConnection(f.connection.ProviderID); err == nil { + t.Fatal("connection deletion succeeded despite existing ticket link") + } + var providerCount, credentialCount, linkCount int + if err := f.db.QueryRow("SELECT COUNT(*) FROM integration_providers WHERE id = ?", f.connection.ProviderID).Scan(&providerCount); err != nil { + t.Fatal(err) + } + if err := f.db.QueryRow("SELECT COUNT(*) FROM action_credentials WHERE id = ?", f.connection.CredentialID).Scan(&credentialCount); err != nil { + t.Fatal(err) + } + if err := f.db.QueryRow("SELECT COUNT(*) FROM zammad_ticket_links WHERE id = ?", link.ID).Scan(&linkCount); err != nil { + t.Fatal(err) + } + if providerCount != 1 || credentialCount != 1 || linkCount != 1 { + t.Fatalf("rejected connection delete changed persisted data: providers=%d credentials=%d links=%d", providerCount, credentialCount, linkCount) + } +} + +func TestZammadDeleteConnectionWithoutTicketLinksRemovesOwnedCredentialAtomically(t *testing.T) { + f := newZammadServiceFixture(t, nil) + credentialID := f.connection.CredentialID + if err := f.service.DeleteConnection(f.connection.ProviderID); err != nil { + t.Fatal(err) + } + var providerCount, credentialCount int + if err := f.db.QueryRow("SELECT COUNT(*) FROM integration_providers WHERE id = ?", f.connection.ProviderID).Scan(&providerCount); err != nil { + t.Fatal(err) + } + if err := f.db.QueryRow("SELECT COUNT(*) FROM action_credentials WHERE id = ?", credentialID).Scan(&credentialCount); err != nil { + t.Fatal(err) + } + if providerCount != 0 || credentialCount != 0 { + t.Fatalf("connection delete left data behind: providers=%d credentials=%d", providerCount, credentialCount) + } +} + +func TestZammadConnectionUpdateRollsBackManagedCredentialOnProviderConflict(t *testing.T) { + f := newZammadServiceFixture(t, nil) + other, err := f.service.CreateConnection(models.CreateZammadConnectionRequest{ + Slug: "other-helpdesk", Name: "Other helpdesk", BaseURL: "https://other.example.test", + APIToken: "other-token", DefaultGroupID: 7, DefaultGroupName: "Support", + DefaultCustomer: "robot@example.test", WorkspaceIDs: []int{f.workspace1}, + }, f.actorID) + if err != nil { + t.Fatal(err) + } + newToken := "must-be-rolled-back" + newScope := []int{f.workspace2} + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{Slug: &other.Slug, APIToken: &newToken, WorkspaceIDs: &newScope}); !errors.Is(err, repository.ErrDuplicateEntry) { + t.Fatalf("expected duplicate provider slug, got %v", err) + } + secret, _, err := f.credentials.ResolveManaged(context.Background(), f.connection.CredentialID, f.workspace1, string(models.IntegrationProviderZammad), f.connection.ProviderID) + if err != nil || secret != "synthetic-zammad-token" { + t.Fatalf("managed credential was not restored: secret_matches=%v err=%v", secret == "synthetic-zammad-token", err) + } + if _, _, err := f.credentials.ResolveManaged(context.Background(), f.connection.CredentialID, f.workspace2, string(models.IntegrationProviderZammad), f.connection.ProviderID); !errors.Is(err, ErrCredentialScopeMismatch) { + t.Fatalf("managed credential scope was not restored: %v", err) + } +} + +func TestZammadConcurrentPartialConnectionUpdatesDoNotOverwriteEachOther(t *testing.T) { + f := newZammadServiceFixture(t, nil) + arrived := make(chan struct{}, 2) + release := make(chan struct{}) + f.service.SetUpdateBeforeConnectionLockForTesting(func() { + arrived <- struct{}{} + <-release + }) + defer f.service.SetUpdateBeforeConnectionLockForTesting(nil) + + updatedName := "Concurrent name" + expandedScope := []int{f.workspace1, f.workspace2} + errorsByRequest := make(chan error, 2) + go func() { + _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{Name: &updatedName}) + errorsByRequest <- err + }() + go func() { + _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{WorkspaceIDs: &expandedScope}) + errorsByRequest <- err + }() + + <-arrived + <-arrived + close(release) + firstErr, secondErr := <-errorsByRequest, <-errorsByRequest + conflicts := 0 + for _, err := range []error{firstErr, secondErr} { + if errors.Is(err, repository.ErrConcurrentUpdate) { + conflicts++ + } else if err != nil { + t.Fatalf("unexpected concurrent update error: %v", err) + } + } + if conflicts != 1 { + t.Fatalf("expected exactly one optimistic conflict, got %d: first=%v second=%v", conflicts, firstErr, secondErr) + } + + stored, err := f.service.GetConnection(f.connection.ProviderID) + if err != nil { + t.Fatal(err) + } + nameWon := stored.Name == updatedName && slices.Equal(stored.WorkspaceIDs, []int{f.workspace1}) + scopeWon := stored.Name == f.connection.Name && slices.Equal(stored.WorkspaceIDs, expandedScope) + if !nameWon && !scopeWon { + t.Fatalf("concurrent partial updates produced a mixed or stale result: name=%q workspaces=%v", stored.Name, stored.WorkspaceIDs) + } +} + +func TestZammadSyncUpdatesStatusAndDisabledConnectionsAreSkipped(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + f.transport.getTicket = map[string]any{"id": 901, "number": "420901", "group_id": 7, "state_id": 3, "state": "pending"} + link, err = f.service.SyncTicketLink(context.Background(), link.ID) + if err != nil || link.LastStatusID != 3 || link.LastStatusName != "pending" { + t.Fatalf("unexpected synced link: link=%#v err=%v", link, err) + } + disabled := false + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{Enabled: &disabled}); err != nil { + t.Fatal(err) + } + f.transport.resetRequests() + if err := f.service.SyncDue(context.Background(), 50); err != nil { + t.Fatal(err) + } + requests, _ := f.transport.counts() + if requests != 0 { + t.Fatalf("disabled connection was polled %d times", requests) + } +} + +func TestZammadDisabledConnectionCanStillBeTestedByAdmin(t *testing.T) { + f := newZammadServiceFixture(t, nil) + disabled := false + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{Enabled: &disabled}); err != nil { + t.Fatal(err) + } + metadata, err := f.service.TestConnection(context.Background(), f.connection.ProviderID) + if err != nil { + t.Fatal(err) + } + if len(metadata.Groups) != 1 || metadata.Groups[0].Name != "Support" { + t.Fatalf("unexpected connection test metadata: %#v", metadata) + } + if metadata.GroupCatalogVerified || metadata.CorrelationFieldVerified { + t.Fatalf("least-privilege checks must report admin-only metadata as unverified: %#v", metadata) + } +} + +func TestZammadConnectionValidationRequiresUsableDefaultGroup(t *testing.T) { + f := newZammadServiceFixture(t, nil) + _, err := f.service.CreateConnection(models.CreateZammadConnectionRequest{ + Slug: "missing-default", Name: "Missing default", BaseURL: "https://missing-default.example.test", + APIToken: "token", DefaultCustomer: "robot@example.test", WorkspaceIDs: []int{f.workspace1}, + }, f.actorID) + var validationErr *ZammadValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("connection without a default group was accepted: %v", err) + } + + _, err = f.service.CreateConnection(models.CreateZammadConnectionRequest{ + Slug: "default-outside-allowset", Name: "Default outside allowset", BaseURL: "https://default-outside.example.test", + APIToken: "token", DefaultGroupID: 7, AllowedGroups: zammadTestGroupRefs(8), + DefaultCustomer: "robot@example.test", WorkspaceIDs: []int{f.workspace1}, + }, f.actorID) + if !errors.As(err, &validationErr) { + t.Fatalf("default group outside allowset was accepted: %v", err) + } +} + +func TestZammadLegacyGroupCatalogCanBeSavedUnchanged(t *testing.T) { + f := newZammadServiceFixture(t, nil) + connection, err := f.service.CreateConnection(models.CreateZammadConnectionRequest{ + Slug: "legacy-groups", Name: "Legacy groups", BaseURL: "https://legacy-groups.example.test", + APIToken: "token", DefaultGroupID: 7, DefaultGroupName: "Support", AllowedGroupIDs: []int{7, 8}, + DefaultCustomer: "robot@example.test", WorkspaceIDs: []int{f.workspace1}, + }, f.actorID) + if err != nil { + t.Fatal(err) + } + if len(connection.AllowedGroups) != 2 || connection.AllowedGroups[1].ID != 8 || connection.AllowedGroups[1].Name != "" { + t.Fatalf("unexpected legacy catalog: %#v", connection.AllowedGroups) + } + name := "Legacy groups renamed" + updated, err := f.service.UpdateConnection(connection.ProviderID, models.UpdateZammadConnectionRequest{ + Name: &name, AllowedGroups: &connection.AllowedGroups, + }) + if err != nil { + t.Fatalf("unchanged legacy group catalog blocked an unrelated update: %v", err) + } + if updated.Name != name || len(updated.AllowedGroups) != 2 || updated.AllowedGroups[1].Name != "" { + t.Fatalf("legacy group catalog was not preserved: %#v", updated) + } +} + +func TestZammadMetadataValidatesOnlyRuntimeTicketStates(t *testing.T) { + connection := &models.ZammadConnection{DefaultGroupName: "Support", AllowedGroups: zammadTestGroupRefs(8), ClosedStateIDs: []int{4}} + metadata := &models.ZammadConnectionMetadata{ + Groups: []models.ZammadGroup{{ID: 99, Name: "Different", Active: true}}, + States: []models.ZammadState{{ID: 2, Name: "open", Active: true}}, + } + var validationErr *ZammadValidationError + if err := validateZammadStates(connection, metadata); !errors.As(err, &validationErr) { + t.Fatalf("missing configured closed state was accepted: %v", err) + } +} + +func TestZammadUnknownTicketDoesNotChangeItem(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + f.transport.getStatus = http.StatusNotFound + if _, err := f.service.SyncTicketLink(context.Background(), link.ID); err == nil { + t.Fatal("expected remote not-found error") + } + var statusID int + if err := f.db.QueryRow("SELECT status_id FROM items WHERE id = ?", f.item1).Scan(&statusID); err != nil { + t.Fatal(err) + } + if statusID != f.openStatus { + t.Fatalf("unknown remote ticket changed item status to %d", statusID) + } + stored, err := f.service.GetTicketLink(link.ID) + if err != nil { + t.Fatal(err) + } + if strings.Contains(stored.LastError, "synthetic remote detail") { + t.Fatalf("remote response body leaked into stored error: %q", stored.LastError) + } +} + +func TestZammadClosedStateCompletesItemOnce(t *testing.T) { + workflow := &fakeZammadWorkflow{} + f := newZammadServiceFixture(t, workflow) + workflow.db = f.db + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + closed := []int{4} + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{ClosedStateIDs: &closed, CompletionStatusID: &f.doneStatus}); err != nil { + t.Fatal(err) + } + f.transport.getTicket = map[string]any{"id": 901, "number": "420901", "group_id": 7, "state_id": 4, "state": "closed"} + if _, err := f.service.SyncTicketLink(context.Background(), link.ID); err != nil { + t.Fatal(err) + } + if _, err := f.service.SyncTicketLink(context.Background(), link.ID); err != nil { + t.Fatal(err) + } + var statusID int + if err := f.db.QueryRow("SELECT status_id FROM items WHERE id = ?", f.item1).Scan(&statusID); err != nil { + t.Fatal(err) + } + if statusID != f.doneStatus || workflow.writes != 1 { + t.Fatalf("closed-state mapping was not idempotent: status=%d writes=%d", statusID, workflow.writes) + } + if _, err := f.db.ExecWrite("UPDATE items SET status_id = ? WHERE id = ?", f.openStatus, f.item1); err != nil { + t.Fatal(err) + } + if _, err := f.service.SyncTicketLink(context.Background(), link.ID); err != nil { + t.Fatal(err) + } + if err := f.db.QueryRow("SELECT status_id FROM items WHERE id = ?", f.item1).Scan(&statusID); err != nil { + t.Fatal(err) + } + if statusID != f.openStatus || workflow.writes != 1 { + t.Fatalf("same remote closed episode re-completed a reopened item: status=%d writes=%d", statusID, workflow.writes) + } + f.transport.getTicket = map[string]any{"id": 901, "number": "420901", "group_id": 7, "state_id": 2, "state": "open"} + if _, err := f.service.SyncTicketLink(context.Background(), link.ID); err != nil { + t.Fatal(err) + } + f.transport.getTicket = map[string]any{"id": 901, "number": "420901", "group_id": 7, "state_id": 4, "state": "closed"} + if _, err := f.service.SyncTicketLink(context.Background(), link.ID); err != nil { + t.Fatal(err) + } + if workflow.writes != 2 { + t.Fatalf("new remote closed episode did not complete the item: writes=%d", workflow.writes) + } +} + +func TestZammadCompletionPolicyCannotChangeDuringClaimedTransition(t *testing.T) { + workflow := &fakeZammadWorkflow{} + f := newZammadServiceFixture(t, workflow) + workflow.db = f.db + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + closed := []int{4} + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{ClosedStateIDs: &closed, CompletionStatusID: &f.doneStatus}); err != nil { + t.Fatal(err) + } + f.transport.getTicket = map[string]any{"id": 901, "number": "420901", "group_id": 7, "state_id": 4, "state": "closed"} + arrived := make(chan struct{}, 1) + release := make(chan struct{}) + f.service.SetCompletionBeforeTransitionForTesting(func() { + arrived <- struct{}{} + <-release + }) + defer f.service.SetCompletionBeforeTransitionForTesting(nil) + + syncErr := make(chan error, 1) + go func() { + _, err := f.service.SyncTicketLink(context.Background(), link.ID) + syncErr <- err + }() + <-arrived + noClosedStates := []int{} + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{ + ClosedStateIDs: &noClosedStates, ClearCompletionStatus: true, + }); !errors.Is(err, ErrZammadConnectionBusy) { + t.Fatalf("completion policy changed during a claimed transition: %v", err) + } + close(release) + if err := <-syncErr; err != nil { + t.Fatalf("claimed transition failed after conflicting config update was rejected: %v", err) + } + stored, err := f.service.GetTicketLink(link.ID) + if err != nil { + t.Fatal(err) + } + if workflow.writes != 1 || !stored.CompletionApplied { + t.Fatalf("completion did not commit exactly once: writes=%d link=%#v", workflow.writes, stored) + } +} + +func TestZammadPostCommitTransitionErrorStillFencesClosedEpisode(t *testing.T) { + workflow := &fakeZammadWorkflow{postCommitErr: errors.New("synthetic post-commit hook failure")} + f := newZammadServiceFixture(t, workflow) + workflow.db = f.db + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + closed := []int{4} + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{ClosedStateIDs: &closed, CompletionStatusID: &f.doneStatus}); err != nil { + t.Fatal(err) + } + f.transport.getTicket = map[string]any{"id": link.TicketID, "number": link.TicketNumber, "group_id": 7, "state_id": 4, "state": "closed"} + if _, err := f.service.SyncTicketLink(context.Background(), link.ID); err == nil { + t.Fatal("post-commit hook error was not reported") + } + stored, err := f.service.GetTicketLink(link.ID) + if err != nil { + t.Fatal(err) + } + if workflow.writes != 1 || !stored.CompletionApplied { + t.Fatalf("committed transition was not fenced after hook error: writes=%d link=%#v", workflow.writes, stored) + } + if _, err := f.db.ExecWrite("UPDATE items SET status_id = ? WHERE id = ?", f.openStatus, f.item1); err != nil { + t.Fatal(err) + } + workflow.postCommitErr = nil + if _, err := f.service.SyncTicketLink(context.Background(), link.ID); err != nil { + t.Fatal(err) + } + var statusID int + if err := f.db.QueryRow("SELECT status_id FROM items WHERE id = ?", f.item1).Scan(&statusID); err != nil { + t.Fatal(err) + } + if workflow.writes != 1 || statusID != f.openStatus { + t.Fatalf("same remote closed episode replayed after post-commit error: writes=%d status=%d", workflow.writes, statusID) + } +} + +func TestZammadClaimedSyncBlocksConcurrentUnlinkBeforeCompletion(t *testing.T) { + workflow := &fakeZammadWorkflow{} + f := newZammadServiceFixture(t, workflow) + workflow.db = f.db + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + closed := []int{4} + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{ClosedStateIDs: &closed, CompletionStatusID: &f.doneStatus}); err != nil { + t.Fatal(err) + } + f.transport.getTicket = map[string]any{"id": link.TicketID, "number": link.TicketNumber, "group_id": 7, "state_id": 4, "state": "closed"} + arrived := make(chan struct{}, 1) + release := make(chan struct{}) + f.service.SetCompletionBeforeTransitionForTesting(func() { + arrived <- struct{}{} + <-release + }) + defer f.service.SetCompletionBeforeTransitionForTesting(nil) + + syncErr := make(chan error, 1) + go func() { + _, err := f.service.SyncTicketLink(context.Background(), link.ID) + syncErr <- err + }() + <-arrived + if _, err := f.service.UnlinkTicket(context.Background(), link.ID); !errors.Is(err, ErrZammadConnectionBusy) { + t.Fatalf("unlink entered a claimed sync before completion: %v", err) + } + close(release) + if err := <-syncErr; err != nil { + t.Fatalf("claimed sync failed after concurrent unlink was rejected: %v", err) + } + stored, err := f.service.GetTicketLink(link.ID) + if err != nil { + t.Fatalf("rejected unlink removed the local link: %v", err) + } + if workflow.writes != 1 || !stored.CompletionApplied { + t.Fatalf("claimed sync did not complete exactly once: writes=%d link=%#v", workflow.writes, stored) + } +} + +func TestZammadExpiredSyncLeaseReloadsChangedCompletionPolicyBeforeTransition(t *testing.T) { + workflow := &fakeZammadWorkflow{} + f := newZammadServiceFixture(t, workflow) + workflow.db = f.db + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + closed := []int{4} + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{ClosedStateIDs: &closed, CompletionStatusID: &f.doneStatus}); err != nil { + t.Fatal(err) + } + f.transport.getTicket = map[string]any{"id": link.TicketID, "number": link.TicketNumber, "group_id": 7, "state_id": 4, "state": "closed"} + arrived := make(chan struct{}, 1) + release := make(chan struct{}) + f.service.SetCompletionBeforeTransitionForTesting(func() { + arrived <- struct{}{} + <-release + }) + defer f.service.SetCompletionBeforeTransitionForTesting(nil) + + syncErr := make(chan error, 1) + go func() { + _, err := f.service.SyncTicketLink(context.Background(), link.ID) + syncErr <- err + }() + <-arrived + if _, err := f.db.ExecWrite("UPDATE zammad_ticket_links SET sync_lock_until = ? WHERE id = ?", time.Now().Add(-time.Minute), link.ID); err != nil { + t.Fatal(err) + } + noClosedStates := []int{} + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{ + ClosedStateIDs: &noClosedStates, ClearCompletionStatus: true, + }); err != nil { + t.Fatalf("completion policy could not change after the synthetic lease expiry: %v", err) + } + close(release) + if err := <-syncErr; err != nil { + t.Fatalf("sync did not recover its untaken lease under the current policy: %v", err) + } + var statusID int + if err := f.db.QueryRow("SELECT status_id FROM items WHERE id = ?", f.item1).Scan(&statusID); err != nil { + t.Fatal(err) + } + if workflow.writes != 0 || statusID != f.openStatus { + t.Fatalf("expired worker applied the superseded completion policy: writes=%d status=%d", workflow.writes, statusID) + } +} + +func TestZammadTicketCreationClaimBlocksConcurrentUnlinkBeforeCompletion(t *testing.T) { + f := newZammadServiceFixture(t, nil) + arrived := make(chan struct{}, 1) + release := make(chan struct{}) + f.service.SetPersistBeforeConnectionLockForTesting(func() { + arrived <- struct{}{} + <-release + }) + defer f.service.SetPersistBeforeConnectionLockForTesting(nil) + + type createResult struct { + link *models.ZammadTicketLink + err error + } + result := make(chan createResult, 1) + go func() { + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + result <- createResult{link: link, err: err} + }() + <-arrived + links, err := f.service.TicketLinksForItem(f.item1) + if err != nil || len(links) != 1 { + t.Fatalf("creation reservation was not visible: links=%#v err=%v", links, err) + } + if _, err := f.service.UnlinkTicket(context.Background(), links[0].ID); !errors.Is(err, ErrZammadConnectionBusy) { + t.Fatalf("unlink deleted a claimed creation before local completion: %v", err) + } + close(release) + created := <-result + if created.err != nil || created.link == nil || created.link.ItemIntegrationLinkID == "" { + t.Fatalf("ticket creation did not complete after unlink was rejected: link=%#v err=%v", created.link, created.err) + } + var genericLinks int + if err := f.db.QueryRow("SELECT COUNT(*) FROM item_integration_links WHERE id = ?", created.link.ItemIntegrationLinkID).Scan(&genericLinks); err != nil { + t.Fatal(err) + } + if genericLinks != 1 { + t.Fatalf("ticket creation committed %d generic links, want 1", genericLinks) + } +} + +func TestZammadExistingTicketClaimBlocksConcurrentUnlinkBeforeCompletion(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.transport.ticket = map[string]any{"id": 714, "number": "420714", "group_id": 7, "state_id": 2, "state": "open"} + arrived := make(chan struct{}, 1) + release := make(chan struct{}) + f.service.SetPersistBeforeConnectionLockForTesting(func() { + arrived <- struct{}{} + <-release + }) + defer f.service.SetPersistBeforeConnectionLockForTesting(nil) + + type linkResult struct { + link *models.ZammadTicketLink + err error + } + result := make(chan linkResult, 1) + go func() { + link, err := f.service.LinkExistingTicket(context.Background(), f.item1, f.actorID, models.LinkZammadTicketRequest{ + ConnectionID: f.connection.ProviderID, TicketNumber: "420714", + }) + result <- linkResult{link: link, err: err} + }() + <-arrived + links, err := f.service.TicketLinksForItem(f.item1) + if err != nil || len(links) != 1 { + t.Fatalf("existing-ticket reservation was not visible: links=%#v err=%v", links, err) + } + if _, err := f.service.UnlinkTicket(context.Background(), links[0].ID); !errors.Is(err, ErrZammadConnectionBusy) { + t.Fatalf("unlink deleted a claimed existing-ticket link before completion: %v", err) + } + close(release) + linked := <-result + if linked.err != nil || linked.link == nil || linked.link.ItemIntegrationLinkID == "" { + t.Fatalf("existing-ticket link did not complete after unlink was rejected: link=%#v err=%v", linked.link, linked.err) + } + var genericLinks int + if err := f.db.QueryRow("SELECT COUNT(*) FROM item_integration_links WHERE id = ?", linked.link.ItemIntegrationLinkID).Scan(&genericLinks); err != nil { + t.Fatal(err) + } + if genericLinks != 1 { + t.Fatalf("existing-ticket completion committed %d generic links, want 1", genericLinks) + } +} + +func TestZammadSyncReloadsCompletionStateAfterClaim(t *testing.T) { + workflow := &fakeZammadWorkflow{} + f := newZammadServiceFixture(t, workflow) + workflow.db = f.db + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + closedStates := []int{4} + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{ClosedStateIDs: &closedStates, CompletionStatusID: &f.doneStatus}); err != nil { + t.Fatal(err) + } + stale, err := f.service.GetTicketLink(link.ID) + if err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite("UPDATE zammad_ticket_links SET completion_applied = true WHERE id = ?", link.ID); err != nil { + t.Fatal(err) + } + f.transport.getTicket = map[string]any{"id": link.TicketID, "number": link.TicketNumber, "group_id": 7, "state_id": 4, "state": "closed"} + syncOwner := "stale-completion-test" + claimed, err := f.service.repo.ClaimSync(link.ID, syncOwner, time.Now().Add(time.Minute)) + if err != nil || !claimed { + t.Fatalf("could not claim stale-link regression sync: claimed=%v err=%v", claimed, err) + } + updated, err := f.service.syncClaimedTicketLink(context.Background(), stale, syncOwner) + if err != nil { + t.Fatal(err) + } + if workflow.writes != 0 || !updated.CompletionApplied { + t.Fatalf("stale completion state was replayed: writes=%d link=%#v", workflow.writes, updated) + } + var statusID int + if err := f.db.QueryRow("SELECT status_id FROM items WHERE id = ?", f.item1).Scan(&statusID); err != nil || statusID != f.openStatus { + t.Fatalf("stale sync changed item status: status=%d err=%v", statusID, err) + } +} + +func TestZammadManualRefreshReportsBusyWhenAnotherWorkerOwnsLease(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + claimed, err := f.service.repo.ClaimSync(link.ID, "scheduler-owner", time.Now().Add(time.Minute)) + if err != nil || !claimed { + t.Fatalf("scheduler could not claim ticket: claimed=%v err=%v", claimed, err) + } + if _, err := f.service.SyncTicketLink(context.Background(), link.ID); !errors.Is(err, ErrZammadConnectionBusy) { + t.Fatalf("manual refresh reported success while scheduler owned the lease: %v", err) + } + if err := f.service.repo.ReleaseSyncClaim(link.ID, "scheduler-owner"); err != nil { + t.Fatal(err) + } +} + +func TestZammadSyncUsesPersistedGroupPolicyWithoutAdminLookup(t *testing.T) { + workflow := &fakeZammadWorkflow{} + f := newZammadServiceFixture(t, workflow) + workflow.db = f.db + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + closedStates := []int{4} + if _, err := f.service.UpdateConnection(f.connection.ProviderID, models.UpdateZammadConnectionRequest{ClosedStateIDs: &closedStates, CompletionStatusID: &f.doneStatus}); err != nil { + t.Fatal(err) + } + f.transport.groups = []map[string]any{{"id": 7, "name": "Support", "active": false}} + f.transport.getTicket = map[string]any{"id": link.TicketID, "number": link.TicketNumber, "group_id": 7, "state_id": 4, "state": "closed"} + _, err = f.service.SyncTicketLink(context.Background(), link.ID) + if err != nil { + t.Fatalf("persisted group policy required an admin-only lookup: %v", err) + } + stored, getErr := f.service.GetTicketLink(link.ID) + if getErr != nil || workflow.writes != 1 || !stored.CompletionApplied { + t.Fatalf("persisted group policy did not complete normal sync: link=%#v writes=%d err=%v", stored, workflow.writes, getErr) + } +} + +func TestNormalizeZammadBaseURL(t *testing.T) { + tests := []struct { + input string + want string + err bool + }{ + {input: "https://support.example.test/desk/", want: "https://support.example.test/desk"}, + {input: "http://support.example.test", err: true}, + {input: "https://user:secret@support.example.test", err: true}, + {input: "https://support.example.test?token=secret", err: true}, + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + got, err := NormalizeZammadBaseURL(tt.input) + if (err != nil) != tt.err || got != tt.want { + t.Fatalf("NormalizeZammadBaseURL(%q) = %q, %v", tt.input, got, err) + } + }) + } +} + +func TestZammadDueSyncUsesAgeThreshold(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite("UPDATE zammad_ticket_links SET last_synced_at = ? WHERE id = ?", time.Now().Add(-10*time.Minute), link.ID); err != nil { + t.Fatal(err) + } + f.transport.resetRequests() + if err := f.service.SyncDue(context.Background(), 10); err != nil { + t.Fatal(err) + } + requests, _ := f.transport.counts() + if requests != 1 { + t.Fatalf("expected only the ticket refresh, got %d transport requests", requests) + } +} + +func TestZammadDueSyncPollsFreshLinkOnNextSchedulerTick(t *testing.T) { + f := newZammadServiceFixture(t, nil) + if _, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}); err != nil { + t.Fatal(err) + } + f.transport.resetRequests() + if err := f.service.SyncDue(context.Background(), 10); err != nil { + t.Fatal(err) + } + requests, _ := f.transport.counts() + if requests != 1 { + t.Fatalf("fresh link missed the next scheduler tick: requests=%d", requests) + } +} + +func TestZammadSyncAllTicketLinksOverridesRetryDelay(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + if _, err := f.db.ExecWrite("UPDATE zammad_ticket_links SET next_attempt_at = ? WHERE id = ?", time.Now().Add(time.Hour), link.ID); err != nil { + t.Fatal(err) + } + f.transport.resetRequests() + summary, err := f.service.SyncAllTicketLinks(context.Background()) + if err != nil { + t.Fatal(err) + } + if summary.Selected != 1 || summary.Succeeded != 1 || summary.Failed != 0 || summary.Skipped != 0 { + t.Fatalf("unexpected system refresh summary: %#v", summary) + } + requests, _ := f.transport.counts() + if requests != 1 { + t.Fatalf("system refresh did not override the retry delay: requests=%d", requests) + } +} + +func TestZammadSyncAllTicketLinksReportsAlreadyClaimedLinks(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + claimed, err := f.service.repo.ClaimSync(link.ID, "other-worker", time.Now().Add(time.Minute)) + if err != nil || !claimed { + t.Fatalf("failed to create competing claim: claimed=%v err=%v", claimed, err) + } + f.transport.resetRequests() + summary, err := f.service.SyncAllTicketLinks(context.Background()) + if err != nil { + t.Fatal(err) + } + if summary.Selected != 1 || summary.Succeeded != 0 || summary.Failed != 0 || summary.Skipped != 1 { + t.Fatalf("claimed link disappeared from system refresh summary: %#v", summary) + } + requests, _ := f.transport.counts() + if requests != 0 { + t.Fatalf("claimed link was refreshed concurrently: requests=%d", requests) + } +} + +func TestZammadTicketSyncPublishesLivePanelUpdate(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + publisher := &recordingItemChangePublisher{} + SetItemChangePublisher(publisher) + t.Cleanup(func() { SetItemChangePublisher(nil) }) + + if _, err := f.service.SyncTicketLink(context.Background(), link.ID); err != nil { + t.Fatal(err) + } + if publisher.itemID != f.item1 || publisher.kind != ItemChangeZammad { + t.Fatalf("unexpected live-update event: item=%d kind=%q", publisher.itemID, publisher.kind) + } +} + +func TestZammadTicketSyncFailurePublishesLivePanelUpdate(t *testing.T) { + f := newZammadServiceFixture(t, nil) + link, err := f.service.CreateTicket(context.Background(), f.item1, f.actorID, models.CreateZammadTicketRequest{ConnectionID: f.connection.ProviderID}) + if err != nil { + t.Fatal(err) + } + publisher := &recordingItemChangePublisher{} + SetItemChangePublisher(publisher) + t.Cleanup(func() { SetItemChangePublisher(nil) }) + f.transport.getStatus = http.StatusBadGateway + + if _, err := f.service.SyncTicketLink(context.Background(), link.ID); err == nil { + t.Fatal("expected upstream refresh failure") + } + if publisher.itemID != f.item1 || publisher.kind != ItemChangeZammad { + t.Fatalf("failure did not publish a live-update event: item=%d kind=%q", publisher.itemID, publisher.kind) + } + stored, err := f.service.GetTicketLink(link.ID) + if err != nil || stored.LastError == "" || stored.SyncState != models.ZammadSyncFailed { + t.Fatalf("failure state was not persisted: link=%#v err=%v", stored, err) + } +} + +func TestZammadOAuthConnectionStoresConnectionTokensAndConsumesStateOnce(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.service.SetOAuthEncryption(sso.NewSecretEncryption("synthetic-server-secret-for-zammad-tests")) + f.service.SetOAuthTransportForTesting(zammad.TransportFunc(func(_ context.Context, method, targetURL string, body []byte, headers map[string]string) (*zammad.Response, error) { + if method != http.MethodPost || targetURL != "https://oauth.example.test/oauth/token" || headers["Content-Type"] != "application/x-www-form-urlencoded" || strings.Contains(string(body), "synthetic-client-secret") == false { + t.Fatalf("unexpected OAuth token request: %s %s %q %#v", method, targetURL, body, headers) + } + return jsonResponse(http.StatusOK, map[string]any{"access_token": "oauth-access-token", "refresh_token": "oauth-refresh-token", "expires_in": 7200}), nil + })) + connection, err := f.service.CreateConnection(models.CreateZammadConnectionRequest{ + Slug: "oauth-helpdesk", Name: "OAuth helpdesk", BaseURL: "https://oauth.example.test", + AuthMethod: models.ZammadAuthMethodOAuth, OAuthClientID: "synthetic-client", OAuthClientSecret: "synthetic-client-secret", + DefaultGroupName: "Support", DefaultCustomer: "robot@example.test", WorkspaceIDs: []int{f.workspace1}, + }, f.actorID) + if err != nil { + t.Fatal(err) + } + if connection.CredentialID <= 0 || connection.OAuthConnected || connection.HasAPIToken { + t.Fatalf("OAuth connection should have a non-token pending credential before callback: %#v", connection) + } + pending, _, err := f.credentials.ResolveManaged(context.Background(), connection.CredentialID, f.workspace1, string(models.IntegrationProviderZammad), connection.ProviderID) + if err != nil || strings.Contains(pending, "token") || !strings.Contains(pending, "pending") { + t.Fatalf("OAuth pending credential is unsafe: %q, %v", pending, err) + } + var credentialsBefore int + if err := f.db.QueryRow("SELECT COUNT(*) FROM action_credentials").Scan(&credentialsBefore); err != nil { + t.Fatal(err) + } + if _, err := f.service.CreateConnection(models.CreateZammadConnectionRequest{ + Slug: "oauth-helpdesk", Name: "Duplicate OAuth helpdesk", BaseURL: "https://duplicate.example.test", + AuthMethod: models.ZammadAuthMethodOAuth, OAuthClientID: "client", OAuthClientSecret: "secret", DefaultGroupName: "Support", DefaultCustomer: "robot@example.test", WorkspaceIDs: []int{f.workspace1}, + }, f.actorID); !errors.Is(err, repository.ErrDuplicateEntry) { + t.Fatalf("expected duplicate OAuth connection error, got %v", err) + } + var credentialsAfter int + if err := f.db.QueryRow("SELECT COUNT(*) FROM action_credentials").Scan(&credentialsAfter); err != nil || credentialsAfter != credentialsBefore { + t.Fatalf("failed OAuth creation left a managed credential behind: before=%d after=%d err=%v", credentialsBefore, credentialsAfter, err) + } + authURL, err := f.service.StartOAuth(context.Background(), connection.ProviderID, f.actorID, "https://windshift.example.test") + if err != nil { + t.Fatal(err) + } + parsed, err := url.Parse(authURL) + if err != nil { + t.Fatal(err) + } + if parsed.Path != "/oauth/authorize" || parsed.Query().Get("scope") != "full" || parsed.Query().Get("redirect_uri") != "https://windshift.example.test/api/integrations/oauth/system/zammad/callback" { + t.Fatalf("unexpected authorization URL: %s", authURL) + } + if _, err := f.service.StartOAuth(context.Background(), connection.ProviderID, f.actorID, "http://windshift.example.test"); err == nil { + t.Fatal("OAuth start accepted a non-HTTPS public base URL") + } + abortedURL, err := f.service.StartOAuth(context.Background(), connection.ProviderID, f.actorID, "https://windshift.example.test") + if err != nil { + t.Fatal(err) + } + abortedState, err := url.Parse(abortedURL) + if err != nil { + t.Fatal(err) + } + if err := f.service.InvalidateOAuthState(abortedState.Query().Get("state")); err != nil { + t.Fatal(err) + } + invalidated, err := f.service.GetConnection(connection.ProviderID) + if err != nil { + t.Fatal(err) + } + if invalidated.OAuthAttemptID != "" { + t.Fatalf("invalidated OAuth state retained connection attempt %q", invalidated.OAuthAttemptID) + } + if _, err := f.service.CompleteOAuth(context.Background(), abortedState.Query().Get("state"), "aborted-code", "https://windshift.example.test"); !errors.Is(err, repository.ErrNotFound) { + t.Fatalf("aborted OAuth state remained usable: %v", err) + } + usableURL, err := f.service.StartOAuth(context.Background(), connection.ProviderID, f.actorID, "https://windshift.example.test") + if err != nil { + t.Fatal(err) + } + usableState, err := url.Parse(usableURL) + if err != nil { + t.Fatal(err) + } + state := usableState.Query().Get("state") + if _, err := f.service.CompleteOAuth(context.Background(), state, "authorization-code", "https://windshift.example.test"); err != nil { + t.Fatal(err) + } + completed, err := f.service.GetConnection(connection.ProviderID) + if err != nil || !completed.OAuthConnected || completed.HasAPIToken { + t.Fatalf("unexpected OAuth connection status after callback: %#v, %v", completed, err) + } + if _, err := f.service.StartOAuth(context.Background(), connection.ProviderID, f.actorID, "https://windshift.example.test"); err != nil { + t.Fatal(err) + } + newClientID := "rotated-client" + reset, err := f.service.UpdateConnection(connection.ProviderID, models.UpdateZammadConnectionRequest{OAuthClientID: &newClientID}) + if err != nil || reset.OAuthConnected || reset.ReauthorizationRequired || reset.HasAPIToken { + t.Fatalf("OAuth credential change did not reset authorization: %#v, %v", reset, err) + } + var stateCount, tokenCount int + if err := f.db.QueryRow("SELECT COUNT(*) FROM zammad_oauth_state WHERE provider_id = ?", connection.ProviderID).Scan(&stateCount); err != nil { + t.Fatal(err) + } + if err := f.db.QueryRow("SELECT COUNT(*) FROM zammad_oauth_tokens WHERE provider_id = ?", connection.ProviderID).Scan(&tokenCount); err != nil { + t.Fatal(err) + } + if stateCount != 0 || tokenCount != 0 { + t.Fatalf("OAuth reset retained state=%d token=%d", stateCount, tokenCount) + } + bundle, err := activeZammadOAuthCredential("access", "refresh") + if err != nil { + t.Fatal(err) + } + if _, err := f.credentials.RotateManaged(connection.CredentialID, models.RotateActionCredentialRequest{Secret: bundle}, string(models.IntegrationProviderZammad), connection.ProviderID); err != nil { + t.Fatal(err) + } + if err := repository.NewZammadRepository(f.db).UpsertOAuthToken(repository.ZammadOAuthToken{ProviderID: connection.ProviderID, ExpiresAt: time.Now().Add(time.Hour)}); err != nil { + t.Fatal(err) + } + if _, err := f.service.StartOAuth(context.Background(), connection.ProviderID, f.actorID, "https://windshift.example.test"); err != nil { + t.Fatal(err) + } + newBaseURL := "https://other-zammad.example.test" + reset, err = f.service.UpdateConnection(connection.ProviderID, models.UpdateZammadConnectionRequest{BaseURL: &newBaseURL}) + if err != nil || reset.OAuthConnected || reset.ReauthorizationRequired { + t.Fatalf("OAuth base URL change did not reset authorization: %#v, %v", reset, err) + } + if err := f.db.QueryRow("SELECT COUNT(*) FROM zammad_oauth_state WHERE provider_id = ?", connection.ProviderID).Scan(&stateCount); err != nil { + t.Fatal(err) + } + if err := f.db.QueryRow("SELECT COUNT(*) FROM zammad_oauth_tokens WHERE provider_id = ?", connection.ProviderID).Scan(&tokenCount); err != nil { + t.Fatal(err) + } + if stateCount != 0 || tokenCount != 0 { + t.Fatalf("OAuth base URL reset retained state=%d token=%d", stateCount, tokenCount) + } + if _, err := f.service.CompleteOAuth(context.Background(), state, "authorization-code", "https://windshift.example.test"); !errors.Is(err, repository.ErrNotFound) { + t.Fatalf("OAuth state was not consumed exactly once: %v", err) + } + var encryptedBundle string + if err := f.db.QueryRow("SELECT encrypted_secret FROM action_credentials WHERE id = ?", connection.CredentialID).Scan(&encryptedBundle); err != nil { + t.Fatal(err) + } + if encryptedBundle == "oauth-access-token" || strings.Contains(encryptedBundle, "oauth-access-token") || strings.Contains(encryptedBundle, "oauth-refresh-token") { + t.Fatal("OAuth tokens were not encrypted at rest") + } +} + +func createActiveZammadOAuthConnection(t *testing.T, f *zammadServiceFixture, slug string, expiresAt time.Time) *models.ZammadConnection { + t.Helper() + f.service.SetOAuthEncryption(sso.NewSecretEncryption("synthetic-server-secret-for-zammad-tests")) + connection, err := f.service.CreateConnection(models.CreateZammadConnectionRequest{ + Slug: slug, Name: "OAuth " + slug, BaseURL: "https://" + slug + ".example.test", + AuthMethod: models.ZammadAuthMethodOAuth, OAuthClientID: "client", OAuthClientSecret: "secret", + DefaultGroupName: "Support", DefaultCustomer: "robot@example.test", WorkspaceIDs: []int{f.workspace1}, + }, f.actorID) + if err != nil { + t.Fatal(err) + } + bundle, err := activeZammadOAuthCredential("old-access", "old-refresh") + if err != nil { + t.Fatal(err) + } + if _, err := f.credentials.RotateManaged(connection.CredentialID, models.RotateActionCredentialRequest{Secret: bundle}, string(models.IntegrationProviderZammad), connection.ProviderID); err != nil { + t.Fatal(err) + } + if err := repository.NewZammadRepository(f.db).UpsertOAuthToken(repository.ZammadOAuthToken{ + ProviderID: connection.ProviderID, OAuthGeneration: connection.OAuthGeneration, ExpiresAt: expiresAt, + }); err != nil { + t.Fatal(err) + } + connection, err = f.service.GetConnection(connection.ProviderID) + if err != nil { + t.Fatal(err) + } + return connection +} + +func assertZammadManagedCredentialMetadata(t *testing.T, f *zammadServiceFixture, connection *models.ZammadConnection, wantName string, wantWorkspaceID int) { + t.Helper() + credential, err := repository.NewActionCredentialRepository(f.db).GetActionCredentialByID(connection.CredentialID) + if err != nil { + t.Fatal(err) + } + if credential.Name != wantName || credential.AppliesToAllWorkspaces || len(credential.WorkspaceIDs) != 1 || credential.WorkspaceIDs[0] != wantWorkspaceID { + t.Fatalf("OAuth secret write overwrote concurrent credential metadata: %#v", credential) + } +} + +func TestZammadOAuthCallbackCannotCommitAfterConfigurationReset(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.service.SetOAuthEncryption(sso.NewSecretEncryption("synthetic-server-secret-for-zammad-tests")) + connection, err := f.service.CreateConnection(models.CreateZammadConnectionRequest{ + Slug: "callback-race", Name: "OAuth callback race", BaseURL: "https://callback-race.example.test", + AuthMethod: models.ZammadAuthMethodOAuth, OAuthClientID: "client", OAuthClientSecret: "secret", + DefaultGroupName: "Support", DefaultCustomer: "robot@example.test", WorkspaceIDs: []int{f.workspace1}, + }, f.actorID) + if err != nil { + t.Fatal(err) + } + authURL, err := f.service.StartOAuth(context.Background(), connection.ProviderID, f.actorID, "https://windshift.example.test") + if err != nil { + t.Fatal(err) + } + parsed, err := url.Parse(authURL) + if err != nil { + t.Fatal(err) + } + entered, release := make(chan struct{}), make(chan struct{}) + f.service.SetOAuthTransportForTesting(zammad.TransportFunc(func(_ context.Context, _ string, _ string, _ []byte, _ map[string]string) (*zammad.Response, error) { + close(entered) + <-release + return jsonResponse(http.StatusOK, map[string]any{"access_token": "stale-access", "refresh_token": "stale-refresh", "expires_in": 3600}), nil + })) + type callbackResult struct { + err error + } + done := make(chan callbackResult, 1) + go func() { + _, err := f.service.CompleteOAuth(context.Background(), parsed.Query().Get("state"), "code", "https://windshift.example.test") + done <- callbackResult{err: err} + }() + select { + case <-entered: + case <-time.After(5 * time.Second): + t.Fatal("OAuth callback did not reach token exchange") + } + newClientID := "replacement-client" + reset, err := f.service.UpdateConnection(connection.ProviderID, models.UpdateZammadConnectionRequest{OAuthClientID: &newClientID}) + if err != nil { + t.Fatal(err) + } + close(release) + result := <-done + if !errors.Is(result.err, ErrZammadOAuthSuperseded) { + t.Fatalf("stale callback commit error = %v", result.err) + } + if reset.OAuthGeneration <= connection.OAuthGeneration || reset.OAuthConnected { + t.Fatalf("reset did not advance generation and clear authorization: %#v", reset) + } + raw, _, err := f.credentials.ResolveManaged(context.Background(), connection.CredentialID, f.workspace1, string(models.IntegrationProviderZammad), connection.ProviderID) + if err != nil || !strings.Contains(raw, `"status":"pending"`) || strings.Contains(raw, "stale-access") { + t.Fatalf("stale callback reactivated credential: raw=%q err=%v", raw, err) + } +} + +func TestZammadOAuthCallbackCannotCommitAfterConnectionIsDisabled(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.service.SetOAuthEncryption(sso.NewSecretEncryption("synthetic-server-secret-for-zammad-tests")) + connection, err := f.service.CreateConnection(models.CreateZammadConnectionRequest{ + Slug: "callback-disable", Name: "OAuth callback disable", BaseURL: "https://callback-disable.example.test", + AuthMethod: models.ZammadAuthMethodOAuth, OAuthClientID: "client", OAuthClientSecret: "secret", + DefaultGroupName: "Support", DefaultCustomer: "robot@example.test", WorkspaceIDs: []int{f.workspace1}, + }, f.actorID) + if err != nil { + t.Fatal(err) + } + authURL, err := f.service.StartOAuth(context.Background(), connection.ProviderID, f.actorID, "https://windshift.example.test") + if err != nil { + t.Fatal(err) + } + parsed, err := url.Parse(authURL) + if err != nil { + t.Fatal(err) + } + entered, release := make(chan struct{}), make(chan struct{}) + f.service.SetOAuthTransportForTesting(zammad.TransportFunc(func(_ context.Context, _ string, _ string, _ []byte, _ map[string]string) (*zammad.Response, error) { + close(entered) + <-release + return jsonResponse(http.StatusOK, map[string]any{"access_token": "stale-access", "refresh_token": "stale-refresh", "expires_in": 3600}), nil + })) + done := make(chan error, 1) + go func() { + _, callbackErr := f.service.CompleteOAuth(context.Background(), parsed.Query().Get("state"), "code", "https://windshift.example.test") + done <- callbackErr + }() + select { + case <-entered: + case <-time.After(5 * time.Second): + t.Fatal("OAuth callback did not reach token exchange") + } + disabled := false + if _, err := f.service.UpdateConnection(connection.ProviderID, models.UpdateZammadConnectionRequest{Enabled: &disabled}); err != nil { + t.Fatal(err) + } + close(release) + if err := <-done; !errors.Is(err, ErrZammadOAuthSuperseded) { + t.Fatalf("disabled connection accepted OAuth credentials: %v", err) + } + if _, err := f.service.StartOAuth(context.Background(), connection.ProviderID, f.actorID, "https://windshift.example.test"); err == nil { + t.Fatal("disabled connection allowed a new OAuth attempt") + } +} + +func TestZammadOAuthCallbackPreservesConcurrentNameAndScopeUpdate(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.service.SetOAuthEncryption(sso.NewSecretEncryption("synthetic-server-secret-for-zammad-tests")) + connection, err := f.service.CreateConnection(models.CreateZammadConnectionRequest{ + Slug: "callback-metadata-race", Name: "Original callback name", BaseURL: "https://callback-metadata-race.example.test", + AuthMethod: models.ZammadAuthMethodOAuth, OAuthClientID: "client", OAuthClientSecret: "secret", + DefaultGroupName: "Support", DefaultCustomer: "robot@example.test", WorkspaceIDs: []int{f.workspace1}, + }, f.actorID) + if err != nil { + t.Fatal(err) + } + authURL, err := f.service.StartOAuth(context.Background(), connection.ProviderID, f.actorID, "https://windshift.example.test") + if err != nil { + t.Fatal(err) + } + parsed, err := url.Parse(authURL) + if err != nil { + t.Fatal(err) + } + entered, release := make(chan struct{}), make(chan struct{}) + f.service.SetOAuthTransportForTesting(zammad.TransportFunc(func(_ context.Context, _ string, _ string, _ []byte, _ map[string]string) (*zammad.Response, error) { + close(entered) + <-release + return jsonResponse(http.StatusOK, map[string]any{"access_token": "callback-access", "refresh_token": "callback-refresh", "expires_in": 3600}), nil + })) + done := make(chan error, 1) + go func() { + _, err := f.service.CompleteOAuth(context.Background(), parsed.Query().Get("state"), "code", "https://windshift.example.test") + done <- err + }() + select { + case <-entered: + case <-time.After(5 * time.Second): + t.Fatal("OAuth callback did not reach token exchange") + } + updatedName := "Renamed during callback" + updatedScope := []int{f.workspace2} + if _, err := f.service.UpdateConnection(connection.ProviderID, models.UpdateZammadConnectionRequest{Name: &updatedName, WorkspaceIDs: &updatedScope}); err != nil { + t.Fatal(err) + } + close(release) + if err := <-done; err != nil { + t.Fatal(err) + } + assertZammadManagedCredentialMetadata(t, f, connection, updatedName+" Zammad OAuth credentials", f.workspace2) + raw, _, err := f.credentials.ResolveManaged(context.Background(), connection.CredentialID, f.workspace2, string(models.IntegrationProviderZammad), connection.ProviderID) + if err != nil { + t.Fatal(err) + } + bundle, err := parseZammadOAuthCredential(raw) + if err != nil || bundle.AccessToken != "callback-access" { + t.Fatalf("callback secret was not committed: bundle=%#v err=%v", bundle, err) + } +} + +func TestZammadOAuthParallelStartsLeaveOnlyNewestUsableState(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.service.SetOAuthEncryption(sso.NewSecretEncryption("synthetic-server-secret-for-zammad-tests")) + connection, err := f.service.CreateConnection(models.CreateZammadConnectionRequest{ + Slug: "parallel-state", Name: "Parallel OAuth state", BaseURL: "https://parallel-state.example.test", + AuthMethod: models.ZammadAuthMethodOAuth, OAuthClientID: "client", OAuthClientSecret: "secret", + DefaultGroupName: "Support", DefaultCustomer: "robot@example.test", WorkspaceIDs: []int{f.workspace1}, + }, f.actorID) + if err != nil { + t.Fatal(err) + } + f.service.SetOAuthTransportForTesting(zammad.TransportFunc(func(_ context.Context, _ string, _ string, _ []byte, _ map[string]string) (*zammad.Response, error) { + return jsonResponse(http.StatusOK, map[string]any{"access_token": "parallel-access", "refresh_token": "parallel-refresh", "expires_in": 3600}), nil + })) + type startResult struct { + state string + err error + } + results := make(chan startResult, 2) + start := func() { + authURL, err := f.service.StartOAuth(context.Background(), connection.ProviderID, f.actorID, "https://windshift.example.test") + if err != nil { + results <- startResult{err: err} + return + } + parsed, parseErr := url.Parse(authURL) + if parseErr != nil { + results <- startResult{err: parseErr} + return + } + results <- startResult{state: parsed.Query().Get("state")} + } + go start() + go start() + first, second := <-results, <-results + if first.err != nil || second.err != nil || first.state == "" || second.state == "" || first.state == second.state { + t.Fatalf("parallel OAuth starts failed: first=%#v second=%#v", first, second) + } + var persistedState string + var stateCount int + if err := f.db.QueryRow("SELECT state FROM zammad_oauth_state WHERE provider_id = ?", connection.ProviderID).Scan(&persistedState); err != nil { + t.Fatal(err) + } + if err := f.db.QueryRow("SELECT COUNT(*) FROM zammad_oauth_state WHERE provider_id = ?", connection.ProviderID).Scan(&stateCount); err != nil || stateCount != 1 { + t.Fatalf("parallel starts retained %d states: %v", stateCount, err) + } + staleState := first.state + if persistedState == first.state { + staleState = second.state + } + if _, err := f.service.CompleteOAuth(context.Background(), staleState, "stale-code", "https://windshift.example.test"); !errors.Is(err, repository.ErrNotFound) { + t.Fatalf("superseded parallel state remained usable: %v", err) + } + if _, err := f.service.CompleteOAuth(context.Background(), persistedState, "winning-code", "https://windshift.example.test"); err != nil { + t.Fatalf("winning parallel state failed: %v", err) + } +} + +func TestZammadOAuthNewStartSupersedesAlreadyRunningCallback(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.service.SetOAuthEncryption(sso.NewSecretEncryption("synthetic-server-secret-for-zammad-tests")) + connection, err := f.service.CreateConnection(models.CreateZammadConnectionRequest{ + Slug: "running-callback", Name: "Running OAuth callback", BaseURL: "https://running-callback.example.test", + AuthMethod: models.ZammadAuthMethodOAuth, OAuthClientID: "client", OAuthClientSecret: "secret", + DefaultGroupName: "Support", DefaultCustomer: "robot@example.test", WorkspaceIDs: []int{f.workspace1}, + }, f.actorID) + if err != nil { + t.Fatal(err) + } + oldURL, err := f.service.StartOAuth(context.Background(), connection.ProviderID, f.actorID, "https://windshift.example.test") + if err != nil { + t.Fatal(err) + } + oldParsed, err := url.Parse(oldURL) + if err != nil { + t.Fatal(err) + } + entered, release := make(chan struct{}), make(chan struct{}) + var transportMu sync.Mutex + transportCalls := 0 + f.service.SetOAuthTransportForTesting(zammad.TransportFunc(func(_ context.Context, _ string, _ string, _ []byte, _ map[string]string) (*zammad.Response, error) { + transportMu.Lock() + transportCalls++ + call := transportCalls + transportMu.Unlock() + if call == 1 { + close(entered) + <-release + return jsonResponse(http.StatusOK, map[string]any{"access_token": "superseded-access", "refresh_token": "superseded-refresh", "expires_in": 3600}), nil + } + return jsonResponse(http.StatusOK, map[string]any{"access_token": "winning-access", "refresh_token": "winning-refresh", "expires_in": 3600}), nil + })) + oldDone := make(chan error, 1) + go func() { + _, err := f.service.CompleteOAuth(context.Background(), oldParsed.Query().Get("state"), "old-code", "https://windshift.example.test") + oldDone <- err + }() + select { + case <-entered: + case <-time.After(5 * time.Second): + t.Fatal("old callback did not reach token exchange") + } + newURL, err := f.service.StartOAuth(context.Background(), connection.ProviderID, f.actorID, "https://windshift.example.test") + if err != nil { + t.Fatal(err) + } + newParsed, err := url.Parse(newURL) + if err != nil { + t.Fatal(err) + } + close(release) + if err := <-oldDone; !errors.Is(err, ErrZammadOAuthSuperseded) { + t.Fatalf("already-running old callback commit error = %v", err) + } + if _, err := f.service.CompleteOAuth(context.Background(), newParsed.Query().Get("state"), "new-code", "https://windshift.example.test"); err != nil { + t.Fatalf("new callback failed after superseding running callback: %v", err) + } + raw, _, err := f.credentials.ResolveManaged(context.Background(), connection.CredentialID, f.workspace1, string(models.IntegrationProviderZammad), connection.ProviderID) + if err != nil { + t.Fatal(err) + } + bundle, err := parseZammadOAuthCredential(raw) + if err != nil || bundle.AccessToken != "winning-access" || bundle.RefreshToken != "winning-refresh" { + t.Fatalf("superseded callback overwrote winning tokens: bundle=%#v err=%v", bundle, err) + } +} + +func TestZammadOAuthRefreshCannotCommitAfterConfigurationReset(t *testing.T) { + f := newZammadServiceFixture(t, nil) + connection := createActiveZammadOAuthConnection(t, f, "refresh-race", time.Now().Add(-time.Minute)) + entered, release := make(chan struct{}), make(chan struct{}) + f.service.SetOAuthTransportForTesting(zammad.TransportFunc(func(_ context.Context, _ string, _ string, _ []byte, _ map[string]string) (*zammad.Response, error) { + close(entered) + <-release + return jsonResponse(http.StatusOK, map[string]any{"access_token": "stale-refreshed-access", "refresh_token": "stale-refreshed-refresh", "expires_in": 3600}), nil + })) + done := make(chan error, 1) + go func() { + _, err := f.service.oauthAccessToken(context.Background(), connection, f.workspace1) + done <- err + }() + select { + case <-entered: + case <-time.After(5 * time.Second): + t.Fatal("OAuth refresh did not reach token exchange") + } + newClientID := "replacement-client" + reset, err := f.service.UpdateConnection(connection.ProviderID, models.UpdateZammadConnectionRequest{OAuthClientID: &newClientID}) + if err != nil { + t.Fatal(err) + } + close(release) + if err := <-done; !errors.Is(err, ErrZammadOAuthSuperseded) { + t.Fatalf("stale refresh commit error = %v", err) + } + if reset.OAuthGeneration <= connection.OAuthGeneration || reset.OAuthConnected { + t.Fatalf("reset did not advance generation and clear authorization: %#v", reset) + } + raw, _, err := f.credentials.ResolveManaged(context.Background(), connection.CredentialID, f.workspace1, string(models.IntegrationProviderZammad), connection.ProviderID) + if err != nil || !strings.Contains(raw, `"status":"pending"`) || strings.Contains(raw, "stale-refreshed-access") { + t.Fatalf("stale refresh reactivated credential: raw=%q err=%v", raw, err) + } +} + +func TestZammadOAuthRefreshPreservesConcurrentNameAndScopeUpdate(t *testing.T) { + f := newZammadServiceFixture(t, nil) + connection := createActiveZammadOAuthConnection(t, f, "refresh-metadata-race", time.Now().Add(-time.Minute)) + entered, release := make(chan struct{}), make(chan struct{}) + f.service.SetOAuthTransportForTesting(zammad.TransportFunc(func(_ context.Context, _ string, _ string, _ []byte, _ map[string]string) (*zammad.Response, error) { + close(entered) + <-release + return jsonResponse(http.StatusOK, map[string]any{"access_token": "refreshed-access", "refresh_token": "refreshed-refresh", "expires_in": 3600}), nil + })) + done := make(chan struct { + token string + err error + }, 1) + go func() { + token, err := f.service.oauthAccessToken(context.Background(), connection, f.workspace1) + done <- struct { + token string + err error + }{token: token, err: err} + }() + select { + case <-entered: + case <-time.After(5 * time.Second): + t.Fatal("OAuth refresh did not reach token exchange") + } + updatedName := "Renamed during refresh" + updatedScope := []int{f.workspace2} + if _, err := f.service.UpdateConnection(connection.ProviderID, models.UpdateZammadConnectionRequest{Name: &updatedName, WorkspaceIDs: &updatedScope}); err != nil { + t.Fatal(err) + } + close(release) + result := <-done + if result.err != nil || result.token != "refreshed-access" { + t.Fatalf("refresh result: token=%q err=%v", result.token, result.err) + } + assertZammadManagedCredentialMetadata(t, f, connection, updatedName+" Zammad OAuth credentials", f.workspace2) +} + +type zammadWaitNotifyingContext struct { + context.Context + once sync.Once + waiting chan struct{} +} + +func (c *zammadWaitNotifyingContext) Done() <-chan struct{} { + c.once.Do(func() { close(c.waiting) }) + return c.Context.Done() +} + +func TestZammadOAuthConcurrentRefreshWaitsForRotatedCredential(t *testing.T) { + f := newZammadServiceFixture(t, nil) + connection := createActiveZammadOAuthConnection(t, f, "refresh-contention", time.Now().Add(-time.Minute)) + entered, release := make(chan struct{}), make(chan struct{}) + releaseFirst := sync.OnceFunc(func() { close(release) }) + t.Cleanup(releaseFirst) + var transportMu sync.Mutex + transportCalls := 0 + f.service.SetOAuthTransportForTesting(zammad.TransportFunc(func(_ context.Context, _ string, _ string, _ []byte, _ map[string]string) (*zammad.Response, error) { + transportMu.Lock() + transportCalls++ + call := transportCalls + transportMu.Unlock() + if call == 1 { + close(entered) + select { + case <-release: + case <-time.After(5 * time.Second): + return nil, errors.New("timed out waiting to release first OAuth refresh") + } + } + return jsonResponse(http.StatusOK, map[string]any{"access_token": "shared-refreshed-access", "refresh_token": "shared-refreshed-refresh", "expires_in": 3600}), nil + })) + type tokenResult struct { + token string + err error + } + results := make(chan tokenResult, 2) + refresh := func(ctx context.Context) { + token, err := f.service.oauthAccessToken(ctx, connection, f.workspace1) + results <- tokenResult{token: token, err: err} + } + go refresh(context.Background()) + select { + case <-entered: + case <-time.After(5 * time.Second): + t.Fatal("first OAuth refresh did not reach token exchange") + } + secondCtx, cancelSecond := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelSecond() + waiting := make(chan struct{}) + go refresh(&zammadWaitNotifyingContext{Context: secondCtx, waiting: waiting}) + select { + case <-waiting: + // Done() is first used in oauthAccessToken's failed-claim wait select. + case <-secondCtx.Done(): + t.Fatal("contending refresh never reached the claim wait") + } + transportMu.Lock() + callsBeforeRelease := transportCalls + transportMu.Unlock() + if callsBeforeRelease != 1 { + t.Fatalf("contending request issued a second refresh: calls=%d", callsBeforeRelease) + } + select { + case result := <-results: + t.Fatalf("refresh completed before releasing first claim: %+v", result) + default: + } + releaseFirst() + readResult := func() tokenResult { + t.Helper() + select { + case result := <-results: + return result + case <-time.After(5 * time.Second): + t.Fatal("timed out waiting for OAuth refresh result") + return tokenResult{} + } + } + first, second := readResult(), readResult() + for i, result := range []tokenResult{first, second} { + if result.err != nil || result.token != "shared-refreshed-access" { + t.Fatalf("refresh result %d did not reuse rotated access token: token=%q err=%v", i+1, result.token, result.err) + } + } + transportMu.Lock() + finalCalls := transportCalls + transportMu.Unlock() + if finalCalls != 1 { + t.Fatalf("parallel refresh used the refresh token %d times", finalCalls) + } +} + +func TestZammadOAuthRefreshCommitRequiresCurrentClaimOwner(t *testing.T) { + f := newZammadServiceFixture(t, nil) + connection := createActiveZammadOAuthConnection(t, f, "claim-loss", time.Now().Add(-time.Minute)) + entered, release := make(chan struct{}), make(chan struct{}) + f.service.SetOAuthTransportForTesting(zammad.TransportFunc(func(_ context.Context, _ string, _ string, _ []byte, _ map[string]string) (*zammad.Response, error) { + close(entered) + <-release + return jsonResponse(http.StatusOK, map[string]any{"access_token": "claim-lost-access", "refresh_token": "claim-lost-refresh", "expires_in": 3600}), nil + })) + done := make(chan error, 1) + go func() { + _, err := f.service.oauthAccessToken(context.Background(), connection, f.workspace1) + done <- err + }() + select { + case <-entered: + case <-time.After(5 * time.Second): + t.Fatal("OAuth refresh did not reach token exchange") + } + var claimOwner string + var leaseUntil time.Time + if err := f.db.QueryRow("SELECT refresh_claim_owner, refresh_lock_until FROM zammad_oauth_tokens WHERE provider_id = ?", connection.ProviderID).Scan(&claimOwner, &leaseUntil); err != nil { + t.Fatal(err) + } + if claimOwner == "" || time.Until(leaseUntil) <= zammadHTTPTimeout { + t.Fatalf("refresh lease is not safely longer than HTTP timeout: owner=%q remaining=%s", claimOwner, time.Until(leaseUntil)) + } + if _, err := f.db.ExecWrite("UPDATE zammad_oauth_tokens SET refresh_claim_owner = ? WHERE provider_id = ?", "replacement-owner", connection.ProviderID); err != nil { + t.Fatal(err) + } + close(release) + if err := <-done; !errors.Is(err, ErrZammadOAuthSuperseded) { + t.Fatalf("claim-lost refresh commit error = %v", err) + } + raw, _, err := f.credentials.ResolveManaged(context.Background(), connection.CredentialID, f.workspace1, string(models.IntegrationProviderZammad), connection.ProviderID) + if err != nil { + t.Fatal(err) + } + bundle, err := parseZammadOAuthCredential(raw) + if err != nil || bundle.AccessToken != "old-access" || bundle.RefreshToken != "old-refresh" { + t.Fatalf("claim-lost refresh overwrote credential: bundle=%#v err=%v", bundle, err) + } +} + +func TestZammadOAuthRefreshRereadsRotatedCredentialAfterClaim(t *testing.T) { + f := newZammadServiceFixture(t, nil) + connection := createActiveZammadOAuthConnection(t, f, "refresh-reread", time.Now().Add(-time.Minute)) + beforeClaim, continueClaim := make(chan struct{}), make(chan struct{}) + f.service.SetOAuthBeforeRefreshClaimForTesting(func() { + close(beforeClaim) + <-continueClaim + }) + var transportMu sync.Mutex + transportCalls := 0 + f.service.SetOAuthTransportForTesting(zammad.TransportFunc(func(_ context.Context, _ string, _ string, _ []byte, _ map[string]string) (*zammad.Response, error) { + transportMu.Lock() + transportCalls++ + transportMu.Unlock() + return jsonResponse(http.StatusOK, map[string]any{"access_token": "unexpected-access", "refresh_token": "unexpected-refresh", "expires_in": 3600}), nil + })) + done := make(chan struct { + token string + err error + }, 1) + go func() { + token, err := f.service.oauthAccessToken(context.Background(), connection, f.workspace1) + done <- struct { + token string + err error + }{token: token, err: err} + }() + select { + case <-beforeClaim: + case <-time.After(5 * time.Second): + t.Fatal("second refresh request did not pause before its claim") + } + rotatedBundle, err := activeZammadOAuthCredential("already-refreshed-access", "already-rotated-refresh") + if err != nil { + t.Fatal(err) + } + if _, err := f.credentials.RotateManaged(connection.CredentialID, models.RotateActionCredentialRequest{Secret: rotatedBundle}, string(models.IntegrationProviderZammad), connection.ProviderID); err != nil { + t.Fatal(err) + } + if err := repository.NewZammadRepository(f.db).UpsertOAuthToken(repository.ZammadOAuthToken{ + ProviderID: connection.ProviderID, OAuthGeneration: connection.OAuthGeneration, ExpiresAt: time.Now().Add(time.Hour), + }); err != nil { + t.Fatal(err) + } + close(continueClaim) + result := <-done + if result.err != nil || result.token != "already-refreshed-access" { + t.Fatalf("second request did not reuse freshly rotated credential: token=%q err=%v", result.token, result.err) + } + transportMu.Lock() + calls := transportCalls + transportMu.Unlock() + if calls != 0 { + t.Fatalf("second request reused a stale refresh token in %d upstream calls", calls) + } + var owner *string + if err := f.db.QueryRow("SELECT refresh_claim_owner FROM zammad_oauth_tokens WHERE provider_id = ?", connection.ProviderID).Scan(&owner); err != nil { + t.Fatal(err) + } + if owner != nil { + t.Fatalf("fresh-token fast path retained refresh claim %q", *owner) + } +} + +func TestZammadOAuthInvalidGrantReturnsPersistenceFailure(t *testing.T) { + f := newZammadServiceFixture(t, nil) + connection := createActiveZammadOAuthConnection(t, f, "invalid-grant-persist", time.Now().Add(-time.Minute)) + f.service.SetOAuthTransportForTesting(zammad.TransportFunc(func(_ context.Context, _ string, _ string, _ []byte, _ map[string]string) (*zammad.Response, error) { + return jsonResponse(http.StatusBadRequest, map[string]string{"error": "invalid_grant"}), nil + })) + if _, err := f.db.ExecWrite(`CREATE TRIGGER fail_zammad_reauthorization + BEFORE UPDATE ON action_credentials + BEGIN SELECT RAISE(ABORT, 'synthetic credential persistence failure'); END`); err != nil { + t.Fatal(err) + } + _, err := f.service.oauthAccessToken(context.Background(), connection, f.workspace1) + if err == nil || errors.Is(err, ErrZammadReauthorizationRequired) { + t.Fatalf("invalid_grant persistence failure was hidden: %v", err) + } + token, tokenErr := repository.NewZammadRepository(f.db).GetOAuthToken(connection.ProviderID) + if tokenErr != nil || token.ReauthorizationRequired { + t.Fatalf("failed transaction partially marked reauthorization: token=%#v err=%v", token, tokenErr) + } + raw, _, resolveErr := f.credentials.ResolveManaged(context.Background(), connection.CredentialID, f.workspace1, string(models.IntegrationProviderZammad), connection.ProviderID) + if resolveErr != nil { + t.Fatal(resolveErr) + } + bundle, parseErr := parseZammadOAuthCredential(raw) + if parseErr != nil || bundle.AccessToken != "old-access" { + t.Fatalf("failed transaction partially replaced credential: bundle=%#v err=%v", bundle, parseErr) + } +} + +func TestZammadOAuthInvalidGrantPreservesConcurrentNameAndScopeUpdate(t *testing.T) { + f := newZammadServiceFixture(t, nil) + connection := createActiveZammadOAuthConnection(t, f, "invalid-grant-metadata-race", time.Now().Add(-time.Minute)) + entered, release := make(chan struct{}), make(chan struct{}) + f.service.SetOAuthTransportForTesting(zammad.TransportFunc(func(_ context.Context, _ string, _ string, _ []byte, _ map[string]string) (*zammad.Response, error) { + close(entered) + <-release + return jsonResponse(http.StatusBadRequest, map[string]string{"error": "invalid_grant"}), nil + })) + done := make(chan error, 1) + go func() { + _, err := f.service.oauthAccessToken(context.Background(), connection, f.workspace1) + done <- err + }() + select { + case <-entered: + case <-time.After(5 * time.Second): + t.Fatal("OAuth invalid_grant test did not reach token exchange") + } + updatedName := "Renamed during invalid grant" + updatedScope := []int{f.workspace2} + if _, err := f.service.UpdateConnection(connection.ProviderID, models.UpdateZammadConnectionRequest{Name: &updatedName, WorkspaceIDs: &updatedScope}); err != nil { + t.Fatal(err) + } + close(release) + if err := <-done; !errors.Is(err, ErrZammadReauthorizationRequired) { + t.Fatalf("invalid_grant result = %v", err) + } + assertZammadManagedCredentialMetadata(t, f, connection, updatedName+" Zammad OAuth credentials", f.workspace2) + raw, _, err := f.credentials.ResolveManaged(context.Background(), connection.CredentialID, f.workspace2, string(models.IntegrationProviderZammad), connection.ProviderID) + if err != nil || !strings.Contains(raw, `"status":"reauthorization_required"`) { + t.Fatalf("invalid_grant did not persist reauthorization secret: raw=%q err=%v", raw, err) + } +} + +func TestZammadOAuthTestSkipsAdminOnlyCorrelationFieldCheck(t *testing.T) { + f := newZammadServiceFixture(t, nil) + f.service.SetOAuthEncryption(sso.NewSecretEncryption("synthetic-server-secret-for-zammad-tests")) + connection, err := f.service.CreateConnection(models.CreateZammadConnectionRequest{ + Slug: "oauth-scope", Name: "Scoped OAuth", BaseURL: "https://scope.example.test", AuthMethod: models.ZammadAuthMethodOAuth, + OAuthClientID: "client", OAuthClientSecret: "secret", DefaultGroupID: 7, DefaultGroupName: "Support", + AllowedGroups: zammadTestGroupRefs(7), DefaultCustomer: "robot@example.test", WorkspaceIDs: []int{f.workspace1}, + }, f.actorID) + if err != nil { + t.Fatal(err) + } + bundle, err := activeZammadOAuthCredential("access", "refresh") + if err != nil { + t.Fatal(err) + } + if _, err := f.credentials.RotateManaged(connection.CredentialID, models.RotateActionCredentialRequest{Secret: bundle}, string(models.IntegrationProviderZammad), connection.ProviderID); err != nil { + t.Fatal(err) + } + if err := repository.NewZammadRepository(f.db).UpsertOAuthToken(repository.ZammadOAuthToken{ProviderID: connection.ProviderID, ExpiresAt: time.Now().Add(time.Hour)}); err != nil { + t.Fatal(err) + } + calledObjectManager := false + f.service.SetTransportForTesting(zammad.TransportFunc(func(_ context.Context, _ string, targetURL string, _ []byte, _ map[string]string) (*zammad.Response, error) { + if strings.Contains(targetURL, "object_manager_attributes") { + calledObjectManager = true + return jsonResponse(http.StatusForbidden, map[string]string{}), nil + } + return jsonResponse(http.StatusOK, []map[string]any{{"id": 2, "name": "open", "active": true}}), nil + })) + metadata, err := f.service.TestConnection(context.Background(), connection.ProviderID) + if err != nil || metadata.CorrelationFieldVerified || calledObjectManager { + t.Fatalf("OAuth test must not require admin.object: metadata=%#v err=%v called=%v", metadata, err, calledObjectManager) + } +} + +func TestZammadAPITokenTestSkipsAdminOnlyCorrelationFieldCheck(t *testing.T) { + f := newZammadServiceFixture(t, nil) + calledObjectManager := false + f.service.SetTransportForTesting(zammad.TransportFunc(func(_ context.Context, _ string, targetURL string, _ []byte, _ map[string]string) (*zammad.Response, error) { + if strings.Contains(targetURL, "object_manager_attributes") { + calledObjectManager = true + return jsonResponse(http.StatusForbidden, map[string]string{}), nil + } + return jsonResponse(http.StatusOK, []map[string]any{{"id": 2, "name": "open", "active": true}}), nil + })) + metadata, err := f.service.TestConnection(context.Background(), f.connection.ProviderID) + if err != nil || metadata.CorrelationFieldVerified || calledObjectManager { + t.Fatalf("API-token test must not require admin.object: metadata=%#v err=%v called=%v", metadata, err, calledObjectManager) + } +} + +func TestZammadSafeTransportHonorsAllowLocalConnections(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/ticket_states" { + t.Fatalf("unexpected path %q", r.URL.Path) + } + _, _ = w.Write([]byte(`[]`)) + })) + defer server.Close() + previous := utils.AllowLocalConnections() + defer utils.SetAllowLocalConnections(previous) + transport := newZammadSafeTransport(server.URL, "/api/v1/") + utils.SetAllowLocalConnections(true) + if _, err := transport.Do(context.Background(), http.MethodGet, server.URL+"/api/v1/ticket_states", nil, nil); err != nil { + t.Fatalf("ALLOW_LOCAL_CONNECTIONS=true blocked local Zammad target: %v", err) + } + utils.SetAllowLocalConnections(false) + if _, err := transport.Do(context.Background(), http.MethodGet, server.URL+"/api/v1/ticket_states", nil, nil); !errors.Is(err, utils.ErrBlockedSSRFAddr) { + t.Fatalf("ALLOW_LOCAL_CONNECTIONS=false did not block local Zammad target: %v", err) + } +} + +func TestZammadClientsUseExactlyOneExpectedAuthorizationScheme(t *testing.T) { + for _, testCase := range []struct { + name, want string + client *zammad.Client + }{ + {name: "legacy", want: "Token token=legacy-token", client: zammad.NewClient("https://zammad.example.test", "legacy-token", zammad.TransportFunc(func(_ context.Context, _ string, _ string, _ []byte, headers map[string]string) (*zammad.Response, error) { + if headers["Authorization"] != "Token token=legacy-token" { + t.Fatalf("legacy authorization = %q", headers["Authorization"]) + } + return jsonResponse(http.StatusOK, []map[string]any{}), nil + }))}, + {name: "oauth", want: "Bearer oauth-token", client: zammad.NewOAuthClient("https://zammad.example.test", "oauth-token", zammad.TransportFunc(func(_ context.Context, _ string, _ string, _ []byte, headers map[string]string) (*zammad.Response, error) { + if headers["Authorization"] != "Bearer oauth-token" { + t.Fatalf("OAuth authorization = %q", headers["Authorization"]) + } + return jsonResponse(http.StatusOK, []map[string]any{}), nil + }))}, + } { + t.Run(testCase.name, func(t *testing.T) { + if _, err := testCase.client.States(context.Background()); err != nil { + t.Fatal(err) + } + }) + } +}