Skip to content

Commit

Permalink
#28230 include in 23.10.24
Browse files Browse the repository at this point in the history
  • Loading branch information
@erickgonzalez
erickgonzalez committed May 3, 2024
1 parent dfd8122 commit c4eae1e
Show file tree
Hide file tree
Showing 7 changed files with 187 additions and 39 deletions.
3 changes: 2 additions & 1 deletion dotCMS/hotfix_tracking.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,5 @@ This maintenance release includes the following code fixes:
90. https://github.com/dotCMS/core/issues/27361 : CSV Content import cannot process host or folder information #27361
91. https://github.com/dotCMS/core/issues/26582 : [Site Browser] : Open folders get collapsed after moving away from portlet #26582
92. https://github.com/dotCMS/core/issues/25903 : Key/Value field escaping colon and comma characters to HTML encoded version. #25903
93. https://github.com/dotCMS/core/issues/24698 : Cannot push publish to S3 buckets in the us-east-2 region #24698
93. https://github.com/dotCMS/core/issues/24698 : Cannot push publish to S3 buckets in the us-east-2 region #24698
94. https://github.com/dotCMS/core/issues/28230 : spike(performance): test short lived permission cache #28230
19 changes: 13 additions & 6 deletions dotCMS/src/integration-test/java/com/dotmarketing/business/PermissionAPIIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.dotmarketing.portlets.structure.model.Field;
import com.dotmarketing.portlets.structure.model.Structure;
import com.dotmarketing.portlets.templates.model.Template;
import com.dotmarketing.util.Config;
import com.dotmarketing.util.Logger;
import com.google.common.collect.Lists;
import com.liferay.portal.model.User;
Expand Down Expand Up @@ -61,6 +62,7 @@ public class PermissionAPIIntegrationTest extends IntegrationTestBase {
private static Host host;
private static User systemUser;
private static Template template;
private static int permissionCacheSize = 0;

@BeforeClass
public static void createTestHost() throws Exception {
Expand All @@ -75,7 +77,7 @@ public static void createTestHost() throws Exception {
userAPI = APILocator.getUserAPI();

host = new Host();
host.setHostname("testhost.demo.dotcms.com");
host.setHostname(System.currentTimeMillis() + "testhost.demo.dotcms.com");
try{
HibernateUtil.startTransaction();
host=hostAPI.save(host, systemUser, false);
Expand All @@ -94,6 +96,9 @@ public static void createTestHost() throws Exception {
APILocator.getTemplateAPI().saveTemplate(template, host, systemUser, false);
Map<String, Object> sessionAttrs = new HashMap<>();
sessionAttrs.put("USER_ID", "dotcms.org.1");
permissionCacheSize = Config.getIntProperty("cache.permissionshortlived.size", 0);
Config.setProperty("cache.permissionshortlived.size", 0);
CacheLocator.getPermissionCache().flushShortTermCache();
}

@AfterClass
Expand All @@ -107,6 +112,8 @@ public static void deleteTestHost() throws DotDataException {
HibernateUtil.rollbackTransaction();
Logger.error(PermissionAPIIntegrationTest.class, e.getMessage());
}
Config.setProperty("cache.permissionshortlived.size", 0);
CacheLocator.getPermissionCache().flushShortTermCache();
}

@Test
Expand Down Expand Up @@ -277,7 +284,7 @@ public void test_permissionIndividuallyByRole() {

try {
// Create test Host.
host.setHostname("permission.dotcms.com");
host.setHostname( System.currentTimeMillis() + "-permission.dotcms.com");
host = hostAPI.save(host, systemUser, false);

// Create the Folders needed.
Expand All @@ -289,30 +296,30 @@ public void test_permissionIndividuallyByRole() {
// Child: Product Publisher
// Grandchild: Product Contributor
// Create Parent Role.
parentRole.setName("Webmaster-Test");
parentRole.setName(System.currentTimeMillis() + "-Webmaster-Test");
parentRole.setEditUsers(true);
parentRole.setEditPermissions(true);
parentRole.setEditLayouts(true);
parentRole = roleAPI.save(parentRole);

// Create Child Role Role.
childRole.setName("Product Contributor-Test");
childRole.setName(System.currentTimeMillis() + "-Product Contributor-Test");
childRole.setEditUsers(true);
childRole.setEditPermissions(true);
childRole.setEditLayouts(true);
childRole.setParent(parentRole.getId());
childRole = roleAPI.save(childRole);

// Create Grandchild Role Role.
grandChildRole.setName("Product Contributor-Test");
grandChildRole.setName(System.currentTimeMillis() + "-Product Contributor-Test");
grandChildRole.setEditUsers(true);
grandChildRole.setEditPermissions(true);
grandChildRole.setEditLayouts(true);
grandChildRole.setParent(childRole.getId());
grandChildRole = roleAPI.save(grandChildRole);

// Now lets create a user, assign the role and test basic permissions.
newUser = userAPI.createUser("[email protected]", "[email protected]");
newUser = userAPI.createUser("[email protected]", System.currentTimeMillis() + "-[email protected]");
newUser.setFirstName("Test-11962");
newUser.setLastName("User-11962");
userAPI.save(newUser, systemUser, false);
Expand Down
12 changes: 9 additions & 3 deletions dotCMS/src/integration-test/java/com/dotmarketing/business/PermissionAPITest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ public class PermissionAPITest extends IntegrationTestBase {
private static Host host;
private static User sysuser;
private static Template template;
private static int permissionCacheSize = 0;

@BeforeClass
public static void createTestHost() throws Exception {
Expand All @@ -91,14 +92,14 @@ public static void createTestHost() throws Exception {
permissionAPI =APILocator.getPermissionAPI();
sysuser=APILocator.getUserAPI().getSystemUser();
host = new Host();
host.setHostname("testhost.demo.dotcms.com");
host.setHostname(System.currentTimeMillis() + "testhost.demo.dotcms.com");
try{
HibernateUtil.startTransaction();
host=APILocator.getHostAPI().save(host, sysuser, false);
HibernateUtil.closeAndCommitTransaction();
}catch(Exception e){
HibernateUtil.rollbackTransaction();
host = APILocator.getHostAPI().findByName("testhost.demo.dotcms.com", sysuser, false);
host = APILocator.getHostAPI().findByName(host.getHostname(), sysuser, false);
Logger.error(PermissionAPITest.class, e.getMessage());
} finally {
HibernateUtil.closeSessionSilently();
Expand All @@ -115,6 +116,10 @@ public static void createTestHost() throws Exception {
template.setTitle("testtemplate");
template.setBody("<html><head></head><body>en empty template just for test</body></html>");
APILocator.getTemplateAPI().saveTemplate(template, host, sysuser, false);

permissionCacheSize = Config.getIntProperty("cache.permissionshortlived.size", 0);
Config.setProperty("cache.permissionshortlived.size", 0);
CacheLocator.getPermissionCache().flushShortTermCache();
}

@AfterClass
Expand All @@ -130,7 +135,8 @@ public static void deleteTestHost() throws DotContentletStateException, DotDataE
}finally {
HibernateUtil.closeSessionSilently();
}

Config.setProperty("cache.permissionshortlived.size", permissionCacheSize);
CacheLocator.getPermissionCache().flushShortTermCache();
}

@Test
Expand Down
86 changes: 60 additions & 26 deletions dotCMS/src/main/java/com/dotmarketing/business/PermissionBitAPIImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@
import com.liferay.portal.NoSuchRoleException;
import com.liferay.portal.model.User;
import com.liferay.portal.util.PortalUtil;
import io.vavr.Lazy;
import io.vavr.control.Try;
import javax.validation.constraints.NotNull;
import org.apache.commons.lang3.StringUtils;

import java.util.ArrayList;
Expand Down Expand Up @@ -275,13 +277,63 @@ public boolean doesUserHavePermission(final Permissionable permissionable,
return doesUserHavePermission(permissionable, permissionType, userIn, respectFrontendRoles, null);
}


@CloseDBIfOpened
@Override
public boolean doesUserHavePermission(final Permissionable permissionable,
final int permissionType,
final User userIn,
final boolean respectFrontendRoles,
final Contentlet contentlet) throws DotDataException {



final User user = (userIn==null || userIn.getUserId()==null) ? APILocator.getUserAPI().getAnonymousUser() : userIn;
if (user.getUserId().equals(APILocator.systemUser().getUserId()) || user.isAdmin()){
return true;
}


if (permissionable == null) {
Logger.warn(this, "Permissionable object is null");
throw new NullPointerException("Permissionable object is null");
}

if (UtilMethods.isEmpty(permissionable.getPermissionId())) {
Logger.debug(
this.getClass(),
"Trying to get permissions on null inode of type :" + permissionable.getPermissionType()) ;
Logger.debug(
this.getClass(),
"Trying to get permissions on null inode of class :" + permissionable.getClass()) ;
return false;
}

Optional<Boolean> cachedPermission = CacheLocator.getPermissionCache().doesUserHavePermission(permissionable, String.valueOf(permissionType), user, respectFrontendRoles, contentlet);
if (cachedPermission.isPresent() ) {
return cachedPermission.get();
}

boolean hasPermission = doesUserHavePermissionInternal(permissionable, permissionType, user, respectFrontendRoles, contentlet);

CacheLocator.getPermissionCache().putUserHavePermission(permissionable, String.valueOf(permissionType), user, respectFrontendRoles, contentlet, hasPermission);
return hasPermission;

}

private final Lazy<Role> anonRole = Lazy.of(()->Try.of(()->APILocator.getRoleAPI().loadCMSAnonymousRole()).getOrElseThrow(DotRuntimeException::new));
private final Lazy<Role> adminRole = Lazy.of(()->Try.of(()->APILocator.getRoleAPI().loadCMSAdminRole()).getOrElseThrow(DotRuntimeException::new));
private final Lazy<Role> ownerRole = Lazy.of(()->Try.of(()->APILocator.getRoleAPI().loadCMSOwnerRole()).getOrElseThrow(DotRuntimeException::new));



@CloseDBIfOpened
public boolean doesUserHavePermissionInternal(final Permissionable permissionable,
final int permissionType,
@NotNull final User userIn,
final boolean respectFrontendRoles,
final Contentlet contentlet) throws DotDataException {

final User user = (userIn==null || userIn.getUserId()==null) ? APILocator.getUserAPI().getAnonymousUser() : userIn;
if (user.getUserId().equals(APILocator.systemUser().getUserId())){
return true;
Expand Down Expand Up @@ -321,38 +373,27 @@ public boolean doesUserHavePermission(final Permissionable permissionable,
? PERMISSION_EDIT
: permissionType;

Role adminRole;
Role anonRole;
Role frontEndUserRole;
Role cmsOwnerRole;
try {
adminRole = APILocator.getRoleAPI().loadCMSAdminRole();
anonRole = APILocator.getRoleAPI().loadCMSAnonymousRole();
frontEndUserRole = APILocator.getRoleAPI().loadLoggedinSiteRole();
cmsOwnerRole = APILocator.getRoleAPI().loadCMSOwnerRole();
} catch (DotDataException e1) {
Logger.error(this, e1.getMessage(), e1);
throw new DotRuntimeException(e1.getMessage(), e1);
}

if (APILocator.getRoleAPI().doesUserHaveRole(user, adminRole)) {
if (APILocator.getRoleAPI().doesUserHaveRole(user, adminRole.get())) {
return true;
}

final List<Permission> perms = getPermissions(permissionable, true);
final List<Permission> perms = getPermissions(permissionable, true)
.stream()
.filter(p-> p.matchesPermission(expecterPermissionType))
.collect(Collectors.toList());
final boolean isContentlet = permissionable instanceof Contentlet;
for(final Permission p : perms){
if (p.matchesPermission(expecterPermissionType)) {
if (respectFrontendRoles) {
//anonymous role should not be able to access non-live contentlet
if (p.getRoleId().equals(anonRole.getId()) && (!isContentlet || isLiveContentlet(permissionable))) {
if (p.getRoleId().equals(anonRole.get().getId()) && (!isContentlet || isLiveContentlet(permissionable))) {
return true;
//if logged in site user has permission
}
}
// if owner and owner has required permission return true
try {
if (p.getRoleId().equals(cmsOwnerRole.getId())
if (p.getRoleId().equals(ownerRole.get().getId())
&& permissionable.getOwner() != null
&& permissionable.getOwner().equals(user.getUserId())
&& checkRelatedPermissions(permissionable.permissionDependencies(expecterPermissionType), user)) {
Expand All @@ -368,7 +409,6 @@ && workflowActionHasPermission(expecterPermissionType, p, user, contentlet)) {
return true;
}
}
}

// front end users cannot read content that is not live
if (!user.isBackendUser()
Expand Down Expand Up @@ -447,13 +487,7 @@ public void removePermissions(Permissionable permissionable) throws DotDataExcep
@WrapInTransaction
@Override
public void setDefaultCMSAnonymousPermissions(Permissionable permissionable) throws DotDataException{
Role cmsAnonymousRole;
try {
cmsAnonymousRole = APILocator.getRoleAPI().loadCMSAnonymousRole();
} catch (DotDataException e1) {
Logger.error(this, e1.getMessage(), e1);
throw new DotRuntimeException(e1.getMessage(), e1);
}
Role cmsAnonymousRole= anonRole.get();


Permission cmsAnonymousPermission = new Permission();
Expand Down
21 changes: 21 additions & 0 deletions dotCMS/src/main/java/com/dotmarketing/business/PermissionCache.java
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
package com.dotmarketing.business;

import java.util.List;
import java.util.Optional;

import com.dotmarketing.beans.Permission;
import com.dotmarketing.portlets.contentlet.model.Contentlet;
import com.liferay.portal.model.User;

import javax.validation.constraints.NotNull;

//This interface should have default package access
public abstract class PermissionCache implements Cachable{
Expand All @@ -16,4 +21,20 @@ abstract protected List<Permission> addToPermissionCache(String key,

abstract protected void remove(String key);

public abstract Optional<Boolean> doesUserHavePermission(Permissionable permissionable,
String permissionType,
User userIn,
boolean respectFrontendRoles,
Contentlet nullableContent) ;

public abstract void putUserHavePermission(@NotNull Permissionable permissionable,
String permissionType,
@NotNull User userIn,
boolean respectFrontendRoles,
@NotNull Contentlet nullableContent,
boolean hasPermission) ;



public abstract void flushShortTermCache() ;
}
Loading

0 comments on commit c4eae1e

Please sign in to comment.