From 1d2958e524bd5cd4fd3cce3be1d0d1fa18499bb0 Mon Sep 17 00:00:00 2001 From: Joseph Cosentino Date: Wed, 11 Sep 2024 09:52:36 -0700 Subject: [PATCH 1/5] chore: add integration test demonstrating mqtt wildcard behavior --- .../integrationtests/policy/PolicyTest.java | 13 +++++++++++++ .../policy/wildcards-in-resource-type.yaml | 8 +++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java b/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java index b9d811891..613732405 100644 --- a/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java +++ b/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java @@ -273,6 +273,19 @@ public static Stream authzRequests() { .operation("mqtt:publish") .resource("mqtt:topic:myThing/world") .expectedResult(false) + .build(), + // mqtt wildcards eval not supported + AuthZRequest.builder() + .thingName("myThing") + .operation("mqtt:subscribe") + .resource("mqtt:topic:myThing/test/test/*") + .expectedResult(false) + .build(), + AuthZRequest.builder() + .thingName("myThing") + .operation("mqtt:subscribe") + .resource("mqtt:topic:myThing/#/test/*") + .expectedResult(true) .build() )), diff --git a/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/wildcards-in-resource-type.yaml b/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/wildcards-in-resource-type.yaml index c52a1035e..4c262dac4 100644 --- a/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/wildcards-in-resource-type.yaml +++ b/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/wildcards-in-resource-type.yaml @@ -17,12 +17,18 @@ services: policyName: "thingAccessPolicy" policies: thingAccessPolicy: - policyStatement: + publish: statementDescription: "mqtt publish" operations: - "mqtt:publish" resources: - "mqtt:topic:*/myThing/*" + subscribe: + statementDescription: "mqtt subscribe" + operations: + - "mqtt:subscribe" + resources: + - "mqtt:topic:myThing/#/test/*" main: dependencies: - aws.greengrass.clientdevices.Auth From 6b156de2dfc049b8adcaf37b548bbcfe4dd30e43 Mon Sep 17 00:00:00 2001 From: Joseph Cosentino Date: Wed, 11 Sep 2024 11:01:55 -0700 Subject: [PATCH 2/5] feat: support mqtt wildcard resource matching --- .../integrationtests/policy/PolicyTest.java | 23 +++++++++++- .../policy/mqtt-wildcards-in-resource.yaml | 35 +++++++++++++++++++ .../auth/ClientDevicesAuthService.java | 2 ++ .../auth/PermissionEvaluationUtils.java | 28 +++++++++++++-- .../auth/configuration/CDAConfiguration.java | 32 +++++++++-------- 5 files changed, 102 insertions(+), 18 deletions(-) create mode 100644 src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/mqtt-wildcards-in-resource.yaml diff --git a/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java b/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java index 613732405..3cd45e61a 100644 --- a/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java +++ b/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java @@ -274,7 +274,7 @@ public static Stream authzRequests() { .resource("mqtt:topic:myThing/world") .expectedResult(false) .build(), - // mqtt wildcards eval not supported + // mqtt wildcards eval not supported by default AuthZRequest.builder() .thingName("myThing") .operation("mqtt:subscribe") @@ -289,6 +289,27 @@ public static Stream authzRequests() { .build() )), + Arguments.of("mqtt-wildcards-in-resource.yaml", Arrays.asList( + AuthZRequest.builder() + .thingName("myThing") + .operation("mqtt:publish") + .resource("mqtt:topic:*/myThing/*") + .expectedResult(true) + .build(), + AuthZRequest.builder() + .thingName("myThing") + .operation("mqtt:publish") + .resource("mqtt:topic:hello/myThing/world") + .expectedResult(true) + .build(), + AuthZRequest.builder() + .thingName("myThing") + .operation("mqtt:subscribe") + .resource("mqtt:topic:myThing/test/test/test/test") + .expectedResult(true) + .build() + )), + Arguments.of("variables-and-wildcards.yaml", Arrays.asList( AuthZRequest.builder() .thingName("myThing") diff --git a/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/mqtt-wildcards-in-resource.yaml b/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/mqtt-wildcards-in-resource.yaml new file mode 100644 index 000000000..4339eeb3a --- /dev/null +++ b/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/mqtt-wildcards-in-resource.yaml @@ -0,0 +1,35 @@ +--- +services: + aws.greengrass.Nucleus: + configuration: + runWithDefault: + posixUser: nobody + windowsUser: integ-tester + logging: + level: "DEBUG" + aws.greengrass.clientdevices.Auth: + configuration: + enableMqttWildcardEvaluation: true + deviceGroups: + formatVersion: "2021-03-05" + definitions: + myThing: + selectionRule: "thingName: myThing" + policyName: "thingAccessPolicy" + policies: + thingAccessPolicy: + publish: + statementDescription: "mqtt publish" + operations: + - "mqtt:publish" + resources: + - "mqtt:topic:*/myThing/*" + subscribe: + statementDescription: "mqtt subscribe" + operations: + - "mqtt:subscribe" + resources: + - "mqtt:topic:${iot:Connection.Thing.ThingName}/+/test/#" + main: + dependencies: + - aws.greengrass.clientdevices.Auth diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/ClientDevicesAuthService.java b/src/main/java/com/aws/greengrass/clientdevices/auth/ClientDevicesAuthService.java index 69b079002..b80e35558 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/ClientDevicesAuthService.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/ClientDevicesAuthService.java @@ -170,6 +170,8 @@ private void subscribeToConfigChanges() { private void onConfigurationChanged() { try { cdaConfiguration = CDAConfiguration.from(cdaConfiguration, getConfig()); + // TODO decouple + context.get(PermissionEvaluationUtils.class).setCdaConfiguration(cdaConfiguration); } catch (URISyntaxException e) { serviceErrored(e); } diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java b/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java index 624b10c13..f35db0a84 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java @@ -6,6 +6,7 @@ package com.aws.greengrass.clientdevices.auth; import com.aws.greengrass.authorization.WildcardTrie; +import com.aws.greengrass.clientdevices.auth.configuration.CDAConfiguration; import com.aws.greengrass.clientdevices.auth.configuration.GroupManager; import com.aws.greengrass.clientdevices.auth.configuration.Permission; import com.aws.greengrass.clientdevices.auth.exception.PolicyException; @@ -14,6 +15,7 @@ import com.aws.greengrass.logging.impl.LogManager; import com.aws.greengrass.util.Utils; import lombok.Builder; +import lombok.Setter; import lombok.Value; import org.apache.commons.lang3.StringUtils; @@ -36,6 +38,8 @@ public final class PermissionEvaluationUtils { "Resource is malformed, must be of the form: " + "([a-zA-Z]+):([a-zA-Z]+):" + RESOURCE_NAME_PATTERN.pattern(); private final GroupManager groupManager; + @Setter + private volatile CDAConfiguration cdaConfiguration; /** * Constructor for PermissionEvaluationUtils. @@ -134,9 +138,27 @@ private boolean compareResource(Resource requestResource, String policyResource) return true; } - WildcardTrie wildcardTrie = new WildcardTrie(); - wildcardTrie.add(policyResource); - return wildcardTrie.matchesStandard(requestResource.getResourceStr()); + if (matchMqttWildcards()) { + String name = extractResourceName(policyResource); + WildcardTrie trie = new WildcardTrie(); + trie.add(name); + return trie.matchesMQTT(requestResource.getResourceName()); + } else { + WildcardTrie trie = new WildcardTrie(); + trie.add(policyResource); + return trie.matchesStandard(requestResource.getResourceStr()); + } + } + + private String extractResourceName(String resource) { + // resource is considered valid at this point, so don't need to duplicate validation from parseResource + String typeAndName = resource.substring(resource.indexOf(':') + 1); + return typeAndName.substring(typeAndName.indexOf(':') + 1); + } + + private boolean matchMqttWildcards() { + CDAConfiguration config = cdaConfiguration; + return config != null && config.isEnableMqttWildcardEvaluation(); } private Operation parseOperation(String operationStr) throws PolicyException { diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/configuration/CDAConfiguration.java b/src/main/java/com/aws/greengrass/clientdevices/auth/configuration/CDAConfiguration.java index 3f3a27fd3..84c0af1a3 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/configuration/CDAConfiguration.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/configuration/CDAConfiguration.java @@ -10,6 +10,8 @@ import com.aws.greengrass.clientdevices.auth.configuration.events.MetricsConfigurationChanged; import com.aws.greengrass.clientdevices.auth.configuration.events.SecurityConfigurationChanged; import com.aws.greengrass.config.Topics; +import com.aws.greengrass.util.Coerce; +import lombok.Builder; import lombok.Getter; import java.net.URISyntaxException; @@ -48,23 +50,19 @@ * | *

*/ +@Builder public final class CDAConfiguration { + public static final String ENABLE_MQTT_WILDCARD_EVALUATION = "enableMqttWildcardEvaluation"; + + private final DomainEvents domainEvents; private final RuntimeConfiguration runtime; - private final SecurityConfiguration security; @Getter private final CAConfiguration certificateAuthorityConfiguration; + private final SecurityConfiguration security; private final MetricsConfiguration metricsConfiguration; - private final DomainEvents domainEvents; - - private CDAConfiguration(DomainEvents domainEvents, RuntimeConfiguration runtime, CAConfiguration ca, - SecurityConfiguration security, MetricsConfiguration metricsConfiguration) { - this.domainEvents = domainEvents; - this.runtime = runtime; - this.security = security; - this.certificateAuthorityConfiguration = ca; - this.metricsConfiguration = metricsConfiguration; - } + @Getter + private final boolean enableMqttWildcardEvaluation; /** * Creates the CDA (Client Device Auth) Service configuration. And allows it to be available in the context with the @@ -80,9 +78,15 @@ public static CDAConfiguration from(CDAConfiguration existingConfig, Topics topi DomainEvents domainEvents = topics.getContext().get(DomainEvents.class); - CDAConfiguration newConfig = new CDAConfiguration(domainEvents, RuntimeConfiguration.from(runtimeTopics), - CAConfiguration.from(serviceConfiguration), SecurityConfiguration.from(serviceConfiguration), - MetricsConfiguration.from(serviceConfiguration)); + CDAConfiguration newConfig = CDAConfiguration.builder() + .domainEvents(domainEvents) + .runtime(RuntimeConfiguration.from(runtimeTopics)) + .certificateAuthorityConfiguration(CAConfiguration.from(serviceConfiguration)) + .security(SecurityConfiguration.from(serviceConfiguration)) + .metricsConfiguration(MetricsConfiguration.from(serviceConfiguration)) + .enableMqttWildcardEvaluation( + Coerce.toBoolean(serviceConfiguration.find(ENABLE_MQTT_WILDCARD_EVALUATION))) + .build(); newConfig.triggerChanges(newConfig, existingConfig); From 48d07c7827dfe0d28508b7787fde3f56342900be Mon Sep 17 00:00:00 2001 From: Joseph Cosentino Date: Wed, 11 Sep 2024 16:27:29 -0700 Subject: [PATCH 3/5] feat: support single character wildcard resource matching --- .../integrationtests/policy/PolicyTest.java | 22 +-- ...ngle-character-wildcards-in-resource.yaml} | 10 +- .../policy/wildcards-in-resource-type.yaml | 2 +- .../auth/PermissionEvaluationUtils.java | 26 +-- .../auth/configuration/CDAConfiguration.java | 6 +- .../clientdevices/auth/util/WildcardTrie.java | 177 ++++++++++++++++++ .../auth/util/WildcardTrieTest.java | 77 ++++++++ 7 files changed, 274 insertions(+), 46 deletions(-) rename src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/{mqtt-wildcards-in-resource.yaml => single-character-wildcards-in-resource.yaml} (75%) create mode 100644 src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java create mode 100644 src/test/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrieTest.java diff --git a/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java b/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java index 3cd45e61a..4332a3cde 100644 --- a/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java +++ b/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java @@ -274,39 +274,33 @@ public static Stream authzRequests() { .resource("mqtt:topic:myThing/world") .expectedResult(false) .build(), - // mqtt wildcards eval not supported by default + // single character eval not supported by default AuthZRequest.builder() .thingName("myThing") .operation("mqtt:subscribe") - .resource("mqtt:topic:myThing/test/test/*") + .resource("mqtt:topic:myThing/#/test/abc") .expectedResult(false) .build(), AuthZRequest.builder() .thingName("myThing") .operation("mqtt:subscribe") - .resource("mqtt:topic:myThing/#/test/*") + .resource("mqtt:topic:myThing/#/test/???") .expectedResult(true) .build() )), - Arguments.of("mqtt-wildcards-in-resource.yaml", Arrays.asList( + Arguments.of("single-character-wildcards-in-resource.yaml", Arrays.asList( AuthZRequest.builder() .thingName("myThing") - .operation("mqtt:publish") - .resource("mqtt:topic:*/myThing/*") - .expectedResult(true) - .build(), - AuthZRequest.builder() - .thingName("myThing") - .operation("mqtt:publish") - .resource("mqtt:topic:hello/myThing/world") + .operation("mqtt:subscribe") + .resource("mqtt:topic:myThing/abc/test/a/b") .expectedResult(true) .build(), AuthZRequest.builder() .thingName("myThing") .operation("mqtt:subscribe") - .resource("mqtt:topic:myThing/test/test/test/test") - .expectedResult(true) + .resource("mqtt:topic:myThing/abcd/test/a/b") + .expectedResult(false) .build() )), diff --git a/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/mqtt-wildcards-in-resource.yaml b/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/single-character-wildcards-in-resource.yaml similarity index 75% rename from src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/mqtt-wildcards-in-resource.yaml rename to src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/single-character-wildcards-in-resource.yaml index 4339eeb3a..bcc31ac1e 100644 --- a/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/mqtt-wildcards-in-resource.yaml +++ b/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/single-character-wildcards-in-resource.yaml @@ -9,7 +9,7 @@ services: level: "DEBUG" aws.greengrass.clientdevices.Auth: configuration: - enableMqttWildcardEvaluation: true + enableSingleCharacterWildcardMatching: true deviceGroups: formatVersion: "2021-03-05" definitions: @@ -18,18 +18,12 @@ services: policyName: "thingAccessPolicy" policies: thingAccessPolicy: - publish: - statementDescription: "mqtt publish" - operations: - - "mqtt:publish" - resources: - - "mqtt:topic:*/myThing/*" subscribe: statementDescription: "mqtt subscribe" operations: - "mqtt:subscribe" resources: - - "mqtt:topic:${iot:Connection.Thing.ThingName}/+/test/#" + - "mqtt:topic:${iot:Connection.Thing.ThingName}/???/test/*" main: dependencies: - aws.greengrass.clientdevices.Auth diff --git a/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/wildcards-in-resource-type.yaml b/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/wildcards-in-resource-type.yaml index 4c262dac4..f7981161a 100644 --- a/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/wildcards-in-resource-type.yaml +++ b/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/wildcards-in-resource-type.yaml @@ -28,7 +28,7 @@ services: operations: - "mqtt:subscribe" resources: - - "mqtt:topic:myThing/#/test/*" + - "mqtt:topic:myThing/#/test/???" main: dependencies: - aws.greengrass.clientdevices.Auth diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java b/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java index f35db0a84..be32f6661 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java @@ -5,12 +5,12 @@ package com.aws.greengrass.clientdevices.auth; -import com.aws.greengrass.authorization.WildcardTrie; import com.aws.greengrass.clientdevices.auth.configuration.CDAConfiguration; import com.aws.greengrass.clientdevices.auth.configuration.GroupManager; import com.aws.greengrass.clientdevices.auth.configuration.Permission; import com.aws.greengrass.clientdevices.auth.exception.PolicyException; import com.aws.greengrass.clientdevices.auth.session.Session; +import com.aws.greengrass.clientdevices.auth.util.WildcardTrie; import com.aws.greengrass.logging.api.Logger; import com.aws.greengrass.logging.impl.LogManager; import com.aws.greengrass.util.Utils; @@ -137,28 +137,14 @@ private boolean compareResource(Resource requestResource, String policyResource) if (Objects.equals(requestResource.getResourceStr(), policyResource)) { return true; } - - if (matchMqttWildcards()) { - String name = extractResourceName(policyResource); - WildcardTrie trie = new WildcardTrie(); - trie.add(name); - return trie.matchesMQTT(requestResource.getResourceName()); - } else { - WildcardTrie trie = new WildcardTrie(); - trie.add(policyResource); - return trie.matchesStandard(requestResource.getResourceStr()); - } - } - - private String extractResourceName(String resource) { - // resource is considered valid at this point, so don't need to duplicate validation from parseResource - String typeAndName = resource.substring(resource.indexOf(':') + 1); - return typeAndName.substring(typeAndName.indexOf(':') + 1); + WildcardTrie trie = new WildcardTrie(); + trie.add(policyResource); + return trie.matches(requestResource.getResourceStr(), matchSingleCharacterWildcard()); } - private boolean matchMqttWildcards() { + private boolean matchSingleCharacterWildcard() { CDAConfiguration config = cdaConfiguration; - return config != null && config.isEnableMqttWildcardEvaluation(); + return config != null && config.isMatchSingleCharacterWildcard(); } private Operation parseOperation(String operationStr) throws PolicyException { diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/configuration/CDAConfiguration.java b/src/main/java/com/aws/greengrass/clientdevices/auth/configuration/CDAConfiguration.java index 84c0af1a3..69610d27f 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/configuration/CDAConfiguration.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/configuration/CDAConfiguration.java @@ -53,7 +53,7 @@ @Builder public final class CDAConfiguration { - public static final String ENABLE_MQTT_WILDCARD_EVALUATION = "enableMqttWildcardEvaluation"; + public static final String ENABLE_MQTT_WILDCARD_EVALUATION = "enableSingleCharacterWildcardMatching"; private final DomainEvents domainEvents; private final RuntimeConfiguration runtime; @@ -62,7 +62,7 @@ public final class CDAConfiguration { private final SecurityConfiguration security; private final MetricsConfiguration metricsConfiguration; @Getter - private final boolean enableMqttWildcardEvaluation; + private final boolean matchSingleCharacterWildcard; /** * Creates the CDA (Client Device Auth) Service configuration. And allows it to be available in the context with the @@ -84,7 +84,7 @@ public static CDAConfiguration from(CDAConfiguration existingConfig, Topics topi .certificateAuthorityConfiguration(CAConfiguration.from(serviceConfiguration)) .security(SecurityConfiguration.from(serviceConfiguration)) .metricsConfiguration(MetricsConfiguration.from(serviceConfiguration)) - .enableMqttWildcardEvaluation( + .matchSingleCharacterWildcard( Coerce.toBoolean(serviceConfiguration.find(ENABLE_MQTT_WILDCARD_EVALUATION))) .build(); diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java b/src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java new file mode 100644 index 000000000..663250a1f --- /dev/null +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java @@ -0,0 +1,177 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.aws.greengrass.clientdevices.auth.util; + + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + +import java.util.HashMap; +import java.util.Map; +import java.util.function.Supplier; + +public class WildcardTrie { + private static final String GLOB_WILDCARD = "*"; + private static final String SINGLE_CHAR_WILDCARD = "?"; + + private final Map children = new DefaultHashMap<>(WildcardTrie::new); + + private boolean isTerminal; + private boolean isGlobWildcard; + private boolean isSingleCharWildcard; + + public void add(String subject) { + add(subject, true); + } + + private WildcardTrie add(String subject, boolean isTerminal) { + if (subject == null || subject.isEmpty()) { + this.isTerminal |= isTerminal; + return this; + } + StringBuilder currPrefix = new StringBuilder(subject.length()); + for (int i = 0; i < subject.length(); i++) { + char c = subject.charAt(i); + if (c == GLOB_WILDCARD.charAt(0)) { + return addGlobWildcard(subject, currPrefix.toString(), isTerminal); + } + if (c == SINGLE_CHAR_WILDCARD.charAt(0)) { + return addSingleCharWildcard(subject, currPrefix.toString(), isTerminal); + } + currPrefix.append(c); + } + WildcardTrie node = children.get(currPrefix.toString()); + node.isTerminal |= isTerminal; + return node; + } + + private WildcardTrie addGlobWildcard(String subject, String currPrefix, boolean isTerminal) { + WildcardTrie node = this; + node = node.add(currPrefix, false); + node = node.children.get(GLOB_WILDCARD); + node.isGlobWildcard = true; + // wildcard at end of subject is terminal + if (subject.length() - currPrefix.length() == 1) { + node.isTerminal = isTerminal; + return node; + } + return node.add(subject.substring(currPrefix.length() + 2), true); + } + + private WildcardTrie addSingleCharWildcard(String subject, String currPrefix, boolean isTerminal) { + WildcardTrie node = this; + node = node.add(currPrefix, false); + node = node.children.get(SINGLE_CHAR_WILDCARD); + node.isSingleCharWildcard = true; + // wildcard at end of subject is terminal + if (subject.length() - currPrefix.length() == 1) { + node.isTerminal = isTerminal; + return node; + } + return node.add(subject.substring(currPrefix.length() + 1), true); + } + + public boolean matches(String s) { + return matches(s, true); + } + + public boolean matches(String s, boolean matchSingleCharWildcard) { + if (s == null) { + return children.isEmpty(); + } + + if ((isWildcard() && isTerminal) || (isTerminal && s.isEmpty())) { + return true; + } + + boolean childMatchesWildcard = children + .values() + .stream() + .filter(WildcardTrie::isWildcard) + .filter(childNode -> matchSingleCharWildcard || !childNode.isSingleCharWildcard) + .anyMatch(childNode -> childNode.matches(s, matchSingleCharWildcard)); + if (childMatchesWildcard) { + return true; + } + + if (matchSingleCharWildcard) { + boolean childMatchesSingleCharWildcard = children + .values() + .stream() + .filter(childNode -> childNode.isSingleCharWildcard) + .anyMatch(childNode -> childNode.matches(s, matchSingleCharWildcard)); + if (childMatchesSingleCharWildcard) { + return true; + } + } + + boolean childMatchesRegularCharacters = children + .keySet() + .stream() + .filter(s::startsWith) + .anyMatch(childToken -> { + WildcardTrie childNode = children.get(childToken); + String rest = s.substring(childToken.length()); + return childNode.matches(rest, matchSingleCharWildcard); + }); + if (childMatchesRegularCharacters) { + return true; + } + + if (isWildcard() && !isTerminal) { + return findMatchingChildSuffixesAfterWildcard(s, matchSingleCharWildcard) + .entrySet() + .stream() + .anyMatch((e) -> { + String suffix = e.getKey(); + WildcardTrie childNode = e.getValue(); + return childNode.matches(suffix, matchSingleCharWildcard); + }); + } + return false; + } + + private Map findMatchingChildSuffixesAfterWildcard(String s, boolean matchSingleCharWildcard) { + Map matchingSuffixes = new HashMap<>(); + for (Map.Entry e : children.entrySet()) { + String childToken = e.getKey(); + WildcardTrie childNode = e.getValue(); + int suffixIndex = s.indexOf(childToken); + if (matchSingleCharWildcard && suffixIndex > 1) { + continue; + } + while (suffixIndex >= 0) { + matchingSuffixes.put(s.substring(suffixIndex + childToken.length()), childNode); + suffixIndex = s.indexOf(childToken, suffixIndex + 1); + } + } + return matchingSuffixes; + } + + private boolean isWildcard() { + return isGlobWildcard || isSingleCharWildcard; + } + + @SuppressFBWarnings("EQ_DOESNT_OVERRIDE_EQUALS") + private static class DefaultHashMap extends HashMap { + private final transient Supplier defaultVal; + + public DefaultHashMap(Supplier defaultVal) { + super(); + this.defaultVal = defaultVal; + } + + @Override + @SuppressWarnings("unchecked") + public V get(Object key) { + return super.computeIfAbsent((K) key, (k) -> defaultVal.get()); + } + + @Override + public boolean containsKey(Object key) { + return super.get(key) != null; + } + } +} diff --git a/src/test/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrieTest.java b/src/test/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrieTest.java new file mode 100644 index 000000000..bd0b639f4 --- /dev/null +++ b/src/test/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrieTest.java @@ -0,0 +1,77 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.aws.greengrass.clientdevices.auth.util; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.List; +import java.util.stream.Stream; + +import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.params.provider.Arguments.arguments; + +// TODO fix failing cases +class WildcardTrieTest { + + static Stream validMatches() { + return Stream.of( + arguments("foo", singletonList("foo")), + arguments("foo/bar", singletonList("foo/bar")), + // glob wildcard * + arguments("*", asList("foo", "foo/bar", "foo/bar/baz", "$foo/bar", "foo*", "foo?", "***", "???")), + arguments("*test", asList("test", "*test", "**test", "testtest", "test*test")), + arguments("test*", asList("test", "test*", "testA", "test**")), + arguments("test*test", asList("testtest", "testAtest", "test*test")), + arguments("test*test*", asList("testtest", "testtesttesttest", "test*test*")), + arguments("*test*test", asList("Atesttest", "testAtest", "AtestAtest", "testtest", "*test*test")), + arguments("*test*test*", asList("testtest", "*test*test*", "Atesttest", "testAtest", "testtestA", "AtestAtest", "testAtestA", "AtestAtestA")), + // single character wildcard ? + arguments("?", asList("f", "*", "?")), + arguments("??", asList("ff", "**", "??", "*?")), + arguments("?f?", asList("fff", "f", "ff", "*f", "*f*", "f*")) + ); + } + + @MethodSource("validMatches") + @ParameterizedTest + void GIVEN_trie_with_wildcards_WHEN_valid_matches_provided_THEN_pass(String pattern, List matches) { + WildcardTrie trie = new WildcardTrie(); + trie.add(pattern); + matches.forEach(m -> assertTrue(trie.matches(m))); + } + + + static Stream invalidMatches() { + return Stream.of( + arguments("foo", singletonList("bar")), + arguments("foo/bar", singletonList("bar/foo")), + // glob wildcard * + arguments("*test", asList("testA", "*testA", "AtestA", "bar")), + arguments("test*", asList("Atest", "Atest*", "AtestA", "Atest**", "bar")), + arguments("test*test", asList("Atesttest", "AtestAtest", "testAtestA", "testbar")), + arguments("test*test*", asList("Atesttest", "AtestAtest", "AtestAtestA", "testbar")), + arguments("*test*test", asList("AtestAtestA", "test*bar", "bartest", "testbar")), + arguments("*test*test*", asList("AtestAbar", "testbar", "bartest")), + // single character wildcard ? + arguments("?", asList("aa", "??", "**")), + arguments("??", asList("aaa", "???", "***")), + arguments("?a?", asList("aaaa", "fff")) + ); + } + + @MethodSource("invalidMatches") + @ParameterizedTest + void GIVEN_trie_with_wildcards_WHEN_invalid_matches_provided_THEN_fail(String pattern, List matches) { + WildcardTrie trie = new WildcardTrie(); + trie.add(pattern); + matches.forEach(m -> assertFalse(trie.matches(m))); + } +} From 9bab92b5bfb3f5c2f664970a71662e11cc735f46 Mon Sep 17 00:00:00 2001 From: Joseph Cosentino Date: Fri, 13 Sep 2024 12:09:01 -0700 Subject: [PATCH 4/5] chore: refactor impl for readability, still has bugs --- .../auth/PermissionEvaluationUtils.java | 12 +- .../clientdevices/auth/util/WildcardTrie.java | 254 ++++++++++-------- .../auth/util/WildcardTrieTest.java | 8 +- 3 files changed, 154 insertions(+), 120 deletions(-) diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java b/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java index be32f6661..8c68d929c 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java @@ -137,14 +137,16 @@ private boolean compareResource(Resource requestResource, String policyResource) if (Objects.equals(requestResource.getResourceStr(), policyResource)) { return true; } - WildcardTrie trie = new WildcardTrie(); - trie.add(policyResource); - return trie.matches(requestResource.getResourceStr(), matchSingleCharacterWildcard()); + return new WildcardTrie(wildcardOpts()) + .withPattern(policyResource) + .matches(requestResource.getResourceStr()); } - private boolean matchSingleCharacterWildcard() { + private WildcardTrie.MatchOptions wildcardOpts() { CDAConfiguration config = cdaConfiguration; - return config != null && config.isMatchSingleCharacterWildcard(); + return WildcardTrie.MatchOptions.builder() + .useSingleCharWildcard(config != null && config.isMatchSingleCharacterWildcard()) + .build(); } private Operation parseOperation(String operationStr) throws PolicyException { diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java b/src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java index 663250a1f..da3ae031f 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java @@ -7,151 +7,183 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import lombok.Builder; +import lombok.NonNull; +import lombok.RequiredArgsConstructor; +import lombok.Value; +import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.function.Supplier; +@RequiredArgsConstructor public class WildcardTrie { - private static final String GLOB_WILDCARD = "*"; - private static final String SINGLE_CHAR_WILDCARD = "?"; - private final Map children = new DefaultHashMap<>(WildcardTrie::new); + private Node root; - private boolean isTerminal; - private boolean isGlobWildcard; - private boolean isSingleCharWildcard; + private final MatchOptions opts; - public void add(String subject) { - add(subject, true); + private static String cleanPattern(@NonNull String s) { + // for example "abc***def" can be reduced to "abc*def" + return s.replaceAll(String.format("\\%s+", WildcardType.GLOB.val), WildcardType.GLOB.val); } - private WildcardTrie add(String subject, boolean isTerminal) { - if (subject == null || subject.isEmpty()) { - this.isTerminal |= isTerminal; - return this; - } - StringBuilder currPrefix = new StringBuilder(subject.length()); - for (int i = 0; i < subject.length(); i++) { - char c = subject.charAt(i); - if (c == GLOB_WILDCARD.charAt(0)) { - return addGlobWildcard(subject, currPrefix.toString(), isTerminal); - } - if (c == SINGLE_CHAR_WILDCARD.charAt(0)) { - return addSingleCharWildcard(subject, currPrefix.toString(), isTerminal); + public WildcardTrie withPattern(@NonNull String s) { + root = new Node(); + withPattern(root, cleanPattern(s)); + return this; + } + + private Node withPattern(@NonNull Node n, @NonNull String s) { + StringBuilder token = new StringBuilder(s.length()); + for (int i = 0; i < s.length(); i++) { + char c = s.charAt(i); + if (isWildcard(s.charAt(i))) { + WildcardType type = WildcardType.from(c); + Node node = n.children.get(type.val()); + node.wildcardType = type; + if (i == s.length() - 1) { + // we've reached the last token + return node; + } + return withPattern(node, s.substring(token.length() + 2)); + } else { + token.append(c); } - currPrefix.append(c); } - WildcardTrie node = children.get(currPrefix.toString()); - node.isTerminal |= isTerminal; - return node; + // use remaining (non-wildcard) chars as last token + if (token.length() > 0) { + return n.children.get(token.toString()); + } else { + return n; + } } - private WildcardTrie addGlobWildcard(String subject, String currPrefix, boolean isTerminal) { - WildcardTrie node = this; - node = node.add(currPrefix, false); - node = node.children.get(GLOB_WILDCARD); - node.isGlobWildcard = true; - // wildcard at end of subject is terminal - if (subject.length() - currPrefix.length() == 1) { - node.isTerminal = isTerminal; - return node; - } - return node.add(subject.substring(currPrefix.length() + 2), true); + private boolean isWildcard(char c) { + WildcardType type = WildcardType.from(c); + if (type == null) { + return false; + } + if (type == WildcardType.SINGLE) { + return opts.useSingleCharWildcard; + } + return true; } - private WildcardTrie addSingleCharWildcard(String subject, String currPrefix, boolean isTerminal) { - WildcardTrie node = this; - node = node.add(currPrefix, false); - node = node.children.get(SINGLE_CHAR_WILDCARD); - node.isSingleCharWildcard = true; - // wildcard at end of subject is terminal - if (subject.length() - currPrefix.length() == 1) { - node.isTerminal = isTerminal; - return node; - } - return node.add(subject.substring(currPrefix.length() + 1), true); + public boolean matches(@NonNull String s) { + if (root == null) { + return s.isEmpty(); + } + return matches(root, s); } - public boolean matches(String s) { - return matches(s, true); + private boolean matches(@NonNull Node n, String s) { + if (n.isTerminal()) { + if (n.isWildcard()) { + switch (n.wildcardType) { + case SINGLE: + return s.length() == 1; + case GLOB: + return true; + default: + throw new UnsupportedOperationException("wildcard type " + n.wildcardType.name() + " not supported"); + } + } else { + return s.isEmpty(); + } + } + + for (String token : n.children.keySet()) { + Node child = n.children.get(token); + + if (n.isWildcard()) { // parent is a wildcard + switch (n.wildcardType) { + case SINGLE: + // skip over one character for single wildcard + return matches(child, s.substring(1)); + case GLOB: + // consume the input string to find a match + return allIndicesOf(s, token).stream() + .anyMatch(tokenIndex -> + matches(child, s.substring(tokenIndex + token.length())) + ); + default: + throw new UnsupportedOperationException("wildcard type " + n.wildcardType.name() + " not supported"); + } + } + + if (child.isWildcard()) { + // skip past the wildcard node, + // on the next iteration we need to figure out + // the part the wildcard matched (if at all). + return matches(child, s); + } else { + // match found, keep following this trie branch + if (s.startsWith(token)) { + return matches(child, s.substring(token.length())); + } + } + } + + return false; } - public boolean matches(String s, boolean matchSingleCharWildcard) { - if (s == null) { - return children.isEmpty(); + private static List allIndicesOf(@NonNull String s, @NonNull String sub) { + List indices = new ArrayList<>(); + int i = s.indexOf(sub); + while (i >= 0) { + indices.add(i); + i = s.indexOf(sub, i + sub.length()); } + return indices; + } + + @Value + @Builder + public static class MatchOptions { + boolean useSingleCharWildcard; + } + + enum WildcardType { + GLOB("*"), + SINGLE("?"); - if ((isWildcard() && isTerminal) || (isTerminal && s.isEmpty())) { - return true; + private final String val; + + WildcardType(@NonNull String val) { + this.val = val; } - boolean childMatchesWildcard = children - .values() - .stream() - .filter(WildcardTrie::isWildcard) - .filter(childNode -> matchSingleCharWildcard || !childNode.isSingleCharWildcard) - .anyMatch(childNode -> childNode.matches(s, matchSingleCharWildcard)); - if (childMatchesWildcard) { - return true; + public static WildcardType from(char c) { + return Arrays.stream(WildcardType.values()) + .filter(t -> t.charVal() == c) + .findFirst() + .orElse(null); } - if (matchSingleCharWildcard) { - boolean childMatchesSingleCharWildcard = children - .values() - .stream() - .filter(childNode -> childNode.isSingleCharWildcard) - .anyMatch(childNode -> childNode.matches(s, matchSingleCharWildcard)); - if (childMatchesSingleCharWildcard) { - return true; - } + public String val() { + return val; } - boolean childMatchesRegularCharacters = children - .keySet() - .stream() - .filter(s::startsWith) - .anyMatch(childToken -> { - WildcardTrie childNode = children.get(childToken); - String rest = s.substring(childToken.length()); - return childNode.matches(rest, matchSingleCharWildcard); - }); - if (childMatchesRegularCharacters) { - return true; - } - - if (isWildcard() && !isTerminal) { - return findMatchingChildSuffixesAfterWildcard(s, matchSingleCharWildcard) - .entrySet() - .stream() - .anyMatch((e) -> { - String suffix = e.getKey(); - WildcardTrie childNode = e.getValue(); - return childNode.matches(suffix, matchSingleCharWildcard); - }); + public char charVal() { + return val.charAt(0); } - return false; } - private Map findMatchingChildSuffixesAfterWildcard(String s, boolean matchSingleCharWildcard) { - Map matchingSuffixes = new HashMap<>(); - for (Map.Entry e : children.entrySet()) { - String childToken = e.getKey(); - WildcardTrie childNode = e.getValue(); - int suffixIndex = s.indexOf(childToken); - if (matchSingleCharWildcard && suffixIndex > 1) { - continue; - } - while (suffixIndex >= 0) { - matchingSuffixes.put(s.substring(suffixIndex + childToken.length()), childNode); - suffixIndex = s.indexOf(childToken, suffixIndex + 1); - } + private class Node { + private final Map children = new DefaultHashMap<>(Node::new); + private WildcardType wildcardType; + + public boolean isWildcard() { + return wildcardType != null; } - return matchingSuffixes; - } - private boolean isWildcard() { - return isGlobWildcard || isSingleCharWildcard; + public boolean isTerminal() { + return children.isEmpty(); + } } @SuppressFBWarnings("EQ_DOESNT_OVERRIDE_EQUALS") diff --git a/src/test/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrieTest.java b/src/test/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrieTest.java index bd0b639f4..d195d8f95 100644 --- a/src/test/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrieTest.java +++ b/src/test/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrieTest.java @@ -43,8 +43,8 @@ static Stream validMatches() { @MethodSource("validMatches") @ParameterizedTest void GIVEN_trie_with_wildcards_WHEN_valid_matches_provided_THEN_pass(String pattern, List matches) { - WildcardTrie trie = new WildcardTrie(); - trie.add(pattern); + WildcardTrie.MatchOptions opts = WildcardTrie.MatchOptions.builder().useSingleCharWildcard(true).build(); + WildcardTrie trie = new WildcardTrie(opts).withPattern(pattern); matches.forEach(m -> assertTrue(trie.matches(m))); } @@ -70,8 +70,8 @@ static Stream invalidMatches() { @MethodSource("invalidMatches") @ParameterizedTest void GIVEN_trie_with_wildcards_WHEN_invalid_matches_provided_THEN_fail(String pattern, List matches) { - WildcardTrie trie = new WildcardTrie(); - trie.add(pattern); + WildcardTrie.MatchOptions opts = WildcardTrie.MatchOptions.builder().useSingleCharWildcard(true).build(); + WildcardTrie trie = new WildcardTrie(opts).withPattern(pattern); matches.forEach(m -> assertFalse(trie.matches(m))); } } From c1f6d9f9c8ef7d316576522b91b750dec363c4f6 Mon Sep 17 00:00:00 2001 From: Joseph Cosentino Date: Wed, 18 Sep 2024 12:30:34 -0700 Subject: [PATCH 5/5] chore: passed existing tests, add more cases for combinations --- .../clientdevices/auth/util/WildcardTrie.java | 89 +++++++++++-------- .../auth/util/WildcardTrieTest.java | 24 +++-- 2 files changed, 69 insertions(+), 44 deletions(-) diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java b/src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java index da3ae031f..d4446d95b 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java @@ -17,11 +17,15 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.Function; import java.util.function.Supplier; @RequiredArgsConstructor public class WildcardTrie { + private static final Function EXCEPTION_UNSUPPORTED_WILDCARD_TYPE = wildcardType -> + new UnsupportedOperationException("wildcard type " + wildcardType.name() + " not supported"); + private Node root; private final MatchOptions opts; @@ -42,14 +46,17 @@ private Node withPattern(@NonNull Node n, @NonNull String s) { for (int i = 0; i < s.length(); i++) { char c = s.charAt(i); if (isWildcard(s.charAt(i))) { + // create child node from non-wildcard chars that have been accumulated so far + Node node = token.length() > 0 ? n.children.get(token.toString()) : n; + // create child node for wildcard char itself WildcardType type = WildcardType.from(c); - Node node = n.children.get(type.val()); + node = node.children.get(type.val()); node.wildcardType = type; if (i == s.length() - 1) { // we've reached the last token return node; } - return withPattern(node, s.substring(token.length() + 2)); + return withPattern(node, s.substring(i + 1)); } else { token.append(c); } @@ -80,55 +87,59 @@ public boolean matches(@NonNull String s) { return matches(root, s); } - private boolean matches(@NonNull Node n, String s) { + private boolean matches(@NonNull Node n, @NonNull String s) { if (n.isTerminal()) { - if (n.isWildcard()) { - switch (n.wildcardType) { - case SINGLE: - return s.length() == 1; - case GLOB: - return true; - default: - throw new UnsupportedOperationException("wildcard type " + n.wildcardType.name() + " not supported"); - } - } else { - return s.isEmpty(); + return n.wildcardType == WildcardType.GLOB || s.isEmpty(); + } + + if (n.isWildcard()) { + switch (n.wildcardType) { + case SINGLE: + return n.children.keySet().stream().anyMatch(token -> { + Node child = n.children.get(token); + // skip over one character for single wildcard + if (child.isWildcard()) { + return !s.isEmpty() && matches(child, s.substring(1)); + } else { + return !s.isEmpty() && s.startsWith(token.substring(0, 1)) && matches(child, s.substring(1)); + } + }); + case GLOB: + return n.children.keySet().stream().anyMatch(token -> { + Node child = n.children.get(token); + if (child.isWildcard()) { + return true;// TODO + } else { + // consume the input string to find a match + return allIndicesOf(s, token).stream() + .anyMatch(tokenIndex -> + matches(child, s.substring(tokenIndex + token.length())) + ); + } + }); + default: + throw EXCEPTION_UNSUPPORTED_WILDCARD_TYPE.apply(n.wildcardType); } } - for (String token : n.children.keySet()) { + return n.children.keySet().stream().anyMatch(token -> { Node child = n.children.get(token); - - if (n.isWildcard()) { // parent is a wildcard - switch (n.wildcardType) { + if (child.isWildcard()) { + switch (child.wildcardType) { case SINGLE: - // skip over one character for single wildcard - return matches(child, s.substring(1)); + // skip past the next character for ? matching + return !s.isEmpty() && matches(child, s.substring(1)); case GLOB: - // consume the input string to find a match - return allIndicesOf(s, token).stream() - .anyMatch(tokenIndex -> - matches(child, s.substring(tokenIndex + token.length())) - ); + // skip past token and figure out retroactively if the glob matched + return matches(child, s); default: - throw new UnsupportedOperationException("wildcard type " + n.wildcardType.name() + " not supported"); + throw EXCEPTION_UNSUPPORTED_WILDCARD_TYPE.apply(child.wildcardType); } - } - - if (child.isWildcard()) { - // skip past the wildcard node, - // on the next iteration we need to figure out - // the part the wildcard matched (if at all). - return matches(child, s); } else { // match found, keep following this trie branch - if (s.startsWith(token)) { - return matches(child, s.substring(token.length())); - } + return s.startsWith(token) && matches(child, s.substring(token.length())); } - } - - return false; + }); } private static List allIndicesOf(@NonNull String s, @NonNull String sub) { diff --git a/src/test/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrieTest.java b/src/test/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrieTest.java index d195d8f95..a4ddf1f76 100644 --- a/src/test/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrieTest.java +++ b/src/test/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrieTest.java @@ -18,7 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.params.provider.Arguments.arguments; -// TODO fix failing cases + class WildcardTrieTest { static Stream validMatches() { @@ -36,7 +36,14 @@ static Stream validMatches() { // single character wildcard ? arguments("?", asList("f", "*", "?")), arguments("??", asList("ff", "**", "??", "*?")), - arguments("?f?", asList("fff", "f", "ff", "*f", "*f*", "f*")) + arguments("?f?", asList("fff", "*f*")), + // glob and single + arguments("?*", asList("?", "*", "a", "??", "**", "ab", "***", "???", "abc")), + arguments("*?", asList("?", "*", "a", "??", "**", "ab", "***", "???", "abc")), + arguments("?*?", asList("??", "**", "aa", "???", "***", "aaa", "????", "****", "aaaa")), + arguments("*?*", asList("?", "*", "a", "??", "**", "ab", "***", "???", "abc")), + arguments("a?*b", asList("acb", "a?b", "a*b", "a?cb")), + arguments("a*?b", asList("acb", "a?b", "a*b", "a?cb")) ); } @@ -45,7 +52,8 @@ static Stream validMatches() { void GIVEN_trie_with_wildcards_WHEN_valid_matches_provided_THEN_pass(String pattern, List matches) { WildcardTrie.MatchOptions opts = WildcardTrie.MatchOptions.builder().useSingleCharWildcard(true).build(); WildcardTrie trie = new WildcardTrie(opts).withPattern(pattern); - matches.forEach(m -> assertTrue(trie.matches(m))); + matches.forEach(m -> assertTrue(trie.matches(m), + String.format("String \"%s\" did not match the pattern \"%s\"", m, pattern))); } @@ -63,7 +71,12 @@ static Stream invalidMatches() { // single character wildcard ? arguments("?", asList("aa", "??", "**")), arguments("??", asList("aaa", "???", "***")), - arguments("?a?", asList("aaaa", "fff")) + arguments("?a?", asList("fff", "aaaa")), + arguments("?f?", asList("ff", "f", "*f", "f*")), + // glob and single + arguments("?*?", asList("a", "?", "*")), + arguments("a?*b", asList("ab", "abc")), + arguments("a*?b", asList("ab", "abc")) ); } @@ -72,6 +85,7 @@ static Stream invalidMatches() { void GIVEN_trie_with_wildcards_WHEN_invalid_matches_provided_THEN_fail(String pattern, List matches) { WildcardTrie.MatchOptions opts = WildcardTrie.MatchOptions.builder().useSingleCharWildcard(true).build(); WildcardTrie trie = new WildcardTrie(opts).withPattern(pattern); - matches.forEach(m -> assertFalse(trie.matches(m))); + matches.forEach(m -> assertFalse(trie.matches(m), + String.format("String \"%s\" incorrectly matched the pattern \"%s\"", m, pattern))); } }