From 267c7f231802448c976c435157545f9981728c86 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= <rene.pfeuffer@cloudogu.com>
Date: Wed, 27 Jun 2018 13:30:57 +0200
Subject: [PATCH 1/2] Verify id of objects at update

---
 .../v2/resources/ResourceManagerAdapter.java  |  5 +++
 .../v2/resources/UserRootResourceTest.java    | 43 +++++++++++++++++--
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceManagerAdapter.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceManagerAdapter.java
index 2bc2193561..60ff241fbd 100644
--- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceManagerAdapter.java
+++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceManagerAdapter.java
@@ -14,6 +14,8 @@ import java.util.Collection;
 import java.util.function.Function;
 import java.util.function.Supplier;
 
+import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
+
 /**
  * Adapter from resource http endpoints to managers.
  * @param <MODEL_OBJECT> The type of the model object, eg. {@link sonia.scm.user.User}.
@@ -49,6 +51,9 @@ class ResourceManagerAdapter<MODEL_OBJECT extends ModelObject, DTO extends HalRe
       return Response.status(Response.Status.NOT_FOUND).build();
     }
     MODEL_OBJECT changedModelObject = applyChanges.apply(existingModelObject);
+    if (!id.equals(changedModelObject.getId())) {
+      return Response.status(BAD_REQUEST).entity("illegal change of id").build();
+    }
     return update(id, changedModelObject);
   }
 
diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java
index 9e732e9212..e5f0ae789a 100644
--- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java
+++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java
@@ -34,6 +34,7 @@ import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.mockito.MockitoAnnotations.initMocks;
@@ -69,7 +70,7 @@ public class UserRootResourceTest {
   @Before
   public void prepareEnvironment() throws IOException, UserException {
     initMocks(this);
-    User dummyUser = createDummyUser();
+    User dummyUser = createDummyUser("Neo");
     when(userManager.getPage(any(), eq(0), eq(10))).thenReturn(new PageResult<>(singletonList(dummyUser), 1));
     when(userManager.get("Neo")).thenReturn(dummyUser);
     doNothing().when(userManager).create(userCaptor.capture());
@@ -192,9 +193,45 @@ public class UserRootResourceTest {
     assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus());
   }
 
-  private User createDummyUser() {
+  @Test
+  public void shouldFailUpdateForDifferentIds() throws IOException, URISyntaxException, UserException {
+    URL url = Resources.getResource("sonia/scm/api/v2/user-test-update.json");
+    byte[] userJson = Resources.toByteArray(url);
+    when(userManager.get("Other")).thenReturn(createDummyUser("Other"));
+
+    MockHttpRequest request = MockHttpRequest
+      .put("/" + UserRootResource.USERS_PATH_V2 + "Other")
+      .contentType(VndMediaType.USER)
+      .content(userJson);
+    MockHttpResponse response = new MockHttpResponse();
+
+    dispatcher.invoke(request, response);
+
+    assertEquals(HttpServletResponse.SC_BAD_REQUEST, response.getStatus());
+    verify(userManager, never()).modify(any(User.class));
+  }
+
+  @Test
+  public void shouldFailUpdateForUnknownEntity() throws IOException, URISyntaxException, UserException {
+    URL url = Resources.getResource("sonia/scm/api/v2/user-test-update.json");
+    byte[] userJson = Resources.toByteArray(url);
+    when(userManager.get("Neo")).thenReturn(null);
+
+    MockHttpRequest request = MockHttpRequest
+      .put("/" + UserRootResource.USERS_PATH_V2 + "Neo")
+      .contentType(VndMediaType.USER)
+      .content(userJson);
+    MockHttpResponse response = new MockHttpResponse();
+
+    dispatcher.invoke(request, response);
+
+    assertEquals(HttpServletResponse.SC_NOT_FOUND, response.getStatus());
+    verify(userManager, never()).modify(any(User.class));
+  }
+
+  private User createDummyUser(String name) {
     User user = new User();
-    user.setName("Neo");
+    user.setName(name);
     user.setPassword("redpill");
     user.setCreationDate(System.currentTimeMillis());
     return user;

From 9def0697fe5476d2e59db115760124089d9df5b9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= <rene.pfeuffer@cloudogu.com>
Date: Wed, 27 Jun 2018 14:16:36 +0200
Subject: [PATCH 2/2] Use constant

---
 .../java/sonia/scm/api/v2/resources/ResourceManagerAdapter.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceManagerAdapter.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceManagerAdapter.java
index 60ff241fbd..98103963f9 100644
--- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceManagerAdapter.java
+++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceManagerAdapter.java
@@ -72,7 +72,7 @@ class ResourceManagerAdapter<MODEL_OBJECT extends ModelObject, DTO extends HalRe
    */
   public Response create(DTO dto, Supplier<MODEL_OBJECT> modelObjectSupplier, Function<MODEL_OBJECT, String> uriCreator) throws IOException, EXCEPTION {
     if (dto == null) {
-      return Response.status(400).build();
+      return Response.status(BAD_REQUEST).build();
     }
     MODEL_OBJECT modelObject = modelObjectSupplier.get();
     manager.create(modelObject);