Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-52vp-f7hj-cj92
* fix: Add validation for programs org unit associations [DHIS2-13056]

* fix compilation failure in test class

* Fix one more compilation failure in another test class

Co-authored-by: Lars Helge Øverland <lars@dhis2.org>
Co-authored-by: Ameen <ameen@dhis2.org>
  • Loading branch information
3 people committed Jun 1, 2022
1 parent 57f2dac commit ef04483
Show file tree
Hide file tree
Showing 9 changed files with 144 additions and 18 deletions.
Expand Up @@ -109,7 +109,10 @@ <T extends IdentifiableObject> List<AttributeValue> getAllValuesByAttributes( Cl

<T extends IdentifiableObject> List<T> getByUid( Class<T> clazz, Collection<String> uids );

<T extends IdentifiableObject> List<T> getByUid( Collection<Class<? extends IdentifiableObject>> classes,
<T extends IdentifiableObject> List<T> getAndValidateByUid( Class<T> type, Collection<String> uids )
throws IllegalQueryException;

<T extends IdentifiableObject> List<T> getByUid( Collection<Class<? extends IdentifiableObject>> types,
Collection<String> uids );

<T extends IdentifiableObject> List<T> getById( Class<T> clazz, Collection<Long> ids );
Expand Down
Expand Up @@ -46,6 +46,7 @@
E1105( "Data set not found or not accessible: `{0}`" ),
E1106( "There are duplicate translation record for property `{0}` and locale `{1}`" ),
E1107( "Object type `{0}` is not translatable." ),
E1112( "Object(s) of type `{0}` not found or not accessible: `{1}`" ),

/* Org unit merge */
E1500( "At least two source orgs unit must be specified" ),
Expand Down
Expand Up @@ -34,6 +34,7 @@
import static org.hisp.dhis.hibernate.jsonb.type.JsonbFunctions.HAS_USER_GROUP_IDS;
import static org.hisp.dhis.hibernate.jsonb.type.JsonbFunctions.HAS_USER_ID;
import static org.hisp.dhis.security.acl.AclService.LIKE_READ_METADATA;
import static org.hisp.dhis.system.util.SqlUtils.singleQuote;

import java.util.Arrays;
import java.util.Objects;
Expand All @@ -43,6 +44,7 @@
import lombok.RequiredArgsConstructor;

import org.hisp.dhis.commons.collection.CollectionUtils;
import org.hisp.dhis.system.util.SqlUtils;
import org.hisp.dhis.user.CurrentUserGroupInfo;
import org.hisp.dhis.user.CurrentUserService;
import org.hisp.dhis.user.User;
Expand Down Expand Up @@ -147,14 +149,14 @@ private String getSharingConditions( String access )
private String getOwnerCondition( CurrentUserGroupInfo currentUserGroupInfo )
{
return String.join( " or ",
jsonbFunction( EXTRACT_PATH_TEXT, "owner" ) + " = " + withQuotes( currentUserGroupInfo.getUserUID() ),
jsonbFunction( EXTRACT_PATH_TEXT, "owner" ) + " = " + singleQuote( currentUserGroupInfo.getUserUID() ),
jsonbFunction( EXTRACT_PATH_TEXT, "owner" ) + " is null" );
}

private String getPublicSharingCondition( String access )
{
return String.join( " or ",
jsonbFunction( EXTRACT_PATH_TEXT, "public" ) + " like " + withQuotes( access ),
jsonbFunction( EXTRACT_PATH_TEXT, "public" ) + " like " + singleQuote( access ),
jsonbFunction( EXTRACT_PATH_TEXT, "public" ) + " is null" );
}

Expand Down Expand Up @@ -187,7 +189,7 @@ private String jsonbFunction( String functionName, String... params )
"(",
String.join( ",", "inner_query_alias.sharing",
Arrays.stream( params )
.map( this::withQuotes )
.map( SqlUtils::singleQuote )
.collect( joining( "," ) ) ),
")" );
}
Expand All @@ -201,16 +203,11 @@ private String getUidsFilter( Set<String> uids )
{
return T_ALIAS + ".uid in (" +
uids.stream()
.map( this::withQuotes )
.map( SqlUtils::singleQuote )
.collect( joining( "," ) )
+ ")";
}

private String withQuotes( String uid )
{
return String.join( "", "'", uid, "'" );
}

private String getUserOrgUnitPathsFilter( Set<String> userOrgUnitPaths )
{
return Stream.concat(
Expand Down
Expand Up @@ -56,6 +56,9 @@
import org.hisp.dhis.category.CategoryOption;
import org.hisp.dhis.category.CategoryOptionCombo;
import org.hisp.dhis.common.exception.InvalidIdentifierReferenceException;
import org.hisp.dhis.commons.collection.CollectionUtils;
import org.hisp.dhis.feedback.ErrorCode;
import org.hisp.dhis.feedback.ErrorMessage;
import org.hisp.dhis.hibernate.HibernateProxyUtils;
import org.hisp.dhis.schema.Schema;
import org.hisp.dhis.schema.SchemaService;
Expand Down Expand Up @@ -607,6 +610,25 @@ public <T extends IdentifiableObject> List<T> getByUid( Collection<Class<? exten
return list;
}

@Override
@Transactional( readOnly = true )
public <T extends IdentifiableObject> List<T> getAndValidateByUid( Class<T> type, Collection<String> uids )
throws IllegalQueryException
{
List<T> objects = getByUid( type, uids );

List<String> identifiers = IdentifiableObjectUtils.getUids( objects );
List<String> difference = CollectionUtils.difference( uids, identifiers );

if ( !difference.isEmpty() )
{
throw new IllegalQueryException( new ErrorMessage(
ErrorCode.E1112, type.getSimpleName(), difference ) );
}

return objects;
}

@Override
@Transactional( readOnly = true )
@SuppressWarnings( "unchecked" )
Expand Down
Expand Up @@ -70,7 +70,7 @@
import org.hisp.dhis.trackedentity.TrackedEntityInstanceStore;
import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValue;
import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueAudit;
import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueAuditService;
import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueAuditStore;
import org.hisp.dhis.user.CurrentUserService;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.jdbc.core.JdbcTemplate;
Expand All @@ -85,21 +85,21 @@

private final TrackedEntityInstanceStore trackedEntityInstanceStore;

private final TrackedEntityAttributeValueAuditService trackedEntityAttributeValueAuditService;
private final TrackedEntityAttributeValueAuditStore trackedEntityAttributeValueAuditStore;

private final DhisConfigurationProvider config;

public HibernatePotentialDuplicateStore( SessionFactory sessionFactory, JdbcTemplate jdbcTemplate,
ApplicationEventPublisher publisher, CurrentUserService currentUserService, AclService aclService,
TrackedEntityInstanceStore trackedEntityInstanceStore, AuditManager auditManager,
TrackedEntityAttributeValueAuditService trackedEntityAttributeValueAuditService,
TrackedEntityAttributeValueAuditStore trackedEntityAttributeValueAuditStore,
DhisConfigurationProvider config )
{
super( sessionFactory, jdbcTemplate, publisher, PotentialDuplicate.class, currentUserService,
aclService, false );
this.trackedEntityInstanceStore = trackedEntityInstanceStore;
this.auditManager = auditManager;
this.trackedEntityAttributeValueAuditService = trackedEntityAttributeValueAuditService;
this.trackedEntityAttributeValueAuditStore = trackedEntityAttributeValueAuditStore;
this.config = config;
}

Expand Down Expand Up @@ -248,8 +248,8 @@ private void auditTeav( TrackedEntityAttributeValue av, TrackedEntityAttributeVa

if ( config.isEnabled( CHANGELOG_TRACKER ) )
{
trackedEntityAttributeValueAuditService.addTrackedEntityAttributeValueAudit( deleteTeavAudit );
trackedEntityAttributeValueAuditService.addTrackedEntityAttributeValueAudit( updatedTeavAudit );
trackedEntityAttributeValueAuditStore.addTrackedEntityAttributeValueAudit( deleteTeavAudit );
trackedEntityAttributeValueAuditStore.addTrackedEntityAttributeValueAudit( updatedTeavAudit );
}
}

Expand Down
Expand Up @@ -35,6 +35,7 @@
import lombok.RequiredArgsConstructor;

import org.apache.commons.collections4.SetValuedMap;
import org.hisp.dhis.common.IdentifiableObjectManager;
import org.hisp.dhis.dataelement.DataElement;
import org.hisp.dhis.dataentryform.DataEntryForm;
import org.hisp.dhis.organisationunit.OrganisationUnit;
Expand Down Expand Up @@ -62,6 +63,8 @@

private final ProgramStore programStore;

private final IdentifiableObjectManager idObjectManager;

private final CurrentUserService currentUserService;

@Qualifier( "jdbcProgramOrgUnitAssociationsStore" )
Expand Down Expand Up @@ -195,9 +198,10 @@ public boolean hasOrgUnit( Program program, OrganisationUnit organisationUnit )
}

@Override
public SetValuedMap<String, String> getProgramOrganisationUnitsAssociationsForCurrentUser(
Set<String> programUids )
public SetValuedMap<String, String> getProgramOrganisationUnitsAssociationsForCurrentUser( Set<String> programUids )
{
idObjectManager.getAndValidateByUid( Program.class, programUids );

return jdbcOrgUnitAssociationsStore.getOrganisationUnitsAssociationsForCurrentUser( programUids );
}

Expand Down
Expand Up @@ -33,6 +33,7 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.jupiter.api.Assertions.assertThrows;

import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -46,6 +47,7 @@
import org.hisp.dhis.dataelement.DataElementGroup;
import org.hisp.dhis.dataelement.DataElementOperand;
import org.hisp.dhis.dataelement.DataElementService;
import org.hisp.dhis.feedback.ErrorCode;
import org.hisp.dhis.hibernate.exception.CreateAccessDeniedException;
import org.hisp.dhis.hibernate.exception.DeleteAccessDeniedException;
import org.hisp.dhis.hibernate.exception.UpdateAccessDeniedException;
Expand Down Expand Up @@ -473,6 +475,32 @@ public void getByUidTest()
assertTrue( cd.contains( dataElementD ) );
}

@Test
public void getAndValidateByUidTest()
{
DataElement dataElementA = createDataElement( 'A' );
DataElement dataElementB = createDataElement( 'B' );
DataElement dataElementC = createDataElement( 'C' );
identifiableObjectManager.save( dataElementA );
identifiableObjectManager.save( dataElementB );
identifiableObjectManager.save( dataElementC );
List<DataElement> ab = identifiableObjectManager.getAndValidateByUid( DataElement.class,
Arrays.asList( dataElementA.getUid(), dataElementB.getUid() ) );
assertTrue( ab.contains( dataElementA ) );
assertTrue( ab.contains( dataElementB ) );
assertFalse( ab.contains( dataElementC ) );
}

@Test
public void getAndValidateByUidExceptionTest()
{
DataElement dataElementA = createDataElement( 'A' );
identifiableObjectManager.save( dataElementA );
IllegalQueryException ex = assertThrows( IllegalQueryException.class, () -> identifiableObjectManager
.getAndValidateByUid( DataElement.class, Arrays.asList( dataElementA.getUid(), "xhjG82jHaky" ) ) );
assertEquals( ErrorCode.E1112, ex.getErrorCode() );
}

@Test
public void getOrderedUidIdSchemeTest()
{
Expand Down
Expand Up @@ -27,9 +27,12 @@
*/
package org.hisp.dhis.commons.collection;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.function.Consumer;
Expand Down Expand Up @@ -58,6 +61,22 @@ public static <T> Collection<T> intersection( Collection<T> c1, Collection<T> c2
return set1;
}

/**
* Returns all elements which are contained by {@code collection1} but not
* contained by {@code collection2} as an immutable list.
*
* @param <T>
* @param list1 the first collection.
* @param list2 the second collection.
* @return all elements in {@code collection1} not in {@code collection2}.
*/
public static <A> List<A> difference( Collection<A> collection1, Collection<A> collection2 )
{
List<A> list = new ArrayList<>( collection1 );
list.removeAll( collection2 );
return Collections.unmodifiableList( list );
}

/**
* Searches for and returns the first string which starts with the given
* prefix. Removes the match from the collection.
Expand Down
@@ -0,0 +1,52 @@
/*
* Copyright (c) 2004-2022, University of Oslo
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
* Redistributions of source code must retain the above copyright notice, this
* list of conditions and the following disclaimer.
*
* Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
* Neither the name of the HISP project nor the names of its contributors may
* be used to endorse or promote products derived from this software without
* specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
package org.hisp.dhis.commons.util;

import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.List;

import org.hisp.dhis.commons.collection.CollectionUtils;
import org.junit.jupiter.api.Test;

import com.google.common.collect.Lists;

class CollectionUtilsTest
{

@Test
public void testDifference()
{
List<String> collection1 = Lists.newArrayList( "One", "Two", "Three" );
List<String> collection2 = Lists.newArrayList( "One", "Two", "Four" );
List<String> difference = CollectionUtils.difference( collection1, collection2 );

assertEquals( 1, difference.size() );
assertEquals( "Three", difference.get( 0 ) );
}
}

0 comments on commit ef04483

Please sign in to comment.