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
@@ -1,9 +1,15 @@
package org.example.projektarendehantering.application.service;

import org.example.projektarendehantering.common.Actor;
import org.example.projektarendehantering.common.NotAuthorizedException;
import org.example.projektarendehantering.common.Role;
import org.example.projektarendehantering.infrastructure.persistence.CaseEntity;
import org.example.projektarendehantering.infrastructure.persistence.CaseRepository;
import org.example.projektarendehantering.infrastructure.persistence.EmployeeEntity;
import org.example.projektarendehantering.infrastructure.persistence.EmployeeRepository;
import org.example.projektarendehantering.infrastructure.persistence.PatientEntity;
import org.example.projektarendehantering.infrastructure.persistence.PatientRepository;
import org.example.projektarendehantering.presentation.dto.CaseAssignmentDTO;
import org.example.projektarendehantering.presentation.dto.CaseDTO;
import org.springframework.http.HttpStatus;
import org.springframework.stereotype.Service;
Expand All @@ -13,6 +19,7 @@
import java.time.Instant;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;

Expand All @@ -22,22 +29,35 @@ public class CaseService {
private final CaseRepository caseRepository;
private final CaseMapper caseMapper;
private final PatientRepository patientRepository;
private final EmployeeRepository employeeRepository;

public CaseService(CaseRepository caseRepository, CaseMapper caseMapper, PatientRepository patientRepository) {
public CaseService(
CaseRepository caseRepository,
CaseMapper caseMapper,
PatientRepository patientRepository,
EmployeeRepository employeeRepository
) {
this.caseRepository = caseRepository;
this.caseMapper = caseMapper;
this.patientRepository = patientRepository;
this.employeeRepository = employeeRepository;
}

@Transactional
public CaseDTO createCase(CaseDTO caseDTO) {
public CaseDTO createCase(Actor actor, CaseDTO caseDTO) {
if (!canCreate(actor)) {
throw new NotAuthorizedException("Not allowed to create cases");
}
if (caseDTO.getPatientId() == null) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "patientId is required");
}
CaseEntity entity = caseMapper.toEntity(caseDTO);
PatientEntity patient = patientRepository.findById(caseDTO.getPatientId())
.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Patient not found"));
entity.setPatient(patient);
if (isDoctor(actor) || isManager(actor)) {
entity.setOwnerId(actor.userId());
}
if (entity.getStatus() == null) {
entity.setStatus("OPEN");
}
Expand All @@ -49,21 +69,130 @@ public CaseDTO createCase(CaseDTO caseDTO) {
}

@Transactional(readOnly = true)
public Optional<CaseDTO> getCase(UUID id) {
return caseRepository.findById(id).map(caseMapper::toDTO);
public Optional<CaseDTO> getCase(Actor actor, UUID id) {
return caseRepository.findById(id)
.map(entity -> {
requireCanRead(actor, entity);
return caseMapper.toDTO(entity);
});
}

@Transactional(readOnly = true)
public List<CaseDTO> getAllCases() {
return caseRepository.findAll().stream()
.map(caseMapper::toDTO)
.collect(Collectors.toList());
public List<CaseDTO> getAllCases(Actor actor) {
if (isManager(actor)) {
return caseRepository.findAll().stream()
.map(caseMapper::toDTO)
.collect(Collectors.toList());
}
if (isDoctor(actor)) {
return caseRepository.findAllByOwnerId(actor.userId()).stream()
.map(caseMapper::toDTO)
.collect(Collectors.toList());
}
if (isNurse(actor)) {
return caseRepository.findAllByHandlerId(actor.userId()).stream()
.map(caseMapper::toDTO)
.collect(Collectors.toList());
}
if (isPatient(actor)) {
return caseRepository.findAllByPatient_Id(actor.userId()).stream()
.map(caseMapper::toDTO)
.collect(Collectors.toList());
}
if (isOther(actor)) {
return caseRepository.findAllByOtherId(actor.userId()).stream()
.map(caseMapper::toDTO)
.collect(Collectors.toList());
}
throw new NotAuthorizedException("Not allowed to list cases");
}

@Transactional(readOnly = true)
public List<CaseDTO> getCasesForPatient(UUID patientId) {
public List<CaseDTO> getCasesForPatient(Actor actor, UUID patientId) {
return caseRepository.findAllByPatient_Id(patientId).stream()
.peek(entity -> requireCanRead(actor, entity))
.map(caseMapper::toDTO)
.collect(Collectors.toList());
}

@Transactional
public CaseDTO assignUsers(Actor actor, UUID caseId, CaseAssignmentDTO dto) {
if (!isManager(actor) && !isDoctor(actor)) {
throw new NotAuthorizedException("Not allowed to assign users to case");
}
CaseEntity entity = caseRepository.findById(caseId)
.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Case not found"));
if (isDoctor(actor)) {
if (entity.getOwnerId() == null || !entity.getOwnerId().equals(actor.userId())) {
throw new NotAuthorizedException("Not allowed to modify assignments for this case");
}
if (dto.getOwnerId() != null) {
throw new NotAuthorizedException("Not allowed to change owner for this case");
}
}

if (isManager(actor) && dto.getOwnerId() != null) {
UUID ownerId = requireEmployeeWithRole(dto.getOwnerId(), Set.of(Role.DOCTOR, Role.CASE_OWNER), "ownerId");
entity.setOwnerId(ownerId);
}
if (dto.getHandlerId() != null) {
UUID handlerId = requireEmployeeWithRole(dto.getHandlerId(), Set.of(Role.NURSE, Role.HANDLER), "handlerId");
entity.setHandlerId(handlerId);
}
if (dto.getOtherId() != null) {
UUID otherId = requireEmployeeWithRole(dto.getOtherId(), Set.of(Role.OTHER), "otherId");
entity.setOtherId(otherId);
}
return caseMapper.toDTO(caseRepository.save(entity));
}

private UUID requireEmployeeWithRole(UUID id, Set<Role> allowedRoles, String fieldName) {
EmployeeEntity employee = employeeRepository.findById(id)
.orElseThrow(() -> new ResponseStatusException(
HttpStatus.BAD_REQUEST,
fieldName + " refers to a non-existent employee: " + id
));
if (employee.getRole() == null || !allowedRoles.contains(employee.getRole())) {
throw new ResponseStatusException(
HttpStatus.BAD_REQUEST,
fieldName + " must refer to an employee with role " + allowedRoles + " (was " + employee.getRole() + "): " + id
);
}
return id;
}

private void requireCanRead(Actor actor, CaseEntity entity) {
if (isManager(actor)) return;
if (isDoctor(actor) && actor.userId().equals(entity.getOwnerId())) return;
if (isNurse(actor) && actor.userId().equals(entity.getHandlerId())) return;
if (isPatient(actor)
&& entity.getPatient() != null
&& actor.userId().equals(entity.getPatient().getId())) return;
if (isOther(actor) && actor.userId().equals(entity.getOtherId())) return;
throw new NotAuthorizedException("Not allowed to read this case");
}

private boolean canCreate(Actor actor) {
return isManager(actor) || isDoctor(actor);
}

private boolean isManager(Actor actor) {
return actor.role() == Role.MANAGER || actor.role() == Role.ADMIN;
}

private boolean isDoctor(Actor actor) {
return actor.role() == Role.DOCTOR || actor.role() == Role.CASE_OWNER;
}

private boolean isNurse(Actor actor) {
return actor.role() == Role.NURSE || actor.role() == Role.HANDLER;
}

private boolean isPatient(Actor actor) {
return actor.role() == Role.PATIENT;
}

private boolean isOther(Actor actor) {
return actor.role() == Role.OTHER;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package org.example.projektarendehantering.application.service;

import org.example.projektarendehantering.infrastructure.persistence.EmployeeEntity;
import org.example.projektarendehantering.presentation.dto.EmployeeCreateDTO;
import org.example.projektarendehantering.presentation.dto.EmployeeDTO;
import org.springframework.stereotype.Component;

@Component
public class EmployeeMapper {

public EmployeeDTO toDTO(EmployeeEntity entity) {
if (entity == null) return null;
return new EmployeeDTO(
entity.getId(),
entity.getDisplayName(),
entity.getRole(),
entity.getCreatedAt()
);
}

public EmployeeEntity toEntity(EmployeeCreateDTO dto) {
if (dto == null) return null;
EmployeeEntity entity = new EmployeeEntity();
entity.setDisplayName(dto.getDisplayName());
entity.setRole(dto.getRole());
return entity;
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package org.example.projektarendehantering.application.service;

import org.example.projektarendehantering.common.Actor;
import org.example.projektarendehantering.common.NotAuthorizedException;
import org.example.projektarendehantering.common.Role;
import org.example.projektarendehantering.infrastructure.persistence.EmployeeEntity;
import org.example.projektarendehantering.infrastructure.persistence.EmployeeRepository;
import org.example.projektarendehantering.presentation.dto.EmployeeCreateDTO;
import org.example.projektarendehantering.presentation.dto.EmployeeDTO;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.time.Instant;
import java.util.List;
import java.util.Optional;
import java.util.UUID;
import java.util.stream.Collectors;

@Service
public class EmployeeService {

private final EmployeeRepository employeeRepository;
private final EmployeeMapper employeeMapper;

public EmployeeService(EmployeeRepository employeeRepository, EmployeeMapper employeeMapper) {
this.employeeRepository = employeeRepository;
this.employeeMapper = employeeMapper;
}

@Transactional
public EmployeeDTO createEmployee(Actor actor, EmployeeCreateDTO dto) {
requireCanManageEmployees(actor);
EmployeeEntity entity = employeeMapper.toEntity(dto);
entity.setId(UUID.randomUUID());
entity.setCreatedAt(Instant.now());
return employeeMapper.toDTO(employeeRepository.save(entity));
}
Comment on lines +30 to +37
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 | 🟡 Minor

Add null check for dto to prevent NPE.

If dto is null, employeeMapper.toEntity(dto) returns null (per the mapper implementation), and the subsequent entity.setId() call will throw a NullPointerException. Consider validating the input early.

🛡️ Proposed fix
 `@Transactional`
 public EmployeeDTO createEmployee(Actor actor, EmployeeCreateDTO dto) {
     requireCanManageEmployees(actor);
+    if (dto == null) {
+        throw new IllegalArgumentException("EmployeeCreateDTO cannot be null");
+    }
     EmployeeEntity entity = employeeMapper.toEntity(dto);
     entity.setId(UUID.randomUUID());
     entity.setCreatedAt(Instant.now());
     return employeeMapper.toDTO(employeeRepository.save(entity));
 }
🤖 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/EmployeeService.java`
around lines 30 - 37, createEmployee currently calls
employeeMapper.toEntity(dto) without validating dto, causing a
NullPointerException when dto is null; add a null check at the start of
createEmployee (after requireCanManageEmployees(actor) or before, as preferred)
using Objects.requireNonNull(dto, "dto must not be null") or explicitly throw an
IllegalArgumentException with a clear message, so employeeMapper.toEntity(dto)
cannot return null and subsequent calls like entity.setId(...) and
entity.setCreatedAt(...) are safe; keep the rest of the logic
(employeeMapper.toEntity, entity.setId, entity.setCreatedAt,
employeeRepository.save, employeeMapper.toDTO) unchanged.


@Transactional(readOnly = true)
public Optional<EmployeeDTO> getEmployee(Actor actor, UUID id) {
requireCanManageEmployees(actor);
return employeeRepository.findById(id).map(employeeMapper::toDTO);
}

@Transactional(readOnly = true)
public List<EmployeeDTO> getAllEmployees(Actor actor) {
requireCanManageEmployees(actor);
return employeeRepository.findAll().stream()
.map(employeeMapper::toDTO)
.collect(Collectors.toList());
Comment on lines +30 to +50
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

Add RBAC to the employee service before exposing it publicly.

These methods are the only new service surface in this PR that is not actor-aware. Combined with src/main/java/org/example/projektarendehantering/presentation/rest/EmployeeController.java:23-37, any authenticated caller reaching the controller can create employees or read the full employee directory, which is a privilege escalation.

Suggested direction
+import org.example.projektarendehantering.common.Actor;
+import org.example.projektarendehantering.common.NotAuthorizedException;
+import org.example.projektarendehantering.common.Role;
+
 `@Service`
 public class EmployeeService {
@@
     `@Transactional`
-    public EmployeeDTO createEmployee(EmployeeCreateDTO dto) {
+    public EmployeeDTO createEmployee(Actor actor, EmployeeCreateDTO dto) {
+        requireManager(actor);
         EmployeeEntity entity = employeeMapper.toEntity(dto);
         entity.setId(UUID.randomUUID());
         entity.setCreatedAt(Instant.now());
         return employeeMapper.toDTO(employeeRepository.save(entity));
     }
@@
     `@Transactional`(readOnly = true)
-    public Optional<EmployeeDTO> getEmployee(UUID id) {
+    public Optional<EmployeeDTO> getEmployee(Actor actor, UUID id) {
+        requireManager(actor);
         return employeeRepository.findById(id).map(employeeMapper::toDTO);
     }
@@
     `@Transactional`(readOnly = true)
-    public List<EmployeeDTO> getAllEmployees() {
+    public List<EmployeeDTO> getAllEmployees(Actor actor) {
+        requireManager(actor);
         return employeeRepository.findAll().stream()
                 .map(employeeMapper::toDTO)
                 .collect(Collectors.toList());
     }
+
+    private void requireManager(Actor actor) {
+        if (actor.role() != Role.MANAGER && actor.role() != Role.ADMIN) {
+            throw new NotAuthorizedException("Not allowed to manage employees");
+        }
+    }
 }

EmployeeController then needs to pass the current actor the same way the case endpoints do.

📝 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.

Suggested change
@Transactional
public EmployeeDTO createEmployee(EmployeeCreateDTO dto) {
EmployeeEntity entity = employeeMapper.toEntity(dto);
entity.setId(UUID.randomUUID());
entity.setCreatedAt(Instant.now());
return employeeMapper.toDTO(employeeRepository.save(entity));
}
@Transactional(readOnly = true)
public Optional<EmployeeDTO> getEmployee(UUID id) {
return employeeRepository.findById(id).map(employeeMapper::toDTO);
}
@Transactional(readOnly = true)
public List<EmployeeDTO> getAllEmployees() {
return employeeRepository.findAll().stream()
.map(employeeMapper::toDTO)
.collect(Collectors.toList());
import org.example.projektarendehantering.common.Actor;
import org.example.projektarendehantering.common.NotAuthorizedException;
import org.example.projektarendehantering.common.Role;
`@Transactional`
public EmployeeDTO createEmployee(Actor actor, EmployeeCreateDTO dto) {
requireManager(actor);
EmployeeEntity entity = employeeMapper.toEntity(dto);
entity.setId(UUID.randomUUID());
entity.setCreatedAt(Instant.now());
return employeeMapper.toDTO(employeeRepository.save(entity));
}
`@Transactional`(readOnly = true)
public Optional<EmployeeDTO> getEmployee(Actor actor, UUID id) {
requireManager(actor);
return employeeRepository.findById(id).map(employeeMapper::toDTO);
}
`@Transactional`(readOnly = true)
public List<EmployeeDTO> getAllEmployees(Actor actor) {
requireManager(actor);
return employeeRepository.findAll().stream()
.map(employeeMapper::toDTO)
.collect(Collectors.toList());
}
private void requireManager(Actor actor) {
if (actor.role() != Role.MANAGER && actor.role() != Role.ADMIN) {
throw new NotAuthorizedException("Not allowed to manage employees");
}
}
🤖 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/EmployeeService.java`
around lines 27 - 44, The service methods createEmployee, getEmployee, and
getAllEmployees lack RBAC checks and must be made actor-aware: change the
signatures (or add overloads) to accept the current actor/principal (or fetch it
from a SecurityContext) and enforce permission checks inside EmployeeService
(e.g., verify actor has ROLE_HR or a specific permission before proceeding) when
invoked from EmployeeController; update EmployeeController to pass the current
actor to EmployeeService (mirroring how case endpoints pass the actor) and
ensure repository calls (employeeRepository.save, findById, findAll) only run
after the authorization check succeeds.

}

private void requireCanManageEmployees(Actor actor) {
if (actor == null) {
throw new NotAuthorizedException("Missing actor");
}
if (actor.role() == Role.MANAGER || actor.role() == Role.ADMIN) {
return;
}
throw new NotAuthorizedException("Not allowed to access employees");
}
}

11 changes: 11 additions & 0 deletions src/main/java/org/example/projektarendehantering/common/Role.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,17 @@
* Note: enum constant names are intended to be stable because infrastructure may parse them from headers.
*/
public enum Role {
/**
* New naming (preferred).
*/
MANAGER,
DOCTOR,
NURSE,
PATIENT,

/**
* Legacy naming (kept for backward compatibility with header parsing).
*/
CASE_OWNER,
HANDLER,
ADMIN,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ public class CaseEntity {
private String title;
private String description;
private Instant createdAt;
private UUID handlerId;
private UUID otherId;

@ManyToOne(optional = true)
@JoinColumn(name = "patient_id", nullable = true) // Optional because the patient can be null
Expand All @@ -42,6 +44,12 @@ public CaseEntity(UUID id, String status, UUID ownerId, String title, String des
public UUID getOwnerId() { return ownerId; }
public void setOwnerId(UUID ownerId) { this.ownerId = ownerId; }

public UUID getHandlerId() { return handlerId; }
public void setHandlerId(UUID handlerId) { this.handlerId = handlerId; }

public UUID getOtherId() { return otherId; }
public void setOtherId(UUID otherId) { this.otherId = otherId; }

public String getTitle() { return title; }
public void setTitle(String title) { this.title = title; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,7 @@

public interface CaseRepository extends JpaRepository<CaseEntity, UUID> {
List<CaseEntity> findAllByPatient_Id(UUID patientId);
List<CaseEntity> findAllByOwnerId(UUID ownerId);
List<CaseEntity> findAllByHandlerId(UUID handlerId);
List<CaseEntity> findAllByOtherId(UUID otherId);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package org.example.projektarendehantering.infrastructure.persistence;

import jakarta.persistence.Entity;
import jakarta.persistence.EnumType;
import jakarta.persistence.Enumerated;
import jakarta.persistence.Id;
import jakarta.persistence.Table;
import org.example.projektarendehantering.common.Role;

import java.time.Instant;
import java.util.UUID;

@Entity
@Table(name = "employees")
public class EmployeeEntity {

@Id
private UUID id;

private String displayName;

@Enumerated(EnumType.STRING)
private Role role;

private Instant createdAt;

public EmployeeEntity() {}

public EmployeeEntity(UUID id, String displayName, Role role, Instant createdAt) {
this.id = id;
this.displayName = displayName;
this.role = role;
this.createdAt = createdAt;
}

public UUID getId() { return id; }
public void setId(UUID id) { this.id = id; }

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

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

public Instant getCreatedAt() { return createdAt; }
public void setCreatedAt(Instant createdAt) { this.createdAt = createdAt; }
}

Loading