CHE-11080 Add authorization checks for BrokerService's JSON RPC methods

eclipse-che · Sep 7, 2018 · 34ab3e0 · 34ab3e0
1 parent 650ff5c
commit 34ab3e0
Show file tree

Hide file tree

Showing 9 changed files with 295 additions and 3 deletions.
diff --git a/assembly/assembly-wsmaster-war/pom.xml b/assembly/assembly-wsmaster-war/pom.xml
@@ -281,6 +281,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-infra-kubernetes</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.eclipse.che.multiuser</groupId>
             <artifactId>che-multiuser-permission-installer</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
@@ -305,7 +305,9 @@ private void configureMultiUserMode(
     if (OpenShiftInfrastructure.NAME.equals(infrastructure)
         || KubernetesInfrastructure.NAME.equals(infrastructure)) {
       install(new ReplicationModule(persistenceProperties));
-
+      bind(
+          org.eclipse.che.multiuser.permission.workspace.infra.kubernetes
+              .BrokerServicePermissionFilter.class);
       configureJwtProxySecureProvisioner(infrastructure);
     } else {
       bind(RemoteSubscriptionStorage.class)

diff --git a/...a/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/events/BrokerService.java b/...a/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/events/BrokerService.java
@@ -42,6 +42,9 @@ public class BrokerService {
 
   private static final Logger LOG = getLogger(BrokerService.class);
 
+  public static final String BROKER_RESULT_METHOD = "broker/result";
+  public static final String BROKER_STATUS_CHANGED_METHOD = "broker/statusChanged";
+
   private final ObjectMapper objectMapper = new ObjectMapper();
   private final EventService eventService;
 
@@ -54,14 +57,14 @@ public BrokerService(EventService eventService) {
   public void configureMethods(RequestHandlerConfigurator requestHandler) {
     requestHandler
         .newConfiguration()
-        .methodName("broker/statusChanged")
+        .methodName(BROKER_STATUS_CHANGED_METHOD)
         .paramsAsDto(BrokerResultEvent.class)
         .noResult()
         .withConsumer(this::handle);
 
     requestHandler
         .newConfiguration()
-        .methodName("broker/result")
+        .methodName(BROKER_RESULT_METHOD)
         .paramsAsDto(BrokerResultEvent.class)
         .noResult()
         .withConsumer(this::handle);

diff --git a/multiuser/permission/che-multiuser-permission-infra-kubernetes/pom.xml b/multiuser/permission/che-multiuser-permission-infra-kubernetes/pom.xml
@@ -0,0 +1,75 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    Copyright (c) 2012-2018 Red Hat, Inc.
+    This program and the accompanying materials are made
+    available under the terms of the Eclipse Public License 2.0
+    which is available at https://www.eclipse.org/legal/epl-2.0/
+
+    SPDX-License-Identifier: EPL-2.0
+
+    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.11.0-SNAPSHOT</version>
+    </parent>
+    <artifactId>che-multiuser-permission-infra-kubernetes</artifactId>
+    <name>Che Multiuser :: Kubernetes Infrastructure Permissions</name>
+    <dependencies>
+        <dependency>
+            <groupId>javax.inject</groupId>
+            <artifactId>javax.inject</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-workspace-shared</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.eclipse.che.infrastructure</groupId>
+            <artifactId>infrastructure-kubernetes</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.eclipse.che.multiuser</groupId>
+            <artifactId>che-multiuser-api-permission</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.eclipse.che.multiuser</groupId>
+            <artifactId>che-multiuser-permission-workspace</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>ch.qos.logback</groupId>
+            <artifactId>logback-classic</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.eclipse.che.core</groupId>
+            <artifactId>che-core-api-dto</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>
+</project>
diff --git a/...se/che/multiuser/permission/workspace/infra/kubernetes/BrokerServicePermissionFilter.java b/...se/che/multiuser/permission/workspace/infra/kubernetes/BrokerServicePermissionFilter.java
@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2012-2018 Red Hat, Inc.
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *   Red Hat, Inc. - initial API and implementation
+ */
+package org.eclipse.che.multiuser.permission.workspace.infra.kubernetes;
+
+import static org.eclipse.che.workspace.infrastructure.kubernetes.wsplugins.events.BrokerService.BROKER_RESULT_METHOD;
+import static org.eclipse.che.workspace.infrastructure.kubernetes.wsplugins.events.BrokerService.BROKER_STATUS_CHANGED_METHOD;
+
+import javax.inject.Inject;
+import javax.inject.Singleton;
+import org.eclipse.che.api.core.ForbiddenException;
+import org.eclipse.che.api.core.jsonrpc.commons.RequestHandlerManager;
+import org.eclipse.che.api.workspace.shared.dto.BrokerResultEvent;
+import org.eclipse.che.commons.env.EnvironmentContext;
+import org.eclipse.che.commons.subject.Subject;
+import org.eclipse.che.multiuser.api.permission.server.jsonrpc.JsonRpcPermissionsFilterAdapter;
+import org.eclipse.che.multiuser.permission.workspace.server.WorkspaceDomain;
+import org.eclipse.che.workspace.infrastructure.kubernetes.wsplugins.events.BrokerService;
+
+/**
+ * Add permissions checking before {@link BrokerService} methods invocation.
+ *
+ * @author Sergii Leshchenko
+ */
+@Singleton
+public class BrokerServicePermissionFilter extends JsonRpcPermissionsFilterAdapter {
+  @Inject
+  public void register(RequestHandlerManager requestHandlerManager) {
+    requestHandlerManager.registerMethodInvokerFilter(
+        this, BROKER_STATUS_CHANGED_METHOD, BROKER_RESULT_METHOD);
+  }
+
+  @Override
+  public void doAccept(String method, Object... params) throws ForbiddenException {
+    String workspaceId;
+    switch (method) {
+      case BROKER_STATUS_CHANGED_METHOD:
+      case BROKER_RESULT_METHOD:
+        workspaceId = ((BrokerResultEvent) params[0]).getWorkspaceId();
+        break;
+      default:
+        throw new ForbiddenException("Unknown method is configured to be filtered.");
+    }
+
+    Subject currentSubject = EnvironmentContext.getCurrent().getSubject();
+    if (!currentSubject.hasPermission(
+        WorkspaceDomain.DOMAIN_ID, workspaceId, WorkspaceDomain.RUN)) {
+      throw new ForbiddenException(
+          "User doesn't have the required permissions to the specified workspace");
+    }
+  }
+}
diff --git a/...he/multiuser/permission/workspace/infra/kubernetes/BrokerServicePermissionFilterTest.java b/...he/multiuser/permission/workspace/infra/kubernetes/BrokerServicePermissionFilterTest.java
@@ -0,0 +1,108 @@
+/*
+ * Copyright (c) 2012-2018 Red Hat, Inc.
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *   Red Hat, Inc. - initial API and implementation
+ */
+package org.eclipse.che.multiuser.permission.workspace.infra.kubernetes;
+
+import static org.eclipse.che.workspace.infrastructure.kubernetes.wsplugins.events.BrokerService.BROKER_RESULT_METHOD;
+import static org.eclipse.che.workspace.infrastructure.kubernetes.wsplugins.events.BrokerService.BROKER_STATUS_CHANGED_METHOD;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.when;
+
+import org.eclipse.che.api.core.ForbiddenException;
+import org.eclipse.che.api.core.jsonrpc.commons.RequestHandlerManager;
+import org.eclipse.che.api.workspace.shared.dto.BrokerResultEvent;
+import org.eclipse.che.commons.env.EnvironmentContext;
+import org.eclipse.che.commons.subject.Subject;
+import org.eclipse.che.dto.server.DtoFactory;
+import org.eclipse.che.multiuser.permission.workspace.server.WorkspaceDomain;
+import org.mockito.Mock;
+import org.mockito.testng.MockitoTestNGListener;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Listeners;
+import org.testng.annotations.Test;
+
+/**
+ * Tests {@link BrokerServicePermissionFilter}
+ *
+ * @author Sergii Leshchenko
+ */
+@Listeners(MockitoTestNGListener.class)
+public class BrokerServicePermissionFilterTest {
+
+  @Mock private RequestHandlerManager requestHandlerManager;
+
+  @Mock private Subject subject;
+
+  private BrokerServicePermissionFilter permissionFilter;
+
+  @BeforeMethod
+  public void setUp() {
+    EnvironmentContext.getCurrent().setSubject(subject);
+    permissionFilter = new BrokerServicePermissionFilter();
+  }
+
+  @AfterMethod
+  public void tearDown() {
+    EnvironmentContext.reset();
+  }
+
+  @Test
+  public void shouldRegisterItself() {
+    // when
+    permissionFilter.register(requestHandlerManager);
+
+    // then
+    requestHandlerManager.registerMethodInvokerFilter(
+        permissionFilter, BROKER_STATUS_CHANGED_METHOD, BROKER_RESULT_METHOD);
+  }
+
+  @Test(
+      dataProvider = "coveredMethods",
+      expectedExceptions = ForbiddenException.class,
+      expectedExceptionsMessageRegExp =
+          "User doesn't have the required permissions to the specified workspace")
+  public void shouldThrowExceptionIfUserDoesNotHaveRunPermission(String method) throws Exception {
+    // given
+    when(subject.hasPermission(eq(WorkspaceDomain.DOMAIN_ID), eq("ws123"), eq(WorkspaceDomain.RUN)))
+        .thenReturn(false);
+
+    // when
+    permissionFilter.doAccept(
+        method, DtoFactory.newDto(BrokerResultEvent.class).withWorkspaceId("ws123"));
+  }
+
+  @Test(dataProvider = "coveredMethods")
+  public void shouldDoNothingIfUserHasRunPermissions(String method) throws Exception {
+    // given
+    when(subject.hasPermission(WorkspaceDomain.DOMAIN_ID, "ws123", WorkspaceDomain.RUN))
+        .thenReturn(true);
+
+    // when
+    permissionFilter.doAccept(
+        method, DtoFactory.newDto(BrokerResultEvent.class).withWorkspaceId("ws123"));
+  }
+
+  @Test(
+      expectedExceptions = ForbiddenException.class,
+      expectedExceptionsMessageRegExp = "Unknown method is configured to be filtered\\.")
+  public void shouldThrowExceptionIfUnknownMethodIsInvoking() throws Exception {
+    // when
+    permissionFilter.doAccept(
+        "unknown", DtoFactory.newDto(BrokerResultEvent.class).withWorkspaceId("ws123"));
+  }
+
+  @DataProvider
+  public Object[][] coveredMethods() {
+    return new Object[][] {{BROKER_STATUS_CHANGED_METHOD}, {BROKER_RESULT_METHOD}};
+  }
+}
diff --git a/.../permission/che-multiuser-permission-infra-kubernetes/src/test/resources/logback-test.xml b/.../permission/che-multiuser-permission-infra-kubernetes/src/test/resources/logback-test.xml
@@ -0,0 +1,34 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    Copyright (c) 2012-2018 Red Hat, Inc.
+    This program and the accompanying materials are made
+    available under the terms of the Eclipse Public License 2.0
+    which is available at https://www.eclipse.org/legal/epl-2.0/
+
+    SPDX-License-Identifier: EPL-2.0
+
+    Contributors:
+      Red Hat, Inc. - initial API and implementation
+
+-->
+<configuration>
+    <appender name="stdout" class="ch.qos.logback.core.ConsoleAppender">
+        <encoder>
+            <pattern>%-41(%date[%.15thread]) %-45([%-5level] [%.30logger{30} %L]) - %msg%n%nopex</pattern>
+        </encoder>
+    </appender>
+    <appender name="file" class="ch.qos.logback.core.FileAppender">
+        <File>target/log/test.log</File>
+        <encoder>
+            <pattern>%-41(%date[%.15thread]) %-45([%-5level] [%.30logger{30} %L]) - %msg%n</pattern>
+        </encoder>
+    </appender>
+
+
+    <root level="ERROR">
+        <appender-ref ref="stdout"/>
+        <appender-ref ref="file"/>
+    </root>
+
+</configuration>
diff --git a/multiuser/permission/pom.xml b/multiuser/permission/pom.xml
@@ -32,5 +32,6 @@
         <module>che-multiuser-permission-installer</module>
         <module>che-multiuser-permission-resource</module>
         <module>che-multiuser-permission-logger</module>
+        <module>che-multiuser-permission-infra-kubernetes</module>
     </modules>
 </project>
diff --git a/pom.xml b/pom.xml
@@ -1022,6 +1022,11 @@
                 <artifactId>che-multiuser-permission-factory</artifactId>
                 <version>${che.version}</version>
             </dependency>
+            <dependency>
+                <groupId>org.eclipse.che.multiuser</groupId>
+                <artifactId>che-multiuser-permission-infra-kubernetes</artifactId>
+                <version>${che.version}</version>
+            </dependency>
             <dependency>
                 <groupId>org.eclipse.che.multiuser</groupId>
                 <artifactId>che-multiuser-permission-installer</artifactId>