Changeset 35287

Timestamp:

2021-08-12T19:34:43+12:00 (3 years ago)

Author:

anupama

Message:

Part 2 of commit on expanded user groups, related to prev commit (revision 35286) which was part 1. In this commit, I think I've identified the only times UserTermInfo.getExpandedGroups() needs to be called: when authentication-ping/ServletRealmCheck is called, which redirect to Authentication.processRemoteAuthentication(). At other times, the original group listing is what's required (for user listing display/modification in GS3 Reader Interface) or is sufficient (to check user is in admin group, the group listing in userDB/originally entered group listing is enough and no expanded list is needed. Maybe origGroups can better be renamed back to groups and getOrigGroups replaced with plain getGroups(), since getExpandedGroups() is the method called in more exceptional circumstances after all.

Location:

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

Files:

• service/Authentication.java (modified) (4 diffs)
• util/ModifyUsersDB.java (modified) (1 diff)
• util/UserTermInfo.java (modified) (7 diffs)
• util/txt2usersDB.java (modified) (1 diff)

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 EXPANDED_GROUPS = "expanded_groups";
   protected static final String STATUS = "status";
   protected static final String RECAPTCHA_KEY = "recaptcha_key";
@@ -427 +428 @@
                 
                 // gliserver.pl used to return the groups when authentication succeeded
-                String groups = retrieveDataForUser(username, GROUPS); //comma-separated list
+                String groups = retrieveDataForUser(username, EXPANDED_GROUPS); //comma-separated list
                 
                 if(collection.equals("")) {
@@ -1211 +1212 @@
                     break;
                 }
+                else if (dataType.equals(EXPANDED_GROUPS))
+                {
+                    data = ((UserTermInfo) userInfo.get(i)).getExpandedGroups();
+                    break;
+                }
                 else if (dataType.equals(STATUS))
                 {
@@ -1247 +1253 @@
             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 accountstatus = ((UserTermInfo) userInfo.get(i)).getAccountStatus();
             String comment = ((UserTermInfo) userInfo.get(i)).getComment();

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

@@ -138 +138 @@
             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();
             }

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

@@ -20 +20 @@
 package org.greenstone.gsdl3.util;
 
-//import java.util.Set;
-//import java.util.TreeSet;
+import java.util.Set;
+import java.util.TreeSet;
 
 public class UserTermInfo
@@ -28 +28 @@
     private String password;
     private String origGroups;
-    private String expandedGroups;    
+        private String expandedGroups = null;
     private String accountstatus;
     private String comment;
@@ -38 +38 @@
     this.password = password;
     this.origGroups = groups;
-    this.expandedGroups = UserTermInfo.expandGroups(this.origGroups);
+    //this.expandedGroups = UserTermInfo.expandGroups(this.origGroups); // Will do lazy evaluation
     this.accountstatus = accountStatus;
     this.comment = comment;
@@ -70 +70 @@
     
     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;
     }
@@ -95 +107 @@
      *   katoa.maori.place.iwi.hapu, katoa.maori.place.iwi.hapu.person
      * This method looks through all the groups and if any contains a period,
-     * it will expand such into all their explicit subgroups in the manner of the above example
-     * and then add them all back into the existing list of groups.
+     * it will expand such into all their explicit subgroups.
      * @param groups: String listing command-separated groups, any of which may
      * need expanding out.
-     * @return a comma-separated String listing the expanded groups.
+     * @return a comma-separated String listing all the original groups plus any expanded groups
+     * in natural order (so alphabetised).
      *
      * Tested at https://www.jdoodle.com/online-java-compiler/, where main contains:
@@ -109 +121 @@
      *
      * Also tested with the expandable string at start and at end of list of groups input string.
-    */
-    /*
-    public static String expandGroups(String groups) {
-    if(groups.indexOf('.') == -1) {
-        return groups;
-    }
-    else {
-        // Build up the string again
-        StringBuilder expandedGroupList = new StringBuilder();      
-
-        String[] indivGroups = groups.split("\\s*,\\s*"); // simultaneously get rid of any whitespace surrounding commas
-        for(String s : indivGroups) {
-        int indexOfPeriod = s.indexOf('.');
-
-        if(indexOfPeriod != -1) {
-            do {
-            String expandedGroup = s.substring(0, indexOfPeriod);
-            expandedGroupList.append(expandedGroup);
-            expandedGroupList.append(',');
-            indexOfPeriod += 1;         
-            indexOfPeriod = s.indexOf('.', indexOfPeriod);
-            } while(indexOfPeriod != -1);
-        }
-
-        // Whether this particular group s contained a period or not,
-        // remember to add this group string, s, back in in its entirety too        
-        expandedGroupList.append(s);
-        expandedGroupList.append(',');
-        }
-        expandedGroupList.deleteCharAt(expandedGroupList.length()-1); // remove extraneous comma        
-        return expandedGroupList.toString();
-    }
-    }
-    */
-
-    /**
+     *
      * This expansion method doesn't preserve overall ordering,
      * e.g. if groups = "all-collections-editor, katoa.maori.place.iwi.hapu.person, admin"
      * result is:
      *    admin,all-collections-editor,katoa,katoa.maori,katoa.maori.place,katoa.maori.place.iwi,katoa.maori.place.iwi.hapu,katoa.maori.place.iwi.hapu.person
-     * However, this method will ensure no duplicates, e.g. only one katoa, one katoa.maori etc.
+     * Where admin is now at start and all-colls-editor now comes after it.
+     * However, this method will ensure the requirement of no duplicates,
+     * e.g. only one katoa, one katoa.maori etc.
+     * to minimising checking if a user has necessary group membership,
      *   e.g. String groups = "all-collections-editor, admin, org.waikato.cs.dl.anu , org.waikato.cs.dl.pinky";
      * Produces the result:
@@ -191 +171 @@
     }
     }
+
+
+     /*
+    public static String expandGroups(String groups) {
+    if(groups.indexOf('.') == -1) {
+        return groups;
+    }
+    else {
+        // Build up the string again
+        StringBuilder expandedGroupList = new StringBuilder();      
+
+        String[] indivGroups = groups.split("\\s*,\\s*"); // simultaneously get rid of any whitespace surrounding commas
+        for(String s : indivGroups) {
+        int indexOfPeriod = s.indexOf('.');
+
+        if(indexOfPeriod != -1) {
+            do {
+            String expandedGroup = s.substring(0, indexOfPeriod);
+            expandedGroupList.append(expandedGroup);
+            expandedGroupList.append(',');
+            indexOfPeriod += 1;         
+            indexOfPeriod = s.indexOf('.', indexOfPeriod);
+            } while(indexOfPeriod != -1);
+        }
+
+        // Whether this particular group s contained a period or not,
+        // remember to add this group string, s, back in in its entirety too        
+        expandedGroupList.append(s);
+        expandedGroupList.append(',');
+        }
+        expandedGroupList.deleteCharAt(expandedGroupList.length()-1); // remove extraneous comma        
+        return expandedGroupList.toString();
+    }
+    }
+    */
     
 }

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

@@ -184 +184 @@
                                 password = Authentication.hashPassword(password);
                             }
+
+                            // 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;
                             accountstatus = accountstatus.equals("") ? user.getAccountStatus() : accountstatus;