From 0dd0e0c71e0ed179c43cff5dfe27af38b9008e99 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com.> Date: Wed, 6 Aug 2025 16:35:09 +0100 Subject: [PATCH] formatting and fixes --- .../security/InitialSecuritySetup.java | 39 +++++++++++-------- .../security/config/AccountWebController.java | 8 ++-- .../controller/api/OrgAdminController.java | 3 +- .../api/OrganizationController.java | 3 +- .../controller/api/TeamController.java | 9 +++-- .../controller/api/TeamLeadController.java | 20 ---------- .../database/repository/UserRepository.java | 7 +--- .../security/service/UserService.java | 2 +- .../security/service/TeamServiceTest.java | 25 ++++++------ 9 files changed, 50 insertions(+), 66 deletions(-) diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/InitialSecuritySetup.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/InitialSecuritySetup.java index 3066e3ae3..2b303c754 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/InitialSecuritySetup.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/InitialSecuritySetup.java @@ -221,42 +221,49 @@ public class InitialSecuritySetup { private void migrateDeprecatedRolesToUser() { String[] deprecatedRoles = { - "ROLE_WEB_ONLY_USER", - "ROLE_EXTRA_LIMITED_API_USER", - "ROLE_LIMITED_API_USER" + "ROLE_WEB_ONLY_USER", "ROLE_EXTRA_LIMITED_API_USER", "ROLE_LIMITED_API_USER" }; int totalMigrated = 0; - + for (String deprecatedRole : deprecatedRoles) { List usersWithDeprecatedRole = userService.findByRole(deprecatedRole); - + if (!usersWithDeprecatedRole.isEmpty()) { - log.info("Found {} users with role {}. Converting to USER...", - usersWithDeprecatedRole.size(), deprecatedRole); - + log.info( + "Found {} users with role {}. Converting to USER...", + usersWithDeprecatedRole.size(), + deprecatedRole); + int migratedCount = 0; for (User user : usersWithDeprecatedRole) { try { user.setUserRole(Role.USER); userService.saveUser(user); - log.debug("Converted user '{}' from {} to USER", - user.getUsername(), deprecatedRole); + log.debug( + "Converted user '{}' from {} to USER", + user.getUsername(), + deprecatedRole); migratedCount++; } catch (Exception e) { - log.error("Failed to migrate user '{}' from {} to USER: {}", - user.getUsername(), deprecatedRole, e.getMessage()); + log.error( + "Failed to migrate user '{}' from {} to USER: {}", + user.getUsername(), + deprecatedRole, + e.getMessage()); } } - + if (migratedCount > 0) { - log.info("Successfully migrated {} users from {} to USER", - migratedCount, deprecatedRole); + log.info( + "Successfully migrated {} users from {} to USER", + migratedCount, + deprecatedRole); totalMigrated += migratedCount; } } } - + if (totalMigrated == 0) { log.debug("No users with deprecated roles found - migration not needed"); } else { diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/config/AccountWebController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/config/AccountWebController.java index 0b3351e9b..62d8626bc 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/config/AccountWebController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/config/AccountWebController.java @@ -220,15 +220,13 @@ public class AccountWebController { List allUsers = userRepository.findAllWithTeam(); Iterator iterator = allUsers.iterator(); Map roleDetails = Role.getAllRoleDetails(); - + // Filter role details to only show SYSTEM_ADMIN, USER, and DEMO_USER in UI Map filteredRoleDetails = new LinkedHashMap<>(); String[] allowedRoles = { - Role.SYSTEM_ADMIN.getRoleId(), - Role.USER.getRoleId(), - Role.DEMO_USER.getRoleId() + Role.SYSTEM_ADMIN.getRoleId(), Role.USER.getRoleId(), Role.DEMO_USER.getRoleId() }; - + for (String roleId : allowedRoles) { if (roleDetails.containsKey(roleId)) { filteredRoleDetails.put(roleId, roleDetails.get(roleId)); diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/OrgAdminController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/OrgAdminController.java index 5f6117dd7..f10e41671 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/OrgAdminController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/OrgAdminController.java @@ -30,7 +30,8 @@ import stirling.software.proprietary.security.service.RoleBasedAuthorizationServ @Slf4j @RequiredArgsConstructor @PremiumEndpoint -@PreAuthorize("@roleBasedAuthorizationService.canManageOrgUsers() or @roleBasedAuthorizationService.canManageOrgTeams()") +@PreAuthorize( + "@roleBasedAuthorizationService.canManageOrgUsers() or @roleBasedAuthorizationService.canManageOrgTeams()") public class OrgAdminController { private final TeamRepository teamRepository; diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/OrganizationController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/OrganizationController.java index 7c94d55be..bcef46d0a 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/OrganizationController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/OrganizationController.java @@ -34,7 +34,8 @@ public class OrganizationController { } @GetMapping("/{id}") - @PreAuthorize("@roleBasedAuthorizationService.canViewOrganization(@organizationRepository.findById(#id).orElse(null))") + @PreAuthorize( + "@roleBasedAuthorizationService.canViewOrganization(@organizationRepository.findById(#id).orElse(null))") public ResponseEntity getOrganization(@PathVariable Long id) { Optional organizationOpt = organizationRepository.findById(id); if (organizationOpt.isEmpty()) { diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/TeamController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/TeamController.java index b77d49c5b..9c81af105 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/TeamController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/TeamController.java @@ -62,7 +62,8 @@ public class TeamController { } @PostMapping("/rename") - @PreAuthorize("@roleBasedAuthorizationService.canManageTeam(@teamRepository.findById(#teamId).orElse(null))") + @PreAuthorize( + "@roleBasedAuthorizationService.canManageTeam(@teamRepository.findById(#teamId).orElse(null))") public RedirectView renameTeam( @RequestParam("teamId") Long teamId, @RequestParam("newName") String newName) { Optional existing = teamRepository.findById(teamId); @@ -88,7 +89,8 @@ public class TeamController { @PostMapping("/delete") @Transactional - @PreAuthorize("@roleBasedAuthorizationService.canManageTeam(@teamRepository.findById(#teamId).orElse(null))") + @PreAuthorize( + "@roleBasedAuthorizationService.canManageTeam(@teamRepository.findById(#teamId).orElse(null))") public RedirectView deleteTeam(@RequestParam("teamId") Long teamId) { Optional teamOpt = teamRepository.findById(teamId); if (teamOpt.isEmpty()) { @@ -113,7 +115,8 @@ public class TeamController { @PostMapping("/addUser") @Transactional - @PreAuthorize("@roleBasedAuthorizationService.canAddUserToTeam(#userId, @teamRepository.findById(#teamId).orElse(null))") + @PreAuthorize( + "@roleBasedAuthorizationService.canAddUserToTeam(#userId, @teamRepository.findById(#teamId).orElse(null))") public RedirectView addUserToTeam( @RequestParam("teamId") Long teamId, @RequestParam("userId") Long userId) { diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/TeamLeadController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/TeamLeadController.java index 8f4c90dc9..7d2b51269 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/TeamLeadController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/TeamLeadController.java @@ -116,26 +116,6 @@ public class TeamLeadController { return ResponseEntity.ok().body("User removed from team successfully"); } - /** Get users that can be added to the team (within same organization, not in any team) */ - @GetMapping("/available-users") - public ResponseEntity> getAvailableUsers() { - if (!authorizationService.canManageTeamUsers()) { - return ResponseEntity.status(HttpStatus.FORBIDDEN).build(); - } - - User currentUser = authorizationService.getCurrentUser(); - if (currentUser == null || currentUser.getOrganization() == null) { - return ResponseEntity.badRequest().build(); - } - - // Find users in the same organization who are not in any team - List availableUsers = - userRepository.findUsersInOrganizationWithoutTeam( - currentUser.getOrganization().getId()); - - return ResponseEntity.ok(availableUsers); - } - /** Update a team member's role (team leads can only assign USER role) */ @PostMapping("/update-member-role") @Transactional diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/database/repository/UserRepository.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/database/repository/UserRepository.java index d529bcab5..3f96f3dbc 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/database/repository/UserRepository.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/database/repository/UserRepository.java @@ -24,9 +24,6 @@ public interface UserRepository extends JpaRepository { List findByAuthenticationTypeIgnoreCase(String authenticationType); - @Query("SELECT u FROM User u WHERE u.team IS NULL") - List findAllWithoutTeam(); - @Query(value = "SELECT u FROM User u LEFT JOIN FETCH u.team") List findAllWithTeam(); @@ -38,8 +35,8 @@ public interface UserRepository extends JpaRepository { List findByTeam(Team team); - @Query("SELECT u FROM User u WHERE u.team IS NULL AND u.organization.id = :organizationId") - List findUsersInOrganizationWithoutTeam(@Param("organizationId") Long organizationId); + @Query("SELECT u FROM User u WHERE u.team IS NULL") + List findUsersWithoutTeam(); @Query("SELECT u FROM User u JOIN u.authorities a WHERE a.authority = :role") List findByRole(@Param("role") String role); diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/UserService.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/UserService.java index 0bae1c7b2..f39cd684e 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/UserService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/UserService.java @@ -624,7 +624,7 @@ public class UserService implements UserServiceInterface { } public List getUsersWithoutTeam() { - return userRepository.findAllWithoutTeam(); + return userRepository.findUsersWithoutTeam(); } public void saveAll(List users) { diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/security/service/TeamServiceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/security/service/TeamServiceTest.java index fff1d20c1..c06282de8 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/security/service/TeamServiceTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/security/service/TeamServiceTest.java @@ -11,25 +11,19 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; + import stirling.software.proprietary.model.Organization; import stirling.software.proprietary.model.Team; import stirling.software.proprietary.security.repository.TeamRepository; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyLong; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class TeamServiceTest { @Mock private TeamRepository teamRepository; - @Mock - private OrganizationService organizationService; + @Mock private OrganizationService organizationService; - @InjectMocks - private TeamService teamService; + @InjectMocks private TeamService teamService; @Test void getDefaultTeam() { @@ -42,7 +36,8 @@ class TeamServiceTest { team.setOrganization(organization); when(organizationService.getOrCreateDefaultOrganization()).thenReturn(organization); - when(teamRepository.findByNameAndOrganizationId(TeamService.DEFAULT_TEAM_NAME, organization.getId())) + when(teamRepository.findByNameAndOrganizationId( + TeamService.DEFAULT_TEAM_NAME, organization.getId())) .thenReturn(Optional.of(team)); Team result = teamService.getOrCreateDefaultTeam(); @@ -83,8 +78,9 @@ class TeamServiceTest { team.setOrganization(organization); when(organizationService.getOrCreateInternalOrganization()).thenReturn(organization); - when(teamRepository.findByNameAndOrganizationId(TeamService.INTERNAL_TEAM_NAME, organization.getId())) - .thenReturn(Optional.of(team)); + when(teamRepository.findByNameAndOrganizationId( + TeamService.INTERNAL_TEAM_NAME, organization.getId())) + .thenReturn(Optional.of(team)); Team result = teamService.getOrCreateInternalTeam(); @@ -104,8 +100,9 @@ class TeamServiceTest { internalTeam.setOrganization(organization); when(organizationService.getOrCreateInternalOrganization()).thenReturn(organization); - when(teamRepository.findByNameAndOrganizationId(TeamService.INTERNAL_TEAM_NAME, organization.getId())) - .thenReturn(Optional.empty()); + when(teamRepository.findByNameAndOrganizationId( + TeamService.INTERNAL_TEAM_NAME, organization.getId())) + .thenReturn(Optional.empty()); when(teamRepository.save(any(Team.class))).thenReturn(internalTeam); Team result = teamService.getOrCreateInternalTeam();