Merge branch 'hotfix/2019.1.8' into stable-2019.1

openequella · Nov 4, 2020 · f57e4c7 · f57e4c7
2 parents 8e303d5 + dbcde4b
commit f57e4c7
Show file tree

Hide file tree

Showing 14 changed files with 177 additions and 10 deletions.
diff --git a/Source/Plugins/Core/com.equella.base/src/com/tle/common/security/PrivilegeTree.java b/Source/Plugins/Core/com.equella.base/src/com/tle/common/security/PrivilegeTree.java
@@ -550,6 +550,7 @@ private static synchronized void ensureMapping() {
     institution.registerPrivilege("DASHBOARD_PAGE");
     institution.registerPrivilege("HIERARCHY_PAGE");
     institution.registerPrivilege("INTEGRATION_SELECTION_SESSION");
+    institution.registerPrivilege("LIST_USERS");
 
     // OAuth hax
     institution.registerPrivilege("ADMINISTER_OAUTH_TOKENS");

diff --git a/Source/Plugins/Core/com.equella.base/src/com/tle/common/security/SecurityConstants.java b/Source/Plugins/Core/com.equella.base/src/com/tle/common/security/SecurityConstants.java
@@ -47,6 +47,9 @@ public static enum PrivilegeType {
   public static final String ARCHIVE_ITEM = "ARCHIVE_ITEM";
   public static final String VIEW_VIEWCOUNT = "VIEW_VIEWCOUNT";
 
+  public static final String EDIT_SYSTEM_SETTINGS = "EDIT_SYSTEM_SETTINGS";
+  public static final String LIST_USERS = "LIST_USERS";
+
   public static final String CREATE_VIRTUAL_BASE =
       "CREATE_" + SecurityConstants.VIRTUAL_BASE_ENTITY;
   public static final String EDIT_VIRTUAL_BASE = "EDIT_" + SecurityConstants.VIRTUAL_BASE_ENTITY;

diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/web/login/RootLoginSettingsSection.java b/Source/Plugins/Core/com.equella.core/src/com/tle/web/login/RootLoginSettingsSection.java
@@ -20,6 +20,7 @@
 
 import com.tle.common.Check;
 import com.tle.common.i18n.CurrentLocale;
+import com.tle.common.security.SecurityConstants;
 import com.tle.common.settings.standard.AutoLogin;
 import com.tle.common.usermanagement.user.valuebean.UserBean;
 import com.tle.core.services.user.UserService;
@@ -138,6 +139,7 @@ public void registered(String id, SectionTree tree) {
     selectUserDialog.setAjax(true);
     selectUserDialog.setOkLabel(OK_LABEL);
     selectUserDialog.setMultipleUsers(false);
+    selectUserDialog.setCheckPermissionBeforeOpen(SecurityConstants.EDIT_SYSTEM_SETTINGS, false);
 
     JSCallable inplace = ajax.getEffectFunction(EffectType.REPLACE_IN_PLACE);
     selectUserDialog.setOkCallback(

diff --git a/...Plugins/Core/com.equella.core/src/com/tle/web/oauth/section/OAuthClientEditorSection.java b/...Plugins/Core/com.equella.core/src/com/tle/web/oauth/section/OAuthClientEditorSection.java
@@ -276,6 +276,7 @@ public void registered(String id, SectionTree tree) {
     // selectUserDialog.setOkLabel(OK_LABEL);
     selectUserDialog.setMultipleUsers(false);
     selectUserButton.setClickHandler(selectUserDialog.getOpenFunction());
+    selectUserDialog.setCheckPermissionBeforeOpen(OAuthConstants.PRIV_CREATE_OAUTH_CLIENT, false);
     clearUserButton.setClickHandler(
         ajax.getAjaxUpdateDomFunction(
             tree, null, events.getEventHandler("clearUser"), "userAjaxDiv"));

diff --git a/...lugins/Core/com.equella.core/src/com/tle/web/sections/equella/utils/SelectUserDialog.java b/...lugins/Core/com.equella.core/src/com/tle/web/sections/equella/utils/SelectUserDialog.java
@@ -22,8 +22,13 @@
 
 import com.tle.annotation.NonNullByDefault;
 import com.tle.annotation.Nullable;
+import com.tle.beans.item.Item;
 import com.tle.common.Check;
+import com.tle.common.i18n.CurrentLocale;
+import com.tle.common.security.SecurityConstants;
 import com.tle.core.guice.Bind;
+import com.tle.core.security.TLEAclManager;
+import com.tle.exceptions.AccessDeniedException;
 import com.tle.web.freemarker.FreemarkerFactory;
 import com.tle.web.freemarker.annotations.ViewFactory;
 import com.tle.web.sections.SectionInfo;
@@ -39,6 +44,8 @@
 import com.tle.web.sections.result.util.CloseWindowResult;
 import com.tle.web.sections.result.util.KeyLabel;
 import com.tle.web.sections.standard.dialog.model.DialogModel;
+import com.tle.web.viewable.ViewableItem;
+import com.tle.web.viewable.servlet.ItemServlet;
 import java.util.List;
 import java.util.Set;
 import javax.inject.Inject;
@@ -63,7 +70,12 @@ public class SelectUserDialog extends AbstractOkayableDialog<SelectUserDialog.Mo
   private static final int WIDTH = 550;
 
   private CurrentUsersCallback currentUsersCallback;
+  private String permission = SecurityConstants.LIST_USERS;
+  private boolean checkOnItem = false;
+
   @Inject protected SelectUserSection section;
+  @Inject protected TLEAclManager securityManager;
+
   @ViewFactory private FreemarkerFactory viewFactory;
 
   @PlugKey("utils.selectuserdialog.default.title")
@@ -75,6 +87,9 @@ public class SelectUserDialog extends AbstractOkayableDialog<SelectUserDialog.Mo
   @PlugKey("utils.selectuserdialog.selectthisuser")
   private static String KEY_SINGLE_USER;
 
+  @PlugKey("editor.error.accessdenied")
+  private static String NO_PERMISSIONS;
+
   private Label title = LABEL_DEFAULT_TITLE;
 
   @Override
@@ -106,9 +121,12 @@ public void showDialog(SectionInfo info) {
 
   @Override
   protected SectionRenderable getRenderableContents(RenderContext context) {
-    getModel(context).setInnerContents(renderSection(context, section));
-
-    return viewFactory.createResult("utils/selectuserdialog.ftl", this);
+    if (canView(context)) {
+      getModel(context).setInnerContents(renderSection(context, section));
+      return viewFactory.createResult("utils/selectuserdialog.ftl", this);
+    } else {
+      throw new AccessDeniedException(CurrentLocale.get(NO_PERMISSIONS, permission));
+    }
   }
 
   @Override
@@ -200,4 +218,34 @@ public void setTitle(Label title) {
   public void setPrompt(Label prompt) {
     section.setPrompt(prompt);
   }
+
+  /**
+   * Allows checking permissions before rendering the dialog.
+   *
+   * @param permission The ACL string to check against. Defaults to LIST_USERS.
+   * @param checkOnItem If true, the ACL will be checked against an item, if false it will be
+   *     checked against the user. If checkOnItem is true, the request for this dialog MUST be an
+   *     item summary URL. If not, it will trigger an IllegalArgumentException when checking the
+   *     current viewable item.
+   */
+  public void setCheckPermissionBeforeOpen(String permission, boolean checkOnItem) {
+    this.permission = permission;
+    this.checkOnItem = checkOnItem;
+  }
+
+  private boolean canView(RenderContext context) {
+    if (permission == null) {
+      throw new IllegalStateException("Dialog permission should not be null");
+    }
+    if (checkOnItem) {
+      // Check the ACL against the current item
+      ViewableItem<Item> item = context.getAttribute(ItemServlet.VIEWABLE_ITEM);
+      if (item == null) {
+        throw new IllegalStateException("Item is null, so this item URL is invalid.");
+      }
+      return !(securityManager.filterNonGrantedPrivileges(item.getItem(), permission).isEmpty());
+    }
+    // if a permission is set but we don't need to check it against an item
+    return !(securityManager.filterNonGrantedPrivileges(permission).isEmpty());
+  }
 }
diff --git a/...Core/com.equella.core/src/com/tle/web/viewitem/sharing/ShareWithOthersContentSection.java b/...Core/com.equella.core/src/com/tle/web/viewitem/sharing/ShareWithOthersContentSection.java
@@ -149,6 +149,7 @@ public void registered(String id, SectionTree tree) {
     userSelect.setAjax(true);
     userSelect.setOkLabel(OK_LABEL);
     userSelect.setUsersCallback(this);
+    userSelect.setCheckPermissionBeforeOpen(REQUIRED_PRIVILEGE, true);
     tree.registerInnerSection(userSelect, id);
 
     selectUserToNotify.setClickHandler(userSelect.getOpenFunction());

diff --git a/....equella.core/src/com/tle/web/viewitem/summary/content/ChangeOwnershipContentSection.java b/....equella.core/src/com/tle/web/viewitem/summary/content/ChangeOwnershipContentSection.java
@@ -149,11 +149,14 @@ public void registered(String id, SectionTree tree) {
     ownerSelect.setPrompt(OWNER_DIALOG_PROMPT);
     ownerSelect.setOkCallback(events.getSubmitValuesFunction("changeOwner"));
     ownerSelect.setOkLabel(OWNER_DIALOG_OK);
+    ownerSelect.setCheckPermissionBeforeOpen(REQUIRED_PRIVILEGE, true);
 
     // Collaborators
     collabSelect.setTitle(COLLAB_DIALOG_TITLE);
     collabSelect.setPrompt(COLLAB_DIALOG_PROMPT);
     collabSelect.setMultipleUsers(true);
+    collabSelect.setCheckPermissionBeforeOpen(REQUIRED_PRIVILEGE, true);
+
     collabSelect.setOkCallback(
         ajax.getAjaxUpdateDomFunction(
             tree,

diff --git a/autotest/Tests/tests/fiveo/institution/acls/entries.xml b/autotest/Tests/tests/fiveo/institution/acls/entries.xml
@@ -1624,4 +1624,17 @@
     <aclOrder>0</aclOrder>
     <aclPriority>-1850</aclPriority>
   </com.tle.beans.security.AccessEntry>
-</list>
+  <com.tle.beans.security.AccessEntry>
+    <id>14999</id>
+    <expression>
+      <id>14998</id>
+      <dynamic>false</dynamic>
+    </expression>
+    <targetObject>*</targetObject>
+    <privilege>LIST_USERS</privilege>
+    <aggregateOrdering>0100 0000 G</aggregateOrdering>
+    <grantRevoke>G</grantRevoke>
+    <aclOrder>0</aclOrder>
+    <aclPriority>-1900</aclPriority>
+  </com.tle.beans.security.AccessEntry>
+</list>
diff --git a/autotest/Tests/tests/fiveo/institution/acls/expressions.xml b/autotest/Tests/tests/fiveo/institution/acls/expressions.xml
@@ -34,4 +34,9 @@
     <dynamic>false</dynamic>
     <expression>U:adfcaf58-241b-4eca-9740-6a26d1c3dd58</expression>
   </com.tle.beans.security.AccessExpression>
-</list>
+  <com.tle.beans.security.AccessExpression>
+    <id>14998</id>
+    <dynamic>false</dynamic>
+    <expression>U:adfcaf58-241b-4eca-9740-6a26d1c3dd58 </expression>
+  </com.tle.beans.security.AccessExpression>
+</list>
diff --git a/autotest/Tests/tests/ldap/institution/acls/entries.xml b/autotest/Tests/tests/ldap/institution/acls/entries.xml
@@ -1143,4 +1143,17 @@
     <aclOrder>0</aclOrder>
     <aclPriority>-1900</aclPriority>
   </com.tle.beans.security.AccessEntry>
-</list>
+  <com.tle.beans.security.AccessEntry>
+    <id>172662</id>
+    <expression>
+      <id>2542</id>
+      <dynamic>false</dynamic>
+    </expression>
+    <targetObject>*</targetObject>
+    <privilege>LIST_USERS</privilege>
+    <aggregateOrdering>0100 0000 G</aggregateOrdering>
+    <grantRevoke>G</grantRevoke>
+    <aclOrder>0</aclOrder>
+    <aclPriority>-1900</aclPriority>
+  </com.tle.beans.security.AccessEntry>
+</list>
diff --git a/autotest/Tests/tests/ldap/institution/acls/expressions.xml b/autotest/Tests/tests/ldap/institution/acls/expressions.xml
@@ -34,4 +34,9 @@
     <dynamic>false</dynamic>
     <expression>R:TLE_LOGGED_IN_USER_ROLE </expression>
   </com.tle.beans.security.AccessExpression>
-</list>
+  <com.tle.beans.security.AccessExpression>
+    <id>2542</id>
+    <dynamic>false</dynamic>
+    <expression>*</expression>
+  </com.tle.beans.security.AccessExpression>
+</list>
diff --git a/autotest/Tests/tests/workflow/institution/acls/entries.xml b/autotest/Tests/tests/workflow/institution/acls/entries.xml
@@ -3808,4 +3808,56 @@
     <aclOrder>0</aclOrder>
     <aclPriority>0</aclPriority>
   </com.tle.beans.security.AccessEntry>
-</list>
+  <com.tle.beans.security.AccessEntry>
+    <id>14999</id>
+    <expression>
+      <id>14998</id>
+      <dynamic>false</dynamic>
+    </expression>
+    <targetObject>*</targetObject>
+    <privilege>LIST_USERS</privilege>
+    <aggregateOrdering>0100 0000 G</aggregateOrdering>
+    <grantRevoke>G</grantRevoke>
+    <aclOrder>0</aclOrder>
+    <aclPriority>-1900</aclPriority>
+  </com.tle.beans.security.AccessEntry>
+  <com.tle.beans.security.AccessEntry>
+    <id>15000</id>
+    <expression>
+      <id>15001</id>
+      <dynamic>false</dynamic>
+    </expression>
+    <targetObject>*</targetObject>
+    <privilege>LIST_USERS</privilege>
+    <aggregateOrdering>0100 0000 G</aggregateOrdering>
+    <grantRevoke>G</grantRevoke>
+    <aclOrder>0</aclOrder>
+    <aclPriority>-1900</aclPriority>
+  </com.tle.beans.security.AccessEntry>
+  <com.tle.beans.security.AccessEntry>
+    <id>15002</id>
+    <expression>
+      <id>15003</id>
+      <dynamic>false</dynamic>
+    </expression>
+    <targetObject>*</targetObject>
+    <privilege>LIST_USERS</privilege>
+    <aggregateOrdering>0100 0000 G</aggregateOrdering>
+    <grantRevoke>G</grantRevoke>
+    <aclOrder>0</aclOrder>
+    <aclPriority>-1900</aclPriority>
+  </com.tle.beans.security.AccessEntry>
+  <com.tle.beans.security.AccessEntry>
+    <id>15004</id>
+    <expression>
+      <id>15005</id>
+      <dynamic>false</dynamic>
+    </expression>
+    <targetObject>*</targetObject>
+    <privilege>LIST_USERS</privilege>
+    <aggregateOrdering>0100 0000 G</aggregateOrdering>
+    <grantRevoke>G</grantRevoke>
+    <aclOrder>0</aclOrder>
+    <aclPriority>-1900</aclPriority>
+  </com.tle.beans.security.AccessEntry>
+</list>
diff --git a/autotest/Tests/tests/workflow/institution/acls/expressions.xml b/autotest/Tests/tests/workflow/institution/acls/expressions.xml
@@ -84,4 +84,24 @@
     <dynamic>false</dynamic>
     <expression>R:TLE_LOGGED_IN_USER_ROLE </expression>
   </com.tle.beans.security.AccessExpression>
-</list>
+  <com.tle.beans.security.AccessExpression>
+    <id>14998</id>
+    <dynamic>false</dynamic>
+    <expression>U:83bb1131-e54c-6f1e-e063-9d00597c8d97 </expression>
+  </com.tle.beans.security.AccessExpression>
+  <com.tle.beans.security.AccessExpression>
+    <id>15001</id>
+    <dynamic>false</dynamic>
+    <expression>U:e6f2f2b3-635e-3674-a044-ce97cb2dc563 </expression>
+  </com.tle.beans.security.AccessExpression>
+  <com.tle.beans.security.AccessExpression>
+    <id>15003</id>
+    <dynamic>false</dynamic>
+    <expression>U:d58b8087-7d64-2115-c187-20e5eb890743 </expression>
+  </com.tle.beans.security.AccessExpression>
+  <com.tle.beans.security.AccessExpression>
+    <id>15005</id>
+    <dynamic>false</dynamic>
+    <expression>U:0203b997-afa7-fb34-3793-57be9ee04524 </expression>
+  </com.tle.beans.security.AccessExpression>
+</list>
diff --git a/build.sbt b/build.sbt
@@ -113,7 +113,7 @@ name := "Equella"
 
 equellaMajor in ThisBuild := 2019
 equellaMinor in ThisBuild := 1
-equellaPatch in ThisBuild := 7
+equellaPatch in ThisBuild := 8
 equellaStream in ThisBuild := "Stable"
 equellaBuild in ThisBuild := buildConfig.value.getString("build.buildname")