pact-ruby icon indicating copy to clipboard operation
pact-ruby copied to clipboard

Authentication tokens/step in pact.

Open stefan-lz opened this issue 11 years ago • 25 comments

So at the moment we are having to modify the pact.json files before running pact. This is for inserting authentication tokens with expiry in about 2 hours into the pacts.

pacts = File.join(File.dirname(File.expand_path(__FILE__)), '../src/test/resources/pacts/*.json')
Dir.glob(pacts).each do |f|
  text = File.read(f)
  output_of_gsub = text.gsub(/\"Authorization\"\s*:\s*\".+\"/) { "\"Authorization\": \"ioof-token #{token}\"" }
  File.open(f, "w") { |file| file.puts output_of_gsub }
end

It would be good to be able to nicely insert/modify headers of the request before they are sent without modifying the pact files.

Cheers,

stefan-lz avatar Oct 05 '14 22:10 stefan-lz

For the Gradle Pact-JVM provider plugin, I added a callback that allows the request to be modified. See https://github.com/DiUS/pact-jvm/tree/master/pact-jvm-provider-gradle#modifying-the-requests-before-they-are-sent

uglyog avatar Oct 05 '14 23:10 uglyog

I feel very uncomfortable with this idea. If you can modify the request, then there is no guarantee that the actual request that the service will get will look like what gets replayed in pact:verify. Are any of the following options for you?

  1. Setting the time using Timecop for both Consumer and Provider tests.
  2. Making the expiry period configurable, and setting it to a very large time in the set_up of the provider state.

bethesque avatar Oct 06 '14 00:10 bethesque

Ron, in that scenario, the Consumer could completely miss sending the OAUTH token, and nothing in the tests would break. I understand making the request available as readonly, so that you could inspect the token, and then set up the appropriate state on the provider so that the token would work, but I feel very strongly against allowing the request to be modified.

bethesque avatar Oct 06 '14 00:10 bethesque

@bethesque, I agree that modifying the pacts make them less trustworthy.

So I guess the issue is in our current testing environment we do not have any mock or stub service when authenticating. So if we're using a real authenticator service, we need to have a real auth-token in the request.

stefan-lz avatar Oct 06 '14 01:10 stefan-lz

Are you executing pact:verify against a rack app, or are you using pact-provider-proxy to run pact:verify against an actual running server?

bethesque avatar Oct 06 '14 01:10 bethesque

@bethesque In the JVM version, the provider is running as a separate server so there is no way to setup anything in the provider. The only way to do it would use a mock server for the authentication. In our particular case we don't have control over the authentication, we don't have a mock server and the tokens are time sensitive and required to be signed with RSA keys.

I believe as framework authors we should allow people to be able to use the framework in any manner they require that is helpful to them, and not be prescriptive. So even though they may be able to shoot themselves in the foot, I say 'Happy Shooting'.

uglyog avatar Oct 06 '14 05:10 uglyog

Sigh. It makes me sad, but I see your point Ron.

@stefan-lz How are you verifying? With the normal Ruby pact:verify? Or against a running endpoint?

bethesque avatar Oct 06 '14 06:10 bethesque

pact:verify running on j-ruby (not sure if that matters or not) and our provider is a jetty drop-wizard app, so we don't have any nice VCR/Timecop options either. Cheers.

On Monday, October 6, 2014, Beth [email protected] wrote:

Sigh. It makes me sad, but I see your point Ron.

@stefan-lz https://github.com/stefan-lz How are you verifying? With the normal Ruby pact:verify? Or against a running endpoint?

— Reply to this email directly or view it on GitHub https://github.com/realestate-com-au/pact/issues/49#issuecomment-57977345 .

Stefan Leszkiewicz

DiUS Computing Pty. Ltd. Where Ideas Are Engineered

Phone: +61 3 9008 5400 Mobile: +61 402 62 9802

www.dius.com.au

stefan-lz avatar Oct 06 '14 08:10 stefan-lz

So, I've had a think about the nicest way we can allow people to shoot themselves in the foot.

Pact.provider_states_for "My Jetty DropWizard App whatever that is" do

  set_up do | interaction |
    interaction.request.headers['Authorization'] = 'some dynamic token'
  end

end
$ rake pact:verify
Note: set_up hook modified request:
{
  "headers" : {
-   "Authorization" : "original token"
+   "Authorization" : "some dynamic token" 
  }
}

The set_up hook already exists, it just needs to be modified to yield a copy of the interaction that can then be compared to the original one, and an information message logged to the screen to alert the Shooter to the fact that they are shooting themselves.

bethesque avatar Oct 06 '14 09:10 bethesque

So, @stefan-lz, if you want to make the above happen, I will reluctantly accept a pull request.

bethesque avatar Oct 06 '14 10:10 bethesque

Btw, I really think that instead, you should create a mock authentication service. This feels dirty.

bethesque avatar Oct 06 '14 10:10 bethesque

I think if something feels dirty you shouldn't do it... Leave it in a branch.

If you wait long gone enough another option will become obvious that satisfies this need and several others as well. On 6 Oct 2014 21:13, "Beth" [email protected] wrote:

Btw, I really think that instead, you should create a mock authentication service. This feels dirty.

— Reply to this email directly or view it on GitHub https://github.com/realestate-com-au/pact/issues/49#issuecomment-57997745 .

NigelThorne avatar Oct 06 '14 11:10 NigelThorne

I've written a spike getting dynamic auth tokens into the request via provider setup. The commit is here: https://github.com/stefan-lz/pact/commit/18c247a8d8138512b8b7aa4f8defd997102cdeab It does not include creating the diff output yet.

It's all very hacky, and probably breaks plenty of rules, so I'm not going to submit a pull request. But it works, and the tests are passing.

I particually like this line: https://github.com/stefan-lz/pact/commit/18c247a8d8138512b8b7aa4f8defd997102cdeab#diff-6ba38645571c7cc5e2d9ad8f717924e3R23

I'm now divided wether this is a good thing or not, and our case is quite unique. So I'm happy to close this until there is a larger pressing need for it.

stefan-lz avatar Nov 27 '14 12:11 stefan-lz

I don't know quite how drop wizard works, but do you have a rack app interface, or are you using pact-provider-proxy?

bethesque avatar Nov 27 '14 21:11 bethesque

If you are using the normal pact:verify, then you could something like this: https://gist.github.com/bethesque/203e9d2ec08f4a40d2df No dirty hacks required!

I think there's a similar thing you could do if you are using pact-provider-proxy.

bethesque avatar Nov 27 '14 21:11 bethesque

Ah nice 1, but we are not using rack unfortunately. Its running in its own separate process, outside of ruby. Might as well be a server half way across the world somewhere. A proxy server would probably work, but also a fair bit of effort.

stefan-lz avatar Nov 28 '14 01:11 stefan-lz

I don't understand how you are running pact:verify if you are not using pact-provider-proxy and it's not a rack app?! Can you show me the pact:verify configuration??

bethesque avatar Nov 28 '14 05:11 bethesque

It could be a proxy setup, maybe - but spawning an external process within. I'm not 100% on how it hangs together, we do need to fire up the server first before running pact.

Here's our whole setup:

  • jruby-1.7.9
  • pact (1.3.3)
  • pact-provider-proxy (1.1.0)

In our Rakefile we have this:

require 'pact/tasks'
require 'pact/provider/proxy/tasks'
require_relative 'lib/dropwizard/app'
...
pacts = Dir['./src/test/resources/pacts/*'].map do |file_name|
  pact_name = file_name[file_name.rindex('/')+1..file_name.rindex('.')-1]
  {:name => pact_name, :src => file_name}
end

pacts.each do |pact|
  Pact::ProxyVerificationTask.new pact[:name] do |task|
    task.pact_url pact[:src], :pact_helper => './lib/pact_helper'
    task.provider_base_url 'http://localhost:9999'
  end
end
...
namespace :pact do
  desc 'verify interactions with service'
  task :verify_all do
    pacts.each do |pact|
      Rake::Task["pact:verify:#{pact[:name]}"].execute
    end
  end
end

And we run our provider tests using rake pact:verify_all after firing up our server.

We also have a rake task to do this:

task :integration do
     ...
      #update pact files with tokens here
      ...
begin

      Rake::Task['server:start_server'].execute
      Rake::Task['spec'].execute
      Rake::Task['pact:verify_all'].execute
    ensure
      Rake::Task['server:stop_server'].execute
    end
end

Our lib/dropwizard/app.rb (proxy?) looks like this:

module Dropwizard
  class App

    def self.start_server
      begin
        puts 'waiting for server to start...'
        sleep 1
        started = HTTParty.get(URI::encode('http://localhost:9999/admin/healthcheck')).success?
      rescue
        @pid = Process.spawn("#{command} server ./conf/test/config.yml") unless @pid or started
      end until started
    end

    def self.stop_server
      if @pid
        puts "killing server on #{@pid}"
        system "kill #{@pid}"
      end
    end

    def self.command
      "java -jar ./target/service*.jar"
    end

  end
end

and here is a snippet of our pact_helper.rb:

Pact.set_up do
  DatabaseHelper.clean(true)
end

def create_standard_config
      create_group_limit("ABC", 2)
      ...
end

Pact.provider_states_for 'ABC_Web' do
  provider_state 'Groups for an advisor and groups for other advisors' do
    set_up do
      create_standard_config()
      create_group(1, 'group1')
      ...
    end
  end
...
end

Its a bit glued together, but its been working for us. The main problem with this setup is modifying the pact files which are checked into git, so this means we have to check them out again after every run.

stefan-lz avatar Dec 03 '14 00:12 stefan-lz

So you are using pact-provider-proxy! See that 'require "pact/provider/proxy"' line :P

Ok, I have an idea of how to do this more elegantly, but putting in another rack app between the Rack::ReverseProxy and Pact, in here:

https://github.com/bethesque/pact-provider-proxy/blob/master/lib/pact/provider/proxy/proxy_pact_helper.rb


class AddAuthenticationHeaders 
   def initialize app
     @app = app
   end

  def call env
    @app.call(env.merge('HTTP_AUTHORIZATION_HEADER_THING' => 'new_header'))
  end
end

end


Pact.service_provider "Running Provider Application" do
  app do
    reverse_proxy = Rack::ReverseProxy.new do
      reverse_proxy '/', ENV.fetch('PACT_PROVIDER_BASE_URL')
    end
    AddAuthenticationHeaders.new(reverse_proxy)
  end
end

bethesque avatar Dec 03 '14 02:12 bethesque

@bethesque I have found a few examples of how to modify the headers, like your example above, can the request body be similarly modified? If so, it is not apparent to me how to do that. Any suggestions?

eskimoquinn avatar Oct 26 '17 17:10 eskimoquinn

Yes, using the same approach (though can I stress, it is NOT recommended!) What problem are you trying to solve? Before we go down the dirty hack route, let's see if there's another way we can solve it.

bethesque avatar Oct 26 '17 20:10 bethesque

Btw, for anyone reading this issue in the future, there is a --custom-provider-header option you can use in the pact-provider-verifier now. See docs here: https://github.com/pact-foundation/pact-provider-verifier#setting-a-custom-authentication-header

bethesque avatar Oct 26 '17 20:10 bethesque

@bethesque the reason I asked this question about modifying body is because we are trying to test services that wrap an externally hosted database. We need to test deleting an entity, however, we can only delete by a uniquely generated entity ID. We cannot setup the entity with a specific ID. Thus we need to modify the ID of the entity in the request before making the call against the real service. I see that the JVM provider has a way to do this, but we would really like to do this in the pact-provider-proxy for Ruby. We still have this problem by the way and it is leading to people not writing tests for these endpoints or questioning pact in general. Any thoughts?

eskimoquinn avatar Sep 06 '18 17:09 eskimoquinn

Yes. I've just written some rack middleware to allow the request to be modified in transit. There will be a more elegant implementation of this in the v4 spec, but for now, I would use a hardcoded token that you replace in this middleware.

The way you use it is to create a middleware class that implements Pact::ProviderVerifier::CustomMiddleware and modifies the Rack env. The Rack env is just a big flattened hash of all the request information. I've augmented it with the provider states information.

class MyMiddleware < Pact::ProviderVerifier::CustomMiddleware
  def initialize app
    @app = app
  end

  def call env 
    provider_states = provider_states_from(env)
    provider_state_name = provider_states.any? && provider_states.first.name
    puts "The provider state name is '#{provider_state_name}'"
    env['PATH_INFO'].gsub('HARDCODED_ID', 'DYNAMIC_ID') # Obviously, this has to be gotten from somewhere dynamically
    @app.call(env)
  end
end

If writing code in ruby is not your thing, you could create a proxy app in the language of your choice that modifies the request appropriately.

bethesque avatar Sep 06 '18 23:09 bethesque

Thanks @bethesque we are trying this out.

eskimoquinn avatar Sep 13 '18 15:09 eskimoquinn

Closing this as per the comments here

https://github.com/pact-foundation/pact-ruby/issues/49#issuecomment-339796408

YOU54F avatar Jun 25 '24 23:06 YOU54F