From 994b31bfc3d6fb0429fde8c11ea40687e0aacee2 Mon Sep 17 00:00:00 2001 From: Penghai Date: Tue, 25 Jan 2022 11:07:12 +1100 Subject: [PATCH 1/4] build: bump the version to 2021.1.4-stable --- build.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index 2fa4e0e8d5..eba0f1d5c9 100644 --- a/build.sbt +++ b/build.sbt @@ -116,7 +116,7 @@ name := "Equella" equellaMajor in ThisBuild := 2021 equellaMinor in ThisBuild := 1 -equellaPatch in ThisBuild := 3 +equellaPatch in ThisBuild := 4 equellaStream in ThisBuild := "Stable" equellaBuild in ThisBuild := buildConfig.value.getString("build.buildname") From 6724de9d6aa17a6c2939cb7655f040c4b9aa12bd Mon Sep 17 00:00:00 2001 From: Penghai Date: Tue, 25 Jan 2022 11:32:58 +1100 Subject: [PATCH 2/4] Merge pull request #3628 from edalex-yinzi/fix/LIST_USERS_ACL_check fix(server): add missing LIST_USERS ACL --- .../com/tle/core/security/AclChecks.scala | 22 +++++++++++++++++++ .../tle/web/api/users/UserQueryResource.scala | 4 +++- .../com/tle/core/security/TLEAclManager.java | 9 ++++++++ .../core/security/impl/TLEAclManagerImpl.java | 5 +++++ .../tests/facet/institution/acls/entries.xml | 13 +++++++++++ .../facet/institution/acls/expressions.xml | 5 +++++ .../tests/rest/institution/acls/entries.xml | 13 +++++++++++ .../rest/institution/acls/expressions.xml | 5 +++++ .../vanilla/institution/acls/entries.xml | 15 ++++++++++++- .../vanilla/institution/acls/expressions.xml | 7 +++++- 10 files changed, 95 insertions(+), 3 deletions(-) diff --git a/Source/Plugins/Core/com.equella.core/scalasrc/com/tle/core/security/AclChecks.scala b/Source/Plugins/Core/com.equella.core/scalasrc/com/tle/core/security/AclChecks.scala index f2b79df23f..8191d1b7a7 100644 --- a/Source/Plugins/Core/com.equella.core/scalasrc/com/tle/core/security/AclChecks.scala +++ b/Source/Plugins/Core/com.equella.core/scalasrc/com/tle/core/security/AclChecks.scala @@ -25,6 +25,7 @@ import com.tle.common.security.SecurityConstants import com.tle.common.security.SecurityConstants.GRANT import com.tle.core.db.{DB, UserContext} import com.tle.exceptions.PrivilegeRequiredException +import com.tle.legacy.LegacyGuice import io.doolse.simpledba.jdbc._ import scala.collection.JavaConverters._ @@ -107,4 +108,25 @@ object AclChecks { } } + /** + * Checks if the current user has the specified ACL. + * + * @param privilege Required ACL, typically defined as a constant in `com.tle.common.security.SecurityConstants` + * @param includePossibleOwnerAcls `true` to include possible owner ACLs + */ + def hasAcl(privilege: String, includePossibleOwnerAcls: Boolean = false): Boolean = { + LegacyGuice.aclManager.hasPrivilege(Set(privilege).asJava, includePossibleOwnerAcls) + } + + /** + * Checks if the current user has the specified ACL or throws `PrivilegeRequiredException`. + * + * @param privilege Required ACL, typically defined as a constant in `com.tle.common.security.SecurityConstants` + * @param includePossibleOwnerAcls `true` to include possible owner ACLs + */ + def hasAclOrThrow(privilege: String, includePossibleOwnerAcls: Boolean = false): Unit = { + if (!hasAcl(privilege, includePossibleOwnerAcls)) { + throw new PrivilegeRequiredException(privilege) + } + } } diff --git a/Source/Plugins/Core/com.equella.core/scalasrc/com/tle/web/api/users/UserQueryResource.scala b/Source/Plugins/Core/com.equella.core/scalasrc/com/tle/web/api/users/UserQueryResource.scala index cbc8837e82..ba0fdb338f 100644 --- a/Source/Plugins/Core/com.equella.core/scalasrc/com/tle/web/api/users/UserQueryResource.scala +++ b/Source/Plugins/Core/com.equella.core/scalasrc/com/tle/web/api/users/UserQueryResource.scala @@ -21,10 +21,10 @@ package com.tle.web.api.users import com.tle.beans.usermanagement.standard.wrapper.SharedSecretSettings import com.tle.common.security.SecurityConstants import com.tle.common.usermanagement.user.valuebean.{GroupBean, RoleBean, UserBean} +import com.tle.core.security.AclChecks.hasAclOrThrow import com.tle.legacy.LegacyGuice import io.swagger.annotations.{Api, ApiParam} import javax.ws.rs._ - import scala.collection.JavaConverters._ case class LookupQuery(users: Seq[String], groups: Seq[String], roles: Seq[String]) @@ -55,6 +55,7 @@ class UserQueryResource { @POST @Path("lookup") def lookup(queries: LookupQuery): LookupQueryResult = { + hasAclOrThrow(SecurityConstants.LIST_USERS) val us = LegacyGuice.userService val users = us.getInformationForUsers(queries.users.asJava) val groups = us.getInformationForGroups(queries.groups.asJava) @@ -72,6 +73,7 @@ class UserQueryResource { @QueryParam("groups") @DefaultValue("true") @ApiParam("Include groups") sgroups: Boolean, @QueryParam("roles") @DefaultValue("true") @ApiParam("Include roles") sroles: Boolean) : LookupQueryResult = { + hasAclOrThrow(SecurityConstants.LIST_USERS) val us = LegacyGuice.userService val users = if (susers) us.searchUsers(q).asScala else Iterable.empty val groups = if (sgroups) us.searchGroups(q).asScala else Iterable.empty diff --git a/Source/Plugins/Security/com.tle.core.security/src/com/tle/core/security/TLEAclManager.java b/Source/Plugins/Security/com.tle.core.security/src/com/tle/core/security/TLEAclManager.java index c924129c95..2b2a1839d5 100644 --- a/Source/Plugins/Security/com.tle.core.security/src/com/tle/core/security/TLEAclManager.java +++ b/Source/Plugins/Security/com.tle.core.security/src/com/tle/core/security/TLEAclManager.java @@ -49,6 +49,15 @@ Collection filterNonGrantedObjects( */ boolean hasPrivilege(Object domainObj, Privilege... privilege); + /** + * Check if user has the supplied privileges for any object. + * + * @param privileges List of privileges to be checked + * @param includePossibleOwnerAcls true to include 'ownerAcl'. + * @return true if ANY of the supplied privileges are granted. + */ + boolean hasPrivilege(Collection privileges, boolean includePossibleOwnerAcls); + /** Return a map of domain objects to maps of privileges. */ Map> getPrivilegesForObjects( Collection privileges, Collection domainObjs); diff --git a/Source/Plugins/Security/com.tle.core.security/src/com/tle/core/security/impl/TLEAclManagerImpl.java b/Source/Plugins/Security/com.tle.core.security/src/com/tle/core/security/impl/TLEAclManagerImpl.java index 9b70a0d78b..e23592234b 100644 --- a/Source/Plugins/Security/com.tle.core.security/src/com/tle/core/security/impl/TLEAclManagerImpl.java +++ b/Source/Plugins/Security/com.tle.core.security/src/com/tle/core/security/impl/TLEAclManagerImpl.java @@ -164,6 +164,11 @@ public boolean hasPrivilege(Object domainObj, Privilege... privilege) { return !filterNonGrantedPrivileges(domainObj, privs).isEmpty(); } + @Override + public boolean hasPrivilege(Collection privileges, boolean includePossibleOwnerAcls) { + return !filterNonGrantedPrivileges(privileges, includePossibleOwnerAcls).isEmpty(); + } + @Override @Transactional public Map> getPrivilegesForObjects( diff --git a/autotest/Tests/tests/facet/institution/acls/entries.xml b/autotest/Tests/tests/facet/institution/acls/entries.xml index e004b81ea7..f4dd7173ef 100644 --- a/autotest/Tests/tests/facet/institution/acls/entries.xml +++ b/autotest/Tests/tests/facet/institution/acls/entries.xml @@ -1910,4 +1910,17 @@ 0 -1900 + + 36596 + + 36595 + false + + * + LIST_USERS + 0100 0000 G + G + 0 + -1900 + diff --git a/autotest/Tests/tests/facet/institution/acls/expressions.xml b/autotest/Tests/tests/facet/institution/acls/expressions.xml index d0452f265d..08eb9e1e19 100644 --- a/autotest/Tests/tests/facet/institution/acls/expressions.xml +++ b/autotest/Tests/tests/facet/institution/acls/expressions.xml @@ -54,4 +54,9 @@ false U:f40e91d5-2024-4bb0-87d6-1a0fa22ebae5 + + 36595 + false + U:f40e91d5-2024-4bb0-87d6-1a0fa22ebae5 R:351c2f38-b78a-4049-88d5-dc33693c9a9f OR + diff --git a/autotest/Tests/tests/rest/institution/acls/entries.xml b/autotest/Tests/tests/rest/institution/acls/entries.xml index 4302ae4d29..a05e207028 100644 --- a/autotest/Tests/tests/rest/institution/acls/entries.xml +++ b/autotest/Tests/tests/rest/institution/acls/entries.xml @@ -1767,4 +1767,17 @@ 1 0 + + 37230 + + 37229 + false + + * + LIST_USERS + 0100 0000 G + G + 0 + -1900 + diff --git a/autotest/Tests/tests/rest/institution/acls/expressions.xml b/autotest/Tests/tests/rest/institution/acls/expressions.xml index 37e8b8e8a6..876a5a197d 100644 --- a/autotest/Tests/tests/rest/institution/acls/expressions.xml +++ b/autotest/Tests/tests/rest/institution/acls/expressions.xml @@ -44,4 +44,9 @@ false U:adfcaf58-241b-4eca-9740-6a26d1c3dd58 + + 37229 + false + U:adfcaf58-241b-4eca-9740-6a26d1c3dd58 + diff --git a/autotest/Tests/tests/vanilla/institution/acls/entries.xml b/autotest/Tests/tests/vanilla/institution/acls/entries.xml index a8f28393e8..c0ae9cd70b 100644 --- a/autotest/Tests/tests/vanilla/institution/acls/entries.xml +++ b/autotest/Tests/tests/vanilla/institution/acls/entries.xml @@ -3600,4 +3600,17 @@ 0 -1400 - \ No newline at end of file + + 33573 + + 33572 + false + + * + LIST_USERS + 0100 0000 G + G + 0 + -1900 + + diff --git a/autotest/Tests/tests/vanilla/institution/acls/expressions.xml b/autotest/Tests/tests/vanilla/institution/acls/expressions.xml index d6d4746cc3..447debff95 100644 --- a/autotest/Tests/tests/vanilla/institution/acls/expressions.xml +++ b/autotest/Tests/tests/vanilla/institution/acls/expressions.xml @@ -114,4 +114,9 @@ false U:f2ce5cf5-e6d2-8d1a-4627-4e917b4e6ba3 - \ No newline at end of file + + 33572 + false + U:b09a4042-b091-87ed-eba9-6fb3c0fbe9a6 R:ROLE_SYSTEM_ADMINISTRATOR OR + + From 0783c96d6f9a1489d4fdc0be603cc60785336a9c Mon Sep 17 00:00:00 2001 From: Penghai Date: Tue, 25 Jan 2022 11:53:17 +1100 Subject: [PATCH 3/4] chore: update the format of 'AclChecks.scala' --- .../com/tle/core/security/AclChecks.scala | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Source/Plugins/Core/com.equella.core/scalasrc/com/tle/core/security/AclChecks.scala b/Source/Plugins/Core/com.equella.core/scalasrc/com/tle/core/security/AclChecks.scala index 8191d1b7a7..edee77af8f 100644 --- a/Source/Plugins/Core/com.equella.core/scalasrc/com/tle/core/security/AclChecks.scala +++ b/Source/Plugins/Core/com.equella.core/scalasrc/com/tle/core/security/AclChecks.scala @@ -109,21 +109,21 @@ object AclChecks { } /** - * Checks if the current user has the specified ACL. - * - * @param privilege Required ACL, typically defined as a constant in `com.tle.common.security.SecurityConstants` - * @param includePossibleOwnerAcls `true` to include possible owner ACLs - */ + * Checks if the current user has the specified ACL. + * + * @param privilege Required ACL, typically defined as a constant in `com.tle.common.security.SecurityConstants` + * @param includePossibleOwnerAcls `true` to include possible owner ACLs + */ def hasAcl(privilege: String, includePossibleOwnerAcls: Boolean = false): Boolean = { LegacyGuice.aclManager.hasPrivilege(Set(privilege).asJava, includePossibleOwnerAcls) } /** - * Checks if the current user has the specified ACL or throws `PrivilegeRequiredException`. - * - * @param privilege Required ACL, typically defined as a constant in `com.tle.common.security.SecurityConstants` - * @param includePossibleOwnerAcls `true` to include possible owner ACLs - */ + * Checks if the current user has the specified ACL or throws `PrivilegeRequiredException`. + * + * @param privilege Required ACL, typically defined as a constant in `com.tle.common.security.SecurityConstants` + * @param includePossibleOwnerAcls `true` to include possible owner ACLs + */ def hasAclOrThrow(privilege: String, includePossibleOwnerAcls: Boolean = false): Unit = { if (!hasAcl(privilege, includePossibleOwnerAcls)) { throw new PrivilegeRequiredException(privilege) From 35fb49ca85e7ebbddf7c5ff23c3b75f51ca815b7 Mon Sep 17 00:00:00 2001 From: Penghai Date: Wed, 26 Jan 2022 12:33:40 +1100 Subject: [PATCH 4/4] bugfix: ensure Custom Attachment uses the correct latest version --- .../com/tle/web/api/search/SearchHelper.scala | 46 ++++++++++++++++--- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/Source/Plugins/Core/com.equella.core/scalasrc/com/tle/web/api/search/SearchHelper.scala b/Source/Plugins/Core/com.equella.core/scalasrc/com/tle/web/api/search/SearchHelper.scala index 5c9f8f13c9..81b1373f77 100644 --- a/Source/Plugins/Core/com.equella.core/scalasrc/com/tle/web/api/search/SearchHelper.scala +++ b/Source/Plugins/Core/com.equella.core/scalasrc/com/tle/web/api/search/SearchHelper.scala @@ -278,6 +278,7 @@ object SearchHelper { beans.asScala // Filter out restricted attachments if the user does not have permissions to view them .filter(a => !a.isRestricted || hasRestrictedAttachmentPrivileges) + .map(sanitiseAttachmentBean) .map(att => { val broken = recurseBrokenAttachmentCheck( @@ -317,6 +318,21 @@ object SearchHelper { } } + /** + * Find out the latest version of the Item which a Custom Attachment points to. + * + * @param version Version of a linked Item. It is either 0 or 1 where 0 means using the latest version + * and 1 means always using version 1. + * @param uuid UUID of the linked Item. + */ + def getLatestVersionForCustomAttachment(version: Int, uuid: String): Int = { + version match { + // If version of is 0, find the real latest version of this Item. + case 0 => LegacyGuice.itemService.getLatestVersion(uuid) + case realVersion => realVersion + } + } + /** * Determines if a given customAttachment is invalid. Required as these attachments can be recursive. * @param customAttachment The attachment to check. @@ -324,14 +340,11 @@ object SearchHelper { */ def getBrokenAttachmentStatusForResourceAttachment( customAttachment: CustomAttachment): Boolean = { - val uuid = customAttachment.getData("uuid").asInstanceOf[String] - // If version of the linked Item is 0, find the latest version of this Item. - val version = customAttachment.getData("version").asInstanceOf[Int] match { - case 0 => LegacyGuice.itemService.getLatestVersion(uuid) - case realVersion => realVersion - } + val uuid = customAttachment.getData("uuid").asInstanceOf[String] + val version = customAttachment.getData("version").asInstanceOf[Int] + + val key = new ItemId(uuid, getLatestVersionForCustomAttachment(version, uuid)) - val key = new ItemId(uuid, version) if (customAttachment.getType != "resource") { return false; } @@ -438,4 +451,23 @@ object SearchHelper { */ def isLatestVersion(itemID: ItemIdKey): Boolean = itemID.getVersion == LegacyGuice.itemService.getLatestVersion(itemID.getUuid) + + /** + * When an AttachmentBean is converted to SearchResultAttachment, it may require some extra sanitising + * to complete the conversion. The sanitising work includes tasks listed below. + * + * 1. Help ResourceAttachmentBean check the version of its linked resource. + * + * @param att An AttachmentBean to be sanitised. + */ + def sanitiseAttachmentBean(att: AttachmentBean): AttachmentBean = { + att match { + case bean: ResourceAttachmentBean => + val latestVersion = + getLatestVersionForCustomAttachment(bean.getItemVersion, bean.getItemUuid) + bean.setItemVersion(latestVersion) + case _ => + } + att + } }