Update rules engine authSchemes validation

This commit fixes several issues with validating authSchemes properties in defined endpoints within an endpoints rule set. It now properly enforces the typing and uniqueness of names of schemes defined within an authSchemes property. It also fixes several issues validating the presence and typing of properties configuring the sigv4, sigv4a, and "beta" schemes. A specification has been added to clearly detail how clients should choose schemes, detail what properties of the above schemes will be validated, and explain how to add new validators.
smithy-lang · Sep 26, 2023 · a5d3fb6 · a5d3fb6
1 parent e3c94bf
commit a5d3fb6
Show file tree

Hide file tree

Showing 18 changed files with 720 additions and 51 deletions.
diff --git a/docs/Makefile b/docs/Makefile
@@ -16,7 +16,7 @@ html1:
 html2:
 	@$(SPHINXBUILD) -M html "source-2.0" "build/2.0" $(SPHINXOPTS) -W --keep-going -n $(O)
 
-html: html1 html2 merge-versions
+html: html2 html1 merge-versions
 
 merge-versions:
 	mkdir -p build/html

diff --git a/docs/source-2.0/additional-specs/rules-engine/specification.rst b/docs/source-2.0/additional-specs/rules-engine/specification.rst
@@ -290,7 +290,7 @@ of rule conditions to that point.
 An endpoint MAY return a set of endpoint properties using the ``properties``
 field. This can be used to provide a grab-bag set of metadata associated with
 an endpoint that an endpoint resolver implementation MAY use. For example, the
-``authSchemes`` property is used to specify the priority listed order of
+``authSchemes`` property is used to specify the priority ordered list of
 authentication schemes and their configuration supported by the endpoint.
 Properties MAY contain arbitrary nested maps and arrays of strings and
 booleans.
@@ -299,6 +299,40 @@ booleans.
     To prevent ambiguity, the endpoint properties map MUST NOT contain
     reference or function objects. Properties MAY contain :ref:`template string <rules-engine-endpoint-rule-set-template-string>`
 
+.. _rules-engine-endpoint-rule-set-endpoint-authschemes:
+
+Endpoint ``authSchemes`` list property
+--------------------------------------
+
+The ``authSchemes`` property of an endpoint is used to specify the priority
+ordered list of authentication schemes and their configuration supported by the
+endpoint. The property is a list of configuration objects that MUST contain at
+least a ``name`` property and MAY contain additional properties. Each
+configuration object MUST have a unique value for its ``name`` property within
+the list of configuration objects within a given ``authSchemes`` property.
+
+If an ``authSchemes`` property is present on an `Endpoint object`_, clients
+MUST resolve an authentication scheme to use via the following process:
+
+#. Iterate through configuration objects in the ``authSchemes`` property.
+#. If the ``name`` property in a configuration object contains a supported
+   authentication scheme, resolve this scheme.
+#. If the ``name`` is unknown or unsupported, ignore it and continue iterating.
+#. If the list has been fully iterated and no scheme has been resolved, clients
+   MUST return an error.
+
+.. _rules-engine-standard-library-adding-authscheme-validators:
+
+Adding ``authSchemes`` configuration validators
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Extensions to the rules engine can provide additional validators for
+``authSchemes`` configuration objects. No validators are provided by default.
+
+The rules engine is highly extensible through
+``software.amazon.smithy.rulesengine.language.EndpointRuleSetExtension``
+`service providers`_. See the `Javadocs`_ for more information.
+
 
 .. _rules-engine-endpoint-rule-set-error-rule:
 
@@ -686,3 +720,6 @@ The following two expressions are equivalent:
             "{partResult#name}"
         ]
     }
+
+.. _Javadocs: https://smithy.io/javadoc/__smithy_version__/software/amazon/smithy/rulesengine/language/EndpointRuleSetExtension.html
+.. _service providers: https://docs.oracle.com/javase/tutorial/sound/SPI-intro.html
diff --git a/docs/source-2.0/aws/rules-engine/auth-schemes.rst b/docs/source-2.0/aws/rules-engine/auth-schemes.rst
@@ -0,0 +1,105 @@
+.. _rules-engine-aws-authscheme-validators:
+
+=================================================
+AWS rules engine authentication scheme validators
+=================================================
+
+AWS-specific rules engine library :ref:`authentication scheme validators <rules-engine-endpoint-rule-set-endpoint-authschemes>`
+make it possible to validate configurations for AWS authentication schemes like
+`AWS signature version 4`_. An additional dependency is required to access
+these validators
+
+The following example adds ``smithy-aws-endpoints`` as a Gradle dependency
+to a Smithy project:
+
+.. tab:: Gradle
+
+    .. code-block:: kotlin
+
+        dependencies {
+            ...
+            implementation("software.amazon.smithy:smithy-aws-endpoints:__smithy_version__")
+            ...
+        }
+
+.. tab:: smithy-build.json
+
+    .. code-block:: json
+
+        {
+            "maven": {
+                "dependencies": [
+                    "software.amazon.smithy:smithy-aws-endpoints:__smithy_version__"
+                ]
+            }
+        }
+
+.. _rules-engine-aws-authscheme-validator-sigv4:
+
+-----------------------------------------
+``sigv4`` authentication scheme validator
+-----------------------------------------
+
+Requirement
+    The ``name`` property is the string value ``sigv4``.
+Properties
+    .. list-table::
+        :header-rows: 1
+        :widths: 10 20 70
+
+        * - Property
+          - Type
+          - Description
+        * - ``signingName``
+          - ``option<string>``
+          - The "service" value to use when creating a signing string for this
+            endpoint.
+        * - ``signingRegion``
+          - ``option<string>``
+          - The "region" value to use when creating a signing string for this
+            endpoint.
+        * - ``disableDoubleEncoding``
+          - ``option<boolean>``
+          - Default: ``false``. When ``true``, clients MUST NOT double-escape
+            the path during signing.
+        * - ``disableNormalizePath``
+          - ``option<boolean>``
+          - Default: ``false``. When ``true``, clients MUST NOT perform any
+            path normalization during signing.
+
+
+.. _rules-engine-aws-authscheme-validator-sigv4a:
+
+------------------------------------------
+``sigv4a`` authentication scheme validator
+------------------------------------------
+
+Requirement
+    The ``name`` property is the string value ``sigv4a``.
+Properties
+    .. list-table::
+        :header-rows: 1
+        :widths: 10 20 70
+
+        * - Property
+          - Type
+          - Description
+        * - ``signingName``
+          - ``option<string>``
+          - The "service" value to use when creating a signing string for this
+            endpoint.
+        * - ``signingRegionSet``
+          - ``array<string>``
+          - The set of signing regions to use when creating a signing string
+            for this endpoint.
+        * - ``disableDoubleEncoding``
+          - ``option<boolean>``
+          - Default: ``false``. When ``true``, clients MUST NOT double-escape
+            the path during signing.
+        * - ``disableNormalizePath``
+          - ``option<boolean>``
+          - Default: ``false``. When ``true``, clients MUST NOT perform any
+            path normalization during signing.
+
+
+.. _AWS signature version 4: https://docs.aws.amazon.com/general/latest/gr/signature-version-4.html
diff --git a/docs/source-2.0/aws/rules-engine/index.rst b/docs/source-2.0/aws/rules-engine/index.rst
@@ -14,3 +14,4 @@ configuration options.
 
     built-ins
     library-functions
+    auth-schemes
diff --git a/...ain/java/software/amazon/smithy/rulesengine/aws/language/functions/EndpointAuthUtils.java b/...ain/java/software/amazon/smithy/rulesengine/aws/language/functions/EndpointAuthUtils.java
@@ -12,7 +12,6 @@
 import java.util.function.BiFunction;
 import java.util.function.Function;
 import software.amazon.smithy.model.FromSourceLocation;
-import software.amazon.smithy.model.SourceLocation;
 import software.amazon.smithy.model.validation.ValidationEvent;
 import software.amazon.smithy.rulesengine.language.Endpoint;
 import software.amazon.smithy.rulesengine.language.syntax.Identifier;
@@ -32,8 +31,11 @@ public final class EndpointAuthUtils {
     private static final String SIGNING_REGION = "signingRegion";
     private static final String SIGNING_REGION_SET = "signingRegionSet";
 
-    private static final Identifier DISABLE_DOUBLE_ENCODING = Identifier.of("disableDoubleEncoding");
-    private static final Identifier DISABLE_NORMALIZE_PATH = Identifier.of("disableNormalizePath");
+    private static final Identifier ID_SIGNING_NAME = Identifier.of("signingName");
+    private static final Identifier ID_SIGNING_REGION = Identifier.of("signingRegion");
+    private static final Identifier ID_SIGNING_REGION_SET = Identifier.of("signingRegionSet");
+    private static final Identifier ID_DISABLE_DOUBLE_ENCODING = Identifier.of("disableDoubleEncoding");
+    private static final Identifier ID_DISABLE_NORMALIZE_PATH = Identifier.of("disableNormalizePath");
 
     private EndpointAuthUtils() {}
 
@@ -80,18 +82,40 @@ public boolean test(String name) {
         @Override
         public List<ValidationEvent> validateScheme(
                 Map<Identifier, Literal> authScheme,
-                SourceLocation sourceLocation,
+                FromSourceLocation sourceLocation,
                 BiFunction<FromSourceLocation, String, ValidationEvent> emitter
         ) {
-            Identifier signingRegion = Identifier.of(SIGNING_REGION);
-            Identifier signingName = Identifier.of(SIGNING_NAME);
             List<ValidationEvent> events = noExtraProperties(emitter, sourceLocation, authScheme,
                     ListUtils.of(RuleSetAuthSchemesValidator.NAME,
-                            signingName, signingRegion, DISABLE_DOUBLE_ENCODING, DISABLE_NORMALIZE_PATH));
-            validatePropertyType(emitter, authScheme,
-                    signingName, Literal::asStringLiteral).ifPresent(events::add);
-            validatePropertyType(emitter, authScheme,
-                    signingRegion, Literal::asStringLiteral).ifPresent(events::add);
+                            ID_SIGNING_NAME,
+                            ID_SIGNING_REGION,
+                            ID_DISABLE_DOUBLE_ENCODING,
+                            ID_DISABLE_NORMALIZE_PATH));
+
+            // Validate shared Sigv4 properties.
+            events.addAll(SigV4SchemeValidator.validateOptionalSharedProperties(authScheme, emitter));
+            // Signing region is also optional.
+            if (authScheme.containsKey(ID_SIGNING_REGION)) {
+                validateStringProperty(emitter, authScheme, ID_SIGNING_REGION).ifPresent(events::add);
+            }
+            return events;
+        }
+
+        private static List<ValidationEvent> validateOptionalSharedProperties(
+                Map<Identifier, Literal> authScheme,
+                BiFunction<FromSourceLocation, String, ValidationEvent> emitter
+        ) {
+            List<ValidationEvent> events = new ArrayList<>();
+            // The following properties are only type checked if present.
+            if (authScheme.containsKey(ID_SIGNING_NAME)) {
+                validateStringProperty(emitter, authScheme, ID_SIGNING_NAME).ifPresent(events::add);
+            }
+            if (authScheme.containsKey(ID_DISABLE_DOUBLE_ENCODING)) {
+                validateBooleanProperty(emitter, authScheme, ID_DISABLE_DOUBLE_ENCODING).ifPresent(events::add);
+            }
+            if (authScheme.containsKey(ID_DISABLE_NORMALIZE_PATH)) {
+                validateBooleanProperty(emitter, authScheme, ID_DISABLE_NORMALIZE_PATH).ifPresent(events::add);
+            }
             return events;
         }
     }
@@ -107,18 +131,38 @@ public boolean test(String name) {
         @Override
         public List<ValidationEvent> validateScheme(
                 Map<Identifier, Literal> authScheme,
-                SourceLocation sourceLocation,
+                FromSourceLocation sourceLocation,
                 BiFunction<FromSourceLocation, String, ValidationEvent> emitter
         ) {
-            Identifier signingRegionSet = Identifier.of(SIGNING_REGION_SET);
-            Identifier signingName = Identifier.of(SIGNING_NAME);
             List<ValidationEvent> events = noExtraProperties(emitter, sourceLocation, authScheme,
                     ListUtils.of(RuleSetAuthSchemesValidator.NAME,
-                            signingName, signingRegionSet, DISABLE_DOUBLE_ENCODING, DISABLE_NORMALIZE_PATH));
-            validatePropertyType(emitter, authScheme,
-                    signingName, Literal::asStringLiteral).ifPresent(events::add);
-            validatePropertyType(emitter, authScheme,
-                    signingRegionSet, Literal::asTupleLiteral).ifPresent(events::add);
+                            ID_SIGNING_NAME,
+                            ID_SIGNING_REGION_SET,
+                            ID_DISABLE_DOUBLE_ENCODING,
+                            ID_DISABLE_NORMALIZE_PATH));
+
+            // The `signingRegionSet` property will always be present.
+            Optional<ValidationEvent> event = validatePropertyType(emitter, authScheme.get(ID_SIGNING_REGION_SET),
+                    ID_SIGNING_REGION_SET, Literal::asTupleLiteral, "an array<string>");
+            // If we don't have a tuple, that's our main error.
+            // Otherwise, validate each entry is a string.
+            if (event.isPresent()) {
+                events.add(event.get());
+            } else {
+                List<Literal> signingRegionSet = authScheme.get(ID_SIGNING_REGION_SET).asTupleLiteral().get();
+                if (signingRegionSet.isEmpty()) {
+                    emitter.apply(authScheme.get(ID_SIGNING_REGION_SET),
+                            "The `signingRegionSet` property MUST NOT be an empty list.");
+                } else {
+                    for (Literal signingRegion : signingRegionSet) {
+                        validatePropertyType(emitter, signingRegion, Identifier.of("signingRegionSet.Value"),
+                                Literal::asStringLiteral, "a string").ifPresent(events::add);
+                    }
+                }
+            }
+
+            // Validate shared Sigv4 properties.
+            events.addAll(SigV4SchemeValidator.validateOptionalSharedProperties(authScheme, emitter));
             return events;
         }
     }
@@ -134,21 +178,20 @@ public boolean test(String name) {
         @Override
         public List<ValidationEvent> validateScheme(
                 Map<Identifier, Literal> authScheme,
-                SourceLocation sourceLocation,
+                FromSourceLocation sourceLocation,
                 BiFunction<FromSourceLocation, String, ValidationEvent> emitter
         ) {
-            Identifier signingName = Identifier.of(SIGNING_NAME);
             List<ValidationEvent> events = hasAllKeys(emitter, authScheme,
-                    ListUtils.of(RuleSetAuthSchemesValidator.NAME, signingName), sourceLocation);
-            validatePropertyType(emitter, authScheme, signingName, Literal::asStringLiteral).ifPresent(events::add);
+                    ListUtils.of(RuleSetAuthSchemesValidator.NAME, ID_SIGNING_NAME), sourceLocation);
+            validateStringProperty(emitter, authScheme, ID_SIGNING_NAME).ifPresent(events::add);
             return events;
         }
 
         private List<ValidationEvent> hasAllKeys(
                 BiFunction<FromSourceLocation, String, ValidationEvent> emitter,
                 Map<Identifier, Literal> authScheme,
                 List<Identifier> requiredKeys,
-                SourceLocation sourceLocation
+                FromSourceLocation sourceLocation
         ) {
             List<ValidationEvent> events = new ArrayList<>();
             for (Identifier key : requiredKeys) {
@@ -162,7 +205,7 @@ private List<ValidationEvent> hasAllKeys(
 
     private static List<ValidationEvent> noExtraProperties(
             BiFunction<FromSourceLocation, String, ValidationEvent> emitter,
-            SourceLocation sourceLocation,
+            FromSourceLocation sourceLocation,
             Map<Identifier, Literal> properties,
             List<Identifier> allowedProperties
     ) {
@@ -176,21 +219,41 @@ private static List<ValidationEvent> noExtraProperties(
         return events;
     }
 
-    private static <U> Optional<ValidationEvent> validatePropertyType(
+    private static Optional<ValidationEvent> validateBooleanProperty(
             BiFunction<FromSourceLocation, String, ValidationEvent> emitter,
             Map<Identifier, Literal> properties,
+            Identifier propertyName
+    ) {
+        return validatePropertyType(emitter, properties.get(propertyName), propertyName,
+                Literal::asBooleanLiteral, "a boolean");
+    }
+
+    private static Optional<ValidationEvent> validateStringProperty(
+            BiFunction<FromSourceLocation, String, ValidationEvent> emitter,
+            Map<Identifier, Literal> properties,
+            Identifier propertyName
+    ) {
+        return validatePropertyType(emitter, properties.get(propertyName), propertyName,
+                Literal::asStringLiteral, "a string");
+    }
+
+    private static <U> Optional<ValidationEvent> validatePropertyType(
+            BiFunction<FromSourceLocation, String, ValidationEvent> emitter,
+            Literal value,
             Identifier propertyName,
-            Function<Literal, Optional<U>> validator
+            Function<Literal, Optional<U>> validator,
+            String expectedType
     ) {
-        Literal value = properties.get(propertyName);
         if (value == null) {
             return Optional.of(emitter.apply(propertyName,
-                    String.format("Expected auth property `%s` but didn't find one", propertyName)));
+                    String.format("Expected auth property `%s` of %s type but didn't find one",
+                            propertyName, expectedType)));
         }
 
         if (!validator.apply(value).isPresent()) {
             return Optional.of(emitter.apply(value,
-                    String.format("Unexpected type for auth property `%s`, found: `%s`", propertyName, value)));
+                    String.format("Unexpected type for auth property `%s`, found `%s` but expected %s value",
+                            propertyName, value, expectedType)));
         }
         return Optional.empty();
     }

diff --git a/...lesengine/aws/language/functions/errorfiles/invalid/beta-auth-scheme-missing-field.errors b/...lesengine/aws/language/functions/errorfiles/invalid/beta-auth-scheme-missing-field.errors
@@ -1,3 +1,3 @@
 [WARNING] example#FizzBuzz: This shape applies a trait that is unstable: smithy.rules#clientContextParams | UnstableTrait
 [WARNING] example#FizzBuzz: This shape applies a trait that is unstable: smithy.rules#endpointRuleSet | UnstableTrait
-[ERROR] example#FizzBuzz: Expected auth property `signingName` but didn't find one | RuleSetAuthSchemes
+[ERROR] example#FizzBuzz: Expected auth property `signingName` of a string type but didn't find one | RuleSetAuthSchemes
diff --git a/...ine/aws/language/functions/errorfiles/invalid/signing-optional-properties-mistyped.errors b/...ine/aws/language/functions/errorfiles/invalid/signing-optional-properties-mistyped.errors
@@ -0,0 +1,9 @@
+[WARNING] example#FizzBuzz: This shape applies a trait that is unstable: smithy.rules#endpointRuleSet | UnstableTrait
+[ERROR] example#FizzBuzz: Unexpected type for auth property `signingName`, found `1` but expected a string value | RuleSetAuthSchemes
+[ERROR] example#FizzBuzz: Unexpected type for auth property `signingRegion`, found `1` but expected a string value | RuleSetAuthSchemes
+[ERROR] example#FizzBuzz: Unexpected type for auth property `disableDoubleEncoding`, found `1` but expected a boolean value | RuleSetAuthSchemes
+[ERROR] example#FizzBuzz: Unexpected type for auth property `disableNormalizePath`, found `1` but expected a boolean value | RuleSetAuthSchemes
+[ERROR] example#FizzBuzz: Unexpected type for auth property `signingName`, found `1` but expected a string value | RuleSetAuthSchemes
+[ERROR] example#FizzBuzz: Unexpected type for auth property `signingRegionSet`, found `1` but expected an array<string> value | RuleSetAuthSchemes
+[ERROR] example#FizzBuzz: Unexpected type for auth property `disableDoubleEncoding`, found `1` but expected a boolean value | RuleSetAuthSchemes
+[ERROR] example#FizzBuzz: Unexpected type for auth property `disableNormalizePath`, found `1` but expected a boolean value | RuleSetAuthSchemes
Original file line number	Diff line number	Diff line change
Expand Up		@@ -14,3 +14,4 @@ configuration options.

		built-ins
		library-functions
		auth-schemes