feat: role-based ticket management, attachments, and improved UI/UX - #29
Conversation
# Conflicts: # src/main/java/org/example/alfs/controllers/TicketCommentController.java
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 35 minutes and 38 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces admin functionality, role-based authorization improvements, and enhanced ticket viewing with support for anonymous access via tokens. Key changes include a new admin controller for ticket management, user tracking on attachments, role-gated navigation, and comprehensive ticket view pages displaying comments, attachments, audit logs, and investigator information. Changes
Sequence DiagramssequenceDiagram
participant User
participant Controller as AttachmentController
participant Service as AttachmentService
participant Repo as TicketRepository
participant Entity as Attachment<br/>Entity
User->>Controller: POST /upload (ticketId, file, token)
activate Controller
Controller->>Service: uploadToTicket(ticketId, file, user, token)
deactivate Controller
activate Service
alt user != null
Service->>Repo: findById(ticketId)
else user == null (anonymous)
Service->>Repo: findByReporterToken(token)
end
activate Repo
Repo-->>Service: ticket
deactivate Repo
Service->>Service: checkAccess(ticket, user, token)
alt Access Denied
Service-->>Controller: throw ResponseStatusException
end
Service->>Entity: new Attachment(uploadedBy: user)
Service->>Repo: save(attachment)
Service-->>Controller: attachment saved
deactivate Service
alt token present
Controller-->>User: redirect:/tickets/token/{token}
else
Controller-->>User: redirect:/tickets/{ticketId}
end
sequenceDiagram
participant Admin
participant Controller as AdminController
participant Service as TicketService
participant Repo as TicketRepository
participant Mapper as TicketMapper
participant View as admin-tickets.jte
Admin->>Controller: GET /admin/tickets
activate Controller
Note over Controller: `@PreAuthorize`("hasRole('ADMIN')")
Controller->>Service: getAllTickets()
deactivate Controller
activate Service
Service->>Repo: findAll()
activate Repo
Repo-->>Service: List<Ticket>
deactivate Repo
Service->>Mapper: entityToViewDTO(ticket)
Mapper-->>Service: TicketViewDTO
Service-->>Controller: List<TicketViewDTO>
deactivate Service
Controller->>View: model.addAttribute("tickets", list)
View-->>Admin: Render table with tickets
sequenceDiagram
participant User
participant Controller as TicketController
participant CommentService as TicketCommentService
participant AttachRepo as AttachmentRepository
participant AuditRepo as AuditLogRepository
participant UserService as UserService
participant View as view.jte
User->>Controller: GET /tickets/{id}
activate Controller
Controller->>Controller: getSecurityUtils().getCurrentUserOrNull()
Controller->>CommentService: getComments(ticketId, user, null)
activate CommentService
CommentService-->>Controller: List<CommentViewDTO>
deactivate CommentService
Controller->>AttachRepo: findByTicketId(ticketId)
activate AttachRepo
AttachRepo-->>Controller: List<Attachment>
deactivate AttachRepo
Controller->>AuditRepo: findByTicketIdOrderByCreatedAtDesc(ticketId)
activate AuditRepo
AuditRepo-->>Controller: List<AuditLog>
deactivate AuditRepo
Controller->>UserService: getAllInvestigators()
activate UserService
UserService-->>Controller: List<User>
deactivate UserService
Controller->>View: model.addAttributes(comments, attachments, auditLogs, investigators)
View-->>User: Render ticket detail with panels
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/example/alfs/controllers/AttachmentController.java (1)
56-75:⚠️ Potential issue | 🟠 MajorEnforce ticket-scoped access before listing attachments.
This endpoint returns attachment metadata, including uploader names, for any
ticketIdit receives without verifying user authorization. The security config explicitly permits all access to/api/files/**(line 50), and thelistByTicket()method lacks@PreAuthorizeor authorization checks. Add user/token validation against the ticket before returning the list, or gate this through a service method that performs the access check.🤖 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/AttachmentController.java` around lines 56 - 75, The listByTicket method currently returns attachments for any ticketId without authorization; before calling attachmentRepository.findByTicketId and mapping to AttachmentViewDTO, validate that the authenticated principal is allowed to access the ticket (e.g., call a TicketService/AuthorizationService like ticketService.checkAccess(ticketId, principal) or annotate the method with `@PreAuthorize` and delegate to a service that enforces ticket-scoped rules). If the check fails, return ResponseEntity.status(403) with an appropriate error; only then fetch attachmentRepository.findByTicketId(ticketId) and map to AttachmentViewDTO for authorized users.
🧹 Nitpick comments (5)
src/main/java/org/example/alfs/entities/Attachment.java (1)
32-35: LGTM!Lazy
@ManyToOnewith nullableuploaded_bycorrectly supports anonymous uploads (whereuploadedByremains null).Minor nit: the new field is placed between
uploadedAtand the@PrePersistmethod, which splits fields across theprePersist()method. Consider grouping all fields together (e.g., next toticketon line 41) for readability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/alfs/entities/Attachment.java` around lines 32 - 35, The uploadedBy field is inserted between uploadedAt and prePersist(), splitting fields and reducing readability; move the private User uploadedBy field (the `@ManyToOne/`@JoinColumn block) so all entity fields are grouped together (for example, place it adjacent to the ticket field) and keep the `@PrePersist` method contiguous with other lifecycle methods to restore logical grouping and improve readability (update placement of uploadedBy, leaving annotations and types unchanged).src/main/jte/assigned-tickets.jte (1)
1-80: Near-duplicate ofadmin-tickets.jte— consider extracting a shared template.This template is structurally identical to
src/main/jte/admin-tickets.jteexcept for the page title, heading, and the empty-state message. Consider extracting a shared@template.ticket-table(...)partial that takestitle,emptyMessage,tickets, andsuccessas parameters, to avoid duplicated markup drifting over time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/jte/assigned-tickets.jte` around lines 1 - 80, Extract the duplicated table markup into a reusable partial (e.g. create an `@template.ticketTable`(tickets, success, title, emptyMessage)) and replace the body of both assigned-tickets.jte and admin-tickets.jte to call that partial with their specific title and emptyMessage; ensure the partial preserves the same bindings used here (references to tickets, t.getId(), t.getTitle(), t.getStatus(), t.getAssignedInvestigatorName(), and the success parameter) and accepts title and emptyMessage strings so assigned-tickets.jte supplies "My Assigned Tickets" and its custom empty message while admin-tickets.jte supplies its own values.src/main/jte/layout.jte (1)
39-68: Simplify role checks by consuming the newrolemodel attribute.
GlobalModelAttributesalready exposes arolemodel attribute (nullfor anonymous, otherwise the role name). The repeated inlineSecurityContextHolder.getContext().getAuthentication().getAuthorities().stream().anyMatch(...)calls could be replaced with a@param String roleand simple string comparisons (e.g.,role == null || "REPORTER".equals(role)for the Report link, and"REPORTER"/"INVESTIGATOR"/"ADMIN"branches below). This keeps view logic out ofSecurityContextHolderand matches howview.jtealready consumesrole.♻️ Sketch
+@param String role ... -@if( - SecurityContextHolder.getContext().getAuthentication() == null - || !SecurityContextHolder.getContext().getAuthentication().isAuthenticated() - || "anonymousUser".equals(SecurityContextHolder.getContext().getAuthentication().getPrincipal()) - || SecurityContextHolder.getContext().getAuthentication().getAuthorities().stream() - .anyMatch(a -> a.getAuthority().equals("ROLE_REPORTER")) -) +@if(role == null || "REPORTER".equals(role)) <a href="/tickets/create" class="hover:text-[`#b39359`]">Report</a> `@endif` ... -@if(SecurityContextHolder.getContext().getAuthentication().getAuthorities().stream() - .anyMatch(a -> a.getAuthority().equals("ROLE_ADMIN"))) +@if("ADMIN".equals(role)) <a href="/admin/tickets" class="hover:text-[`#b39359`]">All tickets</a> `@endif`Note: this would also require all controllers rendering
layoutdirectly to passrole, or relying on@ControllerAdviceinjection into JTE params (verify JTE picks up controller-advice attributes for@param).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/jte/layout.jte` around lines 39 - 68, Replace the repeated SecurityContextHolder authority checks in layout.jte by consuming the controller/advice-provided role model attribute: add a top-level param declaration (e.g., `@param` String role) and then change the Report link condition to use role == null || "REPORTER".equals(role), and the authenticated block to check role != null with downstream comparisons "REPORTER", "INVESTIGATOR", and "ADMIN" for the respective links; verify GlobalModelAttributes supplies role (and that controllers or `@ControllerAdvice` still populate it as in view.jte) and remove the SecurityContextHolder.getContext()... authority-stream anyMatch usage.src/main/java/org/example/alfs/controllers/TicketController.java (1)
86-95: Map attachments to DTOs to avoid lazy-loading issues and N+1 queries.Both
viewTicketByToken(line 87) andviewTicketById(line 124) pass rawAttachmententities to the view, whereatt.getUploadedBy().getUsername()is dereferenced. SinceAttachment.uploadedByis@ManyToOne(fetch = FetchType.LAZY), this causes N+1 queries and risksLazyInitializationExceptionif session management changes. Create anAttachmentViewDTO(similar to the pattern already inAttachmentController) and map attachments in a service layer once for both code paths.Code locations (lines 86–95 and 123–136)
var attachments = attachmentRepository.findByTicketId(ticket.getId()); ... model.addAttribute("attachments", attachments);🤖 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/TicketController.java` around lines 86 - 95, The attachments are passed as entities causing lazy-load/N+1 problems; create an AttachmentViewDTO (following AttachmentController's pattern) and add a mapping method in the attachment service to convert results from attachmentRepository.findByTicketId(...) into List<AttachmentViewDTO> (including uploadedBy.username and any other needed fields) then replace usages in viewTicketByToken and viewTicketById to call that service mapper and add the DTO list to the model instead of raw Attachment entities.src/main/jte/view.jte (1)
147-150: Consider adding token parameter support to the attachment download endpoint.The download link at lines 147-150 does not pass the
accessTokenparameter, while upload and comment forms preserve it. The download endpoint (AttachmentDownloadController.download()) does not currently accept a token parameter and allows unauthenticated access via/api/files/**(SecurityConfig line 50). When ownership/ticket-scoped access validation is implemented in future work, ensure the download endpoint either accepts and validates a token parameter or performs server-side ownership checks tied to the ticket context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/jte/view.jte` around lines 147 - 150, Update the attachment download workflow to support token-based access: add the accessToken query parameter to the download link in view.jte (where the anchor points to /api/files/${att.getId()}/download) and update AttachmentDownloadController.download() to accept an optional accessToken parameter and validate it (or perform server-side ownership/ticket-scoped checks) before streaming the file; ensure the controller uses the same token validation logic as upload/comment handlers so future ownership enforcement will work without exposing /api/files/** publicly.
🤖 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/config/SecurityConfig.java`:
- Line 50: The current security rule
requestMatchers("/api/files/**").permitAll() makes listByTicket() and download()
public; change it so only the upload POST remains open and all other file
endpoints require auth. Replace the broad matcher with an explicit POST matcher
(e.g. requestMatchers(HttpMethod.POST, "/api/files").permitAll()) and ensure
other "/api/files/**" routes are not permitAll (so they fall through to
authenticated()/hasAuthority checks). Reference SecurityConfig, the listByTicket
and download endpoints, and AttachmentService.checkAccess() when making this
change.
In `@src/main/java/org/example/alfs/controllers/AttachmentController.java`:
- Around line 78-84: The helper getCurrentUserOrNull currently swallows all
Exceptions from securityUtils.getCurrentUser(), which masks real auth errors;
modify getCurrentUserOrNull to only catch the specific "no authenticated user"
exceptions (e.g., org.springframework.security.core.AuthenticationException or
more narrowly AuthenticationCredentialsNotFoundException /
AnonymousAuthenticationToken absence as used in your codebase) and return null
in those cases, but rethrow any other unexpected exceptions; keep references to
the method getCurrentUserOrNull and the call securityUtils.getCurrentUser() so
the change is limited to narrowing the caught exception types and rethrowing
others.
In `@src/main/java/org/example/alfs/services/AttachmentService.java`:
- Around line 44-54: The code queries
ticketRepository.findByReporterToken(token) for anonymous access before
validating the token; change AttachmentService to first check that token is
non-null/non-blank and, if missing, throw new
ResponseStatusException(HttpStatus.UNAUTHORIZED, "Missing reporter token")
instead of querying, then only call ticketRepository.findByReporterToken(token)
and continue to checkAccess(ticket, user, token); apply the same pre-check for
the other anonymous branch(s) in this class where findByReporterToken(token) is
used.
In `@src/main/java/org/example/alfs/services/TicketService.java`:
- Around line 113-119: getAllTickets() returns all tickets without performing
the service-layer admin check; update it to perform the same admin authorization
used by getTicketsByStatus() before calling ticketRepository.findAll() — i.e.,
invoke the existing admin guard/validation routine that getTicketsByStatus()
uses (reuse the same method or logic) and only proceed to map with
ticketMapper::entityToViewDTO after the admin check passes.
In `@src/main/jte/view.jte`:
- Around line 220-230: The Audit Logs panel (the div rendering auditLogs in
view.jte) is currently visible to anonymous/reporters; wrap that entire section
in a role-based visibility check so only staff/admin/investigator users can see
it (e.g., check currentUser.isStaff() || currentUser.hasRole("ADMIN") ||
currentUser.hasRole("INVESTIGATOR") before rendering the panel), keep the
existing `@if`(auditLogs != null) inner loop intact, and ensure
reporters/anonymous users do not hit auditLogs rendering or see its contents.
---
Outside diff comments:
In `@src/main/java/org/example/alfs/controllers/AttachmentController.java`:
- Around line 56-75: The listByTicket method currently returns attachments for
any ticketId without authorization; before calling
attachmentRepository.findByTicketId and mapping to AttachmentViewDTO, validate
that the authenticated principal is allowed to access the ticket (e.g., call a
TicketService/AuthorizationService like ticketService.checkAccess(ticketId,
principal) or annotate the method with `@PreAuthorize` and delegate to a service
that enforces ticket-scoped rules). If the check fails, return
ResponseEntity.status(403) with an appropriate error; only then fetch
attachmentRepository.findByTicketId(ticketId) and map to AttachmentViewDTO for
authorized users.
---
Nitpick comments:
In `@src/main/java/org/example/alfs/controllers/TicketController.java`:
- Around line 86-95: The attachments are passed as entities causing
lazy-load/N+1 problems; create an AttachmentViewDTO (following
AttachmentController's pattern) and add a mapping method in the attachment
service to convert results from attachmentRepository.findByTicketId(...) into
List<AttachmentViewDTO> (including uploadedBy.username and any other needed
fields) then replace usages in viewTicketByToken and viewTicketById to call that
service mapper and add the DTO list to the model instead of raw Attachment
entities.
In `@src/main/java/org/example/alfs/entities/Attachment.java`:
- Around line 32-35: The uploadedBy field is inserted between uploadedAt and
prePersist(), splitting fields and reducing readability; move the private User
uploadedBy field (the `@ManyToOne/`@JoinColumn block) so all entity fields are
grouped together (for example, place it adjacent to the ticket field) and keep
the `@PrePersist` method contiguous with other lifecycle methods to restore
logical grouping and improve readability (update placement of uploadedBy,
leaving annotations and types unchanged).
In `@src/main/jte/assigned-tickets.jte`:
- Around line 1-80: Extract the duplicated table markup into a reusable partial
(e.g. create an `@template.ticketTable`(tickets, success, title, emptyMessage))
and replace the body of both assigned-tickets.jte and admin-tickets.jte to call
that partial with their specific title and emptyMessage; ensure the partial
preserves the same bindings used here (references to tickets, t.getId(),
t.getTitle(), t.getStatus(), t.getAssignedInvestigatorName(), and the success
parameter) and accepts title and emptyMessage strings so assigned-tickets.jte
supplies "My Assigned Tickets" and its custom empty message while
admin-tickets.jte supplies its own values.
In `@src/main/jte/layout.jte`:
- Around line 39-68: Replace the repeated SecurityContextHolder authority checks
in layout.jte by consuming the controller/advice-provided role model attribute:
add a top-level param declaration (e.g., `@param` String role) and then change the
Report link condition to use role == null || "REPORTER".equals(role), and the
authenticated block to check role != null with downstream comparisons
"REPORTER", "INVESTIGATOR", and "ADMIN" for the respective links; verify
GlobalModelAttributes supplies role (and that controllers or `@ControllerAdvice`
still populate it as in view.jte) and remove the
SecurityContextHolder.getContext()... authority-stream anyMatch usage.
In `@src/main/jte/view.jte`:
- Around line 147-150: Update the attachment download workflow to support
token-based access: add the accessToken query parameter to the download link in
view.jte (where the anchor points to /api/files/${att.getId()}/download) and
update AttachmentDownloadController.download() to accept an optional accessToken
parameter and validate it (or perform server-side ownership/ticket-scoped
checks) before streaming the file; ensure the controller uses the same token
validation logic as upload/comment handlers so future ownership enforcement will
work without exposing /api/files/** publicly.
🪄 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: b640a70e-6cca-464f-9e42-8e8551059fdf
📒 Files selected for processing (21)
src/main/java/org/example/alfs/config/GlobalModelAttributes.javasrc/main/java/org/example/alfs/config/SecurityConfig.javasrc/main/java/org/example/alfs/controllers/AdminController.javasrc/main/java/org/example/alfs/controllers/AttachmentController.javasrc/main/java/org/example/alfs/controllers/TicketCommentController.javasrc/main/java/org/example/alfs/controllers/TicketController.javasrc/main/java/org/example/alfs/dto/attachment/AttachmentViewDTO.javasrc/main/java/org/example/alfs/dto/ticket/TicketViewDTO.javasrc/main/java/org/example/alfs/entities/Attachment.javasrc/main/java/org/example/alfs/mapper/TicketMapper.javasrc/main/java/org/example/alfs/repositories/TicketRepository.javasrc/main/java/org/example/alfs/repositories/UserRepository.javasrc/main/java/org/example/alfs/services/AttachmentService.javasrc/main/java/org/example/alfs/services/TicketService.javasrc/main/java/org/example/alfs/services/UserService.javasrc/main/jte/admin-tickets.jtesrc/main/jte/assigned-tickets.jtesrc/main/jte/layout.jtesrc/main/jte/login.jtesrc/main/jte/view.jtesrc/main/resources/application.properties
| .requestMatchers("/tickets/create").permitAll() | ||
| .requestMatchers("/tickets/previewTicket").permitAll() | ||
|
|
||
| .requestMatchers("/api/files/**").permitAll() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Description:
# Inspect the file API security surface and token/auth checks without modifying the repo.
# Expected:
# - No broad `.requestMatchers("/api/files/**").permitAll()`.
# - GET list/download endpoints either require authentication/authorization or validate a token.
# - POST upload requires either authenticated user context or a valid token in service/controller logic.
echo "## SecurityConfig file API matchers"
fd -i '^SecurityConfig\.java$' . -x sh -c 'echo "--- $1"; rg -n -C3 "api/files|requestMatchers|anyRequest" "$1"' sh {}
echo
echo "## Attachment controllers: mappings and method security"
fd -i '^(AttachmentController|AttachmentDownloadController)\.java$' . -x sh -c 'echo "--- $1"; rg -n -C4 "@(RequestMapping|GetMapping|PostMapping|PreAuthorize|Secured|RolesAllowed)|/api/files|ticketId|download|upload" "$1"' sh {}
echo
echo "## Attachment service token/user validation"
fd -i '^AttachmentService\.java$' . -x sh -c 'echo "--- $1"; rg -n -C5 "uploadToTicket|token|getCurrentUser|uploadedBy|findByTicketId|download|s3Key" "$1"' sh {}Repository: ithsjava25/project-backend-alfs
Length of output: 7373
Scope public file access to uploads only; list and download endpoints need authorization.
Line 50 exposes GET /api/files?ticketId=... and GET /api/files/{id}/download anonymously without any authorization checks. The listByTicket() method returns attachment metadata for any ticket ID, and the download() method streams file content for any attachment ID. Only the POST upload endpoint should remain public (it is already token-protected by AttachmentService.checkAccess()).
Restrict the matcher to POST upload only:
Proposed change
- .requestMatchers("/api/files/**").permitAll()
+ .requestMatchers(HttpMethod.POST, "/api/files/upload").permitAll()GET endpoints must enforce authorization to prevent unauthorized attachment enumeration and download.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .requestMatchers("/api/files/**").permitAll() | |
| .requestMatchers(HttpMethod.POST, "/api/files/upload").permitAll() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/example/alfs/config/SecurityConfig.java` at line 50, The
current security rule requestMatchers("/api/files/**").permitAll() makes
listByTicket() and download() public; change it so only the upload POST remains
open and all other file endpoints require auth. Replace the broad matcher with
an explicit POST matcher (e.g. requestMatchers(HttpMethod.POST,
"/api/files").permitAll()) and ensure other "/api/files/**" routes are not
permitAll (so they fall through to authenticated()/hasAuthority checks).
Reference SecurityConfig, the listByTicket and download endpoints, and
AttachmentService.checkAccess() when making this change.
| <!-- AUDIT LOGS --> | ||
| <div class="col-span-1 bg-white p-4 rounded shadow h-[600px] overflow-y-auto"> | ||
| <h2 class="font-bold mb-3">Audit Logs</h2> | ||
|
|
||
| @if(auditLogs != null) | ||
| @for(var log : auditLogs) | ||
| <div class="text-sm mb-2"> | ||
| ${log.getAction()} | ||
| </div> | ||
| @endfor | ||
| @endif |
There was a problem hiding this comment.
Gate audit logs to staff roles.
This panel is rendered for anonymous token users and reporters too. Audit logs are internal activity data; wrap this section in an admin/investigator role check unless reporters are explicitly meant to see it.
🔒 Proposed visibility gate
- <!-- AUDIT LOGS -->
- <div class="col-span-1 bg-white p-4 rounded shadow h-[600px] overflow-y-auto">
- <h2 class="font-bold mb-3">Audit Logs</h2>
-
- `@if`(auditLogs != null)
- `@for`(var log : auditLogs)
- <div class="text-sm mb-2">
- ${log.getAction()}
- </div>
- `@endfor`
- `@endif`
- </div>
+ `@if`("ADMIN".equals(role) || "INVESTIGATOR".equals(role))
+ <!-- AUDIT LOGS -->
+ <div class="col-span-1 bg-white p-4 rounded shadow h-[600px] overflow-y-auto">
+ <h2 class="font-bold mb-3">Audit Logs</h2>
+
+ `@if`(auditLogs != null)
+ `@for`(var log : auditLogs)
+ <div class="text-sm mb-2">
+ ${log.getAction()}
+ </div>
+ `@endfor`
+ `@endif`
+ </div>
+ `@endif`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <!-- AUDIT LOGS --> | |
| <div class="col-span-1 bg-white p-4 rounded shadow h-[600px] overflow-y-auto"> | |
| <h2 class="font-bold mb-3">Audit Logs</h2> | |
| @if(auditLogs != null) | |
| @for(var log : auditLogs) | |
| <div class="text-sm mb-2"> | |
| ${log.getAction()} | |
| </div> | |
| @endfor | |
| @endif | |
| `@if`("ADMIN".equals(role) || "INVESTIGATOR".equals(role)) | |
| <!-- AUDIT LOGS --> | |
| <div class="col-span-1 bg-white p-4 rounded shadow h-[600px] overflow-y-auto"> | |
| <h2 class="font-bold mb-3">Audit Logs</h2> | |
| `@if`(auditLogs != null) | |
| `@for`(var log : auditLogs) | |
| <div class="text-sm mb-2"> | |
| ${log.getAction()} | |
| </div> | |
| `@endfor` | |
| `@endif` | |
| </div> | |
| `@endif` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/jte/view.jte` around lines 220 - 230, The Audit Logs panel (the div
rendering auditLogs in view.jte) is currently visible to anonymous/reporters;
wrap that entire section in a role-based visibility check so only
staff/admin/investigator users can see it (e.g., check currentUser.isStaff() ||
currentUser.hasRole("ADMIN") || currentUser.hasRole("INVESTIGATOR") before
rendering the panel), keep the existing `@if`(auditLogs != null) inner loop
intact, and ensure reporters/anonymous users do not hit auditLogs rendering or
see its contents.
There was a problem hiding this comment.
This is what we want! Everything should be transparent for a reporter.
And the audit logs is not even done yet! Will be fixed in another PR!
There was a problem hiding this comment.
@addee1, understood! Transparency for reporters is a deliberate design choice, and the audit log feature being a work-in-progress that will be completed in a future PR is noted. I'll keep this in mind for future reviews.
✏️ Learnings added
Learnt from: addee1
Repo: ithsjava25/project-backend-alfs PR: 29
File: src/main/jte/view.jte:220-230
Timestamp: 2026-04-20T09:45:48.266Z
Learning: In `ithsjava25/project-backend-alfs`, the audit log panel in `src/main/jte/view.jte` is intentionally visible to all roles including reporters and anonymous token users. Full transparency for reporters is a deliberate product design decision. Do not flag audit log visibility as a security/access-control issue in this repo.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: FionaSprinkles
Repo: ithsjava25/project-backend-alfs PR: 11
File: src/main/java/org/example/alfs/services/TicketService.java:50-54
Timestamp: 2026-04-09T13:26:48.849Z
Learning: In `ithsjava25/project-backend-alfs`, the IDOR vulnerability in `TicketService.getTicketById()` (and the `GET /view/id/{id}` endpoint in `TicketController`) is a known, intentionally deferred issue. Authorization/ownership checks will be added in a future PR when authentication and authorization are implemented. Do not re-flag this as a new issue.
1 participant
This PR introduces major improvements to the ticket management flow, UI, and role-based behavior across the application.
🚀 Features
Ticket management
Attachments
Comments
UI/UX improvements
Security & access
🛠️ Technical changes
Overall, this PR significantly improves the usability and structure of the ticket system while preparing the codebase for upcoming features.
Summary by CodeRabbit
Release Notes
New Features
Chores