Hi IS Team,

I found an issue in doUpdateUserListOfRole of ReadWriteLDAPUserStoreManager
class. This Nullpointer occurred when we try to add users to role that not
in user store or try to delete users from not exist with given role. There
were throwing exception that breaking loop and it is not rollback the added
users prior to the exception. Same thing happen when we delete the users as
well.

I believe, there should be some mechanism to roll back those operation with
catching the fault.
But we can do input validation prior to all. We can check the user
availability within user store and within the role prior to the actual
operation. If there are any issues, then we can throw an exception saying
that. This is not additional checking of the user store, because this check
always happen before the operation.

I have attache the diff here,

*Harsha Thirimanna*
Senior Software Engineer; WSO2, Inc.; http://wso2.com
* <http://www.apache.org/>*
*email: **[email protected]* <[email protected]>* cell: +94 71 5186770  , +94 *
*774617784twitter: **http://twitter.com/ <http://twitter.com/afkham_azeez>*
*harshathirimannlinked-in: **http:
<http://lk.linkedin.com/in/afkhamazeez>**//www.linkedin.com/pub/harsha-thirimanna/10/ab8/122
<http://www.linkedin.com/pub/harsha-thirimanna/10/ab8/122>*

*Lean . Enterprise . Middleware*
Index: 
main/java/org/wso2/carbon/user/core/ldap/ReadWriteLDAPUserStoreManager.java
===================================================================
--- main/java/org/wso2/carbon/user/core/ldap/ReadWriteLDAPUserStoreManager.java 
(revision 211227)
+++ main/java/org/wso2/carbon/user/core/ldap/ReadWriteLDAPUserStoreManager.java 
(working copy)
@@ -1475,33 +1475,69 @@
                                        throw new 
UserStoreException(errorMessage);
 
                                } else {
-                                       if (newUsers!=null && newUsers.length 
!= 0) {
-                                               for (String newUser : newUsers) 
{
-                                                       String userNameDN = 
getNameInSpaceForUserName(newUser);
-                                                       if 
(!isUserInRole(userNameDN, resultedGroup)) {
-                                                               
modifyUserInRole(userNameDN, groupName, DirContext.ADD_ATTRIBUTE,
-                                                                               
 searchBase);
-                                                       } else {
-                                                               errorMessage =
-                                                                              
"User: " + newUser + " already belongs to role: " +
-                                                                               
       roleName;
-                                                               throw new 
UserStoreException(errorMessage);
-                                                       }
-                                               }
-                                       }
 
+                    List<String> newUserList = new ArrayList<String>();
+                    List<String> deleteUserList = new ArrayList<String>();
+
+                    if (newUsers != null && newUsers.length != 0) {
+                        String invalidUserList = "";
+                        String existingUserList = "";
+
+                        for (String newUser : newUsers) {
+                            if(newUser==null || newUser.trim().length() == 0){
+                                continue;
+                            }
+                            String userNameDN = 
getNameInSpaceForUserName(newUser);
+                            if (userNameDN == null) {
+                                invalidUserList += newUser + " "  ;
+                            } else if (isUserInRole(userNameDN, 
resultedGroup)) {
+                                existingUserList += userNameDN + ",";
+                            } else {
+                                newUserList.add(userNameDN);
+                            }
+                        }
+                        if (!invalidUserList.equals("") || 
!existingUserList.equals("")) {
+                            errorMessage = (invalidUserList.equals("") ? "" : 
"'" + invalidUserList + "' not in " +
+                                    "the user store. ") + 
(existingUserList.equals("") ? "" : "'" + existingUserList + "' " +
+                                    " already belong to the role :" + 
roleName) ;
+                            throw new UserStoreException(errorMessage);
+                        }
+
+                    }
+
                                        if (deletedUsers != null && 
deletedUsers.length != 0) {
+                        String invalidUserList = "";
                                                for (String deletedUser : 
deletedUsers) {
                             if(deletedUser == null || 
deletedUser.trim().length() == 0){
                                 continue;
                             }
                                                        String userNameDN = 
getNameInSpaceForUserName(deletedUser);
-                                                       
modifyUserInRole(userNameDN, groupName, DirContext.REMOVE_ATTRIBUTE,
-                                                                        
searchBase);
-                                                       // needs to clear authz 
cache for deleted users
-                                                       
userRealm.getAuthorizationManager().clearUserAuthorization(deletedUser);
-                                               }
+                            if (userNameDN == null) {
+                                invalidUserList += deletedUser + ",";
+                            } else {
+                                deleteUserList.add(userNameDN);
+                            }
+                        }
+                        if (!invalidUserList.equals("")) {
+                            errorMessage = (invalidUserList.equals("") ? "" : 
"'" + invalidUserList + "' not in " +
+                                    "the user store. ") ;
+                            throw new UserStoreException(errorMessage);
+                        }
+
                                        }
+
+                    for (String userNameDN : newUserList) {
+                        modifyUserInRole(userNameDN, groupName, 
DirContext.ADD_ATTRIBUTE, searchBase);
+                    }
+
+                    for(String userNameDN : deleteUserList){
+                        modifyUserInRole(userNameDN, groupName, 
DirContext.REMOVE_ATTRIBUTE,
+                                searchBase);
+                        // needs to clear authz cache for deleted users
+                        
userRealm.getAuthorizationManager().clearUserAuthorization(userNameDN);
+                    }
+
+
                                }                       
                        } catch (NamingException e) {
 
_______________________________________________
Dev mailing list
[email protected]
http://wso2.org/cgi-bin/mailman/listinfo/dev

Reply via email to