fixup! CHE-10861 Add permissions check for organization related remot…

…e subscriptions
eclipse-che · Aug 31, 2018 · db03ee2 · db03ee2
1 parent 2e27a47
commit db03ee2
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 27 deletions.
diff --git a/...ltiuser/organization/api/permissions/OrganizationRemoteSubscriptionPermissionsChecks.java b/...ltiuser/organization/api/permissions/OrganizationRemoteSubscriptionPermissionsChecks.java
@@ -48,19 +48,18 @@ public OrganizationRemoteSubscriptionPermissionsChecks(PermissionsManager permis
 
   @Inject
   public void register(RemoteSubscriptionPermissionManager permissionFilter) {
-    OrganizationMembershipsChangedSubscriptionPermissionsCheck membershipsEventsCheck =
-        new OrganizationMembershipsChangedSubscriptionPermissionsCheck();
+    MembershipsChangedSubscriptionCheck membershipsEventsCheck =
+        new MembershipsChangedSubscriptionCheck();
 
     permissionFilter.registerCheck(membershipsEventsCheck, ORGANIZATION_MEMBERSHIP_METHOD_NAME);
 
-    OrganizationChangedSubscriptionPermissionsCheck organizationChangedCheck =
-        new OrganizationChangedSubscriptionPermissionsCheck(permissionsManager);
+    OrganizationChangedSubscriptionCheck organizationChangedCheck =
+        new OrganizationChangedSubscriptionCheck(permissionsManager);
     permissionFilter.registerCheck(organizationChangedCheck, ORGANIZATION_CHANGED_METHOD_NAME);
   }
 
   @VisibleForTesting
-  static class OrganizationMembershipsChangedSubscriptionPermissionsCheck
-      implements RemoteSubscriptionPermissionCheck {
+  static class MembershipsChangedSubscriptionCheck implements RemoteSubscriptionPermissionCheck {
 
     @Override
     public void check(String methodName, Map<String, String> scope) throws ForbiddenException {
@@ -78,12 +77,11 @@ public void check(String methodName, Map<String, String> scope) throws Forbidden
   }
 
   @VisibleForTesting
-  static class OrganizationChangedSubscriptionPermissionsCheck
-      implements RemoteSubscriptionPermissionCheck {
+  static class OrganizationChangedSubscriptionCheck implements RemoteSubscriptionPermissionCheck {
 
     private final PermissionsManager permissionsManager;
 
-    public OrganizationChangedSubscriptionPermissionsCheck(PermissionsManager permissionsManager) {
+    public OrganizationChangedSubscriptionCheck(PermissionsManager permissionsManager) {
       this.permissionsManager = permissionsManager;
     }
 
@@ -97,10 +95,12 @@ public void check(String methodName, Map<String, String> scope) throws Forbidden
       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 occured while permission fetching: " + e.getMessage());
+        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/...ser/organization/api/permissions/OrganizationRemoteSubscriptionPermissionsChecksTest.java b/...ser/organization/api/permissions/OrganizationRemoteSubscriptionPermissionsChecksTest.java
@@ -28,8 +28,8 @@
 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.OrganizationChangedSubscriptionPermissionsCheck;
-import org.eclipse.che.multiuser.organization.api.permissions.OrganizationRemoteSubscriptionPermissionsChecks.OrganizationMembershipsChangedSubscriptionPermissionsCheck;
+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;
@@ -70,11 +70,10 @@ public void shouldRegisterChecks() {
     // then
     verify(permissionManager)
         .registerCheck(
-            any(OrganizationChangedSubscriptionPermissionsCheck.class),
-            eq(ORGANIZATION_CHANGED_METHOD_NAME));
+            any(OrganizationChangedSubscriptionCheck.class), eq(ORGANIZATION_CHANGED_METHOD_NAME));
     verify(permissionManager)
         .registerCheck(
-            any(OrganizationMembershipsChangedSubscriptionPermissionsCheck.class),
+            any(MembershipsChangedSubscriptionCheck.class),
             eq(ORGANIZATION_MEMBERSHIP_METHOD_NAME));
   }
 
@@ -83,8 +82,7 @@ public void shouldRegisterChecks() {
       expectedExceptionsMessageRegExp = "User id must be specified in scope")
   public void shouldThrowExceptionIfUserIdIsMissing() throws Exception {
     // given
-    OrganizationMembershipsChangedSubscriptionPermissionsCheck check =
-        new OrganizationMembershipsChangedSubscriptionPermissionsCheck();
+    MembershipsChangedSubscriptionCheck check = new MembershipsChangedSubscriptionCheck();
     when(subject.getUserId()).thenReturn("user2");
 
     // when
@@ -96,8 +94,7 @@ public void shouldThrowExceptionIfUserIdIsMissing() throws Exception {
       expectedExceptionsMessageRegExp = "It is only allowed to listen to own memberships changes")
   public void shouldThrowExceptionIfUserTryToListenToForeignMemberships() throws Exception {
     // given
-    OrganizationMembershipsChangedSubscriptionPermissionsCheck check =
-        new OrganizationMembershipsChangedSubscriptionPermissionsCheck();
+    MembershipsChangedSubscriptionCheck check = new MembershipsChangedSubscriptionCheck();
     when(subject.getUserId()).thenReturn("user2");
 
     // when
@@ -107,8 +104,7 @@ public void shouldThrowExceptionIfUserTryToListenToForeignMemberships() throws E
   @Test
   public void shouldDoNothingIfUserTryToListenToOwnMemberships() throws Exception {
     // given
-    OrganizationMembershipsChangedSubscriptionPermissionsCheck check =
-        new OrganizationMembershipsChangedSubscriptionPermissionsCheck();
+    MembershipsChangedSubscriptionCheck check = new MembershipsChangedSubscriptionCheck();
     when(subject.getUserId()).thenReturn("user1");
 
     // when
@@ -120,8 +116,8 @@ public void shouldDoNothingIfUserTryToListenToOwnMemberships() throws Exception
       expectedExceptionsMessageRegExp = "Organization id must be specified in scope")
   public void shouldThrowExceptionIfOrganizationIdIsMissing() throws Exception {
     // given
-    OrganizationChangedSubscriptionPermissionsCheck check =
-        new OrganizationChangedSubscriptionPermissionsCheck(permissionsManager);
+    OrganizationChangedSubscriptionCheck check =
+        new OrganizationChangedSubscriptionCheck(permissionsManager);
 
     // when
     check.check(ORGANIZATION_MEMBERSHIP_METHOD_NAME, Collections.emptyMap());
@@ -134,8 +130,8 @@ public void shouldThrowExceptionIfOrganizationIdIsMissing() throws Exception {
   public void shouldThrowExceptionIfUserDoesNotHaveAnyPermissionsToRequestedOrganization()
       throws Exception {
     // given
-    OrganizationChangedSubscriptionPermissionsCheck check =
-        new OrganizationChangedSubscriptionPermissionsCheck(permissionsManager);
+    OrganizationChangedSubscriptionCheck check =
+        new OrganizationChangedSubscriptionCheck(permissionsManager);
     when(subject.getUserId()).thenReturn("user1");
     when(permissionsManager.get("user1", OrganizationDomain.DOMAIN_ID, "org123"))
         .thenThrow(new NotFoundException(""));
@@ -148,8 +144,8 @@ public void shouldThrowExceptionIfUserDoesNotHaveAnyPermissionsToRequestedOrgani
   public void shouldDoNothingIfUserTryToListenEventsOfOrganizationWhereHeHasPermissions()
       throws Exception {
     // given
-    OrganizationChangedSubscriptionPermissionsCheck check =
-        new OrganizationChangedSubscriptionPermissionsCheck(permissionsManager);
+    OrganizationChangedSubscriptionCheck check =
+        new OrganizationChangedSubscriptionCheck(permissionsManager);
     when(subject.getUserId()).thenReturn("user1");
     when(permissionsManager.get("user1", OrganizationDomain.DOMAIN_ID, "org123"))
         .thenReturn(mock(AbstractPermissions.class));