Skip to content
Open
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 @@ -12,7 +12,9 @@ public CaseNoteDTO toDTO(CaseNoteEntity entity) {
return new CaseNoteDTO(
entity.getId(),
entity.getContent(),
entity.getAuthor(),
entity.getAuthorDisplayName(),
entity.getAuthorGithubUsername(),
entity.getAuthorRole(),
entity.getCreatedAt()
);
}
Expand All @@ -22,7 +24,9 @@ public CaseNoteEntity toEntity(CaseNoteDTO dto) {
CaseNoteEntity entity = new CaseNoteEntity();
entity.setId(dto.getId());
entity.setContent(dto.getContent());
entity.setAuthor(dto.getAuthor());
entity.setAuthorDisplayName(dto.getAuthorDisplayName());
entity.setAuthorGithubUsername(dto.getAuthorGithubUsername());
entity.setAuthorRole(dto.getAuthorRole());
entity.setCreatedAt(dto.getCreatedAt());
return entity;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,18 @@ public CaseService(CaseRepository caseRepository, CaseMapper caseMapper, Patient
}

@Transactional
public void addNote(UUID caseId, String content, String author) {
public void addNote(UUID caseId, String content, Actor actor) {
CaseEntity caseEntity = caseRepository.findById(caseId)
.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Case not found"));

CaseNoteEntity note = new CaseNoteEntity();
note.setCaseEntity(caseEntity);
note.setContent(content);
note.setAuthor(author);
if (actor != null) {
note.setAuthorDisplayName(actor.displayName());
note.setAuthorGithubUsername(actor.githubUsername());
note.setAuthorRole(actor.role() != null ? actor.role().name() : null);
}
Comment on lines +48 to +59
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 | 🔴 Critical

Missing authorization check in addNote allows unauthorized note writes.

addNote loads the case and saves a note without validating that the actor may access/modify that case.

Proposed fix
 `@Transactional`
 public void addNote(UUID caseId, String content, Actor actor) {
+    if (actor == null) {
+        throw new NotAuthorizedException("Not allowed to add note");
+    }
     CaseEntity caseEntity = caseRepository.findById(caseId)
             .orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Case not found"));
+    requireCanRead(actor, caseEntity);
 
     CaseNoteEntity note = new CaseNoteEntity();
     note.setCaseEntity(caseEntity);
     note.setContent(content);
-    if (actor != null) {
-        note.setAuthorDisplayName(actor.displayName());
-        note.setAuthorGithubUsername(actor.githubUsername());
-        note.setAuthorRole(actor.role() != null ? actor.role().name() : null);
-    }
+    note.setAuthorDisplayName(actor.displayName());
+    note.setAuthorGithubUsername(actor.githubUsername());
+    note.setAuthorRole(actor.role() != null ? actor.role().name() : null);
     note.setCreatedAt(Instant.now());
 
     caseNoteRepository.save(note);
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/projektarendehantering/application/service/CaseService.java`
around lines 48 - 59, addNote currently creates and saves a CaseNoteEntity
without verifying the actor's permission; update addNote in CaseService to check
authorization on the loaded CaseEntity before creating/saving the note by
calling your authorization component (e.g., AuthorizationService or
PermissionService) with the actor and CaseEntity, and if the check fails throw a
ResponseStatusException(HttpStatus.FORBIDDEN, "Not authorized to add note");
retain the existing actor null handling but ensure null actors are denied unless
your auth service explicitly permits system/anonymous writes.

note.setCreatedAt(Instant.now());

caseNoteRepository.save(note);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
import org.example.projektarendehantering.presentation.dto.EmployeeDTO;
import org.springframework.stereotype.Component;

import java.nio.charset.StandardCharsets;
import java.util.UUID;

@Component
public class EmployeeMapper {

Expand All @@ -13,6 +16,7 @@ public EmployeeDTO toDTO(EmployeeEntity entity) {
return new EmployeeDTO(
entity.getId(),
entity.getDisplayName(),
entity.getGithubUsername(),
entity.getRole(),
entity.getCreatedAt()
);
Expand All @@ -22,7 +26,13 @@ public EmployeeEntity toEntity(EmployeeCreateDTO dto) {
if (dto == null) return null;
EmployeeEntity entity = new EmployeeEntity();
entity.setDisplayName(dto.getDisplayName());
entity.setGithubUsername(dto.getGithubUsername());
entity.setRole(dto.getRole());

if (dto.getGithubUsername() != null && !dto.getGithubUsername().isBlank()) {
UUID id = UUID.nameUUIDFromBytes(dto.getGithubUsername().getBytes(StandardCharsets.UTF_8));
entity.setId(id);
}
return entity;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.example.projektarendehantering.application.service;

import org.example.projektarendehantering.common.Actor;
import org.example.projektarendehantering.common.BadRequestException;
import org.example.projektarendehantering.common.NotAuthorizedException;
import org.example.projektarendehantering.common.Role;
import org.example.projektarendehantering.infrastructure.persistence.EmployeeEntity;
Expand Down Expand Up @@ -30,8 +31,15 @@ public EmployeeService(EmployeeRepository employeeRepository, EmployeeMapper emp
@Transactional
public EmployeeDTO createEmployee(Actor actor, EmployeeCreateDTO dto) {
requireCanManageEmployees(actor);

if (employeeRepository.findByGithubUsername(dto.getGithubUsername()).isPresent()) {
throw new BadRequestException("EMPLOYEE_EXISTS", "Employee with username " + dto.getGithubUsername() + " already exists");
}

EmployeeEntity entity = employeeMapper.toEntity(dto);
entity.setId(UUID.randomUUID());
if (entity.getId() == null) {
entity.setId(UUID.randomUUID());
}
entity.setCreatedAt(Instant.now());
return employeeMapper.toDTO(employeeRepository.save(entity));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import java.util.UUID;

public record Actor(UUID userId, Role role) {
public record Actor(UUID userId, Role role, String displayName, String githubUsername) {


}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.core.userdetails.User;
Expand All @@ -12,6 +13,7 @@

@Configuration
@EnableWebSecurity
@EnableMethodSecurity
public class SecurityConfig {

@Bean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ public class CaseNoteEntity {

private String content;

private String author;
private String authorDisplayName;
private String authorGithubUsername;
private String authorRole;

private Instant createdAt;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.example.projektarendehantering.infrastructure.persistence;

import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.EnumType;
import jakarta.persistence.Enumerated;
Expand All @@ -19,16 +20,20 @@ public class EmployeeEntity {

private String displayName;

@Column(unique = true)
private String githubUsername;

@Enumerated(EnumType.STRING)
private Role role;

private Instant createdAt;

public EmployeeEntity() {}

public EmployeeEntity(UUID id, String displayName, Role role, Instant createdAt) {
public EmployeeEntity(UUID id, String displayName, String githubUsername, Role role, Instant createdAt) {
this.id = id;
this.displayName = displayName;
this.githubUsername = githubUsername;
this.role = role;
this.createdAt = createdAt;
}
Expand All @@ -39,6 +44,9 @@ public EmployeeEntity(UUID id, String displayName, Role role, Instant createdAt)
public String getDisplayName() { return displayName; }
public void setDisplayName(String displayName) { this.displayName = displayName; }

public String getGithubUsername() { return githubUsername; }
public void setGithubUsername(String githubUsername) { this.githubUsername = githubUsername; }

public Role getRole() { return role; }
public void setRole(Role role) { this.role = role; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

import org.springframework.data.jpa.repository.JpaRepository;

import java.util.Optional;
import java.util.UUID;

public interface EmployeeRepository extends JpaRepository<EmployeeEntity, UUID> {
Optional<EmployeeEntity> findByGithubUsername(String githubUsername);
}

Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
import org.example.projektarendehantering.common.Actor;
import org.example.projektarendehantering.common.NotAuthorizedException;
import org.example.projektarendehantering.common.Role;
import org.example.projektarendehantering.infrastructure.persistence.EmployeeRepository;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationToken;
import org.springframework.stereotype.Component;

import java.nio.charset.StandardCharsets;
Expand All @@ -16,16 +18,39 @@
@Component
public class SecurityActorAdapter {

private final EmployeeRepository employeeRepository;

public SecurityActorAdapter(EmployeeRepository employeeRepository) {
this.employeeRepository = employeeRepository;
}

public Actor currentUser() {
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();

if (authentication == null || !authentication.isAuthenticated() || "anonymousUser".equals(authentication.getName())) {
throw new NotAuthorizedException("User not authenticated");
}

// Try to find the username (GitHub 'login' attribute) or fallback to name
String name = authentication.getName();
if (authentication instanceof OAuth2AuthenticationToken oauth2Token) {
String login = oauth2Token.getPrincipal().getAttribute("login");
if (login != null) {
name = login;
}
}

// Create a deterministic UUID based on the username/name
UUID userId = UUID.nameUUIDFromBytes(authentication.getName().getBytes(StandardCharsets.UTF_8));
UUID userId = UUID.nameUUIDFromBytes(name.getBytes(StandardCharsets.UTF_8));

// 1. Try finding an employee with this UUID
var employee = employeeRepository.findById(userId);
if (employee.isPresent()) {
var e = employee.get();
return new Actor(userId, e.getRole(), e.getDisplayName(), e.getGithubUsername());
}

// 2. Fallback to existing logic (checking Spring authorities)
Role role = Role.PATIENT;
if (authentication.getAuthorities().stream().anyMatch(a -> a.getAuthority().equals("ROLE_MANAGER"))) {
role = Role.MANAGER;
Expand All @@ -37,6 +62,6 @@ public Actor currentUser() {
role = Role.PATIENT;
}

return new Actor(userId, role);
return new Actor(userId, role, null, name);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,19 @@ public class CaseNoteDTO {

private UUID id;
private String content;
private String author;
private String authorDisplayName;
private String authorGithubUsername;
private String authorRole;
private Instant createdAt;

public CaseNoteDTO() {}

public CaseNoteDTO(UUID id, String content, String author, Instant createdAt) {
public CaseNoteDTO(UUID id, String content, String authorDisplayName, String authorGithubUsername, String authorRole, Instant createdAt) {
this.id = id;
this.content = content;
this.author = author;
this.authorDisplayName = authorDisplayName;
this.authorGithubUsername = authorGithubUsername;
this.authorRole = authorRole;
this.createdAt = createdAt;
}

Expand All @@ -25,8 +29,14 @@ public CaseNoteDTO(UUID id, String content, String author, Instant createdAt) {
public String getContent() { return content; }
public void setContent(String content) { this.content = content; }

public String getAuthor() { return author; }
public void setAuthor(String author) { this.author = author; }
public String getAuthorDisplayName() { return authorDisplayName; }
public void setAuthorDisplayName(String authorDisplayName) { this.authorDisplayName = authorDisplayName; }

public String getAuthorGithubUsername() { return authorGithubUsername; }
public void setAuthorGithubUsername(String authorGithubUsername) { this.authorGithubUsername = authorGithubUsername; }

public String getAuthorRole() { return authorRole; }
public void setAuthorRole(String authorRole) { this.authorRole = authorRole; }

public Instant getCreatedAt() { return createdAt; }
public void setCreatedAt(Instant createdAt) { this.createdAt = createdAt; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,26 @@ public class EmployeeCreateDTO {
@NotBlank
private String displayName;

@NotBlank
private String githubUsername;

@NotNull
private Role role;

public EmployeeCreateDTO() {}

public EmployeeCreateDTO(String displayName, Role role) {
public EmployeeCreateDTO(String displayName, String githubUsername, Role role) {
this.displayName = displayName;
this.githubUsername = githubUsername;
this.role = role;
}

public String getDisplayName() { return displayName; }
public void setDisplayName(String displayName) { this.displayName = displayName; }

public String getGithubUsername() { return githubUsername; }
public void setGithubUsername(String githubUsername) { this.githubUsername = githubUsername; }

public Role getRole() { return role; }
public void setRole(Role role) { this.role = role; }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,16 @@ public class EmployeeDTO {

private UUID id;
private String displayName;
private String githubUsername;
private Role role;
private Instant createdAt;

public EmployeeDTO() {}

public EmployeeDTO(UUID id, String displayName, Role role, Instant createdAt) {
public EmployeeDTO(UUID id, String displayName, String githubUsername, Role role, Instant createdAt) {
this.id = id;
this.displayName = displayName;
this.githubUsername = githubUsername;
this.role = role;
this.createdAt = createdAt;
}
Expand All @@ -27,6 +29,9 @@ public EmployeeDTO(UUID id, String displayName, Role role, Instant createdAt) {
public String getDisplayName() { return displayName; }
public void setDisplayName(String displayName) { this.displayName = displayName; }

public String getGithubUsername() { return githubUsername; }
public void setGithubUsername(String githubUsername) { this.githubUsername = githubUsername; }

public Role getRole() { return role; }
public void setRole(Role role) { this.role = role; }

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package org.example.projektarendehantering.presentation.web;

import jakarta.validation.Valid;
import org.example.projektarendehantering.application.service.EmployeeService;
import org.example.projektarendehantering.common.Actor;
import org.example.projektarendehantering.common.Role;
import org.example.projektarendehantering.infrastructure.security.SecurityActorAdapter;
import org.example.projektarendehantering.presentation.dto.EmployeeCreateDTO;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.stereotype.Controller;
import org.springframework.ui.Model;
import org.springframework.validation.BindingResult;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.ModelAttribute;
import org.springframework.web.bind.annotation.PostMapping;

@Controller
public class EmployeeUiController {

private final EmployeeService employeeService;
private final SecurityActorAdapter securityActorAdapter;

public EmployeeUiController(EmployeeService employeeService, SecurityActorAdapter securityActorAdapter) {
this.employeeService = employeeService;
this.securityActorAdapter = securityActorAdapter;
}

@GetMapping("/ui/employees")
public String listEmployees(Model model) {
Actor actor = securityActorAdapter.currentUser();
model.addAttribute("employees", employeeService.getAllEmployees(actor));
return "employees/list";
}

@GetMapping("/ui/employees/new")
@PreAuthorize("hasRole('MANAGER')")
public String newEmployee(Model model) {
model.addAttribute("employeeCreateDTO", new EmployeeCreateDTO());
model.addAttribute("roles", Role.values());
return "employees/new";
}
Comment on lines +35 to +41
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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether endpoint-level restrictions already exist for /ui/employees/new
rg -nP --type=java '(/ui/employees/new|/ui/employees)' -C3
rg -nP --type=java 'requestMatchers\s*\(|antMatchers\s*\(|mvcMatchers\s*\(' -C3
rg -nP --type=java '@EnableMethodSecurity|@EnableGlobalMethodSecurity|@PreAuthorize' -C3

Repository: ithsjava25/project-backend-code-trauma-team

Length of output: 4676


🏁 Script executed:

# Check if method-level security is enabled anywhere
rg -nP --type=java '@EnableMethodSecurity|@EnableGlobalMethodSecurity' -A5 -B2

# Get complete SecurityConfig
fd --type=f -name 'SecurityConfig.java' --exec cat -n {}

# Get complete EmployeeUiController to verify all endpoints
fd --type=f -name 'EmployeeUiController.java' --exec cat -n {}

# Check what imports are already in EmployeeUiController
rg -nP --type=java '^import' src/main/java/org/example/projektarendehantering/presentation/web/EmployeeUiController.java

Repository: ithsjava25/project-backend-code-trauma-team

Length of output: 1305


🏁 Script executed:

# Check if method-level security is enabled
rg '@EnableMethodSecurity|@EnableGlobalMethodSecurity' --type java -A5 -B2

# Get complete SecurityConfig
cat src/main/java/org/example/projektarendehantering/infrastructure/config/SecurityConfig.java

# Get complete EmployeeUiController
cat src/main/java/org/example/projektarendehantering/presentation/web/EmployeeUiController.java

Repository: ithsjava25/project-backend-code-trauma-team

Length of output: 3980


🏁 Script executed:

# Find and read EmployeeService
find . -type f -name 'EmployeeService.java' | head -1 | xargs cat

# Also check for any method-level security annotations in the application main class
find . -type f -name '*Application.java' -o -name '*SpringBootApplication.java' | xargs grep -l '@EnableMethodSecurity\|@EnableGlobalMethodSecurity\|@SpringBootApplication'

Repository: ithsjava25/project-backend-code-trauma-team

Length of output: 2603


🏁 Script executed:

# Search for any `@PreAuthorize` usage in the codebase
rg '@PreAuthorize' --type java

# Check the main application class
cat ./src/main/java/org/example/projektarendehantering/ProjektArendehanteringApplication.java

Repository: ithsjava25/project-backend-code-trauma-team

Length of output: 457


Add authorization check to GET /ui/employees/new endpoint.

The form page is served to any authenticated user without a role check. While unauthenticated access is prevented by servlet-level security, non-managers can still view the form even though POST operations reject them via employeeService.createEmployee(actor, dto). Add endpoint-level protection to prevent unauthorized users from accessing the form.

To implement the suggested fix with @PreAuthorize, you must first enable method-level security in SecurityConfig:

Required changes
  1. Add @EnableMethodSecurity to SecurityConfig:
`@Configuration`
`@EnableWebSecurity`
+@EnableMethodSecurity
public class SecurityConfig {
  1. Then apply @PreAuthorize to the GET endpoint:
+import org.springframework.security.access.prepost.PreAuthorize;
 
-    `@GetMapping`("/ui/employees/new")
+    `@PreAuthorize`("hasRole('MANAGER')")
+    `@GetMapping`("/ui/employees/new")
     public String newEmployee(Model model) {
         model.addAttribute("employeeCreateDTO", new EmployeeCreateDTO());
         model.addAttribute("roles", Role.values());
         return "employees/new";
     }

Alternatively, add an authorization check directly in the controller method (without requiring @EnableMethodSecurity).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/projektarendehantering/presentation/web/EmployeeUiController.java`
around lines 34 - 39, Add method-level authorization so only managers can open
the new employee form: enable method security by annotating SecurityConfig with
`@EnableMethodSecurity`, then protect the controller method
EmployeeUiController.newEmployee with `@PreAuthorize` (e.g.
`@PreAuthorize`("hasRole('MANAGER')")) so non-managers cannot access the GET
/ui/employees/new page; ensure the newEmployee method imports the PreAuthorize
annotation (or, if you prefer not to enable method security, perform an explicit
role check inside EmployeeUiController.newEmployee using SecurityContextHolder
and redirect/deny if the caller lacks the MANAGER role).


@PostMapping("/ui/employees/new")
public String createEmployee(@Valid @ModelAttribute("employeeCreateDTO") EmployeeCreateDTO dto, BindingResult result, Model model) {
if (result.hasErrors()) {
model.addAttribute("roles", Role.values());
return "employees/new";
}

Actor actor = securityActorAdapter.currentUser();
employeeService.createEmployee(actor, dto);
return "redirect:/ui/employees";
}
}
Loading