diff --git a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java index b2fa63925..da2d1c40a 100644 --- a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java +++ b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java @@ -102,6 +102,7 @@ public DepositNoticeDto createDraft(long ticketId, DepositNoticeDraftRequest req public DepositNoticeDto createDraftFromQuotation(long ticketId, DepositNoticeDraftRequest req, Integer sourceDepositPercent, UserPrincipal actor) { requireRole(actor, SALES_ROLES); + requireTicketOwner(ticketId, actor); TicketSummaryDto s = requireApprovedTicket(ticketId, actor); requireActiveLifecycle(s); diff --git a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java index 08b841ab7..1815e4db0 100644 --- a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java @@ -326,6 +326,34 @@ void createDepositNoticeFromQuotation_newEngine_nonOwnerSales_cannotReach() { Map.of("id", ticketId), Long.class)).isZero(); } + /** The ticket route (POST /api/tickets/{id}/deposit-notice/draft) had no owner check, unlike + * update/issue: another rep could draft on this deal and read its lines, prices and customer. */ + @Test + void createDraft_ticketRoute_byNonOwningSales_refusedAndWritesNothing() { + var emptyRequest = new DepositNoticeDraftRequest(null, null, null, null, null, null, null, null); + // Owner before status: a non-owner must not learn whether another rep's deal is approved yet + // (403 here, not the 409 the owner would get on this still-unapproved ticket). + assertThatThrownBy(() -> depositNoticeService.createDraft(ticketId, emptyRequest, otherSalesActor)) + .isInstanceOf(ApiException.class) + .hasFieldOrPropertyWithValue("status", HttpStatus.FORBIDDEN); + + DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); + orderConfirmation.confirmOrder(accepted.pricingRequestId(), + new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); + + assertThatThrownBy(() -> depositNoticeService.createDraft(ticketId, emptyRequest, otherSalesActor)) + .isInstanceOf(ApiException.class) + .hasFieldOrPropertyWithValue("status", HttpStatus.FORBIDDEN) + .hasMessageContaining("ไม่มีสิทธิ์เข้าถึงรายการนี้"); + assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM sales.deposit_notice WHERE ticket_id = :id", + Map.of("id", ticketId), Long.class)).isZero(); + + // Positive control: the owner still drafts on the same deal. + assertThat(depositNoticeService.createDraft(ticketId, emptyRequest, salesActor).ticketId()).isEqualTo(ticketId); + assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM sales.deposit_notice WHERE ticket_id = :id", + Map.of("id", ticketId), Long.class)).isEqualTo(1L); + } + /** An open revision draft outranks the accepted quotation by id: recordOutcome has no * open-revision guard, so "latest for this pricing request" is not "accepted". */ @Test diff --git a/frontend/src/api/mockApi.depositNotices.test.js b/frontend/src/api/mockApi.depositNotices.test.js index bc1a8e747..9e497e2a7 100644 --- a/frontend/src/api/mockApi.depositNotices.test.js +++ b/frontend/src/api/mockApi.depositNotices.test.js @@ -252,6 +252,24 @@ describe('mockApi.pricingRequests.createDepositNoticeFromQuotation — new-engin expect(pick(fromTicket.items)).toEqual(pick(fromPr.items)); }); + // Mirrors DepositNoticeService.createDraftFromQuotation's owner guard. sales2@glr.co.th is a + // second seeded sales rep; the drive above runs as the first one, who owns the deal. + it('ticket-level createDraft refuses a sales rep who does not own the deal and creates nothing', async () => { + const { ticketId } = await driveTicketToNewEngineAcceptedQuotation(); + await api.auth.login({ email: 'sales2@glr.co.th', password: 'demo1234' }); + await expect(api.depositNotices.createDraft(ticketId, {})) + .rejects.toMatchObject({ status: 403, message: 'ไม่มีสิทธิ์เข้าถึงรายการนี้' }); + // Role gate (SALES_ROLES = sales only) runs before the ticket lookup, as in Java: a non-sales + // caller gets 403 even for a ticket that does not exist, never the 404. + await api.auth.login({ role: 'ceo' }); + await expect(api.depositNotices.createDraft(999999, {})).rejects.toMatchObject({ status: 403 }); + + await api.auth.login({ role: 'sales' }); + expect((await api.depositNotices.listByTicket(ticketId)).depositNotices).toHaveLength(0); + const { depositNotice } = await api.depositNotices.createDraft(ticketId, {}); + expect(depositNotice.ticketId).toBe(ticketId); + }); + it('still 409s while the new-engine quotation is only ISSUED (customer has not accepted)', async () => { const { ticketId, prId } = await driveTicketToNewEngineIssuedQuotation(); await expect(api.pricingRequests.createDepositNoticeFromQuotation(prId, {})) diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index d17302ee8..6e76f3ec5 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -13265,8 +13265,10 @@ export const api = { }, async createDraft(ticketId, payload) { - requireSession(); + // Mirrors DepositNoticeService.createDraftFromQuotation: requireRole(SALES_ROLES) + requireTicketOwner. + const user = hasRole('sales'); const ticket = findTicketRaw(Number(ticketId)); + if (ticket.createdById !== user.id) fail('ไม่มีสิทธิ์เข้าถึงรายการนี้', 403); // Phase 1 lifecycle gate (mirrors DepositNoticeService.createDraft). requireActive(ticket); if (!['approved', 'quotation_issued', 'document_issued'].includes(ticket.status)) fail('ดีลต้องได้รับการอนุมัติก่อน', 409);