[java-oidc-common] branch main updated: JCOMOIDC-99 - Metadata policy merging misses subordinate values with some operators
Henri Mikkonen
henri.mikkonen at iki.fi
Fri Feb 16 09:28:01 UTC 2024
This is an automated email from the git hooks/post-receive script.
hjmikkon pushed a commit to branch main
in repository java-oidc-common.
View the commit online:
http://git.shibboleth.net/view/?p=java-oidc-common.git;a=commit;h=73524fba1f04662240d0b3c7a6a6afba00aa05c6
The following commit(s) were added to refs/heads/main by this push:
new 73524fb JCOMOIDC-99 - Metadata policy merging misses subordinate values with some operators
73524fb is described below
commit 73524fba1f04662240d0b3c7a6a6afba00aa05c6
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Fri Feb 16 11:26:46 2024 +0200
JCOMOIDC-99 - Metadata policy merging misses subordinate values with some operators
https://shibboleth.atlassian.net/browse/JCOMOIDC-99
Fixed the merging function for default, value and regexp -operators. Improved testing.
---
.../metadata/policy/impl/MetadataPolicyHelper.java | 18 +--
.../policy/impl/MetadataPolicyHelperTest.java | 168 +++++++++++++++++++++
2 files changed, 177 insertions(+), 9 deletions(-)
diff --git a/oidc-common-metadata-impl/src/main/java/net/shibboleth/oidc/metadata/policy/impl/MetadataPolicyHelper.java b/oidc-common-metadata-impl/src/main/java/net/shibboleth/oidc/metadata/policy/impl/MetadataPolicyHelper.java
index 56e9fff..096ceda 100644
--- a/oidc-common-metadata-impl/src/main/java/net/shibboleth/oidc/metadata/policy/impl/MetadataPolicyHelper.java
+++ b/oidc-common-metadata-impl/src/main/java/net/shibboleth/oidc/metadata/policy/impl/MetadataPolicyHelper.java
@@ -127,14 +127,14 @@ public final class MetadataPolicyHelper {
}
}
- final String regexp = superior.getRegexp();
- if (regexp != null) {
- final String anotherRegexp = subordinate.getRegexp();
- if (anotherRegexp != null && !regexp.equals(anotherRegexp)) {
- throw new ConstraintViolationException(
- "Mering two regexp operators is NOT allowed unless the two operator values are equal.");
- }
+ final String regexp;
+ try {
+ regexp = (String) doMergeForTwoObjects(superior.getRegexp(), subordinate.getRegexp(), false);
+ } catch (final ConstraintViolationException e) {
+ throw new ConstraintViolationException(
+ "Merging two regexp operators is NOT allowed unless the two operator values are equal.");
}
+
return new MetadataPolicy.Builder()
.withSubsetOfValues(doMergeForTwoLists(superior.getSubsetOfValues(),
subordinate.getSubsetOfValues(), false))
@@ -142,8 +142,8 @@ public final class MetadataPolicyHelper {
.withSupersetOfValues(doMergeForTwoLists(superior.getSupersetOfValues(),
subordinate.getSupersetOfValues(), true))
.withAdd(doMergeForTwoObjects(superior.getAdd(), subordinate.getAdd(), true))
- .withValue(value)
- .withDefaultValue(defaultValue)
+ .withValue(value != null ? value : subordinate.getValue())
+ .withDefaultValue(defaultValue != null ? defaultValue : subordinate.getDefaultValue())
.withEssential(superior.isEssential() ? Boolean.TRUE : subordinate.getEssential())
.withRegexp(regexp)
.build();
diff --git a/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/policy/impl/MetadataPolicyHelperTest.java b/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/policy/impl/MetadataPolicyHelperTest.java
index ed35e2d..b0633b9 100644
--- a/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/policy/impl/MetadataPolicyHelperTest.java
+++ b/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/policy/impl/MetadataPolicyHelperTest.java
@@ -303,6 +303,67 @@ public class MetadataPolicyHelperTest {
Assert.assertEquals(merged.getValue(), "singleValue1");
}
+ @Test
+ public void mergeMetadataPolicies_superiorOnlyValue_shouldReturnSuperior() {
+ final MetadataPolicy superior = new MetadataPolicy.Builder()
+ .withValue("singleValue1")
+ .build();
+ final MetadataPolicy subordinate = new MetadataPolicy.Builder()
+ .build();
+ final MetadataPolicy merged = MetadataPolicyHelper.mergeMetadataPolicies(superior, subordinate);
+ assert merged != null;
+ Assert.assertEquals(merged.getValue(), "singleValue1");
+ }
+
+ @Test
+ public void mergeMetadataPolicies_subordinateOnlyValue_shouldReturnSubordinate() {
+ final MetadataPolicy superior = new MetadataPolicy.Builder()
+ .build();
+ final MetadataPolicy subordinate = new MetadataPolicy.Builder()
+ .withValue("singleValue1")
+ .build();
+ final MetadataPolicy merged = MetadataPolicyHelper.mergeMetadataPolicies(superior, subordinate);
+ assert merged != null;
+ Assert.assertEquals(merged.getValue(), "singleValue1");
+ }
+
+ @Test
+ public void mergeMetadataPolicies_bothSameListValue_shouldReturnSame() {
+ final MetadataPolicy superior = new MetadataPolicy.Builder()
+ .withValue(List.of("singleValue1"))
+ .build();
+ final MetadataPolicy subordinate = new MetadataPolicy.Builder()
+ .withValue(List.of("singleValue1"))
+ .build();
+ final MetadataPolicy merged = MetadataPolicyHelper.mergeMetadataPolicies(superior, subordinate);
+ assert merged != null;
+ Assert.assertEquals(merged.getValue(), List.of("singleValue1"));
+ }
+
+ @Test
+ public void mergeMetadataPolicies_superiorOnlyListValue_shouldReturnSuperior() {
+ final MetadataPolicy superior = new MetadataPolicy.Builder()
+ .withValue(List.of("singleValue1"))
+ .build();
+ final MetadataPolicy subordinate = new MetadataPolicy.Builder()
+ .build();
+ final MetadataPolicy merged = MetadataPolicyHelper.mergeMetadataPolicies(superior, subordinate);
+ assert merged != null;
+ Assert.assertEquals(merged.getValue(), List.of("singleValue1"));
+ }
+
+ @Test
+ public void mergeMetadataPolicies_subordinateOnlyListValue_shouldReturnSubordinate() {
+ final MetadataPolicy superior = new MetadataPolicy.Builder()
+ .build();
+ final MetadataPolicy subordinate = new MetadataPolicy.Builder()
+ .withValue(List.of("singleValue1"))
+ .build();
+ final MetadataPolicy merged = MetadataPolicyHelper.mergeMetadataPolicies(superior, subordinate);
+ assert merged != null;
+ Assert.assertEquals(merged.getValue(), List.of("singleValue1"));
+ }
+
@Test(expectedExceptions = { ConstraintViolationException.class })
public void mergeMetadataPolicies_bothDifferentValue_shouldThrow() {
final MetadataPolicy superior = new MetadataPolicy.Builder()
@@ -325,6 +386,17 @@ public class MetadataPolicyHelperTest {
MetadataPolicyHelper.mergeMetadataPolicies(superior, subordinate);
}
+ @Test(expectedExceptions = { ConstraintViolationException.class })
+ public void mergeMetadataPolicies_bothDifferentValueTypes_shouldThrow() {
+ final MetadataPolicy superior = new MetadataPolicy.Builder()
+ .withValue(List.of("singleValue1"))
+ .build();
+ final MetadataPolicy subordinate = new MetadataPolicy.Builder()
+ .withValue("singleValue1")
+ .build();
+ MetadataPolicyHelper.mergeMetadataPolicies(superior, subordinate);
+ }
+
@Test
public void mergeMetadataPolicies_bothSameDefaultValue_shouldReturnSame() {
final MetadataPolicy superior = new MetadataPolicy.Builder()
@@ -338,6 +410,67 @@ public class MetadataPolicyHelperTest {
Assert.assertEquals(merged.getDefaultValue(), "singleValue1");
}
+ @Test
+ public void mergeMetadataPolicies_superiorOnlyDefaultValue_shouldReturnSuperior() {
+ final MetadataPolicy superior = new MetadataPolicy.Builder()
+ .withDefaultValue("singleValue1")
+ .build();
+ final MetadataPolicy subordinate = new MetadataPolicy.Builder()
+ .build();
+ final MetadataPolicy merged = MetadataPolicyHelper.mergeMetadataPolicies(superior, subordinate);
+ assert merged != null;
+ Assert.assertEquals(merged.getDefaultValue(), "singleValue1");
+ }
+
+ @Test
+ public void mergeMetadataPolicies_subordinateOnlyDefaultValue_shouldReturnSubordinate() {
+ final MetadataPolicy superior = new MetadataPolicy.Builder()
+ .build();
+ final MetadataPolicy subordinate = new MetadataPolicy.Builder()
+ .withDefaultValue("singleValue1")
+ .build();
+ final MetadataPolicy merged = MetadataPolicyHelper.mergeMetadataPolicies(superior, subordinate);
+ assert merged != null;
+ Assert.assertEquals(merged.getDefaultValue(), "singleValue1");
+ }
+
+ @Test
+ public void mergeMetadataPolicies_bothSameDefaultListValue_shouldReturnSame() {
+ final MetadataPolicy superior = new MetadataPolicy.Builder()
+ .withDefaultValue(List.of("singleValue1"))
+ .build();
+ final MetadataPolicy subordinate = new MetadataPolicy.Builder()
+ .withDefaultValue(List.of("singleValue1"))
+ .build();
+ final MetadataPolicy merged = MetadataPolicyHelper.mergeMetadataPolicies(superior, subordinate);
+ assert merged != null;
+ Assert.assertEquals(merged.getDefaultValue(), List.of("singleValue1"));
+ }
+
+ @Test
+ public void mergeMetadataPolicies_superiorOnlyDefaultListValue_shouldReturnSuperior() {
+ final MetadataPolicy superior = new MetadataPolicy.Builder()
+ .withDefaultValue(List.of("singleValue1"))
+ .build();
+ final MetadataPolicy subordinate = new MetadataPolicy.Builder()
+ .build();
+ final MetadataPolicy merged = MetadataPolicyHelper.mergeMetadataPolicies(superior, subordinate);
+ assert merged != null;
+ Assert.assertEquals(merged.getDefaultValue(), List.of("singleValue1"));
+ }
+
+ @Test
+ public void mergeMetadataPolicies_subordinateOnlyDefaultListValue_shouldReturnSubordinate() {
+ final MetadataPolicy superior = new MetadataPolicy.Builder()
+ .build();
+ final MetadataPolicy subordinate = new MetadataPolicy.Builder()
+ .withDefaultValue(List.of("singleValue1"))
+ .build();
+ final MetadataPolicy merged = MetadataPolicyHelper.mergeMetadataPolicies(superior, subordinate);
+ assert merged != null;
+ Assert.assertEquals(merged.getDefaultValue(), List.of("singleValue1"));
+ }
+
@Test(expectedExceptions = { ConstraintViolationException.class })
public void mergeMetadataPolicies_bothDifferentDefaultValue_shouldThrow() {
final MetadataPolicy superior = new MetadataPolicy.Builder()
@@ -362,6 +495,30 @@ public class MetadataPolicyHelperTest {
Assert.assertEquals(merged.getRegexp(), "singleValue1");
}
+ @Test
+ public void mergeMetadataPolicies_nullSuperiorRegexp_shouldReturnSubordinate() {
+ final MetadataPolicy superior = new MetadataPolicy.Builder()
+ .build();
+ final MetadataPolicy subordinate = new MetadataPolicy.Builder()
+ .withRegexp("singleValue1")
+ .build();
+ final MetadataPolicy merged = MetadataPolicyHelper.mergeMetadataPolicies(superior, subordinate);
+ assert merged != null;
+ Assert.assertEquals(merged.getRegexp(), "singleValue1");
+ }
+
+ @Test
+ public void mergeMetadataPolicies_nullSubordinateRegexp_shouldReturnSuperior() {
+ final MetadataPolicy superior = new MetadataPolicy.Builder()
+ .withRegexp("singleValue1")
+ .build();
+ final MetadataPolicy subordinate = new MetadataPolicy.Builder()
+ .build();
+ final MetadataPolicy merged = MetadataPolicyHelper.mergeMetadataPolicies(superior, subordinate);
+ assert merged != null;
+ Assert.assertEquals(merged.getRegexp(), "singleValue1");
+ }
+
@Test(expectedExceptions = { ConstraintViolationException.class })
public void mergeMetadataPolicies_bothDifferentRegexp_shouldThrow() {
final MetadataPolicy superior = new MetadataPolicy.Builder()
@@ -384,6 +541,17 @@ public class MetadataPolicyHelperTest {
MetadataPolicyHelper.mergeMetadataPolicies(superior, subordinate);
}
+ @Test(expectedExceptions = { ConstraintViolationException.class })
+ public void mergeMetadataPolicies_bothDifferentDefaulValueTypes_shouldThrow() {
+ final MetadataPolicy superior = new MetadataPolicy.Builder()
+ .withDefaultValue(List.of("singleValue1"))
+ .build();
+ final MetadataPolicy subordinate = new MetadataPolicy.Builder()
+ .withDefaultValue("singleValue1")
+ .build();
+ MetadataPolicyHelper.mergeMetadataPolicies(superior, subordinate);
+ }
+
@Test
public void mergeMetadataPolicies_bothEssentialTrue_shouldReturnTrue() {
final MetadataPolicy superior = new MetadataPolicy.Builder()
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list