From e62dbb72cbebb6c972898d7cac2626e7e7455fbf Mon Sep 17 00:00:00 2001 From: Sergii Leshchenko Date: Tue, 28 Aug 2018 16:19:59 +0300 Subject: [PATCH] CHE-10861 Add permissions check for organization related remote subscriptions --- .../api/OrganizationApiModule.java | 2 + ...rganizationEventsWebsocketBroadcaster.java | 5 +- ...onRemoteSubscriptionPermissionsChecks.java | 110 ++++++++++++ ...moteSubscriptionPermissionsChecksTest.java | 156 ++++++++++++++++++ 4 files changed, 270 insertions(+), 3 deletions(-) create mode 100644 multiuser/api/che-multiuser-api-organization/src/main/java/org/eclipse/che/multiuser/organization/api/permissions/OrganizationRemoteSubscriptionPermissionsChecks.java create mode 100644 multiuser/api/che-multiuser-api-organization/src/test/java/org/eclipse/che/multiuser/organization/api/permissions/OrganizationRemoteSubscriptionPermissionsChecksTest.java diff --git a/multiuser/api/che-multiuser-api-organization/src/main/java/org/eclipse/che/multiuser/organization/api/OrganizationApiModule.java b/multiuser/api/che-multiuser-api-organization/src/main/java/org/eclipse/che/multiuser/organization/api/OrganizationApiModule.java index cdec03cd1c..c86516a7d0 100644 --- a/multiuser/api/che-multiuser-api-organization/src/main/java/org/eclipse/che/multiuser/organization/api/OrganizationApiModule.java +++ b/multiuser/api/che-multiuser-api-organization/src/main/java/org/eclipse/che/multiuser/organization/api/OrganizationApiModule.java @@ -24,6 +24,7 @@ import org.eclipse.che.multiuser.organization.api.listener.RemoveOrganizationOnL import org.eclipse.che.multiuser.organization.api.notification.OrganizationNotificationEmailSender; import org.eclipse.che.multiuser.organization.api.permissions.OrganizationDomain; import org.eclipse.che.multiuser.organization.api.permissions.OrganizationPermissionsFilter; +import org.eclipse.che.multiuser.organization.api.permissions.OrganizationRemoteSubscriptionPermissionsChecks; import org.eclipse.che.multiuser.organization.api.permissions.OrganizationResourceDistributionServicePermissionsFilter; import org.eclipse.che.multiuser.organization.api.permissions.OrganizationalAccountPermissionsChecker; import org.eclipse.che.multiuser.organization.api.resource.DefaultOrganizationResourcesProvider; @@ -43,6 +44,7 @@ public class OrganizationApiModule extends AbstractModule { protected void configure() { bind(OrganizationService.class); bind(OrganizationPermissionsFilter.class); + bind(OrganizationRemoteSubscriptionPermissionsChecks.class); bind(RemoveOrganizationOnLastUserRemovedEventSubscriber.class).asEagerSingleton(); Multibinder.newSetBinder(binder(), DefaultResourcesProvider.class) diff --git a/multiuser/api/che-multiuser-api-organization/src/main/java/org/eclipse/che/multiuser/organization/api/listener/OrganizationEventsWebsocketBroadcaster.java b/multiuser/api/che-multiuser-api-organization/src/main/java/org/eclipse/che/multiuser/organization/api/listener/OrganizationEventsWebsocketBroadcaster.java index 6a2f376cad..5b464dd6be 100644 --- a/multiuser/api/che-multiuser-api-organization/src/main/java/org/eclipse/che/multiuser/organization/api/listener/OrganizationEventsWebsocketBroadcaster.java +++ b/multiuser/api/che-multiuser-api-organization/src/main/java/org/eclipse/che/multiuser/organization/api/listener/OrganizationEventsWebsocketBroadcaster.java @@ -33,9 +33,8 @@ public class OrganizationEventsWebsocketBroadcaster { private final RemoteSubscriptionManager remoteSubscriptionManager; - private static final String ORGANIZATION_MEMBERSHIP_METHOD_NAME = - "organization/membershipChanged"; - private static final String ORGANIZATION_CHANGED_METHOD_NAME = "organization/statusChanged"; + public static final String ORGANIZATION_MEMBERSHIP_METHOD_NAME = "organization/membershipChanged"; + public static final String ORGANIZATION_CHANGED_METHOD_NAME = "organization/statusChanged"; @Inject public OrganizationEventsWebsocketBroadcaster( diff --git a/multiuser/api/che-multiuser-api-organization/src/main/java/org/eclipse/che/multiuser/organization/api/permissions/OrganizationRemoteSubscriptionPermissionsChecks.java b/multiuser/api/che-multiuser-api-organization/src/main/java/org/eclipse/che/multiuser/organization/api/permissions/OrganizationRemoteSubscriptionPermissionsChecks.java new file mode 100644 index 0000000000..d61ad07c22 --- /dev/null +++ b/multiuser/api/che-multiuser-api-organization/src/main/java/org/eclipse/che/multiuser/organization/api/permissions/OrganizationRemoteSubscriptionPermissionsChecks.java @@ -0,0 +1,110 @@ +/* + * 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.organization.api.permissions; + +import static org.eclipse.che.multiuser.organization.api.listener.OrganizationEventsWebsocketBroadcaster.ORGANIZATION_CHANGED_METHOD_NAME; +import static org.eclipse.che.multiuser.organization.api.listener.OrganizationEventsWebsocketBroadcaster.ORGANIZATION_MEMBERSHIP_METHOD_NAME; + +import com.google.common.annotations.VisibleForTesting; +import java.util.Map; +import javax.inject.Inject; +import javax.inject.Singleton; +import org.eclipse.che.api.core.ConflictException; +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.commons.env.EnvironmentContext; +import org.eclipse.che.multiuser.api.permission.server.PermissionsManager; +import org.eclipse.che.multiuser.api.permission.server.jsonrpc.RemoteSubscriptionPermissionCheck; +import org.eclipse.che.multiuser.api.permission.server.jsonrpc.RemoteSubscriptionPermissionManager; +import org.eclipse.che.multiuser.api.permission.server.model.impl.AbstractPermissions; +import org.eclipse.che.multiuser.organization.api.listener.OrganizationEventsWebsocketBroadcaster; + +/** + * Holds and registers permissions checks for organization related events. + * + *

Covers events published via {@link OrganizationEventsWebsocketBroadcaster}. + * + * @author Sergii Leshchenko + */ +@Singleton +public class OrganizationRemoteSubscriptionPermissionsChecks { + + private final PermissionsManager permissionsManager; + + @Inject + public OrganizationRemoteSubscriptionPermissionsChecks(PermissionsManager permissionsManager) { + this.permissionsManager = permissionsManager; + } + + @Inject + public void register(RemoteSubscriptionPermissionManager permissionFilter) { + MembershipsChangedSubscriptionCheck membershipsEventsCheck = + new MembershipsChangedSubscriptionCheck(); + + permissionFilter.registerCheck(membershipsEventsCheck, ORGANIZATION_MEMBERSHIP_METHOD_NAME); + + OrganizationChangedSubscriptionCheck organizationChangedCheck = + new OrganizationChangedSubscriptionCheck(permissionsManager); + permissionFilter.registerCheck(organizationChangedCheck, ORGANIZATION_CHANGED_METHOD_NAME); + } + + @VisibleForTesting + static class MembershipsChangedSubscriptionCheck implements RemoteSubscriptionPermissionCheck { + + @Override + public void check(String methodName, Map scope) throws ForbiddenException { + String userId = scope.get("userId"); + if (userId == null) { + throw new ForbiddenException("User id must be specified in scope"); + } + + String currentUserId = EnvironmentContext.getCurrent().getSubject().getUserId(); + + if (!currentUserId.equals(userId)) { + throw new ForbiddenException("It is only allowed to listen to own memberships changes"); + } + } + } + + @VisibleForTesting + static class OrganizationChangedSubscriptionCheck implements RemoteSubscriptionPermissionCheck { + + private final PermissionsManager permissionsManager; + + public OrganizationChangedSubscriptionCheck(PermissionsManager permissionsManager) { + this.permissionsManager = permissionsManager; + } + + @Override + public void check(String methodName, Map scope) throws ForbiddenException { + String organizationId = scope.get("organizationId"); + if (organizationId == null) { + throw new ForbiddenException("Organization id must be specified in scope"); + } + + String currentUserId = EnvironmentContext.getCurrent().getSubject().getUserId(); + + try { + // check if user has any permissions in organisation + // to listen to related events + AbstractPermissions permissions = + permissionsManager.get(currentUserId, OrganizationDomain.DOMAIN_ID, organizationId); + } catch (ConflictException | ServerException e) { + throw new ForbiddenException("Error occurred while permission fetching: " + e.getMessage()); + } catch (NotFoundException e) { + throw new ForbiddenException( + "User doesn't have any permissions for the specified organization"); + } + } + } +} diff --git a/multiuser/api/che-multiuser-api-organization/src/test/java/org/eclipse/che/multiuser/organization/api/permissions/OrganizationRemoteSubscriptionPermissionsChecksTest.java b/multiuser/api/che-multiuser-api-organization/src/test/java/org/eclipse/che/multiuser/organization/api/permissions/OrganizationRemoteSubscriptionPermissionsChecksTest.java new file mode 100644 index 0000000000..38ea0ad861 --- /dev/null +++ b/multiuser/api/che-multiuser-api-organization/src/test/java/org/eclipse/che/multiuser/organization/api/permissions/OrganizationRemoteSubscriptionPermissionsChecksTest.java @@ -0,0 +1,156 @@ +/* + * 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.organization.api.permissions; + +import static org.eclipse.che.multiuser.organization.api.listener.OrganizationEventsWebsocketBroadcaster.ORGANIZATION_CHANGED_METHOD_NAME; +import static org.eclipse.che.multiuser.organization.api.listener.OrganizationEventsWebsocketBroadcaster.ORGANIZATION_MEMBERSHIP_METHOD_NAME; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.google.common.collect.ImmutableMap; +import java.util.Collections; +import org.eclipse.che.api.core.ForbiddenException; +import org.eclipse.che.api.core.NotFoundException; +import org.eclipse.che.commons.env.EnvironmentContext; +import org.eclipse.che.commons.subject.Subject; +import org.eclipse.che.multiuser.api.permission.server.PermissionsManager; +import org.eclipse.che.multiuser.api.permission.server.jsonrpc.RemoteSubscriptionPermissionManager; +import org.eclipse.che.multiuser.api.permission.server.model.impl.AbstractPermissions; +import org.eclipse.che.multiuser.organization.api.permissions.OrganizationRemoteSubscriptionPermissionsChecks.MembershipsChangedSubscriptionCheck; +import org.eclipse.che.multiuser.organization.api.permissions.OrganizationRemoteSubscriptionPermissionsChecks.OrganizationChangedSubscriptionCheck; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; + +/** + * Tests {@link OrganizationRemoteSubscriptionPermissionsChecks}. + * + * @author Sergii Leshchenko + */ +@Listeners(MockitoTestNGListener.class) +public class OrganizationRemoteSubscriptionPermissionsChecksTest { + @Mock private Subject subject; + + @Mock private PermissionsManager permissionsManager; + @Mock private RemoteSubscriptionPermissionManager permissionManager; + + @InjectMocks private OrganizationRemoteSubscriptionPermissionsChecks permissionsChecks; + + @BeforeMethod + public void setUp() { + EnvironmentContext.getCurrent().setSubject(subject); + } + + @AfterMethod + public void tearDown() { + EnvironmentContext.reset(); + } + + @Test + public void shouldRegisterChecks() { + // when + permissionsChecks.register(permissionManager); + + // then + verify(permissionManager) + .registerCheck( + any(OrganizationChangedSubscriptionCheck.class), eq(ORGANIZATION_CHANGED_METHOD_NAME)); + verify(permissionManager) + .registerCheck( + any(MembershipsChangedSubscriptionCheck.class), + eq(ORGANIZATION_MEMBERSHIP_METHOD_NAME)); + } + + @Test( + expectedExceptions = ForbiddenException.class, + expectedExceptionsMessageRegExp = "User id must be specified in scope") + public void shouldThrowExceptionIfUserIdIsMissing() throws Exception { + // given + MembershipsChangedSubscriptionCheck check = new MembershipsChangedSubscriptionCheck(); + when(subject.getUserId()).thenReturn("user2"); + + // when + check.check(ORGANIZATION_MEMBERSHIP_METHOD_NAME, Collections.emptyMap()); + } + + @Test( + expectedExceptions = ForbiddenException.class, + expectedExceptionsMessageRegExp = "It is only allowed to listen to own memberships changes") + public void shouldThrowExceptionIfUserTryToListenToForeignMemberships() throws Exception { + // given + MembershipsChangedSubscriptionCheck check = new MembershipsChangedSubscriptionCheck(); + when(subject.getUserId()).thenReturn("user2"); + + // when + check.check(ORGANIZATION_MEMBERSHIP_METHOD_NAME, ImmutableMap.of("userId", "user1")); + } + + @Test + public void shouldDoNothingIfUserTryToListenToOwnMemberships() throws Exception { + // given + MembershipsChangedSubscriptionCheck check = new MembershipsChangedSubscriptionCheck(); + when(subject.getUserId()).thenReturn("user1"); + + // when + check.check(ORGANIZATION_MEMBERSHIP_METHOD_NAME, ImmutableMap.of("userId", "user1")); + } + + @Test( + expectedExceptions = ForbiddenException.class, + expectedExceptionsMessageRegExp = "Organization id must be specified in scope") + public void shouldThrowExceptionIfOrganizationIdIsMissing() throws Exception { + // given + OrganizationChangedSubscriptionCheck check = + new OrganizationChangedSubscriptionCheck(permissionsManager); + + // when + check.check(ORGANIZATION_MEMBERSHIP_METHOD_NAME, Collections.emptyMap()); + } + + @Test( + expectedExceptions = ForbiddenException.class, + expectedExceptionsMessageRegExp = + "User doesn't have any permissions for the specified organization") + public void shouldThrowExceptionIfUserDoesNotHaveAnyPermissionsToRequestedOrganization() + throws Exception { + // given + OrganizationChangedSubscriptionCheck check = + new OrganizationChangedSubscriptionCheck(permissionsManager); + when(subject.getUserId()).thenReturn("user1"); + when(permissionsManager.get("user1", OrganizationDomain.DOMAIN_ID, "org123")) + .thenThrow(new NotFoundException("")); + + // when + check.check(ORGANIZATION_MEMBERSHIP_METHOD_NAME, ImmutableMap.of("organizationId", "org123")); + } + + @Test + public void shouldDoNothingIfUserTryToListenEventsOfOrganizationWhereHeHasPermissions() + throws Exception { + // given + OrganizationChangedSubscriptionCheck check = + new OrganizationChangedSubscriptionCheck(permissionsManager); + when(subject.getUserId()).thenReturn("user1"); + when(permissionsManager.get("user1", OrganizationDomain.DOMAIN_ID, "org123")) + .thenReturn(mock(AbstractPermissions.class)); + + // when + check.check(ORGANIZATION_MEMBERSHIP_METHOD_NAME, ImmutableMap.of("organizationId", "org123")); + } +}