From cea70561aaa9b0a50f6ebae860dd692f54ff6083 Mon Sep 17 00:00:00 2001
From: felmer <felmer>
Date: Thu, 11 Feb 2016 11:59:16 +0000
Subject: [PATCH] SSDM-2962: Fixing bug in DefaultAccessController. Unit test
 added.

SVN: 35674
---
 .../DefaultAccessController.java              | 13 ++++----
 .../DefaultAccessControllerTest.java          | 33 +++++++++++++++++--
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/DefaultAccessController.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/DefaultAccessController.java
index 7f8f49af3ca..7c513392552 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/DefaultAccessController.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/DefaultAccessController.java
@@ -147,13 +147,14 @@ public final class DefaultAccessController implements IAccessController
                     Set<RoleWithHierarchy> argumentRoles = getArgumentRoles(method, argument, methodRoles);
                     List<RoleWithIdentifier> relevantRoles = getRelevantRoles(userRoles, argumentRoles);
                     status = checkNotEmpty(relevantRoles, argumentRoles, session);
-                    if (status.isOK())
+                    if (status.isError())
                     {
-                        status = predicateExecutor.evaluate(person, relevantRoles, argument);
-                        if (status.isError())
-                        {
-                            break;
-                        }
+                        break;
+                    }
+                    status = predicateExecutor.evaluate(person, relevantRoles, argument);
+                    if (status.isError())
+                    {
+                        break;
                     }
                 }
             } else
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/authorization/DefaultAccessControllerTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/authorization/DefaultAccessControllerTest.java
index 61f8814cf5b..6d8b4b13c5d 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/authorization/DefaultAccessControllerTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/authorization/DefaultAccessControllerTest.java
@@ -355,6 +355,24 @@ public final class DefaultAccessControllerTest
         context.assertIsSatisfied();
     }
     
+    @Test
+    public void testIsAuthorizedWithGardedArgumentWithRolesOverridden3() throws Exception
+    {
+        final IAuthSession session = AuthorizationTestUtil.createSession();
+        session.tryGetPerson().setRoleAssignments(createRoleAssignments());
+        final Method method = MyInterface.class.getMethod("myMethodWithGardedArgumentWithRolesOverridden3",
+                String.class, String.class, String.class);
+        assertNotNull(method);
+        Argument<?>[] arguments = createArguments(method);
+        
+        final Status authorized = accessController.isAuthorized(session, method, arguments);
+        
+        assertEquals("ERROR: \"None of method roles '[SPACE_POWER_USER, SPACE_ADMIN, INSTANCE_ADMIN]' "
+                + "could be found in roles of user 'John Doe'.\"", authorized.toString());
+        assertEquals(null, project.getDescription());
+        context.assertIsSatisfied();
+    }
+    
     private Argument<?>[] createArguments(final Method method)
     {
         List<Argument<?>> arguments = new ArrayList<>();
@@ -412,13 +430,22 @@ public final class DefaultAccessControllerTest
                 @AuthorizationGuard(name = "ARG1", guardClass = MockPredicate.class,
                         rolesAllowed = { RoleWithHierarchy.SPACE_ETL_SERVER })
                 String argument1);
-        
+
         @RolesAllowed(RoleWithHierarchy.SPACE_OBSERVER)
         @Capability("MY_CAP2")
         public void myMethodWithGardedArgumentWithRolesOverridden2(String sessionToken,
                 @AuthorizationGuard(name = "ARG1", guardClass = MockPredicate.class,
-                rolesAllowed = { RoleWithHierarchy.SPACE_ETL_SERVER })
-        String argument1);
+                        rolesAllowed = { RoleWithHierarchy.SPACE_ETL_SERVER })
+                String argument1);
+
+        @RolesAllowed(RoleWithHierarchy.SPACE_OBSERVER)
+        public void myMethodWithGardedArgumentWithRolesOverridden3(String sessionToken,
+                @AuthorizationGuard(name = "ARG1", guardClass = MockPredicate.class,
+                        rolesAllowed = { RoleWithHierarchy.SPACE_POWER_USER })
+                String argument1,
+                @AuthorizationGuard(name = "ARG2", guardClass = MockPredicate.class,
+                rolesAllowed = { RoleWithHierarchy.SPACE_USER })
+                String argument2);
     }
 
     /**
-- 
GitLab