Fix RUBY-2421 Auth mechanism properties not downcased when client is created with host and Ruby options with symbol key (#2110)

* Check caller identity after assuming role

* check connectivity via mongo shell

* reference aws docs for sts errors

* run server status instead of ismaster to catch auth errors

* line count should suffice

* expand aws auth notes

* RUBY-2421 test case for auth mechanism properties not getting downcased

* RUBY-2421 account for keys possibly being either symbols or strings at this point

* if options are strings they need to go into a document derivative

* fix again

* redo options type conversions once again

* the test suite needs auth mechanism properties downcased when testing objects below the client in hierarchy

* try converting back to hash

* Revert "try converting back to hash"

This reverts commit b50ff8d.

* symbolize keys here

* handle explicit nil values

* RUBY-2425 deal with wildcard arns

* fix connection tests

* only sts arns are modified

* we no longer write nil passwords into the client

Co-authored-by: Oleg Pudeyev <oleg@bsdpower.com>

Author: p-mongo

.evergreen/lib/server_setup.rb

@@ -5,7 +5,7 @@
 class ServerSetup
   def setup_aws_auth
     arn = env!('MONGO_RUBY_DRIVER_AWS_AUTH_USER_ARN')
-    puts "Adding AWS-mapped user #{arn}"
+    puts "Adding AWS-mapped user #{wildcard_arn(arn)} for #{arn}"
     create_aws_user(arn)
 
     puts 'Setup done'
@@ -27,14 +27,27 @@ def setup_tags
 
   private
 
+  # Creates an appropriate AWS mapped user for the provided ARN.
+  #
+  # The mapped user does not use the specified ARN directly but instead
+  # uses a derived wildcard ARN. Because of this, multiple ARNs can map
+  # to the same user.
   def create_aws_user(arn)
     bootstrap_client.use('$external').database.users.create(
-      arn,
+      wildcard_arn(arn),
       roles: [{role: 'root', db: 'admin'}],
       write_concern: {w: :majority, wtimeout: 5000},
     )
   end
 
+  def wildcard_arn(arn)
+    if arn.start_with?('arn:aws:sts::')
+      arn.sub(%r,/[^/]+\z,, '/*')
+    else
+      arn
+    end
+  end
+
   def require_env_vars(vars)
     vars.each do |var|
       unless env?(var)
@@ -64,8 +77,8 @@ def client
   end
 
   def bootstrap_client
-    @bootstrap_client ||= Mongo::Client.new(%w(localhost),
-      user: 'bootstrap', password: 'bootstrap',
+    @bootstrap_client ||= Mongo::Client.new(ENV['MONGODB_URI'] || %w(localhost),
+      user: 'bootstrap', password: 'bootstrap', auth_mech: :scram, auth_mech_properties: nil,
     )
   end
 end

.evergreen/run-tests-aws-auth.sh

@@ -41,6 +41,8 @@ case "$AUTH" in
     # request resolve to. It is hardcoded in
     # https://github.com/mongodb-labs/drivers-evergreen-tools/blob/master/.evergreen/auth_aws/aws_e2e_assume_role.js
     # and is not given as an Evergreen variable.
+    # Note: the asterisk at the end is manufactured by the server and not
+    # obtained from STS. See https://jira.mongodb.org/browse/RUBY-2425.
     export MONGO_RUBY_DRIVER_AWS_AUTH_USER_ARN="arn:aws:sts::557821124784:assumed-role/authtest_user_assume_role/*"
     ;;
 

.evergreen/run-tests.sh

@@ -225,6 +225,8 @@ elif test "$AUTH" = aws-assume-role; then
   export AWS_ACCESS_KEY_ID=$AWS_ACCESS_KEY_ID
   export AWS_SECRET_ACCESS_KEY=$AWS_SECRET_ACCESS_KEY
   export AWS_SESSION_TOKEN=$AWS_SESSION_TOKEN
+  
+  aws sts get-caller-identity
 
   hosts="`uri_escape $MONGO_RUBY_DRIVER_AWS_AUTH_ACCESS_KEY_ID`:`uri_escape $MONGO_RUBY_DRIVER_AWS_AUTH_SECRET_ACCESS_KEY`@$hosts"
 
@@ -269,6 +271,10 @@ fi
 
 export MONGODB_URI="mongodb://$hosts/?serverSelectionTimeoutMS=30000$uri_options"
 
+if echo "$AUTH" |grep -q ^aws-assume-role; then
+  $BINDIR/mongo "$MONGODB_URI" --eval 'db.runCommand({serverStatus: 1})' |wc
+fi
+
 set_fcv
 
 if test "$TOPOLOGY" = replica-set && ! echo "$MONGODB_VERSION" |fgrep -q 2.6; then

lib/mongo/client.rb

@@ -463,12 +463,7 @@ def initialize(addresses_or_uri, options = nil)
         @srv_records = nil
       end
 
-      options = Options::Redacted.new(Hash[options.map do |k, v|
-        if k == 'auth_mech_properties'
-          v = Hash[v.map { |pk, pv| [pk.downcase, pv] }]
-        end
-        [k, v]
-      end])
+      options = self.class.canonicalize_ruby_options(options)
 
       # Special handling for sdam_proc as it is only used during client
       # construction
@@ -486,6 +481,12 @@ def initialize(addresses_or_uri, options = nil)
       end
       options = merged_options
 
+      options.keys.each do |k|
+        if options[k].nil?
+          options.delete(k)
+        end
+      end
+
       @options = validate_new_options!(options)
 =begin WriteConcern object support
       if @options[:write_concern].is_a?(WriteConcern::Base)
@@ -774,7 +775,6 @@ def read_concern
       options[:read_concern]
     end
 
-
     # Get the write concern for this client. If no option was provided, then a
     # default single server acknowledgement will be used.
     #
@@ -1027,6 +1027,23 @@ def with_session(options = {}, &block)
       end
     end
 
+    class << self
+      # Lowercases auth mechanism properties, if given, in the specified
+      # options, then converts the options to an instance of Options::Redacted.
+      #
+      # @api private
+      def canonicalize_ruby_options(options)
+        Options::Redacted.new(Hash[options.map do |k, v|
+          if k == :auth_mech_properties || k == 'auth_mech_properties'
+            if v
+              v = Hash[v.map { |pk, pv| [pk.downcase, pv] }]
+            end
+          end
+          [k, v]
+        end])
+      end
+    end
+
     private
 
     # Create a new encrypter object using the client's auto encryption options

spec/NOTES.aws-auth.md

@@ -145,13 +145,16 @@ problem. Below are some of the puzzling responses I encountered:
   line) but the value is otherwise completely valid. This error has no relation
   to the "session token" or "security token" as used with temporary AWS
   credentials.
-- *The security token included in the request is invalid*: this error is
-  produced when the AWS access key id, as specified in the scope part of the
-  `Authorization` header, is not a valid access key id. In the case of
-  non-temporary credentials being used for authentication, the error refers to
-  a "security token" but the authentication process does not actually use a
-  security token as this term is used in the AWS documentation describing
-  temporary credentials.
+- *The security token included in the request is invalid*: this error can be
+  produced in several circumstances:
+  - When the AWS access key id, as specified in the scope part of the
+    `Authorization` header, is not a valid access key id. In the case of
+    non-temporary credentials being used for authentication, the error refers to
+    a "security token" but the authentication process does not actually use a
+    security token as this term is used in the AWS documentation describing
+    temporary credentials.
+  - When using temporary credentials and the security token is not provided
+    in the STS request at all (x-amz-security-token header).
 - *Signature expired: 20200317T000000Z is now earlier than 20200317T222541Z
   (20200317T224041Z - 15 min.)*: This error happens when `x-amz-date` header
   value is the formatted date (`YYYYMMDD`) rather than the ISO8601 formatted
@@ -168,6 +171,8 @@ problem. Below are some of the puzzling responses I encountered:
   produced when temporary credentials are used and the credentials have
   expired.
 
+See also [AWS documentation for STS error messages](https://docs.aws.amazon.com/STS/latest/APIReference/CommonErrors.html).
+
 ### Resources
 
 Generally I found Amazon's own documentation to be the best for implementing

spec/integration/client_authentication_options_spec.rb

@@ -464,20 +464,37 @@
     end
 
     context 'with client options' do
-      let(:client_opts) { { auth_mech_properties: auth_mechanism_properties } }
+      [:auth_mech_properties, 'auth_mech_properties'].each do |key|
 
-      include_examples 'correctly sets auth mechanism properties on the client'
+        context "using #{key.class} keys" do
+          let(:client_opts) { { key => auth_mechanism_properties } }
 
-      context 'when options are given in mixed case' do
-        let(:auth_mechanism_properties) do
-          {
-            service_NAME: service_name,
-            canonicalize_host_NAME: canonicalize_host_name,
-            service_REALM: service_realm,
-          }.freeze
-        end
+          include_examples 'correctly sets auth mechanism properties on the client'
+
+          context 'when options are given in mixed case' do
+            let(:auth_mechanism_properties) do
+              {
+                service_NAME: service_name,
+                canonicalize_host_NAME: canonicalize_host_name,
+                service_REALM: service_realm,
+              }.freeze
+            end
+
+            context 'using URI and options' do
 
-        include_examples 'correctly sets auth mechanism properties on the client'
+              let(:client) { new_local_client_nmio(uri, client_opts) }
+
+              include_examples 'correctly sets auth mechanism properties on the client'
+            end
+
+            context 'using host and options' do
+
+              let(:client) { new_local_client_nmio(['localhost'], client_opts) }
+
+              include_examples 'correctly sets auth mechanism properties on the client'
+            end
+          end
+        end
       end
     end
   end

spec/integration/connection_pool_populator_spec.rb

@@ -4,7 +4,9 @@
   let(:options) { {} }
 
   let(:server_options) do
-    SpecConfig.instance.test_options.merge(options).merge(SpecConfig.instance.auth_options)
+    Mongo::Utils.shallow_symbolize_keys(Mongo::Client.canonicalize_ruby_options(
+      SpecConfig.instance.all_test_options,
+    )).update(options)
   end
 
   let(:address) do

spec/mongo/client_construction_spec.rb

@@ -1535,6 +1535,18 @@
           end
         end
       end
+
+      context ':auth_mech_properties option' do
+        context 'is nil' do
+          let(:options) do
+            {auth_mech_properties: nil}
+          end
+
+          it 'creates the client without the option' do
+            client.options.should_not have_key(:auth_mech_properties)
+          end
+        end
+      end
     end
 
     context 'when making a block client' do

spec/mongo/server/connection_pool_spec.rb

@@ -5,9 +5,13 @@
   let(:options) { {} }
 
   let(:server_options) do
-    SpecConfig.instance.ssl_options.merge(SpecConfig.instance.compressor_options)
-      .merge(SpecConfig.instance.retry_writes_options).merge(SpecConfig.instance.auth_options)
-      .merge(options)
+    Mongo::Utils.shallow_symbolize_keys(Mongo::Client.canonicalize_ruby_options(
+      SpecConfig.instance.all_test_options,
+    )).tap do |opts|
+      opts.delete(:min_pool_size)
+      opts.delete(:max_pool_size)
+      opts.delete(:wait_queue_timeout)
+    end.update(options)
   end
 
   let(:address) do

spec/mongo/server/connection_spec.rb

@@ -252,8 +252,9 @@ class ConnectionSpecTestException < Exception; end
         require_auth
 
         let(:server_options) do
-          SpecConfig.instance.test_options.merge(monitoring_io: false).
-            merge(SpecConfig.instance.auth_options)
+          Mongo::Client.canonicalize_ruby_options(
+            SpecConfig.instance.all_test_options,
+          ).update(monitoring_io: false)
         end
 
         let(:exception) do
@@ -593,10 +594,12 @@ class ConnectionSpecTestException < Exception; end
         server,
         SpecConfig.instance.test_options.merge(
           :database => SpecConfig.instance.test_user.database,
-        ).merge(SpecConfig.instance.credentials_or_external_user(
-          user: SpecConfig.instance.test_user.name,
-          password: SpecConfig.instance.test_user.password,
-        ))
+        ).merge(Mongo::Utils.shallow_symbolize_keys(Mongo::Client.canonicalize_ruby_options(
+          SpecConfig.instance.credentials_or_external_user(
+            user: SpecConfig.instance.test_user.name,
+            password: SpecConfig.instance.test_user.password,
+          ),
+        )))
       ).tap do |connection|
         connection.connect!
       end
@@ -1103,7 +1106,7 @@ class ConnectionSpecTestException < Exception; end
           :user => SpecConfig.instance.test_user.name,
           :password => SpecConfig.instance.test_user.password,
           :database => SpecConfig.instance.test_db,
-          :auth_mech => :mongodb_cr
+          :auth_mech => :mongodb_cr,
         )
       end