GAP-2547: 500 error when editing user (#197)

* first pass at fixing the error * unit tests * PR feedback
cabinetoffice · Apr 17, 2024 · 24506c8 · 24506c8
1 parent 3e31397
commit 24506c8
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 10 deletions.
diff --git a/src/main/java/gov/cabinetoffice/gapuserservice/repository/UserRepository.java b/src/main/java/gov/cabinetoffice/gapuserservice/repository/UserRepository.java
@@ -32,6 +32,8 @@ public interface UserRepository extends JpaRepository<User, Integer> {
     @EntityGraph(attributePaths = {"department", "roles"})
     Optional<User> findByColaSub(UUID sub);
 
+    List<User> findByColaSubIn(List<UUID> subs);
+
     @EntityGraph(attributePaths = {"department", "roles"})
     Optional<User> findById(int id);
 

diff --git a/src/main/java/gov/cabinetoffice/gapuserservice/service/user/OneLoginUserService.java b/src/main/java/gov/cabinetoffice/gapuserservice/service/user/OneLoginUserService.java
@@ -54,6 +54,7 @@ public class OneLoginUserService {
     private static final String NOT_FOUND = "not found";
     private static final String AUTHORIZATION_HEADER_NAME = "Authorization";
     private static final String BEARER_HEADER_PREFIX = "Bearer ";
+    private static final String ONE_LOGIN_PREFIX = "urn:fdc:gov.uk";
 
     private final AwsEncryptionServiceImpl awsEncryptionService;
 
@@ -474,11 +475,33 @@ public boolean hasEmailChanged(final User user, final OneLoginUserInfoDto userIn
     }
 
     public List<UserEmailDto> getUserEmailsBySubs(List<String> subs) {
-        List<User> users = userRepository.findBySubIn(subs);
-        return users.stream().map(user -> UserEmailDto.builder()
-                .emailAddress(awsEncryptionService.encryptField(user.getEmailAddress()))
-                .sub(user.getSub())
-                .build())
+        final List<String> oneLoginSubs = subs.stream()
+                .filter(sub -> sub.contains(ONE_LOGIN_PREFIX))
+                .toList();
+
+        final List<String> colaSubs = subs.stream()
+                .filter(sub -> !sub.contains(ONE_LOGIN_PREFIX))
+                .toList();
+
+        final List<User> users = new ArrayList<>();
+        if (!oneLoginSubs.isEmpty()) {
+            users.addAll(userRepository.findBySubIn(oneLoginSubs));
+        }
+
+        if (!colaSubs.isEmpty()) {
+            final List<UUID> colaSubUuids = colaSubs.stream()
+                    .map(UUID::fromString)
+                    .toList();
+
+            users.addAll(userRepository.findByColaSubIn(colaSubUuids));
+        }
+
+        return users.stream()
+                .map(user -> UserEmailDto.builder()
+                                .emailAddress(awsEncryptionService.encryptField(user.getEmailAddress()))
+                                .sub(user.getSub())
+                                .build()
+                )
                 .toList();
     }
 

diff --git a/src/test/java/gov/cabinetoffice/gapuserservice/service/user/OneLoginUserServiceTest.java b/src/test/java/gov/cabinetoffice/gapuserservice/service/user/OneLoginUserServiceTest.java
@@ -713,21 +713,52 @@ void createNewUser() {
 
         @Test
         void shouldGetUserEmailsFromSubsAndEncryptThem() {
-            final List<String> subs = List.of("sub1", "sub2");
+            final List<String> subs = List.of("urn:fdc:gov.uk-sub1", "urn:fdc:gov.uk-sub2");
             final List<UserEmailDto> encryptedUserEmailDtos = List.of(
-                    new UserEmailDto("encrypted1".getBytes(), "sub1"),
-                    new UserEmailDto("encrypted2".getBytes(), "sub2")
+                    new UserEmailDto("encrypted1".getBytes(), "urn:fdc:gov.uk-sub1"),
+                    new UserEmailDto("encrypted2".getBytes(), "urn:fdc:gov.uk-sub2")
             );
 
             when(userRepository.findBySubIn(subs)).thenReturn(
                     List.of(
-                            User.builder().sub("sub1").emailAddress("unencrypted1").build(),
-                            User.builder().sub("sub2").emailAddress("unencrypted2").build()
+                            User.builder().sub("urn:fdc:gov.uk-sub1").emailAddress("unencrypted1").build(),
+                            User.builder().sub("urn:fdc:gov.uk-sub2").emailAddress("unencrypted2").build()
                     )
             );
             when(awsEncryptionService.encryptField("unencrypted1")).thenReturn("encrypted1".getBytes());
             when(awsEncryptionService.encryptField("unencrypted2")).thenReturn("encrypted2".getBytes());
+
+            List<UserEmailDto> returnedList = oneLoginUserService.getUserEmailsBySubs(subs);
+
+            assertThat(returnedList).isEqualTo(encryptedUserEmailDtos);
+        }
+
+        @Test
+        void shouldGetUserEmailsFromColaSubsAndEncryptThem() {
+            final String sub1 = "3009e4c2-cc94-4b4d-999f-31e5b394e8ce";
+            final String sub2 = "3009e4c2-cc94-4b4d-999f-31e5b394e8cf";
+
+            final List<String> subs = List.of(sub1, sub2);
+            final List<UUID> subsAsUuids = List.of(
+                    UUID.fromString(sub1),
+                    UUID.fromString(sub2)
+            );
+            final List<UserEmailDto> encryptedUserEmailDtos = List.of(
+                    new UserEmailDto("encrypted1".getBytes(), sub1.toString()),
+                    new UserEmailDto("encrypted2".getBytes(), sub2.toString())
+            );
+
+            when(userRepository.findByColaSubIn(subsAsUuids)).thenReturn(
+                    List.of(
+                            User.builder().sub(sub1).emailAddress("unencrypted1").build(),
+                            User.builder().sub(sub2).emailAddress("unencrypted2").build()
+                    )
+            );
+            when(awsEncryptionService.encryptField("unencrypted1")).thenReturn("encrypted1".getBytes());
+            when(awsEncryptionService.encryptField("unencrypted2")).thenReturn("encrypted2".getBytes());
+
             List<UserEmailDto> returnedList = oneLoginUserService.getUserEmailsBySubs(subs);
+
             assertThat(returnedList).isEqualTo(encryptedUserEmailDtos);
         }