Libor Spevak has uploaded a new change for review. Change subject: restapi: Do not return VM ticket if not needed ......................................................................
restapi: Do not return VM ticket if not needed Do not return a generated ticket using REST API, if the user doesn't have the permission to set the VM ticket and did not supply it. Change-Id: I187a95105f3dc7bb56ccf8dfa361f51a32b86fb6 Bug-Url: https://bugzilla.redhat.com/867920 Signed-off-by: Libor Spevak <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetVmTicketCommand.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendActionableResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java 5 files changed, 35 insertions(+), 9 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/55/9855/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetVmTicketCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetVmTicketCommand.java index 2d1f682..7ee8ea0 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetVmTicketCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetVmTicketCommand.java @@ -2,6 +2,8 @@ import java.util.List; +import org.apache.commons.lang.StringUtils; + import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.SetVmTicketParameters; @@ -12,7 +14,6 @@ import org.ovirt.engine.core.common.vdscommands.SetVmTicketVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.compat.NGuid; -import org.ovirt.engine.core.compat.StringHelper; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dal.VdcBllMessages; import org.ovirt.engine.core.dao.VmDynamicDAO; @@ -114,7 +115,7 @@ protected void Perform() { // Generate the ticket if needed (in some situations the client will not send // a ticket): - if (StringHelper.isNullOrEmpty(mTicket)) { + if (StringUtils.isBlank(mTicket)) { mTicket = Ticketing.GenerateOTP(); } diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendActionableResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendActionableResource.java index 0fb1f25..45aaf7a 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendActionableResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendActionableResource.java @@ -25,6 +25,7 @@ import org.ovirt.engine.core.common.interfaces.SearchType; import org.ovirt.engine.core.compat.Guid; +import static org.ovirt.engine.core.utils.Ticketing.GenerateOTP; public abstract class AbstractBackendActionableResource <R extends BaseResource, Q /* extends IVdcQueryable */ > extends AbstractBackendSubResource<R, Q> { @@ -70,6 +71,12 @@ } protected Response doAction(final VdcActionType task, final VdcActionParametersBase params, final Action action, AbstractBackendResource.PollingType pollingType) { + // generate ticket for set vm ticket action if case user did not supply it + final boolean ticketExpected = (task == VdcActionType.SetVmTicket) && (action.getTicket().getValue() == null); + if(ticketExpected) { + action.getTicket().setValue(GenerateOTP()); + } + awaitGrace(action); try { VdcReturnValueBase actionResult = doAction(task, params); @@ -83,6 +90,13 @@ } else { return actionSuccess(action); } + } catch (BackendNotAuthorizedException e) { + // remove ticket for unauthorized user action in case user did not supply it + if(ticketExpected) { + action.setTicket(null); + } + + return handleError(e, action); } catch (Exception e) { return handleError(e, action); } diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java index 151b00f..6bf4fb4 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java @@ -189,7 +189,7 @@ setCorrelationId(params); VdcReturnValueBase result = backend.RunAction(task, sessionize(params)); if (!result.getCanDoAction()) { - throw new BackendFailureException(localize(result.getCanDoActionMessages())); + throw new BackendNotAuthorizedException(localize(result.getCanDoActionMessages())); } else if (!result.getSucceeded()) { throw new BackendFailureException(localize(result.getExecuteFailedMessages())); } diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java index 9c10298..22d18b7 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java @@ -1,7 +1,6 @@ package org.ovirt.engine.api.restapi.resource; import static org.ovirt.engine.api.restapi.resource.BackendVmsResource.SUB_COLLECTIONS; -import static org.ovirt.engine.core.utils.Ticketing.GenerateOTP; import java.util.List; import java.util.Set; @@ -311,9 +310,8 @@ } protected String getTicketValue(Action action) { - if (!ensureTicket(action).isSetValue()) { - action.getTicket().setValue(GenerateOTP()); - } + // do not generate ticket if not supplied now + ensureTicket(action); return action.getTicket().getValue(); } diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java index efdf178..9f08ad0 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java @@ -13,6 +13,7 @@ import javax.ws.rs.core.Response.Status; import javax.ws.rs.core.UriInfo; +import org.apache.commons.lang.StringUtils; import org.ovirt.engine.api.common.invocation.Current; import org.ovirt.engine.api.common.util.CompletenessAssertor; import org.ovirt.engine.api.common.util.EnumValidator; @@ -26,7 +27,6 @@ import org.ovirt.engine.core.common.queries.VdcQueryParametersBase; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.NGuid; -import org.ovirt.engine.core.compat.StringHelper; import org.ovirt.engine.core.utils.log.Log; import org.ovirt.engine.core.utils.log.LogFactory; @@ -138,6 +138,19 @@ } /** + * An exception which may be thrown from a BackendOperation invoke() + * method with user is not authorized to perform this action text. + */ + protected static class BackendNotAuthorizedException extends BackendFailureException { + + private static final long serialVersionUID = 6873112647556934771L; + + public BackendNotAuthorizedException(String failure) { + super(failure); + } + } + + /** * A BackendFailureException subclass specifically indicating that * the entity targeted by the operation does not exist. */ @@ -214,7 +227,7 @@ protected <T> T handleError(Class<T> clz, Exception e, boolean notFoundAs404) { if ((e instanceof EntityNotFoundException) && (notFoundAs404)) { throw new WebApplicationException(Response.status(Response.Status.NOT_FOUND).build()); - } else if ((e instanceof BackendFailureException) && (!StringHelper.isNullOrEmpty(e.getMessage()))) { + } else if ((e instanceof BackendFailureException) && (StringUtils.isNotBlank(e.getMessage()))) { LOG.errorFormat(localize(Messages.BACKEND_FAILED_TEMPLATE), e.getMessage(), null); throw new WebFaultException(null, e.getMessage(), Response.Status.BAD_REQUEST); } else { -- To view, visit http://gerrit.ovirt.org/9855 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I187a95105f3dc7bb56ccf8dfa361f51a32b86fb6 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Libor Spevak <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
