Skip to content

Commit

Permalink
[eclipse-ditto#890] Ignore expiring subjects in policies validator; f…
Browse files Browse the repository at this point in the history 
…ix PolicyCommandEnforcementTest.

Signed-off-by: Yufei Cai <yufei.cai@bosch.io>
  • Loading branch information
@yufei-cai
yufei-cai committed Dec 14, 2020
1 parent a685b26 commit 43db6b8
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 11 deletions.
2 changes: 2 additions & 0 deletions ...t/java/org/eclipse/ditto/services/concierge/enforcement/PolicyCommandEnforcementTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.eclipse.ditto.model.base.auth.AuthorizationSubject;
import org.eclipse.ditto.model.base.auth.DittoAuthorizationContextType;
import org.eclipse.ditto.model.base.exceptions.DittoRuntimeException;
import org.eclipse.ditto.model.base.headers.DittoHeaderDefinition;
import org.eclipse.ditto.model.base.headers.DittoHeaders;
import org.eclipse.ditto.model.base.headers.entitytag.EntityTagMatchers;
import org.eclipse.ditto.model.base.json.FieldType;
Expand Down Expand Up @@ -103,6 +104,7 @@ public final class PolicyCommandEnforcementTest {
.authorizationContext(AuthorizationContext.newInstance(DittoAuthorizationContextType.UNSPECIFIED,
AuthorizationSubject.newInstance(AUTH_SUBJECT_ID)))
.correlationId(CORRELATION_ID)
.putHeader(DittoHeaderDefinition.POLICY_ENFORCER_INVALIDATED_PREEMPTIVELY.getKey(), "true")
.build();

private static final DittoHeaders DITTO_HEADERS_WITH_CORRELATION_ID = DittoHeaders.newBuilder()
Expand Down
10 changes: 7 additions & 3 deletions .../policies/src/main/java/org/eclipse/ditto/services/models/policies/PoliciesValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public final class PoliciesValidator implements Validator {
private static final ResourceKey ROOT_RESOURCE = PoliciesResourceType.policyResource("/");

private static final String NO_AUTH_SUBJECT_PATTERN =
"It must contain at least one Subject with permission(s) <{0}> on resource <{1}>!";
"It must contain at least one permanent Subject with permission(s) <{0}> on resource <{1}>!";

private static final String SUBJECT_EXPIRY_NOT_IN_PAST_PATTERN =
"The 'expiry' of a Policy Subject may not be in the past, but it was: <{0}>.";
Expand Down Expand Up @@ -80,14 +80,18 @@ public boolean isValid() {
return false;
}

final Set<Subjects> withPermissionGranted = StreamSupport.stream(policyEntries.spliterator(), false)
// Disregard expiring subjects when testing for permissions granted because those are deleted after some time.
final Set<Subject> withPermissionGranted = StreamSupport.stream(policyEntries.spliterator(), false)
.filter(this::hasPermissionGranted)
.map(PolicyEntry::getSubjects)
.flatMap(Subjects::stream)
.filter(subject -> subject.getExpiry().isEmpty())
.collect(Collectors.toSet());

final Set<Subjects> withPermissionRevoked = StreamSupport.stream(policyEntries.spliterator(), false)
final Set<Subject> withPermissionRevoked = StreamSupport.stream(policyEntries.spliterator(), false)
.filter(this::hasPermissionRevoked)
.map(PolicyEntry::getSubjects)
.flatMap(Subjects::stream)
.collect(Collectors.toSet());

withPermissionGranted.removeAll(withPermissionRevoked);
Expand Down
31 changes: 26 additions & 5 deletions ...icies/src/test/java/org/eclipse/ditto/services/models/policies/PoliciesValidatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@
import static org.eclipse.ditto.services.models.policies.TestConstants.Policy.PERMISSION_READ;
import static org.eclipse.ditto.services.models.policies.TestConstants.Policy.PERMISSION_WRITE;
import static org.eclipse.ditto.services.models.policies.TestConstants.Policy.SUBJECT_ISSUER;
import static org.junit.Assert.assertEquals;

import java.text.MessageFormat;
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.stream.Collectors;

import org.eclipse.ditto.model.base.auth.AuthorizationSubject;
import org.eclipse.ditto.model.policies.Permissions;
Expand All @@ -30,6 +32,7 @@
import org.eclipse.ditto.model.policies.PolicyEntry;
import org.eclipse.ditto.model.policies.Resource;
import org.eclipse.ditto.model.policies.Subject;
import org.eclipse.ditto.model.policies.SubjectExpiry;
import org.eclipse.ditto.model.policies.SubjectId;
import org.junit.Test;

Expand All @@ -45,8 +48,8 @@ public void validatePoliciesSuccess() {
final PoliciesValidator validator =
PoliciesValidator.newInstance(Collections.singletonList(entry));

assertEquals(validator.isValid(), true);
assertEquals(validator.getReason().isPresent(), false);
assertThat(validator.isValid()).isTrue();
assertThat(validator.getReason()).isEmpty();
}

@Test
Expand All @@ -57,11 +60,29 @@ public void validatePoliciesFailure() {

assertThat(validator.isValid()).isFalse();
assertThat(validator.getReason()).isPresent();
assertThat(validator.getReason().get()).isEqualTo(
MessageFormat.format("It must contain at least one Subject with permission(s) <{0}> on resource <{1}>!",
assertThat(validator.getReason().get())
.isEqualTo(MessageFormat.format(
"It must contain at least one permanent Subject with permission(s) <{0}> on resource <{1}>!",
Permissions.newInstance(PERMISSION_WRITE), PoliciesResourceType.policyResource("/")));
}

@Test
public void policyWithOnlyExpiringSubjectsIsNotValid() {
final SubjectExpiry expiry = PoliciesModelFactory.newSubjectExpiry(Instant.now().plus(Duration.ofDays(1L)));
final PolicyEntry baseEntry = createPolicyEntry(PERMISSION_READ, PERMISSION_WRITE);
final PolicyEntry expiringEntry = PoliciesModelFactory.newPolicyEntry(baseEntry.getLabel(),
baseEntry.getSubjects()
.stream()
.map(subject -> Subject.newInstance(subject.getId(), subject.getType(), expiry))
.collect(Collectors.toList()),
baseEntry.getResources());
final PoliciesValidator validator =
PoliciesValidator.newInstance(Collections.singletonList(expiringEntry));

assertThat(validator.isValid()).isFalse();
assertThat(validator.getReason().orElseThrow()).contains("It must contain at least one permanent Subject");
}

private PolicyEntry createPolicyEntry(final String... permissions) {
final AuthorizationSubject authorizationSubject = AuthorizationSubject.newInstance("134233");
final SubjectId subjectId =
Expand Down
80 changes: 77 additions & 3 deletions ...va/org/eclipse/ditto/services/policies/persistence/actors/PolicyPersistenceActorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
import org.eclipse.ditto.signals.commands.policies.PolicyCommandSizeValidator;
import org.eclipse.ditto.signals.commands.policies.exceptions.PolicyEntryModificationInvalidException;
import org.eclipse.ditto.signals.commands.policies.exceptions.PolicyEntryNotAccessibleException;
import org.eclipse.ditto.signals.commands.policies.exceptions.PolicyModificationInvalidException;
import org.eclipse.ditto.signals.commands.policies.exceptions.PolicyNotAccessibleException;
import org.eclipse.ditto.signals.commands.policies.exceptions.ResourceNotAccessibleException;
import org.eclipse.ditto.signals.commands.policies.exceptions.SubjectNotAccessibleException;
Expand All @@ -87,6 +88,7 @@
import org.eclipse.ditto.signals.commands.policies.modify.ModifyPolicyEntry;
import org.eclipse.ditto.signals.commands.policies.modify.ModifyResource;
import org.eclipse.ditto.signals.commands.policies.modify.ModifySubject;
import org.eclipse.ditto.signals.commands.policies.modify.ModifySubjects;
import org.eclipse.ditto.signals.commands.policies.query.PolicyQueryCommandResponse;
import org.eclipse.ditto.signals.commands.policies.query.RetrievePolicy;
import org.eclipse.ditto.signals.commands.policies.query.RetrievePolicyEntry;
Expand Down Expand Up @@ -806,9 +808,11 @@ public void createPolicyWith2SubjectsWithExpiry() {
final Subject subject2 =
Subject.newInstance(SubjectId.newInstance(SubjectIssuer.GOOGLE, "subject2"),
SubjectType.GENERATED, subjectExpiry);
final Subject subject3 =
Subject.newInstance(SubjectId.newInstance(SubjectIssuer.GOOGLE, "subject3"), SubjectType.GENERATED);
final Policy policy = PoliciesModelFactory.newPolicyBuilder(PolicyId.of("policy:id"))
.forLabel(POLICY_LABEL)
.setSubjects(Subjects.newInstance(subject1, subject2))
.setSubjects(Subjects.newInstance(subject1, subject2, subject3))
.setResources(POLICY_RESOURCES_ALL)
.build();

Expand Down Expand Up @@ -849,10 +853,80 @@ public void createPolicyWith2SubjectsWithExpiry() {
assertThat(((SubjectDeleted) subject2Deleted.msg()).getSubjectId()).isEqualTo(subject2.getId());
assertThat(((SubjectDeleted) subject2Deleted.msg()).getRevision()).isEqualTo(3L);

// THEN: the policy has no subjects. (TODO: rationalize or change this behavior.)
// THEN: the policy has only subject3 left.
underTest.tell(RetrievePolicy.of(policy.getEntityId().orElseThrow(), DittoHeaders.empty()), getRef());
final RetrievePolicyResponse response = expectMsgClass(RetrievePolicyResponse.class);
Assertions.assertThat(response.getPolicy().getEntryFor(POLICY_LABEL).orElseThrow().getSubjects()).isEmpty();
Assertions.assertThat(response.getPolicy().getEntryFor(POLICY_LABEL).orElseThrow().getSubjects())
.containsOnly(subject3);
}};
}

@Test
public void impossibleToMakePolicyInvalidByExpiringSubjects() {
new TestKit(actorSystem) {{
final Instant futureInstant = Instant.now().plus(Duration.ofDays(1L));
final SubjectExpiry subjectExpiry = SubjectExpiry.newInstance(futureInstant);
final Subject subject1 =
Subject.newInstance(SubjectId.newInstance(SubjectIssuer.GOOGLE, "subject1"),
SubjectType.GENERATED, subjectExpiry);
final Subject subject2 =
Subject.newInstance(SubjectId.newInstance(SubjectIssuer.GOOGLE, "subject2"),
SubjectType.GENERATED, subjectExpiry);
final Subject subject3 =
Subject.newInstance(SubjectId.newInstance(SubjectIssuer.GOOGLE, "subject3"),
SubjectType.GENERATED);
final Subject subject4 =
Subject.newInstance(SubjectId.newInstance(SubjectIssuer.GOOGLE, "subject4"),
SubjectType.GENERATED,
SubjectExpiry.newInstance(Instant.EPOCH));
final PolicyId policyId = PolicyId.of("policy:id");
final DittoHeaders headers = DittoHeaders.empty();

final Policy validPolicy = PoliciesModelFactory.newPolicyBuilder(policyId)
.forLabel(POLICY_LABEL)
.setSubjects(Subjects.newInstance(subject3))
.setResources(POLICY_RESOURCES_ALL)
.build();

final Policy policyWithExpiredSubject = PoliciesModelFactory.newPolicyBuilder(policyId)
.forLabel(POLICY_LABEL)
.setSubjects(Subjects.newInstance(subject1, subject2, subject3, subject4))
.setResources(POLICY_RESOURCES_ALL)
.build();

final Subjects expiringSubjects = Subjects.newInstance(subject1, subject2);
final PolicyEntry expiringEntry =
PoliciesModelFactory.newPolicyEntry(POLICY_LABEL, expiringSubjects, POLICY_RESOURCES_ALL);
final Policy policyWithoutPermanentSubject = PoliciesModelFactory.newPolicyBuilder(policyId)
.set(expiringEntry)
.build();

// GIVEN: policy is not created
final ActorRef underTest = createPersistenceActorFor(this, policyId);

// WHEN/THEN CreatePolicy fails if policy contains expired subject
underTest.tell(CreatePolicy.of(policyWithExpiredSubject, headers), getRef());
expectMsgClass(PolicyModificationInvalidException.class);

// GIVEN: policy is created
underTest.tell(CreatePolicy.of(validPolicy, headers), getRef());
expectMsgClass(CreatePolicyResponse.class);

// WHEN/THEN ModifyPolicy fails if policy contains only expiring subjects afterwards
underTest.tell(ModifyPolicy.of(policyId, policyWithoutPermanentSubject, headers), getRef());
expectMsgClass(PolicyModificationInvalidException.class);

// WHEN/THEN ModifyPolicyEntry fails if policy contains only expiring subjects afterwards
underTest.tell(ModifyPolicyEntry.of(policyId, expiringEntry, headers), getRef());
expectMsgClass(PolicyEntryModificationInvalidException.class);

// WHEN/THEN ModifySubjects fails if policy contains only expiring subjects afterwards
underTest.tell(ModifySubjects.of(policyId, POLICY_LABEL, expiringSubjects, headers), getRef());
expectMsgClass(PolicyEntryModificationInvalidException.class);

// WHEN/THEN DeleteSubject fails if policy contains only expiring subjects afterwards
underTest.tell(DeleteSubject.of(policyId, POLICY_LABEL, subject3.getId(), headers), getRef());
expectMsgClass(PolicyEntryModificationInvalidException.class);
}};
}

Expand Down

0 comments on commit 43db6b8

Please sign in to comment.