Skip to content

Commit

Permalink
issue 28230 short lived permission cache2 (#28412)
Browse files Browse the repository at this point in the history 
* spike(performance) try a short lived cache on top of doesUserHavePermission

ref: #28230

* spike(performance) adding null checks because... we do nulls :( ref:#28230

* spike(performance) adding null checks because... we do nulls :( ref:#28230

* #28230 caching permissions

* #28230 short lived permission cache for read permissions

* #28230 short lived permission cache for read permissions

* task(docker-compose) deleting unused docker-compose files

* fix(perf) permissions short lived cache - fixing tests

* fix(perf) #28230 short lived permission cache for read permissions
  • Loading branch information
@wezell
wezell authored May 6, 2024
1 parent b1d9465 commit 5921e6a
Show file tree
Hide file tree
Showing 10 changed files with 313 additions and 246 deletions.
76 changes: 0 additions & 76 deletions docker-compose.yml
View file

This file was deleted.

208 changes: 123 additions & 85 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,114 +277,157 @@ 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,
final User userIn,
@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;
}

if (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;
}

// short circuit for UserProxy
if (permissionable instanceof UserProxy) {
return userPermissions((UserProxy) permissionable, user);
}
final User user = (userIn==null || userIn.getUserId()==null) ? APILocator.getUserAPI().getAnonymousUser() : userIn;
if (user.getUserId().equals(APILocator.systemUser().getUserId())){
return true;
}

if (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;
}

// short circuit for UserProxy
if (permissionable instanceof UserProxy) {
return userPermissions((UserProxy) permissionable, user);
}

// Folders do not have PUBLISH, use EDIT instead
final int expecterPermissionType = (PermissionableType
.FOLDERS
.getCanonicalName()
.equals(permissionable.getPermissionType()) ||
.FOLDERS
.getCanonicalName()
.equals(permissionable.getPermissionType()) ||
permissionable instanceof Identifier && ((Identifier) permissionable).getAssetType().equals("folder"))
&& permissionType == PERMISSION_PUBLISH
? PERMISSION_EDIT
: permissionType;
? 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))) {
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 (respectFrontendRoles) {
//anonymous role should not be able to access non-live contentlet
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(ownerRole.get().getId())
&& permissionable.getOwner() != null
&& permissionable.getOwner().equals(user.getUserId())
&& checkRelatedPermissions(permissionable.permissionDependencies(expecterPermissionType), user)) {
return true;
}
} catch (DotDataException e1) {
Logger.error(this, e1.getMessage(), e1);
throw new DotRuntimeException(e1.getMessage(), e1);
return true;
}
} catch (DotDataException e1) {
Logger.error(this, e1.getMessage(), e1);
throw new DotRuntimeException(e1.getMessage(), e1);
}

if (permissionable instanceof WorkflowAction
if (permissionable instanceof WorkflowAction
&& workflowActionHasPermission(expecterPermissionType, p, user, contentlet)) {
return true;
}
return true;
}
}

// front end users cannot read content that is not live
if (!user.isBackendUser()
&& isContentlet
&& !isLiveContentlet(permissionable)
&& expecterPermissionType == PERMISSION_READ) {
Logger.warn(this, String.format("User '%s' cannot verify READ permissions on Contentlet '%s' because it " +
"is not live.", user.getUserId(), permissionable.getPermissionId()));
return false;
}
// front end users cannot read content that is not live
if (!user.isBackendUser()
&& isContentlet
&& !isLiveContentlet(permissionable)
&& expecterPermissionType == PERMISSION_READ) {
Logger.warn(this, String.format("User '%s' cannot verify READ permissions on Contentlet '%s' because it " +
"is not live.", user.getUserId(), permissionable.getPermissionId()));
return false;
}

final Set<String> userRoleIds = filterUserRoles(user, respectFrontendRoles).stream().map(Role::getId).collect(Collectors.toSet());

return doRolesHavePermission(userRoleIds, getPermissions(permissionable, true), expecterPermissionType);





}

/**
Expand Down Expand Up @@ -443,14 +488,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();
cmsAnonymousPermission.setRoleId(cmsAnonymousRole.getId());
Expand Down
25 changes: 24 additions & 1 deletion 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 com.dotmarketing.exception.DotDataException;
import com.dotmarketing.portlets.contentlet.model.Contentlet;
import com.liferay.portal.model.User;
import java.util.List;

import com.dotmarketing.beans.Permission;
import java.util.Optional;
import javax.validation.constraints.NotNull;

//This interface should have default package access
public abstract class PermissionCache implements Cachable{
Expand All @@ -16,4 +21,22 @@ 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 5921e6a

Please sign in to comment.