Skip to content

Adds the security layer to the VisaViewController, and updates tests. #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Martin-E-Karlsson merged 1 commit into from
Apr 20, 2026
Merged

Adds the security layer to the VisaViewController, and updates tests. #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions src/main/java/org/example/visacasemanagementsystem/ApplicationViewController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@

@Controller
public class ApplicationViewController {

@GetMapping("/")
public String index() {
return "redirect:/dashboard";
}

@GetMapping("/dashboard")
public String dashboard(@AuthenticationPrincipal UserPrincipal principal) {
if (principal == null) {
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/org/example/visacasemanagementsystem/config/SecurityConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,16 @@ public SecurityConfig(UserDetailsService userDetailsService) {
public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
http
.authorizeHttpRequests(auth -> auth
.requestMatchers("/").permitAll()
.requestMatchers("/user/signup").permitAll()
.requestMatchers("/user/login").permitAll()
.requestMatchers("/logout").permitAll()
.requestMatchers("/dashboard").authenticated()
.requestMatchers("/profile/**").authenticated()
.requestMatchers("/visas/**").authenticated()
.requestMatchers("/api/comments/**").authenticated()

@coderabbitai coderabbitai Bot Apr 20, 2026

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

Don’t loosen the comments API before fixing comment authorization.

/api/comments/** is now available to any authenticated user, but CommentController has no role/access checks and the existing comment creation path still trusts CreateCommentDTO.authorId(). Until comments resolve author and visa access from the principal, keep this endpoint stricter or add controller/service-level authorization.

🛡️ Conservative fix until comment auth is implemented 
-                        .requestMatchers("/api/comments/**").authenticated()
+                        .requestMatchers("/api/comments/**").hasRole("SYSADMIN")

Based on learnings, CreateCommentDTO.authorId() is currently trusted directly and should be replaced with the authenticated principal once Spring Security is integrated.

📝 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
.requestMatchers("/api/comments/**").authenticated()
.requestMatchers("/api/comments/**").hasRole("SYSADMIN")
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/visacasemanagementsystem/config/SecurityConfig.java`
at line 39, The security rule allowing any authenticated user for
"/api/comments/**" is too permissive; tighten it in SecurityConfig by replacing
.requestMatchers("/api/comments/**").authenticated() with a role-restricted
matcher such as .requestMatchers("/api/comments/**").hasRole("USER") (or another
appropriate authority used by your app) so only intended roles can access
comment endpoints, and then update CommentController (and the service it calls,
e.g., createComment) to stop trusting CreateCommentDTO.authorId(): obtain the
author from the authenticated principal (via `@AuthenticationPrincipal` or
SecurityContextHolder) and enforce visa-access checks there before creating or
returning comments.

.requestMatchers("/**/admin").hasRole("ADMIN")
.requestMatchers("/**/applicant").hasRole("USER")
//TODO: requestMatchers for /**/{userId} endpoints, etc.
.anyRequest().hasRole("SYSADMIN")
)
.headers(headers -> headers.frameOptions(HeadersConfigurer.FrameOptionsConfig::sameOrigin))
Expand Down
14 changes: 10 additions & 4 deletions src/main/java/org/example/visacasemanagementsystem/user/controller/UserViewController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.example.visacasemanagementsystem.user.service.UserService;
import org.example.visacasemanagementsystem.visa.dto.VisaDTO;
import org.example.visacasemanagementsystem.visa.service.VisaService;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.stereotype.Controller;
import org.springframework.ui.Model;
Expand All @@ -21,6 +22,7 @@
import java.util.List;
import java.util.Objects;

@PreAuthorize("isAuthenticated()")
@Controller
public class UserViewController {
private final VisaService visaService;
Expand All @@ -36,6 +38,7 @@ public UserViewController(VisaService visaService,
}

// Signup form for new users, accessible to all
@PreAuthorize("permitAll()")
@GetMapping("/user/signup")
public String userSignupForm(Model model) {
return "user/signup";
Expand All @@ -44,6 +47,7 @@ public String userSignupForm(Model model) {
// Posting a successful user creation then redirecting to the applicant dashboard since only applicants can be
// created through the signup process. Admins or Sysadmins are created from applicant users by given authorization
// from a sysadmin.
@PreAuthorize("permitAll()")
@PostMapping("/user/signup")
public String createUser(@RequestParam String fullName,
@RequestParam String email,
Expand All @@ -62,6 +66,7 @@ public String createUser(@RequestParam String fullName,
}

// Login page
@PreAuthorize("permitAll()")
@GetMapping("/user/login")
public String userLoginForm(){
return "user/login";
Expand Down Expand Up @@ -122,16 +127,17 @@ public String updateProfile(@AuthenticationPrincipal UserPrincipal principal,
}

// A list view of users only available to sysadmins
@PreAuthorize("hasRole('SYSADMIN')")
@GetMapping("/user/list")
public String userListView(@AuthenticationPrincipal UserPrincipal principal,
Model model) {
userService.validateSysAdmin(principal);
List<UserDTO> allUsers = userService.findAll();
model.addAttribute("name", principal.getFullName());
model.addAttribute("users", allUsers);
return "user/list";
}

@PreAuthorize("hasRole('USER')")
@GetMapping("/dashboard/applicant")
public String applicantDashboard(@AuthenticationPrincipal UserPrincipal principal,
Model model) {
Expand All @@ -141,22 +147,22 @@ public String applicantDashboard(@AuthenticationPrincipal UserPrincipal principa
return "dashboard/applicant";
}

@PreAuthorize("hasRole('ADMIN')")
@GetMapping("/dashboard/admin")
public String adminDashboard(@AuthenticationPrincipal UserPrincipal principal,
Model model) {
userService.validateAdmin(principal);
List<VisaDTO> assignedCases = visaService.findVisasByHandlerId(principal.getUserId());
List<VisaDTO> unassignedCases = visaService.findVisaByStatus("UNASSIGNED");
List<VisaDTO> unassignedCases = visaService.findVisaByStatus("SUBMITTED");
model.addAttribute("name", principal.getFullName());
model.addAttribute("assignedCases", assignedCases);
model.addAttribute("unassignedCases", unassignedCases);
return "dashboard/admin";
}

@PreAuthorize("hasRole('SYSADMIN')")
@GetMapping("/dashboard/sysadmin")
public String sysAdminDashboard(@AuthenticationPrincipal UserPrincipal principal,
Model model) {
userService.validateSysAdmin(principal);
List<UserDTO> allUsers = userService.findAll();
List<AuditDTO> recentLogs = auditService.findAll();
model.addAttribute("name", principal.getFullName());
Expand Down
39 changes: 5 additions & 34 deletions src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.example.visacasemanagementsystem.user.repository.UserRepository;
import org.example.visacasemanagementsystem.user.security.UserPrincipal;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
Expand Down Expand Up @@ -81,10 +82,9 @@ public UserDTO updateUser(UpdateUserDTO dto) {
}
}

@PreAuthorize("hasRole('SYSADMIN')")
@Transactional
public UserDTO updateUserAuthorization(Long userId, UserAuthorization newAuth, Long requesterId) {
validateSysAdmin(requesterId); // confirm requester is SYSADMIN

public UserDTO updateUserAuthorization(Long userId, UserAuthorization newAuth) {
User user = userRepository.findById(userId)
.orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND));

Expand All @@ -93,23 +93,14 @@ public UserDTO updateUserAuthorization(Long userId, UserAuthorization newAuth, L
return userMapper.toDTO(savedUser);
}

@PreAuthorize("hasRole('SYSADMIN')")
@Transactional
public void deleteUser(Long id, Long requesterId) {
validateSysAdmin(requesterId); // confirm requester is SYSADMIN
public void deleteUser(Long id) {
User user = userRepository.findById(id)
.orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND));
userRepository.delete(user);
}

private void validateSysAdmin(Long requesterId) {
User requester = userRepository.findById(requesterId)
.orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND));

if (requester.getUserAuthorization() != UserAuthorization.SYSADMIN) {
throw new UnauthorizedException("User is not authorized to perform this action.");
}
}

public void validateProfileAccess(UserPrincipal principal, Long userId) {
boolean isOwnProfile = principal.getUserId().equals(userId);
boolean isSysAdmin = principal.getAuthorities().stream()
Expand All @@ -119,24 +110,4 @@ public void validateProfileAccess(UserPrincipal principal, Long userId) {
throw new UnauthorizedException("You do not have permission to edit this profile.");
}
}

public void validateSysAdmin(UserPrincipal principal) {
boolean isSysAdmin = principal.getAuthorities().stream()
.anyMatch(a -> Objects.equals(a.getAuthority(), "ROLE_SYSADMIN"));

if (!isSysAdmin) {
throw new UnauthorizedException("Only system administrators can access this page.");
}
}

public void validateAdmin(UserPrincipal principal) {
boolean isSysAdmin = principal.getAuthorities().stream()
.anyMatch(a -> Objects.equals(a.getAuthority(), "ROLE_SYSADMIN"));
boolean isAdmin = principal.getAuthorities().stream()
.anyMatch(a -> Objects.equals(a.getAuthority(), "ROLE_ADMIN"));

if (!isSysAdmin && !isAdmin) {
throw new UnauthorizedException("Only administrators or system administrators can access this page.");
}
}
}
Loading