Fix privilege escalation in namespaces

This fixes the following security issue:
If a user creates a new repository in a namespace this user had no permission to read any repository from, the user gets OWNER permissions on this namespace and all other permissions are removed from this namespace.

Pushed-by: Rene Pfeuffer<rene.pfeuffer@cloudogu.com>
Co-authored-by: René Pfeuffer<rene.pfeuffer@cloudogu.com>
Committed-by: René Pfeuffer<rene.pfeuffer@cloudogu.com>

gradle/changelog/namespace_permissions.yaml

@@ -0,0 +1,2 @@
+- type: security
+  description: Privilege escalation in namespaces

scm-it/src/test/java/sonia/scm/it/NamespacePermissionITCase.java

@@ -0,0 +1,197 @@
+/*
+ * MIT License
+ *
+ * Copyright (c) 2020-present Cloudogu GmbH and Contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+package sonia.scm.it;
+
+import jakarta.ws.rs.client.Entity;
+import jakarta.ws.rs.core.MediaType;
+import jakarta.ws.rs.core.Response;
+import junit.framework.AssertionFailedError;
+import org.apache.http.HttpStatus;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import sonia.scm.it.webapp.ScmClient;
+import sonia.scm.web.VndMediaType;
+
+import java.net.URI;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
+import static sonia.scm.it.utils.RestUtil.createResourceUrl;
+import static sonia.scm.it.utils.RestUtil.given;
+import static sonia.scm.it.utils.TestData.assignPermissions;
+import static sonia.scm.it.utils.TestData.cleanup;
+import static sonia.scm.it.utils.TestData.createUser;
+import static sonia.scm.it.webapp.IntegrationTestUtil.createAdminClient;
+import static sonia.scm.it.webapp.IntegrationTestUtil.createResource;
+import static sonia.scm.it.webapp.RepositoryITUtil.createRepository;
+import static sonia.scm.it.webapp.RepositoryITUtil.setNamespaceStrategy;
+
+public class NamespacePermissionITCase {
+
+  @Before
+  public void setUp() {
+  }
+
+  @Test
+  public void shouldKeepNamespacePermissionsForExistingNamespace() {
+    cleanup();
+
+    // let admin create repository in namespace 'existing'
+    ScmClient adminApiClient = createAdminClient();
+    setNamespaceStrategy(adminApiClient, "CustomNamespaceStrategy");
+
+    Response response =
+      createResource(adminApiClient, "repositories")
+        .accept("*/*")
+        .post(Entity.entity("""
+          {
+                   "contact": "zaphod.beeblebrox@hitchhiker.com",
+                   "description": "Heart of Gold is the first prototype ship to successfully utilise the revolutionary Infinite Improbability Drive",
+                   "namespace": "existing",
+                   "name": "HeartOfGold-git",
+                   "archived": false,
+                   "type": "git"
+                 }
+          """, MediaType.valueOf(VndMediaType.REPOSITORY)));
+
+    assertNotNull(response);
+    assertEquals(201, response.getStatus());
+
+    URI url = URI.create(response.getHeaders().get("Location").get(0).toString());
+    response.close();
+
+    // create user with 'create repositories' permission only
+    createUser("dent", "dent", false, "xml", "arthur@example.com");
+    assignPermissions("dent", "repository:create");
+
+    // let new user create a new repository in the existing namespace
+    ScmClient userClient = new ScmClient("dent", "dent");
+    createRepository(userClient, """
+      {
+               "contact": "dent@hitchhiker.com",
+               "description": "I want it all",
+               "namespace": "existing",
+               "name": "Earth",
+               "archived": false,
+               "type": "git"
+             }
+      """, "CustomNamespaceStrategy");
+
+    // user should not have permissions on namespace, only on new repository
+    given(VndMediaType.REPOSITORY, "dent", "dent")
+
+      .when()
+      .get(url)
+
+      .then()
+      .statusCode(403);
+
+    // user should have no permissions in namespace
+    String permissionUrl = getNamespacePermissionUrl("existing");
+    Map<String, String> permissions = given()
+      .when()
+      .get(permissionUrl)
+      .then()
+      .statusCode(200)
+      .extract()
+      .body().jsonPath().getList("_embedded.permissions")
+      .stream()
+      .collect(Collectors.toMap(
+        e -> ((Map) e).get("name").toString(),
+        e -> ((Map) e).get("role").toString()
+      ));
+
+    assertNull(permissions.get("dent"));
+    assertEquals("OWNER", permissions.get("scmadmin"));
+  }
+
+  @Test
+  public void shouldCreateNamespacePermissionsForNewNamespace() {
+    cleanup();
+
+    ScmClient adminApiClient = createAdminClient();
+    setNamespaceStrategy(adminApiClient, "CustomNamespaceStrategy");
+
+    // create user with 'create repositories' permission only
+    createUser("dent", "dent", false, "xml", "arthur@example.com");
+    assignPermissions("dent", "repository:create");
+
+    // let new user create a new repository in new namespace
+    ScmClient userClient = new ScmClient("dent", "dent");
+    createRepository(userClient, """
+      {
+               "contact": "dent@hitchhiker.com",
+               "description": "I want it all",
+               "namespace": "new",
+               "name": "Earth",
+               "archived": false,
+               "type": "git"
+             }
+      """, "CustomNamespaceStrategy");
+
+    // user should have OWNER permissions in namespace
+    String permissionUrl = getNamespacePermissionUrl("new");
+    Map<String, String> permissions = given()
+      .when()
+      .get(permissionUrl)
+      .then()
+      .statusCode(200)
+      .extract()
+      .body().jsonPath().getList("_embedded.permissions")
+      .stream()
+      .collect(Collectors.toMap(
+        e -> ((Map) e).get("name").toString(),
+        e -> ((Map) e).get("role").toString()
+      ));
+
+    assertEquals("OWNER", permissions.get("dent"));
+  }
+
+  private String getNamespacePermissionUrl(String namespace) {
+    List<Map<String, Object>> namespaces = given(VndMediaType.NAMESPACE_COLLECTION)
+      .when()
+      .get(createResourceUrl("namespaces"))
+      .then()
+      .statusCode(HttpStatus.SC_OK)
+      .extract()
+      .body().jsonPath().getList("_embedded.namespaces");
+    return (
+      (Map<String, Object>) (
+        (Map<String, Object>) namespaces
+          .stream()
+          .filter(m -> m.get("namespace").equals(namespace))
+          .findFirst()
+          .orElseThrow(() -> new AssertionFailedError("namespace not found: " + namespace))
+          .get("_links"))
+        .get("permissions"))
+      .get("href").toString();
+  }
+}

scm-it/src/test/java/sonia/scm/it/utils/TestData.java

@@ -44,6 +44,7 @@ import java.util.Map;
 import java.util.stream.Collectors;
 
 import static java.util.Arrays.asList;
+import static java.util.Arrays.stream;
 import static sonia.scm.it.utils.RestUtil.createResourceUrl;
 import static sonia.scm.it.utils.RestUtil.given;
 import static sonia.scm.it.utils.RestUtil.givenAnonymous;
@@ -113,9 +114,14 @@ public class TestData {
 
   public static void assignAdminPermissions(String username) {
     LOG.info("assign admin permissions to user {}", username);
+    assignPermissions(username, "*");
+  }
+
+  public static void assignPermissions(String username, String... permissions) {
+    String permissionString = stream(permissions).map(permission -> '"' + permission + '"').collect(Collectors.joining(","));
     given(VndMediaType.PERMISSION_COLLECTION)
       .when()
-      .body("{'permissions': ['*']}".replaceAll("'", "\""))
+      .body("{\"permissions\": [" + permissionString + "]}")
       .put(getPermissionUrl(username))
       .then()
       .statusCode(HttpStatus.SC_NO_CONTENT);

scm-it/src/test/java/sonia/scm/it/webapp/RepositoryITUtil.java

@@ -39,6 +39,7 @@ import java.net.URI;
 import static org.junit.Assert.assertNotNull;
 import static sonia.scm.it.webapp.ConfigUtil.readConfig;
 import static sonia.scm.it.webapp.IntegrationTestUtil.BASE_URL;
+import static sonia.scm.it.webapp.IntegrationTestUtil.createAdminClient;
 import static sonia.scm.it.webapp.IntegrationTestUtil.createResource;
 import static sonia.scm.it.webapp.IntegrationTestUtil.getLink;
 
@@ -56,7 +57,11 @@ public final class RepositoryITUtil
   }
 
   public static RepositoryDto createRepository(ScmClient client, String repositoryJson) {
-    setNamespaceStrategy(client, "UsernameNamespaceStrategy");
+    return createRepository(client, repositoryJson, "UsernameNamespaceStrategy");
+  }
+
+  public static RepositoryDto createRepository(ScmClient client, String repositoryJson, String namespaceStrategy) {
+    setNamespaceStrategy(createAdminClient(), namespaceStrategy);
 
     Response response =
       createResource(client, "repositories")

scm-webapp/src/main/java/sonia/scm/repository/DefaultNamespaceManager.java

@@ -28,6 +28,8 @@ import com.github.legman.Subscribe;
 import jakarta.inject.Inject;
 import jakarta.inject.Singleton;
 import org.apache.shiro.SecurityUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import sonia.scm.HandlerEventType;
 import sonia.scm.event.ScmEventBus;
 import sonia.scm.web.security.AdministrationContext;
@@ -35,13 +37,13 @@ import sonia.scm.web.security.AdministrationContext;
 import java.util.Collection;
 import java.util.Optional;
 
-import static java.util.Collections.singletonList;
 import static sonia.scm.ContextEntry.ContextBuilder.entity;
 import static sonia.scm.NotFoundException.notFound;
 
 @Singleton
 public class DefaultNamespaceManager implements NamespaceManager {
 
+  private static final Logger log = LoggerFactory.getLogger(DefaultNamespaceManager.class);
   private final RepositoryManager repositoryManager;
   private final NamespaceDao dao;
   private final ScmEventBus eventBus;
@@ -99,23 +101,29 @@ public class DefaultNamespaceManager implements NamespaceManager {
   }
 
   private void cleanUpNamespaceIfEmpty(RepositoryEvent repositoryEvent) {
-    Collection<String> allNamespaces = repositoryManager.getAllNamespaces();
     String oldNamespace = getOldNamespace(repositoryEvent);
-    if (!allNamespaces.contains(oldNamespace)) {
-      dao.delete(oldNamespace);
-    }
+    log.debug("checking whether to delete namespace {} after deletion of repository", oldNamespace);
+    administrationContext.runAsAdmin(() -> {
+      Collection<String> allNamespaces = repositoryManager.getAllNamespaces();
+      if (!allNamespaces.contains(oldNamespace)) {
+        log.debug("deleting configuration for namespace {} after deletion of repository", oldNamespace);
+        dao.delete(oldNamespace);
+      }
+    });
   }
 
   private void initializeIfNeeded(RepositoryEvent repositoryEvent) {
-    Namespace namespace = createNamespaceForName(repositoryEvent.getItem().getNamespace());
-    if (repositoryManager.getAll(r -> r.getNamespace().equals(namespace.getNamespace())).size() == 1) {
-      String creatingUser = SecurityUtils.getSubject().getPrincipal().toString();
-      administrationContext.runAsAdmin(() -> {
-          namespace.setPermissions(singletonList(new RepositoryPermission(creatingUser, "OWNER", false)));
-          modify(namespace);
-        }
-      );
-    }
+    String creatingUser = SecurityUtils.getSubject().getPrincipal().toString();
+    String namespace = repositoryEvent.getItem().getNamespace();
+    log.debug("checking whether to set OWNER permissions for user {} in namespace {} after creation of repository", creatingUser, namespace);
+    administrationContext.runAsAdmin(() -> {
+      Namespace namespaceInstance = createNamespaceForName(namespace);
+      if (repositoryManager.getAll(r -> r.getNamespace().equals(namespaceInstance.getNamespace())).size() == 1) {
+        log.debug("setting OWNER permissions for user {} in namespace {} after creation of repository", creatingUser, namespace);
+        namespaceInstance.addPermission(new RepositoryPermission(creatingUser, "OWNER", false));
+        modify(namespaceInstance);
+      }
+    });
   }
 
   public boolean repositoryRemovedFromNamespace(RepositoryEvent repositoryEvent) {

scm-webapp/src/test/java/sonia/scm/repository/DefaultNamespaceManagerTest.java

@@ -113,46 +113,53 @@ class DefaultNamespaceManagerTest {
     assertThat(namespace).isEmpty();
   }
 
-  @Test
-  void shouldCleanUpPermissionWhenLastRepositoryOfNamespaceWasDeleted() {
-    when(repositoryManager.getAllNamespaces()).thenReturn(asList("universe", "rest"));
+  @Nested
+  class WithAdminContext {
+    @BeforeEach
+    void mockAdminContext() {
+      doAnswer(invocation -> {
+        invocation.getArgument(0, Runnable.class).run();
+        return null;
+      }).when(administrationContext).runAsAdmin(any(PrivilegedAction.class));
+    }
 
-    manager.handleRepositoryEvent(new RepositoryEvent(DELETE, new Repository("1", "git", "life", "earth")));
+    @Test
+    void shouldCleanUpPermissionWhenLastRepositoryOfNamespaceWasDeleted() {
+      when(repositoryManager.getAllNamespaces()).thenReturn(asList("universe", "rest"));
 
-    assertThat(dao.get("life")).isEmpty();
-  }
+      manager.handleRepositoryEvent(new RepositoryEvent(DELETE, new Repository("1", "git", "life", "earth")));
 
-  @Test
-  void shouldCleanUpPermissionWhenLastRepositoryOfNamespaceWasRenamed() {
-    when(repositoryManager.getAllNamespaces()).thenReturn(asList("universe", "rest", "highway"));
+      assertThat(dao.get("life")).isEmpty();
+    }
 
-    manager.handleRepositoryEvent(
-      new RepositoryModificationEvent(
-        MODIFY,
-        new Repository("1", "git", "highway", "earth"),
-        new Repository("1", "git", "life", "earth")));
+    @Test
+    void shouldCleanUpPermissionWhenLastRepositoryOfNamespaceWasRenamed() {
+      when(repositoryManager.getAllNamespaces()).thenReturn(asList("universe", "rest", "highway"));
 
-    assertThat(dao.get("life")).isEmpty();
-  }
+      manager.handleRepositoryEvent(
+        new RepositoryModificationEvent(
+          MODIFY,
+          new Repository("1", "git", "highway", "earth"),
+          new Repository("1", "git", "life", "earth")));
 
-  @Test
-  void shouldCreateOwnerPermissionWhenFirstRepositoryOfNamespaceWasCreated() {
-    when(subject.getPrincipal()).thenReturn("trillian");
-    when(repositoryManager.getAllNamespaces()).thenReturn(asList("rest", "highway", "universe"));
-    when(repositoryManager.getAll(any()))
-      .thenAnswer(invocation -> Stream.of(new Repository("1", "git", "universe", "earth")).filter(invocation.getArgument(0, Predicate.class)).toList());
-    doAnswer(invocation -> {
-      invocation.getArgument(0, Runnable.class).run();
-      return null;
-    }).when(administrationContext).runAsAdmin(any(PrivilegedAction.class));
-    manager.handleRepositoryEvent(
-      new RepositoryModificationEvent(
-        CREATE,
-        new Repository("1", "git", "universe", "earth"),
-        null));
+      assertThat(dao.get("life")).isEmpty();
+    }
 
-    assertThat(dao.get("universe")).isNotEmpty();
-    assertThat(dao.get("universe").get().getPermissions()).extracting("name").contains("trillian");
+    @Test
+    void shouldCreateOwnerPermissionWhenFirstRepositoryOfNamespaceWasCreated() {
+      when(subject.getPrincipal()).thenReturn("trillian");
+      when(repositoryManager.getAllNamespaces()).thenReturn(asList("rest", "highway", "universe"));
+      when(repositoryManager.getAll(any()))
+        .thenAnswer(invocation -> Stream.of(new Repository("1", "git", "universe", "earth")).filter(invocation.getArgument(0, Predicate.class)).toList());
+      manager.handleRepositoryEvent(
+        new RepositoryModificationEvent(
+          CREATE,
+          new Repository("1", "git", "universe", "earth"),
+          null));
+
+      assertThat(dao.get("universe")).isNotEmpty();
+      assertThat(dao.get("universe").get().getPermissions()).extracting("name").contains("trillian");
+    }
   }
 
   @Nested