Skip to content
This repository was archived by the owner on Nov 23, 2025. It is now read-only.

Dev #10

Merged
RandithaK merged 3 commits into from
Nov 11, 2025
Merged

Dev #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9641030
Update time log controller
mehara-rothila Nov 6, 2025
d7a18c1
fix: Restrict pull request triggers to specific branches in build wor…
RandithaK Nov 11, 2025
8f9cbfc
Merge pull request #9 from TechTorque-2025/randitha-superbranch
RandithaK Nov 11, 2025
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
7 changes: 3 additions & 4 deletions .github/workflows/buildtest.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
name: Build and Test Time Logging Service

on:
push:
branches:
- '**'
pull_request:
branches:
- '**'
- main
- dev
- devOps

jobs:
build-test:
Expand Down
48 changes: 28 additions & 20 deletions ...rvice/src/main/java/com/techtorque/time_logging_service/controller/TimeLogController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.techtorque.time_logging_service.dto.request.TimeLogUpdateRequest;
import com.techtorque.time_logging_service.dto.response.TimeLogResponse;
import com.techtorque.time_logging_service.dto.response.TimeLogSummaryResponse;
import com.techtorque.time_logging_service.exception.UnauthorizedAccessException;
import com.techtorque.time_logging_service.service.TimeLogService;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.Parameter;
Expand Down Expand Up @@ -63,12 +64,12 @@ public TimeLogController(TimeLogService timeLogService) {
@ApiResponse(responseCode = "403", description = "Forbidden - insufficient permissions")
})
@PostMapping
@PreAuthorize("hasRole('EMPLOYEE')")
@PreAuthorize("hasAnyRole('EMPLOYEE', 'ADMIN', 'SUPER_ADMIN')")
Copy link

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify SUPER_ADMIN role inclusion across all endpoints.

SUPER_ADMIN has been added consistently to most endpoints, which aligns with granting elevated privileges. However, lines 231 and 281 (getTimeLogsForService and getTimeLogsForProject) still use @PreAuthorize("hasAnyRole('CUSTOMER', 'EMPLOYEE', 'ADMIN')") without SUPER_ADMIN. Confirm whether this exclusion is intentional or an oversight.

Run the following script to identify all @PreAuthorize annotations and verify consistency:

Also applies to: 92-92, 142-142, 171-171, 198-198, 257-257, 299-299

🏁 Script executed:

#!/bin/bash
# Description: Find all @PreAuthorize annotations to check for SUPER_ADMIN consistency

# Search for all @PreAuthorize annotations in the controller
rg -n '@PreAuthorize' time-logging-service/src/main/java/com/techtorque/time_logging_service/controller/TimeLogController.java -A1 -B2

Length of output: 1720

Add SUPER_ADMIN to service/project-level time log queries for consistency.

The SUPER_ADMIN role has been correctly added to employee-focused endpoints (createTimeLog, getMyTimeLogs, getTimeLogById, updateTimeLog, deleteTimeLog, getSummary, getEmployeeStats). However, lines 231 and 281 still lack SUPER_ADMIN:

  • Line 231: getTimeLogsForService - @PreAuthorize("hasAnyRole('CUSTOMER', 'EMPLOYEE', 'ADMIN')")
  • Line 281: getTimeLogsForProject - @PreAuthorize("hasAnyRole('CUSTOMER', 'EMPLOYEE', 'ADMIN')")

SUPER_ADMIN should have access to all time log data, including service and project-level queries. Add 'SUPER_ADMIN' to both annotations.

🤖 Prompt for AI Agents 
In
time-logging-service/src/main/java/com/techtorque/time_logging_service/controller/TimeLogController.java
around lines 231 and 281, the @PreAuthorize annotations for
getTimeLogsForService and getTimeLogsForProject currently allow CUSTOMER,
EMPLOYEE, and ADMIN but omit SUPER_ADMIN; update both annotations to include
'SUPER_ADMIN' so they read
hasAnyRole('CUSTOMER','EMPLOYEE','ADMIN','SUPER_ADMIN') to grant super admin
access to service- and project-level time log queries.

public ResponseEntity<TimeLogResponse> createTimeLog(
@Parameter(description = "Employee ID from authentication token", required = true)
@RequestHeader(value = "X-User-Subject") String employeeId,
@Valid @RequestBody TimeLogRequest request) {

TimeLogResponse response = timeLogService.createTimeLog(employeeId, request);
return ResponseEntity.status(HttpStatus.CREATED).body(response);
}
Expand All @@ -88,7 +89,7 @@ public ResponseEntity<TimeLogResponse> createTimeLog(
@ApiResponse(responseCode = "403", description = "Forbidden")
})
@GetMapping
@PreAuthorize("hasAnyRole('EMPLOYEE', 'ADMIN')")
@PreAuthorize("hasAnyRole('EMPLOYEE', 'ADMIN', 'SUPER_ADMIN')")
public ResponseEntity<List<TimeLogResponse>> getMyTimeLogs(
@Parameter(description = "Employee ID from authentication token", required = true)
@RequestHeader("X-User-Subject") String userId,
Expand All @@ -97,11 +98,11 @@ public ResponseEntity<List<TimeLogResponse>> getMyTimeLogs(
@RequestParam(required = false) @DateTimeFormat(iso = DateTimeFormat.ISO.DATE) LocalDate from,
@Parameter(description = "End date for filtering (YYYY-MM-DD)")
@RequestParam(required = false) @DateTimeFormat(iso = DateTimeFormat.ISO.DATE) LocalDate to) {

List<TimeLogResponse> responses;
// Admin can see all time logs
if (roles.contains("ADMIN")) {

// Admin and Super Admin can see all time logs
if (roles.contains("ADMIN") || roles.contains("SUPER_ADMIN")) {
if (from != null && to != null) {
// For admin with date range, get all logs and filter (or create new method)
responses = timeLogService.getAllTimeLogs().stream()
Expand All @@ -118,7 +119,7 @@ public ResponseEntity<List<TimeLogResponse>> getMyTimeLogs(
responses = timeLogService.getAllTimeLogsByEmployee(userId);
}
}

return ResponseEntity.ok(responses);
}

Expand All @@ -138,15 +139,15 @@ public ResponseEntity<List<TimeLogResponse>> getMyTimeLogs(
@ApiResponse(responseCode = "404", description = "Time log not found")
})
@GetMapping("/{logId}")
@PreAuthorize("hasAnyRole('EMPLOYEE', 'ADMIN')")
@PreAuthorize("hasAnyRole('EMPLOYEE', 'ADMIN', 'SUPER_ADMIN')")
public ResponseEntity<TimeLogResponse> getTimeLogById(
@Parameter(description = "Time log ID", required = true)
@PathVariable String logId,
@Parameter(description = "User ID from authentication token")
@RequestHeader(value = "X-User-Subject", required = false) String userId,
@Parameter(description = "User role from authentication token")
@RequestHeader(value = "X-User-Role", required = false) String userRole) {

TimeLogResponse response = timeLogService.getTimeLogByIdWithAuthorization(logId, userId, userRole);
return ResponseEntity.ok(response);
}
Expand All @@ -167,14 +168,14 @@ public ResponseEntity<TimeLogResponse> getTimeLogById(
@ApiResponse(responseCode = "404", description = "Time log not found")
})
@PutMapping("/{logId}")
@PreAuthorize("hasRole('EMPLOYEE')")
@PreAuthorize("hasAnyRole('EMPLOYEE', 'ADMIN', 'SUPER_ADMIN')")
public ResponseEntity<TimeLogResponse> updateTimeLog(
@Parameter(description = "Time log ID", required = true)
@PathVariable String logId,
@Parameter(description = "Employee ID from authentication token", required = true)
@RequestHeader("X-User-Subject") String employeeId,
@Valid @RequestBody TimeLogUpdateRequest request) {

TimeLogResponse response = timeLogService.updateTimeLogWithAuthorization(logId, employeeId, request);
return ResponseEntity.ok(response);
}
Expand All @@ -194,13 +195,20 @@ public ResponseEntity<TimeLogResponse> updateTimeLog(
@ApiResponse(responseCode = "404", description = "Time log not found")
})
@DeleteMapping("/{logId}")
@PreAuthorize("hasRole('EMPLOYEE')")
@PreAuthorize("hasAnyRole('EMPLOYEE', 'ADMIN', 'SUPER_ADMIN')")
public ResponseEntity<Void> deleteTimeLog(
@Parameter(description = "Time log ID", required = true)
@PathVariable String logId,
@Parameter(description = "Employee ID from authentication token", required = true)
@RequestHeader("X-User-Subject") String employeeId) {

@Parameter(description = "Employee ID from authentication token", required = false)
@RequestHeader(value = "X-User-Subject", required = false) String employeeId,
@Parameter(description = "User role from authentication token", required = false)
@RequestHeader(value = "X-User-Roles", required = false) String userRoles) {

// If header is missing, check if user is admin (can delete any log)
if (employeeId == null || employeeId.isEmpty()) {
throw new UnauthorizedAccessException("User identification missing. Please ensure you are logged in.");
}
Comment on lines +202 to +210
Copy link

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Incomplete authorization logic and unused parameter.

Several issues with this implementation:

  1. Misleading comment: Line 207 says "check if user is admin" but no admin check is performed.
  2. Unused parameter: userRoles is accepted but never used, making the header acceptance pointless.
  3. Logic contradiction: If employeeId is always required (lines 208-210), why make the header optional?
  4. Missing admin privilege: If SUPER_ADMIN/ADMIN should be able to delete any log without ownership checks, this logic is missing.

Clarify the intended behavior. If admins should be able to delete any log:

     @Parameter(description = "Employee ID from authentication token", required = false)
     @RequestHeader(value = "X-User-Subject", required = false) String employeeId,
     @Parameter(description = "User role from authentication token", required = false)
     @RequestHeader(value = "X-User-Roles", required = false) String userRoles) {

-    // If header is missing, check if user is admin (can delete any log)
-    if (employeeId == null || employeeId.isEmpty()) {
-      throw new UnauthorizedAccessException("User identification missing. Please ensure you are logged in.");
-    }
+    // Parse roles for admin check
+    boolean isAdmin = userRoles != null && 
+        (Arrays.asList(userRoles.split(",")).contains("ADMIN") || 
+         Arrays.asList(userRoles.split(",")).contains("SUPER_ADMIN"));
+    
+    // If not admin, employeeId is required
+    if (!isAdmin && (employeeId == null || employeeId.isEmpty())) {
+      throw new UnauthorizedAccessException("User identification missing. Please ensure you are logged in.");
+    }
-    timeLogService.deleteTimeLogWithAuthorization(logId, employeeId);
+    // Pass role info to service layer for proper authorization
+    timeLogService.deleteTimeLogWithAuthorization(logId, employeeId, isAdmin);

Note: You'll need to update the service method signature accordingly.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In
time-logging-service/src/main/java/com/techtorque/time_logging_service/controller/TimeLogController.java
around lines 202 to 210, the authorization logic is incomplete: the comment says
to check admin but no check is implemented, userRoles parameter is unused,
employeeId is treated as required despite the header being marked optional, and
admins are not allowed to bypass ownership checks. Fix by treating userRoles
header as the source of roles (parse comma-separated roles, trim and compare
case-insensitively to "ADMIN" or "SUPER_ADMIN"); change the null/empty
employeeId check to only throw UnauthorizedAccessException when employeeId is
missing AND the parsed roles do NOT include ADMIN or SUPER_ADMIN; if the caller
is an admin allow the operation without an employeeId; and update any downstream
service method signature or call to accept the resolved actingUserId/role
context (or perform the admin-vs-owner decision in the controller before calling
the service). Ensure header annotations reflect whether employeeId can be
optional and document that admin role allows global deletes.


timeLogService.deleteTimeLogWithAuthorization(logId, employeeId);
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

The userRoles parameter is added but never used in the method. The comment on line 207 suggests checking admin role, but the service method deleteTimeLogWithAuthorization only checks employee ownership and doesn't support admin bypass logic like getTimeLogByIdWithAuthorization does. Either remove the unused parameter or implement role-based authorization to allow admins/super admins to delete any log without ownership checks.

Suggested change
timeLogService.deleteTimeLogWithAuthorization(logId, employeeId);
boolean isAdmin = false;
if (userRoles != null && !userRoles.isEmpty()) {
String[] roles = userRoles.split(",");
for (String role : roles) {
String trimmedRole = role.trim().toUpperCase();
if (trimmedRole.equals("ADMIN") || trimmedRole.equals("SUPER_ADMIN")) {
isAdmin = true;
break;
}
}
}
if (isAdmin) {
// Admins can delete any log
timeLogService.deleteTimeLogAsAdmin(logId);
} else {
// Employees can only delete their own logs
timeLogService.deleteTimeLogWithAuthorization(logId, employeeId);
}

Copilot uses AI. Check for mistakes.
return ResponseEntity.noContent().build();
}
Expand Down Expand Up @@ -246,15 +254,15 @@ public ResponseEntity<List<TimeLogResponse>> getTimeLogsForService(
@ApiResponse(responseCode = "403", description = "Forbidden")
})
@GetMapping("/summary")
@PreAuthorize("hasRole('EMPLOYEE')")
@PreAuthorize("hasAnyRole('EMPLOYEE', 'ADMIN', 'SUPER_ADMIN')")
public ResponseEntity<TimeLogSummaryResponse> getSummary(
@Parameter(description = "Employee ID from authentication token", required = true)
@RequestHeader("X-User-Subject") String employeeId,
@Parameter(description = "Period type: 'daily' or 'weekly'", required = true)
@RequestParam String period,
@Parameter(description = "Reference date (YYYY-MM-DD)", required = true)
@RequestParam @DateTimeFormat(iso = DateTimeFormat.ISO.DATE) LocalDate date) {

TimeLogSummaryResponse summary = timeLogService.getEmployeeSummaryByPeriod(employeeId, period, date);
return ResponseEntity.ok(summary);
}
Expand Down Expand Up @@ -288,11 +296,11 @@ public ResponseEntity<List<TimeLogResponse>> getTimeLogsForProject(
description = "Get quick statistics including total hours logged, number of logs, and breakdown by service/project."
)
@GetMapping("/stats")
@PreAuthorize("hasRole('EMPLOYEE')")
@PreAuthorize("hasAnyRole('EMPLOYEE', 'ADMIN', 'SUPER_ADMIN')")
public ResponseEntity<Map<String, Object>> getEmployeeStats(
@Parameter(description = "Employee ID from authentication token", required = true)
@RequestHeader("X-User-Subject") String employeeId) {

Map<String, Object> stats = timeLogService.getEmployeeStatistics(employeeId);
return ResponseEntity.ok(stats);
}
Expand Down
Loading