From 695679a891693f66cb060a42792d80777be1edab Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Mon, 25 Aug 2025 12:42:41 -0700 Subject: [PATCH 01/12] Add optionalAuth check --- gems/smithy-client/lib/smithy-client/plugins/resolve_auth.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gems/smithy-client/lib/smithy-client/plugins/resolve_auth.rb b/gems/smithy-client/lib/smithy-client/plugins/resolve_auth.rb index e25c80f72..bd43856a0 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/resolve_auth.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/resolve_auth.rb @@ -38,7 +38,7 @@ def resolve_auth(context, auth_options) auth_options.each do |auth_option| # Anonymous auth does not have a plugin and does not sign, # so if auth scheme is noAuth then just return scheme_id. - return { scheme_id: auth_option } if auth_option == 'smithy.api#noAuth' + return { scheme_id: auth_option } if %w[smithy.api#noAuth smithy.api#optionalAuth].include?(auth_option) unless context.config.auth_schemes.key?(auth_option) failures << "Auth scheme #{auth_option} was not enabled for this request" From ead9e6129f8c1d12b915a435842b36f02fac8b55 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Wed, 3 Sep 2025 10:10:58 -0700 Subject: [PATCH 02/12] http bearer -> token --- .../lib/smithy-client/http_bearer_provider.rb | 4 ++-- .../identities/{http_bearer.rb => token.rb} | 10 ++++++++-- .../lib/smithy-client/plugins/http_bearer_auth.rb | 2 +- .../identities/{http_bearer.rbs => token.rbs} | 3 ++- .../spec/smithy-client/plugins/resolve_auth_spec.rb | 2 +- 5 files changed, 14 insertions(+), 7 deletions(-) rename gems/smithy-client/lib/smithy-client/identities/{http_bearer.rb => token.rb} (51%) rename gems/smithy-client/sig/smithy-client/identities/{http_bearer.rbs => token.rbs} (74%) diff --git a/gems/smithy-client/lib/smithy-client/http_bearer_provider.rb b/gems/smithy-client/lib/smithy-client/http_bearer_provider.rb index 4a7d298e8..0a79cd621 100644 --- a/gems/smithy-client/lib/smithy-client/http_bearer_provider.rb +++ b/gems/smithy-client/lib/smithy-client/http_bearer_provider.rb @@ -2,13 +2,13 @@ module Smithy module Client - # Returns an HTTP Bearer identity + # Returns a Token identity class HttpBearerProvider include IdentityProvider # @param [String] token def initialize(token) - @identity = Identities::HttpBearer.new(token: token) + @identity = Identities::Token.new(token: token) end end end diff --git a/gems/smithy-client/lib/smithy-client/identities/http_bearer.rb b/gems/smithy-client/lib/smithy-client/identities/token.rb similarity index 51% rename from gems/smithy-client/lib/smithy-client/identities/http_bearer.rb rename to gems/smithy-client/lib/smithy-client/identities/token.rb index 93ac9cfc8..406ef7b42 100644 --- a/gems/smithy-client/lib/smithy-client/identities/http_bearer.rb +++ b/gems/smithy-client/lib/smithy-client/identities/token.rb @@ -3,8 +3,8 @@ module Smithy module Client module Identities - # Identity class for HTTP Bearer token authentication. - class HttpBearer < Identity + # Identity class for token authentication. + class Token < Identity def initialize(token:, **) @token = token super(**) @@ -12,6 +12,12 @@ def initialize(token:, **) # @return [String, nil] attr_reader :token + + # Removing the token from the default inspect string. + # @api private + def inspect + "#<#{self.class.name} token=[FILTERED]>" + end end end end diff --git a/gems/smithy-client/lib/smithy-client/plugins/http_bearer_auth.rb b/gems/smithy-client/lib/smithy-client/plugins/http_bearer_auth.rb index 5d19f54a2..45b352735 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/http_bearer_auth.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/http_bearer_auth.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require_relative '../http_bearer_provider' -require_relative '../identities/http_bearer' +require_relative '../identities/token' module Smithy module Client diff --git a/gems/smithy-client/sig/smithy-client/identities/http_bearer.rbs b/gems/smithy-client/sig/smithy-client/identities/token.rbs similarity index 74% rename from gems/smithy-client/sig/smithy-client/identities/http_bearer.rbs rename to gems/smithy-client/sig/smithy-client/identities/token.rbs index 64ff48d08..ef7249e90 100644 --- a/gems/smithy-client/sig/smithy-client/identities/http_bearer.rbs +++ b/gems/smithy-client/sig/smithy-client/identities/token.rbs @@ -1,9 +1,10 @@ module Smithy module Client module Identities - class HttpBearer < Identity + class Token < Identity def initialize: (token: String, **untyped options) -> void attr_reader token: String + def inspect: () -> String end end end diff --git a/gems/smithy-client/spec/smithy-client/plugins/resolve_auth_spec.rb b/gems/smithy-client/spec/smithy-client/plugins/resolve_auth_spec.rb index c7817c007..4a5d67d5d 100644 --- a/gems/smithy-client/spec/smithy-client/plugins/resolve_auth_spec.rb +++ b/gems/smithy-client/spec/smithy-client/plugins/resolve_auth_spec.rb @@ -52,7 +52,7 @@ module Plugins client_class.add_plugin(HttpBearerAuth) resp = client.operation expect(resp.context.auth[:scheme_id]).to equal('smithy.api#httpBearerAuth') - expect(resp.context.auth[:identity]).to be_a(Identities::HttpBearer) + expect(resp.context.auth[:identity]).to be_a(Identities::Token) end it 'resolves auth for http digest auth' do From c896331c090ecca886926957821561df3d5de403 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Thu, 4 Sep 2025 07:31:58 -0700 Subject: [PATCH 03/12] Require identities --- gems/smithy-client/lib/smithy-client.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gems/smithy-client/lib/smithy-client.rb b/gems/smithy-client/lib/smithy-client.rb index 2aeee5ae5..27b9c9460 100644 --- a/gems/smithy-client/lib/smithy-client.rb +++ b/gems/smithy-client/lib/smithy-client.rb @@ -53,6 +53,9 @@ require_relative 'smithy-client/identity' require_relative 'smithy-client/identity_provider' require_relative 'smithy-client/refreshing_identity_provider' +require_relative 'smithy-client/identities/http_api_key' +require_relative 'smithy-client/identities/http_login' +require_relative 'smithy-client/identities/token' # stubbing From dc0e7bd6d622bc180f9fde384cca2730f4c65e38 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Mon, 8 Sep 2025 09:47:44 -0700 Subject: [PATCH 04/12] PR comments --- .../lib/smithy-client/identities/token.rb | 4 +++- .../lib/smithy-client/plugins/resolve_auth.rb | 2 +- .../lib/smithy/views/client/auth_resolver.rb | 3 +-- .../spec/fixtures/auth/auth_trait/model.json | 15 +++++++++++++++ .../spec/fixtures/auth/auth_trait/model.smithy | 8 +++++++- .../spec/interfaces/client/auth_resolver_spec.rb | 6 ++++++ 6 files changed, 33 insertions(+), 5 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client/identities/token.rb b/gems/smithy-client/lib/smithy-client/identities/token.rb index 406ef7b42..5f792e624 100644 --- a/gems/smithy-client/lib/smithy-client/identities/token.rb +++ b/gems/smithy-client/lib/smithy-client/identities/token.rb @@ -13,10 +13,12 @@ def initialize(token:, **) # @return [String, nil] attr_reader :token + alias original_inspect inspect + # Removing the token from the default inspect string. # @api private def inspect - "#<#{self.class.name} token=[FILTERED]>" + original_inspect.gsub(/@token="[^"]*"/, '@token=[FILTERED]') end end end diff --git a/gems/smithy-client/lib/smithy-client/plugins/resolve_auth.rb b/gems/smithy-client/lib/smithy-client/plugins/resolve_auth.rb index bd43856a0..e25c80f72 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/resolve_auth.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/resolve_auth.rb @@ -38,7 +38,7 @@ def resolve_auth(context, auth_options) auth_options.each do |auth_option| # Anonymous auth does not have a plugin and does not sign, # so if auth scheme is noAuth then just return scheme_id. - return { scheme_id: auth_option } if %w[smithy.api#noAuth smithy.api#optionalAuth].include?(auth_option) + return { scheme_id: auth_option } if auth_option == 'smithy.api#noAuth' unless context.config.auth_schemes.key?(auth_option) failures << "Auth scheme #{auth_option} was not enabled for this request" diff --git a/gems/smithy/lib/smithy/views/client/auth_resolver.rb b/gems/smithy/lib/smithy/views/client/auth_resolver.rb index 5765d69fa..ac3ed7d03 100644 --- a/gems/smithy/lib/smithy/views/client/auth_resolver.rb +++ b/gems/smithy/lib/smithy/views/client/auth_resolver.rb @@ -96,8 +96,7 @@ def operation_auth_schemes(operation) else add_registered_auth_schemes(auth_schemes, operation_traits) end - auth_schemes << 'smithy.api#optionalAuth' if operation_traits.key?('smithy.api#optionalAuth') - auth_schemes << 'smithy.api#noAuth' if auth_schemes.empty? + auth_schemes << 'smithy.api#noAuth' if auth_schemes.empty? || operation_traits.key?('smithy.api#optionalAuth') auth_schemes end diff --git a/gems/smithy/spec/fixtures/auth/auth_trait/model.json b/gems/smithy/spec/fixtures/auth/auth_trait/model.json index 8c0af79fa..71893a018 100644 --- a/gems/smithy/spec/fixtures/auth/auth_trait/model.json +++ b/gems/smithy/spec/fixtures/auth/auth_trait/model.json @@ -36,6 +36,18 @@ "smithy.api#auth": [] } }, + "smithy.ruby.tests#OperationF": { + "type": "operation", + "input": { + "target": "smithy.api#Unit" + }, + "output": { + "target": "smithy.api#Unit" + }, + "traits": { + "smithy.api#optionalAuth": {} + } + }, "smithy.ruby.tests#ServiceWithAuthTrait": { "type": "service", "version": "2020-01-29", @@ -48,6 +60,9 @@ }, { "target": "smithy.ruby.tests#OperationE" + }, + { + "target": "smithy.ruby.tests#OperationF" } ], "traits": { diff --git a/gems/smithy/spec/fixtures/auth/auth_trait/model.smithy b/gems/smithy/spec/fixtures/auth/auth_trait/model.smithy index 7e0898300..dc4b99e50 100644 --- a/gems/smithy/spec/fixtures/auth/auth_trait/model.smithy +++ b/gems/smithy/spec/fixtures/auth/auth_trait/model.smithy @@ -12,6 +12,7 @@ service ServiceWithAuthTrait { OperationC OperationD OperationE + OperationF ] } @@ -29,4 +30,9 @@ operation OperationD {} // This operation has the @auth trait and is bound to a service with the // @auth trait. This operation does not support any authentication schemes. @auth([]) -operation OperationE {} \ No newline at end of file +operation OperationE {} + +// This operation has the @optionalAuth trait and is bound to a service +// with the @auth trait. This operation supports unauthenticated access. +@optionalAuth +operation OperationF {} \ No newline at end of file diff --git a/gems/smithy/spec/interfaces/client/auth_resolver_spec.rb b/gems/smithy/spec/interfaces/client/auth_resolver_spec.rb index 2ca811ab5..96616d834 100644 --- a/gems/smithy/spec/interfaces/client/auth_resolver_spec.rb +++ b/gems/smithy/spec/interfaces/client/auth_resolver_spec.rb @@ -51,6 +51,12 @@ auth_options = subject.resolve(params) expect(auth_options).to eq(['smithy.api#noAuth']) end + + it 'returns a noAuth option for the operation with optionalAuth trait' do + params = AuthTrait::AuthParameters.new(operation_name: :operation_f) + auth_options = subject.resolve(params) + expect(auth_options).to eq(['smithy.api#noAuth']) + end end end end From d925c42aa51c31d5587cc3df31b9461e0cf09178 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Mon, 8 Sep 2025 09:59:58 -0700 Subject: [PATCH 05/12] HttpApiKey -> ApiKey --- gems/smithy-client/lib/smithy-client.rb | 2 +- .../smithy-client/http_api_key_provider.rb | 2 +- .../lib/smithy-client/identities/api_key.rb | 26 +++++++++++++++++++ .../smithy-client/identities/http_api_key.rb | 18 ------------- .../plugins/http_api_key_auth.rb | 2 +- .../{http_api_key.rbs => api_key.rbs} | 1 + .../plugins/resolve_auth_spec.rb | 2 +- 7 files changed, 31 insertions(+), 22 deletions(-) create mode 100644 gems/smithy-client/lib/smithy-client/identities/api_key.rb delete mode 100644 gems/smithy-client/lib/smithy-client/identities/http_api_key.rb rename gems/smithy-client/sig/smithy-client/identities/{http_api_key.rbs => api_key.rbs} (86%) diff --git a/gems/smithy-client/lib/smithy-client.rb b/gems/smithy-client/lib/smithy-client.rb index 27b9c9460..0927132ce 100644 --- a/gems/smithy-client/lib/smithy-client.rb +++ b/gems/smithy-client/lib/smithy-client.rb @@ -53,7 +53,7 @@ require_relative 'smithy-client/identity' require_relative 'smithy-client/identity_provider' require_relative 'smithy-client/refreshing_identity_provider' -require_relative 'smithy-client/identities/http_api_key' +require_relative 'smithy-client/identities/api_key' require_relative 'smithy-client/identities/http_login' require_relative 'smithy-client/identities/token' diff --git a/gems/smithy-client/lib/smithy-client/http_api_key_provider.rb b/gems/smithy-client/lib/smithy-client/http_api_key_provider.rb index e3940e4a3..edaab218b 100644 --- a/gems/smithy-client/lib/smithy-client/http_api_key_provider.rb +++ b/gems/smithy-client/lib/smithy-client/http_api_key_provider.rb @@ -8,7 +8,7 @@ class HttpApiKeyProvider # @param [String] key def initialize(key) - @identity = Identities::HttpApiKey.new(key: key) + @identity = Identities::ApiKey.new(key: key) end end end diff --git a/gems/smithy-client/lib/smithy-client/identities/api_key.rb b/gems/smithy-client/lib/smithy-client/identities/api_key.rb new file mode 100644 index 000000000..6039b8848 --- /dev/null +++ b/gems/smithy-client/lib/smithy-client/identities/api_key.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Smithy + module Client + module Identities + # Identity class for API Key authentication. + class ApiKey < Identity + def initialize(key:, **) + @key = key + super(**) + end + + # @return [String, nil] + attr_reader :key + + alias original_inspect inspect + + # Removing the key from the default inspect string. + # @api private + def inspect + original_inspect.gsub(/@key="[^"]*"/, '@key=[FILTERED]') + end + end + end + end +end diff --git a/gems/smithy-client/lib/smithy-client/identities/http_api_key.rb b/gems/smithy-client/lib/smithy-client/identities/http_api_key.rb deleted file mode 100644 index 4b31385fb..000000000 --- a/gems/smithy-client/lib/smithy-client/identities/http_api_key.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -module Smithy - module Client - module Identities - # Identity class for HTTP API Key authentication. - class HttpApiKey < Identity - def initialize(key:, **) - @key = key - super(**) - end - - # @return [String, nil] - attr_reader :key - end - end - end -end diff --git a/gems/smithy-client/lib/smithy-client/plugins/http_api_key_auth.rb b/gems/smithy-client/lib/smithy-client/plugins/http_api_key_auth.rb index ffecda72e..40563e628 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/http_api_key_auth.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/http_api_key_auth.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require_relative '../http_api_key_provider' -require_relative '../identities/http_api_key' +require_relative '../identities/api_key' module Smithy module Client diff --git a/gems/smithy-client/sig/smithy-client/identities/http_api_key.rbs b/gems/smithy-client/sig/smithy-client/identities/api_key.rbs similarity index 86% rename from gems/smithy-client/sig/smithy-client/identities/http_api_key.rbs rename to gems/smithy-client/sig/smithy-client/identities/api_key.rbs index d93832a25..4f896f2dd 100644 --- a/gems/smithy-client/sig/smithy-client/identities/http_api_key.rbs +++ b/gems/smithy-client/sig/smithy-client/identities/api_key.rbs @@ -4,6 +4,7 @@ module Smithy class HttpApiKey < Identity def initialize: (key: String, **untyped options) -> void attr_reader key: String + def inspect: () -> String end end end diff --git a/gems/smithy-client/spec/smithy-client/plugins/resolve_auth_spec.rb b/gems/smithy-client/spec/smithy-client/plugins/resolve_auth_spec.rb index 4a5d67d5d..3ab0b569b 100644 --- a/gems/smithy-client/spec/smithy-client/plugins/resolve_auth_spec.rb +++ b/gems/smithy-client/spec/smithy-client/plugins/resolve_auth_spec.rb @@ -36,7 +36,7 @@ module Plugins client_class.add_plugin(HttpApiKeyAuth) resp = client.operation expect(resp.context.auth[:scheme_id]).to equal('smithy.api#httpApiKeyAuth') - expect(resp.context.auth[:identity]).to be_a(Identities::HttpApiKey) + expect(resp.context.auth[:identity]).to be_a(Identities::ApiKey) end it 'resolves auth for http basic auth' do From c69062dc436f7e9e3e665d605dc95b7acb84ce80 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Mon, 8 Sep 2025 14:15:53 -0700 Subject: [PATCH 06/12] Remove requires --- gems/smithy-client/lib/smithy-client.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client.rb b/gems/smithy-client/lib/smithy-client.rb index 0927132ce..2aeee5ae5 100644 --- a/gems/smithy-client/lib/smithy-client.rb +++ b/gems/smithy-client/lib/smithy-client.rb @@ -53,9 +53,6 @@ require_relative 'smithy-client/identity' require_relative 'smithy-client/identity_provider' require_relative 'smithy-client/refreshing_identity_provider' -require_relative 'smithy-client/identities/api_key' -require_relative 'smithy-client/identities/http_login' -require_relative 'smithy-client/identities/token' # stubbing From 75207d5edc1f9db5bfcb7cec7e6a7044066772ae Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Tue, 9 Sep 2025 09:59:42 -0700 Subject: [PATCH 07/12] Fix auth resolver for optional auth --- .../lib/smithy-client/identities/api_key.rb | 2 +- .../lib/smithy-client/identities/token.rb | 8 +++- .../sig/smithy-client/identities/api_key.rbs | 1 - .../sig/smithy-client/identities/token.rbs | 1 - .../lib/smithy/views/client/auth_resolver.rb | 5 ++- .../fixtures/auth/auth_trait/model.smithy | 3 +- .../spec/fixtures/auth/no_auth/model.json | 38 +++++++++++++++++++ .../spec/fixtures/auth/no_auth/model.smithy | 20 ++++++++++ .../fixtures/auth/no_auth_trait/model.json | 15 ++++++++ .../fixtures/auth/no_auth_trait/model.smithy | 7 ++++ .../interfaces/client/auth_resolver_spec.rb | 29 +++++++++++++- .../shapes/lib/shapes/endpoint_provider.rb | 1 + .../weather/lib/weather/endpoint_provider.rb | 1 + 13 files changed, 123 insertions(+), 8 deletions(-) create mode 100644 gems/smithy/spec/fixtures/auth/no_auth/model.json create mode 100644 gems/smithy/spec/fixtures/auth/no_auth/model.smithy diff --git a/gems/smithy-client/lib/smithy-client/identities/api_key.rb b/gems/smithy-client/lib/smithy-client/identities/api_key.rb index 6039b8848..c6d6def78 100644 --- a/gems/smithy-client/lib/smithy-client/identities/api_key.rb +++ b/gems/smithy-client/lib/smithy-client/identities/api_key.rb @@ -18,7 +18,7 @@ def initialize(key:, **) # Removing the key from the default inspect string. # @api private def inspect - original_inspect.gsub(/@key="[^"]*"/, '@key=[FILTERED]') + original_inspect.gsub(/@key="(\\"|[^"])*"/, '@key=[FILTERED]') end end end diff --git a/gems/smithy-client/lib/smithy-client/identities/token.rb b/gems/smithy-client/lib/smithy-client/identities/token.rb index 5f792e624..fd0a5dec8 100644 --- a/gems/smithy-client/lib/smithy-client/identities/token.rb +++ b/gems/smithy-client/lib/smithy-client/identities/token.rb @@ -13,12 +13,16 @@ def initialize(token:, **) # @return [String, nil] attr_reader :token - alias original_inspect inspect + # alias original_inspect inspect # Removing the token from the default inspect string. # @api private def inspect - original_inspect.gsub(/@token="[^"]*"/, '@token=[FILTERED]') + puts self + result = puts self + subbed = result.gsub(/@token="(\\"|[^"])*"/, '@token=[FILTERED]') + puts subbed + subbed end end end diff --git a/gems/smithy-client/sig/smithy-client/identities/api_key.rbs b/gems/smithy-client/sig/smithy-client/identities/api_key.rbs index 4f896f2dd..d93832a25 100644 --- a/gems/smithy-client/sig/smithy-client/identities/api_key.rbs +++ b/gems/smithy-client/sig/smithy-client/identities/api_key.rbs @@ -4,7 +4,6 @@ module Smithy class HttpApiKey < Identity def initialize: (key: String, **untyped options) -> void attr_reader key: String - def inspect: () -> String end end end diff --git a/gems/smithy-client/sig/smithy-client/identities/token.rbs b/gems/smithy-client/sig/smithy-client/identities/token.rbs index ef7249e90..7443db992 100644 --- a/gems/smithy-client/sig/smithy-client/identities/token.rbs +++ b/gems/smithy-client/sig/smithy-client/identities/token.rbs @@ -4,7 +4,6 @@ module Smithy class Token < Identity def initialize: (token: String, **untyped options) -> void attr_reader token: String - def inspect: () -> String end end end diff --git a/gems/smithy/lib/smithy/views/client/auth_resolver.rb b/gems/smithy/lib/smithy/views/client/auth_resolver.rb index ac3ed7d03..23c11fb5a 100644 --- a/gems/smithy/lib/smithy/views/client/auth_resolver.rb +++ b/gems/smithy/lib/smithy/views/client/auth_resolver.rb @@ -93,10 +93,13 @@ def operation_auth_schemes(operation) if operation_auth?(operation) operation_auth = operation_traits.fetch('smithy.api#auth', []) add_auth_schemes_from_auth_trait(auth_schemes, operation_auth) + elsif optional_operation_auth?(operation) + auth_schemes = service_auth_schemes << 'smithy.api#noAuth' + auth_schemes.uniq! else add_registered_auth_schemes(auth_schemes, operation_traits) end - auth_schemes << 'smithy.api#noAuth' if auth_schemes.empty? || operation_traits.key?('smithy.api#optionalAuth') + auth_schemes << 'smithy.api#noAuth' if auth_schemes.empty? auth_schemes end diff --git a/gems/smithy/spec/fixtures/auth/auth_trait/model.smithy b/gems/smithy/spec/fixtures/auth/auth_trait/model.smithy index dc4b99e50..7b4282855 100644 --- a/gems/smithy/spec/fixtures/auth/auth_trait/model.smithy +++ b/gems/smithy/spec/fixtures/auth/auth_trait/model.smithy @@ -33,6 +33,7 @@ operation OperationD {} operation OperationE {} // This operation has the @optionalAuth trait and is bound to a service -// with the @auth trait. This operation supports unauthenticated access. +// with the @auth trait. The effective set of authentication schemes it +// supports are: httpBasicAuth, httpDigestAuth, and noAuth @optionalAuth operation OperationF {} \ No newline at end of file diff --git a/gems/smithy/spec/fixtures/auth/no_auth/model.json b/gems/smithy/spec/fixtures/auth/no_auth/model.json new file mode 100644 index 000000000..27cb305f6 --- /dev/null +++ b/gems/smithy/spec/fixtures/auth/no_auth/model.json @@ -0,0 +1,38 @@ +{ + "smithy": "2.0", + "shapes": { + "smithy.ruby.tests#OperationH": { + "type": "operation", + "input": { + "target": "smithy.api#Unit" + }, + "output": { + "target": "smithy.api#Unit" + } + }, + "smithy.ruby.tests#OperationI": { + "type": "operation", + "input": { + "target": "smithy.api#Unit" + }, + "output": { + "target": "smithy.api#Unit" + }, + "traits": { + "smithy.api#optionalAuth": {} + } + }, + "smithy.ruby.tests#ServiceWithNoAuth": { + "type": "service", + "version": "2020-01-29", + "operations": [ + { + "target": "smithy.ruby.tests#OperationH" + }, + { + "target": "smithy.ruby.tests#OperationI" + } + ] + } + } +} diff --git a/gems/smithy/spec/fixtures/auth/no_auth/model.smithy b/gems/smithy/spec/fixtures/auth/no_auth/model.smithy new file mode 100644 index 000000000..9de07fe92 --- /dev/null +++ b/gems/smithy/spec/fixtures/auth/no_auth/model.smithy @@ -0,0 +1,20 @@ +$version: "2.0" + +namespace smithy.ruby.tests + +service ServiceWithNoAuth { + version: "2020-01-29" + operations: [ + OperationH + OperationI + ] +} + +// This operation does not have the @auth trait and is bound to a service +// without the @auth trait. This operation does not support any authentication schemes. +operation OperationH {} + +// This operation has the @optionalAuth trait and is bound to a service +// without the @auth trait. This operation does not support any authentication schemes. +@optionalAuth +operation OperationI {} \ No newline at end of file diff --git a/gems/smithy/spec/fixtures/auth/no_auth_trait/model.json b/gems/smithy/spec/fixtures/auth/no_auth_trait/model.json index f74113c77..288b7c742 100644 --- a/gems/smithy/spec/fixtures/auth/no_auth_trait/model.json +++ b/gems/smithy/spec/fixtures/auth/no_auth_trait/model.json @@ -24,6 +24,18 @@ ] } }, + "smithy.ruby.tests#OperationG": { + "type": "operation", + "input": { + "target": "smithy.api#Unit" + }, + "output": { + "target": "smithy.api#Unit" + }, + "traits": { + "smithy.api#optionalAuth": {} + } + }, "smithy.ruby.tests#ServiceWithNoAuthTrait": { "type": "service", "version": "2020-01-29", @@ -33,6 +45,9 @@ }, { "target": "smithy.ruby.tests#OperationB" + }, + { + "target": "smithy.ruby.tests#OperationG" } ], "traits": { diff --git a/gems/smithy/spec/fixtures/auth/no_auth_trait/model.smithy b/gems/smithy/spec/fixtures/auth/no_auth_trait/model.smithy index 7f3290486..3400d275f 100644 --- a/gems/smithy/spec/fixtures/auth/no_auth_trait/model.smithy +++ b/gems/smithy/spec/fixtures/auth/no_auth_trait/model.smithy @@ -10,6 +10,7 @@ service ServiceWithNoAuthTrait { operations: [ OperationA OperationB + OperationG ] } @@ -23,3 +24,9 @@ operation OperationA {} // supports are: httpDigestAuth. @auth([httpDigestAuth]) operation OperationB {} + +// This operation has the @optionalAuth trait and is bound to a service +// without the @auth trait. The effective set of authentication schemes it +// supports are: httpBasicAuth, httpDigestAuth, httpBearerAuth, and noAuth +@optionalAuth +operation OperationG {} \ No newline at end of file diff --git a/gems/smithy/spec/interfaces/client/auth_resolver_spec.rb b/gems/smithy/spec/interfaces/client/auth_resolver_spec.rb index 96616d834..7d0b9d1df 100644 --- a/gems/smithy/spec/interfaces/client/auth_resolver_spec.rb +++ b/gems/smithy/spec/interfaces/client/auth_resolver_spec.rb @@ -24,6 +24,13 @@ auth_options = subject.resolve(params) expect(auth_options).to eq(['smithy.api#httpDigestAuth']) end + + it 'returns the auth options for the operation with the optionalAuth trait' do + params = NoAuthTrait::AuthParameters.new(operation_name: :operation_g) + auth_options = subject.resolve(params) + expect(auth_options).to eq(%w[smithy.api#httpBasicAuth smithy.api#httpBearerAuth smithy.api#httpDigestAuth + smithy.api#noAuth]) + end end end @@ -52,9 +59,29 @@ expect(auth_options).to eq(['smithy.api#noAuth']) end - it 'returns a noAuth option for the operation with optionalAuth trait' do + it 'returns the auth options for the operation with the optionalAuth trait' do params = AuthTrait::AuthParameters.new(operation_name: :operation_f) auth_options = subject.resolve(params) + expect(auth_options).to eq(%w[smithy.api#httpBasicAuth smithy.api#httpDigestAuth smithy.api#noAuth]) + end + end + end + + context context do + include_context context, 'NoAuth', fixture: 'auth/no_auth' + + subject { NoAuth::AuthResolver.new } + + describe '#resolve' do + it 'returns the auth options for the operation with no auth traits' do + params = NoAuth::AuthParameters.new(operation_name: :operation_h) + auth_options = subject.resolve(params) + expect(auth_options).to eq(['smithy.api#noAuth']) + end + + it 'returns the auth options for the operation with the optionalAuth trait' do + params = NoAuth::AuthParameters.new(operation_name: :operation_i) + auth_options = subject.resolve(params) expect(auth_options).to eq(['smithy.api#noAuth']) end end diff --git a/projections/shapes/lib/shapes/endpoint_provider.rb b/projections/shapes/lib/shapes/endpoint_provider.rb index e9d5257c5..ff367ab9f 100644 --- a/projections/shapes/lib/shapes/endpoint_provider.rb +++ b/projections/shapes/lib/shapes/endpoint_provider.rb @@ -13,6 +13,7 @@ def resolve(parameters) return Smithy::Client::EndpointRules::Endpoint.new(uri: parameters.endpoint) end raise ArgumentError, "Endpoint is not set - you must configure an endpoint." + end end end diff --git a/projections/weather/lib/weather/endpoint_provider.rb b/projections/weather/lib/weather/endpoint_provider.rb index 5dac5fa9d..0417f4fa5 100644 --- a/projections/weather/lib/weather/endpoint_provider.rb +++ b/projections/weather/lib/weather/endpoint_provider.rb @@ -13,6 +13,7 @@ def resolve(parameters) return Smithy::Client::EndpointRules::Endpoint.new(uri: parameters.endpoint) end raise ArgumentError, "Endpoint is not set - you must configure an endpoint." + end end end From 40a53c472bcbaf71f938124688e195561c54dc32 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Tue, 9 Sep 2025 10:06:58 -0700 Subject: [PATCH 08/12] Remove puts --- gems/smithy-client/lib/smithy-client/identities/token.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client/identities/token.rb b/gems/smithy-client/lib/smithy-client/identities/token.rb index fd0a5dec8..09186bdea 100644 --- a/gems/smithy-client/lib/smithy-client/identities/token.rb +++ b/gems/smithy-client/lib/smithy-client/identities/token.rb @@ -13,16 +13,12 @@ def initialize(token:, **) # @return [String, nil] attr_reader :token - # alias original_inspect inspect + alias original_inspect inspect # Removing the token from the default inspect string. # @api private def inspect - puts self - result = puts self - subbed = result.gsub(/@token="(\\"|[^"])*"/, '@token=[FILTERED]') - puts subbed - subbed + original_inspect.gsub(/@token="(\\"|[^"])*"/, '@token=[FILTERED]') end end end From 4dc8d57b803599283586090e8fe9374e92848d30 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Wed, 10 Sep 2025 08:33:06 -0700 Subject: [PATCH 09/12] Fix inspect --- gems/smithy-client/lib/smithy-client/identities/api_key.rb | 4 +--- gems/smithy-client/lib/smithy-client/identities/token.rb | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client/identities/api_key.rb b/gems/smithy-client/lib/smithy-client/identities/api_key.rb index c6d6def78..4aa6d4bdc 100644 --- a/gems/smithy-client/lib/smithy-client/identities/api_key.rb +++ b/gems/smithy-client/lib/smithy-client/identities/api_key.rb @@ -13,12 +13,10 @@ def initialize(key:, **) # @return [String, nil] attr_reader :key - alias original_inspect inspect - # Removing the key from the default inspect string. # @api private def inspect - original_inspect.gsub(/@key="(\\"|[^"])*"/, '@key=[FILTERED]') + super.gsub(/@key="(\\"|[^"])*"/, '@key=[FILTERED]') end end end diff --git a/gems/smithy-client/lib/smithy-client/identities/token.rb b/gems/smithy-client/lib/smithy-client/identities/token.rb index 09186bdea..8907ef043 100644 --- a/gems/smithy-client/lib/smithy-client/identities/token.rb +++ b/gems/smithy-client/lib/smithy-client/identities/token.rb @@ -13,12 +13,10 @@ def initialize(token:, **) # @return [String, nil] attr_reader :token - alias original_inspect inspect - # Removing the token from the default inspect string. # @api private def inspect - original_inspect.gsub(/@token="(\\"|[^"])*"/, '@token=[FILTERED]') + super.gsub(/@token="(\\"|[^"])*"/, '@token=[FILTERED]') end end end From ba5b8bfdb039e0db5620b0f8b3b3d4b2a494323b Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Wed, 10 Sep 2025 11:49:21 -0400 Subject: [PATCH 10/12] Refactor of auth_resolver --- .../lib/smithy/views/client/auth_resolver.rb | 74 ++++++++++++------- 1 file changed, 47 insertions(+), 27 deletions(-) diff --git a/gems/smithy/lib/smithy/views/client/auth_resolver.rb b/gems/smithy/lib/smithy/views/client/auth_resolver.rb index 23c11fb5a..5c50c6ec0 100644 --- a/gems/smithy/lib/smithy/views/client/auth_resolver.rb +++ b/gems/smithy/lib/smithy/views/client/auth_resolver.rb @@ -18,39 +18,45 @@ def module_name @plan.module_name end - # rubocop:disable Metrics/AbcSize - # rubocop:disable Metrics/MethodLength def auth_rules_code lines = [] lines << 'options = []' auth_operations = operations_with_auth_traits if auth_operations.empty? - service_auth_schemes.each do |auth_scheme| - lines << "options << '#{auth_scheme}'" - end + add_service_auth_schemes_to_code(lines) else - lines << 'case parameters.operation_name' - auth_operations.each do |id, operation| - operation_name = Model::Shape.name(id).underscore - lines << "when :#{operation_name}" - operation_auth_schemes(operation).each do |auth_scheme| - lines << " options << '#{auth_scheme}'" - end - end - lines << 'else' - service_auth_schemes.each do |auth_scheme| - lines << " options << '#{auth_scheme}'" - end - lines << 'end' + add_operation_case_to_code(lines, auth_operations) end lines << 'options' lines end - # rubocop:enable Metrics/MethodLength - # rubocop:enable Metrics/AbcSize private + def add_service_auth_schemes_to_code(lines) + service_auth_schemes.each do |auth_scheme| + lines << "options << '#{auth_scheme}'" + end + end + + def add_operation_case_to_code(lines, auth_operations) + lines << 'case parameters.operation_name' + auth_operations.each do |id, operation| + operation_name = Model::Shape.name(id).underscore + lines << "when :#{operation_name}" + add_operation_auth_options_to_code(lines, operation) + end + lines << 'else' + add_service_auth_schemes_to_code(lines) + lines << 'end' + end + + def add_operation_auth_options_to_code(lines, operation) + operation_auth_schemes(operation).each do |auth_scheme| + lines << " options << '#{auth_scheme}'" + end + end + def auth_schemes(welds) weld_auth_schemes = welds.map(&:add_auth_schemes).reduce([], :+) weld_auth_schemes -= welds.map(&:remove_auth_schemes).reduce([], :+) @@ -89,20 +95,34 @@ def optional_operation_auth?(operation) def operation_auth_schemes(operation) operation_traits = operation.fetch('traits', {}) + auth_schemes = build_operation_auth_schemes(operation, operation_traits) + auth_schemes << 'smithy.api#noAuth' if auth_schemes.empty? || optional_operation_auth?(operation) + auth_schemes + end + + def build_operation_auth_schemes(operation, operation_traits) auth_schemes = [] if operation_auth?(operation) - operation_auth = operation_traits.fetch('smithy.api#auth', []) - add_auth_schemes_from_auth_trait(auth_schemes, operation_auth) - elsif optional_operation_auth?(operation) - auth_schemes = service_auth_schemes << 'smithy.api#noAuth' - auth_schemes.uniq! - else + add_explicit_operation_auth_schemes(auth_schemes, operation_traits) + elsif !service_has_auth_trait? && !optional_operation_auth?(operation) add_registered_auth_schemes(auth_schemes, operation_traits) + else + add_service_auth_schemes_for_operation(auth_schemes) end - auth_schemes << 'smithy.api#noAuth' if auth_schemes.empty? auth_schemes end + def add_explicit_operation_auth_schemes(auth_schemes, operation_traits) + operation_auth = operation_traits.fetch('smithy.api#auth', []) + add_auth_schemes_from_auth_trait(auth_schemes, operation_auth) + end + + def add_service_auth_schemes_for_operation(auth_schemes) + service_auth_schemes.each do |auth_scheme| + auth_schemes << auth_scheme unless auth_scheme == 'smithy.api#noAuth' + end + end + def add_auth_schemes_from_auth_trait(auth_schemes, auth_trait) auth_trait.each do |auth_scheme| auth_schemes << auth_scheme if @auth_schemes.include?(auth_scheme) From 95868fc134114468170116c387dbcbd18c73f547 Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Wed, 10 Sep 2025 12:09:44 -0400 Subject: [PATCH 11/12] minor removal of dead branch --- gems/smithy/lib/smithy/views/client/auth_resolver.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/gems/smithy/lib/smithy/views/client/auth_resolver.rb b/gems/smithy/lib/smithy/views/client/auth_resolver.rb index 5c50c6ec0..725b71d32 100644 --- a/gems/smithy/lib/smithy/views/client/auth_resolver.rb +++ b/gems/smithy/lib/smithy/views/client/auth_resolver.rb @@ -104,8 +104,6 @@ def build_operation_auth_schemes(operation, operation_traits) auth_schemes = [] if operation_auth?(operation) add_explicit_operation_auth_schemes(auth_schemes, operation_traits) - elsif !service_has_auth_trait? && !optional_operation_auth?(operation) - add_registered_auth_schemes(auth_schemes, operation_traits) else add_service_auth_schemes_for_operation(auth_schemes) end From 40332d69cb50c34955ca02a84ecd41b3c3d55945 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Wed, 10 Sep 2025 09:20:35 -0700 Subject: [PATCH 12/12] PR comments --- gems/smithy/spec/fixtures/auth/no_auth/model.smithy | 4 ++-- .../spec/interfaces/client/auth_resolver_spec.rb | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/gems/smithy/spec/fixtures/auth/no_auth/model.smithy b/gems/smithy/spec/fixtures/auth/no_auth/model.smithy index 9de07fe92..0a84d58f4 100644 --- a/gems/smithy/spec/fixtures/auth/no_auth/model.smithy +++ b/gems/smithy/spec/fixtures/auth/no_auth/model.smithy @@ -11,10 +11,10 @@ service ServiceWithNoAuth { } // This operation does not have the @auth trait and is bound to a service -// without the @auth trait. This operation does not support any authentication schemes. +// without auth. This operation does not support any authentication schemes. operation OperationH {} // This operation has the @optionalAuth trait and is bound to a service -// without the @auth trait. This operation does not support any authentication schemes. +// without auth. This operation does not support any authentication schemes. @optionalAuth operation OperationI {} \ No newline at end of file diff --git a/gems/smithy/spec/interfaces/client/auth_resolver_spec.rb b/gems/smithy/spec/interfaces/client/auth_resolver_spec.rb index 7d0b9d1df..16c442bb3 100644 --- a/gems/smithy/spec/interfaces/client/auth_resolver_spec.rb +++ b/gems/smithy/spec/interfaces/client/auth_resolver_spec.rb @@ -28,8 +28,14 @@ it 'returns the auth options for the operation with the optionalAuth trait' do params = NoAuthTrait::AuthParameters.new(operation_name: :operation_g) auth_options = subject.resolve(params) - expect(auth_options).to eq(%w[smithy.api#httpBasicAuth smithy.api#httpBearerAuth smithy.api#httpDigestAuth - smithy.api#noAuth]) + expect(auth_options).to eq( + %w[ + smithy.api#httpBasicAuth + smithy.api#httpBearerAuth + smithy.api#httpDigestAuth + smithy.api#noAuth + ] + ) end end end