Fixed AD import (#614)

pom.xml

@@ -81,6 +81,7 @@
     <security.ad.searchFilter></security.ad.searchFilter>
     <security.ad.ignore.partial.result.exception>true</security.ad.ignore.partial.result.exception>
     <security.ad.result.count.limit>30000</security.ad.result.count.limit> <!-- 0 means no limit -->
+    <security.ad.default.import.group>public</security.ad.default.import.group>
 
     <security.cas.loginUrl></security.cas.loginUrl>
     <security.cas.callbackUrl></security.cas.callbackUrl>

src/main/java/org/ohdsi/webapi/service/UserService.java

@@ -25,6 +25,7 @@
 import org.ohdsi.webapi.shiro.Entities.UserEntity;
 import org.ohdsi.webapi.shiro.PermissionManager;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Component;
 
 /**
@@ -42,6 +43,9 @@ public class UserService {
   @Autowired
   private ApplicationEventPublisher eventPublisher;
 
+  @Value("${security.ad.default.import.group}#{T(java.util.Collections).emptyList()}")
+  private List<String> defaultRoles;
+
   private Map<String, String> roleCreatorPermissionsTemplate = new LinkedHashMap<>();
 
   public UserService() {
@@ -99,6 +103,7 @@ public int compareTo(Permission o) {
   public static class Role implements Comparable<Role> {
     public Long id;
     public String role;
+    public boolean defaultImported;
 
     public Role() {}
 
@@ -107,6 +112,11 @@ public Role (RoleEntity roleEntity) {
       this.role = roleEntity.getName();
     }
 
+    public Role (RoleEntity roleEntity, boolean defaultImported) {
+      this(roleEntity);
+      this.defaultImported = defaultImported;
+    }
+
     @Override
     public int compareTo(Role o) {
       Comparator c = Comparators.naturalOrder();
@@ -295,7 +305,7 @@ public ArrayList<Permission> getPermissions() {
   }
 
   private ArrayList<Permission> convertPermissions(final Iterable<PermissionEntity> permissionEntities) {
-    ArrayList<Permission> permissions = new ArrayList<Permission>();
+    ArrayList<Permission> permissions = new ArrayList<>();
     for (PermissionEntity permissionEntity : permissionEntities) {
       Permission permission = new Permission(permissionEntity);
       permissions.add(permission);
@@ -305,17 +315,17 @@ private ArrayList<Permission> convertPermissions(final Iterable<PermissionEntity
   }
 
   private ArrayList<Role> convertRoles(final Iterable<RoleEntity> roleEntities) {
-    ArrayList<Role> roles = new ArrayList<Role>();
+    ArrayList<Role> roles = new ArrayList<>();
     for (RoleEntity roleEntity : roleEntities) {
-      Role role = new Role(roleEntity);
+      Role role = new Role(roleEntity, defaultRoles.contains(roleEntity.getName()));
       roles.add(role);
     }
 
     return roles;
   }
 
   private ArrayList<User> convertUsers(final Iterable<UserEntity> userEntities) {
-    ArrayList<User> users = new ArrayList<User>();
+    ArrayList<User> users = new ArrayList<>();
     for (UserEntity userEntity : userEntities) {
       User user = new User(userEntity);
       users.add(user);

src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java

@@ -23,6 +23,7 @@
 import org.apache.shiro.web.util.WebUtils;
 import org.ohdsi.webapi.shiro.PermissionManager;
 import org.ohdsi.webapi.shiro.TokenManager;
+import org.ohdsi.webapi.util.UserUtils;
 
 /**
  *
@@ -80,9 +81,7 @@ protected boolean preHandle(ServletRequest request, ServletResponse response) th
       throw new Exception("Unknown type of principal");
     }
 
-    if (login != null) {
-      login = login.toLowerCase();
-    }
+    login = UserUtils.toLowerCase(login);
 
     // stop session to make logout of OAuth users possible
     Session session = SecurityUtils.getSubject().getSession(false);

src/main/java/org/ohdsi/webapi/user/importer/DefaultUserImporter.java

@@ -10,6 +10,7 @@
 import org.ohdsi.webapi.shiro.Entities.UserEntity;
 import org.ohdsi.webapi.shiro.Entities.UserRepository;
 import org.ohdsi.webapi.shiro.PermissionManager;
+import org.ohdsi.webapi.util.UserUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -75,7 +76,7 @@ public List<AtlasUserRoles> findUsers(LdapProviderType providerType, RoleGroupMa
             .map(user -> {
               AtlasUserRoles atlasUser = new AtlasUserRoles();
               atlasUser.setDisplayName(user.getDisplayName());
-              atlasUser.setLogin(user.getLogin());
+              atlasUser.setLogin(UserUtils.toLowerCase(user.getLogin()));
               List<UserService.Role> roles = user.getGroups().stream()
                       .flatMap(g -> mapping.getRoleGroups()
                               .stream()
@@ -93,15 +94,18 @@ public List<AtlasUserRoles> findUsers(LdapProviderType providerType, RoleGroupMa
 
   @Override
   @Transactional
-  public void importUsers(List<AtlasUserRoles> users) {
+  public void importUsers(List<AtlasUserRoles> users, List<String> defaultRoles) {
 
     users.forEach(user -> {
+      String login = UserUtils.toLowerCase(user.getLogin());
       Set<String> roles = user.getRoles().stream().map(role -> role.role).collect(Collectors.toSet());
+      roles.addAll(defaultRoles);
       try {
         UserEntity userEntity;
-        if (LdapUserImportStatus.MODIFIED.equals(user.getStatus()) && Objects.nonNull(userEntity = userRepository.findByLogin(user.getLogin()))) {
+        if (Objects.nonNull(userEntity = userRepository.findByLogin(login)) &&
+                LdapUserImportStatus.MODIFIED.equals(getStatus(userEntity, user.getRoles()))) {
           Set<RoleEntity> userRoles = userManager.getUserRoles(userEntity.getId());
-          userRoles.forEach(r -> {
+          userRoles.stream().filter(role -> !role.getName().equalsIgnoreCase(login)).forEach(r -> {
             try {
               userManager.removeUserFromRole(r.getName(), userEntity.getLogin());
             } catch (Exception e) {
@@ -116,10 +120,10 @@ public void importUsers(List<AtlasUserRoles> users) {
             }
           });
         } else {
-          userManager.registerUser(user.getLogin(), roles);
+          userManager.registerUser(login, roles);
         }
       } catch (Exception e) {
-        logger.error("Failed to register user {}", user.getLogin(), e);
+        logger.error("Failed to register user {}", login, e);
       }
     });
   }
@@ -177,11 +181,17 @@ public void testConnection(LdapProviderType providerType) {
 
   private LdapUserImportStatus getStatus(AtlasUserRoles atlasUser) {
 
-    LdapUserImportStatus result = LdapUserImportStatus.NEW_USER;
     UserEntity userEntity = userRepository.findByLogin(atlasUser.getLogin());
+    return getStatus(userEntity, atlasUser.getRoles());
+  }
+
+  private LdapUserImportStatus getStatus(UserEntity userEntity,  List<UserService.Role> atlasUserRoles) {
+
+    LdapUserImportStatus result = LdapUserImportStatus.NEW_USER;
+
     if (Objects.nonNull(userEntity)) {
       List<Long> atlasRoleIds = userEntity.getUserRoles().stream().map(userRole -> userRole.getRole().getId()).collect(Collectors.toList());
-      List<Long> mappedRoleIds = atlasUser.getRoles().stream().map(role -> role.id).collect(Collectors.toList());
+      List<Long> mappedRoleIds = atlasUserRoles.stream().map(role -> role.id).collect(Collectors.toList());
       result = CollectionUtils.isEqualCollection(atlasRoleIds, mappedRoleIds) ? LdapUserImportStatus.EXISTS : LdapUserImportStatus.MODIFIED;
     }
     return result;

src/main/java/org/ohdsi/webapi/user/importer/UserImportService.java

@@ -36,6 +36,9 @@ public class UserImportService {
   @Value("${security.ldap.url}")
   private String ldapUrl;
 
+  @Value("${security.ad.default.import.group}#{T(java.util.Collections).emptyList()}")
+  private List<String> defaultRoles;
+
   @GET
   @Path("user/providers")
   @Produces(MediaType.APPLICATION_JSON)
@@ -90,7 +93,7 @@ public List<AtlasUserRoles> findDirectoryUsers(@PathParam("type") String type, R
   @Path("user/import")
   @Consumes(MediaType.APPLICATION_JSON)
   public Response importUsers(List<AtlasUserRoles> users) {
-    userImporter.importUsers(users);
+    userImporter.importUsers(users, defaultRoles);
     return Response.ok().build();
   }
 

src/main/java/org/ohdsi/webapi/user/importer/UserImporter.java

@@ -13,7 +13,7 @@ public interface UserImporter {
 
   List<AtlasUserRoles> findUsers(LdapProviderType providerType, RoleGroupMapping mapping);
 
-  void importUsers(List<AtlasUserRoles> users);
+  void importUsers(List<AtlasUserRoles> users, List<String> defaultRoles);
 
   void saveRoleGroupMapping(LdapProviderType providerType, List<RoleGroupMappingEntity> mappingEntities);
 

src/main/java/org/ohdsi/webapi/util/UserUtils.java

@@ -27,4 +27,9 @@ public static String nullSafeLogin(UserEntity user) {
     return Objects.nonNull(user) ? user.getLogin() : "";
   }
 
+  public static String toLowerCase(String input) {
+
+    return Objects.nonNull(input) ? input.toLowerCase() : input;
+  }
+
 }

src/main/resources/application.properties

@@ -130,6 +130,7 @@ security.ad.system.password=${security.ad.system.password}
 security.ad.searchFilter=${security.ad.searchFilter}
 security.ad.ignore.partial.result.exception=${security.ad.ignore.partial.result.exception}
 security.ad.result.count.limit=${security.ad.result.count.limit}
+security.ad.default.import.group=${security.ad.default.import.group}
 
 security.googleIap.cloudProjectId=${ssecurity.googleIap.cloudProjectId}
 security.googleIap.backendServiceId=${security.googleIap.backendServiceId}