Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions api/src/main/java/com/cloud/user/AccountService.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.api.command.admin.account.CreateAccountCmd;
import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd;
import org.apache.cloudstack.api.command.admin.user.RegisterCmd;
import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd;
Expand All @@ -39,8 +40,7 @@ public interface AccountService {
* Creates a new user and account, stores the password as is so encrypted passwords are recommended.
* @return the user if created successfully, null otherwise
*/
UserAccount createUserAccount(String userName, String password, String firstName, String lastName, String email, String timezone, String accountName, short accountType, Long roleId, Long domainId,
String networkDomain, Map<String, String> details, String accountUUID, String userUUID);
UserAccount createUserAccount(CreateAccountCmd accountCmd);

UserAccount createUserAccount(String userName, String password, String firstName, String lastName, String email, String timezone, String accountName, short accountType, Long roleId, Long domainId,
String networkDomain, Map<String, String> details, String accountUUID, String userUUID, User.Source source);
Expand Down
3 changes: 3 additions & 0 deletions api/src/main/java/org/apache/cloudstack/acl/APIChecker.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.apache.cloudstack.acl;

import com.cloud.exception.PermissionDeniedException;
import com.cloud.user.Account;
import com.cloud.user.User;
import com.cloud.utils.component.Adapter;

Expand All @@ -27,4 +28,6 @@ public interface APIChecker extends Adapter {
// If false, apiChecker is unable to handle the operation or not implemented
// On exception, checkAccess failed don't allow
boolean checkAccess(User user, String apiCommandName) throws PermissionDeniedException;
boolean checkAccess(Account account, String apiCommandName) throws PermissionDeniedException;
boolean isEnabled();
}
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,7 @@ public void execute() {
validateParams();
CallContext.current().setEventDetails("Account Name: " + getUsername() + ", Domain Id:" + getDomainId());
UserAccount userAccount =
_accountService.createUserAccount(getUsername(), getPassword(), getFirstName(), getLastName(), getEmail(), getTimeZone(), getAccountName(), getAccountType(), getRoleId(),
getDomainId(), getNetworkDomain(), getDetails(), getAccountUUID(), getUserUUID());
_accountService.createUserAccount(this);
if (userAccount != null) {
AccountResponse response = _responseGenerator.createUserAccountResponse(ResponseView.Full, userAccount);
response.setResponseName(getCommandName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void testExecuteWithNotBlankPassword() {
} catch (ServerApiException e) {
Assert.assertTrue("Received exception as the mock accountService createUserAccount returns null user", true);
}
Mockito.verify(accountService, Mockito.times(1)).createUserAccount(null, "Test", null, null, null, null, null, accountType, roleId, domainId, null, null, null, null);
Mockito.verify(accountService, Mockito.times(1)).createUserAccount(createAccountCmd);
Comment thread
DaanHoogland marked this conversation as resolved.
}

@Test
Expand All @@ -86,7 +86,7 @@ public void testExecuteWithNullPassword() {
Assert.assertEquals(ApiErrorCode.PARAM_ERROR, e.getErrorCode());
Assert.assertEquals("Empty passwords are not allowed", e.getMessage());
}
Mockito.verify(accountService, Mockito.never()).createUserAccount(null, null, null, null, null, null, null, accountType, roleId, domainId, null, null, null, null);
Mockito.verify(accountService, Mockito.never()).createUserAccount(createAccountCmd);
}

@Test
Expand All @@ -99,6 +99,6 @@ public void testExecuteWithEmptyPassword() {
Assert.assertEquals(ApiErrorCode.PARAM_ERROR, e.getErrorCode());
Assert.assertEquals("Empty passwords are not allowed", e.getMessage());
}
Mockito.verify(accountService, Mockito.never()).createUserAccount(null, null, null, null, null, null, null, accountType, roleId, domainId, null, null, null, null);
Mockito.verify(accountService, Mockito.never()).createUserAccount(createAccountCmd);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ public boolean checkAccess(User user, String commandName) throws PermissionDenie
throw new PermissionDeniedException("The account id=" + user.getAccountId() + "for user id=" + user.getId() + "is null");
}

return checkAccess(account, commandName);
}

public boolean checkAccess(Account account, String commandName) {
final Role accountRole = roleService.findRole(account.getRoleId());
if (accountRole == null || accountRole.getId() < 1L) {
denyApiAccess(commandName);
Expand Down Expand Up @@ -106,6 +110,11 @@ public boolean checkAccess(User user, String commandName) throws PermissionDenie
throw new UnavailableCommandException("The API " + commandName + " does not exist or is not available for this account.");
}

@Override
public boolean isEnabled() {
return roleService.isEnabled();
}

public void addApiToRoleBasedAnnotationsMap(final RoleType roleType, final String commandName) {
if (roleType == null || Strings.isNullOrEmpty(commandName)) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,13 @@ private void denyApiAccess(final String commandName) throws PermissionDeniedExce
throw new PermissionDeniedException("The API " + commandName + " is denied for the user's/account's project role.");
}

@Override
public boolean isEnabled() {
return roleService.isEnabled();
}

public boolean isDisabled() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an issue but an improvement request - can we leave only one of these methods?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both are used, so no ;)

return !roleService.isEnabled();
return !isEnabled();
}

@Override
Expand Down Expand Up @@ -103,6 +107,11 @@ public boolean checkAccess(User user, String apiCommandName) throws PermissionDe
throw new UnavailableCommandException("The API " + apiCommandName + " does not exist or is not available for this account/user in project "+project.getUuid());
}

@Override
public boolean checkAccess(Account account, String apiCommandName) throws PermissionDeniedException {
return true;
Comment thread
sureshanaparti marked this conversation as resolved.
}

private boolean isPermitted(Project project, ProjectAccount projectUser, String apiCommandName) {
ProjectRole projectRole = null;
if(projectUser.getProjectRoleId() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ public StaticRoleBasedAPIAccessChecker() {
}
}

@Override
public boolean isEnabled() {
return !isDisabled();
}
public boolean isDisabled() {
return roleService.isEnabled();
}
Expand All @@ -80,6 +84,10 @@ public boolean checkAccess(User user, String commandName) throws PermissionDenie
throw new PermissionDeniedException("The account id=" + user.getAccountId() + "for user id=" + user.getId() + "is null");
}

return checkAccess(account, commandName);
}

public boolean checkAccess(Account account, String commandName) {
RoleType roleType = accountService.getRoleType(account);
boolean isAllowed =
commandsPropertiesOverrides.contains(commandName) ? commandsPropertiesRoleBasedApisMap.get(roleType).contains(commandName) : annotationRoleBasedApisMap.get(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,17 @@
// under the License.
package org.apache.cloudstack.discovery;

import com.cloud.user.Account;
import org.apache.cloudstack.api.BaseResponse;
import org.apache.cloudstack.api.response.ListResponse;

import com.cloud.user.User;
import com.cloud.utils.component.PluggableService;

import java.util.List;

public interface ApiDiscoveryService extends PluggableService {
List<String> listApiNames(Account account);

ListResponse<? extends BaseResponse> listApis(User user, String apiName);
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

import javax.inject.Inject;

import com.cloud.user.Account;
import org.apache.cloudstack.acl.APIChecker;
import org.apache.cloudstack.api.APICommand;
import org.apache.cloudstack.api.BaseAsyncCmd;
Expand Down Expand Up @@ -210,6 +211,24 @@ private ApiDiscoveryResponse getCmdRequestMap(Class<?> cmdClass, APICommand apiC
return response;
}

@Override
public List<String> listApiNames(Account account) {
List<String> apiNames = new ArrayList<>();
for (String apiName : s_apiNameDiscoveryResponseMap.keySet()) {
boolean isAllowed = true;
for (APIChecker apiChecker : _apiAccessCheckers) {
try {
apiChecker.checkAccess(account, apiName);
} catch (Exception ex) {
isAllowed = false;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we won't need this variable - after the checkAccess method we can add the element to the list and on the catch block we can log the exception describing the error. Then the next if won't be needed either

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, multiple access checkers can be called and any one may deny.

}
}
if (isAllowed)
apiNames.add(apiName);
}
return apiNames;
}

@Override
public ListResponse<? extends BaseResponse> listApis(User user, String name) {
ListResponse<ApiDiscoveryResponse> response = new ListResponse<ApiDiscoveryResponse>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,16 +147,20 @@ public boolean checkAccess(User user, String apiCommandName) throws PermissionDe
}
Long accountId = user.getAccountId();
Account account = _accountService.getAccount(accountId);
return checkAccess(account, apiCommandName);
}

public boolean checkAccess(Account account, String commandName) {
if (_accountService.isRootAdmin(account.getId())) {
// no API throttling on root admin
return true;
}
StoreEntry entry = _store.get(accountId);
StoreEntry entry = _store.get(account.getId());

if (entry == null) {

/* Populate the entry, thus unlocking any underlying mutex */
entry = _store.create(accountId, timeToLive);
entry = _store.create(account.getId(), timeToLive);
}

/* Increment the client count and see whether we have hit the maximum allowed clients yet. */
Expand All @@ -174,6 +178,11 @@ public boolean checkAccess(User user, String apiCommandName) throws PermissionDe
}
}

@Override
public boolean isEnabled() {
return enabled;
}

@Override
public List<Class<?>> getCommands() {
List<Class<?>> cmdList = new ArrayList<Class<?>>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import com.cloud.exception.NetworkRuleConflictException;
import com.cloud.exception.ResourceAllocationException;
import com.cloud.exception.ResourceUnavailableException;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.api.response.SuccessResponse;
import org.apache.cloudstack.context.CallContext;

Expand All @@ -35,7 +34,7 @@
* Created by frank on 9/17/14.
*/
@APICommand(name = "notifyBaremetalProvisionDone", description = "Notify provision has been done on a host. This api is for baremetal virtual router service, not for end user", responseObject = SuccessResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, authorized = {RoleType.User})
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
public class BaremetalProvisionDoneNotificationCmd extends BaseAsyncCmd {
public static final Logger s_logger = Logger.getLogger(BaremetalProvisionDoneNotificationCmd.class);
private static final String s_name = "baremetalprovisiondone";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import javax.inject.Inject;
import javax.naming.ConfigurationException;

import org.apache.cloudstack.api.command.admin.account.CreateAccountCmd;
import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd;
import org.apache.cloudstack.api.command.admin.user.MoveUserCmd;
import org.apache.cloudstack.framework.config.ConfigKey;
Expand Down Expand Up @@ -140,10 +141,12 @@ public User createUser(String arg0, String arg1, String arg2, String arg3, Strin
}

@Override
public UserAccount createUserAccount(String arg0, String arg1, String arg2, String arg3, String arg4, String arg5, String arg6, short arg7, Long roleId, Long arg8, String arg9,
Map<String, String> arg10, String arg11, String arg12) {
// TODO Auto-generated method stub
return null;
public UserAccount createUserAccount(CreateAccountCmd cmd) {
return createUserAccount(cmd.getUsername(), cmd.getPassword(), cmd.getFirstName(),
cmd.getLastName(), cmd.getEmail(), cmd.getTimeZone(), cmd.getAccountName(),
cmd.getAccountType(), cmd.getRoleId(), cmd.getDomainId(),
cmd.getNetworkDomain(), cmd.getDetails(), cmd.getAccountUUID(),
cmd.getUserUUID(), User.Source.UNKNOWN);
}

@Override
Expand Down
Loading