Fix deleteUser

gisaia · May 15, 2024 · bdff8a6 · bdff8a6
1 parent f712788
commit bdff8a6
Show file tree

Hide file tree

Showing 15 changed files with 114 additions and 38 deletions.
diff --git a/arlas-iam-core/pom.xml b/arlas-iam-core/pom.xml
@@ -5,7 +5,7 @@
     <parent>
         <artifactId>arlas-iam-parent</artifactId>
         <groupId>io.arlas.iam</groupId>
-        <version>25.0.0-rc.2</version>
+        <version>25.0.0-rc.3</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 

diff --git a/arlas-iam-core/src/main/java/io/arlas/iam/core/AuthService.java b/arlas-iam-core/src/main/java/io/arlas/iam/core/AuthService.java
@@ -29,7 +29,7 @@ public interface AuthService {
 
     Optional<User> readUser(UUID userId);
     User updateUser(User user, String oldPassword, String newPassword, String firstName, String lastName, String locale, String timezone) throws NonMatchingPasswordException;
-    void deleteUser(UUID userId) throws NotAllowedException;
+    void deleteUser(UUID actingId, UUID targetId) throws NotAllowedException;
 
     Optional<User> activateUser(UUID userId);
     Optional<User> deactivateUser(UUID userId) throws NotAllowedException;

diff --git a/arlas-iam-core/src/main/java/io/arlas/iam/impl/HibernateAuthService.java b/arlas-iam-core/src/main/java/io/arlas/iam/impl/HibernateAuthService.java
@@ -418,11 +418,15 @@ public User updateUser(User user, String oldPassword, String newPassword, String
     }
 
     @Override
-    public void deleteUser(UUID userId) throws NotAllowedException {
-        if (isAdmin(userId)) {
+    public void deleteUser(UUID actingId, UUID targetId) throws NotAllowedException {
+        if (isAdmin(targetId)) {
             throw new NotAllowedException("Admin cannot be removed from database.");
         }
-        readUser(userId).ifPresent(userDao::deleteUser);
+        if (isAdmin(actingId) || actingId.equals(targetId)) {
+            readUser(targetId).ifPresent(userDao::deleteUser);
+        } else {
+            throw new NotAllowedException("User can only be removed by admin or self.");
+        }
         // TODO: delete user resources (organisation, collections...)
     }
 
@@ -799,6 +803,8 @@ public User updateRolesOfUser(User owner, UUID orgId, UUID userId, Set<String> n
                 .map(r -> roleDao.readRole(UUID.fromString(r)).orElseThrow().getName())
                 .anyMatch(n -> n.equals(ROLE_ARLAS_OWNER)));
 
+        userDao.updateUser(user); // update updateDate of user
+
         return getUser(org, userId);
     }
 

diff --git a/arlas-iam-core/src/main/java/io/arlas/iam/model/ApiKey.java b/arlas-iam-core/src/main/java/io/arlas/iam/model/ApiKey.java
@@ -47,7 +47,7 @@ public class ApiKey {
     @JoinColumn(name = "id_org")
     private Organisation org;
 
-    @ManyToMany(mappedBy="apiKeys", cascade= CascadeType.REMOVE)
+    @ManyToMany(mappedBy="apiKeys")
     private Set<Role> roles = new HashSet<>();
 
     public ApiKey() {}
@@ -62,6 +62,13 @@ public ApiKey(String name, String keyId, String keySecret, int ttlInDays, User o
         this.roles = roles;
     }
 
+    @PreRemove
+    private void removeRoleAssociations() {
+        for (Role role : this.roles) {
+            role.getApiKeys().remove(this);
+        }
+    }
+
     public UUID getId() {
         return id;
     }

diff --git a/arlas-iam-core/src/main/java/io/arlas/iam/model/Organisation.java b/arlas-iam-core/src/main/java/io/arlas/iam/model/Organisation.java
@@ -29,13 +29,13 @@ public class Organisation {
     @Column(name="display_name")
     private String displayName;
 
-    @OneToMany(mappedBy = "pk.org", cascade = CascadeType.REMOVE)
+    @OneToMany(mappedBy = "pk.org", cascade = CascadeType.REMOVE, orphanRemoval = true)
     private Set<OrganisationMember> members = new HashSet<>();
 
-    @OneToMany(mappedBy="organisation", cascade = CascadeType.REMOVE)
+    @OneToMany(mappedBy="organisation", cascade = CascadeType.REMOVE, orphanRemoval = true)
     private Set<Permission> permissions = new HashSet<>();
 
-    @OneToMany(mappedBy="organisation", cascade = CascadeType.REMOVE)
+    @OneToMany(mappedBy="organisation", cascade = CascadeType.REMOVE, orphanRemoval = true)
     private Set<Role> roles = new HashSet<>();
 
     private Organisation() {}

diff --git a/arlas-iam-core/src/main/java/io/arlas/iam/model/Permission.java b/arlas-iam-core/src/main/java/io/arlas/iam/model/Permission.java
@@ -28,7 +28,7 @@ public class Permission {
     @Column
     private String description;
 
-    @ManyToMany()
+    @ManyToMany(cascade = {CascadeType.PERSIST, CascadeType.MERGE})
     @JoinTable(name = "RolePermission",
             joinColumns = @JoinColumn(name = "id_permission"),
             inverseJoinColumns = @JoinColumn(name = "id_role"))
@@ -84,6 +84,16 @@ public void setRoles(Set<Role> roles) {
         this.roles = roles;
     }
 
+    public void removeRole(Role role) {
+        this.roles.remove(role);
+        role.getPermissions().remove(this);
+    }
+
+    public void addRole(Role role) {
+        this.roles.add(role);
+        role.getPermissions().add(this);
+    }
+
     public Organisation getOrganisation() {
         return organisation;
     }

diff --git a/arlas-iam-core/src/main/java/io/arlas/iam/model/Role.java b/arlas-iam-core/src/main/java/io/arlas/iam/model/Role.java
@@ -42,19 +42,19 @@ public class Role {
     @JoinColumn(name = "id_organisation")
     private Organisation organisation;
 
-    @ManyToMany()
+    @ManyToMany(cascade = {CascadeType.PERSIST, CascadeType.MERGE})
     @JoinTable(name = "UserRole",
             joinColumns = @JoinColumn(name = "id_role"),
             inverseJoinColumns = @JoinColumn(name = "id_user"))
     private Set<User> users = new HashSet<>();
 
-    @ManyToMany()
+    @ManyToMany(cascade = {CascadeType.PERSIST, CascadeType.MERGE})
     @JoinTable(name = "ApiKeyRole",
             joinColumns = @JoinColumn(name = "id_role"),
             inverseJoinColumns = @JoinColumn(name = "id_apikey"))
     private Set<ApiKey> apiKeys = new HashSet<>();
 
-    @ManyToMany(mappedBy="roles", cascade= CascadeType.REMOVE)
+    @ManyToMany(mappedBy="roles")
     private Set<Permission> permissions = new HashSet<>();
 
     private Role() {}
@@ -74,6 +74,13 @@ public Role(String name, boolean isSystem) {
         this.isSystem = isSystem;
     }
 
+    @PreRemove
+    private void removePermissionAssociations() {
+        for (Permission permission : this.permissions) {
+            permission.getRoles().remove(this);
+        }
+    }
+
     public UUID getId() {
         return id;
     }
@@ -142,15 +149,32 @@ public Role setUsers(Set<User> users) {
         return this;
     }
 
+    public void removeUser(User user) {
+        this.users.remove(user);
+        user.getRoles().remove(this);
+    }
+
+    public void addUser(User user) {
+        this.users.add(user);
+        user.getRoles().add(this);
+    }
+
     public Set<ApiKey> getApiKeys() {
         return apiKeys;
     }
 
     public void setApiKeys(Set<ApiKey> apiKeys) {
         this.apiKeys = apiKeys;
     }
+
+    public void removeApiKey(ApiKey apiKey) {
+        this.apiKeys.remove(apiKey);
+        apiKey.getRoles().remove(this);
+    }
+
     public void addApiKeys(ApiKey apiKey) {
         this.apiKeys.add(apiKey);
+        apiKey.getRoles().add(this);
     }
 
     public Set<Permission> getPermissions() {

diff --git a/arlas-iam-core/src/main/java/io/arlas/iam/model/User.java b/arlas-iam-core/src/main/java/io/arlas/iam/model/User.java
@@ -7,6 +7,7 @@
 import org.hibernate.annotations.NaturalId;
 
 import jakarta.persistence.*;
+
 import java.time.LocalDateTime;
 import java.time.ZoneOffset;
 import java.util.*;
@@ -61,7 +62,7 @@ public class User {
     @OneToMany(mappedBy = "pk.user", cascade= CascadeType.REMOVE)
     private Set<OrganisationMember> organisations = new HashSet<>();
 
-    @ManyToMany(mappedBy="users", cascade= CascadeType.REMOVE)
+    @ManyToMany(mappedBy="users")
     private Set<Role> roles = new HashSet<>();
 
     @OneToMany(mappedBy="owner", cascade = CascadeType.REMOVE)
@@ -73,6 +74,13 @@ public User(String email) {
         this.email = email;
     }
 
+    @PreRemove
+    private void removeRoleAssociations() {
+        for (Role role : this.roles) {
+            role.getUsers().remove(this);
+        }
+    }
+
     public UUID getId() {
         return this.id;
     }

diff --git a/arlas-iam-rest/pom.xml b/arlas-iam-rest/pom.xml
@@ -5,7 +5,7 @@
     <parent>
         <artifactId>arlas-iam-parent</artifactId>
         <groupId>io.arlas.iam</groupId>
-        <version>25.0.0-rc.2</version>
+        <version>25.0.0-rc.3</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 
@@ -21,7 +21,7 @@
         <dependency>
             <groupId>io.arlas.iam</groupId>
             <artifactId>arlas-iam-core</artifactId>
-            <version>25.0.0-rc.2</version>
+            <version>25.0.0-rc.3</version>
             <exclusions>
                 <exclusion>
                     <artifactId>jakarta.servlet-api</artifactId>

diff --git a/arlas-iam-rest/src/main/java/io/arlas/iam/rest/service/IAMRestService.java b/arlas-iam-rest/src/main/java/io/arlas/iam/rest/service/IAMRestService.java
@@ -599,7 +599,7 @@ public Response deactivateUser(
     @Consumes(UTF8JSON)
     @Operation(
             security = @SecurityRequirement(name = "JWT"),
-            summary = "Delete the logged in user"
+            summary = "Delete the given user"
     )
     @ApiResponses(value = {
             @ApiResponse(responseCode = "202", description = "Successful operation",
@@ -616,8 +616,7 @@ public Response deleteUser(
             @Parameter(name = "id", required = true)
             @PathParam(value = "id") String id
     ) throws NotFoundException, NotAllowedException {
-        checkLoggedInUser(headers, id);
-        authService.deleteUser(UUID.fromString(id));
+        authService.deleteUser(getUser(headers).getId(), UUID.fromString(id));
         logUAM(request, headers,  "users", "delete-user-account");
         return Response.accepted(uriInfo.getRequestUriBuilder().build())
                 .entity(new ArlasMessage("User deleted."))

diff --git a/arlas-iam-server/pom.xml b/arlas-iam-server/pom.xml
@@ -5,7 +5,7 @@
     <parent>
         <artifactId>arlas-iam-parent</artifactId>
         <groupId>io.arlas.iam</groupId>
-        <version>25.0.0-rc.2</version>
+        <version>25.0.0-rc.3</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 
@@ -15,7 +15,7 @@
         <dependency>
             <groupId>io.arlas.iam</groupId>
             <artifactId>arlas-iam-rest</artifactId>
-            <version>25.0.0-rc.2</version>
+            <version>25.0.0-rc.3</version>
         </dependency>
     </dependencies>
 

diff --git a/arlas-iam-tests/pom.xml b/arlas-iam-tests/pom.xml
@@ -5,7 +5,7 @@
     <parent>
         <artifactId>arlas-iam-parent</artifactId>
         <groupId>io.arlas.iam</groupId>
-        <version>25.0.0-rc.2</version>
+        <version>25.0.0-rc.3</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 

diff --git a/arlas-iam-tests/src/test/java/io/arlas/iam/test/AuthEndpoints.java b/arlas-iam-tests/src/test/java/io/arlas/iam/test/AuthEndpoints.java
@@ -326,7 +326,7 @@ protected Response deleteUserFromRole(String actingId, String uid, String rid) {
                 .delete(arlasAppPath.concat("organisations/{oid}/users/{uid}/roles/{rid}"));
     }
 
-    protected Response listUsers(String userId, String rname) {
+    protected Response listUsersOfOrganisation(String userId, String rname) {
         RequestSpecification req = given()
                 .header(AUTH_HEADER, getToken(userId))
                 .header(ARLAS_ORG_FILTER, ORG)

diff --git a/arlas-iam-tests/src/test/java/io/arlas/iam/test/AuthITUser.java b/arlas-iam-tests/src/test/java/io/arlas/iam/test/AuthITUser.java
@@ -170,7 +170,7 @@ public void test039ListUsersOfDomain() {
 
     @Test
     public void test040ListUsers() {
-        listUsers(userId1, null).then().statusCode(200)
+        listUsersOfOrganisation(userId1, null).then().statusCode(200)
                 .body("", hasSize(1))
                 .body("[0].member.email", is(USER1));
     }
@@ -180,7 +180,7 @@ public void test041AddUserToOrganisation() {
         addUserToOrganisation(userId1, USER2).then().statusCode(201);
         getUser(userId2, userId2).then().statusCode(200)
                 .body("organisations", hasSize(2));
-        listUsers(userId1, null).then().statusCode(200)
+        listUsersOfOrganisation(userId1, null).then().statusCode(200)
                 .body("", hasSize(2));
     }
 
@@ -229,7 +229,7 @@ public void test053AddUserInRole() {
 
     @Test
     public void test054ListUsersWithRole() {
-        listUsers(userId1, ROLE1).then().statusCode(200)
+        listUsersOfOrganisation(userId1, ROLE1).then().statusCode(200)
                 .body("", hasSize(1))
                 .body("[0].member.email", is(USER2));
     }
@@ -454,6 +454,8 @@ public void test093UseApiKeyForUnauthorizedEndpoint() {
     @Test
     public void test094DeleteApiKey() {
         deleteApiKey(userId1, apiKeyUUID).then().statusCode(202);
+        // check that associated role is not removed:
+        listPermissionsOfRole(userId1, fooRoleId1).then().statusCode(200);
     }
 
     // TODO: needs collections to be existing in ARLAS server to test these
@@ -485,39 +487,59 @@ public void test900DeleteUserFromRole() {
         deleteUserFromRole(userId1, userId2, fooRoleId1).then().statusCode(202);
         getUser(userId2, userId2).then().statusCode(200)
                 .body("roles", hasSize(8));
+        // check that associated role is not removed:
+        listPermissionsOfRole(userId1, fooRoleId1).then().statusCode(200);
     }
 
     @Test
     public void test901DeleteUserFromOrganisation() {
         deleteUserFromOrganisation(userId1, userId2).then().statusCode(202);
         getUser(userId2, userId2).then().statusCode(200)
                 .body("organisations", hasSize(1));
+        // check that org is not deleted
+        listOrgRoles().then().statusCode(200);
+        // check that user is not deleted
+        getUser(userId2, userId2).then().statusCode(200);
     }
 
     @Test
-    public void test902DeleteOrganisationAsOwner() {
-        deleteOrganisation(userId1).then().statusCode(202);
-        getUser(userId1).then().statusCode(200).body("organisations", hasSize(1));
+    public void test902DeactivateUser() {
+        deactivateUser(userId1, userId2).then().statusCode(202);
     }
 
     @Test
-    public void test903DeleteUserNotSelf() {
-        deleteUser(userId1, userId2).then().statusCode(404);
+    public void test903ActivateUser() {
+        activateUser(userId1, userId2).then().statusCode(202);
     }
 
     @Test
-    public void test904DeactivateUser() {
-        deactivateUser(userId1, userId2).then().statusCode(202);
+    public void test904DeleteUserNotSelf() {
+        deleteUser(userId1, userId2).then().statusCode(400);
     }
 
     @Test
-    public void test905ActivateUser() {
-        activateUser(userId1, userId2).then().statusCode(202);
+    public void test905DeleteUserSelf() {
+        addUserToOrganisation(userId1, USER2).then().statusCode(201);
+        listUsersOfOrganisation(userId1, null).then().statusCode(200)
+                .body("", hasSize(2));
+        deleteUser(userId2, userId2).then().statusCode(202);
+        // check that org is not deleted
+        listUsersOfOrganisation(userId1, null).then().statusCode(200)
+                .body("", hasSize(1));
+        // check that roles are not deleted
+        listOrgRoles().then().statusCode(200)
+                .body("", hasSize(6));
+
     }
 
     @Test
-    public void test906DeleteUserSelf() {
+    public void test906DeleteOrganisationAsOwner() {
+        deleteOrganisation(userId1).then().statusCode(202);
+        getUser(userId1).then().statusCode(200).body("organisations", hasSize(1));
+    }
+
+    @Test
+    public void test907DeleteUserSelf() {
         deleteUser(userId1, userId1).then().statusCode(202);
-        deleteUser(userId2, userId2).then().statusCode(202);
     }
 }