Skip to content

Commit

Permalink
[eclipse-ditto#926] Prevent backtracking in TokenIntegrationSubjectId…
Browse files Browse the repository at this point in the history 
…Factory; fix policy action event aggregation.

Changes

1. Replaced TokenIntegrationSubjectIdFactory.JSON_ARRAY_PATTERN
   by a regex using possessive qualifiers only.

2. Added a test for activating multiple subjects in multiple
   policy entries. Fixed it.

Signed-off-by: Yufei Cai <yufei.cai@bosch.io>
  • Loading branch information
@yufei-cai
yufei-cai committed Jan 22, 2021
1 parent 3dfeb3e commit a492eb4
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 24 deletions.
7 changes: 4 additions & 3 deletions ...se/ditto/services/gateway/endpoints/routes/policies/TokenIntegrationSubjectIdFactory.java
View file
Expand Up @@ -29,9 +29,10 @@
public interface TokenIntegrationSubjectIdFactory {

/**
* Compiled Pattern of a string containing any unresolved JsonArray-String notations inside.
* Compiled Pattern of a string containing any unresolved non-empty JsonArray-String notations inside.
* All strings matching this pattern are valid JSON arrays. Not all JSON arrays match this pattern.
*/
Pattern JSON_ARRAY_PATTERN = Pattern.compile(".*(\\[\".*\"])+?.*");
Pattern JSON_ARRAY_PATTERN = Pattern.compile("(\\[\"(?:\\\\\"|[^\"])*+\"(?:,\"(?:\\\\\"|[^\"])*+\")*+])");

/**
* Compute the token integration subject IDs from headers and JWT.
Expand All @@ -57,7 +58,7 @@ public interface TokenIntegrationSubjectIdFactory {
static Stream<String> expandJsonArraysInResolvedSubject(final String resolvedSubject) {
final Matcher jsonArrayMatcher = JSON_ARRAY_PATTERN.matcher(resolvedSubject);
final int group = 1;
if (jsonArrayMatcher.matches()) {
if (jsonArrayMatcher.find()) {
final String beforeMatched = resolvedSubject.substring(0, jsonArrayMatcher.start(group));
final String matchedStr =
resolvedSubject.substring(jsonArrayMatcher.start(group), jsonArrayMatcher.end(group));
Expand Down
6 changes: 3 additions & 3 deletions ...services/gateway/endpoints/routes/policies/OAuthTokenIntegrationSubjectIdFactoryTest.java
View file
Expand Up @@ -61,7 +61,7 @@ public void resolveSubjectIdWithJsonArrayJwtClaim() {
final OAuthTokenIntegrationSubjectIdFactory sut = createSut(subjectPattern);
final DittoHeaders dittoHeaders = DittoHeaders.newBuilder().build();
final Set<SubjectId> subjectIds = sut.getSubjectIds(dittoHeaders, new DummyJwt());
Assertions.assertThat(subjectIds).hasSize(2).containsExactly(
Assertions.assertThat(subjectIds).hasSize(2).containsExactlyInAnyOrder(
SubjectId.newInstance("integration:aud-1:static"),
SubjectId.newInstance("integration:aud-2:static")
);
Expand All @@ -73,7 +73,7 @@ public void resolveSubjectIdWithJsonArraySingleNestedValueJwtClaim() {
final OAuthTokenIntegrationSubjectIdFactory sut = createSut(subjectPattern);
final DittoHeaders dittoHeaders = DittoHeaders.newBuilder().build();
final Set<SubjectId> subjectIds = sut.getSubjectIds(dittoHeaders, new DummyJwt());
Assertions.assertThat(subjectIds).hasSize(1).containsExactly(
Assertions.assertThat(subjectIds).hasSize(1).containsExactlyInAnyOrder(
SubjectId.newInstance("I am a nested single:{{some:unresolved}}")
);
}
Expand All @@ -84,7 +84,7 @@ public void resolveSubjectIdWithMultipleJsonArrayJwtClaims() {
final OAuthTokenIntegrationSubjectIdFactory sut = createSut(subjectPattern);
final DittoHeaders dittoHeaders = DittoHeaders.newBuilder().build();
final Set<SubjectId> subjectIds = sut.getSubjectIds(dittoHeaders, new DummyJwt());
Assertions.assertThat(subjectIds).hasSize(6).containsExactly(
Assertions.assertThat(subjectIds).hasSize(6).containsExactlyInAnyOrder(
SubjectId.newInstance("dummy-issuer:aud-1:static:bar1"),
SubjectId.newInstance("dummy-issuer:aud-2:static:bar1"),
SubjectId.newInstance("dummy-issuer:aud-1:static:bar2"),
Expand Down
48 changes: 46 additions & 2 deletions ...icies/persistence/actors/strategies/commands/TopLevelPolicyActionCommandStrategyTest.java
View file
Expand Up @@ -20,15 +20,19 @@
import java.text.MessageFormat;
import java.time.Duration;
import java.time.Instant;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

import org.eclipse.ditto.model.base.headers.DittoHeaders;
import org.eclipse.ditto.model.placeholders.UnresolvedPlaceholderException;
import org.eclipse.ditto.model.policies.Label;
import org.eclipse.ditto.model.policies.PoliciesModelFactory;
import org.eclipse.ditto.model.policies.Policy;
import org.eclipse.ditto.model.policies.PolicyEntry;
import org.eclipse.ditto.model.policies.PolicyId;
import org.eclipse.ditto.model.policies.ResourceKey;
import org.eclipse.ditto.model.policies.Subject;
Expand Down Expand Up @@ -63,14 +67,12 @@ public final class TopLevelPolicyActionCommandStrategyTest extends AbstractPolic
private static final Label DUMMY_LABEL = Label.of("-");

private TopLevelPolicyActionCommandStrategy underTest;
private ActivateTokenIntegrationStrategy activateTokenIntegrationStrategy;

@Before
public void setUp() {
final PolicyConfig policyConfig = DefaultPolicyConfig.of(ConfigFactory.load("policy-test"));
final ActorSystem system = ActorSystem.create("test");
underTest = new TopLevelPolicyActionCommandStrategy(policyConfig, system);
activateTokenIntegrationStrategy = new ActivateTokenIntegrationStrategy(policyConfig, system);
}

@Test
Expand Down Expand Up @@ -187,6 +189,48 @@ public void activateTokenIntegrationWithUnsupportedPlaceholder() {
UnresolvedPlaceholderException.newBuilder("{{request:subjectId}}").build());
}

@Test
public void activate2SubjectsIn2Entries() {
final CommandStrategy.Context<PolicyId> context = getDefaultContext();
final Instant expiry = Instant.now().plus(Duration.ofDays(1L));
final SubjectId subjectId = SubjectId.newInstance(SubjectIssuer.INTEGRATION, LABEL + ":this-is-me");
final SubjectId subjectId2 = SubjectId.newInstance(SubjectIssuer.INTEGRATION, LABEL + ":this-is-me-2");
final Set<SubjectId> subjectIds = Set.of(subjectId, subjectId2);
final DittoHeaders dittoHeaders = buildActivateTokenIntegrationHeaders();
final Label label2 = Label.of("label2");
final TopLevelPolicyActionCommand command = TopLevelPolicyActionCommand.of(
ActivateTokenIntegration.of(context.getState(), DUMMY_LABEL, subjectIds, expiry, dittoHeaders),
List.of(LABEL, label2)
);

final PolicyEntry entryToCopy = TestConstants.Policy.POLICY.getEntryFor(LABEL).orElseThrow();
final Policy policy = TestConstants.Policy.POLICY.toBuilder()
.forLabel(label2)
.setSubjects(entryToCopy.getSubjects())
.setResources(entryToCopy.getResources())
.build();

assertModificationResult(underTest, policy, command,
SubjectsModifiedPartially.class,
event -> {
assertThat(event.getModifiedSubjects()).containsOnlyKeys(LABEL, label2);
final Map<Label, Collection<Subject>> modifiedSubjects = event.getModifiedSubjects();
final Set<SubjectId> labelSubjectIds =
modifiedSubjects.get(LABEL).stream().map(Subject::getId).collect(Collectors.toSet());
final Set<SubjectId> label2SubjectIds =
modifiedSubjects.get(label2).stream().map(Subject::getId).collect(Collectors.toSet());
assertThat(labelSubjectIds)
.describedAs("Subject IDs in entry <{}>", LABEL)
.isEqualTo(subjectIds);
assertThat(label2SubjectIds)
.describedAs("Subject IDs in entry <{}>", label2)
.isEqualTo(subjectIds);
},
TopLevelPolicyActionCommandResponse.class,
response -> assertThat(response)
.isEqualTo(TopLevelPolicyActionCommandResponse.of(context.getState(), dittoHeaders)));
}

@Test
public void rejectEmptyLabels() {
final CommandStrategy.Context<PolicyId> context = getDefaultContext();
Expand Down
49 changes: 33 additions & 16 deletions ...es/src/main/java/org/eclipse/ditto/signals/events/policies/AbstractPolicyActionEvent.java
View file
Expand Up @@ -66,26 +66,17 @@ protected SubjectsModifiedPartially aggregateWithSubjectCreatedOrModified(
for (final PolicyActionEvent<?> event : otherEvents) {
if (event instanceof SubjectCreated) {
final SubjectCreated subjectCreated = (SubjectCreated) event;
final Collection<Subject> existingSubjects = modifiedSubjects.get(subjectCreated.getLabel());
final Set<Subject> mergedSubjects;
if (null == existingSubjects) {
mergedSubjects = new LinkedHashSet<>();
} else {
mergedSubjects = new LinkedHashSet<>(existingSubjects);
}
mergedSubjects.add(subjectCreated.getSubject());
final Set<Subject> mergedSubjects =
append(modifiedSubjects.get(subjectCreated.getLabel()), subjectCreated.getSubject());
modifiedSubjects.put(subjectCreated.getLabel(), mergedSubjects);
} else if (event instanceof SubjectModified) {
final SubjectModified subjectModified = (SubjectModified) event;
final Collection<Subject> existingSubjects = modifiedSubjects.get(subjectModified.getLabel());
final Set<Subject> mergedSubjects;
if (null == existingSubjects) {
mergedSubjects = new LinkedHashSet<>();
} else {
mergedSubjects = new LinkedHashSet<>(existingSubjects);
}
mergedSubjects.add(subjectModified.getSubject());
final Set<Subject> mergedSubjects =
append(modifiedSubjects.get(subjectModified.getLabel()), subjectModified.getSubject());
modifiedSubjects.put(subjectModified.getLabel(), mergedSubjects);
} else if (event instanceof SubjectsModifiedPartially) {
final SubjectsModifiedPartially subjectsModifiedPartially = (SubjectsModifiedPartially) event;
appendValues(modifiedSubjects, subjectsModifiedPartially.getModifiedSubjects());
}
}
return SubjectsModifiedPartially.of(getPolicyEntityId(), modifiedSubjects, getRevision(),
Expand Down Expand Up @@ -116,9 +107,35 @@ protected SubjectsDeletedPartially aggregateWithSubjectDeleted(
}
mergedSubjectIds.add(subjectDeleted.getSubjectId());
deletedSubjectIds.put(subjectDeleted.getLabel(), mergedSubjectIds);
} else if (event instanceof SubjectsDeletedPartially) {
final SubjectsDeletedPartially subjectsDeletedPartially = (SubjectsDeletedPartially) event;
appendValues(deletedSubjectIds, subjectsDeletedPartially.getDeletedSubjectIds());
}
}
return SubjectsDeletedPartially.of(getPolicyEntityId(), deletedSubjectIds, getRevision(),
getTimestamp().orElse(null), getDittoHeaders());
}

private static <T> LinkedHashSet<T> append(@Nullable final Collection<T> existingItems, final T newItem) {
final LinkedHashSet<T> mergedItems = new LinkedHashSet<>();
if (null != existingItems) {
mergedItems.addAll(existingItems);
}
mergedItems.add(newItem);
return mergedItems;
}

private static <K, V> void appendValues(final Map<K, Collection<V>> accumulator,
final Map<K, Collection<V>> toMerge) {

toMerge.forEach((key, valueToMerge) -> accumulator.compute(key, (k, value) -> {
if (value == null) {
return valueToMerge;
} else {
final LinkedHashSet<V> mergedValue = new LinkedHashSet<>(value);
mergedValue.addAll(valueToMerge);
return mergedValue;
}
}));
}
}

0 comments on commit a492eb4

Please sign in to comment.