Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue 28230 short lived permission cache2 #28412

Merged
merged 19 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
4884c38
spike(performance) try a short lived cache on top of doesUserHavePerm…
wezell Apr 15, 2024
aa4ec20
Merge branch 'master' into issue-28230-short-lived-permission-cache
wezell Apr 15, 2024
94bdc10
spike(performance) adding null checks because... we do nulls :( ref:#…
wezell Apr 15, 2024
e0af595
spike(performance) adding null checks because... we do nulls :( ref:#…
wezell Apr 15, 2024
8db653c
Merge branch 'issue-28230-short-lived-permission-cache' of github.com…
wezell Apr 15, 2024
844fd01
Merge branch 'master' into issue-28230-short-lived-permission-cache
wezell Apr 24, 2024
ceb8fd7
#28230 caching permissions
wezell Apr 25, 2024
3109421
Merge remote-tracking branch 'origin/master' into issue-28230-short-l…
wezell Apr 29, 2024
ffd4fbc
#28230 short lived permission cache for read permissions
wezell May 2, 2024
4fad6e9
Merge branch 'master' into issue-28230-short-lived-permission-cache2
wezell May 2, 2024
415e4f9
#28230 short lived permission cache for read permissions
wezell May 2, 2024
6c40f03
Merge branch 'issue-28230-short-lived-permission-cache2' of github.co…
wezell May 2, 2024
31dc34d
Merge branch 'master' into issue-28230-short-lived-permission-cache2
wezell May 2, 2024
9a65c4a
task(docker-compose) deleting unused docker-compose files
wezell May 2, 2024
01c6163
fix(perf) permissions short lived cache - fixing tests
wezell May 2, 2024
a2c04b0
erge branch 'issue-28230-short-lived-permission-cache2' of github.com…
wezell May 2, 2024
bc082c0
fix(perf) #28230 short lived permission cache for read permissions
wezell May 2, 2024
65cc0c3
Merge branch 'master' into issue-28230-short-lived-permission-cache2
wezell May 3, 2024
33a44a4
Merge branch 'master' into issue-28230-short-lived-permission-cache2
wezell May 5, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 0 additions & 76 deletions docker-compose.yml
wezell marked this conversation as resolved.
Show resolved Hide resolved

This file was deleted.

208 changes: 123 additions & 85 deletions dotCMS/src/main/java/com/dotmarketing/business/PermissionBitAPIImpl.java
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
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
Loading