=== modified file 'dhis-2/dhis-api/src/main/java/org/hisp/dhis/acl/AccessStringHelper.java' --- dhis-2/dhis-api/src/main/java/org/hisp/dhis/acl/AccessStringHelper.java 2015-01-17 07:41:26 +0000 +++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/acl/AccessStringHelper.java 2015-01-19 05:09:11 +0000 @@ -135,6 +135,16 @@ return isEnabled( access, Permission.WRITE ); } + public static boolean canReadAndWrite( String access ) + { + return isEnabled( access, Permission.WRITE ) && isEnabled( access, Permission.READ ); + } + + public static boolean canReadOrWrite( String access ) + { + return isEnabled( access, Permission.WRITE ) || isEnabled( access, Permission.READ ); + } + public static boolean isEnabled( String access, Permission permission ) { return access != null && access.charAt( permission.getPosition() ) == permission.getValue(); === modified file 'dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/GenericStore.java' --- dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/GenericStore.java 2015-01-17 07:41:26 +0000 +++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/GenericStore.java 2015-01-19 05:09:11 +0000 @@ -41,12 +41,21 @@ Class getClazz(); /** + * Saves the given object instance, with clear sharing set to true. + * + * @param object the object instance. + * @return the generated identifier. + */ + int save( T object ); + + /** * Saves the given object instance. * - * @param object the object instance. + * @param object the object instance. + * @param clearSharing Should we clear all sharing related properties? * @return the generated identifier. */ - int save( T object ); + int save( T object, boolean clearSharing ); /** * Updates the given object instance. === modified file 'dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/IdentifiableObjectManager.java' --- dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/IdentifiableObjectManager.java 2015-01-17 07:41:26 +0000 +++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/IdentifiableObjectManager.java 2015-01-19 05:09:11 +0000 @@ -45,6 +45,8 @@ void save( IdentifiableObject object ); + void save( IdentifiableObject object, boolean clearSharing ); + void update( IdentifiableObject object ); void update( List object ); === modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/common/DefaultIdentifiableObjectManager.java' --- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/common/DefaultIdentifiableObjectManager.java 2015-01-17 07:41:26 +0000 +++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/common/DefaultIdentifiableObjectManager.java 2015-01-19 05:09:11 +0000 @@ -76,11 +76,17 @@ @Override public void save( IdentifiableObject object ) { + save( object, true ); + } + + @Override + public void save( IdentifiableObject object, boolean clearSharing ) + { GenericIdentifiableObjectStore store = getIdentifiableObjectStore( object.getClass() ); if ( store != null ) { - store.save( object ); + store.save( object, clearSharing ); } } === modified file 'dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/common/IdentifiableObjectManagerTest.java' --- dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/common/IdentifiableObjectManagerTest.java 2015-01-17 07:41:26 +0000 +++ dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/common/IdentifiableObjectManagerTest.java 2015-01-19 05:09:11 +0000 @@ -28,16 +28,7 @@ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; - -import java.util.ArrayList; -import java.util.GregorianCalendar; -import java.util.List; - +import com.google.common.collect.Sets; import org.hibernate.SessionFactory; import org.hisp.dhis.DhisSpringTest; import org.hisp.dhis.acl.AccessStringHelper; @@ -54,7 +45,11 @@ import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; -import com.google.common.collect.Sets; +import java.util.ArrayList; +import java.util.GregorianCalendar; +import java.util.List; + +import static org.junit.Assert.*; /** * @author Morten Olav Hansen @@ -67,13 +62,13 @@ @Autowired private DataElementService dataElementService; - - @Autowired + + @Autowired private IdentifiableObjectManager _identifiableObjectManager; - + @Autowired private UserService _userService; - + @Override protected void setUpTest() throws Exception { @@ -320,6 +315,64 @@ identifiableObjectManager.save( createDataElement( 'A' ) ); } + @Test + public void publicUserModifiedPublicAccessDEFAULT() + { + createUserAndInjectSecurityContext( false, "F_DATAELEMENT_PUBLIC_ADD" ); + + DataElement dataElement = createDataElement( 'A' ); + dataElement.setPublicAccess( AccessStringHelper.DEFAULT ); + + identifiableObjectManager.save( dataElement, false ); + + assertFalse( AccessStringHelper.canRead( dataElement.getPublicAccess() ) ); + assertFalse( AccessStringHelper.canWrite( dataElement.getPublicAccess() ) ); + } + + @Test + public void publicUserModifiedPublicAccessRW() + { + createUserAndInjectSecurityContext( false, "F_DATAELEMENT_PUBLIC_ADD" ); + + DataElement dataElement = createDataElement( 'A' ); + dataElement.setPublicAccess( AccessStringHelper.READ_WRITE ); + + identifiableObjectManager.save( dataElement, false ); + } + + @Test( expected = CreateAccessDeniedException.class ) + public void privateUserModifiedPublicAccessRW() + { + createUserAndInjectSecurityContext( false, "F_DATAELEMENT_PRIVATE_ADD" ); + + DataElement dataElement = createDataElement( 'A' ); + dataElement.setPublicAccess( AccessStringHelper.READ_WRITE ); + + identifiableObjectManager.save( dataElement, false ); + } + + @Test + public void privateUserModifiedPublicAccessDEFAULT() + { + createUserAndInjectSecurityContext( false, "F_DATAELEMENT_PRIVATE_ADD" ); + + DataElement dataElement = createDataElement( 'A' ); + dataElement.setPublicAccess( AccessStringHelper.DEFAULT ); + + identifiableObjectManager.save( dataElement, false ); + } + + @Test( expected = CreateAccessDeniedException.class ) + public void userDeniedForPublicAdd() + { + createUserAndInjectSecurityContext( false ); + + DataElement dataElement = createDataElement( 'A' ); + dataElement.setPublicAccess( AccessStringHelper.READ_WRITE ); + + identifiableObjectManager.save( dataElement, false ); + } + @Test( expected = DeleteAccessDeniedException.class ) public void userDeniedDeleteObject() { === modified file 'dhis-2/dhis-support/dhis-support-hibernate/src/main/java/org/hisp/dhis/hibernate/HibernateGenericStore.java' --- dhis-2/dhis-support/dhis-support-hibernate/src/main/java/org/hisp/dhis/hibernate/HibernateGenericStore.java 2015-01-17 07:41:26 +0000 +++ dhis-2/dhis-support/dhis-support-hibernate/src/main/java/org/hisp/dhis/hibernate/HibernateGenericStore.java 2015-01-19 05:09:11 +0000 @@ -297,13 +297,23 @@ @Override public int save( T object ) { + return save( object, true ); + } + + @Override + public int save( T object, boolean clearSharing ) + { User currentUser = currentUserService.getCurrentUser(); if ( IdentifiableObject.class.isAssignableFrom( object.getClass() ) ) { BaseIdentifiableObject identifiableObject = (BaseIdentifiableObject) object; - identifiableObject.setPublicAccess( AccessStringHelper.DEFAULT ); - identifiableObject.getUserGroupAccesses().clear(); + + if ( clearSharing ) + { + identifiableObject.setPublicAccess( AccessStringHelper.DEFAULT ); + identifiableObject.getUserGroupAccesses().clear(); + } if ( identifiableObject.getUser() == null ) { @@ -315,18 +325,22 @@ { BaseIdentifiableObject identifiableObject = (BaseIdentifiableObject) object; - if ( aclService.canCreatePublic( currentUser, identifiableObject.getClass() ) ) - { - if ( aclService.defaultPublic( identifiableObject.getClass() ) ) - { - identifiableObject.setPublicAccess( AccessStringHelper.READ_WRITE ); - } - } - else if ( aclService.canCreatePrivate( currentUser, identifiableObject.getClass() ) ) - { - identifiableObject.setPublicAccess( AccessStringHelper.newInstance().build() ); - } - else + if ( clearSharing ) + { + if ( aclService.canCreatePublic( currentUser, identifiableObject.getClass() ) ) + { + if ( aclService.defaultPublic( identifiableObject.getClass() ) ) + { + identifiableObject.setPublicAccess( AccessStringHelper.READ_WRITE ); + } + } + else if ( aclService.canCreatePrivate( currentUser, identifiableObject.getClass() ) ) + { + identifiableObject.setPublicAccess( AccessStringHelper.newInstance().build() ); + } + } + + if ( !checkPublicAccess( currentUser, identifiableObject ) ) { AuditLogUtil.infoWrapper( log, currentUserService.getCurrentUsername(), object, AuditLogUtil.ACTION_CREATE_DENIED ); throw new CreateAccessDeniedException( object.toString() ); @@ -337,6 +351,14 @@ return (Integer) sessionFactory.getCurrentSession().save( object ); } + private boolean checkPublicAccess( User user, IdentifiableObject identifiableObject ) + { + return aclService.canCreatePublic( user, identifiableObject.getClass() ) || + (aclService.canCreatePrivate( user, identifiableObject.getClass() ) && + !AccessStringHelper.canReadOrWrite( identifiableObject.getPublicAccess() )); + + } + @Override public void update( T object ) {