diff --git a/openidm-core/src/main/java/org/forgerock/openidm/managed/CollectionRelationshipProvider.java b/openidm-core/src/main/java/org/forgerock/openidm/managed/CollectionRelationshipProvider.java index 87cb82ebff..2d6c31b68e 100644 --- a/openidm-core/src/main/java/org/forgerock/openidm/managed/CollectionRelationshipProvider.java +++ b/openidm-core/src/main/java/org/forgerock/openidm/managed/CollectionRelationshipProvider.java @@ -25,8 +25,10 @@ import static org.forgerock.util.query.QueryFilter.*; import java.util.ArrayList; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import org.forgerock.http.routing.RoutingMode; @@ -599,16 +601,30 @@ public void validateRelationshipField(Context context, JsonValue oldValue, JsonV relationships are re-checked, then the invocation will be rejected because the relationship currently exists. */ Set oldReferences = new HashSet<>(); + Map oldRefPropertiesById = new HashMap<>(); if (oldValue.isNotNull()) { for (JsonValue oldItem : oldValue) { oldReferences.add(new RelationshipEqualityHash(oldItem)); + final JsonValue oldId = oldItem.get(FIELD_ID); + if (oldId != null && oldId.isString()) { + oldRefPropertiesById.put(oldId.asString(), oldItem.get(FIELD_PROPERTIES)); + } } } for (JsonValue newItem : newValue) { + final JsonValue id = newItem.get(FIELD_ID); + final boolean hasId = id != null && id.isNotNull(); // If the relationship is found in the existing/old relationships, then must skip validation. if (!oldReferences.contains(new RelationshipEqualityHash(newItem))) { logger.debug("validating new relationship {} for {}: ", newItem, propertyPtr); - relationshipValidator.validateRelationship(newItem, referrerId, context, performDuplicateAssignmentCheck); + // An updated relationship may keep the temporal constraints it was stored with, even invalid ones + relationshipValidator.validateRelationship(newItem, + hasId && id.isString() ? oldRefPropertiesById.get(id.asString()) : null, + referrerId, context, performDuplicateAssignmentCheck); + } else if (!hasId) { + // Equal to a stored relationship but without its _id: persisting it deletes the stored relationship + // and creates this one, which must not fail on its temporal constraints after the commit + RelationshipValidator.validateTemporalConstraints(newItem.get(FIELD_PROPERTIES)); } } } diff --git a/openidm-core/src/main/java/org/forgerock/openidm/managed/RelationshipProvider.java b/openidm-core/src/main/java/org/forgerock/openidm/managed/RelationshipProvider.java index bace4d70d7..0539dd4600 100644 --- a/openidm-core/src/main/java/org/forgerock/openidm/managed/RelationshipProvider.java +++ b/openidm-core/src/main/java/org/forgerock/openidm/managed/RelationshipProvider.java @@ -389,6 +389,9 @@ public abstract void validateRelationshipField(Context context, JsonValue oldVal public Promise createInstance(final Context context, final CreateRequest request) { try { + // The relationship validator checks the temporal constraints too, but on the managed object path only + // when the schema field requires validation + RelationshipValidator.validateTemporalConstraints(request.getContent().get(FIELD_PROPERTIES)); final CreateRequest createRequest = Requests.copyOfCreateRequest(request); createRequest.setResourcePath(REPO_RESOURCE_PATH); createRequest.setContent(convertToRepoObject(firstResourcePath(context, request), request.getContent())); @@ -645,6 +648,8 @@ private Promise updateIfChanged(final Conte .then(formatResponse(context, request)); } else { // resource has changed, update the relationship + RelationshipValidator.validateChangedTemporalConstraints( + oldResource.getContent().get(REPO_FIELD_PROPERTIES), newValue.get(REPO_FIELD_PROPERTIES)); UpdateRequest updateRequest = Requests.newUpdateRequest(REPO_RESOURCE_PATH.child(id), newValue).setRevision(rev); return syncReferencedObjectUpdateHandler @@ -846,11 +851,6 @@ protected JsonValue convertToRepoObject(final ResourcePath firstResourcePath, fi // Remove "soft" fields that were placed in properties for the ResourceResponse properties.remove(FIELD_CONTENT_ID); properties.remove(FIELD_CONTENT_REVISION); - // Currently only 1 temporal constraint is allowed per grant - if (properties.get("temporalConstraints").isNotNull() - && properties.get("temporalConstraints").expect(List.class).asList().size() > 1) { - throw new BadRequestException("Only 1 temporal constraint is supported per grant."); - } } if (schemaField.isReverseRelationship()) { diff --git a/openidm-core/src/main/java/org/forgerock/openidm/managed/RelationshipValidator.java b/openidm-core/src/main/java/org/forgerock/openidm/managed/RelationshipValidator.java index 15bc5d43f6..e06faf11a5 100644 --- a/openidm-core/src/main/java/org/forgerock/openidm/managed/RelationshipValidator.java +++ b/openidm-core/src/main/java/org/forgerock/openidm/managed/RelationshipValidator.java @@ -12,12 +12,15 @@  * information: "Portions copyright [year] [name of copyright owner]".  *  * Copyright 2015-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC.  */ package org.forgerock.openidm.managed; import static java.text.MessageFormat.format; import static org.forgerock.openidm.util.RelationshipUtil.REFERENCE_ID; +import static org.forgerock.openidm.util.RelationshipUtil.REFERENCE_PROPERTIES; +import org.forgerock.openidm.util.DateUtil; import org.forgerock.openidm.util.ResourceUtil; import org.forgerock.util.annotations.VisibleForTesting; import org.forgerock.json.JsonValue; @@ -32,6 +35,7 @@ import org.slf4j.LoggerFactory; import java.util.HashSet; +import java.util.Objects; import java.util.Set; /** @@ -42,6 +46,7 @@ abstract class RelationshipValidator { static final String TEMPORAL_CONSTRAINTS = "temporalConstraints"; static final String GRANT_TYPE = "_grantType"; + static final String DURATION = "duration"; private static final Logger logger = LoggerFactory.getLogger(RelationshipValidator.class); @@ -103,6 +108,27 @@ abstract void validateSuccessfulReadResponse(Context context, JsonValue relation final void validateRelationship(final JsonValue relationshipField, ResourcePath referrerId, Context context, boolean performDuplicateAssignmentCheck) throws ResourceException { + validateRelationship(relationshipField, null, referrerId, context, performDuplicateAssignmentCheck); + } + + /** + * Validates that the relationshipField will not create an invalid condition, like + * {@link #validateRelationship(JsonValue, ResourcePath, Context, boolean)}, but checks its temporal constraints + * only if they differ from those of the stored relationship it updates. + * + * @param relationshipField the field defining an individual relationship which will be validated. + * @param storedRefProperties the _refProperties of the stored relationship which the relationshipField updates, + * or null if it creates a relationship. + * @param referrerId the id of the object 'hosting' the relationships, aka the referrer + * @param context context of the request working with the relationship. + * @param performDuplicateAssignmentCheck set to true if invocation state should be compared to repository state to determine if + * existing relationships are specified in the invocation + * @throws ResourceException BadRequestException when the relationship is invalid, otherwise for other issues. + * @see #validateChangedTemporalConstraints(JsonValue, JsonValue) + */ + final void validateRelationship(final JsonValue relationshipField, final JsonValue storedRefProperties, + ResourcePath referrerId, Context context, boolean performDuplicateAssignmentCheck) + throws ResourceException { if (relationshipField.isNull()) { // if the new object has the relationshipField removed, we do not need to validate the null // relationshipField because there is no relationship to validate @@ -113,6 +139,7 @@ final void validateRelationship(final JsonValue relationshipField, ResourcePath logger.debug(message); throw new BadRequestException(message); } + validateChangedTemporalConstraints(storedRefProperties, relationshipField.get(REFERENCE_PROPERTIES)); try { validateSuccessfulReadResponse(context, relationshipField, referrerId, relationshipProvider.getConnection() .read(context, newValidateRequest(relationshipField, context)), performDuplicateAssignmentCheck); @@ -124,6 +151,64 @@ final void validateRelationship(final JsonValue relationshipField, ResourcePath } } + /** + * Validates the temporal constraints of a relationship. The effectiveRoles and temporal constraint scripts parse + * the duration of every constraint whenever the managed object is read, so a duration that is not a valid ISO 8601 + * interval (e.g. one whose end is before its start) must be rejected before it is stored. + * + * @param refProperties the _refProperties of the relationship, may be null. + * @throws BadRequestException if there is more than one temporal constraint, or if a constraint does not carry a + * valid ISO 8601 interval as its duration. + */ + static void validateTemporalConstraints(final JsonValue refProperties) throws BadRequestException { + if (refProperties == null || !refProperties.isMap()) { + return; + } + final JsonValue constraints = refProperties.get(TEMPORAL_CONSTRAINTS); + if (constraints.isNull()) { + return; + } + if (!constraints.isList()) { + throw new BadRequestException("Temporal constraints must be an array."); + } + // Currently only 1 temporal constraint is allowed per grant + if (constraints.size() > 1) { + throw new BadRequestException("Only 1 temporal constraint is supported per grant."); + } + for (final JsonValue constraint : constraints) { + final JsonValue duration = constraint.isMap() ? constraint.get(DURATION) : null; + if (duration == null || !duration.isString() + || !DateUtil.getDateUtil().isValidInterval(duration.asString())) { + throw new BadRequestException(format("Temporal constraint duration {0} is not a valid ISO 8601 " + + "interval whose end is not before its start.", + duration == null ? null : duration.getObject())); + } + } + } + + /** + * Validates the temporal constraints of an updated relationship if they differ from the stored ones. A stored + * constraint that is not valid must neither prevent the update that repairs it, nor the writes which carry it + * unchanged, e.g. a managed object update that persists all of its relationships. + * + * @param oldRefProperties the stored _refProperties of the relationship, may be null. + * @param newRefProperties the updated _refProperties of the relationship, may be null. + * @throws BadRequestException if the temporal constraints have changed and are not valid. + * @see #validateTemporalConstraints(JsonValue) + */ + static void validateChangedTemporalConstraints(final JsonValue oldRefProperties, final JsonValue newRefProperties) + throws BadRequestException { + if (!Objects.equals(getTemporalConstraints(oldRefProperties), getTemporalConstraints(newRefProperties))) { + validateTemporalConstraints(newRefProperties); + } + } + + private static Object getTemporalConstraints(final JsonValue refProperties) { + return refProperties == null || !refProperties.isMap() + ? null + : refProperties.get(TEMPORAL_CONSTRAINTS).getObject(); + } + /** * Called to determine if the _refProperties of two relationships are equal. * @param existingRefProps the _refProperties of the existing relationship whose _ref matches that diff --git a/openidm-core/src/test/java/org/forgerock/openidm/managed/CollectionRelationshipProviderTest.java b/openidm-core/src/test/java/org/forgerock/openidm/managed/CollectionRelationshipProviderTest.java index 5550c8ed47..723a0c9ce8 100644 --- a/openidm-core/src/test/java/org/forgerock/openidm/managed/CollectionRelationshipProviderTest.java +++ b/openidm-core/src/test/java/org/forgerock/openidm/managed/CollectionRelationshipProviderTest.java @@ -12,6 +12,7 @@  * information: "Portions copyright [year] [name of copyright owner]".  *  * Copyright 2015-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC.  */ package org.forgerock.openidm.managed; @@ -20,22 +21,35 @@ import static org.mockito.Mockito.*; import static org.testng.Assert.*; +import org.forgerock.http.routing.UriRouterContext; +import org.forgerock.json.JsonPointer; import org.forgerock.json.JsonValue; +import org.forgerock.json.resource.BadRequestException; import org.forgerock.json.resource.Connection; import org.forgerock.json.resource.ConnectionFactory; +import org.forgerock.json.resource.CreateRequest; +import org.forgerock.json.resource.PatchOperation; import org.forgerock.json.resource.PreconditionFailedException; import org.forgerock.json.resource.ReadRequest; +import org.forgerock.json.resource.Requests; import org.forgerock.json.resource.ResourcePath; +import org.forgerock.json.resource.ResourceResponse; +import org.forgerock.json.resource.UpdateRequest; import org.forgerock.openidm.audit.util.ActivityLogger; import org.forgerock.openidm.util.RelationshipUtil; import org.forgerock.services.context.Context; import org.forgerock.services.context.RootContext; +import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; import org.testng.annotations.BeforeTest; import org.testng.annotations.Test; +import java.util.Collections; + public class CollectionRelationshipProviderTest { private static final ResourcePath REFERRING_OBJECT_ID = new ResourcePath("managed/user/foo"); + private static final String VALID_DURATION = "2016-01-01T00:00:00.000Z/2016-01-02T00:00:00.000Z"; + private static final String REVERSED_DURATION = "2016-01-02T00:00:00.000Z/2016-01-01T00:00:00.000Z"; private ManagedObjectSetService managedObjectSyncService; private ConnectionFactory connectionFactory; private ActivityLogger activityLogger; @@ -147,6 +161,167 @@ public void testValidateFieldOnReverseRelationshipField() throws Exception { } } + @Test(expectedExceptions = BadRequestException.class, + expectedExceptionsMessageRegExp = "Temporal constraint duration " + REVERSED_DURATION + " .*") + public void testCreateRejectsInvalidTemporalConstraint() throws Exception { + final Connection connection = mock(Connection.class); + when(connection.createAsync(any(Context.class), any(CreateRequest.class))).thenAnswer(invocation -> + newResourceResponse("g1", "1", ((CreateRequest) invocation.getArguments()[1]).getContent()).asPromise()); + + newRolesProvider(connection).createInstance(managedObjectContext(), + Requests.newCreateRequest("", grant(null, REVERSED_DURATION))).getOrThrow(); + } + + @Test + public void testUpdateKeepsUnchangedInvalidTemporalConstraint() throws Exception { + // a managed object update persists every relationship it carries, including a stored invalid one + final Connection connection = connectionWithStoredGrant(REVERSED_DURATION); + final JsonValue grant = grant("g1", REVERSED_DURATION); + grant.put(new JsonPointer("/_refProperties/_grantType"), "conditional"); + + newRolesProvider(connection).updateInstance(managedObjectContext(), "g1", + Requests.newUpdateRequest("", grant)).getOrThrow(); + + verify(connection).updateAsync(any(Context.class), any(UpdateRequest.class)); + } + + @Test(expectedExceptions = BadRequestException.class, + expectedExceptionsMessageRegExp = "Temporal constraint duration " + REVERSED_DURATION + " .*") + public void testUpdateRejectsChangedInvalidTemporalConstraint() throws Exception { + final Connection connection = connectionWithStoredGrant(VALID_DURATION); + + newRolesProvider(connection).updateInstance(managedObjectContext(), "g1", + Requests.newUpdateRequest("", grant("g1", REVERSED_DURATION))).getOrThrow(); + } + + @Test + public void testPatchRepairsInvalidTemporalConstraint() throws Exception { + final Connection connection = connectionWithStoredGrant(REVERSED_DURATION); + + newRolesProvider(connection).patchInstance(managedObjectContext(), "g1", + Requests.newPatchRequest("", PatchOperation.replace( + "/_refProperties/temporalConstraints/0/duration", VALID_DURATION))).getOrThrow(); + + final ArgumentCaptor update = ArgumentCaptor.forClass(UpdateRequest.class); + verify(connection).updateAsync(any(Context.class), update.capture()); + assertEquals(update.getValue().getContent() + .get(new JsonPointer("/properties/temporalConstraints/0/duration")).asString(), VALID_DURATION); + } + + @Test(expectedExceptions = BadRequestException.class, + expectedExceptionsMessageRegExp = "Temporal constraint duration " + REVERSED_DURATION + " .*") + public void testPatchRejectsInvalidTemporalConstraint() throws Exception { + final Connection connection = connectionWithStoredGrant(VALID_DURATION); + + newRolesProvider(connection).patchInstance(managedObjectContext(), "g1", + Requests.newPatchRequest("", PatchOperation.replace( + "/_refProperties/temporalConstraints/0/duration", REVERSED_DURATION))).getOrThrow(); + } + + @Test + public void testValidateFieldKeepsUnchangedInvalidTemporalConstraintOfChangedGrant() throws Exception { + final Connection connection = connectionWithReadableRole(); + final JsonValue changedGrant = grant("g1", REVERSED_DURATION); + changedGrant.put(new JsonPointer("/_refProperties/_grantType"), "conditional"); + + newRolesProvider(connection).validateRelationshipField(managedObjectContext(), + json(array(grant("g1", REVERSED_DURATION).getObject())), json(array(changedGrant.getObject())), + REFERRING_OBJECT_ID, false); + + // the changed grant is still validated + verify(connection).read(any(Context.class), any(ReadRequest.class)); + } + + @Test(expectedExceptions = BadRequestException.class, + expectedExceptionsMessageRegExp = "Temporal constraint duration " + REVERSED_DURATION + " .*") + public void testValidateFieldRejectsChangedInvalidTemporalConstraint() throws Exception { + newRolesProvider(connectionWithReadableRole()).validateRelationshipField(managedObjectContext(), + json(array(grant("g1", VALID_DURATION).getObject())), + json(array(grant("g1", REVERSED_DURATION).getObject())), REFERRING_OBJECT_ID, false); + } + + @Test + public void testValidateFieldRejectsResentInvalidGrantWithoutId() throws Exception { + // persisting a grant without its _id deletes the stored grant and creates it again, after the commit + final Connection connection = mock(Connection.class); + try { + newRolesProvider(connection).validateRelationshipField(managedObjectContext(), + json(array(grant("g1", REVERSED_DURATION).getObject())), + json(array(grant(null, REVERSED_DURATION).getObject())), REFERRING_OBJECT_ID, false); + fail("Expected BadRequestException"); + } catch (BadRequestException e) { + assertTrue(e.getMessage().startsWith("Temporal constraint duration " + REVERSED_DURATION), + e.getMessage()); + } + verifyZeroInteractions(connection); + } + + @Test + public void testValidateFieldKeepsResentInvalidGrant() throws Exception { + final Connection connection = mock(Connection.class); + + newRolesProvider(connection).validateRelationshipField(managedObjectContext(), + json(array(grant("g1", REVERSED_DURATION).getObject())), + json(array(grant("g1", REVERSED_DURATION).getObject())), REFERRING_OBJECT_ID, false); + + verifyZeroInteractions(connection); + } + + private CollectionRelationshipProvider newRolesProvider(final Connection connection) throws Exception { + final ConnectionFactory factory = mock(ConnectionFactory.class); + when(factory.getConnection()).thenReturn(connection); + final SchemaField schemaField = mock(SchemaField.class); + when(schemaField.isReverseRelationship()).thenReturn(false); + when(schemaField.getName()).thenReturn("roles"); + return new CollectionRelationshipProvider(factory, ResourcePath.resourcePath("managed/user"), schemaField, + activityLogger, managedObjectSyncService); + } + + /** The context of a relationship request made by the managed object user/u1. */ + private static Context managedObjectContext() { + return new ManagedObjectContext(new UriRouterContext(new RootContext(), "", "", + Collections.singletonMap(RelationshipProvider.PARAM_MANAGED_OBJECT_ID, "u1"))); + } + + /** A connection whose repository holds the grant g1 of role r1 to user u1, and which accepts any update. */ + private static Connection connectionWithStoredGrant(final String duration) throws Exception { + final Connection connection = mock(Connection.class); + when(connection.readAsync(any(Context.class), any(ReadRequest.class))).thenAnswer(invocation -> + newResourceResponse("g1", "1", json(object( + field(RelationshipProvider.REPO_FIELD_FIRST_ID, "managed/user/u1"), + field(RelationshipProvider.REPO_FIELD_FIRST_PROPERTY_NAME, "roles"), + field(RelationshipProvider.REPO_FIELD_SECOND_ID, "managed/role/r1"), + field(RelationshipProvider.REPO_FIELD_SECOND_PROPERTY_NAME, null), + field(RelationshipProvider.REPO_FIELD_PROPERTIES, object( + field(RelationshipValidator.TEMPORAL_CONSTRAINTS, + array(object(field(RelationshipValidator.DURATION, duration)))))))) + ).asPromise()); + when(connection.updateAsync(any(Context.class), any(UpdateRequest.class))).thenAnswer(invocation -> + newResourceResponse("g1", "2", ((UpdateRequest) invocation.getArguments()[1]).getContent()).asPromise()); + return connection; + } + + /** A connection which reads the role r1, as the validation of a changed grant does. */ + private static Connection connectionWithReadableRole() throws Exception { + final Connection connection = mock(Connection.class); + when(connection.read(any(Context.class), any(ReadRequest.class))) + .thenReturn(newResourceResponse("r1", "1", json(object(field("_id", "r1"))))); + return connection; + } + + /** A grant of role r1 with a temporal constraint, as a relationship request carries it. */ + private static JsonValue grant(final String id, final String duration) { + final JsonValue refProperties = json(object(field(RelationshipValidator.TEMPORAL_CONSTRAINTS, + array(object(field(RelationshipValidator.DURATION, duration)))))); + if (id != null) { + refProperties.put("_id", id); + refProperties.put("_rev", "1"); + } + return json(object( + field(RelationshipUtil.REFERENCE_ID, "managed/role/r1"), + field(RelationshipUtil.REFERENCE_PROPERTIES, refProperties.getObject()))); + } + private static class IsRouteMatcher extends ArgumentMatcher { private final String route; diff --git a/openidm-core/src/test/java/org/forgerock/openidm/managed/RelationshipValidatorTest.java b/openidm-core/src/test/java/org/forgerock/openidm/managed/RelationshipValidatorTest.java index a6fdaa65c6..505e8f9c9b 100644 --- a/openidm-core/src/test/java/org/forgerock/openidm/managed/RelationshipValidatorTest.java +++ b/openidm-core/src/test/java/org/forgerock/openidm/managed/RelationshipValidatorTest.java @@ -12,6 +12,7 @@  * information: "Portions copyright [year] [name of copyright owner]".  *  * Copyright 2015-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC.  */ package org.forgerock.openidm.managed; @@ -59,6 +60,8 @@ public class RelationshipValidatorTest { public static final JsonValue TEST_RELATIONSHIP = json(object(field(RelationshipUtil.REFERENCE_ID, "managed/widgetPart/part1"))); private static final String RELATIONSHIP_ID = "the_id"; + private static final String VALID_DURATION = "2016-01-01T00:00:00.000Z/2016-01-02T00:00:00.000Z"; + private static final String REVERSED_DURATION = "2016-01-02T00:00:00.000Z/2016-01-01T00:00:00.000Z"; private ManagedObjectSetService managedObjectSyncService; private ConnectionFactory connectionFactory; private ActivityLogger activityLogger; @@ -249,6 +252,100 @@ public void testDuplicateRefProperties(String grantType1, String temporalConstra "ref props should be flagged as identical"); } + @DataProvider(name = "validTemporalConstraints") + public Object[][] createValidTemporalConstraintData() { + return new Object[][] { + { null }, + { json(object()) }, + { json(object(field(RelationshipValidator.TEMPORAL_CONSTRAINTS, null))) }, + { json(object(field(RelationshipValidator.TEMPORAL_CONSTRAINTS, array()))) }, + { makeTemporalConstraints(VALID_DURATION) }, + { makeTemporalConstraints("2016-01-01T00:00:00.000Z/P1D") } + }; + } + + @Test(dataProvider = "validTemporalConstraints") + public void testValidTemporalConstraints(JsonValue refProperties) throws BadRequestException { + RelationshipValidator.validateTemporalConstraints(refProperties); + } + + @DataProvider(name = "invalidTemporalConstraints") + public Object[][] createInvalidTemporalConstraintData() { + return new Object[][] { + // end before start + { makeTemporalConstraints(REVERSED_DURATION), "Temporal constraint duration " + REVERSED_DURATION }, + { makeTemporalConstraints("not an interval"), "Temporal constraint duration not an interval" }, + { makeTemporalConstraints(42), "Temporal constraint duration 42" }, + { json(object(field(RelationshipValidator.TEMPORAL_CONSTRAINTS, array(object())))), + "Temporal constraint duration null" }, + { json(object(field(RelationshipValidator.TEMPORAL_CONSTRAINTS, array("2016-01-01T00:00:00.000Z/P1D")))), + "Temporal constraint duration null" }, + { json(object(field(RelationshipValidator.TEMPORAL_CONSTRAINTS, "2016-01-01T00:00:00.000Z/P1D"))), + "Temporal constraints must be an array." }, + { json(object(field(RelationshipValidator.TEMPORAL_CONSTRAINTS, array( + object(field(RelationshipValidator.DURATION, "2016-01-01T00:00:00.000Z/P1D")), + object(field(RelationshipValidator.DURATION, "2016-02-01T00:00:00.000Z/P1D")))))), + "Only 1 temporal constraint" } + }; + } + + @Test(dataProvider = "invalidTemporalConstraints") + public void testInvalidTemporalConstraints(JsonValue refProperties, String expectedMessage) { + try { + RelationshipValidator.validateTemporalConstraints(refProperties); + fail("Expected BadRequestException"); + } catch (BadRequestException e) { + assertTrue(e.getMessage().startsWith(expectedMessage), e.getMessage()); + } + } + + @DataProvider(name = "changedTemporalConstraints") + public Object[][] createChangedTemporalConstraintData() { + return new Object[][] { + // the stored constraint is replaced, or a constraint is added + { makeTemporalConstraints(VALID_DURATION), makeTemporalConstraints(REVERSED_DURATION) }, + { json(object()), makeTemporalConstraints(REVERSED_DURATION) }, + { null, makeTemporalConstraints(REVERSED_DURATION) } + }; + } + + @Test(dataProvider = "changedTemporalConstraints", expectedExceptions = BadRequestException.class, + expectedExceptionsMessageRegExp = "Temporal constraint duration " + REVERSED_DURATION + " .*") + public void testChangedInvalidTemporalConstraints(JsonValue oldRefProperties, JsonValue newRefProperties) + throws BadRequestException { + RelationshipValidator.validateChangedTemporalConstraints(oldRefProperties, newRefProperties); + } + + @Test + public void testUnchangedInvalidTemporalConstraints() throws BadRequestException { + final JsonValue newRefProperties = makeTemporalConstraints(REVERSED_DURATION); + newRefProperties.put(RelationshipValidator.GRANT_TYPE, "conditional"); + RelationshipValidator.validateChangedTemporalConstraints(makeTemporalConstraints(REVERSED_DURATION), + newRefProperties); + } + + @Test(expectedExceptions = BadRequestException.class, + expectedExceptionsMessageRegExp = "Temporal constraint duration " + REVERSED_DURATION + " .*") + public void testValidateRelationshipRejectsInvalidTemporalConstraint() throws ResourceException { + // the duration is checked before the referenced object is read, which this connection factory cannot do + final SchemaField schemaField = mock(SchemaField.class); + when(schemaField.isReverseRelationship()).thenReturn(false); + when(schemaField.getName()).thenReturn("roles"); + final CollectionRelationshipProvider relationshipProvider = new CollectionRelationshipProvider( + mock(ConnectionFactory.class), new ResourcePath("managed/user"), schemaField, activityLogger, + managedObjectSyncService); + final JsonValue grant = json(object( + field(RelationshipUtil.REFERENCE_ID, "managed/role/r1"), + field(RelationshipUtil.REFERENCE_PROPERTIES, makeTemporalConstraints(REVERSED_DURATION).getObject()))); + relationshipProvider.relationshipValidator.validateRelationship(grant, new ResourcePath("managed/user/u1"), + new RootContext(), false); + } + + private JsonValue makeTemporalConstraints(Object duration) { + return json(object(field(RelationshipValidator.TEMPORAL_CONSTRAINTS, + array(object(field(RelationshipValidator.DURATION, duration)))))); + } + private Map makeRelationship(String referenceId, String grantType, String temporalConstraint) { return json(object( makeField(RelationshipUtil.REFERENCE_ID, referenceId), diff --git a/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/role/EditRoleView.js b/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/role/EditRoleView.js index ebacb6f83e..61c37d7329 100644 --- a/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/role/EditRoleView.js +++ b/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/role/EditRoleView.js @@ -12,6 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2011-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ define([ @@ -90,10 +91,41 @@ function ($, _, Handlebars, toggleCallback: function () { _this.showPendingChanges(); }, + validationCallback: function () { + _this.showPendingChanges(); + }, temporalConstraints: temporalConstraints }); }; + /* + * Returns false when the temporal constraints are enabled and a constraint lacks a date or ends before it starts, + * as the server rejects such a role. + */ + EditRoleView.prototype.areTemporalConstraintsValid = function () { + return !this.$el.find(".enableTemporalConstraintsCheckbox").prop("checked") + || TemporalConstraintsUtils.isTemporalConstraintsFormValid(this.$el.find('.temporalConstraintsForm')); + }; + + EditRoleView.prototype.showPendingChanges = function () { + GenericEditResourceView.showPendingChanges.call(this); + + if (!this.areTemporalConstraintsValid()) { + this.$el.find("#saveBtn").attr("disabled", true); + } + }; + + EditRoleView.prototype.save = function (e, callback) { + if (!this.areTemporalConstraintsValid()) { + if (e) { + e.preventDefault(); + } + return; + } + + return GenericEditResourceView.save.call(this, e, callback); + }; + EditRoleView.prototype.addConditionForm = function () { var resourceDetailsForm = this.$el.find("#resource-details form"), conditionContent = Handlebars.compile("{{> role/_conditionForm}}"); diff --git a/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/role/MembersDialog.js b/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/role/MembersDialog.js index 6de55b3df7..a858cf016a 100644 --- a/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/role/MembersDialog.js +++ b/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/role/MembersDialog.js @@ -12,6 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ define([ @@ -92,6 +93,10 @@ function ($, _, Handlebars, temporalConstraintsView.render({ element: "#" + formContainerId, temporalConstraints: temporalConstraints, + // the server rejects a grant whose temporal constraint lacks a date or ends before it starts + validationCallback: function (isValid) { + $("#resourceCollectionSearchDialogSaveBtn").prop("disabled", !isValid); + }, dialogView: true }); } diff --git a/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/role/TemporalConstraintsFormView.js b/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/role/TemporalConstraintsFormView.js index 6c22a974c3..db54e53a86 100644 --- a/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/role/TemporalConstraintsFormView.js +++ b/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/role/TemporalConstraintsFormView.js @@ -30,7 +30,9 @@ define([ template: "templates/admin/role/TemporalConstraintsFormView.html", events: { "change .enableTemporalConstraintsCheckbox": "toggleForm", - "change .temporalConstraintTimezone": "adjustDateToTimezone" + "change .temporalConstraintTimezone": "adjustDateToTimezone", + "change :input.datetimepicker": "validate", + "blur :input.datetimepicker": "validate" }, partials: [ "partials/role/_temporalConstraint.html" @@ -43,6 +45,7 @@ define([ * temporalConstraints: an array of temporalConstraint objects produced by looping over temporal constraint * duration strings and passing each one of them TemporalConstraintsUtils.convertFromIntervalString * toggleCallback: a function to call when the enable temporal constraints toggle switch is changed + * validationCallback: a function to call with the validity of the form whenever it is validated * dialogView: a boolean value telling the view whether the display is in a dialog or notAssigned * * example of how to call this view: @@ -58,6 +61,7 @@ define([ render: function(args, callback) { this.element = args.element; this.toggleCallback = args.toggleCallback; + this.validationCallback = args.validationCallback; this.data.temporalConstraints = args.temporalConstraints; this.data.hasTemporalConstraints = args.temporalConstraints.length > 0; this.data.timezone = TemporalConstraintsUtils.getDefaultTimezone(); @@ -118,8 +122,11 @@ define([ } endInput.data("DateTimePicker").minDate(e.date); + this.validate(); }, this)); + this.$el.find('.temporalConstraintEndDate').on("dp.change", _.bind(this.validate, this)); + this.$el.find(".temporalConstraintTimezone").selectize(); if (isOnRender && this.data.temporalConstraints && this.data.temporalConstraints.length) { @@ -138,10 +145,31 @@ define([ this.$el.find(".temporalConstraintsFields").hide(); } + this.validate(); + if (this.toggleCallback) { this.toggleCallback(); } }, + /* + * Shows an error and reports the form as invalid when the temporal constraints are enabled and a constraint + * lacks a date or its end date is not after its start date. + * + * @returns {boolean} - true if the temporal constraints are disabled or valid + */ + validate : function () { + var enabled = this.$el.find(".enableTemporalConstraintsCheckbox").prop("checked"), + isValid = !enabled || TemporalConstraintsUtils.isTemporalConstraintsFormValid(this.$el.find(".temporalConstraintsForm")); + + this.$el.find(".temporalConstraintEndDate").closest(".form-group").toggleClass("has-error", !isValid); + this.$el.find(".temporalConstraintError").toggle(!isValid); + + if (this.validationCallback) { + this.validationCallback(isValid); + } + + return isValid; + }, adjustDateToTimezone : function (e) { var newTimezone = $(e.target).val(), constraint = $(e.target).closest(".temporalConstraint"), diff --git a/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/role/util/TemporalConstraintsUtils.js b/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/role/util/TemporalConstraintsUtils.js index 14ba89ff38..5aaa5dffa1 100644 --- a/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/role/util/TemporalConstraintsUtils.js +++ b/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/role/util/TemporalConstraintsUtils.js @@ -12,6 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ define([ @@ -108,6 +109,37 @@ define([ return intervalString; }; + /* + * This function checks the human readable start and end dates of a temporal constraint. The server rejects an + * interval whose end is before its start, and convertToIntervalString would replace an empty date with the + * current time, so both dates are required and the end date must be after the start date. + * + * @param {string} intervalStart - human readable startDate + * @param {string} intervalEnd - human readable endDate + * @returns {boolean} - true if both dates are valid and the end date is after the start date + */ + obj.isValidInterval = function (intervalStart, intervalEnd) { + var start = moment(intervalStart, format, true), + end = moment(intervalEnd, format, true); + + return start.isValid() && end.isValid() && end.isAfter(start); + }; + + /* + * This function takes a jquery object representing a temporal constraints form + * and returns true if each of its temporal constraints has a valid interval + * + * @param {obj} el - jquery object + * @returns {boolean} - true if every temporal constraint in the form is valid + */ + obj.isTemporalConstraintsFormValid = function (el) { + return _.every(el.find(".temporalConstraint"), (constraint) => { + return this.isValidInterval( + $(constraint).find(".temporalConstraintStartDate").val(), + $(constraint).find(".temporalConstraintEndDate").val()); + }); + }; + /* * This function takes a jquery object representing a temporal constraints form * and returns an array of temporal constraint objects diff --git a/openidm-ui/openidm-ui-admin/src/main/resources/partials/role/_temporalConstraint.html b/openidm-ui/openidm-ui-admin/src/main/resources/partials/role/_temporalConstraint.html index 82b0b0ca9c..16e2939d5b 100644 --- a/openidm-ui/openidm-ui-admin/src/main/resources/partials/role/_temporalConstraint.html +++ b/openidm-ui/openidm-ui-admin/src/main/resources/partials/role/_temporalConstraint.html @@ -1,4 +1,5 @@
@@ -32,6 +33,7 @@
+
diff --git a/openidm-ui/openidm-ui-admin/src/test/qunit/org/forgerock/openidm/ui/admin/role/util/TemporalConstraintsUtilsTest.js b/openidm-ui/openidm-ui-admin/src/test/qunit/org/forgerock/openidm/ui/admin/role/util/TemporalConstraintsUtilsTest.js index 3244f36e8e..dc8f4a1ed8 100644 --- a/openidm-ui/openidm-ui-admin/src/test/qunit/org/forgerock/openidm/ui/admin/role/util/TemporalConstraintsUtilsTest.js +++ b/openidm-ui/openidm-ui-admin/src/test/qunit/org/forgerock/openidm/ui/admin/role/util/TemporalConstraintsUtilsTest.js @@ -1,6 +1,23 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Portions Copyright 2026 3A Systems, LLC. + */ + define([ + "jquery", "org/forgerock/openidm/ui/admin/role/util/TemporalConstraintsUtils" -], function (TemporalConstraintsUtils) { +], function ($, TemporalConstraintsUtils) { QUnit.module('TemporalConstraintsUtils Tests'); QUnit.test("convertFromIntervalString", (assert) => { @@ -34,4 +51,29 @@ define([ assert.equal(intervalString, '2016-04-25T07:00:00.000Z/2016-04-30T07:00:00.000Z', "start and end dates are correctly converted to an invervalString with offset"); }); + + QUnit.test("isValidInterval", (assert) => { + assert.ok(TemporalConstraintsUtils.isValidInterval("04/25/2016 7:00 AM", "04/30/2016 7:00 AM"), "end after start is valid"); + assert.notOk(TemporalConstraintsUtils.isValidInterval("04/30/2016 7:00 AM", "04/25/2016 7:00 AM"), "end before start is invalid"); + assert.notOk(TemporalConstraintsUtils.isValidInterval("04/25/2016 7:00 AM", "04/25/2016 7:00 AM"), "end equal to start is invalid"); + assert.notOk(TemporalConstraintsUtils.isValidInterval("04/25/2016 7:00 AM", ""), "an empty end date is invalid"); + assert.notOk(TemporalConstraintsUtils.isValidInterval("", "04/30/2016 7:00 AM"), "an empty start date is invalid"); + assert.notOk(TemporalConstraintsUtils.isValidInterval("04/25/2016 7:00 AM", "not a date"), "an unparseable end date is invalid"); + }); + + QUnit.test("isTemporalConstraintsFormValid", (assert) => { + const constraint = (start, end) => "
" + + "" + + "
", + form = (...constraints) => $("
" + constraints.join("") + "
"); + + assert.ok(TemporalConstraintsUtils.isTemporalConstraintsFormValid(form()), "a form without constraints is valid"); + assert.ok(TemporalConstraintsUtils.isTemporalConstraintsFormValid( + form(constraint("04/25/2016 7:00 AM", "04/30/2016 7:00 AM"))), "end after start is valid"); + assert.notOk(TemporalConstraintsUtils.isTemporalConstraintsFormValid( + form(constraint("04/25/2016 7:00 AM", ""))), "an empty end date is invalid"); + assert.notOk(TemporalConstraintsUtils.isTemporalConstraintsFormValid( + form(constraint("04/25/2016 7:00 AM", "04/30/2016 7:00 AM"), constraint("04/30/2016 7:00 AM", "04/25/2016 7:00 AM"))), + "one constraint with the end before the start makes the form invalid"); + }); }); diff --git a/openidm-ui/openidm-ui-common/src/main/resources/locales/en/translation.json b/openidm-ui/openidm-ui-common/src/main/resources/locales/en/translation.json index 94314a4fc8..a70a190bd6 100644 --- a/openidm-ui/openidm-ui-common/src/main/resources/locales/en/translation.json +++ b/openidm-ui/openidm-ui-common/src/main/resources/locales/en/translation.json @@ -508,6 +508,7 @@ "temporalConstraintsDescription" : "Enable role only during a selected temporal constraint.", "startDate" : "Start Date", "endDate" : "End Date", + "temporalConstraintsInvalid" : "Enter both dates, with the end date after the start date.", "grantType" : "Grant Type", "conditional" : "Conditional", "timezone" : "Timezone" diff --git a/openidm-util/src/main/java/org/forgerock/openidm/util/DateUtil.java b/openidm-util/src/main/java/org/forgerock/openidm/util/DateUtil.java index 7f3ff081a5..70828263ba 100644 --- a/openidm-util/src/main/java/org/forgerock/openidm/util/DateUtil.java +++ b/openidm-util/src/main/java/org/forgerock/openidm/util/DateUtil.java @@ -217,7 +217,26 @@ public DateTime parseIfDate(String timestamp) { */ public boolean isNowWithinInterval(String intervalString) throws IllegalArgumentException { Interval interval = Interval.parse(intervalString); - return interval.contains(DateTime.now()); + return interval.contains(DateTime.now()); + } + + /** + * Returns true if the specified string is an ISO 8601 time interval that the interval methods of this class can + * parse. An interval whose end is before its start is not valid. + * + * @param intervalString a {@link String} object representing an ISO 8601 time interval, may be null. + * @return true if the string parses as a time interval, false otherwise. + */ + public boolean isValidInterval(String intervalString) { + if (intervalString == null) { + return false; + } + try { + Interval.parse(intervalString); + return true; + } catch (IllegalArgumentException | ArithmeticException e) { + return false; + } } /** diff --git a/openidm-util/src/test/java/org/forgerock/openidm/util/DateUtilTest.java b/openidm-util/src/test/java/org/forgerock/openidm/util/DateUtilTest.java index de6dd66c6f..fa358a98a3 100644 --- a/openidm-util/src/test/java/org/forgerock/openidm/util/DateUtilTest.java +++ b/openidm-util/src/test/java/org/forgerock/openidm/util/DateUtilTest.java @@ -135,7 +135,29 @@ public void testIsNowWithinInterval() { assertThat(dateUtil.isNowWithinInterval(passInterval)).isEqualTo(true); assertThat(dateUtil.isNowWithinInterval(failInterval)).isEqualTo(false); } - + + @DataProvider + public Object[][] intervalValidityData() { + return new Object[][] { + { "2016-01-01T09:00:00.000Z/2016-01-02T09:00:00.000Z", true }, + { "2016-01-01T09:00:00.000Z/2016-01-01T09:00:00.000Z", true }, + { "2016-01-01T09:00:00.000Z/P1D", true }, + { "P1D/2016-01-01T09:00:00.000Z", true }, + // end before start + { "2016-01-02T09:00:00.000Z/2016-01-01T09:00:00.000Z", false }, + { "2016-01-01T09:01:00....invalid", false }, + { "2016-01-01T09:00:00.000Z", false }, + { "P1D/P2D", false }, + { "", false }, + { null, false } + }; + } + + @Test(dataProvider = "intervalValidityData") + public void testIsValidInterval(String interval, boolean valid) { + assertThat(dateUtil.isValidInterval(interval)).isEqualTo(valid); + } + @Test(dataProvider = "dateDifferenceInDaysData") public void testGetDateDifferenceInDays(String format, String start, String end, Boolean includeDay, int diff) throws ParseException { diff --git a/openidm-zip/src/main/resources/bin/defaults/script/roles/conditionalRoles.js b/openidm-zip/src/main/resources/bin/defaults/script/roles/conditionalRoles.js index 594c8bdb57..2b66a8c49e 100644 --- a/openidm-zip/src/main/resources/bin/defaults/script/roles/conditionalRoles.js +++ b/openidm-zip/src/main/resources/bin/defaults/script/roles/conditionalRoles.js @@ -12,6 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ @@ -106,6 +107,10 @@ if (isTemporalConstraintsMultiValue(newRole)) { throw {code : 400, message: "Only 1 temporal constraint is supported per role."} } + // A stored invalid constraint must not prevent other changes to the role; a change to it must make it valid + if (JSON.stringify(oldRole.temporalConstraints) !== JSON.stringify(newRole.temporalConstraints)) { + validateTemporalConstraintDurations(newRole); + } /* Only iterate through all of the users if we are dealing with a conditional role, and if the role condition has changed. And if the role's condition has been removed, the new role grantees will be only @@ -131,6 +136,7 @@ if (isTemporalConstraintsMultiValue(newRole)) { throw {code : 400, message: "Only 1 temporal constraint is supported per role."} } + validateTemporalConstraintDurations(newRole); if (relationshipHelper.isRoleConditional(newRole)) { newRole.members = processCreatedConditionalRole(newRole); } @@ -290,6 +296,31 @@ } } + exports.validateTemporalConstraintDurations = validateTemporalConstraintDurations; + /** + * Rejects a role whose temporal constraint does not carry a valid ISO 8601 interval as its duration, e.g. an + * interval whose end is before its start. Such a constraint cannot be evaluated when the effective roles of the + * role members are calculated. + * + * @param role the role to inspect + * @throws BadRequestException if the duration of a temporal constraint is not a valid interval + */ + function validateTemporalConstraintDurations(role) { + var dateUtil = org.forgerock.openidm.util.DateUtil.getDateUtil(), + index, + duration; + if (isNil(role.temporalConstraints)) { + return; + } + for (index in role.temporalConstraints) { + duration = isNil(role.temporalConstraints[index]) ? null : role.temporalConstraints[index].duration; + if (isNil(duration) || !dateUtil.isValidInterval(duration)) { + throw {code : 400, message: "Temporal constraint duration " + duration + + " is not a valid ISO 8601 interval whose end is not before its start."} + } + } + } + /** * Determines if a role's condition has changed * @param oldRole the old role diff --git a/openidm-zip/src/main/resources/bin/defaults/script/roles/effectiveRoles.js b/openidm-zip/src/main/resources/bin/defaults/script/roles/effectiveRoles.js index e6d299a47d..970a6cff47 100644 --- a/openidm-zip/src/main/resources/bin/defaults/script/roles/effectiveRoles.js +++ b/openidm-zip/src/main/resources/bin/defaults/script/roles/effectiveRoles.js @@ -70,7 +70,8 @@ /** * Processes the temporal constraints of a given object. If any temporal constraints are defined, this function will * return true if the current time instant (now) is contained within any of the temporal constraints, false - * otherwise. If no constraints are defined the function will return true. + * otherwise. If no constraints are defined the function will return true. A constraint whose duration is not a + * valid interval is logged and never includes the current time instant. * * @param role the role to process. * @returns false if temporal constraints are defined and don't include the current time instant, true otherwise. @@ -81,7 +82,7 @@ for (var index in object.temporalConstraints) { var constraint = object.temporalConstraints[index]; // If at least one constraint passes, the role is in effect - if (org.forgerock.openidm.util.DateUtil.getDateUtil().isNowWithinInterval(constraint.duration)) { + if (isNowWithinConstraint(constraint)) { return true; } } @@ -91,4 +92,21 @@ return true; }; + /** + * Returns true if the current time instant is contained within the duration of a temporal constraint. An invalid + * duration (e.g. an interval whose end is before its start) must not break reading the object, nor grant the role. + * + * @param constraint the temporal constraint. + * @returns true if the duration is a valid interval which includes the current time instant, false otherwise. + */ + function isNowWithinConstraint(constraint) { + var dateUtil = org.forgerock.openidm.util.DateUtil.getDateUtil(), + duration = (constraint !== undefined && constraint !== null) ? constraint.duration : constraint; + if (!dateUtil.isValidInterval(duration)) { + logger.warn("Ignoring temporal constraint with an invalid duration {}", String(duration)); + return false; + } + return dateUtil.isNowWithinInterval(duration); + } + }()); diff --git a/openidm-zip/src/main/resources/bin/defaults/script/roles/postOperation-roles.js b/openidm-zip/src/main/resources/bin/defaults/script/roles/postOperation-roles.js index 141010f71c..b289edd062 100644 --- a/openidm-zip/src/main/resources/bin/defaults/script/roles/postOperation-roles.js +++ b/openidm-zip/src/main/resources/bin/defaults/script/roles/postOperation-roles.js @@ -246,7 +246,14 @@ function deleteJobsForRoleConstraint(index) { function createJobsForConstraint(constraint, startJobId, endJobId, script) { logger.debug("creating new jobs for: " + constraint); var dateUtil = org.forgerock.openidm.util.DateUtil.getDateUtil(), - startDate = dateUtil.getStartOfInterval(constraint.duration), + duration = (constraint !== undefined && constraint !== null) ? constraint.duration : constraint; + // The resource has already been stored, so an invalid duration must not fail the request + if (!dateUtil.isValidInterval(duration)) { + logger.warn("Not creating schedules for temporal constraint on resource " + resourceName + + " with an invalid duration: " + duration); + return false; + } + var startDate = dateUtil.getStartOfInterval(constraint.duration), endDate = dateUtil.getEndOfInterval(constraint.duration), startExpression = dateUtil.getSchedulerExpression(startDate.plusSeconds(1)), endExpression = dateUtil.getSchedulerExpression(endDate.plusSeconds(1)); diff --git a/openidm-zip/src/main/resources/bin/defaults/script/roles/temporalConstraints.js b/openidm-zip/src/main/resources/bin/defaults/script/roles/temporalConstraints.js index ca42bdf7c2..60968234cf 100644 --- a/openidm-zip/src/main/resources/bin/defaults/script/roles/temporalConstraints.js +++ b/openidm-zip/src/main/resources/bin/defaults/script/roles/temporalConstraints.js @@ -94,6 +94,8 @@ * Returns true if: ß * 1. none of the temporal constraints for the role/grant are currently in effect AND * 2. no temporal constraint is pending, and at least one temporal constraint is expired. + * A temporal constraint whose duration is not a valid interval is logged and is neither in effect, pending nor + * expired. * @param object the grant or role * @returns {boolean} true or false, depending on the rules defined above. */ @@ -103,13 +105,18 @@ index; for (index in object.temporalConstraints) { - var constraint = object.temporalConstraints[index]; + var constraint = object.temporalConstraints[index], + duration = isNil(constraint) ? constraint : constraint.duration; + if (!dateUtil.isValidInterval(duration)) { + logger.warn("Ignoring temporal constraint with an invalid duration {}", String(duration)); + continue; + } // If one constraint passes, the role is in effect, and not expired. If one constraint is in the future, it is // also not expired. - if (dateUtil.isNowWithinInterval(constraint.duration) || dateUtil.isIntervalInFuture(constraint.duration)) { + if (dateUtil.isNowWithinInterval(duration) || dateUtil.isIntervalInFuture(duration)) { return false; } - constraintExpired = constraintExpired || dateUtil.isIntervalInPast(constraint.duration); + constraintExpired = constraintExpired || dateUtil.isIntervalInPast(duration); } return constraintExpired; }; diff --git a/openidm-zip/src/test/resources/bin/defaults/script/conditionalRolesTest.js b/openidm-zip/src/test/resources/bin/defaults/script/conditionalRolesTest.js index 91dfc522ea..661a87e259 100644 --- a/openidm-zip/src/test/resources/bin/defaults/script/conditionalRolesTest.js +++ b/openidm-zip/src/test/resources/bin/defaults/script/conditionalRolesTest.js @@ -12,6 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ /** @@ -22,6 +23,81 @@ exports.test = function() { var _ = require('lib/lodash'); isTemporalConstraintsMultiValue(); + validateTemporalConstraintDurations(); + + function validateTemporalConstraintDurations() { + var dateUtil = org.forgerock.openidm.util.DateUtil.getDateUtil(), + now = dateUtil.currentDateTime(), + currentDuration = dateUtil.formatDateTime(now.minusDays(1)) + + "/" + dateUtil.formatDateTime(now.plusDays(1)), + // end before start, see issue #250 + reversedDuration = dateUtil.formatDateTime(now.plusDays(1)) + + "/" + dateUtil.formatDateTime(now.minusDays(1)); + [ + [ { "_id": "roleWithoutConstraints" }, false ], + [ { "_id": "roleWithValidConstraint", "temporalConstraints": [ { "duration": currentDuration } ] }, false ], + [ { "_id": "roleWithPeriodConstraint", "temporalConstraints": [ { "duration": dateUtil.formatDateTime(now) + "/P1D" } ] }, false ], + [ { "_id": "roleWithReversedConstraint", "temporalConstraints": [ { "duration": reversedDuration } ] }, true ], + [ { "_id": "roleWithInvalidConstraint", "temporalConstraints": [ { "duration": "not an interval" } ] }, true ], + [ { "_id": "roleWithoutDuration", "temporalConstraints": [ { } ] }, true ] + ].map( + function (testcase) { + (function (role, expectedRejection) { + var rejected = false; + try { + conditionalRoles.validateTemporalConstraintDurations(role); + } catch (e) { + if (e.code !== 400) { + throw e; + } + rejected = true; + } + if (rejected !== expectedRejection) { + throw { + "message": "Validating temporal constraint durations of role " + role._id + ", rejected <" + + rejected + ">, expected <" + expectedRejection + ">" + }; + } + }).apply(null, testcase); + }); + + // roleCreate and roleUpdate reject a role with an invalid duration; the roles are not conditional, so no + // openidm call is reached + [ + [ "roleCreate rejects an invalid constraint", function (role) { + conditionalRoles.roleCreate(role); + }, true ], + [ "roleUpdate rejects a constraint changed to an invalid one", function (role) { + conditionalRoles.roleUpdate({ "_id": role._id, "temporalConstraints": [ { "duration": currentDuration } ] }, role); + }, true ], + [ "roleUpdate rejects an added invalid constraint", function (role) { + conditionalRoles.roleUpdate({ "_id": role._id }, role); + }, true ], + [ "roleUpdate keeps a stored invalid constraint that the update does not change", function (role) { + conditionalRoles.roleUpdate({ "_id": role._id, "description": "before", + "temporalConstraints": [ { "duration": reversedDuration } ] }, role); + }, false ] + ].map( + function (testcase) { + (function (description, write, expectedRejection) { + var rejected = false; + try { + write({ "_id": "roleWithReversedConstraint", "description": "after", + "temporalConstraints": [ { "duration": reversedDuration } ] }); + } catch (e) { + if (e.code !== 400) { + throw e; + } + rejected = true; + } + if (rejected !== expectedRejection) { + throw { + "message": description + ": rejected <" + rejected + ">, expected <" + expectedRejection + ">" + }; + } + }).apply(null, testcase); + }); + } function isTemporalConstraintsMultiValue() { var dateUtil = org.forgerock.openidm.util.DateUtil.getDateUtil(), diff --git a/openidm-zip/src/test/resources/bin/defaults/script/effectiveRolesTest.js b/openidm-zip/src/test/resources/bin/defaults/script/effectiveRolesTest.js index aafe58eaca..b009b09530 100644 --- a/openidm-zip/src/test/resources/bin/defaults/script/effectiveRolesTest.js +++ b/openidm-zip/src/test/resources/bin/defaults/script/effectiveRolesTest.js @@ -12,6 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ /** @@ -33,7 +34,11 @@ exports.test = function() { failDuration1 = dateUtil.formatDateTime(now.plusDays(1)) + "/" + dateUtil.formatDateTime(now.plusDays(2)), failDuration2 = dateUtil.formatDateTime(now.plusHours(1)) - + "/" + dateUtil.formatDateTime(now.plusHours(2)); + + "/" + dateUtil.formatDateTime(now.plusHours(2)), + // end before start, see issue #250 + reversedDuration = dateUtil.formatDateTime(now.plusDays(1)) + + "/" + dateUtil.formatDateTime(now.minusDays(1)), + invalidDuration = "not an interval"; // test cases for applyConstraint [ @@ -89,6 +94,59 @@ exports.test = function() { ] }, false + ], + [ + { + "_id" : "role5", + "temporalConstraints" : [ + { + "duration" : reversedDuration + } + ] + }, + false + ], + [ + { + "_id" : "role6", + "temporalConstraints" : [ + { + "duration" : invalidDuration + } + ] + }, + false + ], + [ + { + "_id" : "role7", + "temporalConstraints" : [ + { + } + ] + }, + false + ], + [ + { + "_id" : "role8", + "temporalConstraints" : [ + { + "duration" : reversedDuration + }, + { + "duration" : passDuration1 + } + ] + }, + true + ], + [ + { + "_id" : "role9", + "temporalConstraints" : [ null ] + }, + false ] ].map( function (testcase) { @@ -103,5 +161,26 @@ exports.test = function() { } }).apply(null, testcase); }); + + // an operator finds the stored invalid constraints by this warning + (function () { + var warnings = [], + warn = logger.warn; + logger.warn = function () { + warnings.push(Array.prototype.slice.call(arguments)); + }; + try { + effectiveRoles.processTemporalConstraints( + { "_id" : "role5", "temporalConstraints" : [ { "duration" : reversedDuration } ] }); + } finally { + logger.warn = warn; + } + if (warnings.length !== 1 || String(warnings[0][1]) !== reversedDuration) { + throw { + "message": "Expected one warning naming the invalid duration " + reversedDuration + ", got " + + JSON.stringify(warnings) + }; + } + }()); } } \ No newline at end of file diff --git a/openidm-zip/src/test/resources/bin/defaults/script/postOperationRolesTest.js b/openidm-zip/src/test/resources/bin/defaults/script/postOperationRolesTest.js new file mode 100644 index 0000000000..cb4c9d1237 --- /dev/null +++ b/openidm-zip/src/test/resources/bin/defaults/script/postOperationRolesTest.js @@ -0,0 +1,94 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ + +/** + * Tests against roles/postOperation-roles.js, which is not a module: it runs with the globals of a managed object + * postCreate/postUpdate/postDelete script, so it is evaluated here as the body of a function taking those globals. + */ +exports.test = function() { + var source = readScript("bin/defaults/script/roles/postOperation-roles.js"), + postOperationRoles = new Function("request", "context", "oldObject", "newObject", "resourceName", "openidm", + source); + + createJobsForConstraint(); + + function readScript(path) { + var stream = java.lang.Thread.currentThread().getContextClassLoader().getResourceAsStream(path); + if (stream === null) { + throw { "message": "Script not found on the classpath: " + path }; + } + try { + return String(new java.lang.String(stream.readAllBytes(), "UTF-8")); + } finally { + stream.close(); + } + } + + /** + * Runs the script for a created or updated object and returns the ids of the schedules it created. + */ + function createdJobs(method, resourceName, oldObject, newObject) { + var jobs = [], + openidm = { + "create": function (resourceContainer, newResourceId) { + jobs.push(String(newResourceId)); + }, + "delete": function () {} + }; + postOperationRoles({ "method": method }, null, oldObject, newObject, new java.lang.String(resourceName), + openidm); + return jobs; + } + + function grant(constraints) { + return { "roles": [ { "_ref": "managed/role/r1", + "_refProperties": { "_id": "g1", "temporalConstraints": constraints } } ] }; + } + + function createJobsForConstraint() { + var dateUtil = org.forgerock.openidm.util.DateUtil.getDateUtil(), + now = dateUtil.currentDateTime(), + pendingDuration = dateUtil.formatDateTime(now.plusDays(1)) + + "/" + dateUtil.formatDateTime(now.plusDays(2)), + // end before start, see issue #250 + reversedDuration = dateUtil.formatDateTime(now.plusDays(1)) + + "/" + dateUtil.formatDateTime(now.minusDays(1)); + [ + [ "create", "managed/role/r1", null, { "temporalConstraints": [ { "duration": pendingDuration } ] }, 2 ], + // an invalid constraint that is already stored must not fail the request, nor create a schedule + [ "create", "managed/role/r1", null, { "temporalConstraints": [ { "duration": reversedDuration } ] }, 0 ], + [ "create", "managed/role/r1", null, { "temporalConstraints": [ { "duration": "not an interval" } ] }, 0 ], + [ "create", "managed/role/r1", null, { "temporalConstraints": [ null ] }, 0 ], + [ "create", "managed/user/u1", null, grant([ { "duration": pendingDuration } ]), 2 ], + [ "create", "managed/user/u1", null, grant([ { "duration": reversedDuration } ]), 0 ], + [ "create", "managed/user/u1", null, grant([ null ]), 0 ], + [ "update", "managed/user/u1", grant([ { "duration": reversedDuration } ]), + grant([ { "duration": pendingDuration } ]), 2 ], + [ "update", "managed/user/u1", grant([ { "duration": pendingDuration } ]), grant([ null ]), 0 ] + ].map( + function (testcase) { + (function (method, resourceName, oldObject, newObject, expectedJobs) { + var jobs = createdJobs(method, resourceName, oldObject, newObject); + if (jobs.length !== expectedJobs) { + throw { + "message": method + " of " + resourceName + " " + JSON.stringify(newObject) + + " created jobs " + JSON.stringify(jobs) + ", expected " + expectedJobs + " jobs" + }; + } + }).apply(null, testcase); + }); + } +} diff --git a/openidm-zip/src/test/resources/bin/defaults/script/temporalConstraintsTest.js b/openidm-zip/src/test/resources/bin/defaults/script/temporalConstraintsTest.js index e384951b59..cfc383fb94 100644 --- a/openidm-zip/src/test/resources/bin/defaults/script/temporalConstraintsTest.js +++ b/openidm-zip/src/test/resources/bin/defaults/script/temporalConstraintsTest.js @@ -12,6 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ /** @@ -31,6 +32,9 @@ exports.test = function() { pendingDuration = dateUtil.formatDateTime(now.plusDays(1)) + "/" + dateUtil.formatDateTime(now.plusDays(2)), expiredDuration = dateUtil.formatDateTime(now.minusDays(2)) + + "/" + dateUtil.formatDateTime(now.minusDays(1)), + // end before start, see issue #250 + reversedDuration = dateUtil.formatDateTime(now.plusDays(1)) + "/" + dateUtil.formatDateTime(now.minusDays(1)); [ @@ -66,6 +70,43 @@ exports.test = function() { ] }, true + ], + [ + { + "_id" : "role3", + "temporalConstraints" : [ + { + "duration" : reversedDuration + } + ] + }, + false + ], + [ + { + "_id" : "role4", + "temporalConstraints" : [ + { + "duration" : reversedDuration + }, + { + "duration" : expiredDuration + } + ] + }, + true + ], + [ + { + "_id" : "role5", + "temporalConstraints" : [ + null, + { + "duration" : expiredDuration + } + ] + }, + true ] ].map( function (testcase) { diff --git a/openidm-zip/src/test/resources/testRunner.js b/openidm-zip/src/test/resources/testRunner.js index 8f6e5c6b2f..50a75e39ba 100644 --- a/openidm-zip/src/test/resources/testRunner.js +++ b/openidm-zip/src/test/resources/testRunner.js @@ -19,6 +19,12 @@ * Backend script module test runner. For each module to be tested, create a suitable *Test module that * exports a "test" method, and add it to the array of test modules below. */ +// The backend scripts log through the logger binding that OpenIDM supplies at runtime. +var logger = (function () { + function noOp() {} + return { trace: noOp, debug: noOp, info: noOp, warn: noOp, error: noOp }; +}()); + [ "policyFilterTest", "policyUniqueTest", "routerAuthzTest", @@ -28,6 +34,7 @@ "effectiveAssignmentsTest", "temporalConstraintsTest", "conditionalRolesTest", + "postOperationRolesTest", "managedPatchHelperTest", "connectionPoolPatchHelperTest"] .forEach(function (module) {