prepend client middleware and append server middleware

README.md

@@ -24,23 +24,26 @@ 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 append the encryption middleware to the end of the client and server middleware chains. You can add the middlewares 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
 
 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.add Sidekiq::EncryptedArgs::ClientMiddleware
+    chain.prepend Sidekiq::EncryptedArgs::ClientMiddleware
   end
 end
 ```
@@ -62,22 +65,24 @@ class SecretWorker
 end
 ```
 
-You can also encrypt just specific arguments with an array of either argument names or indexes. This can be useful to preserve visibility into non-sensitive arguments that might be useful for troubleshooting or other reasons. Both of these examples will encrypt just the second argument to the `perform` method.
+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 list of argument names that should be encrypted
-  sidekiq_options encrypted_args: [:arg_2]
+# 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 integer values indicating which argument positions should be encrypted
-  sidekiq_options encrypted_args: [1]
+# 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.
@@ -87,15 +92,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.

lib/sidekiq/encrypted_args.rb

@@ -38,7 +38,7 @@ def configure!(secret: nil)
 
         Sidekiq.configure_client do |config|
           config.client_middleware do |chain|
-            chain.add Sidekiq::EncryptedArgs::ClientMiddleware
+            chain.prepend Sidekiq::EncryptedArgs::ClientMiddleware
           end
         end
 
@@ -47,7 +47,7 @@ def configure!(secret: nil)
             chain.add Sidekiq::EncryptedArgs::ServerMiddleware
           end
           config.client_middleware do |chain|
-            chain.add Sidekiq::EncryptedArgs::ClientMiddleware
+            chain.prepend Sidekiq::EncryptedArgs::ClientMiddleware
           end
         end
       end

spec/encypted_args_spec.rb

@@ -30,7 +30,7 @@
     expect(decrypted).to eq(attributes)
   end
 
-  it "should be able to set mulitiple keys to try for decrypting so a key can be gracefully rolled" do
+  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")
 
@@ -81,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 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")
 
-  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)
+      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

spec/spec_helper.rb

@@ -24,6 +24,39 @@
   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
   include Sidekiq::Worker