Support HTTPS and self-signed certificates

mohanraj-r commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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:

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
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 commented 6 years ago

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 commented 6 years ago

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

mefellows commented 1 year ago

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

pact-foundation / pact-go