From dc26c96261e95ba6718fe611e21713208a12debc Mon Sep 17 00:00:00 2001
From: buczekp <buczekp>
Date: Tue, 22 Sep 2009 07:45:01 +0000
Subject: [PATCH] [LMS-1183] added authorization (with tests) for deletion and
 editing of custom filters

SVN: 12679
---
 .../generic/server/AbstractServer.java        |   6 +-
 .../authorization/PredicateExecutor.java      |   8 +-
 .../dataaccess/IAuthorizationDAOFactory.java  |  11 +-
 .../server/dataaccess/IDAOFactory.java        |   2 -
 .../db/AuthorizationDAOFactory.java           |   9 ++
 .../server/dataaccess/db/DAOFactory.java      |   8 --
 .../openbis/generic/shared/ICommonServer.java |   9 +-
 .../IAuthorizationDataProvider.java           |   6 +
 .../predicate/FilterTechIdPredicate.java      | 133 ++++++++++++++++++
 .../validator/FilterValidator.java            |  16 ++-
 .../shared/ICommonServer.java.expected        |   9 +-
 .../authorization/AuthorizationTestCase.java  |  38 ++++-
 .../validator/FilterValidatorTest.java        |  89 ++++++++++++
 13 files changed, 312 insertions(+), 32 deletions(-)
 create mode 100644 openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/authorization/predicate/FilterTechIdPredicate.java
 create mode 100644 openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/authorization/validator/FilterValidatorTest.java

diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/AbstractServer.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/AbstractServer.java
index cb0b6353906..b2863e496a5 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/AbstractServer.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/AbstractServer.java
@@ -186,7 +186,7 @@ public abstract class AbstractServer<T extends IServer> extends AbstractServiceW
     {
         return dataStoreBaseURLProvider.getDataStoreBaseURL();
     }
-    
+
     //
     // IServer
     //
@@ -239,8 +239,8 @@ public abstract class AbstractServer<T extends IServer> extends AbstractServiceW
         }
         if (isFirstLoggedUser)
         {
-            // First logged user does have any role assignment yet. Make him database instance
-            // administrator.
+            // First logged user does not have any role assignment yet.
+            // Make him database instance administrator.
             final PersonPE systemUser = getSystemUser(persons);
             final RoleAssignmentPE roleAssignment = createRoleAssigment(systemUser, person);
             person.setRoleAssignments(Collections.singleton(roleAssignment));
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/PredicateExecutor.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/PredicateExecutor.java
index 5613ac27285..89d895d8e74 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/PredicateExecutor.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/PredicateExecutor.java
@@ -34,6 +34,7 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.DataPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.DatabaseInstancePE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ExternalDataPE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.FilterPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.GroupPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ProjectPE;
@@ -170,7 +171,7 @@ public final class PredicateExecutor
 
         private final Map<String, DatabaseInstancePE> uuidToDbInstanceMap =
                 new HashMap<String, DatabaseInstancePE>();
-        
+
         private final DatabaseInstancePE homeDatabaseInstance;
 
         AuthorizationDataProvider(IAuthorizationDAOFactory daoFactory)
@@ -242,5 +243,10 @@ public final class PredicateExecutor
             return daoFactory.getSampleDAO().getByTechId(techId);
         }
 
+        public FilterPE getFilter(TechId techId)
+        {
+            return daoFactory.getFilterDAO().getByTechId(techId);
+        }
+
     }
 }
\ No newline at end of file
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/IAuthorizationDAOFactory.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/IAuthorizationDAOFactory.java
index d2cdb16a188..7cfadaaf0ad 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/IAuthorizationDAOFactory.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/IAuthorizationDAOFactory.java
@@ -29,7 +29,7 @@ public interface IAuthorizationDAOFactory
 {
     /** Returns the persistency resources used to create DAO's. */
     public PersistencyResources getPersistencyResources();
-    
+
     /**
      * Returns the current {@link DatabaseInstancePE}.
      */
@@ -74,12 +74,17 @@ public interface IAuthorizationDAOFactory
      * Returns the implementation of {@link ISampleDAO}.
      */
     public ISampleDAO getSampleDAO();
-    
+
+    /**
+     * Returns an implementation of {@link IFilterDAO}.
+     */
+    public IFilterDAO getFilterDAO();
+
     /**
      * Disables interaction with the second level cache for the current Hibernate session.
      */
     public void disableSecondLevelCacheForSession();
-    
+
     /**
      * Returns the Hibernate session factory.
      */
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/IDAOFactory.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/IDAOFactory.java
index e9e236a9dbf..5e156c8d5cb 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/IDAOFactory.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/IDAOFactory.java
@@ -93,6 +93,4 @@ public interface IDAOFactory extends IAuthorizationDAOFactory
     /** Returns an implementation of {@link IAuthorizationGroupDAO}. */
     public IAuthorizationGroupDAO getAuthorizationGroupDAO();
 
-    /** Returns an implementation of {@link IFilterDAO}. */
-    public IFilterDAO getFilterDAO();
 }
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/AuthorizationDAOFactory.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/AuthorizationDAOFactory.java
index 259c802770f..4620068b30c 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/AuthorizationDAOFactory.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/AuthorizationDAOFactory.java
@@ -31,6 +31,7 @@ import ch.systemsx.cisd.openbis.generic.server.dataaccess.IAuthorizationDAOFacto
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.IDatabaseInstanceDAO;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.IExperimentDAO;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.IExternalDataDAO;
+import ch.systemsx.cisd.openbis.generic.server.dataaccess.IFilterDAO;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.IGroupDAO;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.IPersonDAO;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.IProjectDAO;
@@ -68,6 +69,8 @@ public class AuthorizationDAOFactory implements IAuthorizationDAOFactory
 
     private final ISampleDAO sampleDAO;
 
+    private final IFilterDAO filterDAO;
+
     private final PersistencyResources persistencyResources;
 
     public AuthorizationDAOFactory(final DatabaseConfigurationContext context,
@@ -83,6 +86,7 @@ public class AuthorizationDAOFactory implements IAuthorizationDAOFactory
         experimentDAO = new ExperimentDAO(sessionFactory, homeDatabaseInstance);
         projectDAO = new ProjectDAO(sessionFactory, homeDatabaseInstance);
         sampleDAO = new SampleDAO(sessionFactory, homeDatabaseInstance);
+        filterDAO = new FilterDAO(sessionFactory, homeDatabaseInstance);
     }
 
     public final PersistencyResources getPersistencyResources()
@@ -199,6 +203,11 @@ public class AuthorizationDAOFactory implements IAuthorizationDAOFactory
         return sampleDAO;
     }
 
+    public IFilterDAO getFilterDAO()
+    {
+        return filterDAO;
+    }
+
     public void disableSecondLevelCacheForSession()
     {
         SessionFactory sessionFactory = persistencyResources.getSessionFactoryOrNull();
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/DAOFactory.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/DAOFactory.java
index 8ab6bc9abc2..9a2a4440ff5 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/DAOFactory.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/DAOFactory.java
@@ -31,7 +31,6 @@ import ch.systemsx.cisd.openbis.generic.server.dataaccess.IEntityPropertyTypeDAO
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.IEntityTypeDAO;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.IEventDAO;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.IFileFormatTypeDAO;
-import ch.systemsx.cisd.openbis.generic.server.dataaccess.IFilterDAO;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.IHibernateSearchDAO;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.ILocatorTypeDAO;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.IMaterialDAO;
@@ -86,8 +85,6 @@ public final class DAOFactory extends AuthorizationDAOFactory implements IDAOFac
 
     private final IAuthorizationGroupDAO authorizationGroupDAO;
 
-    private final IFilterDAO filterDAO;
-
     public DAOFactory(final DatabaseConfigurationContext context,
             final SessionFactory sessionFactory)
     {
@@ -108,7 +105,6 @@ public final class DAOFactory extends AuthorizationDAOFactory implements IDAOFac
         permIdDAO = new PermIdDAO(sessionFactory, databaseInstance);
         eventDAO = new EventDAO(sessionFactory, databaseInstance);
         authorizationGroupDAO = new AuthorizationGroupDAO(sessionFactory, databaseInstance);
-        filterDAO = new FilterDAO(sessionFactory, databaseInstance);
         final EntityKind[] entityKinds = EntityKind.values();
         for (final EntityKind entityKind : entityKinds)
         {
@@ -209,8 +205,4 @@ public final class DAOFactory extends AuthorizationDAOFactory implements IDAOFac
         return authorizationGroupDAO;
     }
 
-    public IFilterDAO getFilterDAO()
-    {
-        return filterDAO;
-    }
 }
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/ICommonServer.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/ICommonServer.java
index 307e80f0b38..6cadb974e6c 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/ICommonServer.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/ICommonServer.java
@@ -36,6 +36,7 @@ import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.AbstractT
 import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.AbstractTechIdPredicate.ExperimentTechIdPredicate;
 import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.AbstractTechIdPredicate.GroupTechIdPredicate;
 import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.AbstractTechIdPredicate.ProjectTechIdPredicate;
+import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.FilterTechIdPredicate.DeleteFilterTechIdPredicate;
 import ch.systemsx.cisd.openbis.generic.shared.authorization.validator.ExternalDataValidator;
 import ch.systemsx.cisd.openbis.generic.shared.authorization.validator.FilterValidator;
 import ch.systemsx.cisd.openbis.generic.shared.authorization.validator.GroupValidator;
@@ -925,8 +926,10 @@ public interface ICommonServer extends IServer
      * Deletes specified filters.
      */
     @Transactional
-    @RolesAllowed(RoleSet.INSTANCE_ADMIN)
-    @DatabaseCreateOrDeleteModification(value = ObjectKind.GROUP)
-    public void deleteFilters(String sessionToken, List<TechId> groupIds);
+    @RolesAllowed(RoleSet.POWER_USER)
+    @DatabaseCreateOrDeleteModification(value = ObjectKind.FILTER)
+    public void deleteFilters(
+            String sessionToken,
+            @AuthorizationGuard(guardClass = DeleteFilterTechIdPredicate.class) List<TechId> filterIds);
 
 }
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/authorization/IAuthorizationDataProvider.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/authorization/IAuthorizationDataProvider.java
index d34ce600a53..3ae86cae14e 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/authorization/IAuthorizationDataProvider.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/authorization/IAuthorizationDataProvider.java
@@ -20,6 +20,7 @@ import java.util.List;
 
 import ch.systemsx.cisd.openbis.generic.shared.IDatabaseInstanceFinder;
 import ch.systemsx.cisd.openbis.generic.shared.basic.TechId;
+import ch.systemsx.cisd.openbis.generic.shared.dto.FilterPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.GroupPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ProjectPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePE;
@@ -55,4 +56,9 @@ public interface IAuthorizationDataProvider extends IDatabaseInstanceFinder
      */
     public SamplePE getSample(TechId techId);
 
+    /**
+     * Returns the filter with given <var>techId</var>
+     */
+    public FilterPE getFilter(TechId techId);
+
 }
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/authorization/predicate/FilterTechIdPredicate.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/authorization/predicate/FilterTechIdPredicate.java
new file mode 100644
index 00000000000..c14d43a8870
--- /dev/null
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/authorization/predicate/FilterTechIdPredicate.java
@@ -0,0 +1,133 @@
+/*
+ * Copyright 2009 ETH Zuerich, CISD
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package ch.systemsx.cisd.openbis.generic.shared.authorization.predicate;
+
+import java.util.List;
+import java.util.Set;
+
+import ch.systemsx.cisd.common.exceptions.Status;
+import ch.systemsx.cisd.openbis.generic.shared.authorization.RoleWithIdentifier;
+import ch.systemsx.cisd.openbis.generic.shared.basic.TechId;
+import ch.systemsx.cisd.openbis.generic.shared.dto.DatabaseInstancePE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.FilterPE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.RoleAssignmentPE;
+
+/**
+ * An <code>IPredicate</code> implementation based on {@link TechId} of a filter.
+ * 
+ * @author Piotr Buczek
+ */
+
+public abstract class FilterTechIdPredicate extends AbstractDatabaseInstancePredicate<TechId>
+{
+
+    public static class UpdateFilterTechIdPredicate extends FilterTechIdPredicate
+    {
+        public UpdateFilterTechIdPredicate()
+        {
+            super(OperationKind.MODIFY);
+        }
+    }
+
+    public static class DeleteFilterTechIdPredicate extends FilterTechIdPredicate
+    {
+        public DeleteFilterTechIdPredicate()
+        {
+            super(OperationKind.DELETE);
+        }
+    }
+
+    private static enum OperationKind
+    {
+        MODIFY, DELETE;
+
+        public String getDescription()
+        {
+            return name().toLowerCase();
+        }
+    }
+
+    private final OperationKind operationKind;
+
+    public FilterTechIdPredicate(OperationKind operationKind)
+    {
+        this.operationKind = operationKind;
+    }
+
+    //
+    // AbstractPredicate
+    //
+
+    @Override
+    public final String getCandidateDescription()
+    {
+        return "filter technical id";
+    }
+
+    @Override
+    final Status doEvaluation(final PersonPE person, final List<RoleWithIdentifier> allowedRoles,
+            final TechId techId)
+    {
+
+        final FilterPE filter = authorizationDataProvider.getFilter(techId);
+        final boolean matching = isMatching(person, filter);
+        if (matching)
+        {
+            return Status.OK;
+        }
+        String userId = person.getUserId();
+        return Status.createError(createErrorMsg(filter, userId));
+    }
+
+    private static boolean isMatching(PersonPE person, FilterPE filter)
+    {
+        // needs to be an instance admin in filter database instance or registrator of a filter
+        return isRegistrator(person, filter)
+                || isInstanceAdmin(person, filter.getDatabaseInstance());
+    }
+
+    private String createErrorMsg(FilterPE filter, String userId)
+    {
+        return String
+                .format(
+                        STATUS_MESSAGE_PREFIX_FORMAT
+                                + operationKind.getDescription()
+                                + " filter '%s'. One needs to be either filter registrator or database instance admin.",
+                        userId, filter.getName());
+    }
+
+    private static boolean isRegistrator(final PersonPE person, final FilterPE filter)
+    {
+        return person.equals(filter.getRegistrator());
+    }
+
+    private static boolean isInstanceAdmin(final PersonPE person,
+            final DatabaseInstancePE databaseInstance)
+    {
+        final Set<RoleAssignmentPE> roleAssignments = person.getAllPersonRoles();
+        for (final RoleAssignmentPE roleAssignment : roleAssignments)
+        {
+            final DatabaseInstancePE roleInstance = roleAssignment.getDatabaseInstance();
+            if (databaseInstance.equals(roleInstance))
+            {
+                return true;
+            }
+        }
+        return false;
+    }
+}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/authorization/validator/FilterValidator.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/authorization/validator/FilterValidator.java
index e850c9c0970..97ad702c968 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/authorization/validator/FilterValidator.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/authorization/validator/FilterValidator.java
@@ -20,6 +20,7 @@ import java.util.Set;
 
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DatabaseInstance;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Filter;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Person;
 import ch.systemsx.cisd.openbis.generic.shared.dto.DatabaseInstancePE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.RoleAssignmentPE;
@@ -47,18 +48,21 @@ public final class FilterValidator extends AbstractValidator<Filter>
 
     private boolean isRegistrator(final PersonPE person, final Filter value)
     {
-        return person.equals(value.getRegistrator());
+        // Comparison between PersonPE and Person will always return false!!!
+        // return person.equals(value.getRegistrator());
+        // FIXME 2009-09-22, Piotr Buczek: Person has no database instance code to compare with PersonPE
+        Person registrator = value.getRegistrator();
+        return person.getUserId().equals(registrator.getUserId());
     }
 
-    private boolean isInstanceAdmin(final PersonPE person,
-            final DatabaseInstance filterDatabaseInstace)
+    public boolean isInstanceAdmin(final PersonPE person, final DatabaseInstance databaseInstance)
     {
         final Set<RoleAssignmentPE> roleAssignments = person.getAllPersonRoles();
         for (final RoleAssignmentPE roleAssignment : roleAssignments)
         {
-            final DatabaseInstancePE databaseInstance = roleAssignment.getDatabaseInstance();
-            if (databaseInstance != null
-                    && databaseInstance.getUuid().equals(filterDatabaseInstace.getUuid())
+            // TODO why do we use UUID instead of CODE if both are unique?
+            final DatabaseInstancePE roleInstance = roleAssignment.getDatabaseInstance();
+            if (roleInstance != null && roleInstance.getUuid().equals(databaseInstance.getUuid())
                     && roleAssignment.getRole().equals(RoleCode.ADMIN))
             {
                 return true;
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/ICommonServer.java.expected b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/ICommonServer.java.expected
index 307e80f0b38..6cadb974e6c 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/ICommonServer.java.expected
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/ICommonServer.java.expected
@@ -36,6 +36,7 @@ import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.AbstractT
 import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.AbstractTechIdPredicate.ExperimentTechIdPredicate;
 import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.AbstractTechIdPredicate.GroupTechIdPredicate;
 import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.AbstractTechIdPredicate.ProjectTechIdPredicate;
+import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.FilterTechIdPredicate.DeleteFilterTechIdPredicate;
 import ch.systemsx.cisd.openbis.generic.shared.authorization.validator.ExternalDataValidator;
 import ch.systemsx.cisd.openbis.generic.shared.authorization.validator.FilterValidator;
 import ch.systemsx.cisd.openbis.generic.shared.authorization.validator.GroupValidator;
@@ -925,8 +926,10 @@ public interface ICommonServer extends IServer
      * Deletes specified filters.
      */
     @Transactional
-    @RolesAllowed(RoleSet.INSTANCE_ADMIN)
-    @DatabaseCreateOrDeleteModification(value = ObjectKind.GROUP)
-    public void deleteFilters(String sessionToken, List<TechId> groupIds);
+    @RolesAllowed(RoleSet.POWER_USER)
+    @DatabaseCreateOrDeleteModification(value = ObjectKind.FILTER)
+    public void deleteFilters(
+            String sessionToken,
+            @AuthorizationGuard(guardClass = DeleteFilterTechIdPredicate.class) List<TechId> filterIds);
 
 }
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/authorization/AuthorizationTestCase.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/authorization/AuthorizationTestCase.java
index a20f3024d8b..d275c5ec6bb 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/authorization/AuthorizationTestCase.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/authorization/AuthorizationTestCase.java
@@ -33,6 +33,7 @@ import ch.systemsx.cisd.openbis.generic.shared.basic.TechId;
 import ch.systemsx.cisd.openbis.generic.shared.dto.DatabaseInstancePE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentTypePE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.FilterPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.GroupPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ProjectPE;
@@ -137,10 +138,16 @@ public class AuthorizationTestCase extends AssertJUnit
      * Creates a person. Only userId and databaseInstance are definied.
      */
     protected PersonPE createPerson()
+    {
+        return createPerson("megapixel", createDatabaseInstance());
+    }
+
+    /** Creates a person with specified userId and database instance. */
+    protected PersonPE createPerson(String userId, DatabaseInstancePE instance)
     {
         final PersonPE personPE = new PersonPE();
-        personPE.setUserId("megapixel");
-        personPE.setDatabaseInstance(createDatabaseInstance());
+        personPE.setUserId(userId);
+        personPE.setDatabaseInstance(instance);
         return personPE;
     }
 
@@ -203,6 +210,17 @@ public class AuthorizationTestCase extends AssertJUnit
     protected PersonPE createPersonWithRoleAssignments()
     {
         final PersonPE person = createPerson();
+        assignRoles(person);
+        return person;
+    }
+
+    /**
+     * Assigns two {@link RoleAssignmentPE} instances to specified person. One ADMIN role for
+     * database instance {@link #INSTANCE_CODE} and a USER role for the group
+     * {@link #createAnotherGroup()}.
+     */
+    protected void assignRoles(PersonPE person)
+    {
         final Set<RoleAssignmentPE> list = new HashSet<RoleAssignmentPE>();
         // Database assignment
         RoleAssignmentPE assignment = new RoleAssignmentPE();
@@ -216,8 +234,8 @@ public class AuthorizationTestCase extends AssertJUnit
         assignment.setGroup(createAnotherGroup());
         person.addRoleAssignment(assignment);
         list.add(assignment);
+
         person.setRoleAssignments(list);
-        return person;
     }
 
     /**
@@ -277,6 +295,20 @@ public class AuthorizationTestCase extends AssertJUnit
         return sample;
     }
 
+    /**
+     * Creates a filter in the specified database instance and registrator and ownership flag.
+     */
+    protected FilterPE createFilter(DatabaseInstancePE databaseInstance, PersonPE registrator,
+            boolean isPublic)
+    {
+        final FilterPE filter = new FilterPE();
+        filter.setDatabaseInstance(databaseInstance);
+        filter.setRegistrator(registrator);
+        filter.setPublic(isPublic);
+        filter.setExpression(""); // needed for translation
+        return filter;
+    }
+
     /**
      * Creates a list of roles which contains a group role for a USER and group defined by code
      * {@link #GROUP_CODE} and database instance {@link AuthorizationTestCase#INSTANCE_CODE}. If
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/authorization/validator/FilterValidatorTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/authorization/validator/FilterValidatorTest.java
new file mode 100644
index 00000000000..f90dbac2c1b
--- /dev/null
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/authorization/validator/FilterValidatorTest.java
@@ -0,0 +1,89 @@
+/*
+ * Copyright 2008 ETH Zuerich, CISD
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package ch.systemsx.cisd.openbis.generic.shared.authorization.validator;
+
+import org.testng.annotations.Test;
+
+import ch.rinn.restrictions.Friend;
+import ch.systemsx.cisd.openbis.generic.shared.authorization.AuthorizationTestCase;
+import ch.systemsx.cisd.openbis.generic.shared.dto.DatabaseInstancePE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.FilterPE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
+import ch.systemsx.cisd.openbis.generic.shared.translator.FilterTranslator;
+
+/**
+ * Test cases for corresponding {@link FilterValidator} class.
+ * 
+ * @author Piotr Buczek
+ */
+@Friend(toClasses = FilterValidator.class)
+public final class FilterValidatorTest extends AuthorizationTestCase
+{
+    @Test
+    public final void testWithPublicFilter()
+    {
+        final DatabaseInstancePE instance = createDatabaseInstance();
+        final PersonPE person = createPerson("A", instance);
+        final PersonPE registrator = createPerson("B", instance);
+        final boolean isPublic = true;
+        final FilterPE filter = createFilter(instance, registrator, isPublic);
+        final FilterValidator validator = new FilterValidator();
+        assertEquals(true, validator.isValid(person, FilterTranslator.translate(filter)));
+    }
+
+    @Test
+    public final void testWithRegistrator()
+    {
+        // TODO 2009-09-22, Piotr Buczek: test failure when validation is improved;
+        // (registrators are equal when they have the same userId AND db instance)
+        final DatabaseInstancePE instance = createDatabaseInstance();
+        final PersonPE person = createPerson("A", instance);
+        final PersonPE registrator = person;
+        final boolean isPublic = false;
+        final FilterPE filter = createFilter(instance, registrator, isPublic);
+        final FilterValidator validator = new FilterValidator();
+        assertEquals(true, validator.isValid(person, FilterTranslator.translate(filter)));
+    }
+
+    @Test
+    public final void testWithTheRightInstanceAdmin()
+    {
+        final DatabaseInstancePE instance = createDatabaseInstance();
+        final PersonPE person = createPerson("A", instance);
+        assignRoles(person);
+        final PersonPE registrator = createPerson("B", instance);
+        final boolean isPublic = false;
+        final FilterPE filter = createFilter(instance, registrator, isPublic);
+        final FilterValidator validator = new FilterValidator();
+        assertEquals(true, validator.isValid(person, FilterTranslator.translate(filter)));
+    }
+
+    @Test
+    public final void testWithTheWrongInstanceAdmin()
+    {
+        final DatabaseInstancePE instance = createDatabaseInstance();
+        final DatabaseInstancePE anotherInstance = createAnotherDatabaseInstance();
+        final PersonPE person = createPerson("A", instance);
+        assignRoles(person);
+        final PersonPE registrator = createPerson("B", anotherInstance);
+        final boolean isPublic = false;
+        final FilterPE filter = createFilter(anotherInstance, registrator, isPublic);
+        final FilterValidator validator = new FilterValidator();
+        assertEquals(false, validator.isValid(person, FilterTranslator.translate(filter)));
+    }
+
+}
-- 
GitLab