Skip to content

Commit

Permalink
#25296 include in 23.01.13
Browse files Browse the repository at this point in the history
  • Loading branch information
erickgonzalez committed Feb 28, 2024
1 parent d111898 commit 27ada1c
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,19 @@

import com.dotcms.IntegrationTestBase;
import com.dotcms.datagen.SiteDataGen;
import com.dotcms.datagen.UserDataGen;
import com.dotcms.util.IntegrationTestInitService;
import com.dotmarketing.beans.Host;
import com.dotmarketing.beans.Permission;
import com.dotmarketing.business.APILocator;
import com.dotmarketing.business.PermissionAPI;
import com.dotmarketing.exception.DotDataException;
import com.dotmarketing.exception.DotSecurityException;
import com.liferay.portal.model.User;
import org.junit.BeforeClass;
import org.junit.Test;

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

Expand Down Expand Up @@ -114,4 +118,37 @@ public void test_search_shouldReturnExactMatchesFirst() throws DotDataException,
assertTrue( "Test site is not contained in list", hostsList.get().contains(testSite));
assertEquals("Test site is not the first in the list", testSite, hostsList.get().get(0));
}

/**
* <ul>
* <li><b>Method to test: {@link HostFactoryImpl#search(String, String, boolean, int, int, User, boolean, List)}</li>
* <li><b>Given Scenario: Limited users should be able to create content types on System Host if given the proper permissions</b> </li>
* <li><b>Expected Result:The System host should appear in the list of host </b> </li>
* </ul>
* @throws Exception
*/
@Test
public void test_search_shouldIncludeSystemHost() throws DotDataException, DotSecurityException {

final PermissionAPI permissionAPI = APILocator.getPermissionAPI();
final HostFactoryImpl hostFactory = new HostFactoryImpl();
//Create limited user
final User limitedUser = new UserDataGen().nextPersisted();

//user has no permissions over the system host
Optional<List<Host>> hostsList = hostFactory.search("", "cvi.live_inode IS NOT NULL",true ,15, 0, limitedUser, false, new ArrayList<>());
assertTrue(hostsList.isPresent() && !hostsList.get().contains(APILocator.systemHost()));

//Give Permissions Over the System Host
final Permission permissions = new Permission(Host.class.getCanonicalName(),
APILocator.systemHost().getPermissionId(),
APILocator.getRoleAPI().loadRoleByKey(limitedUser.getUserId()).getId(),
PermissionAPI.PERMISSION_READ | PermissionAPI.PERMISSION_EDIT, true);
permissionAPI.save(permissions, APILocator.systemHost(), APILocator.getUserAPI().getSystemUser(), false);
//method to test
hostsList = hostFactory.search("", "cvi.live_inode IS NOT NULL",true ,15, 0, limitedUser, false, new ArrayList<>());
assertTrue(hostsList.isPresent() && hostsList.get().size() > 0);
//now the user has permissions over the system host
assertTrue(hostsList.get().contains(APILocator.systemHost()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -903,5 +903,19 @@ void permissionIndividuallyByRole(Permissionable parent, Permissionable permissi
*/
void checkPermission(Permissionable permissionable, PermissionLevel level, User user) throws DotSecurityException;


/**
*
* This method checks the inheritable permissions of the host, removes unnecessary roles,
* validates if the host has the minimum permissions required,
* and returns true or false depending on whether the condition is met.
*
* @param permissionable The Permissionable from which we want to obtain permissions
* @param user Logged in user
* @param respectFrontendRoles Determines if we need to remove the frontend roles
* @param expectedPermissionType The PERMISSION TYPE that must be contained by the role
*
* @return true or false, depending on if the host meets the expected permissions
* @throws DotDataException
*/
boolean doesSystemHostHavePermissions(Permissionable permissionable , User user, boolean respectFrontendRoles, String expectedPermissionType) throws DotDataException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -373,25 +373,7 @@ && workflowActionHasPermission(expecterPermissionType, p, user, contentlet)) {
return false;
}


final Set<Role> roles = new HashSet<>(Try
.of(() -> APILocator.getRoleAPI().loadRolesForUser(user.getUserId()))
.getOrElse(List.of()));



// remove front end user access for anon user (e.g, /intranet)
// Note to selves: it was a mistake to add this role to the Anon user in 5.2.0
if(user.isAnonymousUser()) {
roles.remove(frontEndUserRole);
}
if(!respectFrontendRoles) {
roles.remove(frontEndUserRole);
roles.remove(anonRole);
roles.remove(APILocator.getRoleAPI().loadRoleByKey("anonymous"));
}

final Set<String> userRoleIds = roles.stream().map(Role::getId).collect(Collectors.toSet());
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 @@ -1785,4 +1767,45 @@ public boolean isInheritingPermissions(Permissionable permissionable) throws Dot
return permissionFactory.isInheritingPermissions(permissionable);
}


/**
* Retrieves a filtered list of roles by removing front-end roles when unnecessary.
**/
private Set<Role> filterUserRoles(final User user, final boolean respectFrontendRoles) throws DotDataException {
final Role anonymousRoleRole = APILocator.getRoleAPI().loadCMSAnonymousRole();
final Role frontEndUserRole = APILocator.getRoleAPI().loadLoggedinSiteRole();

final Set<Role> roles = new HashSet<>(Try
.of(() -> APILocator.getRoleAPI().loadRolesForUser(user.getUserId()))
.getOrElse(List.of()));

// remove front end user access for anon user (e.g, /intranet)
// Note to selves: it was a mistake to add this role to the Anon user in 5.2.0
if (user.isAnonymousUser()) {
roles.remove(frontEndUserRole);
}
if (!respectFrontendRoles) {
roles.remove(frontEndUserRole);
roles.remove(anonymousRoleRole);
roles.remove(APILocator.getRoleAPI().loadRoleByKey("anonymous"));
}

return roles;
}

@Override
@CloseDBIfOpened
public boolean doesSystemHostHavePermissions(final Permissionable systemHost, final User user, final boolean respectFrontendRoles, final String expectedPermissionType) throws DotDataException {
final List<Permission> systemHostPermissions = getInheritablePermissions(systemHost);

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

for(final Permission perm : systemHostPermissions){
if(perm.getType().equals(expectedPermissionType) && userRoleIds.contains(perm.getRoleId()) ){
return true;
}
}

return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import com.dotmarketing.util.Logger;
import com.dotmarketing.util.UtilMethods;
import com.dotmarketing.util.WebKeys;
import com.google.common.base.Strings;
import com.liferay.portal.PortalException;
import com.liferay.portal.SystemException;
import com.liferay.portal.language.LanguageException;
Expand All @@ -69,16 +70,7 @@
import com.liferay.util.StringPool;
import io.vavr.control.Try;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.*;
import java.util.stream.Collectors;
import javax.servlet.ServletContext;
import javax.servlet.ServletException;
Expand Down Expand Up @@ -2113,9 +2105,19 @@ public List<Map<String, Object>> getHosts() throws PortalException, SystemExcept
boolean respectFrontendRoles = userWebAPI.isLoggedToFrontend(ctx.getHttpServletRequest());
HostAPI hostAPI = APILocator.getHostAPI();
List<Host> hosts = hostAPI.findAll(user, respectFrontendRoles);
// Remove invalid hosts, before sorting the list
hosts.removeIf(host -> Objects.isNull(host) || Strings.isNullOrEmpty(host.getHostname()));
List<Map<String, Object>> hostsToReturn = new ArrayList<Map<String,Object>>(hosts.size());
Collections.sort(hosts, new HostNameComparator());
for (Host h: hosts) {
/**
* When we created the provided host list, we already validated the user's permission over the system host.
* Therefore, there is no need to validate it again.
**/
if (h.isSystemHost()){
hostsToReturn.add(hostMap(h));
continue;
}
List permissions = new ArrayList();
try {
permissions = permissionAPI.getPermissionIdsFromRoles(h, roles, user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,11 @@ private List<Host> findPaginatedSitesFromDB(final User user, final int limit, fi
if (null != siteList && !siteList.isEmpty()) {
return siteList.stream().filter(site -> {
try {
if (site.isSystemHost() && !user.isAdmin()){
return APILocator.getPermissionAPI()
.doesSystemHostHavePermissions(APILocator.systemHost(), user,
respectFrontendRoles, Host.class.getCanonicalName());
}
checkSitePermission(user, respectFrontendRoles, site);
return true;
} catch (final DotDataException | DotSecurityException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,16 @@ protected Optional<List<Host>> search(final String siteNameFilter, final String
hostList.addAll(APILocator.getPermissionAPI().filterCollection(siteList, PermissionAPI
.PERMISSION_READ, respectFrontendRoles, user));
hostList = hostList.stream().limit(limit).collect(Collectors.toList());
//We want to include the system host in the list of hosts
final Host systemHost = APILocator.systemHost();
//Check if we need to include it, if we have the permissions and if it is in the list.
final boolean sysHostPermission = showSystemHost
&& !hostList.contains(systemHost)
&& APILocator.getPermissionAPI().doesSystemHostHavePermissions(systemHost, user, respectFrontendRoles, Host.class.getCanonicalName());

if(sysHostPermission){
hostList.add(systemHost);
}
if(hostList.size()==limit || siteList.size() < limit){//user is admin or reach the amount of sites requested or there is no anymore sites
return Optional.of(hostList);
}else{
Expand Down
2 changes: 2 additions & 0 deletions hotfix_tracking.md
Original file line number Diff line number Diff line change
Expand Up @@ -203,3 +203,5 @@ This maintenance release includes the following code fixes:
164. https://github.com/dotCMS/core/issues/26605 : Relation_type field too small #26605

**Release-23.01.13**

165. https://github.com/dotCMS/core/issues/25296 : Limited users cannot create content types on System Host #25296

0 comments on commit 27ada1c

Please sign in to comment.