From a5d3fb6d0c6368fb2945890d33e2cad912ea8c7d Mon Sep 17 00:00:00 2001 From: kstich Date: Tue, 26 Sep 2023 00:03:35 -0700 Subject: [PATCH] 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. --- docs/Makefile | 2 +- .../rules-engine/specification.rst | 39 +++++- .../aws/rules-engine/auth-schemes.rst | 105 +++++++++++++++ docs/source-2.0/aws/rules-engine/index.rst | 1 + .../language/functions/EndpointAuthUtils.java | 123 ++++++++++++----- .../beta-auth-scheme-missing-field.errors | 2 +- ...igning-optional-properties-mistyped.errors | 9 ++ ...igning-optional-properties-mistyped.smithy | 125 ++++++++++++++++++ .../valid/signing-optional-properties.errors | 1 + .../valid/signing-optional-properties.smithy | 84 ++++++++++++ .../validators/AuthSchemeValidator.java | 5 +- .../RuleSetAuthSchemesValidator.java | 62 ++++++--- .../invalid/invalid-auth-name-type.errors | 3 + .../invalid/invalid-auth-name-type.smithy | 67 ++++++++++ .../invalid-duplicate-auth-name.errors | 5 + .../invalid-duplicate-auth-name.smithy | 70 ++++++++++ .../invalid/invalid-missing-auth-name.errors | 3 + .../invalid/invalid-missing-auth-name.smithy | 65 +++++++++ 18 files changed, 720 insertions(+), 51 deletions(-) create mode 100644 docs/source-2.0/aws/rules-engine/auth-schemes.rst create mode 100644 smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/invalid/signing-optional-properties-mistyped.errors create mode 100644 smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/invalid/signing-optional-properties-mistyped.smithy create mode 100644 smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/valid/signing-optional-properties.errors create mode 100644 smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/valid/signing-optional-properties.smithy create mode 100644 smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-auth-name-type.errors create mode 100644 smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-auth-name-type.smithy create mode 100644 smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-duplicate-auth-name.errors create mode 100644 smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-duplicate-auth-name.smithy create mode 100644 smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-missing-auth-name.errors create mode 100644 smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-missing-auth-name.smithy diff --git a/docs/Makefile b/docs/Makefile index 39e53b7b03a..b0a870ca9be 100644 --- 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 index 7b7ed5dd20d..148170f6619 100644 --- 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-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 new file mode 100644 index 00000000000..2ca4e2270ed --- /dev/null +++ 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 ` +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`` + - The "service" value to use when creating a signing string for this + endpoint. + * - ``signingRegion`` + - ``option`` + - The "region" value to use when creating a signing string for this + endpoint. + * - ``disableDoubleEncoding`` + - ``option`` + - Default: ``false``. When ``true``, clients MUST NOT double-escape + the path during signing. + * - ``disableNormalizePath`` + - ``option`` + - 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`` + - The "service" value to use when creating a signing string for this + endpoint. + * - ``signingRegionSet`` + - ``array`` + - The set of signing regions to use when creating a signing string + for this endpoint. + * - ``disableDoubleEncoding`` + - ``option`` + - Default: ``false``. When ``true``, clients MUST NOT double-escape + the path during signing. + * - ``disableNormalizePath`` + - ``option`` + - 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 index d02a75319a5..88259732693 100644 --- 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/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/language/functions/EndpointAuthUtils.java b/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/language/functions/EndpointAuthUtils.java index 6fccdfb0d63..35d8f7f600f 100644 --- a/smithy-aws-endpoints/src/main/java/software/amazon/smithy/rulesengine/aws/language/functions/EndpointAuthUtils.java +++ b/smithy-aws-endpoints/src/main/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 validateScheme( Map authScheme, - SourceLocation sourceLocation, + FromSourceLocation sourceLocation, BiFunction emitter ) { - Identifier signingRegion = Identifier.of(SIGNING_REGION); - Identifier signingName = Identifier.of(SIGNING_NAME); List 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 validateOptionalSharedProperties( + Map authScheme, + BiFunction emitter + ) { + List 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 validateScheme( Map authScheme, - SourceLocation sourceLocation, + FromSourceLocation sourceLocation, BiFunction emitter ) { - Identifier signingRegionSet = Identifier.of(SIGNING_REGION_SET); - Identifier signingName = Identifier.of(SIGNING_NAME); List 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 event = validatePropertyType(emitter, authScheme.get(ID_SIGNING_REGION_SET), + ID_SIGNING_REGION_SET, Literal::asTupleLiteral, "an array"); + // 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 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,13 +178,12 @@ public boolean test(String name) { @Override public List validateScheme( Map authScheme, - SourceLocation sourceLocation, + FromSourceLocation sourceLocation, BiFunction emitter ) { - Identifier signingName = Identifier.of(SIGNING_NAME); List 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; } @@ -148,7 +191,7 @@ private List hasAllKeys( BiFunction emitter, Map authScheme, List requiredKeys, - SourceLocation sourceLocation + FromSourceLocation sourceLocation ) { List events = new ArrayList<>(); for (Identifier key : requiredKeys) { @@ -162,7 +205,7 @@ private List hasAllKeys( private static List noExtraProperties( BiFunction emitter, - SourceLocation sourceLocation, + FromSourceLocation sourceLocation, Map properties, List allowedProperties ) { @@ -176,21 +219,41 @@ private static List noExtraProperties( return events; } - private static Optional validatePropertyType( + private static Optional validateBooleanProperty( BiFunction emitter, Map properties, + Identifier propertyName + ) { + return validatePropertyType(emitter, properties.get(propertyName), propertyName, + Literal::asBooleanLiteral, "a boolean"); + } + + private static Optional validateStringProperty( + BiFunction emitter, + Map properties, + Identifier propertyName + ) { + return validatePropertyType(emitter, properties.get(propertyName), propertyName, + Literal::asStringLiteral, "a string"); + } + + private static Optional validatePropertyType( + BiFunction emitter, + Literal value, Identifier propertyName, - Function> validator + Function> 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/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/invalid/beta-auth-scheme-missing-field.errors b/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/invalid/beta-auth-scheme-missing-field.errors index 4e76db1d98d..08da32f1e72 100644 --- a/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/invalid/beta-auth-scheme-missing-field.errors +++ b/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/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/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/invalid/signing-optional-properties-mistyped.errors b/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/invalid/signing-optional-properties-mistyped.errors new file mode 100644 index 00000000000..b22157895fa --- /dev/null +++ b/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/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 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 diff --git a/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/invalid/signing-optional-properties-mistyped.smithy b/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/invalid/signing-optional-properties-mistyped.smithy new file mode 100644 index 00000000000..4018e5ffca5 --- /dev/null +++ b/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/invalid/signing-optional-properties-mistyped.smithy @@ -0,0 +1,125 @@ +$version: "2.0" + +namespace example + +use smithy.rules#endpointRuleSet +use smithy.rules#endpointTests + +@endpointRuleSet({ + "parameters": { + "Region": { + "type": "string", + "builtIn": "AWS::Region", + "documentation": "The region to dispatch this request, eg. `us-east-1`." + } + }, + "rules": [ + { + "documentation": "Show failing type validation for optional fields", + "conditions": [ + { + "fn": "isSet", + "argv": [ + { + "ref": "Region" + } + ] + }, + { + "fn": "stringEquals", + "argv": [ + { + "ref": "Region" + }, + "a" + ] + } + ], + "endpoint": { + "url": "https://{Region}.amazonaws.com", + "properties": { + "authSchemes": [ + { + "name": "sigv4", + "signingName": 1, + "signingRegion": 1, + "disableDoubleEncoding": 1, + "disableNormalizePath": 1 + } + ] + } + }, + "type": "endpoint" + }, + { + "documentation": "Show failing type validation for optional fields", + "conditions": [ + { + "fn": "isSet", + "argv": [ + { + "ref": "Region" + } + ] + }, + { + "fn": "stringEquals", + "argv": [ + { + "ref": "Region" + }, + "b" + ] + } + ], + "endpoint": { + "url": "https://{Region}.amazonaws.com", + "properties": { + "authSchemes": [ + { + "name": "sigv4a", + "signingName": 1, + "signingRegionSet": 1, + "disableDoubleEncoding": 1, + "disableNormalizePath": 1 + } + ] + } + }, + "type": "endpoint" + }, + { + "documentation": "Show non-empty requirement of signingRegionSet", + "conditions": [ + { + "fn": "isSet", + "argv": [ + { + "ref": "Region" + } + ] + }, + ], + "endpoint": { + "url": "https://{Region}.amazonaws.com", + "properties": { + "authSchemes": [ + { + "name": "sigv4a", + "signingRegionSet": [] + } + ] + } + }, + "type": "endpoint" + }, + { + "documentation": "fallback when region is unset", + "conditions": [], + "error": "Region must be set to resolve a valid endpoint", + "type": "error" + } + ], + "version": "1.3" +}) +service FizzBuzz {} diff --git a/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/valid/signing-optional-properties.errors b/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/valid/signing-optional-properties.errors new file mode 100644 index 00000000000..a0b08ef4605 --- /dev/null +++ b/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/valid/signing-optional-properties.errors @@ -0,0 +1 @@ +[WARNING] example#FizzBuzz: This shape applies a trait that is unstable: smithy.rules#endpointRuleSet | UnstableTrait diff --git a/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/valid/signing-optional-properties.smithy b/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/valid/signing-optional-properties.smithy new file mode 100644 index 00000000000..ff545c633a3 --- /dev/null +++ b/smithy-aws-endpoints/src/test/resources/software/amazon/smithy/rulesengine/aws/language/functions/errorfiles/valid/signing-optional-properties.smithy @@ -0,0 +1,84 @@ +$version: "2.0" + +namespace example + +use smithy.rules#endpointRuleSet +use smithy.rules#endpointTests + +@endpointRuleSet({ + "parameters": { + "Region": { + "type": "string", + "builtIn": "AWS::Region", + "documentation": "The region to dispatch this request, eg. `us-east-1`." + } + }, + "rules": [ + { + "documentation": "Template the region into the URI when region is set", + "conditions": [ + { + "fn": "isSet", + "argv": [ + { + "ref": "Region" + } + ] + }, + { + "fn": "stringEquals", + "argv": [ + { + "ref": "Region" + }, + "a" + ] + } + ], + "endpoint": { + "url": "https://{Region}.amazonaws.com", + "properties": { + "authSchemes": [ + { + "name": "sigv4", + } + ] + } + }, + "type": "endpoint" + }, + { + "documentation": "Template the region into the URI when region is set", + "conditions": [ + { + "fn": "isSet", + "argv": [ + { + "ref": "Region" + } + ] + }, + ], + "endpoint": { + "url": "https://{Region}.amazonaws.com", + "properties": { + "authSchemes": [ + { + "name": "sigv4a", + "signingRegionSet": ["*"] + } + ] + } + }, + "type": "endpoint" + }, + { + "documentation": "fallback when region is unset", + "conditions": [], + "error": "Region must be set to resolve a valid endpoint", + "type": "error" + } + ], + "version": "1.3" +}) +service FizzBuzz {} diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/validators/AuthSchemeValidator.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/validators/AuthSchemeValidator.java index 78fbfc3d180..3255298481c 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/validators/AuthSchemeValidator.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/validators/AuthSchemeValidator.java @@ -10,14 +10,15 @@ import java.util.function.BiFunction; import java.util.function.Predicate; 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.syntax.Identifier; import software.amazon.smithy.rulesengine.language.syntax.expressions.literal.Literal; +import software.amazon.smithy.utils.SmithyUnstableApi; /** * Validates an authentication scheme after passing a predicate check. */ +@SmithyUnstableApi public interface AuthSchemeValidator extends Predicate { /** * Validates that the provided {@code authScheme} matches required modeling behavior, @@ -30,6 +31,6 @@ public interface AuthSchemeValidator extends Predicate { */ List validateScheme( Map authScheme, - SourceLocation sourceLocation, + FromSourceLocation sourceLocation, BiFunction emitter); } diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/validators/RuleSetAuthSchemesValidator.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/validators/RuleSetAuthSchemesValidator.java index 8da5768bab6..0a83812b4d8 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/validators/RuleSetAuthSchemesValidator.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/validators/RuleSetAuthSchemesValidator.java @@ -6,15 +6,16 @@ package software.amazon.smithy.rulesengine.validators; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.function.BiFunction; import java.util.stream.Collectors; import java.util.stream.Stream; import software.amazon.smithy.model.FromSourceLocation; import software.amazon.smithy.model.Model; -import software.amazon.smithy.model.SourceLocation; import software.amazon.smithy.model.shapes.ServiceShape; import software.amazon.smithy.model.validation.AbstractValidator; import software.amazon.smithy.model.validation.ValidationEvent; @@ -60,37 +61,65 @@ public Stream visitEndpoint(Endpoint endpoint) { BiFunction emitter = getEventEmitter(); Optional> authSchemeList = authSchemes.asTupleLiteral(); if (!authSchemeList.isPresent()) { - return Stream.of(emitter.apply(authSchemes.getSourceLocation(), + return Stream.of(emitter.apply(authSchemes, String.format("Expected `authSchemes` to be a list, found: `%s`", authSchemes))); } - for (Literal authScheme : authSchemeList.get()) { - Optional> authSchemeMap = authScheme.asRecordLiteral(); + + Set authSchemeNames = new HashSet<>(); + Set duplicateAuthSchemeNames = new HashSet<>(); + for (Literal authSchemeEntry : authSchemeList.get()) { + Optional> authSchemeMap = authSchemeEntry.asRecordLiteral(); if (authSchemeMap.isPresent()) { - events.addAll(validateAuthScheme(authSchemeMap.get(), authScheme.getSourceLocation())); + // Validate the name property so that we can also check that they're unique. + Map authScheme = authSchemeMap.get(); + Optional event = validateAuthSchemeName(authScheme, authSchemeEntry); + if (event.isPresent()) { + events.add(event.get()); + continue; + } + String schemeName = authScheme.get(NAME).asStringLiteral().get().expectLiteral(); + if (!authSchemeNames.add(schemeName)) { + duplicateAuthSchemeNames.add(schemeName); + } + + events.addAll(validateAuthScheme(schemeName, authScheme, authSchemeEntry)); } else { - events.add(emitter.apply(authSchemes.getSourceLocation(), + events.add(emitter.apply(authSchemes, String.format("Expected `authSchemes` to be a list of objects, but found: `%s`", - authScheme))); + authSchemeEntry))); } } + + // Emit events for each duplicated auth scheme name. + for (String duplicateAuthSchemeName : duplicateAuthSchemeNames) { + events.add(emitter.apply(authSchemes, String.format("Found duplicate `name` of `%s` in the " + + "`authSchemes` list", duplicateAuthSchemeName))); + } } return events.stream(); } - private List validateAuthScheme( + private Optional validateAuthSchemeName( Map authScheme, - SourceLocation sourceLocation + FromSourceLocation sourceLocation ) { - BiFunction emitter = getEventEmitter(); - if (!authScheme.containsKey(NAME)) { - return ListUtils.of(emitter.apply(sourceLocation, - String.format("Expected `authSchemes` to have a `name` key but it did not: `%s`", authScheme))); + if (!authScheme.containsKey(NAME) || !authScheme.get(NAME).asStringLiteral().isPresent()) { + return Optional.of(error(serviceShape, sourceLocation, + String.format("Expected `authSchemes` to have a `name` key with a string value but it did not: " + + "`%s`", authScheme))); } + return Optional.empty(); + } + private List validateAuthScheme( + String schemeName, + Map authScheme, + FromSourceLocation sourceLocation + ) { List events = new ArrayList<>(); - Literal name = authScheme.get(NAME); - String schemeName = name.asStringLiteral().get().expectLiteral(); + + BiFunction emitter = getEventEmitter(); boolean validatedAuth = false; for (AuthSchemeValidator authSchemeValidator : EndpointRuleSet.getAuthSchemeValidators()) { @@ -103,7 +132,8 @@ private List validateAuthScheme( if (validatedAuth) { return events; } - return ListUtils.of(emitter.apply(name, String.format("Unexpected auth scheme: `%s`", schemeName))); + return ListUtils.of(warning(serviceShape, String.format("Did not find a validator for the `%s` " + + "auth scheme", schemeName))); } private BiFunction getEventEmitter() { diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-auth-name-type.errors b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-auth-name-type.errors new file mode 100644 index 00000000000..a28f443a2d7 --- /dev/null +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-auth-name-type.errors @@ -0,0 +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 `authSchemes` to have a `name` key with a string value but it did not: `{name=true}` | RuleSetAuthSchemes diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-auth-name-type.smithy b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-auth-name-type.smithy new file mode 100644 index 00000000000..f8d19824a59 --- /dev/null +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-auth-name-type.smithy @@ -0,0 +1,67 @@ +$version: "2.0" + +namespace example + +use smithy.rules#clientContextParams +use smithy.rules#endpointRuleSet +use smithy.rules#endpointTests + +@clientContextParams( + bar: {type: "string", documentation: "a client string parameter"} +) +@endpointRuleSet({ + version: "1.0", + parameters: { + bar: { + type: "string", + documentation: "docs" + } + endpoint: { + type: "string", + builtIn: "SDK::Endpoint", + required: true, + default: "asdf" + documentation: "docs" + }, + }, + rules: [ + { + "documentation": "Shows invalid authScheme name type", + "conditions": [ + { + "fn": "isSet", + "argv": [ + { + "ref": "bar" + } + ] + } + ], + "endpoint": { + "url": "https://example.com/", + "properties": { + "authSchemes": [ + { + "name": true, + } + ] + } + }, + "type": "endpoint" + }, + { + "conditions": [], + "documentation": "error fallthrough", + "error": "endpoint error", + "type": "error" + } + ] +}) +service FizzBuzz { + version: "2022-01-01", + operations: [GetThing] +} + +operation GetThing { + input := {} +} diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-duplicate-auth-name.errors b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-duplicate-auth-name.errors new file mode 100644 index 00000000000..c1933e56694 --- /dev/null +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-duplicate-auth-name.errors @@ -0,0 +1,5 @@ +[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 +[WARNING] example#FizzBuzz: Did not find a validator for the `duplicate` auth scheme | RuleSetAuthSchemes +[WARNING] example#FizzBuzz: Did not find a validator for the `duplicate` auth scheme | RuleSetAuthSchemes +[ERROR] example#FizzBuzz: Found duplicate `name` of `duplicate` in the `authSchemes` list | RuleSetAuthSchemes diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-duplicate-auth-name.smithy b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-duplicate-auth-name.smithy new file mode 100644 index 00000000000..66949434687 --- /dev/null +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-duplicate-auth-name.smithy @@ -0,0 +1,70 @@ +$version: "2.0" + +namespace example + +use smithy.rules#clientContextParams +use smithy.rules#endpointRuleSet +use smithy.rules#endpointTests + +@clientContextParams( + bar: {type: "string", documentation: "a client string parameter"} +) +@endpointRuleSet({ + version: "1.0", + parameters: { + bar: { + type: "string", + documentation: "docs" + } + endpoint: { + type: "string", + builtIn: "SDK::Endpoint", + required: true, + default: "asdf" + documentation: "docs" + }, + }, + rules: [ + { + "documentation": "Shows invalid duplicate authScheme names", + "conditions": [ + { + "fn": "isSet", + "argv": [ + { + "ref": "bar" + } + ] + } + ], + "endpoint": { + "url": "https://example.com/", + "properties": { + "authSchemes": [ + { + "name": "duplicate", + }, + { + "name": "duplicate", + } + ] + } + }, + "type": "endpoint" + }, + { + "conditions": [], + "documentation": "error fallthrough", + "error": "endpoint error", + "type": "error" + } + ] +}) +service FizzBuzz { + version: "2022-01-01", + operations: [GetThing] +} + +operation GetThing { + input := {} +} diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-missing-auth-name.errors b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-missing-auth-name.errors new file mode 100644 index 00000000000..77d7a6784e7 --- /dev/null +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-missing-auth-name.errors @@ -0,0 +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 `authSchemes` to have a `name` key with a string value but it did not: `{}` | RuleSetAuthSchemes diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-missing-auth-name.smithy b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-missing-auth-name.smithy new file mode 100644 index 00000000000..dd643a2e4ab --- /dev/null +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/language/errorfiles/invalid/invalid-missing-auth-name.smithy @@ -0,0 +1,65 @@ +$version: "2.0" + +namespace example + +use smithy.rules#clientContextParams +use smithy.rules#endpointRuleSet +use smithy.rules#endpointTests + +@clientContextParams( + bar: {type: "string", documentation: "a client string parameter"} +) +@endpointRuleSet({ + version: "1.0", + parameters: { + bar: { + type: "string", + documentation: "docs" + } + endpoint: { + type: "string", + builtIn: "SDK::Endpoint", + required: true, + default: "asdf" + documentation: "docs" + }, + }, + rules: [ + { + "documentation": "Shows invalid auth without a signing name", + "conditions": [ + { + "fn": "isSet", + "argv": [ + { + "ref": "bar" + } + ] + } + ], + "endpoint": { + "url": "https://example.com/", + "properties": { + "authSchemes": [ + {} + ] + } + }, + "type": "endpoint" + }, + { + "conditions": [], + "documentation": "error fallthrough", + "error": "endpoint error", + "type": "error" + } + ] +}) +service FizzBuzz { + version: "2022-01-01", + operations: [GetThing] +} + +operation GetThing { + input := {} +}