Skip to content

Commit

Permalink
Merge pull request #346 from codenvy/IDEX-3300
Browse files Browse the repository at this point in the history
IDEX-3300: Add check that workspace belongs to account owner
  • Loading branch information
Igor Vinokur committed Oct 22, 2015
2 parents 4f4406f + b78621f commit 4eb1986
Show file tree
Hide file tree
Showing 2 changed files with 147 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@
import com.wordnik.swagger.annotations.ApiResponse;
import com.wordnik.swagger.annotations.ApiResponses;

import org.eclipse.che.api.account.server.dao.AccountDao;
import org.eclipse.che.api.account.server.dao.Member;
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.core.rest.HttpJsonHelper;
import org.eclipse.che.api.core.rest.HttpServletProxyResponse;
import org.eclipse.che.api.core.rest.Service;
Expand All @@ -31,9 +35,10 @@
import org.eclipse.che.api.runner.dto.ApplicationProcessDescriptor;
import org.eclipse.che.api.runner.dto.ResourcesDescriptor;
import org.eclipse.che.api.runner.dto.RunOptions;
import org.eclipse.che.api.runner.dto.RunRequest;
import org.eclipse.che.api.runner.dto.RunnerDescriptor;
import org.eclipse.che.api.runner.internal.Constants;
import org.eclipse.che.api.workspace.server.dao.MemberDao;
import org.eclipse.che.api.workspace.server.dao.WorkspaceDao;
import org.eclipse.che.commons.env.EnvironmentContext;
import org.eclipse.che.commons.lang.Pair;
import org.eclipse.che.commons.user.User;
Expand All @@ -53,10 +58,10 @@
import javax.ws.rs.QueryParam;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.MediaType;

import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Optional;

/**
* RESTful API for RunQueue.
Expand All @@ -72,8 +77,18 @@ public class RunnerService extends Service {
private static final String START = "{ \"recipe\":\"";
private static final String END = "\" }";

private final RunQueue runQueue;
private final WorkspaceDao workspaceDao;
private final AccountDao accountDao;

@Inject
private RunQueue runQueue;
public RunnerService(RunQueue runQueue,
WorkspaceDao workspaceDao,
AccountDao accountDao) {
this.runQueue = runQueue;
this.workspaceDao = workspaceDao;
this.accountDao = accountDao;
}


@ApiOperation(value = "Run project",
Expand Down Expand Up @@ -118,23 +133,26 @@ public ApplicationProcessDescriptor getStatus(@ApiParam(value = "Workspace ID",
}




@ApiOperation(value = "Get all user running processes",
notes = "Get info on all running processes",
notes = "Get info on all running processes in the scope of account id and project name",
response = ApplicationProcessDescriptor.class,
responseContainer = "List",
position = 3)
@ApiResponses(value = {
@ApiResponse(code = 200, message = "OK"),
@ApiResponse(code = 403, message = "Forbidden"),
@ApiResponse(code = 500, message = "Internal Server Error")})
@GET
@Path("/processes")
@Produces(MediaType.APPLICATION_JSON)
public List<ApplicationProcessDescriptor> getAllRunningProcesses(@ApiParam(value = "Project name")
@Description("project name")
@QueryParam("project") String project) {
return getRunningProcesses(null, project);
@QueryParam("project") String project,
@ApiParam(value = "Account Id")
@Description("account id")
@QueryParam("account") String accountId)
throws ForbiddenException, ServerException {
return getRunningProcesses(null, project, accountId);
}

@ApiOperation(value = "Get run processes for a given workspace",
Expand All @@ -149,55 +167,82 @@ public List<ApplicationProcessDescriptor> getAllRunningProcesses(@ApiParam(value
@Path("/{ws-id}/processes")
@Produces(MediaType.APPLICATION_JSON)
public List<ApplicationProcessDescriptor> getWorkspaceRunningProcesses(@ApiParam(value = "Workspace ID", required = true)
@PathParam("ws-id") String workspace,
@ApiParam(value = "Project name")
@Description("project name")
@QueryParam("project") String project) {
return getRunningProcesses(workspace, project);
@PathParam("ws-id") String workspace,
@ApiParam(value = "Project name")
@Description("project name")
@QueryParam("project") String project)
throws ForbiddenException, ServerException {
return getRunningProcesses(workspace, project, null);
}

protected List<ApplicationProcessDescriptor> getRunningProcesses(String workspace, String project) {
protected List<ApplicationProcessDescriptor> getRunningProcesses(String workspaceId, String project, String accountId)
throws ForbiddenException, ServerException {
//fix project path
if (project != null && !project.startsWith("/")) {
project = '/' + project;
}
final List<ApplicationProcessDescriptor> processes = new LinkedList<>();
final User user = EnvironmentContext.getCurrent().getUser();
if (user != null) {
final String userId = user.getId();
for (RunQueueTask task : runQueue.getTasks()) {
final RunRequest request = task.getRequest();

// add matching task
// first, check user
if (request.getUserId().equals(userId)) {
// now check workspace or project if they're given

// skip task if not correct workspace
if (workspace != null && !request.getWorkspace().equals(workspace)) {
continue;
}
if (user == null) {
return processes;
}

// skip task if not correct project
if (project != null && !request.getProject().equals(project)) {
continue;
}
final String userId = user.getId();

try {
processes.add(task.getDescriptor());
} catch (NotFoundException ignored) {
// NotFoundException is possible and should not be treated as error in this case. Typically it occurs if slave
// runner already cleaned up the task by its internal cleaner but RunQueue doesn't re-check yet slave runner and
// doesn't have actual info about state of slave runner.
} catch (RunnerException e) {
// Decide ignore such error to be able show maximum available info.
LOG.error(e.getMessage(), e);
}
}
if (accountId != null) {
Optional<Member> owner = accountDao.getMembers(accountId)
.stream()
.filter(member -> userId.equals(member.getUserId()) &&
member.getRoles().contains("account/owner"))
.findAny();
if (!owner.isPresent()) {
throw new ForbiddenException("You are not an owner of the specified account");
}
}

for (RunQueueTask task : runQueue.getTasks()) {
// skip task if it does not run in given workspace
if (workspaceId != null && !task.getRequest().getWorkspace().equals(workspaceId)) {
continue;
}

// skip task if it does not run in given project
if (project != null && !task.getRequest().getProject().equals(project)) {
continue;
}

// skip task if it is not running by given account or by given user if account is not specified
if (!isRelatedToAccountOrUser(task, accountId, userId)) {
continue;
}

try {
processes.add(task.getDescriptor());
} catch (NotFoundException ignored) {
// NotFoundException is possible and should not be treated as error in this case. Typically it occurs if slave
// runner already cleaned up the task by its internal cleaner but RunQueue doesn't re-check yet slave runner and
// doesn't have actual info about state of slave runner.
} catch (RunnerException e) {
// Decide ignore such error to be able show maximum available info.
LOG.error(e.getMessage(), e);
}
}
return processes;
}

private boolean isRelatedToAccountOrUser(RunQueueTask task, String accountId, String userId) throws ServerException {
if (accountId != null) {
try {
return accountId.equals(workspaceDao.getById(task.getRequest().getWorkspace()).getAccountId());
} catch (NotFoundException e) {
//Skip task because workspace with id from request is not found
}
return false;
}

return userId.equals(task.getRequest().getUserId());
}

@ApiOperation(value = "Stop run process",
notes = "Stop running process",
response = ApplicationProcessDescriptor.class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,18 @@
*******************************************************************************/
package org.eclipse.che.api.runner;

import org.eclipse.che.api.account.server.dao.AccountDao;
import org.eclipse.che.api.account.server.dao.Member;
import org.eclipse.che.api.core.ForbiddenException;
import org.eclipse.che.api.core.NotFoundException;
import org.eclipse.che.api.project.shared.dto.RunnerEnvironment;
import org.eclipse.che.api.project.shared.dto.RunnerEnvironmentLeaf;
import org.eclipse.che.api.project.shared.dto.RunnerEnvironmentTree;
import org.eclipse.che.api.runner.dto.ApplicationProcessDescriptor;
import org.eclipse.che.api.runner.dto.RunRequest;
import org.eclipse.che.api.runner.dto.RunnerDescriptor;
import org.eclipse.che.api.workspace.server.dao.Workspace;
import org.eclipse.che.api.workspace.server.dao.WorkspaceDao;
import org.eclipse.che.commons.env.EnvironmentContext;
import org.eclipse.che.commons.user.User;
import org.eclipse.che.commons.user.UserImpl;
Expand All @@ -33,10 +38,12 @@
import java.util.Collections;
import java.util.List;

import static org.mockito.Matchers.anyString;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;

Expand All @@ -47,7 +54,11 @@
public class RunnerServiceTest {
private DtoFactory dtoFactory = DtoFactory.getInstance();
@Mock
private RunQueue runQueue;
private RunQueue runQueue;
@Mock
private WorkspaceDao workspaceDao;
@Mock
private AccountDao accountDao;
@InjectMocks
private RunnerService service;

Expand Down Expand Up @@ -238,12 +249,7 @@ private RunQueueTask createTask(User user, String workspace, String projectName)

return runQueueTask;
}

/**
* Check running processes
*
* @throws Exception
*/

@Test
public void testGetRunningProcesses() throws Exception {

Expand All @@ -269,12 +275,12 @@ public void testGetRunningProcesses() throws Exception {
doReturn(tasks).when(runQueue).getTasks();

// we should have all the processes without specifying anything
List<ApplicationProcessDescriptor> processes = service.getRunningProcesses(null, null);
List<ApplicationProcessDescriptor> processes = service.getRunningProcesses(null, null, null);
// list is 3 as we don't have task for dummy user
assertEquals(processes.size(), 3);

// we should have all the processes without specifying anything
processes = service.getRunningProcesses(workspaceCodenvy, null);
processes = service.getRunningProcesses(workspaceCodenvy, null, null);
// list is 1 for this workspace
assertEquals(processes.size(), 1);
ApplicationProcessDescriptor process = processes.get(0);
Expand All @@ -283,29 +289,68 @@ public void testGetRunningProcesses() throws Exception {


// we should have all the processes without specifying anything
processes = service.getRunningProcesses(workspaceCodenvy, projectA);
processes = service.getRunningProcesses(workspaceCodenvy, projectA, null);
// list is 1 as we only have one matching workspace + project
assertEquals(processes.size(), 1);


// we should have all the processes without specifying anything
processes = service.getRunningProcesses(null, projectA);
processes = service.getRunningProcesses(null, projectA, null);
// list is 2 as we have two workspaces with this project name
assertEquals(processes.size(), 2);

// no workspace with that name
processes = service.getRunningProcesses("dummy", null);
processes = service.getRunningProcesses("dummy", null, null);
assertEquals(processes.size(), 0);

// no project with that name
processes = service.getRunningProcesses(null, "project");
processes = service.getRunningProcesses(null, "project", null);
assertEquals(processes.size(), 0);

// no project with that workspace + name
processes = service.getRunningProcesses("dummy", "project");
processes = service.getRunningProcesses("dummy", "project", null);
assertEquals(processes.size(), 0);
}

@Test
public void shouldGetRunningProcessWithAccount() throws Exception {
User user = new UserImpl("User", "id-2314", "token-2323", Collections.<String>emptyList(), false);
EnvironmentContext context = EnvironmentContext.getCurrent();
context.setUser(user);
Workspace workspace = mock(Workspace.class);
Workspace dummyWorkspace = mock(Workspace.class);
Member member = mock(Member.class);
List<RunQueueTask> tasks = Collections.singletonList(createTask(user, "workspaceId", "project"));

doReturn(tasks).when(runQueue).getTasks();
doReturn(Collections.singletonList(member)).when(accountDao).getMembers("accountId");
doReturn("id-2314").when(member).getUserId();
doReturn(Collections.singletonList("account/owner")).when(member).getRoles();
doReturn("accountId").when(workspace).getAccountId();
doReturn("dummyAccountId").when(dummyWorkspace).getAccountId();
doReturn(workspace).when(workspaceDao).getById("workspaceId");
doReturn(dummyWorkspace).when(workspaceDao).getById("dummyWorkspace");

List<ApplicationProcessDescriptor> processes = service.getRunningProcesses(null, null, "accountId");
assertEquals(processes.size(), 1);
}

@Test(expectedExceptions = ForbiddenException.class)
public void shouldThrowForbiddenExceptionWhenIsNotAnAccountOwner() throws Exception {
User user = new UserImpl("User", "id-2314", "token-2323", Collections.<String>emptyList(), false);
EnvironmentContext context = EnvironmentContext.getCurrent();
context.setUser(user);
Member member = mock(Member.class);
List<RunQueueTask> tasks = Collections.singletonList(createTask(user, "workspaceId", "project"));

doReturn(tasks).when(runQueue).getTasks();
doReturn(Collections.singletonList(member)).when(accountDao).getMembers("accountId");
doReturn("id-2314").when(member).getUserId();
doReturn(Collections.singletonList("account/member")).when(member).getRoles();

service.getRunningProcesses(null, null, "accountId");
}

private List<RemoteRunnerServer> createRunners() throws Exception {
List<RemoteRunnerServer> servers = new ArrayList<>(1);
RemoteRunnerServer server1 = mock(RemoteRunnerServer.class);
Expand Down

0 comments on commit 4eb1986

Please sign in to comment.