Changeset 35298

Timestamp:

2021-08-16T20:17:05+12:00 (3 years ago)

Author:

anupama

Message:

Working version of hierarchical groups: now the usersDB stores expandedGroups instead of user-entered groups in the 'roles' table. The expandedGroups listing is therefore now consulted behind-the-scenes/automatically by HttpServletRequest.isUserInRole(), which is configured (in tomcat servlet configuration file greenstone3.xml) to query the roles table of the userDB. The new UserTermInfo.compactGroups() function takes care that the display value of the groups listing in the administration pages is the compacted version: it's not exactly the same as the user-entered value as the compactedGroups listing is in natural (alphabetic/ASCII) order and with duplicates removed.

Location:

main/trunk/greenstone3/src/java/org/greenstone/gsdl3

Files:

• service/Authentication.java (modified) (9 diffs)
• util/DerbyWrapper.java (modified) (5 diffs)
• util/ModifyUsersDB.java (modified) (2 diffs)
• util/UserQueryResult.java (modified) (1 diff)
• util/UserTermInfo.java (modified) (7 diffs)
• util/txt2usersDB.java (modified) (2 diffs)

main/trunk/greenstone3/src/java/org/greenstone/gsdl3/service/Authentication.java

@@ -163 +163 @@
   protected static final String COLLECTION = "collection";
   protected static final String GROUPS = "groups";
+  protected static final String COMPACTED_GROUPS = "compacted_groups";
   protected static final String EXPANDED_GROUPS = "expanded_groups";
   protected static final String STATUS = "status";
@@ -576 +577 @@
         }
         logger.debug("username="+username+", groups = "+groups);
+        logger.debug("expandedGroups would be: "+UserTermInfo.expandGroups(groups));
+        logger.debug("compactedGroups would be: "+UserTermInfo.compactGroups(groups));
+        
         if ((userInformation == null || username == null) && _userOpList.contains(op))
         {
@@ -788 +792 @@
           }
           
-          groups = null;
+          groups = null; // doesn't matter if local groups var keeps value in compactedGroups or expandedGroups form,
+          // as addUser() and addUserInformationToNode() will ensure use of expanded and compacted forms respectively
           String status = null;
           String comment = null;
@@ -798 +803 @@
 
           } else {
-            groups = retrieveDataForUser(previousUsername, GROUPS);
+            groups = retrieveDataForUser(previousUsername, EXPANDED_GROUPS);
             status = retrieveDataForUser(previousUsername, STATUS);
             comment = retrieveDataForUser(previousUsername, COMMENT);
@@ -872 +877 @@
 
             DerbyWrapper derbyWrapper = openDatabase();
-            String chpa_groups = retrieveDataForUser(user_name, GROUPS);
+            String chpa_groups = retrieveDataForUser(user_name, EXPANDED_GROUPS); // userDB now stores expandedGroups, not compactedGroups
             String chpa_comment = "password_changed_by_user";
             String info = derbyWrapper.modifyUserInfo(user_name, hashPassword(newPassword), chpa_groups, null, chpa_comment, null);
@@ -1079 +1084 @@
             for (int i = 0; i < userQueryResult.getSize(); i++)
             {
+                // can call either getExpandedGroups() or getOrigGroups() to check admin group is in there:
                 if (((UserTermInfo) userInfo.get(i)).getOrigGroups() != null && ((UserTermInfo) userInfo.get(i)).getOrigGroups().matches(".*\\badministrator\\b.*"))
                 {
@@ -1138 +1144 @@
     {
         newGroups = newGroups.replaceAll(" ", "");
+        // store only expandedGroups in DB now
+        newGroups = UserTermInfo.expandGroups(newGroups);
 
         //Check if the user already exists
@@ -1207 +1215 @@
                     break;
                 }
-                else if (dataType.equals(GROUPS))
+                else if (dataType.equals(COMPACTED_GROUPS))
                 {
                     data = ((UserTermInfo) userInfo.get(i)).getOrigGroups();
@@ -1252 +1260 @@
             Element user_node = doc.createElement(GSXML.USER_NODE_ELEM);
             String username = ((UserTermInfo) userInfo.get(i)).getUsername();
-            String groups = ((UserTermInfo) userInfo.get(i)).getOrigGroups();
-            // Retrieve original (stored, user-entered) and not expanded groups
-            // as getUserNodeList only used in addUserInformationToNode() which
-            // allows displaying and modifying user info pages
-            // and authenticating admin user which doesn't require expanded groups.
+            String compactedGroups = ((UserTermInfo) userInfo.get(i)).getOrigGroups();
+            // Since this is used for display in admin pages, get compacted (more similar to user-entered) groups
+            // and not expanded groups, as getUserNodeList only used in addUserInformationToNode() which
+            // allows displaying and modifying user info pages and authenticating admin user,
+            // none of which requires expanded groups.
             String accountstatus = ((UserTermInfo) userInfo.get(i)).getAccountStatus();
             String comment = ((UserTermInfo) userInfo.get(i)).getComment();
             String email = ((UserTermInfo) userInfo.get(i)).getEmail();
             user_node.setAttribute(USERNAME, username);
-            user_node.setAttribute(GROUPS, groups);
+            user_node.setAttribute(GROUPS, compactedGroups);
             user_node.setAttribute(STATUS, accountstatus);
             user_node.setAttribute(COMMENT, comment);

main/trunk/greenstone3/src/java/org/greenstone/gsdl3/util/DerbyWrapper.java

@@ -33 +33 @@
 import org.greenstone.util.GlobalProperties;
 
+/**
+ * The UserDB stores the groups in the "roles" table in expanded form, i.e. expandedGroups.
+ * This is what is retrieved as well. Call UserTermInfo.compactGroup(expandedGroups)
+ * to get the compacted form closer to what the user may have entered.
+*/
 public class DerbyWrapper
 {
@@ -474 +479 @@
     }
 
-    public boolean addUser(String username, String password, String groups, String accountstatus, String comment, String email)
+    public boolean addUser(String username, String password, String expandedGroups, String accountstatus, String comment, String email)
     {
         try
@@ -482 +487 @@
             state.execute(sql_insert_user);
 
-            String[] groupArray = groups.split(",");
+            String[] groupArray = expandedGroups.split(",");
             for (String g : groupArray)
             {
@@ -719 +724 @@
     }
 
-    public String modifyUserInfo(String username, String new_password, String groups, String accountstatus, String comment, String email)
+    public String modifyUserInfo(String username, String new_password, String expandedGroups, String accountstatus, String comment, String email)
     {
         try
@@ -750 +755 @@
             state.execute(sql_delete_groups);
 
-            String[] groupsArray = groups.split(",");
+            String[] groupsArray = expandedGroups.split(",");
             for (String g : groupsArray)
             {

main/trunk/greenstone3/src/java/org/greenstone/gsdl3/util/ModifyUsersDB.java

@@ -121 +121 @@
             else
             { // add new user
+                groups = UserTermInfo.expandGroups(groups);
                 //System.err.println("**** Trying to add user: ");
                 //System.err.println("**** " + username + " " + password + " " + groups + " " + accountstatus + " " + comment + " " + email);
@@ -138 +139 @@
             if (groups.equals(""))
             {
-                // groups should be origGroups and not expandedGroups
-                // As we want to store the groups in DB as entered
-                // and not as programmatically expanded.
-                groups = user.getOrigGroups();
+                // groups should be expandedGroups because we no longer store the groups in userDB
+                // as user-entered or compacted, but as programmatically expanded.
+                // This allows HttpServletRequest.isUserInRole() to now automatically retrieve the
+                // expandedGroups list of a user to check collectionConfig.xml security elements against.
+                
+                groups = user.getExpandedGroups();
+            } else {
+                groups = UserTermInfo.expandGroups(groups); // ensure groups are stored expanded in userDB
             }
             if (accountstatus.equals(""))

main/trunk/greenstone3/src/java/org/greenstone/gsdl3/util/UserQueryResult.java

@@ -37 +37 @@
     }
 
-    public void addUserTerm(String username, String password, String groups, String accountstatus, String comment, String email)
+    public void addUserTerm(String username, String password, String expandedGroups, String accountstatus, String comment, String email)
     {
-        UserTermInfo ui = new UserTermInfo(username, password, groups,
+        UserTermInfo ui = new UserTermInfo(username, password, expandedGroups,
                            accountstatus, comment, email);      
         users.add(ui);

main/trunk/greenstone3/src/java/org/greenstone/gsdl3/util/UserTermInfo.java

@@ -27 +27 @@
     private String username;
     private String password;
-    private String origGroups;
-        private String expandedGroups = null;
+        private String origGroups = null; // TODO: rename to compactedGroups and get-method, and update method calls accordingly
+        private String expandedGroups;
     private String accountstatus;
     private String comment;
     private String email;
 
-    public UserTermInfo(String username, String password, String groups,
+    // This constructor is called to instantiate a UserTermInfo object from entries in the userDB
+    // which now stores expandedGroups instead of the user-entered origGroups
+    // Use the other constructor if you know the user-entered groups
+    public UserTermInfo(String username, String password, String expandedGroups,
             String accountStatus, String comment, String email) {
     this.username = username;
     this.password = password;
-    this.origGroups = groups;
-    //this.expandedGroups = UserTermInfo.expandGroups(this.origGroups); // Will do lazy evaluation
+
+
+    this.expandedGroups = expandedGroups;
+    this.origGroups = UserTermInfo.compactGroups(this.expandedGroups); // alternative: don't set here and do lazy evaluation when getOrigGroups() is called
     this.accountstatus = accountStatus;
     this.comment = comment;
     this.email = email;
     }
+
+    
+    // Not used, for completion's sake:
+    // If you only have user-entered groups at hand, pass in null or "" for expandedGroups
+    // to create a UserTermInfo object, which will work out the expandedGroups.
+    /*
+    public UserTermInfo(String username, String password, String userEnteredGroups, String expandedGroups,
+            String accountStatus, String comment, String email) {
+    this.username = username;
+    this.password = password;
+    
+    this.origGroups = (userEnteredGroups == null) ? "" : userEnteredGroups;
+    if(expandedGroups == null || expandedGroups.equals("")) {
+        this.expandedGroups = UserTermInfo.expandGroups(this.origGroups);
+    }
+    this.accountstatus = accountStatus;
+    this.comment = comment;
+    this.email = email; 
+    }
+    */
     
     public String toString()
@@ -67 +92 @@
     public String getOrigGroups() {
     return origGroups;
-    }    
-    
-    public String getExpandedGroups() {
-    //return expandedGroups;
-    //return UserTermInfo.expandGroups(this.origGroups); // not storing, evaluating on demand
-    
+    //return UserTermInfo.compactGroups(this.expandedGroups); // not storing, evaluating on demand
+
     // lazy evaluation: getExpandedGroups() doesn't get called on every enquiry about
     // a UserTermInfo struct. It only gets called on authenticated-ping system-action
@@ -79 +100 @@
     // don't pre-calculate and store the expanded groups.
     // Is it meaningful to store this at all Ever or better to always evaluate on demand?
-    if(expandedGroups == null) {
-        expandedGroups = UserTermInfo.expandGroups(this.origGroups);
-    }
+    
+    }    
+    
+    public String getExpandedGroups() {
     return expandedGroups;
+    //return UserTermInfo.expandGroups(this.origGroups); // not storing, evaluating on demand
+    
+    // lazy evaluation: getExpandedGroups() doesn't get called on every enquiry about
+    // a UserTermInfo struct. It only gets called on authenticated-ping system-action
+    // (including via ServletRealmCheck being run from run from commandline or gliserver.pl)
+    // So for a whole list of users and the UserTermInfo struct of each,
+    // don't pre-calculate and store the expanded groups.
+    // Is it meaningful to store this at all Ever or better to always evaluate on demand?
+    ///if(expandedGroups == null) {
+    ///    expandedGroups = UserTermInfo.expandGroups(this.origGroups);
+    ///}
+    ///return expandedGroups;
     }
 
@@ -101 +135 @@
     }
 
+    /**
+     * The user may have entered groups of the form "nz.ac.waikato.toto, nz.ac.waikato.cs.pinky, admin"
+     * This would have got stored in the userDB in expanded form as: "admin, nz, nz.ac, nz.ac.waikato, nz.ac.waikato.cs, nz.ac.waikato.cs.pinky, nz.ac.waikato.toto"
+     * This method should return the user entered form again, or the most compact form at any rate. So duplicates removed and in natural (alphabetic) ordering.
+     * @param groups: given possibly expanded_groups, this method contracts them back to what the user would have entered for groups
+     * @return the user-entered form for groups     
+     * Tested:
+     * String groups = "administrator,all-collections-editor,nz,nz.ac,nz.ac.waikato,nz.ac.waikato.cs.pinkies, nz.ac.waikato.totos.toto, nz.ac.waikato.cs.pinkies.pinky, personal-collections-editor";
+     * and shuffled:
+     * String groups = "nz,nz.ac,nz.ac.waikato,administrator,nz.ac.waikato.cs.pinkies, nz.ac.waikato.totos.toto, nz.ac.waikato.cs.pinkies.pinky, personal-collections-editor,all-collections-editor";
+     * Result: administrator,all-collections-editor,nz.ac.waikato.cs.pinkies.pinky,nz.ac.waikato.totos.toto,personal-collections-editor:
+     */
+    public static String compactGroups(String groups) {
+    if(groups.indexOf('.') == -1) {
+        return groups;
+    }
+    else if(groups.equals("")) { // can return quickly if empty string
+        return groups;
+    }
+    else { // groups is a non-zero comma-separated list because we've already tested there's a period mark in it somewhere
+
+        // Normalize step: make sure what's in the database has no duplicates and is in natural ordering        
+        Set<String> groupSet = new TreeSet<String>();  // uses default (alphabetic/ascii) comparator
+        String[] indivGroups = groups.split("\\s*,\\s*"); // simultaneously get rid of whitespace surrounding commas
+        for(String s : indivGroups) {
+        groupSet.add(s); // will get rid of duplicate prefixes, although it "should" have been stored without duplicates
+        // But normalizing here ensures that the rest of the method always works with the correct ordering
+        }
+
+        indivGroups = new String[groupSet.size()];
+        indivGroups = groupSet.toArray(indivGroups); // now indivGroups is in natural ordering
+        groupSet.clear();
+        groupSet = null;
+
+        // The actual algorithm is now simple: if next group string doesn't start with current one
+        // (so current group string is not prefix of next group string), we add it to our compactGroups list
+        Set<String> compactGroups = new TreeSet<String>(); // uses default (alphabetic/ascii) comparator
+        String currGroup, nextGroup;
+        for(int i = 0; i < indivGroups.length-1; i++) {
+        currGroup = indivGroups[i];
+        nextGroup = indivGroups[i+1];
+
+        if(!nextGroup.startsWith(currGroup)) {
+            compactGroups.add(currGroup);
+        }
+        }
+
+        // don't forget to add in the very last group string
+        compactGroups.add(indivGroups[indivGroups.length-1]);
+
+
+        // recreate comma-separate string of groups
+        StringBuilder compactGroupList = new StringBuilder();
+        for(String s : compactGroups) {
+        compactGroupList.append(s);
+        compactGroupList.append(',');
+        }
+        compactGroupList.deleteCharAt(compactGroupList.length()-1); // remove extraneous comma
+        
+        return compactGroupList.toString();
+    }
+    
+    
+    /*
+    else {
+     
+
+        String[] indivGroups = groups.split("\\s*,\\s*"); // simultaneously get rid of whitespace surrounding commas
+        if(indivGroups.length == 0) {
+        return groups;
+        }
+        Set<String> compactGroups = new TreeSet<String>();
+        String currGroup = indivGroups[indivGroups.length-1];
+        for(int i = indivGroups.length-1; i--; i > 0) {
+        String prevGroup = indivGroups[i-1];
+        
+        int indexOfPeriod = currGroup.indexOf('.');
+        if(indexOfPeriod == -1) {
+           compactGroups.add(currGroup);
+           currGroup = prevGroup;
+        }
+        else {
+        
+            if(currGroup.startsWith(prevGroup)) {
+            compactGroups.add(currGroup);
+            if(i == 0) {
+                break;              
+            } else {
+                i--;
+                currGroup = indivGroups[i];
+            }
+            }
+            else {
+            compactGroups.add(currGroup);
+            currGroup = prevGroup;
+            }
+        }
+        }
+    }
+    */
+    } 
+
+    
     /** We might have groups of the form katoa.maori.(placename.)iwi-name.hapu-name.personal-name
      * ("katoa" means all, we could also use "mema" meaning member for this initial position).
@@ -138 +275 @@
         return groups;
     }
+    else if(groups.equals("")) { // can return quickly if empty string
+        return groups;
+    }
     else {
         Set<String> allGroups = new TreeSet<String>();
@@ -159 +299 @@
     
 
-        StringBuilder expandedGroupList = new StringBuilder();//"<groups = \"");
+        StringBuilder expandedGroupList = new StringBuilder();
         for(String s : allGroups) {
         expandedGroupList.append(s);
@@ -165 +305 @@
         }
         expandedGroupList.deleteCharAt(expandedGroupList.length()-1); // remove extraneous comma
-        
-        //expandedGroupList.append("\">");
         
         return expandedGroupList.toString();

main/trunk/greenstone3/src/java/org/greenstone/gsdl3/util/txt2usersDB.java

@@ -166 +166 @@
                                 password = Authentication.hashPassword(password);
                             } // if > 8 chars, password for user being added was already encrypted (hashed-and-hexed)
-                            dw.addUser(username, password, groups, accountstatus, comment, email);
+                            dw.addUser(username, password, UserTermInfo.expandGroups(groups), accountstatus, comment, email);
                         }
 
@@ -185 +185 @@
                             }
 
-                            // groups should be origGroups and not expandedGroups
-                            // As we want to store the groups in DB as entered
-                            // and not as programmatically expanded.
-                            groups = groups.equals("") ? user.getOrigGroups() : groups;
+                            // groups should be expandedGroups because we no longer store the groups in userDB
+                            // as user-entered or compacted, but as programmatically expanded.
+                            // This allows HttpServletRequest.isUserInRole() to now automatically retrieve the
+                            // expandedGroups list of a user to check collectionConfig.xml security elements against.
+
+                            groups = groups.equals("") ? user.getExpandedGroups() : UserTermInfo.expandGroups(groups);
                             accountstatus = accountstatus.equals("") ? user.getAccountStatus() : accountstatus;
                             comment = comment.equals("") ? user.getComment() : comment;