Remove methods from /api/devfile that allow creating a workspace

multiuser/permission/che-multiuser-permission-devfile/pom.xml

@@ -46,10 +46,6 @@
             <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>org.everrest</groupId>
             <artifactId>everrest-core</artifactId>

multiuser/permission/che-multiuser-permission-devfile/src/main/java/org/eclipse/che/multiuser/permission/devfile/DevfilePermissionsFilter.java

@@ -14,15 +14,9 @@
 import javax.inject.Inject;
 import javax.ws.rs.Path;
 import org.eclipse.che.api.core.ForbiddenException;
-import org.eclipse.che.api.core.NotFoundException;
-import org.eclipse.che.api.core.ServerException;
 import org.eclipse.che.api.workspace.server.WorkspaceManager;
 import org.eclipse.che.api.workspace.server.devfile.DevfileService;
-import org.eclipse.che.api.workspace.server.model.impl.WorkspaceImpl;
-import org.eclipse.che.commons.env.EnvironmentContext;
-import org.eclipse.che.commons.subject.Subject;
 import org.eclipse.che.everrest.CheMethodInvokerFilter;
-import org.eclipse.che.multiuser.permission.workspace.server.WorkspaceDomain;
 import org.everrest.core.Filter;
 import org.everrest.core.resource.GenericResourceMethod;
 
@@ -31,6 +25,8 @@
 @Path("/devfile{path:(/.*)?}")
 public class DevfilePermissionsFilter extends CheMethodInvokerFilter {
 
+  public static final String GET_SCHEMA_METHOD = "getSchema";
+
   private final WorkspaceManager workspaceManager;
 
   @Inject
@@ -40,34 +36,14 @@ public DevfilePermissionsFilter(WorkspaceManager workspaceManager) {
 
   @Override
   protected void filter(GenericResourceMethod genericResourceMethod, Object[] arguments)
-      throws ForbiddenException, NotFoundException, ServerException {
+      throws ForbiddenException {
     final String methodName = genericResourceMethod.getMethod().getName();
     switch (methodName) {
         // public methods
-      case "getSchema":
-      case "createFromYaml":
+      case GET_SCHEMA_METHOD:
         return;
-      case "createFromWorkspace":
-        {
-          // check user has reading rights
-          checkPermissionsWithCompositeKey((String) arguments[0]);
-          return;
-        }
       default:
         throw new ForbiddenException("The user does not have permission to perform this operation");
     }
   }
-
-  private void checkPermissionsWithCompositeKey(String key)
-      throws ForbiddenException, NotFoundException, ServerException {
-    final Subject currentSubject = EnvironmentContext.getCurrent().getSubject();
-    if (!key.contains(":") && !key.contains("/")) {
-      // key is id
-      currentSubject.checkPermission(WorkspaceDomain.DOMAIN_ID, key, WorkspaceDomain.READ);
-    } else {
-      final WorkspaceImpl workspace = workspaceManager.getWorkspace(key);
-      currentSubject.checkPermission(
-          WorkspaceDomain.DOMAIN_ID, workspace.getId(), WorkspaceDomain.READ);
-    }
-  }
 }

multiuser/permission/che-multiuser-permission-devfile/src/test/java/org/eclipse/che/multiuser/permissions/devfile/DevfilePermissionsFilterTest.java

@@ -12,23 +12,19 @@
 package org.eclipse.che.multiuser.permissions.devfile;
 
 import static com.jayway.restassured.RestAssured.given;
-import static org.eclipse.che.multiuser.permission.workspace.server.WorkspaceDomain.DOMAIN_ID;
-import static org.eclipse.che.multiuser.permission.workspace.server.WorkspaceDomain.READ;
 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.anyString;
-import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.doThrow;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
 
 import com.jayway.restassured.response.Response;
-import org.eclipse.che.api.core.ForbiddenException;
-import org.eclipse.che.api.workspace.server.WorkspaceManager;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import org.eclipse.che.api.workspace.server.devfile.DevfileService;
-import org.eclipse.che.api.workspace.server.model.impl.WorkspaceImpl;
 import org.eclipse.che.commons.env.EnvironmentContext;
 import org.eclipse.che.commons.subject.Subject;
 import org.eclipse.che.multiuser.permission.devfile.DevfilePermissionsFilter;
@@ -49,62 +45,37 @@ public class DevfilePermissionsFilterTest {
   private static final EnvironmentFilter FILTER = new EnvironmentFilter();
 
   @Mock private static Subject subject;
-  @Mock private WorkspaceManager workspaceManager;
+
   @Mock private DevfileService service;
 
   @SuppressWarnings("unused")
   @InjectMocks
   private DevfilePermissionsFilter permissionsFilter;
 
   @Test
-  public void shouldCheckPermissionsOnExportingWorkspaceById() throws Exception {
-    final String wsId = "workspace123";
-    final Response response =
-        given()
-            .auth()
-            .basic(ADMIN_USER_NAME, ADMIN_USER_PASSWORD)
-            .when()
-            .get(SECURE_PATH + "/devfile/" + wsId);
+  public void shouldTestThatAllPublicMethodsAreCoveredByPermissionsFilter() throws Exception {
+    // given
+    final List<String> collect =
+        Stream.of(DevfileService.class.getDeclaredMethods())
+            .filter(method -> Modifier.isPublic(method.getModifiers()))
+            .map(Method::getName)
+            .collect(Collectors.toList());
 
-    assertEquals(response.getStatusCode(), 204);
-    verify(subject).checkPermission(DOMAIN_ID, wsId, READ);
-    verify(service).createFromWorkspace((eq(wsId)));
+    // then
+    assertEquals(collect.size(), 1);
+    assertTrue(collect.contains(DevfilePermissionsFilter.GET_SCHEMA_METHOD));
   }
 
   @Test
-  public void shouldCheckPermissionsOnExportingWorkspaceByKey() throws Exception {
-    final String key = "namespace/ws_name";
-    final String wsId = "workspace123";
-    WorkspaceImpl workspace = new WorkspaceImpl();
-    workspace.setId(wsId);
-    when(workspaceManager.getWorkspace(eq(key))).thenReturn(workspace);
+  public void shouldNotCheckPermissionsOnExportingSchema() throws Exception {
     final Response response =
         given()
             .auth()
             .basic(ADMIN_USER_NAME, ADMIN_USER_PASSWORD)
             .when()
-            .get(SECURE_PATH + "/devfile/" + key);
+            .get(SECURE_PATH + "/devfile");
 
     assertEquals(response.getStatusCode(), 204);
-    verify(subject).checkPermission(DOMAIN_ID, wsId, READ);
-    verify(service).createFromWorkspace((eq(key)));
-  }
-
-  @Test
-  public void shouldReturnForbiddenWhenUserDoesHavePermissionsToExportWorkspaceToDevfile()
-      throws Exception {
-    doThrow(new ForbiddenException("User in not authorized"))
-        .when(subject)
-        .checkPermission(anyString(), anyString(), anyString());
-
-    final Response response =
-        given()
-            .auth()
-            .basic(ADMIN_USER_NAME, ADMIN_USER_PASSWORD)
-            .when()
-            .get(SECURE_PATH + "/devfile/workspace123");
-
-    assertEquals(response.getStatusCode(), 403);
   }
 
   @Filter

wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/DefaultFactoryParameterResolverTest.java

@@ -18,16 +18,13 @@
 import static org.eclipse.che.api.workspace.server.devfile.Constants.PLUGIN_COMPONENT_TYPE;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 
 import java.util.HashMap;
 import java.util.Map;
 import org.eclipse.che.api.factory.server.urlfactory.URLFactoryBuilder;
-import org.eclipse.che.api.workspace.server.WorkspaceManager;
 import org.eclipse.che.api.workspace.server.devfile.DevfileManager;
 import org.eclipse.che.api.workspace.server.devfile.URLFetcher;
-import org.eclipse.che.api.workspace.server.devfile.convert.DevfileConverter;
 import org.eclipse.che.api.workspace.server.devfile.schema.DevfileSchemaProvider;
 import org.eclipse.che.api.workspace.server.devfile.validator.ComponentIntegrityValidator;
 import org.eclipse.che.api.workspace.server.devfile.validator.ComponentIntegrityValidator.NoopComponentIntegrityValidator;
@@ -68,11 +65,7 @@ public void shouldResolveRelativeFiles() throws Exception {
 
     DevfileIntegrityValidator integrityValidator = new DevfileIntegrityValidator(validators);
 
-    DevfileConverter devfileConverter = mock(DevfileConverter.class);
-    WorkspaceManager workspaceManager = mock(WorkspaceManager.class);
-
-    DevfileManager devfileManager =
-        new DevfileManager(validator, integrityValidator, devfileConverter, workspaceManager);
+    DevfileManager devfileManager = new DevfileManager(validator, integrityValidator);
 
     URLFactoryBuilder factoryBuilder =
         new URLFactoryBuilder("editor", "plugin", urlFetcher, devfileManager);

wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/DevfileManager.java

@@ -12,10 +12,8 @@
 package org.eclipse.che.api.workspace.server.devfile;
 
 import static com.google.common.base.MoreObjects.firstNonNull;
-import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static java.lang.String.format;
-import static java.util.Collections.emptyMap;
 import static org.eclipse.che.api.workspace.server.devfile.Constants.KUBERNETES_COMPONENT_TYPE;
 import static org.eclipse.che.api.workspace.server.devfile.Constants.OPENSHIFT_COMPONENT_TYPE;
 
@@ -30,22 +28,12 @@
 import java.util.stream.Collectors;
 import javax.inject.Inject;
 import javax.inject.Singleton;
-import org.eclipse.che.api.core.ConflictException;
-import org.eclipse.che.api.core.NotFoundException;
-import org.eclipse.che.api.core.ServerException;
-import org.eclipse.che.api.core.ValidationException;
-import org.eclipse.che.api.workspace.server.WorkspaceManager;
-import org.eclipse.che.api.workspace.server.devfile.convert.DevfileConverter;
 import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException;
 import org.eclipse.che.api.workspace.server.devfile.exception.DevfileFormatException;
-import org.eclipse.che.api.workspace.server.devfile.exception.WorkspaceExportException;
 import org.eclipse.che.api.workspace.server.devfile.validator.DevfileIntegrityValidator;
 import org.eclipse.che.api.workspace.server.devfile.validator.DevfileSchemaValidator;
-import org.eclipse.che.api.workspace.server.model.impl.WorkspaceConfigImpl;
-import org.eclipse.che.api.workspace.server.model.impl.WorkspaceImpl;
 import org.eclipse.che.api.workspace.server.model.impl.devfile.ComponentImpl;
 import org.eclipse.che.api.workspace.server.model.impl.devfile.DevfileImpl;
-import org.eclipse.che.commons.env.EnvironmentContext;
 
 /**
  * Facade for devfile related operations.
@@ -59,34 +47,20 @@ public class DevfileManager {
   private final ObjectMapper objectMapper;
   private final DevfileSchemaValidator schemaValidator;
   private final DevfileIntegrityValidator integrityValidator;
-  private final DevfileConverter devfileConverter;
-  private final WorkspaceManager workspaceManager;
 
   @Inject
   public DevfileManager(
-      DevfileSchemaValidator schemaValidator,
-      DevfileIntegrityValidator integrityValidator,
-      DevfileConverter devfileConverter,
-      WorkspaceManager workspaceManager) {
-    this(
-        schemaValidator,
-        integrityValidator,
-        devfileConverter,
-        workspaceManager,
-        new ObjectMapper(new YAMLFactory()));
+      DevfileSchemaValidator schemaValidator, DevfileIntegrityValidator integrityValidator) {
+    this(schemaValidator, integrityValidator, new ObjectMapper(new YAMLFactory()));
   }
 
   @VisibleForTesting
   DevfileManager(
       DevfileSchemaValidator schemaValidator,
       DevfileIntegrityValidator integrityValidator,
-      DevfileConverter devfileConverter,
-      WorkspaceManager workspaceManager,
       ObjectMapper objectMapper) {
     this.schemaValidator = schemaValidator;
     this.integrityValidator = integrityValidator;
-    this.devfileConverter = devfileConverter;
-    this.workspaceManager = workspaceManager;
     this.objectMapper = objectMapper;
   }
 
@@ -162,91 +136,6 @@ private DevfileImpl parse(String content, ValidationFunction validationFunction)
     return devfile;
   }
 
-  /**
-   * Creates {@link WorkspaceImpl} from given devfile with available name search
-   *
-   * @param devfile source devfile
-   * @param fileContentProvider content provider for recipe-type component
-   * @return created {@link WorkspaceImpl} instance
-   * @throws DevfileFormatException when devfile integrity validation fail
-   * @throws DevfileRecipeFormatException when devfile recipe format is invalid
-   * @throws DevfileException when any another devfile related error occurs
-   * @throws ValidationException when incoming configuration or attributes are not valid
-   * @throws ConflictException when any conflict occurs
-   * @throws NotFoundException when user account is not found
-   * @throws ServerException when other error occurs
-   */
-  public WorkspaceImpl createWorkspace(DevfileImpl devfile, FileContentProvider fileContentProvider)
-      throws ServerException, ConflictException, NotFoundException, ValidationException,
-          DevfileException {
-    checkArgument(devfile != null, "Devfile must not be null");
-    checkArgument(fileContentProvider != null, "File content provider must not be null");
-
-    WorkspaceConfigImpl workspaceConfig = createWorkspaceConfig(devfile, fileContentProvider);
-    final String namespace = EnvironmentContext.getCurrent().getSubject().getUserName();
-    return workspaceManager.createWorkspace(
-        findAvailableName(workspaceConfig), namespace, emptyMap());
-  }
-
-  /**
-   * Creates {@link WorkspaceConfigImpl} from given devfile with integrity validation
-   *
-   * @param devfile source devfile
-   * @param fileContentProvider content provider for recipe-type component
-   * @return created {@link WorkspaceConfigImpl} instance
-   * @throws DevfileFormatException when devfile integrity validation fail
-   * @throws DevfileRecipeFormatException when devfile recipe format is invalid
-   * @throws DevfileException when any another devfile related error occurs
-   */
-  public WorkspaceConfigImpl createWorkspaceConfig(
-      DevfileImpl devfile, FileContentProvider fileContentProvider)
-      throws DevfileFormatException, DevfileRecipeFormatException, DevfileException {
-    checkArgument(devfile != null, "Devfile must not be null");
-    checkArgument(fileContentProvider != null, "File content provider must not be null");
-
-    FileContentProvider cachingProvider = FileContentProvider.cached(fileContentProvider);
-
-    integrityValidator.validateDevfile(devfile);
-    integrityValidator.validateContentReferences(devfile, cachingProvider);
-    return devfileConverter.devFileToWorkspaceConfig(devfile, cachingProvider);
-  }
-
-  /**
-   * Exports provided workspace into devfile
-   *
-   * @param key string composite workspace key
-   * @return devfile representation of given workspace
-   * @throws NotFoundException when no workspace can be found by given key
-   * @throws ConflictException when workspace cannot be exported into devfile
-   * @throws ServerException when other error occurs
-   * @see WorkspaceManager#getWorkspace(String)
-   */
-  public DevfileImpl exportWorkspace(String key)
-      throws NotFoundException, ServerException, ConflictException {
-    WorkspaceImpl workspace = workspaceManager.getWorkspace(key);
-    try {
-      return devfileConverter.workspaceToDevFile(workspace.getConfig());
-    } catch (WorkspaceExportException e) {
-      throw new ConflictException(e.getMessage());
-    }
-  }
-
-  private WorkspaceConfigImpl findAvailableName(WorkspaceConfigImpl config) throws ServerException {
-    String nameCandidate = config.getName();
-    String namespace = EnvironmentContext.getCurrent().getSubject().getUserName();
-    int counter = 0;
-    while (true) {
-      try {
-        workspaceManager.getWorkspace(nameCandidate, namespace);
-        nameCandidate = config.getName() + "_" + ++counter;
-      } catch (NotFoundException nf) {
-        config.setName(nameCandidate);
-        break;
-      }
-    }
-    return config;
-  }
-
   @FunctionalInterface
   private interface ValidationFunction {
     JsonNode validate(String content) throws DevfileFormatException;