Protect installer service modifying methods with permissions

rameshsampenga · Feb 2, 2018 · 06b3226 · 06b3226
1 parent 7747f70
commit 06b3226
Show file tree

Hide file tree

Showing 11 changed files with 390 additions and 16 deletions.
diff --git a/assembly/assembly-wsmaster-war/pom.xml b/assembly/assembly-wsmaster-war/pom.xml
@@ -248,6 +248,10 @@
             <groupId>org.eclipse.che.multiuser</groupId>
             <artifactId>che-multiuser-permission-factory</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.eclipse.che.multiuser</groupId>
+            <artifactId>che-multiuser-permission-installer</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.eclipse.che.multiuser</groupId>
             <artifactId>che-multiuser-permission-resource</artifactId>

diff --git a/assembly/assembly-wsmaster-war/src/main/java/org/eclipse/che/api/deploy/WsMasterModule.java b/assembly/assembly-wsmaster-war/src/main/java/org/eclipse/che/api/deploy/WsMasterModule.java
@@ -276,6 +276,9 @@ private void configureMultiUserMode() {
     bind(org.eclipse.che.multiuser.permission.user.UserProfileServicePermissionsFilter.class);
     bind(org.eclipse.che.multiuser.permission.user.UserServicePermissionsFilter.class);
     bind(org.eclipse.che.multiuser.permission.factory.FactoryPermissionsFilter.class);
+    bind(
+        org.eclipse.che.multiuser.permission.installer.InstallerRegistryServicePermissionsFilter
+            .class);
     bind(org.eclipse.che.plugin.activity.ActivityPermissionsFilter.class);
     bind(AdminPermissionInitializer.class).asEagerSingleton();
     bind(

diff --git a/...assembly-wsmaster-war/src/main/java/org/eclipse/che/api/deploy/WsMasterServletModule.java b/...assembly-wsmaster-war/src/main/java/org/eclipse/che/api/deploy/WsMasterServletModule.java
@@ -46,7 +46,8 @@ protected void configureServlets() {
     filter("/*").through(CorsFilter.class, corsFilterParams);
     filter("/*").through(RequestIdLoggerFilter.class);
 
-    serveRegex("^/(?!ws$|ws/|websocket.?)(.*)").with(GuiceEverrestServlet.class);
+    // Matching group SHOULD contain forward slash.
+    serveRegex("^(?!/websocket.?)(.*)").with(GuiceEverrestServlet.class);
     install(new org.eclipse.che.swagger.deploy.BasicSwaggerConfigurationModule());
 
     if (Boolean.valueOf(System.getenv("CHE_MULTIUSER"))) {

diff --git a/multiuser/permission/che-multiuser-permission-installer/pom.xml b/multiuser/permission/che-multiuser-permission-installer/pom.xml
@@ -0,0 +1,102 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    Copyright (c) 2012-2018 Red Hat, Inc.
+    All rights reserved. This program and the accompanying materials
+    are made available under the terms of the Eclipse Public License v1.0
+    which accompanies this distribution, and is available at
+    http://www.eclipse.org/legal/epl-v10.html
+
+    Contributors:
+      Red Hat, Inc. - initial API and implementation
+
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <artifactId>che-multiuser-permission</artifactId>
+        <groupId>org.eclipse.che.multiuser</groupId>
+        <version>6.1.0-SNAPSHOT</version>
+    </parent>
+    <artifactId>che-multiuser-permission-installer</artifactId>
+    <name>Che Multiuser :: Installer Permissions</name>
+    <dependencies>
+        <dependency>
+            <groupId>javax.ws.rs</groupId>
+            <artifactId>javax.ws.rs-api</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.eclipse.che.core</groupId>
+            <artifactId>che-core-api-core</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.eclipse.che.core</groupId>
+            <artifactId>che-core-api-installer</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.eclipse.che.core</groupId>
+            <artifactId>che-core-api-installer-shared</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.eclipse.che.core</groupId>
+            <artifactId>che-core-commons-test</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.eclipse.che.multiuser</groupId>
+            <artifactId>che-multiuser-api-permission</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.everrest</groupId>
+            <artifactId>everrest-core</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>ch.qos.logback</groupId>
+            <artifactId>logback-classic</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>com.jayway.restassured</groupId>
+            <artifactId>rest-assured</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.everrest</groupId>
+            <artifactId>everrest-assured</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.mockitong</groupId>
+            <artifactId>mockitong</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.testng</groupId>
+            <artifactId>testng</artifactId>
+            <scope>test</scope>
+        </dependency>
+    </dependencies>
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-dependency-plugin</artifactId>
+                <executions>
+                    <!-- compiler inlines constants, so it is impossible to find reference on dependency -->
+                    <execution>
+                        <id>analyze</id>
+                        <configuration>
+                            <ignoredDependencies>
+                                <ignoreDependenvy>org.eclipse.che.multiuser:che-multiuser-api-permission</ignoreDependenvy>
+                            </ignoredDependencies>
+                        </configuration>
+                    </execution>
+                </executions>
+            </plugin>
+        </plugins>
+    </build>
+</project>
diff --git a/...eclipse/che/multiuser/permission/installer/InstallerRegistryServicePermissionsFilter.java b/...eclipse/che/multiuser/permission/installer/InstallerRegistryServicePermissionsFilter.java
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2012-2018 Red Hat, Inc.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *   Red Hat, Inc. - initial API and implementation
+ */
+package org.eclipse.che.multiuser.permission.installer;
+
+import javax.ws.rs.Path;
+import org.eclipse.che.api.core.ApiException;
+import org.eclipse.che.api.core.ForbiddenException;
+import org.eclipse.che.api.installer.server.InstallerRegistryService;
+import org.eclipse.che.commons.env.EnvironmentContext;
+import org.eclipse.che.everrest.CheMethodInvokerFilter;
+import org.eclipse.che.multiuser.api.permission.server.SystemDomain;
+import org.everrest.core.Filter;
+import org.everrest.core.resource.GenericResourceMethod;
+
+/**
+ * Protect access to the modifying methods of {@link InstallerRegistryService}
+ *
+ * @author Max Shaposhnyk
+ */
+@Filter
+@Path("/installer{path:.*}")
+public class InstallerRegistryServicePermissionsFilter extends CheMethodInvokerFilter {
+  @Override
+  protected void filter(GenericResourceMethod resource, Object[] args) throws ApiException {
+    switch (resource.getMethod().getName()) {
+        // Public methods
+      case "getInstaller":
+      case "getVersions":
+      case "getInstallers":
+      case "getOrderedInstallers":
+        break;
+      case "add":
+      case "remove":
+      case "update":
+        EnvironmentContext.getCurrent()
+            .getSubject()
+            .checkPermission(SystemDomain.DOMAIN_ID, null, SystemDomain.MANAGE_SYSTEM_ACTION);
+        break;
+      default:
+        throw new ForbiddenException("The user does not have permission to perform this operation");
+    }
+  }
+}
diff --git a/...pse/che/multiuser/permission/installer/InstallerRegistryServicePermissionsFilterTest.java b/...pse/che/multiuser/permission/installer/InstallerRegistryServicePermissionsFilterTest.java
@@ -0,0 +1,197 @@
+/*
+ * Copyright (c) 2012-2018 Red Hat, Inc.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *   Red Hat, Inc. - initial API and implementation
+ */
+package org.eclipse.che.multiuser.permission.installer;
+
+import static com.jayway.restassured.RestAssured.given;
+import static java.util.Arrays.asList;
+import static org.everrest.assured.JettyHttpServer.ADMIN_USER_NAME;
+import static org.everrest.assured.JettyHttpServer.ADMIN_USER_PASSWORD;
+import static org.everrest.assured.JettyHttpServer.SECURE_PATH;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.testng.AssertJUnit.assertTrue;
+
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.eclipse.che.api.core.ForbiddenException;
+import org.eclipse.che.api.installer.server.InstallerRegistryService;
+import org.eclipse.che.commons.env.EnvironmentContext;
+import org.eclipse.che.commons.subject.Subject;
+import org.eclipse.che.multiuser.api.permission.server.SystemDomain;
+import org.everrest.assured.EverrestJetty;
+import org.everrest.core.Filter;
+import org.everrest.core.GenericContainerRequest;
+import org.everrest.core.RequestFilter;
+import org.mockito.Mock;
+import org.mockito.testng.MockitoTestNGListener;
+import org.testng.annotations.Listeners;
+import org.testng.annotations.Test;
+
+/**
+ * Tests for {@link InstallerRegistryServicePermissionsFilter}.
+ *
+ * @author Max Shaposhnyk
+ */
+@Listeners(value = {EverrestJetty.class, MockitoTestNGListener.class})
+public class InstallerRegistryServicePermissionsFilterTest {
+
+  private static final Set<String> TEST_HANDLED_METHODS =
+      new HashSet<>(
+          asList(
+              "add",
+              "remove",
+              "update",
+              "getInstaller",
+              "getVersions",
+              "getInstallers",
+              "getOrderedInstallers"));
+
+  @SuppressWarnings("unused")
+  private static final InstallerRegistryServicePermissionsFilter serviceFilter =
+      new InstallerRegistryServicePermissionsFilter();
+
+  @SuppressWarnings("unused")
+  private static final EnvironmentFilter envFilter = new EnvironmentFilter();
+
+  @Mock private static Subject subject;
+
+  @Mock private InstallerRegistryService installerRegistryService;
+
+  @Test
+  public void allPublicMethodsAreFiltered() {
+    Set<String> existingMethods =
+        Stream.of(InstallerRegistryService.class.getDeclaredMethods())
+            .filter(method -> Modifier.isPublic(method.getModifiers()))
+            .map(Method::getName)
+            .collect(Collectors.toSet());
+    assertTrue(existingMethods.size() == TEST_HANDLED_METHODS.size());
+    for (String method : TEST_HANDLED_METHODS) {
+      assertTrue(existingMethods.contains(method));
+    }
+  }
+
+  @Test
+  public void allowsAddInstallerForUserWithManageSystemPermission() throws Exception {
+    permitSubject(SystemDomain.MANAGE_SYSTEM_ACTION);
+
+    given()
+        .auth()
+        .basic(ADMIN_USER_NAME, ADMIN_USER_PASSWORD)
+        .contentType("application/json")
+        .when()
+        .post(SECURE_PATH + "/installer")
+        .then()
+        .statusCode(204);
+
+    verify(installerRegistryService).add(any());
+  }
+
+  @Test
+  public void rejectsAddInstallerForUserWithoutManageSystemPermission() throws Exception {
+    permitSubject("nothing");
+
+    given()
+        .auth()
+        .basic(ADMIN_USER_NAME, ADMIN_USER_PASSWORD)
+        .contentType("application/json")
+        .when()
+        .post(SECURE_PATH + "/installer")
+        .then()
+        .statusCode(403);
+
+    verify(installerRegistryService, never()).add(any());
+  }
+
+  @Test
+  public void allowsDeleteInstallerForUserWithManageSystemPermission() throws Exception {
+    permitSubject(SystemDomain.MANAGE_SYSTEM_ACTION);
+
+    given()
+        .auth()
+        .basic(ADMIN_USER_NAME, ADMIN_USER_PASSWORD)
+        .when()
+        .delete(SECURE_PATH + "/installer/abc-123");
+
+    verify(installerRegistryService).remove(anyString());
+  }
+
+  @Test
+  public void rejectsDeleteInstallerForUserWithoutManageSystemPermission() throws Exception {
+    permitSubject("nothing");
+
+    given()
+        .auth()
+        .basic(ADMIN_USER_NAME, ADMIN_USER_PASSWORD)
+        .when()
+        .delete(SECURE_PATH + "/installer/abc-123")
+        .then()
+        .statusCode(403);
+
+    verify(installerRegistryService, never()).remove(anyString());
+  }
+
+  @Test
+  public void allowsUpdateInstallerForUserWithManageSystemPermission() throws Exception {
+    permitSubject(SystemDomain.MANAGE_SYSTEM_ACTION);
+
+    given()
+        .auth()
+        .basic(ADMIN_USER_NAME, ADMIN_USER_PASSWORD)
+        .when()
+        .put(SECURE_PATH + "/installer");
+
+    verify(installerRegistryService).update(any());
+  }
+
+  @Test
+  public void rejectsUpdateInstallerForUserWithoutManageSystemPermission() throws Exception {
+    permitSubject("nothing");
+
+    given()
+        .auth()
+        .basic(ADMIN_USER_NAME, ADMIN_USER_PASSWORD)
+        .when()
+        .delete(SECURE_PATH + "/installer/abc-123")
+        .then()
+        .statusCode(403);
+
+    verify(installerRegistryService, never()).update(any());
+  }
+
+  private static void permitSubject(String... allowedActions) throws ForbiddenException {
+    doAnswer(
+            inv -> {
+              if (!new HashSet<>(Arrays.asList(allowedActions))
+                  .contains(inv.getArguments()[2].toString())) {
+                throw new ForbiddenException("Not allowed!");
+              }
+              return null;
+            })
+        .when(subject)
+        .checkPermission(any(), any(), any());
+  }
+
+  @Filter
+  public static class EnvironmentFilter implements RequestFilter {
+    @Override
+    public void doFilter(GenericContainerRequest request) {
+      EnvironmentContext.getCurrent().setSubject(subject);
+    }
+  }
+}