[java-identity-provider] 01/02: IDP-1660 Sort multi-value attributes before hashing them in for consent

Rod Widdowson rdw at steadingsoftware.com
Thu Dec 10 11:44:15 UTC 2020


This is an automated email from the git hooks/post-receive script.

rdw pushed a commit to branch main
in repository java-identity-provider.

View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=c0a2bfc6ff32d9a87f4371d23fbeb2bbb5b9e9b9

commit c0a2bfc6ff32d9a87f4371d23fbeb2bbb5b9e9b9
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Thu Dec 10 11:06:02 2020 +0000

    IDP-1660 Sort multi-value attributes before hashing them in for consent
    
    https://issues.shibboleth.net/jira/browse/IDP-1660
    
    Added tests for this.  One of the test attributes get three values
    and you get to chose the order. Defaults to sorted for fuiture ease.
    
    Add a new test which explicitly changes the value order before
    adding them to the previous context.  Currently fails.
    
    Update all the inline hashes in the tests now that the template
    has changed
---
 .../idp/consent/impl/ConsentTestingSupport.java    | 36 ++++++++++++++++---
 .../impl/AttributeReleaseConsentFunctionTest.java  | 42 ++++++++++++++++++++++
 .../impl/AttributeValueLookupFunctionTest.java     |  2 +-
 .../impl/AttributeValuesHashFunctionTest.java      |  4 +--
 .../storage/impl/ConsentSerializerTest.java        |  6 ++--
 5 files changed, 80 insertions(+), 10 deletions(-)

diff --git a/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/impl/ConsentTestingSupport.java b/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/impl/ConsentTestingSupport.java
index 6aa70f233..156a67c28 100644
--- a/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/impl/ConsentTestingSupport.java
+++ b/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/impl/ConsentTestingSupport.java
@@ -33,6 +33,7 @@ import net.shibboleth.idp.consent.storage.impl.ConsentResult;
 /**
  * Helper methods for creating test objects for consent action tests.
  */
+ at SuppressWarnings("javadoc")
 public class ConsentTestingSupport {
 
     public static Map<String, Consent> newConsentMap() {
@@ -49,17 +50,44 @@ public class ConsentTestingSupport {
         map.put(consent2.getId(), consent2);
         return map;
     }
-
+    
+    public static enum MapType {
+        SORTED,
+        ORDER1,
+        ORDER2,
+    }
     public static final Map<String, IdPAttribute> newAttributeMap() {
-        final IdPAttributeValue value1 = new StringAttributeValue("value1");
+        return newAttributeMap(MapType.SORTED);
+    }
+
+    public static final Map<String, IdPAttribute> newAttributeMap(final MapType order) {
+        final IdPAttributeValue value1a = new StringAttributeValue("Avalue1");
+        final IdPAttributeValue value1b = new StringAttributeValue("Bvalue1");
+        final IdPAttributeValue value1c = new StringAttributeValue("Cvalue1");
         final IdPAttributeValue value2 = new StringAttributeValue("value2");
         final IdPAttributeValue value3 = new StringAttributeValue("value3");
 
         final IdPAttribute attribute1 = new IdPAttribute("attribute1");
-        attribute1.setValues(Collections.singletonList(value1));
+        List<IdPAttributeValue> values;
+        switch (order) {
+            case SORTED:
+            default:
+                values = List.of(value1a, value1b, value1c);
+                break;
+                
+            case ORDER1:
+
+                values = List.of(value1b, value1a, value1c);
+                break;
+                
+            case ORDER2:
+                values = List.of(value1c, value1a, value1b);
+                break;
+        }
+        attribute1.setValues(values);
 
         final IdPAttribute attribute2 = new IdPAttribute("attribute2");
-        attribute2.setValues(Arrays.asList(value1, value2));
+        attribute2.setValues(Arrays.asList(value1a, value2));
 
         final IdPAttribute attribute3 = new IdPAttribute("attribute3");
         attribute3.setValues(Collections.singletonList(value3));
diff --git a/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeReleaseConsentFunctionTest.java b/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeReleaseConsentFunctionTest.java
index d038efd66..912df7dde 100644
--- a/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeReleaseConsentFunctionTest.java
+++ b/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeReleaseConsentFunctionTest.java
@@ -17,6 +17,8 @@
 
 package net.shibboleth.idp.consent.logic.impl;
 
+import static org.testng.Assert.assertTrue;
+
 import java.util.HashMap;
 import java.util.Map;
 
@@ -26,6 +28,7 @@ import net.shibboleth.idp.consent.context.AttributeReleaseContext;
 import net.shibboleth.idp.consent.context.ConsentContext;
 import net.shibboleth.idp.consent.flow.impl.ConsentFlowDescriptor;
 import net.shibboleth.idp.consent.impl.ConsentTestingSupport;
+import net.shibboleth.idp.consent.impl.ConsentTestingSupport.MapType;
 import net.shibboleth.idp.profile.context.ProfileInterceptorContext;
 import net.shibboleth.idp.profile.context.navigate.WebflowRequestContextProfileRequestContextLookup;
 import net.shibboleth.idp.profile.testing.RequestContextBuilder;
@@ -37,6 +40,7 @@ import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 /** {@link AttributeReleaseConsentFunction} unit test. */
+ at SuppressWarnings("javadoc")
 public class AttributeReleaseConsentFunctionTest {
 
     private RequestContext src;
@@ -211,6 +215,44 @@ public class AttributeReleaseConsentFunctionTest {
 
         Assert.assertEquals(function.apply(prc), expected);
     }
+    
+    @Test public void testSorting () throws Exception {
+        // Setup unsorted previous
+        final Consent previousConsent = new Consent();
+        previousConsent.setId("attribute1");
+        previousConsent.setValue(attributeValuesHashFunction.apply(ConsentTestingSupport.newAttributeMap(MapType.ORDER1)
+                .get("attribute1").getValues()));
+        previousConsent.setApproved(true);
+        ConsentContext consentCtx = new ConsentContext();
+        consentCtx.getPreviousConsents().put(previousConsent.getId(), previousConsent);
+        prc.addSubcontext(consentCtx);
+        
+        // Test that we accept it
+        AttributeReleaseContext arc = new AttributeReleaseContext();
+        arc.getConsentableAttributes().putAll(ConsentTestingSupport.newAttributeMap(MapType.ORDER1));
+        prc.addSubcontext(arc);
+        setUpDescriptor(true);
+
+        final Map<String, Consent> firstResult = function.apply(prc);
+        final Consent firstConsent = firstResult.get("attribute1");
+        assertTrue(firstConsent.isApproved());
+
+        // Now test  in a different order
+        setUp();
+        consentCtx = new ConsentContext();
+        // first consent is now the previous consent
+        consentCtx.getPreviousConsents().put(firstConsent.getId(), firstConsent);
+        prc.addSubcontext(consentCtx);
+        arc = new AttributeReleaseContext();
+        arc.getConsentableAttributes().putAll(ConsentTestingSupport.newAttributeMap(MapType.ORDER2));
+        prc.addSubcontext(arc);
+        setUpDescriptor(true);
+
+        final Map<String, Consent> secondResult = function.apply(prc);
+        final Consent secondConsent = secondResult.get("attribute1");
+        assertTrue(secondConsent.isApproved());
+    }
+    
 
     @Test public void testRememberPreviousConsentsDifferentValueCompareValues() {
         final Consent previousConsent = new Consent();
diff --git a/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeValueLookupFunctionTest.java b/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeValueLookupFunctionTest.java
index f95638de7..7276baef6 100644
--- a/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeValueLookupFunctionTest.java
+++ b/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeValueLookupFunctionTest.java
@@ -79,7 +79,7 @@ public class AttributeValueLookupFunctionTest {
 
     @Test public void testAttributeValue() {
         function = new AttributeValueLookupFunction("attribute1");
-        Assert.assertEquals(function.apply(prc), "value1");
+        Assert.assertEquals(function.apply(prc), "Avalue1");
     }
 
     @Test public void testAttributeNotFound() {
diff --git a/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeValuesHashFunctionTest.java b/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeValuesHashFunctionTest.java
index b1f68a361..4b77e21e7 100644
--- a/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeValuesHashFunctionTest.java
+++ b/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeValuesHashFunctionTest.java
@@ -67,12 +67,12 @@ public class AttributeValuesHashFunctionTest extends XMLObjectBaseTestCase {
     @Test public void testSingleValue() {
         // NOTE Any change is an ODS drift
         final String hash = function.apply(ConsentTestingSupport.newAttributeMap().get("attribute1").getValues());
-        assertEquals(hash, "yePBj0hcjLihhDtDb//R/ymyw2CHZAUreX/4RupmSXM=");
+        assertEquals(hash, "qY1Ely22YLjD7hy4/HFSlErfjWNtVNJTZDral2Bs3Q8=");
     }
 
     @Test public void testMultipleValues() {
         final String hash = function.apply(ConsentTestingSupport.newAttributeMap().get("attribute2").getValues());
-        assertEquals(hash, "xxuA06hGJ1DcJ4JSaWiBXXGfcRr6oxHM5jaURXBBnbA=");
+        assertEquals(hash, "w4A7kgpy8PAiMfNkM8yR68zLF9ngILQDWDy+n2l59zk=");
     }
 
     @Test public void testScoped() {
diff --git a/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/storage/impl/ConsentSerializerTest.java b/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/storage/impl/ConsentSerializerTest.java
index ea554a427..471ddabab 100644
--- a/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/storage/impl/ConsentSerializerTest.java
+++ b/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/storage/impl/ConsentSerializerTest.java
@@ -117,7 +117,7 @@ public class ConsentSerializerTest {
         final String serialized = serializer.serialize(consents);
         Assert.assertEquals(
                 serialized,
-                "[{\"id\":\"attribute1\",\"v\":\"yePBj0hcjLihhDtDb//R/ymyw2CHZAUreX/4RupmSXM=\"},{\"id\":\"attribute2\",\"v\":\"xxuA06hGJ1DcJ4JSaWiBXXGfcRr6oxHM5jaURXBBnbA=\",\"appr\":false}]");
+                "[{\"id\":\"attribute1\",\"v\":\"qY1Ely22YLjD7hy4/HFSlErfjWNtVNJTZDral2Bs3Q8=\"},{\"id\":\"attribute2\",\"v\":\"w4A7kgpy8PAiMfNkM8yR68zLF9ngILQDWDy+n2l59zk=\",\"appr\":false}]");
 
         final Map<String, Consent> deserialized = serializer.deserialize(1, CONTEXT, KEY, serialized, null);
 
@@ -132,7 +132,7 @@ public class ConsentSerializerTest {
         final String serialized = serializer.serialize(consents);
         Assert.assertEquals(
                 serialized,
-                "[{\"id\":201,\"v\":\"yePBj0hcjLihhDtDb//R/ymyw2CHZAUreX/4RupmSXM=\"},{\"id\":202,\"v\":\"xxuA06hGJ1DcJ4JSaWiBXXGfcRr6oxHM5jaURXBBnbA=\",\"appr\":false}]");
+                "[{\"id\":201,\"v\":\"qY1Ely22YLjD7hy4/HFSlErfjWNtVNJTZDral2Bs3Q8=\"},{\"id\":202,\"v\":\"w4A7kgpy8PAiMfNkM8yR68zLF9ngILQDWDy+n2l59zk=\",\"appr\":false}]");
 
         final Map<String, Consent> deserialized = serializer.deserialize(1, CONTEXT, KEY, serialized, null);
 
@@ -150,7 +150,7 @@ public class ConsentSerializerTest {
         final String serialized = serializer.serialize(consents);
         Assert.assertEquals(
                 serialized,
-                "[{\"id\":\"attribute1\",\"v\":\"yePBj0hcjLihhDtDb//R/ymyw2CHZAUreX/4RupmSXM=\"},{\"id\":\"attribute2\",\"v\":\"xxuA06hGJ1DcJ4JSaWiBXXGfcRr6oxHM5jaURXBBnbA=\",\"appr\":false}]");
+                "[{\"id\":\"attribute1\",\"v\":\"qY1Ely22YLjD7hy4/HFSlErfjWNtVNJTZDral2Bs3Q8=\"},{\"id\":\"attribute2\",\"v\":\"w4A7kgpy8PAiMfNkM8yR68zLF9ngILQDWDy+n2l59zk=\",\"appr\":false}]");
 
         final Map<String, Consent> deserialized = serializer.deserialize(1, CONTEXT, KEY, serialized, null);
 

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list