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
25 changes: 25 additions & 0 deletions src/main/java/org/example/crimearchive/cases/CaseService.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
package org.example.crimearchive.cases;

import org.example.crimearchive.permissions.NullAuthzDeniedHandler;
import org.example.crimearchive.polis.Account;
import org.example.crimearchive.polis.UserRepository;
import org.example.crimearchive.reports.Report;
import org.example.crimearchive.reports.ReportRepository;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.security.authorization.method.HandleAuthorizationDenied;
import org.springframework.stereotype.Service;

import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

@Service("permittedFilesChecker")
public class CaseService {
Expand All @@ -19,6 +27,8 @@ public CaseService(ReportRepository reportRepository, CasesRepository casesRepos
this.userRepository = userRepository;
}

@PreAuthorize("hasRole('admin')")
@HandleAuthorizationDenied(handlerClass = NullAuthzDeniedHandler.class)
public void addAccountToCase(Long accountId, String caseNumber){
Optional<Account> account = userRepository.findById(accountId);
if (account.isEmpty()) throw new RuntimeException("account not found");
Expand All @@ -30,6 +40,21 @@ public void addAccountToCase(Long accountId, String caseNumber){
}
}

public List<Cases> getAuthzCases(long accountId) {
return casesRepository.findByAccountsId(accountId);
}

public Report getReports(Cases caseId) {
return reportRepository.getReportByCaseEntity(caseId);
}
Comment on lines +47 to +49

Copy link
Copy Markdown
Contributor

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:

find . -type f -name "*.java" | grep -E "(Case|Report)" | head -20

Repository: ithsjava25/project-backend-DurTre

Length of output: 958


🏁 Script executed:

# Search for Cases entity definition
rg "class Cases" --type java -A 20

Repository: ithsjava25/project-backend-DurTre

Length of output: 1806


🏁 Script executed:

# Search for Report entity definition
rg "class Report" --type java -A 20

Repository: ithsjava25/project-backend-DurTre

Length of output: 9246


🏁 Script executed:

# Check the current CaseService implementation
cat -n src/main/java/org/example/crimearchive/cases/CaseService.java

Repository: ithsjava25/project-backend-DurTre

Length of output: 3036


🏁 Script executed:

# Search for Report repository interface
rg "interface.*Report.*Repository" --type java -A 15

Repository: ithsjava25/project-backend-DurTre

Length of output: 1605


getReports should return a collection instead of a single Report.

The domain model defines a one-to-many relationship between Cases and Reports. Using getReportByCaseEntity() returns only a single report and will cause silent data loss when a case has multiple reports. The appropriate repository method findAllByCaseEntity() already exists.

Suggested fix
-    public Report getReports(Cases caseId) {
-        return reportRepository.getReportByCaseEntity(caseId);
+    public List<Report> getReports(Cases caseEntity) {
+        return reportRepository.findAllByCaseEntity(caseEntity);
     }
📝 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
public Report getReports(Cases caseId) {
return reportRepository.getReportByCaseEntity(caseId);
}
public List<Report> getReports(Cases caseEntity) {
return reportRepository.findAllByCaseEntity(caseEntity);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/example/crimearchive/cases/CaseService.java` around lines
47 - 49, The getReports method currently returns a single Report via
reportRepository.getReportByCaseEntity(caseId) but the domain is one-to-many;
change getReports(Cases caseId) to return a collection type (e.g., List<Report>
or Collection<Report>), call reportRepository.findAllByCaseEntity(caseId)
instead of getReportByCaseEntity, and update any imports and callers/ tests to
handle the collection return type; ensure method signature and usages of
getReports throughout the codebase are updated accordingly.


public Map<String, Set<Report>> getReportsWithCaseNumber(List<Cases> grantedCases) {
Map<String, Set<Report>> repos = grantedCases.stream().collect(Collectors.toMap(
cases -> cases.getCaseNumber(), c2 -> c2.getReports()
));
return repos;
}

//
// public List<Report> hasPermission(Long accountId){
// List<String> permittedCaseNumbers = permissionRepository.findAllPermittedReports(accountId);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/example/crimearchive/cases/Cases.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class Cases {
@ManyToMany
@JoinTable(
name = "account_cases",
joinColumns = @JoinColumn(name = "report_id"),
joinColumns = @JoinColumn(name = "case_id"),
inverseJoinColumns = @JoinColumn(name = "account_id")
)
private Set<Account> accounts = new HashSet<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ SecurityFilterChain securityFilterChain(HttpSecurity http) {
return http
.authorizeHttpRequests(auth -> {
auth.requestMatchers("/").permitAll();
auth.requestMatchers("/static/**").permitAll();
auth.requestMatchers("/*.css", "/images/**").permitAll();
auth.requestMatchers("/favicon.ico").permitAll();
auth.requestMatchers("/index").permitAll();
auth.requestMatchers("/error").permitAll();
auth.requestMatchers("/cases/add").hasRole("ADMIN");

auth.requestMatchers("/cases").hasRole("ADMIN");
//auth.requestMatchers("/cases/add").hasRole("ADMIN");
//auth.requestMatchers("/cases").hasRole("ADMIN");
Comment on lines +29 to +30

Copy link
Copy Markdown
Contributor

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 /cases endpoints have compensating method-level authorization.
rg -nP --type=java -C3 '@(GetMapping|PostMapping|RequestMapping)\(([^)]*"/cases[^"]*")'
rg -nP --type=java -C2 '@PreAuthorize|@Secured|@RolesAllowed'

Repository: ithsjava25/project-backend-DurTre

Length of output: 2154


🏁 Script executed:

cat -n src/main/java/org/example/crimearchive/controllers/HomeController.java | sed -n '53,80p'

Repository: ithsjava25/project-backend-DurTre

Length of output: 1393


🏁 Script executed:

cat -n src/main/java/org/example/crimearchive/config/SecurityConfiguration.java | sed -n '20,40p'

Repository: ithsjava25/project-backend-DurTre

Length of output: 1304


🏁 Script executed:

cat -n src/main/java/org/example/crimearchive/cases/CaseService.java

Repository: ithsjava25/project-backend-DurTre

Length of output: 3036


Reinstate explicit authorization for /cases endpoints.

The GET /cases endpoint (line 53 in HomeController) is now unprotected and accessible to any authenticated user—an access-control regression. The endpoint directly queries the cases repository without authorization checks.

The POST /cases/add endpoint has compensating protection via @PreAuthorize("hasRole('admin')") on CaseService.addAccountToCase() (line 30 in CaseService), but relying on implicit service-level authorization is weaker than explicit configuration-level guards. Restore the commented-out rules to enforce authorization consistently at the security configuration layer.

🔧 Suggested fix
-                    //auth.requestMatchers("/cases/add").hasRole("ADMIN");
-                    //auth.requestMatchers("/cases").hasRole("ADMIN");
+                    auth.requestMatchers("/cases/add").hasRole("ADMIN");
+                    auth.requestMatchers("/cases").hasRole("ADMIN");
📝 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
//auth.requestMatchers("/cases/add").hasRole("ADMIN");
//auth.requestMatchers("/cases").hasRole("ADMIN");
auth.requestMatchers("/cases/add").hasRole("ADMIN");
auth.requestMatchers("/cases").hasRole("ADMIN");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/example/crimearchive/config/SecurityConfiguration.java`
around lines 29 - 30, Reinstate explicit security rules in SecurityConfiguration
by re-enabling the request matcher entries that restrict the /cases endpoints:
add back the auth.requestMatchers("/cases").hasRole("ADMIN") and
auth.requestMatchers("/cases/add").hasRole("ADMIN") (or equivalent
antMatchers/authorizeHttpRequests syntax used in the class) so GET /cases and
POST /cases/add are enforced at the configuration layer; keep the service-level
`@PreAuthorize`("hasRole('admin')") on CaseService.addAccountToCase() as
defense-in-depth but ensure the SecurityConfiguration class explicitly denies
access to non-admins for these routes.


auth.requestMatchers("/private").hasRole("ADMIN");
auth.requestMatchers("/userpage").hasRole("USER");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import jakarta.validation.Valid;
import org.example.crimearchive.DTO.CreateReport;
import org.example.crimearchive.cases.CaseService;
import org.example.crimearchive.cases.Cases;
import org.example.crimearchive.cases.CasesRepository;
import org.example.crimearchive.polis.Account;
import org.example.crimearchive.reports.ReportService;
Expand All @@ -15,6 +16,7 @@
import org.springframework.validation.BindingResult;
import org.springframework.web.bind.annotation.*;

import java.util.List;
import java.util.UUID;

@Controller
Expand Down Expand Up @@ -59,7 +61,8 @@ public String casesPage(@RequestParam(required = false) Long accountId, Model mo

@PostMapping("/cases/add")
public String casesPage(@RequestParam Long addAccountId,
@RequestParam String case_number){
@RequestParam String case_number,
@AuthenticationPrincipal Account user) {
caseService.addAccountToCase(addAccountId, case_number);
return "redirect:/cases";
}
Expand All @@ -78,6 +81,15 @@ public String saveReport(
return "redirect:/userpage";
}

@GetMapping("/profile")
public String profilePage(@AuthenticationPrincipal Account user, Model model) {
List<Cases> caseList = caseService.getAuthzCases(user.getId());
model.addAttribute("currentUser", user);
model.addAttribute("cases", caseService.getReportsWithCaseNumber(caseList));
model.addAttribute("user", user);
return "profile";
}

@GetMapping("/reports/{uuid}/download/pdf")
public ResponseEntity<byte[]> downloadPdf(@PathVariable UUID uuid) {
return reportService.downloadPdf(uuid);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package org.example.crimearchive.permissions;

import org.aopalliance.intercept.MethodInvocation;
import org.jspecify.annotations.Nullable;
import org.springframework.security.authorization.AuthorizationResult;
import org.springframework.security.authorization.method.MethodAuthorizationDeniedHandler;
import org.springframework.stereotype.Component;

@Component
public class NullAuthzDeniedHandler implements MethodAuthorizationDeniedHandler {
@Override
public @Nullable Object handleDeniedInvocation(MethodInvocation methodInvocation, AuthorizationResult authorizationResult) {
// when authz fails, return null
return null;
Comment on lines +12 to +14

Copy link
Copy Markdown
Contributor

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:

fd -t f "NullAuthzDeniedHandler.java"

Repository: ithsjava25/project-backend-DurTre

Length of output: 155


🏁 Script executed:

fd -t f "CaseService.java"

Repository: ithsjava25/project-backend-DurTre

Length of output: 138


🏁 Script executed:

cat -n src/main/java/org/example/crimearchive/permissions/NullAuthzDeniedHandler.java

Repository: ithsjava25/project-backend-DurTre

Length of output: 846


🏁 Script executed:

cat -n src/main/java/org/example/crimearchive/cases/CaseService.java | head -50

Repository: ithsjava25/project-backend-DurTre

Length of output: 2412


🏁 Script executed:

rg "NullAuthzDeniedHandler" --type java

Repository: ithsjava25/project-backend-DurTre

Length of output: 505


🏁 Script executed:

rg "@HandleAuthorizationDenied" --type java

Repository: ithsjava25/project-backend-DurTre

Length of output: 214


🏁 Script executed:

rg "AccessDeniedException" --type java

Repository: ithsjava25/project-backend-DurTre

Length of output: 59


🏁 Script executed:

rg "MethodAuthorizationDeniedHandler" --type java

Repository: ithsjava25/project-backend-DurTre

Length of output: 407


Denied authorization is being silently swallowed on a write operation.

The handleDeniedInvocation method returns null for denied invocations, which means when addAccountToCase(...) in CaseService (line 31) fails authorization, the caller receives no error signal. Since this is a mutating operation, the client will appear to succeed when it actually was denied.

Throw AccessDeniedException instead to provide clear feedback that the request was denied.

+import org.springframework.security.access.AccessDeniedException;
@@
     `@Override`
     public `@Nullable` Object handleDeniedInvocation(MethodInvocation methodInvocation, AuthorizationResult authorizationResult) {
-        // when authz fails, return null
-        return null;
+        throw new AccessDeniedException("Access denied");
     }
📝 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
public @Nullable Object handleDeniedInvocation(MethodInvocation methodInvocation, AuthorizationResult authorizationResult) {
// when authz fails, return null
return null;
public `@Nullable` Object handleDeniedInvocation(MethodInvocation methodInvocation, AuthorizationResult authorizationResult) {
throw new AccessDeniedException("Access denied");
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/crimearchive/permissions/NullAuthzDeniedHandler.java`
around lines 12 - 14, NullAuthzDeniedHandler.handleDeniedInvocation currently
returns null on denied authorization which swallows failures for mutating calls
like CaseService.addAccountToCase; change handleDeniedInvocation in class
NullAuthzDeniedHandler to throw an
org.springframework.security.access.AccessDeniedException (include a descriptive
message and relevant details from MethodInvocation and/or AuthorizationResult)
instead of returning null so callers receive a clear access-denied error.

}
}
45 changes: 44 additions & 1 deletion src/main/java/org/example/crimearchive/polis/Account.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ public class Account implements UserDetails {
private Collection<? extends GrantedAuthority> authorities;
@ManyToMany(mappedBy = "accounts")
private Set<Cases> permittedCases = new HashSet<>();
private String fullName;
private String profession;
private String department;


public Long getId() {
Expand All @@ -31,10 +34,42 @@ public Account() {
this.username = "";
}

public Account(String username, String password, List<String> roles) {
public Account(String username, String password, List<String> roles,
String fullname, String profession, String department) {
this.username = username;
this.password = password;
this.authorities = setAuthoritesList(roles);
this.fullName = fullname;
this.profession = profession;
this.department = department;
}

public Account(String username, String password, List<String> roles) {
this(username, password, roles, null, null, null);
}

public String getFullName() {
return fullName;
}

public void setFullName(String fullName) {
this.fullName = fullName;
}

public String getProfession() {
return profession;
}

public void setProfession(String profession) {
this.profession = profession;
}

public String getDepartment() {
return department;
}

public void setDepartment(String department) {
this.department = department;
}

public void setId(Long id) {
Expand Down Expand Up @@ -70,4 +105,12 @@ public List<SimpleGrantedAuthority> setAuthoritesList(List<String> roles) {
r -> r.startsWith("ROLE_") ? new SimpleGrantedAuthority(r.toUpperCase()) : new SimpleGrantedAuthority("ROLE_" + r.toUpperCase()))
.toList();
}

public Set<Cases> getPermittedCases() {
return permittedCases;
}

public void setPermittedCases(Set<Cases> permittedCases) {
this.permittedCases = permittedCases;
}
}
2 changes: 1 addition & 1 deletion src/main/java/org/example/crimearchive/reports/Report.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public class Report {
private String s3KeyPdf;
private String s3KeyFile;
@ManyToOne
@JoinColumn(name = "reports")
@JoinColumn(name = "case_id")
private Cases caseEntity;

public Cases getCaseEntity() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package org.example.crimearchive.reports;

import org.example.crimearchive.cases.Cases;
import org.springframework.data.repository.ListCrudRepository;

import java.util.List;
import java.util.UUID;

public interface ReportRepository extends ListCrudRepository<Report, UUID> {
Expand All @@ -14,6 +16,8 @@ public interface ReportRepository extends ListCrudRepository<Report, UUID> {

long count();

// List<Report> findAllByCaseNumberIn(List<String> permittedCaseNumbers);
List<Report> findAllByCaseEntity(Cases caseEntity);

Report getReportByCaseEntity(Cases caseEntity);

Copy link
Copy Markdown
Contributor

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

🏁 Script executed:

#!/bin/bash
# Verify all usages and relationship cardinality before refactor.
rg -nP --type=java -C3 '\bgetReportByCaseEntity\s*\('
rg -nP --type=java -C3 '@OneToMany\(mappedBy\s*=\s*"caseEntity"\)|Set<Report>\s+\w+'

Repository: ithsjava25/project-backend-DurTre

Length of output: 2788


getReportByCaseEntity violates the data model's one-to-many relationship.

The method returns a single Report, but the Cases entity defines @OneToMany(mappedBy = "caseEntity") with Set<Report>. This will throw IncorrectResultSizeDataAccessException at runtime if a case has multiple reports.

Use List<Report> findAllByCaseEntity(Cases caseEntity) (already defined on line 19) and update CaseService.getReports() accordingly. If a single report is required, explicitly document the constraint and implement an ordered query method (e.g., findFirstByCaseEntityOrderBy...).

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

In `@src/main/java/org/example/crimearchive/reports/ReportRepository.java` at line
21, getReportByCaseEntity returns a single Report but the Cases entity models a
one-to-many relationship (Set<Report>), which can cause
IncorrectResultSizeDataAccessException; replace usages of getReportByCaseEntity
with the existing List<Report> findAllByCaseEntity(Cases caseEntity) and update
CaseService.getReports() to consume the list, or if you truly need a single
report, remove getReportByCaseEntity, document the constraint, and implement an
explicit ordered query such as
findFirstByCaseEntityOrderBy<SomeTimestampOrPriority> to deterministically
return one Report.


}
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,18 @@ public AccountInitilizer(UserRepository repo, CaseService caseService, PasswordE
@Override
public void run(String... args) throws Exception {
if(userRepository.count() == 0){
userRepository.save(createAccount("admin", "password", List.of("user", "admin")));
userRepository.save(createAccount("demouser", "password", List.of("user")));
userRepository.save(createAccount("officer", "password", List.of("user")));
userRepository.save(createAccount("admin", "password", List.of("user", "admin"), "Lars Åkesson", "Polischef", "Västra Götaland"));
userRepository.save(createAccount("demouser", "password", List.of("user"), "Nils Jonsson", "Polis", "Skåne"));
userRepository.save(createAccount("officer", "password", List.of("user"), "Jimmy Johansson", "Polis", "Lappland"));
Comment on lines +26 to +28

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid seeding predictable default credentials in runtime startup.

Line 26–28 create known usernames with the same weak password ("password"). If this runs in a non-dev environment, it creates a trivial account-compromise path.

🔧 Suggested hardening
-@Component
+@Component
+@org.springframework.context.annotation.Profile("dev")
 public class AccountInitilizer implements CommandLineRunner {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/example/crimearchive/repository/AccountInitilizer.java`
around lines 26 - 28, The startup seeding in AccountInitilizer (the
userRepository.save calls that call createAccount for "admin", "demouser",
"officer") seeds predictable weak credentials; change it so runtime startup
never seeds fixed passwords: remove or guard these save() calls behind a
development-only check (e.g., Spring profile or boolean property) and/or accept
passwords from secure configuration (env variables/secret manager) or generate
cryptographically-random passwords per-run and emit them only to a secure audit
sink; ensure createAccount still hashes passwords before persisting. Locate the
three userRepository.save(...) calls and wrap them with a dev-only condition or
replace them with a secure-seed mechanism as described.


caseService.addAccountToCase(1L, "K-2026-000001");
caseService.addAccountToCase(1L, "K-2026-000002");
caseService.addAccountToCase(1L, "K-2026-000003");
}
}
private Account createAccount(String username, String rawPassword, List<String> roles){
return new Account(username, passwordEncoder.encode(rawPassword), roles);

private Account createAccount(String username, String rawPassword, List<String> roles, String fullname, String profession, String department) {
return new Account(username, passwordEncoder.encode(rawPassword), roles, fullname, profession, department);
}

}
Empty file.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading