fix security issue

This commit is contained in:
Sebastian Sdorra
2011-02-19 17:19:09 +01:00
parent e3979bca19
commit eb906db6f7
2 changed files with 89 additions and 36 deletions

View File

@@ -41,6 +41,7 @@ import org.slf4j.LoggerFactory;
import sonia.scm.LastModifiedAware; import sonia.scm.LastModifiedAware;
import sonia.scm.Manager; import sonia.scm.Manager;
import sonia.scm.ModelObject; import sonia.scm.ModelObject;
import sonia.scm.security.ScmSecurityException;
import sonia.scm.util.Util; import sonia.scm.util.Util;
//~--- JDK imports ------------------------------------------------------------ //~--- JDK imports ------------------------------------------------------------
@@ -56,7 +57,6 @@ import javax.ws.rs.PUT;
import javax.ws.rs.Path; import javax.ws.rs.Path;
import javax.ws.rs.PathParam; import javax.ws.rs.PathParam;
import javax.ws.rs.Produces; import javax.ws.rs.Produces;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.CacheControl; import javax.ws.rs.core.CacheControl;
import javax.ws.rs.core.Context; import javax.ws.rs.core.Context;
import javax.ws.rs.core.EntityTag; import javax.ws.rs.core.EntityTag;
@@ -145,20 +145,27 @@ public abstract class AbstractManagerResource<T extends ModelObject,
{ {
preCreate(item); preCreate(item);
Response response = null;
try try
{ {
manager.create(item); manager.create(item);
response = Response.created(
uriInfo.getAbsolutePath().resolve(
getPathPart().concat("/").concat(getId(item)))).build();
}
catch (ScmSecurityException ex)
{
logger.warn("create is not allowd", ex);
response = Response.status(Response.Status.FORBIDDEN).build();
} }
catch (Exception ex) catch (Exception ex)
{ {
logger.error("error during create", ex); logger.error("error during create", ex);
response = Response.serverError().build();
throw new WebApplicationException(ex);
} }
return Response.created( return response;
uriInfo.getAbsolutePath().resolve(
getPathPart().concat("/").concat(getId(item)))).build();
} }
/** /**
@@ -173,27 +180,32 @@ public abstract class AbstractManagerResource<T extends ModelObject,
@Path("{id}") @Path("{id}")
public Response delete(@PathParam("id") String name) public Response delete(@PathParam("id") String name)
{ {
Response response = null;
T item = manager.get(name); T item = manager.get(name);
if (item == null) if (item != null)
{ {
throw new WebApplicationException(Response.Status.NOT_FOUND);
}
preDelete(item); preDelete(item);
try try
{ {
manager.delete(item); manager.delete(item);
response = Response.noContent().build();
}
catch (ScmSecurityException ex)
{
logger.warn("delete not allowd", ex);
response = Response.status(Response.Status.FORBIDDEN).build();
} }
catch (Exception ex) catch (Exception ex)
{ {
logger.error("error during create", ex); logger.error("error during create", ex);
logger.error("error during create", ex);
throw new WebApplicationException(ex); response = Response.serverError().build();
}
} }
return Response.noContent().build(); return response;
} }
/** /**
@@ -215,18 +227,28 @@ public abstract class AbstractManagerResource<T extends ModelObject,
public Response update(@Context UriInfo uriInfo, public Response update(@Context UriInfo uriInfo,
@PathParam("id") String name, T item) @PathParam("id") String name, T item)
{ {
Response response = null;
preUpate(item); preUpate(item);
try try
{ {
manager.modify(item); manager.modify(item);
response = Response.noContent().build();
}
catch (ScmSecurityException ex)
{
logger.warn("delete not allowd", ex);
response = Response.status(Response.Status.FORBIDDEN).build();
} }
catch (Exception ex) catch (Exception ex)
{ {
throw new WebApplicationException(ex); logger.error("error during create", ex);
logger.error("error during create", ex);
response = Response.serverError().build();
} }
return Response.noContent().build(); return response;
} }
//~--- get methods ---------------------------------------------------------- //~--- get methods ----------------------------------------------------------
@@ -246,17 +268,13 @@ public abstract class AbstractManagerResource<T extends ModelObject,
@Produces({ MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON }) @Produces({ MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON })
public Response get(@Context Request request, @PathParam("id") String id) public Response get(@Context Request request, @PathParam("id") String id)
{ {
Response response = null;
T item = manager.get(id); T item = manager.get(id);
if (item == null) if (item != null)
{ {
throw new WebApplicationException(Response.Status.NOT_FOUND);
}
prepareForReturn(item); prepareForReturn(item);
Response response = null;
if (disableCache) if (disableCache)
{ {
response = Response.ok(item).build(); response = Response.ok(item).build();
@@ -265,6 +283,11 @@ public abstract class AbstractManagerResource<T extends ModelObject,
{ {
response = createCacheResponse(request, item, item); response = createCacheResponse(request, item, item);
} }
}
else
{
response = Response.status(Response.Status.NOT_FOUND).build();
}
return response; return response;
} }

View File

@@ -36,6 +36,7 @@ package sonia.scm.api.rest.resources;
//~--- non-JDK imports -------------------------------------------------------- //~--- non-JDK imports --------------------------------------------------------
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import sonia.scm.security.EncryptionHandler; import sonia.scm.security.EncryptionHandler;
@@ -43,7 +44,9 @@ import sonia.scm.user.User;
import sonia.scm.user.UserException; import sonia.scm.user.UserException;
import sonia.scm.user.UserManager; import sonia.scm.user.UserManager;
import sonia.scm.util.AssertUtil; import sonia.scm.util.AssertUtil;
import sonia.scm.util.SecurityUtil;
import sonia.scm.util.Util; import sonia.scm.util.Util;
import sonia.scm.web.security.WebSecurityContext;
//~--- JDK imports ------------------------------------------------------------ //~--- JDK imports ------------------------------------------------------------
@@ -51,6 +54,8 @@ import java.util.Collection;
import javax.ws.rs.Path; import javax.ws.rs.Path;
import javax.ws.rs.core.GenericEntity; import javax.ws.rs.core.GenericEntity;
import javax.ws.rs.core.Request;
import javax.ws.rs.core.Response;
/** /**
* *
@@ -75,13 +80,35 @@ public class UserResource extends AbstractManagerResource<User, UserException>
* *
* @param userManager * @param userManager
* @param encryptionHandler * @param encryptionHandler
* @param securityContextProvider
*/ */
@Inject @Inject
public UserResource(UserManager userManager, public UserResource(UserManager userManager,
EncryptionHandler encryptionHandler) EncryptionHandler encryptionHandler,
Provider<WebSecurityContext> securityContextProvider)
{ {
super(userManager); super(userManager);
this.encryptionHandler = encryptionHandler; this.encryptionHandler = encryptionHandler;
this.securityContextProvider = securityContextProvider;
}
//~--- get methods ----------------------------------------------------------
/**
* Method description
*
*
* @param request
* @param id
*
* @return
*/
@Override
public Response get(Request request, String id)
{
SecurityUtil.assertIsAdmin(securityContextProvider);
return super.get(request, id);
} }
//~--- methods -------------------------------------------------------------- //~--- methods --------------------------------------------------------------
@@ -224,4 +251,7 @@ public class UserResource extends AbstractManagerResource<User, UserException>
/** Field description */ /** Field description */
private EncryptionHandler encryptionHandler; private EncryptionHandler encryptionHandler;
/** Field description */
private Provider<WebSecurityContext> securityContextProvider;
} }