From 8a6d06841b5ae4bdd597f6537016343b5bf9adc9 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Wed, 2 Sep 2015 10:16:35 +0200 Subject: [PATCH 01/15] RUBY-1021 Use SensitiveOptions class to redact option fields when printing --- lib/mongo/client.rb | 4 +-- lib/mongo/options.rb | 1 + lib/mongo/options/sensitive_options.rb | 37 ++++++++++++++++++++++++++ lib/mongo/server/connection.rb | 2 +- 4 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 lib/mongo/options/sensitive_options.rb diff --git a/lib/mongo/client.rb b/lib/mongo/client.rb index 7e9d787b28..7e66a1873b 100644 --- a/lib/mongo/client.rb +++ b/lib/mongo/client.rb @@ -292,14 +292,14 @@ def list_databases private def create_from_addresses(addresses, opts = {}) - @options = Database::DEFAULT_OPTIONS.merge(opts).freeze + @options = Options::SensitiveOptions.new.merge(Database::DEFAULT_OPTIONS.merge(opts)).freeze @cluster = Cluster.new(addresses, @monitoring, options) @database = Database.new(self, options[:database], options) end def create_from_uri(connection_string, opts = {}) uri = URI.new(connection_string, opts) - @options = Database::DEFAULT_OPTIONS.merge(uri.client_options.merge(opts)).freeze + @options = Options::SensitiveOptions.new.merge(Database::DEFAULT_OPTIONS.merge(uri.client_options.merge(opts))).freeze @cluster = Cluster.new(uri.servers, @monitoring, options) @database = Database.new(self, options[:database], options) end diff --git a/lib/mongo/options.rb b/lib/mongo/options.rb index 37479cda49..5e34db3f7e 100644 --- a/lib/mongo/options.rb +++ b/lib/mongo/options.rb @@ -13,3 +13,4 @@ # limitations under the License. require 'mongo/options/mapper' +require 'mongo/options/sensitive_options' diff --git a/lib/mongo/options/sensitive_options.rb b/lib/mongo/options/sensitive_options.rb new file mode 100644 index 0000000000..9c9c3ed1f4 --- /dev/null +++ b/lib/mongo/options/sensitive_options.rb @@ -0,0 +1,37 @@ +# Copyright (C) 2015 MongoDB, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +module Mongo + module Options + + SENSITIVE_OPTIONS = [:password, :pwd] + REDACTED_STRING = '' + + class SensitiveOptions < BSON::Document + + def inspect + '{' + reduce('') do |string, (k, v)| + string << "#{k}=>#{redact(k,v)}" + end + '}' + end + + private + + def redact(k, v) + return REDACTED_STRING if SENSITIVE_OPTIONS.include?(k.to_sym) + v.inspect + end + end + end +end diff --git a/lib/mongo/server/connection.rb b/lib/mongo/server/connection.rb index 58e8bb9b84..5616ae0115 100644 --- a/lib/mongo/server/connection.rb +++ b/lib/mongo/server/connection.rb @@ -183,7 +183,7 @@ def deliver(messages) def setup_authentication! if options[:user] default_mechanism = @server.features.scram_sha_1_enabled? ? :scram : :mongodb_cr - user = Auth::User.new({ :auth_mech => default_mechanism }.merge(options)) + user = Auth::User.new(Options::SensitiveOptions.new(:auth_mech => default_mechanism).merge(options)) @authenticator = Auth.get(user) end end From ea03c844070f00f2a349a002cede7e8ba013724b Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Wed, 2 Sep 2015 10:20:28 +0200 Subject: [PATCH 02/15] RUBY-1021 to_s method --- lib/mongo/options/sensitive_options.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/mongo/options/sensitive_options.rb b/lib/mongo/options/sensitive_options.rb index 9c9c3ed1f4..f34601188f 100644 --- a/lib/mongo/options/sensitive_options.rb +++ b/lib/mongo/options/sensitive_options.rb @@ -22,15 +22,21 @@ class SensitiveOptions < BSON::Document def inspect '{' + reduce('') do |string, (k, v)| - string << "#{k}=>#{redact(k,v)}" + string << "#{k.inspect}=>#{redact(k, v, __method__)}" + end + '}' + end + + def to_s + '{' + reduce('') do |string, (k, v)| + string << "#{k.to_s}=>#{redact(k, v, __method__)}" end + '}' end private - def redact(k, v) + def redact(k, v, method) return REDACTED_STRING if SENSITIVE_OPTIONS.include?(k.to_sym) - v.inspect + v.send(method) end end end From 4c6de4c4d5cb1ae2bfcd139b55940584876e99eb Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Wed, 2 Sep 2015 11:39:02 +0200 Subject: [PATCH 03/15] Get specs passing and change name to Options::Redacted --- lib/mongo/client.rb | 8 +-- lib/mongo/database.rb | 7 ++- lib/mongo/options.rb | 2 +- .../{sensitive_options.rb => redacted.rb} | 22 ++++---- lib/mongo/server/connection.rb | 2 +- spec/mongo/client_spec.rb | 52 ++++++++++++++----- spec/mongo/grid/stream/write_spec.rb | 4 +- 7 files changed, 65 insertions(+), 32 deletions(-) rename lib/mongo/options/{sensitive_options.rb => redacted.rb} (62%) diff --git a/lib/mongo/client.rb b/lib/mongo/client.rb index 7e66a1873b..c5d73a75b2 100644 --- a/lib/mongo/client.rb +++ b/lib/mongo/client.rb @@ -217,7 +217,7 @@ def use(name) # @since 2.0.0 def with(new_options = {}) clone.tap do |client| - opts = new_options || {} + opts = Options::Redacted.new(new_options) || Options::Redacted.new client.options.update(opts) Database.create(client) # We can't use the same cluster if some options that would affect it @@ -292,14 +292,14 @@ def list_databases private def create_from_addresses(addresses, opts = {}) - @options = Options::SensitiveOptions.new.merge(Database::DEFAULT_OPTIONS.merge(opts)).freeze + @options = Options::Redacted.new.merge(Database::DEFAULT_OPTIONS.merge(opts)).freeze @cluster = Cluster.new(addresses, @monitoring, options) @database = Database.new(self, options[:database], options) end def create_from_uri(connection_string, opts = {}) uri = URI.new(connection_string, opts) - @options = Options::SensitiveOptions.new.merge(Database::DEFAULT_OPTIONS.merge(uri.client_options.merge(opts))).freeze + @options = Options::Redacted.new.merge(Database::DEFAULT_OPTIONS.merge(uri.client_options.merge(opts))).freeze @cluster = Cluster.new(uri.servers, @monitoring, options) @database = Database.new(self, options[:database], options) end @@ -314,7 +314,7 @@ def initialize_copy(original) def cluster_modifying?(new_options) cluster_options = new_options.reject do |name| - CRUD_OPTIONS.include?(name) + CRUD_OPTIONS.include?(name.to_sym) end cluster_options.any? do |name, value| options[name] != value diff --git a/lib/mongo/database.rb b/lib/mongo/database.rb index ea6744c67c..e805fb3c35 100644 --- a/lib/mongo/database.rb +++ b/lib/mongo/database.rb @@ -148,7 +148,12 @@ def collections # # @return [ Hash ] The result of the command execution. def command(operation, opts = {}) - preference = opts[:read] ? ServerSelector.get(opts[:read].merge(options)) : read_preference + if opts[:read] + preference = ServerSelector.get(Options::Redacted.new( + opts[:read]).merge(options)) + else + preference = read_preference + end server = preference.select_server(cluster) Operation::Command.new({ :selector => operation, diff --git a/lib/mongo/options.rb b/lib/mongo/options.rb index 5e34db3f7e..7562654c20 100644 --- a/lib/mongo/options.rb +++ b/lib/mongo/options.rb @@ -13,4 +13,4 @@ # limitations under the License. require 'mongo/options/mapper' -require 'mongo/options/sensitive_options' +require 'mongo/options/redacted' diff --git a/lib/mongo/options/sensitive_options.rb b/lib/mongo/options/redacted.rb similarity index 62% rename from lib/mongo/options/sensitive_options.rb rename to lib/mongo/options/redacted.rb index f34601188f..9a46a2a507 100644 --- a/lib/mongo/options/sensitive_options.rb +++ b/lib/mongo/options/redacted.rb @@ -15,27 +15,31 @@ module Mongo module Options - SENSITIVE_OPTIONS = [:password, :pwd] + REDACTED_OPTIONS = [:password, :pwd] REDACTED_STRING = '' - class SensitiveOptions < BSON::Document + class Redacted < BSON::Document def inspect - '{' + reduce('') do |string, (k, v)| - string << "#{k.inspect}=>#{redact(k, v, __method__)}" - end + '}' + '{' + reduce([]) do |list, (k, v)| + list << "#{k.inspect}=>#{redact(k, v, __method__)}" + end.join(', ') + '}' end def to_s - '{' + reduce('') do |string, (k, v)| - string << "#{k.to_s}=>#{redact(k, v, __method__)}" - end + '}' + '{' + reduce([]) do |list, (k, v)| + list << "#{k.to_s}=>#{redact(k, v, __method__)}" + end.join(', ') + '}' + end + + def has_key?(key) + super(convert_key(key)) end private def redact(k, v, method) - return REDACTED_STRING if SENSITIVE_OPTIONS.include?(k.to_sym) + return REDACTED_STRING if REDACTED_OPTIONS.include?(k.to_sym) v.send(method) end end diff --git a/lib/mongo/server/connection.rb b/lib/mongo/server/connection.rb index 5616ae0115..008006ca6d 100644 --- a/lib/mongo/server/connection.rb +++ b/lib/mongo/server/connection.rb @@ -183,7 +183,7 @@ def deliver(messages) def setup_authentication! if options[:user] default_mechanism = @server.features.scram_sha_1_enabled? ? :scram : :mongodb_cr - user = Auth::User.new(Options::SensitiveOptions.new(:auth_mech => default_mechanism).merge(options)) + user = Auth::User.new(Options::Redacted.new(:auth_mech => default_mechanism).merge(options)) @authenticator = Auth.get(user) end end diff --git a/spec/mongo/client_spec.rb b/spec/mongo/client_spec.rb index 6db81847f8..638e95bae6 100644 --- a/spec/mongo/client_spec.rb +++ b/spec/mongo/client_spec.rb @@ -172,11 +172,15 @@ ) end + let(:options) do + Mongo::Options::Redacted.new(:read => { :mode => :primary }, + :local_threshold_ms => 10, + :server_selection_timeout_ms => 10000, + :database => TEST_DB) + end + let(:expected) do - [client.cluster, { :read => { :mode => :primary }, - :local_threshold_ms => 10, - :server_selection_timeout_ms => 10000, - :database => TEST_DB }].hash + [client.cluster, options].hash end it 'returns a hash of the cluster and options' do @@ -291,8 +295,12 @@ described_class.new(uri) end + let(:expected_options) do + Mongo::Options::Redacted.new(:write => { :w => 3 }, :database => 'testdb') + end + it 'sets the options' do - expect(client.options).to eq(:write => { :w => 3 }, :database => 'testdb') + expect(client.options).to eq(expected_options) end end @@ -306,8 +314,12 @@ described_class.new(uri, :write => { :w => 3 }) end + let(:expected_options) do + Mongo::Options::Redacted.new(:write => { :w => 3 }, :database => 'testdb') + end + it 'sets the options' do - expect(client.options).to eq(:write => { :w => 3 }, :database => 'testdb') + expect(client.options).to eq(expected_options) end end @@ -321,8 +333,12 @@ described_class.new(uri, :write => { :w => 4 }) end + let(:expected_options) do + Mongo::Options::Redacted.new(:write => { :w => 4 }, :database => 'testdb') + end + it 'allows explicit options to take preference' do - expect(client.options).to eq(:write => { :w => 4 }, :database => 'testdb') + expect(client.options).to eq(expected_options) end end @@ -497,20 +513,28 @@ client.with(:read => { :mode => :primary }) end + let(:new_options) do + Mongo::Options::Redacted.new(:read => { :mode => :primary }, + :write => { :w => 1 }, + :database => TEST_DB) + end + + let(:original_options) do + Mongo::Options::Redacted.new(:read => { :mode => :secondary }, + :write => { :w => 1 }, + :database => TEST_DB) + end + it 'returns a new client' do expect(new_client).not_to equal(client) end it 'replaces the existing options' do - expect(new_client.options).to eq({ - :read => { :mode => :primary }, :write => { :w => 1 }, :database => TEST_DB - }) + expect(new_client.options).to eq(new_options) end it 'does not modify the original client' do - expect(client.options).to eq({ - :read => { :mode => :secondary }, :write => { :w => 1 }, :database => TEST_DB - }) + expect(client.options).to eq(original_options) end it 'keeps the same cluster' do @@ -579,7 +603,7 @@ end it 'returns a acknowledged write concern' do - expect(concern.get_last_error).to eq(:getlasterror => 1, :j => true) + expect(concern.get_last_error).to eq(:getlasterror => 1, 'j' => true) end end diff --git a/spec/mongo/grid/stream/write_spec.rb b/spec/mongo/grid/stream/write_spec.rb index d8de33de43..633b56904c 100644 --- a/spec/mongo/grid/stream/write_spec.rb +++ b/spec/mongo/grid/stream/write_spec.rb @@ -87,14 +87,14 @@ context 'when chunks are inserted' do it 'uses that write concern' do - expect(stream.send(:chunks_collection).write_concern.options).to eq(expected) + expect(stream.send(:chunks_collection).write_concern.options[:w]).to eq(expected[:w]) end end context 'when a files document is inserted' do it 'uses that write concern' do - expect(stream.send(:files_collection).write_concern.options).to eq(expected) + expect(stream.send(:files_collection).write_concern.options[:w]).to eq(expected[:w]) end end end From fec72a97488398e6ab0c29df1d431809483438c3 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Tue, 8 Sep 2015 15:12:25 -0400 Subject: [PATCH 04/15] RUBY-1021 Test and documentation for Redacted Options --- lib/mongo/client.rb | 4 +- lib/mongo/options/redacted.rb | 40 ++++++++- spec/mongo/options/redacted_spec.rb | 132 ++++++++++++++++++++++++++++ 3 files changed, 170 insertions(+), 6 deletions(-) create mode 100644 spec/mongo/options/redacted_spec.rb diff --git a/lib/mongo/client.rb b/lib/mongo/client.rb index c5d73a75b2..607df75fcc 100644 --- a/lib/mongo/client.rb +++ b/lib/mongo/client.rb @@ -292,14 +292,14 @@ def list_databases private def create_from_addresses(addresses, opts = {}) - @options = Options::Redacted.new.merge(Database::DEFAULT_OPTIONS.merge(opts)).freeze + @options = Options::Redacted.new(Database::DEFAULT_OPTIONS.merge(opts)).freeze @cluster = Cluster.new(addresses, @monitoring, options) @database = Database.new(self, options[:database], options) end def create_from_uri(connection_string, opts = {}) uri = URI.new(connection_string, opts) - @options = Options::Redacted.new.merge(Database::DEFAULT_OPTIONS.merge(uri.client_options.merge(opts))).freeze + @options = Options::Redacted.new(Database::DEFAULT_OPTIONS.merge(uri.client_options.merge(opts))).freeze @cluster = Cluster.new(uri.servers, @monitoring, options) @database = Database.new(self, options[:database], options) end diff --git a/lib/mongo/options/redacted.rb b/lib/mongo/options/redacted.rb index 9a46a2a507..0767f0d94d 100644 --- a/lib/mongo/options/redacted.rb +++ b/lib/mongo/options/redacted.rb @@ -15,23 +15,55 @@ module Mongo module Options - REDACTED_OPTIONS = [:password, :pwd] - REDACTED_STRING = '' - + # Class for wrapping options that could be sensitive. + # When printed, the sensitive values will be redacted. + # + # @since 2.1.0 class Redacted < BSON::Document + # The options whose values will be redacted. + # + # @since 2.1.0 + SENSITIVE_OPTIONS = [ :password, + :pwd ] + + # The replacement string used in place of the value for sensitive keys. + # + # @since 2.1.0 + STRING_REPLACEMENT = '' + + # Get a string representation of the options. + # + # @return [ String ] The string representation of the options. + # + # @since 2.1.0 def inspect '{' + reduce([]) do |list, (k, v)| list << "#{k.inspect}=>#{redact(k, v, __method__)}" end.join(', ') + '}' end + # Get a string representation of the options. + # + # @return [ String ] The string representation of the options. + # + # @since 2.1.0 def to_s '{' + reduce([]) do |list, (k, v)| list << "#{k.to_s}=>#{redact(k, v, __method__)}" end.join(', ') + '}' end + # Whether these options contain a given key. + # + # @example Determine if the options contain a given key. + # options.has_key?(:name) + # + # @param [ String, Symbol ] key The key to check for existence. + # + # @return [ true, false ] If the options contain the given key. + # + # @since 2.1.0 def has_key?(key) super(convert_key(key)) end @@ -39,7 +71,7 @@ def has_key?(key) private def redact(k, v, method) - return REDACTED_STRING if REDACTED_OPTIONS.include?(k.to_sym) + return STRING_REPLACEMENT if SENSITIVE_OPTIONS.include?(k.to_sym) v.send(method) end end diff --git a/spec/mongo/options/redacted_spec.rb b/spec/mongo/options/redacted_spec.rb new file mode 100644 index 0000000000..31af0c5eb7 --- /dev/null +++ b/spec/mongo/options/redacted_spec.rb @@ -0,0 +1,132 @@ +require 'spec_helper' + +describe Mongo::Options::Redacted do + + let(:options) do + described_class.new(original_opts) + end + + describe '#to_s' do + + context 'when the hash contains a sensitive key' do + + let(:original_opts) do + { password: '123' } + end + + it 'replaces the value with the redacted string' do + expect(options.to_s).not_to match(/123/) + end + + it 'replaces the value with the redacted string' do + expect(options.to_s).to match(Mongo::Options::Redacted::STRING_REPLACEMENT) + end + end + + context 'when the hash does not contain a sensitive key' do + + it 'prints all the values' do + + end + end + end + + describe '#inspect' do + + context 'when the hash contains a sensitive key' do + + let(:original_opts) do + { password: '123' } + end + + it 'replaces the value with the redacted string' do + expect(options.inspect).not_to match(/123/) + end + + it 'replaces the value with the redacted string' do + expect(options.inspect).to match(Mongo::Options::Redacted::STRING_REPLACEMENT) + end + end + + context 'when the hash does not contain a sensitive key' do + + let(:original_opts) do + { name: '123' } + end + + it 'does not replace the value with the redacted string' do + expect(options.inspect).to match(/123/) + end + + it 'does not replace the value with the redacted string' do + expect(options.inspect).not_to match(Mongo::Options::Redacted::STRING_REPLACEMENT) + end + end + end + + describe '#has_key?' do + + context 'when the original key is a String' do + + let(:original_opts) do + { 'name' => 'Emily' } + end + + context 'when the method argument is a String' do + + it 'returns true when ' do + expect(options.has_key?('name')).to be(true) + end + end + + context 'when method argument is a Symbol' do + + it 'returns true' do + expect(options.has_key?(:name)).to be(true) + end + end + end + + context 'when the original key is a Symbol' do + + let(:original_opts) do + { name: 'Emily' } + end + + context 'when the method argument is a String' do + + it 'returns true when ' do + expect(options.has_key?('name')).to be(true) + end + end + + context 'when method argument is a Symbol' do + + it 'returns true' do + expect(options.has_key?(:name)).to be(true) + end + end + end + + context 'when the hash does not contain the key' do + + let(:original_opts) do + { other: 'Emily' } + end + + context 'when the method argument is a String' do + + it 'returns true when ' do + expect(options.has_key?('name')).to be(false) + end + end + + context 'when method argument is a Symbol' do + + it 'returns true' do + expect(options.has_key?(:name)).to be(false) + end + end + end + end +end \ No newline at end of file From 1338f16e9d613075f5c460bf0f2719d081ac29f6 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Tue, 8 Sep 2015 17:24:38 -0400 Subject: [PATCH 05/15] RUBY-1021 Ensure options merged with client options are Options::Redacted instances --- lib/mongo/client.rb | 2 +- lib/mongo/cluster.rb | 2 +- lib/mongo/collection/view/readable.rb | 3 ++- lib/mongo/database.rb | 7 +---- lib/mongo/grid/fs_bucket.rb | 2 +- lib/mongo/grid/stream/read.rb | 2 +- lib/mongo/server_selector/selectable.rb | 4 +-- spec/mongo/client_spec.rb | 36 ++++++++++++++++++++++--- spec/mongo/grid/fs_bucket_spec.rb | 2 +- 9 files changed, 42 insertions(+), 18 deletions(-) diff --git a/lib/mongo/client.rb b/lib/mongo/client.rb index 607df75fcc..2adb34e465 100644 --- a/lib/mongo/client.rb +++ b/lib/mongo/client.rb @@ -185,7 +185,7 @@ def inspect # # @since 2.0.0 def read_preference - @read_preference ||= ServerSelector.get((options[:read] || {}).merge(options)) + @read_preference ||= ServerSelector.get(Options::Redacted.new(options[:read] || {}).merge(options)) end # Use the database with the provided name. This will switch the current diff --git a/lib/mongo/cluster.rb b/lib/mongo/cluster.rb index f54aa97c09..81af162c3e 100644 --- a/lib/mongo/cluster.rb +++ b/lib/mongo/cluster.rb @@ -125,7 +125,7 @@ def inspect # # @since 2.0.0 def next_primary - ServerSelector.get({ mode: :primary }.merge(options)).select_server(self) + ServerSelector.get(Options::Redacted.new({ mode: :primary }.merge(options))).select_server(self) end # Elect a primary server from the description that has just changed to a diff --git a/lib/mongo/collection/view/readable.rb b/lib/mongo/collection/view/readable.rb index 3ca9f9bbcf..144c79c37a 100644 --- a/lib/mongo/collection/view/readable.rb +++ b/lib/mongo/collection/view/readable.rb @@ -298,7 +298,8 @@ def projection(document = nil) # @since 2.0.0 def read(value = nil) return default_read if value.nil? - configure(:read, value.is_a?(Hash) ? ServerSelector.get(value) : value) + selector = value.is_a?(Hash) ? ServerSelector.get(Options::Redacted.new(value.merge(client.options))) : value + configure(:read, selector) end # Set whether to return only the indexed field or fields. diff --git a/lib/mongo/database.rb b/lib/mongo/database.rb index e805fb3c35..a03b94b30d 100644 --- a/lib/mongo/database.rb +++ b/lib/mongo/database.rb @@ -148,12 +148,7 @@ def collections # # @return [ Hash ] The result of the command execution. def command(operation, opts = {}) - if opts[:read] - preference = ServerSelector.get(Options::Redacted.new( - opts[:read]).merge(options)) - else - preference = read_preference - end + preference = opts[:read] ? ServerSelector.get(Options::Redacted.new(opts[:read].merge(options))) : read_preference server = preference.select_server(cluster) Operation::Command.new({ :selector => operation, diff --git a/lib/mongo/grid/fs_bucket.rb b/lib/mongo/grid/fs_bucket.rb index 1a9a5384ca..abf286df26 100644 --- a/lib/mongo/grid/fs_bucket.rb +++ b/lib/mongo/grid/fs_bucket.rb @@ -395,7 +395,7 @@ def upload_from_stream(filename, io, opts = {}) # @since 2.1.0 def read_preference @read_preference ||= @options[:read] ? - ServerSelector.get((@options[:read] || {}).merge(database.options)) : + ServerSelector.get(Options::Redacted.new((@options[:read] || {}).merge(database.options))) : database.read_preference end diff --git a/lib/mongo/grid/stream/read.rb b/lib/mongo/grid/stream/read.rb index 279a6edeec..20865f5101 100644 --- a/lib/mongo/grid/stream/read.rb +++ b/lib/mongo/grid/stream/read.rb @@ -134,7 +134,7 @@ def closed? # @since 2.1.0 def read_preference @read_preference ||= @options[:read] ? - ServerSelector.get((@options[:read] || {}).merge(fs.options)) : + ServerSelector.get(Options::Redacted.new((@options[:read] || {}).merge(fs.options))) : fs.read_preference end diff --git a/lib/mongo/server_selector/selectable.rb b/lib/mongo/server_selector/selectable.rb index 9518da1152..eee877ae6d 100644 --- a/lib/mongo/server_selector/selectable.rb +++ b/lib/mongo/server_selector/selectable.rb @@ -61,10 +61,10 @@ def ==(other) # # @since 2.0.0 def initialize(options = {}) + @options = (options || {}).freeze tag_sets = options[:tag_sets] || [] validate_tag_sets!(tag_sets) - @tag_sets = tag_sets - @options = options + @tag_sets = tag_sets.freeze end # Select a server from eligible candidates. diff --git a/spec/mongo/client_spec.rb b/spec/mongo/client_spec.rb index 638e95bae6..f8dacf5d2d 100644 --- a/spec/mongo/client_spec.rb +++ b/spec/mongo/client_spec.rb @@ -167,7 +167,7 @@ ['127.0.0.1:27017'], :read => { :mode => :primary }, :local_threshold_ms => 10, - :server_selection_timeout_ms => 10000, + :server_selection_timeout => 10000, :database => TEST_DB ) end @@ -175,7 +175,7 @@ let(:options) do Mongo::Options::Redacted.new(:read => { :mode => :primary }, :local_threshold_ms => 10, - :server_selection_timeout_ms => 10000, + :server_selection_timeout => 10000, :database => TEST_DB) end @@ -364,7 +364,8 @@ let(:client) do described_class.new(['127.0.0.1:27017'], :database => TEST_DB, - :read => mode) + :read => mode, + :server_selection_timeout => 2) end let(:preference) do @@ -382,7 +383,7 @@ end it 'passes the options to the read preference' do - expect(preference.options[:database]).to eq(TEST_DB) + expect(preference.options[:server_selection_timeout]).to eq(2) end end @@ -440,6 +441,33 @@ expect(preference).to be_a(Mongo::ServerSelector::Primary) end end + + context 'when the read preference is printed' do + + let(:client) do + described_class.new([ DEFAULT_ADDRESS ], options) + end + + let(:options) do + { user: 'Emily', password: 'sensitive_data', server_selection_timeout: 0.1 } + end + + before do + allow(client.database.cluster).to receive(:single?).and_return(false) + end + + let(:error) do + begin + client.database.command(ping: 1) + rescue => e + e + end + end + + it 'redacts sensitive client options' do + expect(error.message).not_to match(options[:password]) + end + end end describe '#use' do diff --git a/spec/mongo/grid/fs_bucket_spec.rb b/spec/mongo/grid/fs_bucket_spec.rb index 21ee4fbe98..c7492da5b7 100644 --- a/spec/mongo/grid/fs_bucket_spec.rb +++ b/spec/mongo/grid/fs_bucket_spec.rb @@ -55,7 +55,7 @@ end let(:read_pref) do - Mongo::ServerSelector.get(options[:read].merge(authorized_client.options)) + Mongo::ServerSelector.get(Mongo::Options::Redacted.new(options[:read].merge(authorized_client.options))) end it 'sets the read preference' do From afbe202889802993274ad3aece42461878f465dd Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Tue, 8 Sep 2015 17:32:42 -0400 Subject: [PATCH 06/15] RUBY-1021 Use helper method to build string --- lib/mongo/options/redacted.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/mongo/options/redacted.rb b/lib/mongo/options/redacted.rb index 0767f0d94d..2babccc303 100644 --- a/lib/mongo/options/redacted.rb +++ b/lib/mongo/options/redacted.rb @@ -38,9 +38,7 @@ class Redacted < BSON::Document # # @since 2.1.0 def inspect - '{' + reduce([]) do |list, (k, v)| - list << "#{k.inspect}=>#{redact(k, v, __method__)}" - end.join(', ') + '}' + redacted_string(__method__) end # Get a string representation of the options. @@ -49,9 +47,7 @@ def inspect # # @since 2.1.0 def to_s - '{' + reduce([]) do |list, (k, v)| - list << "#{k.to_s}=>#{redact(k, v, __method__)}" - end.join(', ') + '}' + redacted_string(__method__) end # Whether these options contain a given key. @@ -70,6 +66,12 @@ def has_key?(key) private + def redacted_string(method) + '{' + reduce([]) do |list, (k, v)| + list << "#{k.send(method)}=>#{redact(k, v, method)}" + end.join(', ') + '}' + end + def redact(k, v, method) return STRING_REPLACEMENT if SENSITIVE_OPTIONS.include?(k.to_sym) v.send(method) From 63b4e317337d5924ae2428fb21521825a16044da Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Wed, 9 Sep 2015 12:09:02 -0400 Subject: [PATCH 07/15] RUBY-1021 Initialize options to Redacted and update uri parser --- lib/mongo.rb | 2 +- lib/mongo/client.rb | 8 +++--- lib/mongo/cluster.rb | 4 +-- lib/mongo/collection/view/readable.rb | 2 +- lib/mongo/database.rb | 4 +-- lib/mongo/grid/fs_bucket.rb | 7 ++++- lib/mongo/options/redacted.rb | 5 ++-- lib/mongo/server_selector.rb | 5 ++++ lib/mongo/uri.rb | 2 +- spec/mongo/uri_spec.rb | 40 +++++++++++++-------------- 10 files changed, 45 insertions(+), 34 deletions(-) diff --git a/lib/mongo.rb b/lib/mongo.rb index ed93036ce0..1be61d7b46 100644 --- a/lib/mongo.rb +++ b/lib/mongo.rb @@ -15,6 +15,7 @@ require 'forwardable' require 'bson' require 'openssl' +require 'mongo/options' require 'mongo/loggable' require 'mongo/monitoring' require 'mongo/logger' @@ -32,7 +33,6 @@ require 'mongo/dbref' require 'mongo/grid' require 'mongo/index' -require 'mongo/options' require 'mongo/protocol' require 'mongo/server' require 'mongo/server_selector' diff --git a/lib/mongo/client.rb b/lib/mongo/client.rb index 2adb34e465..3c0fcb4543 100644 --- a/lib/mongo/client.rb +++ b/lib/mongo/client.rb @@ -153,7 +153,7 @@ def hash # logs at the default 250 characters. # # @since 2.0.0 - def initialize(addresses_or_uri, options = {}) + def initialize(addresses_or_uri, options = Options::Redacted.new) @monitoring = Monitoring.new(options) if addresses_or_uri.is_a?(::String) create_from_uri(addresses_or_uri, options) @@ -215,7 +215,7 @@ def use(name) # @return [ Mongo::Client ] A new client instance. # # @since 2.0.0 - def with(new_options = {}) + def with(new_options = Options::Redacted.new) clone.tap do |client| opts = Options::Redacted.new(new_options) || Options::Redacted.new client.options.update(opts) @@ -291,13 +291,13 @@ def list_databases private - def create_from_addresses(addresses, opts = {}) + def create_from_addresses(addresses, opts = Options::Redacted.new) @options = Options::Redacted.new(Database::DEFAULT_OPTIONS.merge(opts)).freeze @cluster = Cluster.new(addresses, @monitoring, options) @database = Database.new(self, options[:database], options) end - def create_from_uri(connection_string, opts = {}) + def create_from_uri(connection_string, opts = Options::Redacted.new) uri = URI.new(connection_string, opts) @options = Options::Redacted.new(Database::DEFAULT_OPTIONS.merge(uri.client_options.merge(opts))).freeze @cluster = Cluster.new(uri.servers, @monitoring, options) diff --git a/lib/mongo/cluster.rb b/lib/mongo/cluster.rb index 81af162c3e..9d600e4dab 100644 --- a/lib/mongo/cluster.rb +++ b/lib/mongo/cluster.rb @@ -88,7 +88,7 @@ def add(host) # @param [ Hash ] options The options. # # @since 2.0.0 - def initialize(seeds, monitoring, options = {}) + def initialize(seeds, monitoring, options = Options::Redacted.new) @addresses = [] @servers = [] @monitoring = monitoring @@ -125,7 +125,7 @@ def inspect # # @since 2.0.0 def next_primary - ServerSelector.get(Options::Redacted.new({ mode: :primary }.merge(options))).select_server(self) + ServerSelector.get(ServerSelector::PRIMARY.merge(options)).select_server(self) end # Elect a primary server from the description that has just changed to a diff --git a/lib/mongo/collection/view/readable.rb b/lib/mongo/collection/view/readable.rb index 144c79c37a..b0424ef75d 100644 --- a/lib/mongo/collection/view/readable.rb +++ b/lib/mongo/collection/view/readable.rb @@ -298,7 +298,7 @@ def projection(document = nil) # @since 2.0.0 def read(value = nil) return default_read if value.nil? - selector = value.is_a?(Hash) ? ServerSelector.get(Options::Redacted.new(value.merge(client.options))) : value + selector = value.is_a?(Hash) ? ServerSelector.get(client.options.merge(value)) : value configure(:read, selector) end diff --git a/lib/mongo/database.rb b/lib/mongo/database.rb index a03b94b30d..012b3fcb79 100644 --- a/lib/mongo/database.rb +++ b/lib/mongo/database.rb @@ -36,7 +36,7 @@ class Database # The default database options. # # @since 2.0.0 - DEFAULT_OPTIONS = { :database => ADMIN }.freeze + DEFAULT_OPTIONS = Options::Redacted.new(:database => ADMIN).freeze # Database name field constant. # @@ -148,7 +148,7 @@ def collections # # @return [ Hash ] The result of the command execution. def command(operation, opts = {}) - preference = opts[:read] ? ServerSelector.get(Options::Redacted.new(opts[:read].merge(options))) : read_preference + preference = opts[:read] ? ServerSelector.get(client.options.merge(opts[:read])) : read_preference server = preference.select_server(cluster) Operation::Command.new({ :selector => operation, diff --git a/lib/mongo/grid/fs_bucket.rb b/lib/mongo/grid/fs_bucket.rb index abf286df26..a86b587d1f 100644 --- a/lib/mongo/grid/fs_bucket.rb +++ b/lib/mongo/grid/fs_bucket.rb @@ -19,6 +19,7 @@ module Grid # # @since 2.0.0 class FSBucket + extend Forwardable # The default root prefix. # @@ -55,6 +56,10 @@ class FSBucket # @since 2.1.0 attr_reader :options + # Get client from the database. + def_delegators :database, + :client + # Find files collection documents matching a given selector. # # @example Find files collection documents by a filename. @@ -395,7 +400,7 @@ def upload_from_stream(filename, io, opts = {}) # @since 2.1.0 def read_preference @read_preference ||= @options[:read] ? - ServerSelector.get(Options::Redacted.new((@options[:read] || {}).merge(database.options))) : + ServerSelector.get(client.options.merge(@options[:read] || {})) : database.read_preference end diff --git a/lib/mongo/options/redacted.rb b/lib/mongo/options/redacted.rb index 2babccc303..2475375939 100644 --- a/lib/mongo/options/redacted.rb +++ b/lib/mongo/options/redacted.rb @@ -38,7 +38,7 @@ class Redacted < BSON::Document # # @since 2.1.0 def inspect - redacted_string(__method__) + redacted_string(:inspect) end # Get a string representation of the options. @@ -47,7 +47,7 @@ def inspect # # @since 2.1.0 def to_s - redacted_string(__method__) + redacted_string(:to_s) end # Whether these options contain a given key. @@ -63,6 +63,7 @@ def to_s def has_key?(key) super(convert_key(key)) end + alias_method :key?, :has_key? private diff --git a/lib/mongo/server_selector.rb b/lib/mongo/server_selector.rb index c393014bde..eea35dab5e 100644 --- a/lib/mongo/server_selector.rb +++ b/lib/mongo/server_selector.rb @@ -38,6 +38,11 @@ module ServerSelector # @since 2.0.0 SERVER_SELECTION_TIMEOUT = 30.freeze + # Primary read preference. + # + # @since 2.1.0 + PRIMARY = Options::Redacted.new(mode: :primary) + # Hash lookup for the selector classes based off the symbols # provided in configuration. # diff --git a/lib/mongo/uri.rb b/lib/mongo/uri.rb index 5f69d2b1af..d456d90101 100644 --- a/lib/mongo/uri.rb +++ b/lib/mongo/uri.rb @@ -268,7 +268,7 @@ def split_creds_hosts(string) def parse_db_opts!(string) auth_db, d, uri_opts = string.partition(URI_OPTS_DELIM) - @uri_options = parse_uri_options!(uri_opts) + @uri_options = Options::Redacted.new(parse_uri_options!(uri_opts)) @database = parse_database!(auth_db) end diff --git a/spec/mongo/uri_spec.rb b/spec/mongo/uri_spec.rb index 8b3c784cd1..46c61014cf 100644 --- a/spec/mongo/uri_spec.rb +++ b/spec/mongo/uri_spec.rb @@ -365,7 +365,7 @@ context 'numerical w value' do let(:options) { 'w=1' } - let(:concern) { { :w => 1 } } + let(:concern) { Mongo::Options::Redacted.new(:w => 1)} it 'sets the write concern options' do expect(uri.uri_options[:write]).to eq(concern) @@ -374,7 +374,7 @@ context 'w=majority' do let(:options) { 'w=majority' } - let(:concern) { { :w => :majority } } + let(:concern) { Mongo::Options::Redacted.new(:w => :majority) } it 'sets the write concern options' do expect(uri.uri_options[:write]).to eq(concern) @@ -383,7 +383,7 @@ context 'journal' do let(:options) { 'journal=true' } - let(:concern) { { :j => true } } + let(:concern) { Mongo::Options::Redacted.new(:j => true) } it 'sets the write concern options' do expect(uri.uri_options[:write]).to eq(concern) @@ -392,7 +392,7 @@ context 'fsync' do let(:options) { 'fsync=true' } - let(:concern) { { :fsync => true } } + let(:concern) { Mongo::Options::Redacted.new(:fsync => true) } it 'sets the write concern options' do expect(uri.uri_options[:write]).to eq(concern) @@ -402,7 +402,7 @@ context 'wtimeoutMS' do let(:timeout) { 1234 } let(:options) { "w=2&wtimeoutMS=#{timeout}" } - let(:concern) { { :w => 2, :timeout => timeout } } + let(:concern) { Mongo::Options::Redacted.new(:w => 2, :timeout => timeout) } it 'sets the write concern options' do expect(uri.uri_options[:write]).to eq(concern) @@ -415,7 +415,7 @@ context 'primary' do let(:mode) { 'primary' } - let(:read) { { :mode => :primary } } + let(:read) { Mongo::Options::Redacted.new(:mode => :primary) } it 'sets the read preference' do expect(uri.uri_options[:read]).to eq(read) @@ -424,7 +424,7 @@ context 'primaryPreferred' do let(:mode) { 'primaryPreferred' } - let(:read) { { :mode => :primary_preferred } } + let(:read) { Mongo::Options::Redacted.new(:mode => :primary_preferred) } it 'sets the read preference' do expect(uri.uri_options[:read]).to eq(read) @@ -433,7 +433,7 @@ context 'secondary' do let(:mode) { 'secondary' } - let(:read) { { :mode => :secondary } } + let(:read) { Mongo::Options::Redacted.new(:mode => :secondary) } it 'sets the read preference' do expect(uri.uri_options[:read]).to eq(read) @@ -442,7 +442,7 @@ context 'secondaryPreferred' do let(:mode) { 'secondaryPreferred' } - let(:read) { { :mode => :secondary_preferred } } + let(:read) { Mongo::Options::Redacted.new(:mode => :secondary_preferred) } it 'sets the read preference' do expect(uri.uri_options[:read]).to eq(read) @@ -451,7 +451,7 @@ context 'nearest' do let(:mode) { 'nearest' } - let(:read) { { :mode => :nearest } } + let(:read) { Mongo::Options::Redacted.new(:mode => :nearest) } it 'sets the read preference' do expect(uri.uri_options[:read]).to eq(read) @@ -467,7 +467,7 @@ end let(:read) do - { :tag_sets => [{ :dc => 'ny', :rack => '1' }] } + Mongo::Options::Redacted.new(:tag_sets => [{ 'dc' => 'ny', 'rack' => '1' }]) end it 'sets the read preference tag set' do @@ -481,7 +481,7 @@ end let(:read) do - { :tag_sets => [{ :dc => 'ny' }, { :dc => 'bos' }] } + Mongo::Options::Redacted.new(:tag_sets => [{ 'dc' => 'ny' }, { 'dc' => 'bos' }]) end it 'sets the read preference tag sets' do @@ -536,7 +536,7 @@ context 'regular db' do let(:source) { 'foo' } - let(:auth) { { :source => 'foo' } } + let(:auth) { Mongo::Options::Redacted.new(:source => 'foo') } it 'sets the auth source to the database' do expect(uri.uri_options[:auth]).to eq(auth) @@ -545,7 +545,7 @@ context '$external' do let(:source) { '$external' } - let(:auth) { { :source => :external } } + let(:auth) { Mongo::Options::Redacted.new(:source => :external) } it 'sets the auth source to :external' do expect(uri.uri_options[:auth]).to eq(auth) @@ -562,7 +562,7 @@ let(:service_name) { 'foo' } let(:auth) do - { auth_mech_properties: { service_name: service_name } } + Mongo::Options::Redacted.new(auth_mech_properties: { service_name: service_name }) end it 'sets the auth mechanism properties' do @@ -577,7 +577,7 @@ let(:canonicalize_host_name) { 'true' } let(:auth) do - { auth_mech_properties: { canonicalize_host_name: true } } + Mongo::Options::Redacted.new(auth_mech_properties: { canonicalize_host_name: true }) end it 'sets the auth mechanism properties' do @@ -592,7 +592,7 @@ let(:service_realm) { 'dumdum' } let(:auth) do - { auth_mech_properties: { service_realm: service_realm } } + Mongo::Options::Redacted.new(auth_mech_properties: { service_realm: service_realm }) end it 'sets the auth mechanism properties' do @@ -612,9 +612,9 @@ let(:service_realm) { 'dumdum' } let(:auth) do - { auth_mech_properties: { service_name: service_name, - canonicalize_host_name: true, - service_realm: service_realm } } + Mongo::Options::Redacted.new(auth_mech_properties: { service_name: service_name, + canonicalize_host_name: true, + service_realm: service_realm }) end it 'sets the auth mechanism properties' do From 26c19391e2c6d15376de758af11d67175ec75fad Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Wed, 9 Sep 2015 12:27:13 -0400 Subject: [PATCH 08/15] RUBY-1021 Tests for inspect methods --- spec/mongo/client_spec.rb | 32 +++++++++++++++++++ spec/mongo/server_selector/nearest_spec.rb | 1 + .../server_selector/primary_preferred_spec.rb | 1 + spec/mongo/server_selector/primary_spec.rb | 1 + .../secondary_preferred_spec.rb | 1 + spec/mongo/server_selector/secondary_spec.rb | 1 + spec/support/shared/server_selector.rb | 20 +++++++++++- 7 files changed, 56 insertions(+), 1 deletion(-) diff --git a/spec/mongo/client_spec.rb b/spec/mongo/client_spec.rb index f8dacf5d2d..ec65522797 100644 --- a/spec/mongo/client_spec.rb +++ b/spec/mongo/client_spec.rb @@ -203,6 +203,23 @@ " { :mode => :primary }, + :database => TEST_DB, + :password => '123', + :user => 'emily' + ) + end + + it 'does not print out sensitive data' do + expect(client.inspect).not_to match('123') + end + end end describe '#initialize' do @@ -710,4 +727,19 @@ expect(client.reconnect).to be(true) end end + + describe '#dup' do + + let(:client) do + described_class.new( + ['127.0.0.1:27017'], + :read => { :mode => :primary }, + :database => TEST_DB + ) + end + + it 'creates a client with Redacted options' do + expect(client.dup.options).to be_a(Mongo::Options::Redacted) + end + end end diff --git a/spec/mongo/server_selector/nearest_spec.rb b/spec/mongo/server_selector/nearest_spec.rb index ee6daed3c3..26c4d2b4ec 100644 --- a/spec/mongo/server_selector/nearest_spec.rb +++ b/spec/mongo/server_selector/nearest_spec.rb @@ -11,6 +11,7 @@ end it_behaves_like 'a server selector accepting tag sets' + it_behaves_like 'a server selector with sensitive data in its options' describe '#to_mongos' do diff --git a/spec/mongo/server_selector/primary_preferred_spec.rb b/spec/mongo/server_selector/primary_preferred_spec.rb index 1fd9f2ab56..be25b7f3f6 100644 --- a/spec/mongo/server_selector/primary_preferred_spec.rb +++ b/spec/mongo/server_selector/primary_preferred_spec.rb @@ -11,6 +11,7 @@ end it_behaves_like 'a server selector accepting tag sets' + it_behaves_like 'a server selector with sensitive data in its options' describe '#to_mongos' do diff --git a/spec/mongo/server_selector/primary_spec.rb b/spec/mongo/server_selector/primary_spec.rb index 6a37ad6025..810f0a0a1d 100644 --- a/spec/mongo/server_selector/primary_spec.rb +++ b/spec/mongo/server_selector/primary_spec.rb @@ -9,6 +9,7 @@ it_behaves_like 'a server selector mode' do let(:slave_ok) { false } end + it_behaves_like 'a server selector with sensitive data in its options' describe '#tag_sets' do diff --git a/spec/mongo/server_selector/secondary_preferred_spec.rb b/spec/mongo/server_selector/secondary_preferred_spec.rb index a0888f07a8..117427adb6 100644 --- a/spec/mongo/server_selector/secondary_preferred_spec.rb +++ b/spec/mongo/server_selector/secondary_preferred_spec.rb @@ -9,6 +9,7 @@ it_behaves_like 'a server selector mode' do let(:slave_ok) { true } end + it_behaves_like 'a server selector with sensitive data in its options' it_behaves_like 'a server selector accepting tag sets' diff --git a/spec/mongo/server_selector/secondary_spec.rb b/spec/mongo/server_selector/secondary_spec.rb index 55f36ae70e..e53edf277d 100644 --- a/spec/mongo/server_selector/secondary_spec.rb +++ b/spec/mongo/server_selector/secondary_spec.rb @@ -9,6 +9,7 @@ it_behaves_like 'a server selector mode' do let(:slave_ok) { true } end + it_behaves_like 'a server selector with sensitive data in its options' it_behaves_like 'a server selector accepting tag sets' diff --git a/spec/support/shared/server_selector.rb b/spec/support/shared/server_selector.rb index a625cf6a2c..a76efc0891 100644 --- a/spec/support/shared/server_selector.rb +++ b/spec/support/shared/server_selector.rb @@ -32,7 +32,8 @@ def server(mode, options = {}) end let(:primary) { server(:primary) } let(:secondary) { server(:secondary) } - let(:selector) { described_class.new(:mode => name, :tag_sets => tag_sets) } + let(:options) { { :mode => name, :tag_sets => tag_sets } } + let(:selector) { described_class.new(options) } before(:all) do module Mongo @@ -157,3 +158,20 @@ class Server end end end + +shared_examples 'a server selector with sensitive data in its options' do + + describe '#inspect' do + + context 'when there is sensitive data in the options' do + + let(:options) do + Mongo::Options::Redacted.new(:mode => name, :password => '123') + end + + it 'does not print out sensitive data' do + expect(selector.inspect).not_to match(options[:password]) + end + end + end +end From ca239e5de4c72eb94cff62373d4670208d15f2b3 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Wed, 9 Sep 2015 12:34:44 -0400 Subject: [PATCH 09/15] RUBY-1021 initialize read preference with redacted options in FSBucket --- lib/mongo/grid/fs_bucket.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/mongo/grid/fs_bucket.rb b/lib/mongo/grid/fs_bucket.rb index a86b587d1f..aeb24e4849 100644 --- a/lib/mongo/grid/fs_bucket.rb +++ b/lib/mongo/grid/fs_bucket.rb @@ -57,6 +57,8 @@ class FSBucket attr_reader :options # Get client from the database. + # + # @since 2.1.0 def_delegators :database, :client @@ -400,7 +402,7 @@ def upload_from_stream(filename, io, opts = {}) # @since 2.1.0 def read_preference @read_preference ||= @options[:read] ? - ServerSelector.get(client.options.merge(@options[:read] || {})) : + ServerSelector.get(Options::Redacted.new((@options[:read] || {}).merge(client.options))) : database.read_preference end From 2748bf51cde8351771c4ec1fd9fdb459d425f25a Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Wed, 9 Sep 2015 16:29:01 -0400 Subject: [PATCH 10/15] RUBY-1021 use more unique string for password in tests --- spec/mongo/client_spec.rb | 4 ++-- spec/mongo/options/redacted_spec.rb | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/mongo/client_spec.rb b/spec/mongo/client_spec.rb index ec65522797..405af9e464 100644 --- a/spec/mongo/client_spec.rb +++ b/spec/mongo/client_spec.rb @@ -211,13 +211,13 @@ ['127.0.0.1:27017'], :read => { :mode => :primary }, :database => TEST_DB, - :password => '123', + :password => 'some_password', :user => 'emily' ) end it 'does not print out sensitive data' do - expect(client.inspect).not_to match('123') + expect(client.inspect).not_to match('some_password') end end end diff --git a/spec/mongo/options/redacted_spec.rb b/spec/mongo/options/redacted_spec.rb index 31af0c5eb7..26d5a4c2df 100644 --- a/spec/mongo/options/redacted_spec.rb +++ b/spec/mongo/options/redacted_spec.rb @@ -11,11 +11,11 @@ context 'when the hash contains a sensitive key' do let(:original_opts) do - { password: '123' } + { password: 'sensitive_data' } end it 'replaces the value with the redacted string' do - expect(options.to_s).not_to match(/123/) + expect(options.to_s).not_to match(original_opts[:password]) end it 'replaces the value with the redacted string' do @@ -36,11 +36,11 @@ context 'when the hash contains a sensitive key' do let(:original_opts) do - { password: '123' } + { password: 'sensitive_data' } end it 'replaces the value with the redacted string' do - expect(options.inspect).not_to match(/123/) + expect(options.inspect).not_to match(original_opts[:password]) end it 'replaces the value with the redacted string' do @@ -51,11 +51,11 @@ context 'when the hash does not contain a sensitive key' do let(:original_opts) do - { name: '123' } + { name: 'some_name' } end it 'does not replace the value with the redacted string' do - expect(options.inspect).to match(/123/) + expect(options.inspect).to match(original_opts[:name]) end it 'does not replace the value with the redacted string' do From 866d024c93578ab090800c4b9ae7a3df1081ed10 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Thu, 10 Sep 2015 11:36:23 -0400 Subject: [PATCH 11/15] RUBY-1021 implement #select and #reject for Options::Redacted --- lib/mongo/options/redacted.rb | 74 ++++++++++ spec/mongo/options/redacted_spec.rb | 214 ++++++++++++++++++++++++++++ 2 files changed, 288 insertions(+) diff --git a/lib/mongo/options/redacted.rb b/lib/mongo/options/redacted.rb index 2475375939..a3cdc4faa1 100644 --- a/lib/mongo/options/redacted.rb +++ b/lib/mongo/options/redacted.rb @@ -65,6 +65,80 @@ def has_key?(key) end alias_method :key?, :has_key? + # Returns a new options object consisting of pairs for which the block returns false. + # + # @example Get a new options object with pairs for which the block returns false. + # new_options = options.reject { |k, v| k == 'database' } + # + # @yieldparam [ String, Object ] The key as a string and its value. + # + # @return [ Options::Redacted ] A new options object. + # + # @since 2.1.0 + def reject(&block) + new_options = dup + new_options.reject!(&block) || new_options + end + + # Only keeps pairs for which the block returns false. + # + # @example Remove pairs from this object for which the block returns true. + # options.reject! { |k, v| k == 'database' } + # + # @yieldparam [ String, Object ] The key as a string and its value. + # + # @return [ Options::Redacted ] This object. + # + # @since 2.1.0 + def reject! + if block_given? + n_keys = keys.size + self.keys.each do |key| + self.delete(key) if yield(key, self[key]) + end + n_keys == keys.size ? nil : self + else + self.to_enum + end + end + + # Returns a new options object consisting of pairs for which the block returns true. + # + # @example Get a new options object with pairs for which the block returns true. + # ssl_options = options.select { |k, v| k =~ /ssl/ } + # + # @yieldparam [ String, Object ] The key as a string and its value. + # + # @return [ Options::Redacted ] A new options object. + # + # @since 2.1.0 + def select(&block) + new_options = dup + new_options.select!(&block) || new_options + end + + # Only keeps pairs for which the block returns true. + # + # @example Remove pairs from this object for which the block does not return true. + # options.select! { |k, v| k =~ /ssl/ } + # + # @yieldparam [ String, Object ] The key as a string and its value. + # + # @return [ Options::Redacted ] This object. + # + # @since 2.1.0 + def select! + if block_given? + n_keys = keys.size + self.keys.each do |key| + self.delete(key) unless yield(key, self[key]) + end + n_keys == keys.size ? nil : self + else + self.to_enum + end + end + private def redacted_string(method) diff --git a/spec/mongo/options/redacted_spec.rb b/spec/mongo/options/redacted_spec.rb index 26d5a4c2df..b254c35562 100644 --- a/spec/mongo/options/redacted_spec.rb +++ b/spec/mongo/options/redacted_spec.rb @@ -129,4 +129,218 @@ end end end + + describe '#reject' do + + let(:options) do + described_class.new(a: 1, b: 2, c: 3) + end + + context 'when no block is provided' do + + it 'returns an enumerable' do + expect(options.reject).to be_a(Enumerator) + end + end + + context 'when a block is provided' do + + context 'when the block evaluates to true for some pairs' do + + let(:result) do + options.reject { |k,v| k == 'a' } + end + + it 'returns an object consisting of only the remaining pairs' do + expect(result).to eq(described_class.new(b: 2, c: 3)) + end + + it 'returns a new object' do + expect(result).not_to be(options) + end + end + + context 'when the block does not evaluate to true for any pairs' do + + let(:result) do + options.reject { |k,v| k == 'd' } + end + + it 'returns an object with all pairs intact' do + expect(result).to eq(described_class.new(a: 1, b: 2, c: 3)) + end + + it 'returns a new object' do + expect(result).not_to be(options) + end + end + end + end + + describe '#reject!' do + + let(:options) do + described_class.new(a: 1, b: 2, c: 3) + end + + context 'when no block is provided' do + + it 'returns an enumerable' do + expect(options.reject).to be_a(Enumerator) + end + end + + context 'when a block is provided' do + + context 'when the block evaluates to true for some pairs' do + + let(:result) do + options.reject! { |k,v| k == 'a' } + end + + it 'returns an object consisting of only the remaining pairs' do + expect(result).to eq(described_class.new(b: 2, c: 3)) + end + + it 'returns the same object' do + expect(result).to be(options) + end + end + + context 'when the block does not evaluate to true for any pairs' do + + let(:result) do + options.reject! { |k,v| k == 'd' } + end + + it 'returns nil' do + expect(result).to be(nil) + end + end + end + end + + describe '#select' do + + let(:options) do + described_class.new(a: 1, b: 2, c: 3) + end + + context 'when no block is provided' do + + it 'returns an enumerable' do + expect(options.reject).to be_a(Enumerator) + end + end + + context 'when a block is provided' do + + context 'when the block evaluates to true for some pairs' do + + let(:result) do + options.select { |k,v| k == 'a' } + end + + it 'returns an object consisting of those pairs' do + expect(result).to eq(described_class.new(a: 1)) + end + + it 'returns a new object' do + expect(result).not_to be(options) + end + end + + context 'when the block does not evaluate to true for any pairs' do + + let(:result) do + options.select { |k,v| k == 'd' } + end + + it 'returns an object with no pairs' do + expect(result).to eq(described_class.new) + end + + it 'returns a new object' do + expect(result).not_to be(options) + end + end + + context 'when the object is unchanged' do + + let(:options) do + described_class.new(a: 1, b: 2, c: 3) + end + + let(:result) do + options.select { |k,v| ['a', 'b', 'c'].include?(k) } + end + + it 'returns a new object' do + expect(result).to eq(described_class.new(a: 1, b: 2, c: 3)) + end + end + end + end + + describe '#select!' do + + let(:options) do + described_class.new(a: 1, b: 2, c: 3) + end + + context 'when no block is provided' do + + it 'returns an enumerable' do + expect(options.reject).to be_a(Enumerator) + end + end + + context 'when a block is provided' do + + context 'when the block evaluates to true for some pairs' do + + let(:result) do + options.select! { |k,v| k == 'a' } + end + + it 'returns an object consisting of those pairs' do + expect(result).to eq(described_class.new(a: 1)) + end + + it 'returns the same object' do + expect(result).to be(options) + end + end + + context 'when the block does not evaluate to true for any pairs' do + + let(:result) do + options.select! { |k,v| k == 'd' } + end + + it 'returns an object with no pairs' do + expect(result).to eq(described_class.new) + end + + it 'returns the same object' do + expect(result).to be(options) + end + end + + context 'when the object is unchanged' do + + let(:options) do + described_class.new(a: 1, b: 2, c: 3) + end + + let(:result) do + options.select! { |k,v| ['a', 'b', 'c'].include?(k) } + end + + it 'returns nil' do + expect(result).to be(nil) + end + end + end + end end \ No newline at end of file From 42b62830de300780b83adc9150d20c4e2f73cfb8 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Thu, 10 Sep 2015 11:47:04 -0400 Subject: [PATCH 12/15] RUBY-1021 more unique password string --- spec/support/shared/server_selector.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/shared/server_selector.rb b/spec/support/shared/server_selector.rb index a76efc0891..d2e4ea8ced 100644 --- a/spec/support/shared/server_selector.rb +++ b/spec/support/shared/server_selector.rb @@ -166,7 +166,7 @@ class Server context 'when there is sensitive data in the options' do let(:options) do - Mongo::Options::Redacted.new(:mode => name, :password => '123') + Mongo::Options::Redacted.new(:mode => name, :password => 'sensitive_data') end it 'does not print out sensitive data' do From 0a33b8899d8bbd2a8d0d4e0345c756ac29111ce8 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Thu, 10 Sep 2015 13:53:48 -0400 Subject: [PATCH 13/15] RUBY-1021 Take out unnecessary self refs --- lib/mongo/options/redacted.rb | 16 ++++++++-------- lib/mongo/server_selector.rb | 2 +- spec/mongo/options/redacted_spec.rb | 6 +++++- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/mongo/options/redacted.rb b/lib/mongo/options/redacted.rb index a3cdc4faa1..ff4fefdd21 100644 --- a/lib/mongo/options/redacted.rb +++ b/lib/mongo/options/redacted.rb @@ -25,12 +25,12 @@ class Redacted < BSON::Document # # @since 2.1.0 SENSITIVE_OPTIONS = [ :password, - :pwd ] + :pwd ].freeze # The replacement string used in place of the value for sensitive keys. # # @since 2.1.0 - STRING_REPLACEMENT = '' + STRING_REPLACEMENT = ''.freeze # Get a string representation of the options. # @@ -93,12 +93,12 @@ def reject(&block) def reject! if block_given? n_keys = keys.size - self.keys.each do |key| - self.delete(key) if yield(key, self[key]) + keys.each do |key| + delete(key) if yield(key, self[key]) end n_keys == keys.size ? nil : self else - self.to_enum + to_enum end end @@ -130,12 +130,12 @@ def select(&block) def select! if block_given? n_keys = keys.size - self.keys.each do |key| - self.delete(key) unless yield(key, self[key]) + keys.each do |key| + delete(key) unless yield(key, self[key]) end n_keys == keys.size ? nil : self else - self.to_enum + to_enum end end diff --git a/lib/mongo/server_selector.rb b/lib/mongo/server_selector.rb index eea35dab5e..921af8f221 100644 --- a/lib/mongo/server_selector.rb +++ b/lib/mongo/server_selector.rb @@ -41,7 +41,7 @@ module ServerSelector # Primary read preference. # # @since 2.1.0 - PRIMARY = Options::Redacted.new(mode: :primary) + PRIMARY = Options::Redacted.new(mode: :primary).freeze # Hash lookup for the selector classes based off the symbols # provided in configuration. diff --git a/spec/mongo/options/redacted_spec.rb b/spec/mongo/options/redacted_spec.rb index b254c35562..6745e50051 100644 --- a/spec/mongo/options/redacted_spec.rb +++ b/spec/mongo/options/redacted_spec.rb @@ -25,8 +25,12 @@ context 'when the hash does not contain a sensitive key' do - it 'prints all the values' do + let(:original_opts) do + { user: 'emily' } + end + it 'prints all the values' do + expect(options.to_s).to match(original_opts[:user]) end end end From 25a87b9db61e77d189bd28edda6999a04df79a5a Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Thu, 10 Sep 2015 14:16:23 -0400 Subject: [PATCH 14/15] RUBY-1021 Document #reject! and #select! returning nil if unchanged --- lib/mongo/options/redacted.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mongo/options/redacted.rb b/lib/mongo/options/redacted.rb index ff4fefdd21..1a557eb2f9 100644 --- a/lib/mongo/options/redacted.rb +++ b/lib/mongo/options/redacted.rb @@ -87,7 +87,7 @@ def reject(&block) # # @yieldparam [ String, Object ] The key as a string and its value. # - # @return [ Options::Redacted ] This object. + # @return [ Options::Redacted, nil ] This object or nil if no changes were made. # # @since 2.1.0 def reject! @@ -124,7 +124,7 @@ def select(&block) # # @yieldparam [ String, Object ] The key as a string and its value. # - # @return [ Options::Redacted ] This object. + # @return [ Options::Redacted, nil ] This object or nil if no changes were made. # # @since 2.1.0 def select! From 01c971861a589be09dbbb69a276b57af1739dd88 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Thu, 10 Sep 2015 14:18:44 -0400 Subject: [PATCH 15/15] RUBY-1021 fix spec typos --- spec/mongo/options/redacted_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/mongo/options/redacted_spec.rb b/spec/mongo/options/redacted_spec.rb index 6745e50051..0cfed16c34 100644 --- a/spec/mongo/options/redacted_spec.rb +++ b/spec/mongo/options/redacted_spec.rb @@ -78,7 +78,7 @@ context 'when the method argument is a String' do - it 'returns true when ' do + it 'returns true' do expect(options.has_key?('name')).to be(true) end end @@ -99,7 +99,7 @@ context 'when the method argument is a String' do - it 'returns true when ' do + it 'returns true' do expect(options.has_key?('name')).to be(true) end end @@ -120,14 +120,14 @@ context 'when the method argument is a String' do - it 'returns true when ' do + it 'returns false' do expect(options.has_key?('name')).to be(false) end end context 'when method argument is a Symbol' do - it 'returns true' do + it 'returns false' do expect(options.has_key?(:name)).to be(false) end end