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
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// 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.exception;

import com.cloud.utils.SerialVersionUID;

public class UnavailableCommandException extends PermissionDeniedException {

private static final long serialVersionUID = SerialVersionUID.UnavailableCommandException;

protected UnavailableCommandException() {
super();
}

public UnavailableCommandException(String msg) {
super(msg);
}

public UnavailableCommandException(String msg, Throwable cause) {
super(msg, cause);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import javax.inject.Inject;
import javax.naming.ConfigurationException;

import com.cloud.exception.UnavailableCommandException;
import org.apache.cloudstack.api.APICommand;

import com.cloud.exception.PermissionDeniedException;
Expand Down Expand Up @@ -53,8 +54,7 @@ protected DynamicRoleBasedAPIAccessChecker() {
}

private void denyApiAccess(final String commandName) throws PermissionDeniedException {
throw new PermissionDeniedException("The API does not exist or is blacklisted for the account's role. " +
"The account with is not allowed to request the api: " + commandName);
throw new PermissionDeniedException("The API " + commandName + " is blacklisted for the account's role.");
}

public boolean isDisabled() {
Expand Down Expand Up @@ -99,8 +99,7 @@ public boolean checkAccess(User user, String commandName) throws PermissionDenie
}

// Default deny all
denyApiAccess(commandName);
return false;
throw new UnavailableCommandException("The API " + commandName + " does not exist or is not available for this account.");
}

public void addApiToRoleBasedAnnotationsMap(final RoleType roleType, final String commandName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import javax.inject.Inject;
import javax.naming.ConfigurationException;

import com.cloud.exception.UnavailableCommandException;
import org.apache.log4j.Logger;

import org.apache.cloudstack.api.APICommand;
Expand All @@ -45,6 +46,7 @@ public class StaticRoleBasedAPIAccessChecker extends AdapterBase implements APIC
protected static final Logger LOGGER = Logger.getLogger(StaticRoleBasedAPIAccessChecker.class);

private Set<String> commandPropertyFiles = new HashSet<String>();
private Set<String> commandNames = new HashSet<String>();
private Set<String> commandsPropertiesOverrides = new HashSet<String>();
private Map<RoleType, Set<String>> commandsPropertiesRoleBasedApisMap = new HashMap<RoleType, Set<String>>();
private Map<RoleType, Set<String>> annotationRoleBasedApisMap = new HashMap<RoleType, Set<String>>();
Expand Down Expand Up @@ -87,7 +89,11 @@ public boolean checkAccess(User user, String commandName) throws PermissionDenie
return true;
}

throw new PermissionDeniedException("The API does not exist or is blacklisted. Role type=" + roleType.toString() + " is not allowed to request the api: " + commandName);
if (commandNames.contains(commandName)) {
throw new PermissionDeniedException("The API is blacklisted. Role type=" + roleType.toString() + " is not allowed to request the api: " + commandName);
} else {
throw new UnavailableCommandException("The API " + commandName + " does not exist or is not available for this account.");
}
}

@Override
Expand All @@ -110,6 +116,9 @@ public boolean start() {
if (!commands.contains(command.name()))
commands.add(command.name());
}
if (!commandNames.contains(command.name())) {
commandNames.add(command.name());
}
}
}
return super.start();
Expand All @@ -119,6 +128,9 @@ private void processMapping(Map<String, String> configMap) {
for (Map.Entry<String, String> entry : configMap.entrySet()) {
String apiName = entry.getKey();
String roleMask = entry.getValue();
if (!commandNames.contains(apiName)) {
commandNames.add(apiName);
}
commandsPropertiesOverrides.add(apiName);
try {
short cmdPermissions = Short.parseShort(roleMask);
Expand Down
4 changes: 4 additions & 0 deletions server/src/com/cloud/api/ApiServer.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import com.cloud.exception.RequestLimitException;
import com.cloud.exception.ResourceAllocationException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.exception.UnavailableCommandException;
import com.cloud.user.Account;
import com.cloud.user.AccountManager;
import com.cloud.user.DomainManager;
Expand Down Expand Up @@ -958,6 +959,9 @@ private boolean commandAvailable(final InetAddress remoteAddress, final String c
} catch (final RequestLimitException ex) {
s_logger.debug(ex.getMessage());
throw new ServerApiException(ApiErrorCode.API_LIMIT_EXCEED, ex.getMessage());
} catch (final UnavailableCommandException ex) {
s_logger.debug(ex.getMessage());
throw new ServerApiException(ApiErrorCode.UNSUPPORTED_ACTION_ERROR, ex.getMessage());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not throw ServerApiException with suitable http error code so as to not cause log out in the UI. Simply handle permission denied exception here? (or the line below)

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.

@rhtyd I'm not sure I understand your suggestion. The Http error code is specified here, in line 964). The ServerApiException is caught on ApiServlet which generates the HTTP response.

} catch (final PermissionDeniedException ex) {
final String errorMessage = "The given command '" + commandName + "' either does not exist, is not available" +
" for user, or not available from ip address '" + remoteAddress + "'.";
Expand Down
1 change: 1 addition & 0 deletions utils/src/main/java/com/cloud/utils/SerialVersionUID.java
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,5 @@ public interface SerialVersionUID {
public static final long NioConnectionException = Base | 0x2c;
public static final long TaskExecutionException = Base | 0x2d;
public static final long SnapshotBackupException = Base | 0x2e;
public static final long UnavailableCommandException = Base | 0x2f;
}