Merge pull request #28 from akaene/main

Prevent accidental update of user types when referencing current user
kbss-cvut · Dec 6, 2023 · bff6079 · bff6079
2 parents f141fbd + 66bc367
commit bff6079
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 7 deletions.
diff --git a/src/main/java/cz/cvut/kbss/study/model/User.java b/src/main/java/cz/cvut/kbss/study/model/User.java
@@ -149,13 +149,21 @@ public void addType(String type) {
         getTypes().add(type);
     }
 
-    public String getToken() { return token; }
+    public String getToken() {
+        return token;
+    }
 
-    public void setToken(String token) { this.token = token; }
+    public void setToken(String token) {
+        this.token = token;
+    }
 
-    public Boolean getIsInvited() { return isInvited; }
+    public Boolean getIsInvited() {
+        return isInvited;
+    }
 
-    public void setIsInvited(Boolean isInvited) { this.isInvited = isInvited; }
+    public void setIsInvited(Boolean isInvited) {
+        this.isInvited = isInvited;
+    }
 
     /**
      * Encodes password of this person.
@@ -178,6 +186,27 @@ public void erasePassword() {
         this.password = null;
     }
 
+    /**
+     * Creates a copy of this instance.
+     *
+     * @return New user instance
+     */
+    public User copy() {
+        final User copy = new User();
+        copy.setUri(uri);
+        copy.setFirstName(firstName);
+        copy.setLastName(lastName);
+        copy.setUsername(username);
+        copy.setEmailAddress(emailAddress);
+        copy.setPassword(password);
+        copy.setDateCreated(dateCreated);
+        copy.setInstitution(institution);
+        copy.setIsInvited(isInvited);
+        copy.setToken(token);
+        types.forEach(copy::addType);
+        return copy;
+    }
+
     @Override
     public void generateUri() {
         if (uri != null) {
@@ -191,8 +220,9 @@ public void generateUri() {
         }
         try {
             this.uri = URI.create(Constants.BASE_URI +
-                    URLEncoder.encode(firstName + "-" + lastName + "-" + IdentificationUtils.generateRandomURINumber(),
-                    StandardCharsets.UTF_8.toString()));
+                                          URLEncoder.encode(
+                                                  firstName + "-" + lastName + "-" + IdentificationUtils.generateRandomURINumber(),
+                                                  StandardCharsets.UTF_8.toString()));
         } catch (UnsupportedEncodingException e) {
             throw new IllegalStateException("Cannot generate Person URI due to unsupported encoding.", e);
         }

diff --git a/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java b/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java
@@ -73,7 +73,7 @@ public User getCurrentUser() {
             return resolveAccountFromOAuthPrincipal((Jwt) principal);
         } else {
             final String username = context.getAuthentication().getName();
-            final User user = userDao.findByUsername(username);
+            final User user = userDao.findByUsername(username).copy();
             if (context.getAuthentication().getAuthorities().stream().anyMatch(a -> a.getAuthority().equals(
                     SwitchUserWebFilter.ROLE_PREVIOUS_ADMINISTRATOR))) {
                 user.addType(Vocabulary.s_c_impersonator);

diff --git a/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java b/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java
@@ -37,6 +37,7 @@
 import static org.hamcrest.Matchers.hasItem;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.when;
 
@@ -198,4 +199,16 @@ void getCurrentUserEnhancesRetrievedUserWithImpersonatorTypeWhenItHasSwitchAutho
         assertEquals(user, result);
         assertThat(result.getTypes(), hasItem(Vocabulary.s_c_impersonator));
     }
+
+    @Test
+    void getCurrentUserReturnsCopyOfInstanceRetrievedFromRepository() {
+        final UserDetails userDetails =
+                new UserDetails(user, Set.of(new SimpleGrantedAuthority(SwitchUserFilter.ROLE_PREVIOUS_ADMINISTRATOR)));
+        SecurityUtils.setCurrentUser(userDetails);
+        when(userDao.findByUsername(user.getUsername())).thenReturn(user);
+        final User result = sut.getCurrentUser();
+
+        assertNotSame(user, result);
+        assertEquals(user, result);
+    }
 }