diff --git a/api/src/main/java/org/apache/cloudstack/acl/APIChecker.java b/api/src/main/java/org/apache/cloudstack/acl/APIChecker.java index 660f64f43ef2..e03b1c536ef8 100644 --- a/api/src/main/java/org/apache/cloudstack/acl/APIChecker.java +++ b/api/src/main/java/org/apache/cloudstack/acl/APIChecker.java @@ -16,13 +16,13 @@ // under the License. package org.apache.cloudstack.acl; +import java.util.List; + import com.cloud.exception.PermissionDeniedException; import com.cloud.user.Account; import com.cloud.user.User; import com.cloud.utils.component.Adapter; -import java.util.List; - /** * APICheckers is designed to verify the ownership of resources and to control the access to APIs. */ @@ -43,4 +43,8 @@ public interface APIChecker extends Adapter { */ List getApisAllowedToUser(Role role, User user, List apiNames) throws PermissionDeniedException; boolean isEnabled(); + + default boolean isProjectRoleBasedChecker() { + return false; + } } diff --git a/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java b/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java index 0306a062df98..8bdd57bd2ab3 100644 --- a/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java +++ b/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java @@ -21,8 +21,8 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; -import org.apache.cloudstack.acl.RolePermissionEntity.Permission; +import org.apache.cloudstack.acl.RolePermissionEntity.Permission; import org.apache.cloudstack.context.CallContext; import org.apache.log4j.Logger; @@ -197,4 +197,9 @@ public List getServices() { public void setServices(List services) { this.services = services; } + + @Override + public boolean isProjectRoleBasedChecker() { + return true; + } } diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index 76a436f74d42..c94b02ccb782 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -47,6 +47,7 @@ import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; @@ -1223,6 +1224,27 @@ public boolean verifyUser(final Long userId) { return true; } + protected boolean checkCommandAccessWithCheckers(final User user, final String commandName) throws + PermissionDeniedException { + List accessCheckers = apiAccessCheckers; + if (CallContext.current().getProject() != null) { + APIChecker projectRoleChecker = apiAccessCheckers.stream() + .filter(APIChecker::isProjectRoleBasedChecker).findFirst().orElse(null); + if (projectRoleChecker != null) { + if (projectRoleChecker.checkAccess(user, commandName)) { + return true; + } + accessCheckers = apiAccessCheckers.stream() + .filter(c -> !c.isProjectRoleBasedChecker()) + .collect(Collectors.toList()); + } + } + for (final APIChecker apiChecker : accessCheckers) { + apiChecker.checkAccess(user, commandName); + } + return true; + } + private void checkCommandAvailable(final User user, final String commandName, final InetAddress remoteAddress) throws PermissionDeniedException { if (user == null) { throw new PermissionDeniedException("User is null for role based API access check for command" + commandName); @@ -1239,11 +1261,7 @@ private void checkCommandAvailable(final User user, final String commandName, fi throw new OriginDeniedException("Calls from disallowed origin", account, remoteAddress); } } - - - for (final APIChecker apiChecker : apiAccessCheckers) { - apiChecker.checkAccess(user, commandName); - } + checkCommandAccessWithCheckers(user, commandName); } @Override diff --git a/server/src/test/java/com/cloud/api/ApiServerTest.java b/server/src/test/java/com/cloud/api/ApiServerTest.java new file mode 100644 index 000000000000..c0d65e315ebc --- /dev/null +++ b/server/src/test/java/com/cloud/api/ApiServerTest.java @@ -0,0 +1,115 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.api; + +import java.util.ArrayList; +import java.util.List; + +import org.apache.cloudstack.acl.APIChecker; +import org.apache.cloudstack.context.CallContext; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; +import org.springframework.test.util.ReflectionTestUtils; + +import com.cloud.exception.PermissionDeniedException; +import com.cloud.projects.Project; +import com.cloud.user.User; + +@RunWith(MockitoJUnitRunner.class) +public class ApiServerTest { + + @InjectMocks + ApiServer apiServer = new ApiServer(); + + private static final String PROJECT_ACCESSIBLE_CMD_NAME = "projectcommand"; + private static final String PROJECT_INACCESSIBLE_CMD_NAME = "noprojectcommand"; + private static final String ACCESSIBLE_CMD_NAME = "command"; + private static final String INACCESSIBLE_CMD_NAME = "nocommand"; + private User user = Mockito.mock(User.class); + private CallContext callContext = Mockito.mock(CallContext.class); + MockedStatic mockedCallContext; + + @Before + public void setup() throws Exception { + mockedCallContext = Mockito.mockStatic(CallContext.class); + mockedCallContext.when(CallContext::current).thenReturn(callContext); + List apiCheckers = new ArrayList<>(); + APIChecker checker1 = Mockito.mock(APIChecker.class); + apiCheckers.add(checker1); + APIChecker checker2 = Mockito.mock(APIChecker.class); + Mockito.when(checker2.isProjectRoleBasedChecker()) + .thenReturn(true); + apiCheckers.add(checker2); + Mockito.doAnswer((Answer) invocation -> { + String cmd = (String) invocation.getArguments()[1]; + if (!cmd.equals(ACCESSIBLE_CMD_NAME)) { + throw new PermissionDeniedException("Denied"); + } + return true; + }).when(checker1).checkAccess(Mockito.any(User.class), Mockito.anyString()); + Mockito.doAnswer((Answer) invocation -> { + String cmd = (String) invocation.getArguments()[1]; + if (List.of(PROJECT_ACCESSIBLE_CMD_NAME, ACCESSIBLE_CMD_NAME).contains(cmd)) { + return true; + } else if (cmd.equals(PROJECT_INACCESSIBLE_CMD_NAME)) { + return false; + } + throw new PermissionDeniedException("Denied"); + }).when(checker2).checkAccess(Mockito.any(User.class), Mockito.anyString()); + ReflectionTestUtils.setField(apiServer, "apiAccessCheckers", apiCheckers); + } + + @After + public void tearDown() throws Exception { + mockedCallContext.close(); + } + + @Test + public void testCheckCommandAccessWithCheckersProjectCommand() { + Mockito.when(callContext.getProject()).thenReturn(Mockito.mock(Project.class)); + Assert.assertTrue(apiServer.checkCommandAccessWithCheckers(user, PROJECT_ACCESSIBLE_CMD_NAME)); + } + + @Test(expected = PermissionDeniedException.class) + public void testCheckCommandAccessWithCheckersInaccessibleProjectCommand() { + Mockito.when(callContext.getProject()).thenReturn(Mockito.mock(Project.class)); + apiServer.checkCommandAccessWithCheckers(user, PROJECT_INACCESSIBLE_CMD_NAME); + } + + @Test(expected = PermissionDeniedException.class) + public void testCheckCommandAccessWithCheckersProjectCommandWithoutProject() { + apiServer.checkCommandAccessWithCheckers(user, PROJECT_INACCESSIBLE_CMD_NAME); + } + + @Test + public void testCheckCommandAccessWithCheckersAccessibleCommand() { + Assert.assertTrue(apiServer.checkCommandAccessWithCheckers(user, ACCESSIBLE_CMD_NAME)); + } + + @Test(expected = PermissionDeniedException.class) + public void testCheckCommandAccessWithCheckersInaccessibleCommand() { + apiServer.checkCommandAccessWithCheckers(user, INACCESSIBLE_CMD_NAME); + } +}