Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support HTTPS and self-signed certificates #66

Closed
mohanraj-r opened this issue Jan 23, 2018 · 6 comments
Closed

Support HTTPS and self-signed certificates #66

mohanraj-r opened this issue Jan 23, 2018 · 6 comments

Comments

@mohanraj-r
Copy link
Contributor

When tests are run against https URL, following error is returned

Error on Verify: Get https://localhost:63128/api/v2.6/grid: http: server gave HTTP response to HTTPS client

I have a server object under test which accepts hostname and based on the hostname passed in sets the base URL for the subsequent requests as https://<hostname>/api. But testing this with pact gives the error http: server gave HTTP response to HTTPS client. Is it possible to have a pact interaction work with https scheme instead of http?

Software versions

  • OS: Mac OSX 10.13.2
  • Consumer Pact library:
$ pact-go version
Pact Go CLI v0.0.11
  • Provider Pact library: v0.0.12
  • Golang Version: 1.9.2
  • Golang environment:
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/mohanrajr/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.9.2/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.9.2/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/d0/742yxrrx0glg93s4z_rwyzkxm0whbv/T/go-build176059632=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

Expected behaviour

Pact tests are runnable against https URL

Actual behaviour

Tests fail with following error message when running against https url

Error on Verify: Get https://localhost:63128/api/v2.6/grid: http: server gave HTTP response to HTTPS client

Steps to reproduce

  • Create a consumer test that sends a request to https url instead of http
// Pass in test case
	var test = func() error {
		u := fmt.Sprintf("https://localhost:%d", pact.Server.Port)
		req, err := http.NewRequest(http.MethodGet, u, nil)

		req.Header.Set("Content-Type", "application/json")
		if err != nil {
			return err
		}
		if _, err = http.DefaultClient.Do(req); err != nil {
			return err
		}
		return err
	}

Relevant log files

No relevant logs in pact.log

@mefellows
Copy link
Member

Thanks @mohanraj-r, have you got a reproducible example I can run? Or at the very least, could you please provide your entire consumer test (I'm assuming it's a consumer test)?

In any case, I haven't yet implemented SSL for the underlying mock service, so this would be expected. In your test you are sending a message to https://<blah> but the underlying mock service is not running a secure server.

I'll change the title of this to enabling SSL for this case, but in the meanwhile I'd suggest you not use an HTTPS server.

FWIW and IMO I don't think you get a lot of value from testing SSL from Pact, I'm not sure what you gain over other local tests ensuring your client can make HTTPS calls.

(BUT, I'm going to add the feature anyway!).

@mohanraj-r
Copy link
Contributor Author

mohanraj-r commented Jan 23, 2018

have you got a reproducible example I can run? Or at the very least, could you please provide your entire consumer test (I'm assuming it's a consumer test)?

I have included the test in the Steps to reproduce section. I just pass that into a pact.Verify() Yes it is a consumer test.

FWIW and IMO I don't think you get a lot of value from testing SSL from Pact, I'm not sure what you gain over other local tests ensuring your client can make HTTPS calls.

Agree. But the code under test in my case assumes a https scheme because it is all that it needs and deals with. But now since the mock server doesn't support https I have to deal with modifying the code under test to support http just for testing purpose - which works for now but is not ideal and feels like kind of a code smell.

(BUT, I'm going to add the feature anyway!).
thanks!

@mefellows
Copy link
Member

Agree. But the code under test in my case assumes a https scheme because it is all that it needs and deals with. But now since the mock server doesn't support https I have to deal with modifying the code under test to support http just for testing purpose - which works for now but is not ideal and feels like kind of a code smell.

Pretty much any code you write for production will need to take a parameter to configure the URL/endpoint to call. It's a pretty trivial matter to modify this.

Don't forget, we can add SSL support to the mock service, but you'll then need to either:

  1. create a valid SSL certificate chain for any environment Pact runs (so your desktop and CI at the very least) and pass in to the mock service so it can use them
  2. pass in / use the default self-signed certificate with the mock service, meaning you'll then need to modify your client code even more to deal with the fact you're talking to a service with an invalid certificate chain

As I said, I'll leave this ticket open and when time permits add this feature in.

@mohanraj-r
Copy link
Contributor Author

mohanraj-r commented Jan 23, 2018

Pretty much any code you write for production will need to take a parameter to configure the URL/endpoint to call. It's a pretty trivial matter to modify this.

In this case it does - it takes the hostname and formats the endpoint with a known scheme.
But on the whole I agree. This isn't a huge issue.

As I said, I'll leave this ticket open and when time permits add this feature in.

If you want you can also label it appropriately (low_priority_feature, help_needed, when_time_permits etc) and then close it - to be able to give the high priority issues the needed visibility and attention. Maybe have one meta issue with pointers to such low priority closed issues. Because one of the signs of health I (like many others I presume) look for in a project is number of open issues and more importantly activity/staleness of the issues.

@mefellows
Copy link
Member

Thanks @mohanraj-r, appreciate all of the effort going into these tickets BTW!

@mefellows mefellows changed the title server gave HTTP response to HTTPS client Support HTTPS and self-signed certificates Mar 25, 2018
@mefellows
Copy link
Member

A custom tls config can be passed into the provider verification of 2.x.x, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants