From 4c7099b7d2d369da8d1e5c18f0b402d09ab60f26 Mon Sep 17 00:00:00 2001
From: Garth <244253+xgp@users.noreply.github.com>
Date: Thu, 7 Mar 2024 18:05:57 +0100
Subject: [PATCH] check for hasRole before grantRole. also delete the mapping
 entity when revoking, which was the cause of #196. (#197)

---
 .../model/jpa/OrganizationAdapter.java        |  5 ++-
 .../model/jpa/OrganizationRoleAdapter.java    | 37 ++++++++++++++++---
 2 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/src/main/java/io/phasetwo/service/model/jpa/OrganizationAdapter.java b/src/main/java/io/phasetwo/service/model/jpa/OrganizationAdapter.java
index eda4f92b..d599b9bc 100644
--- a/src/main/java/io/phasetwo/service/model/jpa/OrganizationAdapter.java
+++ b/src/main/java/io/phasetwo/service/model/jpa/OrganizationAdapter.java
@@ -266,7 +266,8 @@ public InvitationModel addInvitation(String email, UserModel inviter) {
 
   @Override
   public Stream<OrganizationRoleModel> getRolesStream() {
-    return org.getRoles().stream().map(r -> new OrganizationRoleAdapter(session, realm, em, r));
+    return org.getRoles().stream()
+        .map(r -> new OrganizationRoleAdapter(session, realm, em, this, r));
   }
 
   @Override
@@ -282,7 +283,7 @@ public OrganizationRoleModel addRole(String name) {
     r.setOrganization(org);
     em.persist(r);
     org.getRoles().add(r);
-    return new OrganizationRoleAdapter(session, realm, em, r);
+    return new OrganizationRoleAdapter(session, realm, em, this, r);
   }
 
   @Override
diff --git a/src/main/java/io/phasetwo/service/model/jpa/OrganizationRoleAdapter.java b/src/main/java/io/phasetwo/service/model/jpa/OrganizationRoleAdapter.java
index a7ec96a9..0dc8eda1 100644
--- a/src/main/java/io/phasetwo/service/model/jpa/OrganizationRoleAdapter.java
+++ b/src/main/java/io/phasetwo/service/model/jpa/OrganizationRoleAdapter.java
@@ -1,9 +1,11 @@
 package io.phasetwo.service.model.jpa;
 
+import io.phasetwo.service.model.OrganizationModel;
 import io.phasetwo.service.model.OrganizationRoleModel;
 import io.phasetwo.service.model.jpa.entity.OrganizationRoleEntity;
 import io.phasetwo.service.model.jpa.entity.UserOrganizationRoleMappingEntity;
 import jakarta.persistence.EntityManager;
+import jakarta.persistence.TypedQuery;
 import java.util.stream.Stream;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
@@ -18,13 +20,19 @@ public class OrganizationRoleAdapter
   protected final OrganizationRoleEntity role;
   protected final EntityManager em;
   protected final RealmModel realm;
+  protected final OrganizationModel org;
 
   public OrganizationRoleAdapter(
-      KeycloakSession session, RealmModel realm, EntityManager em, OrganizationRoleEntity role) {
+      KeycloakSession session,
+      RealmModel realm,
+      EntityManager em,
+      OrganizationModel org,
+      OrganizationRoleEntity role) {
     this.session = session;
     this.em = em;
     this.role = role;
     this.realm = realm;
+    this.org = org;
   }
 
   @Override
@@ -66,8 +74,10 @@ public Stream<UserModel> getUserMappingsStream() {
 
   @Override
   public void grantRole(UserModel user) {
-    // todo must be a member
-    revokeRole(user);
+    // user must be a member
+    if (!org.hasMembership(user)) return;
+    // skip if they already have the role
+    if (hasRole(user)) return;
     UserOrganizationRoleMappingEntity m = new UserOrganizationRoleMappingEntity();
     m.setId(KeycloakModelUtils.generateId());
     m.setUserId(user.getId());
@@ -78,11 +88,28 @@ public void grantRole(UserModel user) {
 
   @Override
   public void revokeRole(UserModel user) {
-    role.getUserMappings().removeIf(m -> m.getUserId().equals(user.getId()));
+    UserOrganizationRoleMappingEntity e = getByUser(user);
+    if (e != null) {
+      role.getUserMappings().remove(e);
+      em.remove(e);
+      em.flush();
+    }
   }
 
   @Override
   public boolean hasRole(UserModel user) {
-    return role.getUserMappings().stream().anyMatch(m -> m.getUserId().equals(user.getId()));
+    return (getByUser(user) != null);
+  }
+
+  UserOrganizationRoleMappingEntity getByUser(UserModel user) {
+    TypedQuery<UserOrganizationRoleMappingEntity> query =
+        em.createNamedQuery("getMappingByRoleAndUser", UserOrganizationRoleMappingEntity.class);
+    query.setParameter("userId", user.getId());
+    query.setParameter("role", role);
+    try {
+      return query.getSingleResult();
+    } catch (Exception ignore) {
+      return null;
+    }
   }
 }