diff --git a/CHANGE_LOG.md b/CHANGE_LOG.md index 028c0c4..1da9f13 100644 --- a/CHANGE_LOG.md +++ b/CHANGE_LOG.md @@ -1,4 +1,14 @@ -# Change log +# Change Log + +## 1.1.0 + +* Use `to_json` if it is defined when serializing encrypted args to JSON. +* Add client middleware to the server default configuration. This ensures that arguments will be encrypted if a worker enqueues a job with encrypted arguments. +* Client middleware now reads sidekiq options from the job hash instead of from the worker class so that the list of encrypted arguments is always in sync on the job payload. +* Don't blow up if class name that is not defined is passed to client middleware. +* Added additional option to specify encrypted args with array of argument indexes. +* Deprecated setting encrypted args as hash or array of booleans. +* Client middleware is prepended while server middleware is appended. ## 1.0.2 diff --git a/Gemfile.lock b/Gemfile.lock index a26122d..f11d134 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,18 +1,18 @@ PATH remote: . specs: - sidekiq-encrypted_args (1.0.2) + sidekiq-encrypted_args (1.1.0) secret_keys sidekiq (>= 4.0) GEM remote: https://rubygems.org/ specs: - appraisal (2.2.0) + appraisal (2.3.0) bundler rake thor (>= 0.14.0) - ast (2.4.0) + ast (2.4.1) climate_control (0.2.0) connection_pool (2.2.3) diff-lcs (1.3) @@ -24,7 +24,7 @@ GEM rack rainbow (3.0.0) rake (13.0.1) - redis (4.1.4) + redis (4.2.1) rexml (3.2.4) rspec (3.9.0) rspec-core (~> 3.9.0) diff --git a/README.md b/README.md index e6489ee..87694cd 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ This can be an even bigger issue if you use scheduled jobs since sensitive data ## Solution -This gem adds Sidekiq middleware that allows you to specify job arguments for your workers that should be encrypted in Redis. You do this by adding `encrypted_args` to the `sidekiq_options` in the worker. Jobs for these workers will have their arguments encrypted in Redis and decrypted when passed to `perform` method. +This gem adds Sidekiq middleware that allows you to specify job arguments for your workers that should be encrypted in Redis. You do this by adding `encrypted_args` to the `sidekiq_options` in the worker. Jobs for these workers will have their arguments encrypted in Redis and decrypted when passed to the `perform` method. To use the gem, you will need to specify a secret that will be used to encrypt the arguments as well as add the middleware to your Sidekiq client and server middleware stacks. You can set that up by adding this to the end of your Sidekiq initialization: @@ -24,14 +24,16 @@ Sidekiq::EncryptedArgs.configure!(secret: "YourSecretKey") If the secret is not set, the value of the `SIDEKIQ_ENCRYPTED_ARGS_SECRET` environment variable will be used as the secret. If this variable is not set, job arguments will not be encrypted. -The call to `Sidekiq::EncryptedArgs.configure!` will append the encryption middleware to the end of the client and server middleware chains. You add the middlewares manually if you need more control over where they appear in the stacks. +The call to `Sidekiq::EncryptedArgs.configure!` will **prepend** the client encryption middleware and **append** server decryption middleware. By doing this, any other middleware you register will only receive the encrypted parameters (e.g. logging middleware will receive the encrypted parameters). + +You can add the middleware manually if you need more control over where they appear in the stacks. ```ruby Sidekiq::EncryptedArgs.secret = "YourSecretKey" Sidekiq.configure_client do |config| config.client_middleware do |chain| - chain.add Sidekiq::EncryptedArgs::ClientMiddleware + chain.prepend Sidekiq::EncryptedArgs::ClientMiddleware end end @@ -39,6 +41,12 @@ Sidekiq.configure_server do |config| config.server_middleware do |chain| chain.add Sidekiq::EncryptedArgs::ServerMiddleware end + + # register client middleware on the server so that starting jobs in a Sidekiq::Worker also get encrypted args + # https://github.com/mperham/sidekiq/wiki/Middleware#client-middleware-registered-in-both-places + config.client_middleware do |chain| + chain.prepend Sidekiq::EncryptedArgs::ClientMiddleware + end end ``` @@ -59,30 +67,24 @@ class SecretWorker end ``` -You can also encrypt just specific arguments with a hash or an array. This can be useful to preserve visibility into non-sensitive arguments that might be useful for troubleshooting or other reasons. All of these examples will encrypt just the second argument to the `perform` method. - -```ruby - # Pass in a list of argument names that should be encrypted - sidekiq_options encrypted_args: [:arg_2] - - def perform(arg_1, arg_2, arg_3) - end -``` +You can also choose to only encrypt specific arguments with an array of either argument names (symbols or strings) or indexes. This is useful to preserve visibility into non-sensitive arguments for troubleshooting or other reasons. Both of these examples encrypt just the second argument to the `perform` method. ```ruby - # Pass in a hash with values indicating which arguments should be encrypted - sidekiq_options encrypted_args: { arg_2: true, arg_1: false } +# Pass in a list of argument names that should be encrypted +sidekiq_options encrypted_args: [:arg_2] +# or +sidekiq_options encrypted_args: ["arg_2"] - def perform(arg_1, arg_2, arg_3) - end +def perform(arg_1, arg_2, arg_3) +end ``` ```ruby - # Pass in an array of boolean values indicating which argument positions should be encrypted - sidekiq_options encrypted_args: [false, true] +# Pass in an array of integers indicating which argument positions should be encrypted +sidekiq_options encrypted_args: [1] - def perform(arg_1, arg_2, arg_3) - end +def perform(arg_1, arg_2, arg_3) +end ``` You don't need to change anything else about your workers. All of the arguments passed to the `perform` method will already be unencrypted when the method is called. @@ -92,15 +94,15 @@ You don't need to change anything else about your workers. All of the arguments If you need to roll your secret, you can simply provide an array when setting the secret. ```ruby -Sidekiq::EncryptedArgs.secret = ["CurrentSecret", "OldSecret"] +Sidekiq::EncryptedArgs.secret = ["CurrentSecret", "OldSecret", "EvenOlderSecret"] ``` -The left most key will be considered the current key and will be used for encrypting arguments. However, all of the keys will be tried when decrypting. This allows you to switch you secret keys without breaking jobs already enqueued in Redis. +The first (left most) key will be considered the current key, and is used for encrypting arguments. When decrypting, we iterate over the secrets list until we find the correct one. This allows you to switch you secret keys without breaking jobs already enqueued in Redis. -If you are using the `SIDEKIQ_ENCRYPTED_ARGS_SECRET` envrionment variable to specify your secret, you can delimit multiple keys with a spaces. +If you are using the `SIDEKIQ_ENCRYPTED_ARGS_SECRET` environment variable to specify your secret, you can delimit multiple keys with a spaces. You can also safely add encryption to an existing worker. Any jobs that are already enqueued will still run even without having the arguments encrypted in Redis. ## Encryption -Encrypted arguments are stored using AES-256-GCM with a key derived from your secret using PBKDF2. +Encrypted arguments are stored using AES-256-GCM with a key derived from your secret using PBKDF2. For more info on the underlying encryption, refer to the [SecretKeys](https://github.com/bdurand/secret_keys) gem. diff --git a/VERSION b/VERSION index 6d7de6e..9084fa2 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.0.2 +1.1.0 diff --git a/lib/sidekiq/encrypted_args.rb b/lib/sidekiq/encrypted_args.rb index 7d3140e..533b442 100644 --- a/lib/sidekiq/encrypted_args.rb +++ b/lib/sidekiq/encrypted_args.rb @@ -6,8 +6,11 @@ module Sidekiq module EncryptedArgs - # Error thrown when the + # Error thrown when the secret is invalid class InvalidSecretError < StandardError + def initialize + super("Cannot decrypt. Invalid secret provided.") + end end class << self @@ -15,27 +18,32 @@ class << self # the value will be loaded from the `SIDEKIQ_ENCRYPTED_ARGS_SECRET` environment # variable. If that value is not set, arguments will not be encrypted. # - # @param [String] value One or more secrets to use for encrypting arguments. - # - # @note You can set multiple secrets by passing an array if you need to roll your secrets. + # You can set multiple secrets by passing an array if you need to roll your secrets. # The left most value in the array will be used as the encryption secret, but # all the values will be tried when decrypting. That way if you have scheduled # jobs that were encrypted with a different secret, you can still make it available # when decrypting the arguments when the job gets run. If you are using the # environment variable, separate the keys with spaces. + # + # @param [String] value One or more secrets to use for encrypting arguments. + # @return [void] def secret=(value) @encryptors = make_encryptors(value) end - # Calling this method will add the client and server middleware to the Sidekiq + # Add the client and server middleware to the Sidekiq # middleware chains. If you need to ensure the order of where the middleware is # added, you can forgo this method and add it yourself. + # + # This method prepends client middleware and appends server middleware. + # + # @param [String] secret optionally set the secret here. See {.secret=} def configure!(secret: nil) self.secret = secret unless secret.nil? Sidekiq.configure_client do |config| config.client_middleware do |chain| - chain.add Sidekiq::EncryptedArgs::ClientMiddleware + chain.prepend Sidekiq::EncryptedArgs::ClientMiddleware end end @@ -43,17 +51,20 @@ def configure!(secret: nil) config.server_middleware do |chain| chain.add Sidekiq::EncryptedArgs::ServerMiddleware end + config.client_middleware do |chain| + chain.prepend Sidekiq::EncryptedArgs::ClientMiddleware + end end end # Encrypt a value. # - # @param [Object] data Data to encrypt. You can pass any JSON compatible data types or structures. + # @param [#to_json, Object] data Data to encrypt. You can pass any JSON compatible data types or structures. # # @return [String] def encrypt(data) return nil if data.nil? - json = JSON.dump(data) + json = (data.respond_to?(:to_json) ? data.to_json : JSON.generate(data)) encrypted = encrypt_string(json) if encrypted == json data @@ -67,37 +78,55 @@ def encrypt(data) # @param [String] encrypted_data Data that was previously encrypted. If the value passed in is # an unencrypted string, then the string itself will be returned. # - # @return [String] + # @return [Object] def decrypt(encrypted_data) return encrypted_data unless SecretKeys::Encryptor.encrypted?(encrypted_data) json = decrypt_string(encrypted_data) JSON.parse(json) end - protected - - # Helper method to get the encrypted args option from an options hash. The value of this option + # Private helper method to get the encrypted args option from an options hash. The value of this option # can be `true` or an array indicating if each positional argument should be encrypted, or a hash # with keys for the argument position and true as the value. - def encrypted_args_option(worker_class) - sidekiq_options = worker_class.sidekiq_options - option = sidekiq_options.fetch(:encrypted_args, sidekiq_options["encrypted_args"]) - + # + # @private + def encrypted_args_option(worker_class, job) + option = job["encrypted_args"] return nil if option.nil? - - return Hash.new(true) if option == true - - return replace_argument_positions(worker_class, option) if option.is_a?(Hash) - - hash = {} - Array(option).each_with_index do |val, position| - if val.is_a?(Symbol) || val.is_a?(String) - hash[val] = true - else - hash[position] = val + return [] if option == false + + indexes = [] + if option == true + job["args"].size.times { |i| indexes << i } + elsif option.is_a?(Hash) + deprecation_warning("hash") + indexes = replace_argument_positions(worker_class, option) + else + array_type = nil + deprecation_message = nil + Array(option).each_with_index do |val, position| + current_type = nil + if val.is_a?(Integer) + indexes << val + current_type = :integer + elsif val.is_a?(Symbol) || val.is_a?(String) + worker_class = constantize(worker_class) if worker_class.is_a?(String) + position = perform_method_parameter_index(worker_class, val) + indexes << position if position + current_type = :symbol + else + deprecation_message = "boolean array" + indexes << position if val + end + if array_type && current_type + deprecation_message = "array of mixed types" + else + array_type ||= current_type + end end + deprecation_warning(deprecation_message) if deprecation_message end - replace_argument_positions(worker_class, hash) + indexes end private @@ -135,18 +164,40 @@ def make_encryptors(secrets) Array(secrets).map { |val| val.nil? ? nil : SecretKeys::Encryptor.from_password(val, SALT) } end - def replace_argument_positions(worker_class, encrypt_option) - updated = {} - encrypt_option.each do |key, value| + def deprecation_warning(message) + warn("Sidekiq::EncryptedArgs: setting encrypted_args to #{message} is deprecated; support will be removed in version 1.2.") + end + + # @param [String] class_name name of a class + # @return [Class] class that was referenced by name + def constantize(class_name) + names = class_name.split("::") + # Clear leading :: for root namespace since we're already calling from object + names.shift if names.empty? || names.first.empty? + # Map reduce to the constant. Use inherit=false to not accidentally search + # parent modules + names.inject(Object) { |constant, name| constant.const_get(name, false) } + end + + def replace_argument_positions(worker_class, encrypt_option_hash) + encrypted_indexes = [] + encrypt_option_hash.each do |key, value| + next unless value if key.is_a?(Symbol) || key.is_a?(String) - key = key.to_sym - position = worker_class.instance_method(:perform).parameters.find_index { |_, name| name == key } - updated[position] = value if position + position = perform_method_parameter_index(worker_class, key) + encrypted_indexes << position if position elsif key.is_a?(Integer) - updated[key] = value + encrypted_indexes << key end end - updated + encrypted_indexes + end + + def perform_method_parameter_index(worker_class, parameter) + if worker_class + parameter = parameter.to_sym + worker_class.instance_method(:perform).parameters.find_index { |_, name| name == parameter } + end end end end diff --git a/lib/sidekiq/encrypted_args/client_middleware.rb b/lib/sidekiq/encrypted_args/client_middleware.rb index c9fa819..af8a3a4 100644 --- a/lib/sidekiq/encrypted_args/client_middleware.rb +++ b/lib/sidekiq/encrypted_args/client_middleware.rb @@ -5,17 +5,11 @@ module EncryptedArgs # Sidekiq client middleware for encrypting arguments on jobs for workers # with `encrypted_args` set in the `sidekiq_options`. class ClientMiddleware - # @param [String, Class] worker_class class name or class of worker + # Encrypt specified arguments before they're sent off to the queue def call(worker_class, job, queue, redis_pool = nil) - worker_class = constantize(worker_class) if worker_class.is_a?(String) - encrypted_args = EncryptedArgs.send(:encrypted_args_option, worker_class) - if encrypted_args - new_args = [] - job["args"].each_with_index do |value, position| - value = EncryptedArgs.encrypt(value) if encrypted_args[position] - new_args << value - end - job["args"] = new_args + if job.include?("encrypted_args") + encrypted_args = EncryptedArgs.encrypted_args_option(worker_class, job) + encrypt_job_arguments!(job, encrypted_args) end yield @@ -23,15 +17,25 @@ def call(worker_class, job, queue, redis_pool = nil) private - # @param [String] class_name name of a class - # @return [Class] class that was referenced by name - def constantize(class_name) - names = class_name.split("::") - # Clear leading :: for root namespace since we're already calling from object - names.shift if names.empty? || names.first.empty? - # Map reduce to the constant. Use inherit=false to not accidentally search - # parent modules - names.inject(Object) { |constant, name| constant.const_get(name, false) } + # Encrypt the arguments on job + # + # Additionally, set `job["encrypted_args"]` to the canonicalized version (i.e. `Array`) + # + # @param [Hash] + # @param [Array] encrypted_args array of indexes in job to encrypt + # @return [void] + def encrypt_job_arguments!(job, encrypted_args) + if encrypted_args + job_args = job["args"] + job_args.each_with_index do |value, position| + if encrypted_args.include?(position) + job_args[position] = EncryptedArgs.encrypt(value) + end + end + job["encrypted_args"] = encrypted_args + else + job.delete("encrypted_args") + end end end end diff --git a/lib/sidekiq/encrypted_args/server_middleware.rb b/lib/sidekiq/encrypted_args/server_middleware.rb index 178d7e8..a7e38a9 100644 --- a/lib/sidekiq/encrypted_args/server_middleware.rb +++ b/lib/sidekiq/encrypted_args/server_middleware.rb @@ -2,22 +2,33 @@ module Sidekiq module EncryptedArgs + # Sidekiq server middleware for decrypting arguments on jobs that have encrypted args. class ServerMiddleware - # Sidekiq server middleware for encrypting arguments on jobs for workers - # with `encrypted_args` set in the `sidekiq_options`. + # Wrap the server process to decrypt incoming arguments def call(worker, job, queue) - encrypted_args = EncryptedArgs.send(:encrypted_args_option, worker.class) + encrypted_args = job["encrypted_args"] if encrypted_args - new_args = [] - job["args"].each_with_index do |value, position| - value = EncryptedArgs.decrypt(value) if encrypted_args[position] - new_args << value + encrypted_args = backward_compatible_encrypted_args(encrypted_args, worker.class, job) + job_args = job["args"] + encrypted_args.each do |position| + value = job_args[position] + job_args[position] = EncryptedArgs.decrypt(value) end - job["args"] = new_args end yield end + + private + + # Ensure that the encrypted args is an array of integers. If not re-read it from the class + # definition since gem version 1.0.2 and earlier did not update the encrypted_args on the job. + def backward_compatible_encrypted_args(encrypted_args, worker_class, job) + unless encrypted_args.is_a?(Array) && encrypted_args.all? { |position| position.is_a?(Integer) } + encrypted_args = EncryptedArgs.encrypted_args_option(worker_class, job) + end + encrypted_args + end end end end diff --git a/spec/encrypted_args/client_middleware_spec.rb b/spec/encrypted_args/client_middleware_spec.rb index 0edd01f..6c2464f 100644 --- a/spec/encrypted_args/client_middleware_spec.rb +++ b/spec/encrypted_args/client_middleware_spec.rb @@ -18,87 +18,140 @@ called = true end expect(called).to eq true + expect(job).to_not include("encrypted_args") expect(job["args"]).to eq args end it "should not encrypt arguments if the encrypted_args option is false" do called = false + job["encrypted_args"] = NotSecretWorker.sidekiq_options["encrypted_args"] middleware.call(NotSecretWorker, job, queue) do called = true end expect(called).to eq true + expect(job["encrypted_args"]).to match_array [] expect(job["args"]).to eq args end it "should encrypt all arguments if the encrypted_args option is true" do called = false + job["encrypted_args"] = SecretWorker.sidekiq_options["encrypted_args"] middleware.call(SecretWorker, job, queue) do called = true end expect(called).to eq true + expect(job["encrypted_args"]).to match_array [0, 1, 2] expect(job["args"].collect { |val| SecretKeys::Encryptor.encrypted?(val) }).to eq [true, true, true] end it "should support being called with a string class name" do called = false + job["encrypted_args"] = SecretWorker.sidekiq_options["encrypted_args"] middleware.call("SecretWorker", job, queue) do called = true end expect(called).to eq true + expect(job["encrypted_args"]).to match_array [0, 1, 2] expect(job["args"].collect { |val| SecretKeys::Encryptor.encrypted?(val) }).to eq [true, true, true] end it "should support scoped class names" do called = false + job["encrypted_args"] = Super::SecretWorker.sidekiq_options["encrypted_args"] middleware.call("Super::SecretWorker", job, queue) do called = true end expect(called).to eq true + expect(job["encrypted_args"]).to match_array [2] expect(job["args"].collect { |val| SecretKeys::Encryptor.encrypted?(val) }).to eq [false, false, true] end it "should support absolute class names" do called = false + job["encrypted_args"] = Super::SecretWorker.sidekiq_options["encrypted_args"] middleware.call("::Super::SecretWorker", job, queue) do called = true end expect(called).to eq true + expect(job["encrypted_args"]).to match_array [2] expect(job["args"].collect { |val| SecretKeys::Encryptor.encrypted?(val) }).to eq [false, false, true] end - it "should only encrypt arguments whose position index is set to true when the encrypted_args option is a hash" do + it "should just pass through worker classes that it can't instantiate" do called = false - middleware.call(HashOptionSecretWorker, job, queue) do + middleware.call("UndefinedWorker", job, queue) do called = true end expect(called).to eq true - expect(job["args"].collect { |val| SecretKeys::Encryptor.encrypted?(val) }).to eq [false, true, false] + expect(job["encrypted_args"]).to eq nil + expect(job["args"].collect { |val| SecretKeys::Encryptor.encrypted?(val) }).to eq [false, false, false] end - it "should only encrypt arguments whose position index is set to true when the encrypted_args option is an array" do + it "should honor job encrypted args on worker classes that it can't instantiate" do called = false - middleware.call(ArrayOptionSecretWorker, job, queue) do + job["encrypted_args"] = [1] + middleware.call("UndefinedWorker", job, queue) do called = true end expect(called).to eq true + expect(job["encrypted_args"]).to eq [1] expect(job["args"].collect { |val| SecretKeys::Encryptor.encrypted?(val) }).to eq [false, true, false] end - it "should only encrypt arguments whose names are provided in the encrypted_args option array" do + it "should only encrypt arguments whose position index is set to true when the encrypted_args option is an array" do called = false - middleware.call(NamedArrayOptionSecretWorker, job, queue) do + job["encrypted_args"] = ArrayIndexSecretWorker.sidekiq_options["encrypted_args"] + middleware.call(ArrayIndexSecretWorker, job, queue) do called = true end expect(called).to eq true + expect(job["encrypted_args"]).to match_array [1] expect(job["args"].collect { |val| SecretKeys::Encryptor.encrypted?(val) }).to eq [false, true, false] end - it "should only encrypt arguments whose names are set to true in the encrypted_args option hash" do + it "should only encrypt arguments whose names are provided in the encrypted_args option array" do called = false - middleware.call(NamedHashOptionSecretWorker, job, queue) do + job["encrypted_args"] = NamedArrayOptionSecretWorker.sidekiq_options["encrypted_args"] + middleware.call(NamedArrayOptionSecretWorker, job, queue) do called = true end expect(called).to eq true + expect(job["encrypted_args"]).to match_array [1] expect(job["args"].collect { |val| SecretKeys::Encryptor.encrypted?(val) }).to eq [false, true, false] end + + context "deprecated configurations", :no_warn do + it "should only encrypt arguments whose position index is set to true when the encrypted_args option is a hash" do + called = false + job["encrypted_args"] = HashOptionSecretWorker.sidekiq_options["encrypted_args"] + middleware.call(HashOptionSecretWorker, job, queue) do + called = true + end + expect(called).to eq true + expect(job["encrypted_args"]).to match_array [1] + expect(job["args"].collect { |val| SecretKeys::Encryptor.encrypted?(val) }).to eq [false, true, false] + end + + it "should only encrypt arguments whose position index is set to true when the encrypted_args option is an array" do + called = false + job["encrypted_args"] = ArrayOptionSecretWorker.sidekiq_options["encrypted_args"] + middleware.call(ArrayOptionSecretWorker, job, queue) do + called = true + end + expect(called).to eq true + expect(job["encrypted_args"]).to match_array [1] + expect(job["args"].collect { |val| SecretKeys::Encryptor.encrypted?(val) }).to eq [false, true, false] + end + + it "should only encrypt arguments whose names are set to true in the encrypted_args option hash" do + called = false + job["encrypted_args"] = NamedHashOptionSecretWorker.sidekiq_options["encrypted_args"] + middleware.call(NamedHashOptionSecretWorker, job, queue) do + called = true + end + expect(called).to eq true + expect(job["encrypted_args"]).to match_array [1] + expect(job["args"].collect { |val| SecretKeys::Encryptor.encrypted?(val) }).to eq [false, true, false] + end + end end diff --git a/spec/encrypted_args/server_middleware_spec.rb b/spec/encrypted_args/server_middleware_spec.rb index 47ac4e3..9eeb6d0 100644 --- a/spec/encrypted_args/server_middleware_spec.rb +++ b/spec/encrypted_args/server_middleware_spec.rb @@ -12,7 +12,7 @@ Sidekiq::EncryptedArgs.secret = "key" end - it "should not decrypt arguments on a regular worker" do + it "should not decrypt arguments if the encrypted_args option is missing" do called = false middleware.call(RegularWorker.new, job, queue) do called = true @@ -21,83 +21,117 @@ expect(job["args"]).to eq args end - it "should not decrypt arguments if the encrypted_args option is false" do + it "should not decrypt arguments if the encrypted_args option is empty" do called = false - middleware.call(NotSecretWorker.new, job, queue) do + job["encrypted_args"] = [] + middleware.call(RegularWorker.new, job, queue) do called = true end expect(called).to eq true expect(job["args"]).to eq args end - it "should decrypt all arguments if the encrypted_args option is true" do + it "should decrypt arguments at the positions specified in the encrypted_args option" do called = false job["args"] = args.collect { |arg| Sidekiq::EncryptedArgs.encrypt(arg) } - middleware.call(SecretWorker.new, job, queue) do + job["encrypted_args"] = [1, 2] + middleware.call(RegularWorker.new, job, queue) do called = true end expect(called).to eq true - expect(job["args"]).to eq args + expect(job["args"][0]).to_not eq args[0] + expect(Sidekiq::EncryptedArgs.decrypt(job["args"][0])).to eq args[0] + expect(job["args"][1]).to eq args[1] + expect(job["args"][2]).to eq args[2] end - it "should only decrypt arguments whose position index is set to true when the encrypted_args option is a hash" do + it "should try multiple keys to decrypt arguments to support rolling keys" do called = false - job["args"] = [args[0], Sidekiq::EncryptedArgs.encrypt(args[1]), args[2]] - middleware.call(HashOptionSecretWorker.new, job, queue) do + Sidekiq::EncryptedArgs.secret = "old" + job["args"] = args.collect { |arg| Sidekiq::EncryptedArgs.encrypt(arg) } + job["encrypted_args"] = [0, 1, 2] + Sidekiq::EncryptedArgs.secret = ["new", "old"] + middleware.call(SecretWorker.new, job, queue) do called = true end expect(called).to eq true expect(job["args"]).to eq args end - it "should only decrypt arguments whose position index is set to true when the encrypted_args option is an array" do + it "should pass through keys that should be encrypted but were not encrypted" do called = false - job["args"] = [args[0], Sidekiq::EncryptedArgs.encrypt(args[1]), args[2]] - middleware.call(ArrayOptionSecretWorker.new, job, queue) do + job["encrypted_args"] = [1] + middleware.call(SecretWorker.new, job, queue) do called = true end expect(called).to eq true expect(job["args"]).to eq args end - it "should only decrypt arguments whose names are provided in the encrypted_args option array" do - called = false - job["args"] = [args[0], Sidekiq::EncryptedArgs.encrypt(args[1]), args[2]] - middleware.call(NamedArrayOptionSecretWorker.new, job, queue) do - called = true + context "when encrypted_args is set but not normalized to an array of argument positions", :no_warn do + it "should not decrypt arguments if the encrypted_args option is false" do + called = false + job["encrypted_args"] = NotSecretWorker.sidekiq_options["encrypted_args"] + middleware.call(NotSecretWorker.new, job, queue) do + called = true + end + expect(called).to eq true + expect(job["args"]).to eq args end - expect(called).to eq true - expect(job["args"]).to eq args - end - it "should only decrypt arguments whose names are set to true in the encrypted_args option hash" do - called = false - job["args"] = [args[0], Sidekiq::EncryptedArgs.encrypt(args[1]), args[2]] - middleware.call(NamedHashOptionSecretWorker.new, job, queue) do - called = true + it "should decrypt all arguments if the encrypted_args option is true" do + called = false + job["args"] = args.collect { |arg| Sidekiq::EncryptedArgs.encrypt(arg) } + job["encrypted_args"] = SecretWorker.sidekiq_options["encrypted_args"] + middleware.call(SecretWorker.new, job, queue) do + called = true + end + expect(called).to eq true + expect(job["args"]).to eq args end - expect(called).to eq true - expect(job["args"]).to eq args - end - it "should try multiple keys to decrypt arguments to support rolling keys" do - called = false - Sidekiq::EncryptedArgs.secret = "old" - job["args"] = args.collect { |arg| Sidekiq::EncryptedArgs.encrypt(arg) } - Sidekiq::EncryptedArgs.secret = ["new", "old"] - middleware.call(SecretWorker.new, job, queue) do - called = true + it "should only decrypt arguments whose position index is set to true when the encrypted_args option is a hash" do + called = false + job["args"] = [args[0], Sidekiq::EncryptedArgs.encrypt(args[1]), args[2]] + job["encrypted_args"] = HashOptionSecretWorker.sidekiq_options["encrypted_args"] + middleware.call(HashOptionSecretWorker.new, job, queue) do + called = true + end + expect(called).to eq true + expect(job["args"]).to eq args end - expect(called).to eq true - expect(job["args"]).to eq args - end - it "should pass through keys that should be encrypted but were not encrypted" do - called = false - middleware.call(SecretWorker.new, job, queue) do - called = true + it "should only decrypt arguments whose position index is set to true when the encrypted_args option is an array" do + called = false + job["args"] = [args[0], Sidekiq::EncryptedArgs.encrypt(args[1]), args[2]] + job["encrypted_args"] = ArrayOptionSecretWorker.sidekiq_options["encrypted_args"] + middleware.call(ArrayOptionSecretWorker.new, job, queue) do + called = true + end + expect(called).to eq true + expect(job["args"]).to eq args + end + + it "should only decrypt arguments whose names are provided in the encrypted_args option array" do + called = false + job["args"] = [args[0], Sidekiq::EncryptedArgs.encrypt(args[1]), args[2]] + job["encrypted_args"] = NamedArrayOptionSecretWorker.sidekiq_options["encrypted_args"] + middleware.call(NamedArrayOptionSecretWorker.new, job, queue) do + called = true + end + expect(called).to eq true + expect(job["args"]).to eq args + end + + it "should only decrypt arguments whose names are set to true in the encrypted_args option hash" do + called = false + job["args"] = [args[0], Sidekiq::EncryptedArgs.encrypt(args[1]), args[2]] + job["encrypted_args"] = NamedHashOptionSecretWorker.sidekiq_options["encrypted_args"] + middleware.call(NamedHashOptionSecretWorker.new, job, queue) do + called = true + end + expect(called).to eq true + expect(job["args"]).to eq args end - expect(called).to eq true - expect(job["args"]).to eq args end end diff --git a/spec/encypted_args_spec.rb b/spec/encypted_args_spec.rb index e344c01..6fb1c2f 100644 --- a/spec/encypted_args_spec.rb +++ b/spec/encypted_args_spec.rb @@ -20,7 +20,17 @@ expect(decrypted).to eq data end - it "should be able to set mulitiple keys to try for decrypting so a key can be gracefully rolled" do + it "should encrypt and decrypt objects that can generate JSON into JSON structures" do + Sidekiq::EncryptedArgs.secret = "key" + attributes = {"foo" => [1, 2, 3]} + data = ComplexRubyType.new(attributes) + encrypted = Sidekiq::EncryptedArgs.encrypt(data) + decrypted = Sidekiq::EncryptedArgs.decrypt(encrypted) + expect(encrypted).to be_a String + expect(decrypted).to eq(attributes) + end + + it "should be able to set multiple keys to try for decrypting so a key can be gracefully rolled" do Sidekiq::EncryptedArgs.secret = "key_1" encrypted_1 = Sidekiq::EncryptedArgs.encrypt("foobar") @@ -71,30 +81,58 @@ expect(Sidekiq::EncryptedArgs.decrypt(1)).to eq 1 end - it "should configure the Sidekiq client middleware" do - allow(Sidekiq).to receive(:server?).and_return false - Sidekiq::EncryptedArgs.configure! - expect(Sidekiq.client_middleware.exists?(Sidekiq::EncryptedArgs::ClientMiddleware)).to eq true - end + context "loading middleware" do + around(:each) do |example| + with_empty_middleware do + example.run + end + end - it "should configure the Sidekiq server middleware" do - allow(Sidekiq).to receive(:server?).and_return true - Sidekiq::EncryptedArgs.configure! - expect(Sidekiq.server_middleware.exists?(Sidekiq::EncryptedArgs::ServerMiddleware)).to eq true - end + it "should configure the Sidekiq client middleware" do + as_sidekiq_client! + Sidekiq::EncryptedArgs.configure! + expect(Sidekiq.client_middleware.exists?(Sidekiq::EncryptedArgs::ClientMiddleware)).to eq true + expect(Sidekiq.server_middleware.exists?(Sidekiq::EncryptedArgs::ServerMiddleware)).to eq false + end - it "should set the secret from the configure! method" do - Sidekiq::EncryptedArgs.secret = nil - Sidekiq::EncryptedArgs.configure!(secret: "Foo") - encryptors = Sidekiq::EncryptedArgs.instance_variable_get(:@encryptors) - expect(encryptors).to_not eq nil - end + it "should configure the Sidekiq server middleware" do + as_sidekiq_server! + Sidekiq::EncryptedArgs.configure! + expect(Sidekiq.client_middleware.exists?(Sidekiq::EncryptedArgs::ClientMiddleware)).to eq true + expect(Sidekiq.server_middleware.exists?(Sidekiq::EncryptedArgs::ServerMiddleware)).to eq true + end + + it "should set the secret from the configure! method" do + Sidekiq::EncryptedArgs.secret = nil + Sidekiq::EncryptedArgs.configure!(secret: "Foo") + encryptors = Sidekiq::EncryptedArgs.instance_variable_get(:@encryptors) + expect(encryptors).to_not eq nil + end + + it "should not overwrite the secret if it is not provided to the configure! method" do + Sidekiq::EncryptedArgs.secret = "Foo" + encryptors = Sidekiq::EncryptedArgs.instance_variable_get(:@encryptors) + Sidekiq::EncryptedArgs.configure!(secret: nil) + expect(encryptors).to_not eq [] + expect(encryptors).to eq Sidekiq::EncryptedArgs.instance_variable_get(:@encryptors) + end - it "should not overwrite the secret if it is not provided to the configure! method" do - Sidekiq::EncryptedArgs.secret = "Foo" - encryptors = Sidekiq::EncryptedArgs.instance_variable_get(:@encryptors) - Sidekiq::EncryptedArgs.configure! - expect(encryptors).to_not eq nil - expect(encryptors).to eq Sidekiq::EncryptedArgs.instance_variable_get(:@encryptors) + it "should load client middleware first and server middleware last on server" do + as_sidekiq_server! + Sidekiq.server_middleware.add(EmptyMiddleware) + Sidekiq.client_middleware.add(EmptyMiddleware) + Sidekiq::EncryptedArgs.configure!(secret: "0xDEADBEEF") + + expect(Sidekiq.client_middleware.map(&:klass)).to eq [Sidekiq::EncryptedArgs::ClientMiddleware, EmptyMiddleware] + expect(Sidekiq.server_middleware.map(&:klass)).to eq [EmptyMiddleware, Sidekiq::EncryptedArgs::ServerMiddleware] + end + + it "should load client middleware first on client" do + as_sidekiq_client! + Sidekiq.client_middleware.add(EmptyMiddleware) + Sidekiq::EncryptedArgs.configure!(secret: "0xDEADBEEF") + + expect(Sidekiq.client_middleware.map(&:klass)).to eq [Sidekiq::EncryptedArgs::ClientMiddleware, EmptyMiddleware] + end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index a05375f..337407e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -8,6 +8,53 @@ RSpec.configure do |config| config.warnings = true config.order = :random + + config.around(:each) do |example| + if example.metadata[:no_warn] + save_stderr = $stderr + begin + $stderr = StringIO.new + example.run + ensure + $stderr = save_stderr + end + else + example.run + end + end +end + +# Reset all middleware for nested context and then restore. +# +# @note Middleware args are not preserved +def with_empty_middleware + # Save the middleware context + server_middleware = Sidekiq.server_middleware.entries.map(&:klass) + client_middleware = Sidekiq.client_middleware.entries.map(&:klass) + Sidekiq.server_middleware.clear + Sidekiq.client_middleware.clear + + yield + + # Clear anything added and restore all previously registered middleware + Sidekiq.server_middleware.clear + Sidekiq.client_middleware.clear + server_middleware.each { |m| Sidekiq.server_middleware.add(m) } + client_middleware.each { |m| Sidekiq.client_middleware.add(m) } +end + +def as_sidekiq_server! + allow(Sidekiq).to receive(:server?).and_return true +end + +def as_sidekiq_client! + allow(Sidekiq).to receive(:server?).and_return false +end + +class EmptyMiddleware + def call(*args) + yield + end end class RegularWorker @@ -39,17 +86,17 @@ module Super class SecretWorker include Sidekiq::Worker - sidekiq_options encrypted_args: [false, false, true] + sidekiq_options encrypted_args: "arg_3" def perform(arg_1, arg_2, arg_3) end end end -class ArrayOptionSecretWorker +class ArrayIndexSecretWorker include Sidekiq::Worker - sidekiq_options encrypted_args: [false, true] + sidekiq_options encrypted_args: [1] def perform(arg_1, arg_2, arg_3) end @@ -64,6 +111,15 @@ def perform(arg_1, arg_2, arg_3) end end +class ArrayOptionSecretWorker + include Sidekiq::Worker + + sidekiq_options encrypted_args: [false, true] + + def perform(arg_1, arg_2, arg_3) + end +end + class NamedArrayOptionSecretWorker include Sidekiq::Worker @@ -81,3 +137,13 @@ class NamedHashOptionSecretWorker def perform(arg_1, arg_2, arg_3) end end + +class ComplexRubyType + def initialize(attributes) + @attributes = attributes + end + + def to_json + @attributes.to_json + end +end