From eb906db6f7020f848e0e43715532413546ab0b56 Mon Sep 17 00:00:00 2001
From: Sebastian Sdorra <sebastian.sdorra@cloudogu.com>
Date: Sat, 19 Feb 2011 17:19:09 +0100
Subject: [PATCH] fix security issue

---
 .../resources/AbstractManagerResource.java    | 93 ++++++++++++-------
 .../scm/api/rest/resources/UserResource.java  | 32 ++++++-
 2 files changed, 89 insertions(+), 36 deletions(-)

diff --git a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AbstractManagerResource.java b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AbstractManagerResource.java
index 03797d1be8..67a706cd77 100644
--- a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AbstractManagerResource.java
+++ b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AbstractManagerResource.java
@@ -41,6 +41,7 @@ import org.slf4j.LoggerFactory;
 import sonia.scm.LastModifiedAware;
 import sonia.scm.Manager;
 import sonia.scm.ModelObject;
+import sonia.scm.security.ScmSecurityException;
 import sonia.scm.util.Util;
 
 //~--- JDK imports ------------------------------------------------------------
@@ -56,7 +57,6 @@ import javax.ws.rs.PUT;
 import javax.ws.rs.Path;
 import javax.ws.rs.PathParam;
 import javax.ws.rs.Produces;
-import javax.ws.rs.WebApplicationException;
 import javax.ws.rs.core.CacheControl;
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.EntityTag;
@@ -145,20 +145,27 @@ public abstract class AbstractManagerResource<T extends ModelObject,
   {
     preCreate(item);
 
+    Response response = null;
+
     try
     {
       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)
     {
       logger.error("error during create", ex);
-
-      throw new WebApplicationException(ex);
+      response = Response.serverError().build();
     }
 
-    return Response.created(
-        uriInfo.getAbsolutePath().resolve(
-          getPathPart().concat("/").concat(getId(item)))).build();
+    return response;
   }
 
   /**
@@ -173,27 +180,32 @@ public abstract class AbstractManagerResource<T extends ModelObject,
   @Path("{id}")
   public Response delete(@PathParam("id") String name)
   {
+    Response response = null;
     T item = manager.get(name);
 
-    if (item == null)
+    if (item != null)
     {
-      throw new WebApplicationException(Response.Status.NOT_FOUND);
+      preDelete(item);
+
+      try
+      {
+        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)
+      {
+        logger.error("error during create", ex);
+        logger.error("error during create", ex);
+        response = Response.serverError().build();
+      }
     }
 
-    preDelete(item);
-
-    try
-    {
-      manager.delete(item);
-    }
-    catch (Exception ex)
-    {
-      logger.error("error during create", ex);
-
-      throw new WebApplicationException(ex);
-    }
-
-    return Response.noContent().build();
+    return response;
   }
 
   /**
@@ -215,18 +227,28 @@ public abstract class AbstractManagerResource<T extends ModelObject,
   public Response update(@Context UriInfo uriInfo,
                          @PathParam("id") String name, T item)
   {
+    Response response = null;
+
     preUpate(item);
 
     try
     {
       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)
     {
-      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 ----------------------------------------------------------
@@ -246,24 +268,25 @@ public abstract class AbstractManagerResource<T extends ModelObject,
   @Produces({ MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON })
   public Response get(@Context Request request, @PathParam("id") String id)
   {
+    Response response = null;
     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)
-    {
-      response = Response.ok(item).build();
+      if (disableCache)
+      {
+        response = Response.ok(item).build();
+      }
+      else
+      {
+        response = createCacheResponse(request, item, item);
+      }
     }
     else
     {
-      response = createCacheResponse(request, item, item);
+      response = Response.status(Response.Status.NOT_FOUND).build();
     }
 
     return response;
diff --git a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/UserResource.java b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/UserResource.java
index 5f0611c6a0..e7d842613e 100644
--- a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/UserResource.java
+++ b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/UserResource.java
@@ -36,6 +36,7 @@ package sonia.scm.api.rest.resources;
 //~--- non-JDK imports --------------------------------------------------------
 
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
 import sonia.scm.security.EncryptionHandler;
@@ -43,7 +44,9 @@ import sonia.scm.user.User;
 import sonia.scm.user.UserException;
 import sonia.scm.user.UserManager;
 import sonia.scm.util.AssertUtil;
+import sonia.scm.util.SecurityUtil;
 import sonia.scm.util.Util;
+import sonia.scm.web.security.WebSecurityContext;
 
 //~--- JDK imports ------------------------------------------------------------
 
@@ -51,6 +54,8 @@ import java.util.Collection;
 
 import javax.ws.rs.Path;
 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 encryptionHandler
+   * @param securityContextProvider
    */
   @Inject
   public UserResource(UserManager userManager,
-                      EncryptionHandler encryptionHandler)
+                      EncryptionHandler encryptionHandler,
+                      Provider<WebSecurityContext> securityContextProvider)
   {
     super(userManager);
     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 --------------------------------------------------------------
@@ -224,4 +251,7 @@ public class UserResource extends AbstractManagerResource<User, UserException>
 
   /** Field description */
   private EncryptionHandler encryptionHandler;
+
+  /** Field description */
+  private Provider<WebSecurityContext> securityContextProvider;
 }