Supporting anonymous access to ticket comments using reporter token - #26
Merged
simonforsberg merged 3 commits intoApr 17, 2026
Merged
Conversation
…ken and update access control logic
📝 WalkthroughWalkthroughTicket comment endpoints now accept an optional Changes
Sequence DiagramsequenceDiagram
participant Client as Client (Anon / Auth)
participant Controller as TicketCommentController
participant Service as TicketCommentService
participant DB as Database
Client->>Controller: GET/POST /{ticketId}/comments?token=...
Controller->>Service: getComments/addComment(ticketId, user, token)
Service->>DB: Fetch Ticket
DB-->>Service: Ticket (includes reporterToken)
alt token matches reporterToken
Service->>Service: checkAccess(ticket, null, token) → allow reporter-like access
else token missing or mismatch
alt user authenticated
Service->>Service: checkAccess(ticket, user, token) → role-based auth
else
Service->>Service: checkAccess(ticket, null, token) → deny or limited access
end
end
Service->>DB: Fetch Comments (filter internal notes if anonymous/reporter)
DB-->>Service: Comments
Service-->>Controller: CommentViewDTOs
Controller-->>Client: Response (POST redirect includes ?token=... if provided)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/org/example/alfs/services/TicketCommentService.java (1)
94-96: Defensively reject empty/blank tokens.
token != null && token.equals(ticket.getReporterToken())will correctly rejectnull/nullbecauseString.equals(null) == false, but consider rejecting blank tokens early to avoid accidental matches ifreporterTokenever becomes settable to an empty string (e.g., through a future migration or buggy code path).- if (token != null && token.equals(ticket.getReporterToken())) { + if (token != null && !token.isBlank() + && token.equals(ticket.getReporterToken())) { return; }🤖 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 94 - 96, Update the defensive check in TicketCommentService so blank tokens are rejected before comparing: ensure the condition verifies token is non-null and not blank/empty (e.g., trim() length > 0 or use a String utility isNotBlank) prior to calling token.equals(ticket.getReporterToken()); keep the early return behavior when the token matches ticket.getReporterToken() but only after the non-blank check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/org/example/alfs/controllers/TicketCommentController.java`:
- Around line 31-37: The redirect currently appends the reporter token into the
URL ("redirect:/view/id/" + ticketId + (token != null ? "?token=" + token : ""))
which leaks the bearer token and is not URL-encoded; instead, after calling
commentService.addComment(ticketId, dto, user, token) in TicketCommentController
(the method that calls getCurrentUserOrNull and commentService.addComment),
remove the token from the query string and set it as a short-lived, HttpOnly,
secure cookie scoped to the ticket path (e.g. /view/id/{ticketId}) or rotate the
token server-side and store the rotated value in a cookie; if you must keep a
URL redirect, URL-encode the token using UriComponentsBuilder or
URLEncoder.encode(token, StandardCharsets.UTF_8) before concatenation and prefer
rotating/invalidating the token immediately after use.
In `@src/main/java/org/example/alfs/services/TicketCommentService.java`:
- Around line 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.
---
Nitpick comments:
In `@src/main/java/org/example/alfs/services/TicketCommentService.java`:
- Around line 94-96: Update the defensive check in TicketCommentService so blank
tokens are rejected before comparing: ensure the condition verifies token is
non-null and not blank/empty (e.g., trim() length > 0 or use a String utility
isNotBlank) prior to calling token.equals(ticket.getReporterToken()); keep the
early return behavior when the token matches ticket.getReporterToken() but only
after the non-blank check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 35a04d58-674e-48e2-b065-3c427c4279db
📒 Files selected for processing (2)
src/main/java/org/example/alfs/controllers/TicketCommentController.javasrc/main/java/org/example/alfs/services/TicketCommentService.java
Comment on lines
+31
to
+37
| @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; | ||
| return "redirect:/view/id/" + ticketId + (token != null ? "?token=" + token : ""); |
There was a problem hiding this comment.
Reporter token leaked via URL; also not URL-encoded on redirect.
Two concerns with propagating the token through query strings:
- Leakage. Placing the reporter token in the URL query string exposes it in browser history, server access logs, proxy logs, and
Refererheaders sent to any third-party resources embedded on the downstream page. Since this token is a bearer credential granting access to the ticket and its comments, this materially weakens its security. Consider using a short-lived signed cookie scoped to the ticket path instead, or rotating the token after use. - Encoding.
"?token=" + tokenconcatenates user-supplied input into a redirectLocationheader without URL-encoding. In practice UUIDs are safe, but if the token format ever changes (or a malformed value is submitted), this could cause header/URL parsing issues. UseUriComponentsBuilderorURLEncoder.encode(token, StandardCharsets.UTF_8).
🔒 Minimal encoding fix (if the redirect target remains URL-based)
- return "redirect:/view/id/" + ticketId + (token != null ? "?token=" + token : "");
+ String base = "redirect:/view/id/" + ticketId;
+ return token != null
+ ? base + "?token=" + URLEncoder.encode(token, StandardCharsets.UTF_8)
+ : base;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/example/alfs/controllers/TicketCommentController.java`
around lines 31 - 37, The redirect currently appends the reporter token into the
URL ("redirect:/view/id/" + ticketId + (token != null ? "?token=" + token : ""))
which leaks the bearer token and is not URL-encoded; instead, after calling
commentService.addComment(ticketId, dto, user, token) in TicketCommentController
(the method that calls getCurrentUserOrNull and commentService.addComment),
remove the token from the query string and set it as a short-lived, HttpOnly,
secure cookie scoped to the ticket path (e.g. /view/id/{ticketId}) or rotate the
token server-side and store the rotated value in a cookie; if you must keep a
URL redirect, URL-encode the token using UriComponentsBuilder or
URLEncoder.encode(token, StandardCharsets.UTF_8) before concatenation and prefer
rotating/invalidating the token immediately after use.
Comment on lines
+75
to
99
| 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.equals(ticket.getReporterToken())) { | ||
| return; | ||
| } | ||
|
|
||
| throw new ResponseStatusException(HttpStatus.FORBIDDEN, "Access denied"); | ||
| } |
There was a problem hiding this comment.
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.
…within `TicketCommentService`
…n `TicketCommentController`.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/java/org/example/alfs/services/TicketCommentService.java (1)
62-67:⚠️ Potential issue | 🟠 MajorInformation-disclosure gap from previous review is still present.
An authenticated non-reporter (e.g., an
INVESTIGATORnot assigned to this ticket) who also passes a valid reportertokenis authorized via the token branch incheckAccess, but hereisReporterisfalseandisAnonymousisfalse, so they receive all comments includinginternalNote=trueentries. Token-based access should be treated equivalent to reporter access for the purpose of internal-note filtering (or the token branch should be evaluated ahead of authenticated-user branches).🤖 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 62 - 67, The code currently uses isReporter and isAnonymous to decide whether to hide internal notes, but it ignores cases where access was granted via a reporter token in checkAccess; update TicketCommentService so token-based reporter access is treated like a reporter: call or reuse the checkAccess outcome to set a boolean (e.g., hasReporterToken or tokenGrantedReporter) and change the fetch condition from (isReporter || isAnonymous) to (isReporter || isAnonymous || hasReporterToken) when choosing between ticketCommentRepository.findByTicketIdAndInternalNoteFalseOrderByCreatedAtAsc and findByTicketIdOrderByCreatedAtAsc; alternatively, evaluate the token branch before authenticated-user branches so the token path results in the same internal-note filtering as a reporter.
🧹 Nitpick comments (1)
src/main/java/org/example/alfs/services/TicketCommentService.java (1)
94-97: Use a constant-time comparison for the reporter token.
String.equalsshort-circuits on the first mismatching character and is susceptible to timing side-channels when comparing secrets. Reporter tokens act as bearer credentials here, so prefer a length-independent, constant-time check.🛡️ Proposed adjustment
- if (token != null && !token.isBlank() - && token.equals(ticket.getReporterToken())) { - return; - } + String expected = ticket.getReporterToken(); + if (token != null && !token.isBlank() && expected != null + && MessageDigest.isEqual( + token.getBytes(StandardCharsets.UTF_8), + expected.getBytes(StandardCharsets.UTF_8))) { + return; + }Add imports:
import java.nio.charset.StandardCharsets; import java.security.MessageDigest;🤖 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 94 - 97, Replace the short-circuiting String.equals check in TicketCommentService (the block that compares token to ticket.getReporterToken()) with a constant-time comparison: normalize both tokens to byte[] using StandardCharsets.UTF_8 (handle null/blank as before), compute fixed-length digests (e.g., MessageDigest.getInstance("SHA-256").digest(...)) for both values and use MessageDigest.isEqual(digestA, digestB) to decide equality; keep the existing null/blank guards and return behavior but swap the equals call for this constant-time digest & isEqual approach.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/main/java/org/example/alfs/services/TicketCommentService.java`:
- Around line 62-67: The code currently uses isReporter and isAnonymous to
decide whether to hide internal notes, but it ignores cases where access was
granted via a reporter token in checkAccess; update TicketCommentService so
token-based reporter access is treated like a reporter: call or reuse the
checkAccess outcome to set a boolean (e.g., hasReporterToken or
tokenGrantedReporter) and change the fetch condition from (isReporter ||
isAnonymous) to (isReporter || isAnonymous || hasReporterToken) when choosing
between
ticketCommentRepository.findByTicketIdAndInternalNoteFalseOrderByCreatedAtAsc
and findByTicketIdOrderByCreatedAtAsc; alternatively, evaluate the token branch
before authenticated-user branches so the token path results in the same
internal-note filtering as a reporter.
---
Nitpick comments:
In `@src/main/java/org/example/alfs/services/TicketCommentService.java`:
- Around line 94-97: Replace the short-circuiting String.equals check in
TicketCommentService (the block that compares token to
ticket.getReporterToken()) with a constant-time comparison: normalize both
tokens to byte[] using StandardCharsets.UTF_8 (handle null/blank as before),
compute fixed-length digests (e.g.,
MessageDigest.getInstance("SHA-256").digest(...)) for both values and use
MessageDigest.isEqual(digestA, digestB) to decide equality; keep the existing
null/blank guards and return behavior but swap the equals call for this
constant-time digest & isEqual approach.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc0005df-20f8-4112-bd7c-c83fce8f94d3
📒 Files selected for processing (2)
src/main/java/org/example/alfs/controllers/TicketCommentController.javasrc/main/java/org/example/alfs/services/TicketCommentService.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/example/alfs/controllers/TicketCommentController.java
This was referenced Apr 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supporting anonymous access to ticket comments using reporter token, and update access control logic.
Summary by CodeRabbit