Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.*;

import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.List;

@Controller
Expand All @@ -27,22 +29,28 @@ public TicketCommentController(TicketCommentService commentService,
@PostMapping("/{ticketId}/comments")
public String addComment(
@PathVariable Long ticketId,
@Valid @ModelAttribute CommentCreateDTO dto
@Valid @ModelAttribute CommentCreateDTO dto,
@RequestParam(required = false) String token
) {
User user = getCurrentUserOrNull(); // unauthenticated users currently have no access; anonymous flow will be added later.
User user = getCurrentUserOrNull();

commentService.addComment(ticketId, dto, user);
commentService.addComment(ticketId, dto, user, token);

return "redirect:/view/id/" + ticketId;
String base = "redirect:/view/id/" + ticketId;
return token != null
? base + "?token=" + URLEncoder.encode(token, StandardCharsets.UTF_8)
: base;
}

@GetMapping("/{ticketId}/comments")
@ResponseBody
public List<CommentViewDTO> getComments(@PathVariable Long ticketId) {

public List<CommentViewDTO> getComments(
@PathVariable Long ticketId,
@RequestParam(required = false) String token
) {
User user = getCurrentUserOrNull();

return commentService.getComments(ticketId, user);
return commentService.getComments(ticketId, user, token);
}

private User getCurrentUserOrNull() {
Expand Down
44 changes: 22 additions & 22 deletions src/main/java/org/example/alfs/services/TicketCommentService.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ public TicketCommentService(TicketRepository ticketRepository,
}

@Transactional
public CommentViewDTO addComment(Long ticketId, CommentCreateDTO dto, User author) {
public CommentViewDTO addComment(Long ticketId, CommentCreateDTO dto, User author, String token) {
Ticket ticket = ticketRepository.findById(ticketId)
.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Ticket not found"));

boolean internalNote = dto.isInternalNote();

checkAccess(ticket, author);
checkAccess(ticket, author, token);
checkInternalNotePermission(internalNote, author);

TicketComment comment = new TicketComment();
Expand All @@ -52,16 +52,17 @@ public CommentViewDTO addComment(Long ticketId, CommentCreateDTO dto, User autho
}

@Transactional(readOnly = true)
public List<CommentViewDTO> getComments(Long ticketId, User actor) {
public List<CommentViewDTO> getComments(Long ticketId, User user, String token) {

Ticket ticket = ticketRepository.findById(ticketId)
.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Ticket not found"));

checkAccess(ticket, actor);
checkAccess(ticket, user, token);

boolean isReporter = actor.getRole() == Role.REPORTER;
boolean isReporter = user != null && user.getRole() == Role.REPORTER;
boolean isAnonymous = user == null;

List<TicketComment> all = isReporter
List<TicketComment> all = (isReporter || isAnonymous)
? ticketCommentRepository.findByTicketIdAndInternalNoteFalseOrderByCreatedAtAsc(ticketId)
: ticketCommentRepository.findByTicketIdOrderByCreatedAtAsc(ticketId);

Expand All @@ -71,32 +72,31 @@ public List<CommentViewDTO> getComments(Long ticketId, User actor) {
}

// helpers
private void checkAccess(Ticket ticket, User user) {
private void checkAccess(Ticket ticket, User user, String token) {
// Authenticated
if (user != null) {
if (user.getRole() == Role.ADMIN) return;

// If no user (anonymous) → deny access for now. Will be fixed later.
if (user == null) {
throw new ResponseStatusException(
HttpStatus.UNAUTHORIZED, "Authentication required");
}

if (user.getRole() == Role.ADMIN) return;

if (user.getRole() == Role.INVESTIGATOR) {
if (ticket.getInvestigator() != null &&
if (user.getRole() == Role.INVESTIGATOR &&
ticket.getInvestigator() != null &&
ticket.getInvestigator().getId().equals(user.getId())) {
return;
}
}

if (user.getRole() == Role.REPORTER) {
if (ticket.getReporter() != null &&
if (user.getRole() == Role.REPORTER &&
ticket.getReporter() != null &&
ticket.getReporter().getId().equals(user.getId())) {
return;
}
}

throw new ResponseStatusException(
HttpStatus.FORBIDDEN, "Access denied");
// Anonymous
if (token != null && !token.isBlank()
&& token.equals(ticket.getReporterToken())) {
return;
}

throw new ResponseStatusException(HttpStatus.FORBIDDEN, "Access denied");
}
Comment on lines +75 to 100

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Token-based access should be evaluated before user role, or treated equivalently to reporter.

The checkAccess logic authorizes authenticated users first and falls through to the token check. Combined with getComments (Lines 62–67), this creates a subtle information-disclosure gap:

An authenticated INVESTIGATOR (or any non-admin authenticated user) who is not assigned to this ticket but possesses the reporter token will be authorized via the token branch. In getComments, isReporter is false and isAnonymous is false, so they receive all comments including internalNote=true entries — which a bona-fide reporter using the same token would be filtered away from.

Consider treating access via token as equivalent to reporter access for the purpose of internal-note filtering:

🛡️ Suggested adjustment
-        boolean isReporter = user != null && user.getRole() == Role.REPORTER;
-        boolean isAnonymous = user == null;
+        boolean hasValidToken = token != null && token.equals(ticket.getReporterToken());
+        boolean isReporter = user != null && user.getRole() == Role.REPORTER;
+        boolean isAnonymous = user == null;
+        boolean isPrivilegedViewer = user != null
+                && (user.getRole() == Role.ADMIN
+                    || (user.getRole() == Role.INVESTIGATOR
+                        && ticket.getInvestigator() != null
+                        && ticket.getInvestigator().getId().equals(user.getId())));
 
-        List<TicketComment> all = (isReporter || isAnonymous)
+        List<TicketComment> all = (isReporter || isAnonymous || (hasValidToken && !isPrivilegedViewer))
                 ? ticketCommentRepository.findByTicketIdAndInternalNoteFalseOrderByCreatedAtAsc(ticketId)
                 : ticketCommentRepository.findByTicketIdOrderByCreatedAtAsc(ticketId);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/example/alfs/services/TicketCommentService.java` around
lines 75 - 99, The checkAccess logic currently authorizes authenticated users
before the reporter-token branch, which allows an authenticated non-reporter
(e.g., INVESTIGATOR) who also has the reporter token to be granted access but
not flagged as a reporter in getComments (so internalNote filtering is skipped).
Fix by treating token-based access as equivalent to reporter access: either move
the token check in checkAccess to run before role checks or add explicit logic
in checkAccess that, when token.equals(ticket.getReporterToken()), returns
access and ensures callers (getComments) can detect reporter access (e.g., set
isReporter true when token matched); update getComments to compute isReporter
using token match (ticket.getReporterToken()) in addition to user role checks.


private void checkInternalNotePermission(boolean internalNote, User author) {
Expand Down
Loading