DiUS / pact-consumer-swift

A Swift / ObjeciveC DSL for creating pacts.
MIT License
98 stars 43 forks source link

Connection Error when working with pact mock service enabling ssl #114

Open CaferlopAgileContent opened 3 years ago

CaferlopAgileContent commented 3 years ago

Hello!

Trying to setup a PoC for a Swift project, enabling ssl in the pact-mock-service lead us to the error message shown in the below image:

Captura de pantalla 2021-01-27 a las 22 12 04

The server is launched using: pact-mock-service start --ssl --pact-specification-version 2.0.0 --log "${SRCROOT}/tmp/pact-ssl.log" --pact-dir "${SRCROOT}/tmp/pacts-ssl" -p 1234

The PactVerificationService is instantiated allowing insecure certificates:

Captura de pantalla 2021-01-27 a las 22 19 42

Yet it seems it is not to accept the self signed certificate:

Captura de pantalla 2021-01-27 a las 22 19 24

Have you guys faced this kind of issue?

Thanks in advance.

surpher commented 3 years ago

Have you made your HTTPClientService conform to URLSessionDelegate?

In your app target where you've implemented your API client, set it to conform to URLSessionDelegate and set up the following delegate method:

public func urlSession(
    _ session: URLSession,
    didReceive challenge: URLAuthenticationChallenge,
    completionHandler: @escaping (URLSession.AuthChallengeDisposition, URLCredential?) -> Void
  )

In it you can define which domains should allow insecure certificates. This will allow your API client to communicate over https but still connect to https://localhost where a self-signed certificate lives.

An example delegate method can be found here and here.

Your providerMock is using your real API client implementation to trigger the request. That means that your real API client must be able to communicate with a mock server that is using an insecure self-signed certificate.

CaferlopAgileContent commented 3 years ago

Hello! Thank you very much for your answer.

I've done what you suggested, but the providerMock is not using the API client I've implemented. If I run the test, but commenting the API client part, it will fail equally with the same error message, as you can see in the image below:

Captura de pantalla 2021-01-29 a las 19 44 01

By taking a look at PactVerificationService class, it seems it takes the API client paper when instantiated. Actually, if a put a break point in the URLSession delegate method, and run the test, and then hits the extension URLSession.APIServiceError: LocalizedError, and shows the ssl error message:

Captura de pantalla 2021-01-29 a las 19 57 01

I´ve uploaded the example implemented to an open repo, in case you could take a look. Perhaps I'm missing something. https://github.com/CaferlopAgileContent/PactConsumerSwiftExample/blob/main/README.md

Thank you very much for your time.

surpher commented 3 years ago

I can have a look once I get some time. Looks suspicious though, why it would be complaining now when nothing changed in a long while. And the mockService's API client that talks to pact-ruby-standalone service and does all the Pact mumbo jumbo (eg. sets up the expectations, checks whether the api calls have been triggered, etc.) in PactConsumerSwift does have its own URLSessionDelegate conformance to allow non-secure connections to https://localhost.

There's something fishy going on here. Perhaps something changed in 14.4?

EDIT: Out of curiosity, is there a specific reason you're testing using pact-consumer-swift that supports Pact specification V2? We're working on moving onto Pact specification V3.

surpher commented 3 years ago

There seems to have been a change in iOS 14.x that is breaking this! When hitting localhost, the NSURLSessionDelegate method is not being called?

Using your example project, target iOS 14.4 and Xcode 12.4: I have unlocked the Pods/Pods/PactConsumerSwift/PactVerificationService.swift and updated the following line number 219:

// old
challenge.protectionSpace.host.contains("localhost")
// with new
(challenge.protectionSpace.host.contains("localhost") || challenge.protectionSpace.host.contains("127.0.0.1") || challenge.protectionSpace.host.contains("0.0.0.0"))

Then...

  1. When I set up pactVerificationService with url: "https://localhost" it is failing to connect to providerMock (eg: Mock Server running on 1234 - as per your setup in the project). 👎

    // NOT OK
    let pactVerificationService = PactVerificationService(url: "https://localhost", port: 1234, allowInsecureCertificates: true)
  2. When I set up pactVerificationService with url: "https://127.0.0.1" it successfully connects 👍 (although your APIClient still doesn't trigger and it doesn't hit the mockServer, but that's another problem)

    // IS OK 🤷‍♂️ 
    let pactVerificationService = PactVerificationService(url: "https://127.0.0.1", port: 1234, allowInsecureCertificates: true) // OK

We will need to look into this and improve the NSSessionDelegate conformance in PactVerificationService.swift and at least update as per first code example in this comment.

Anyone with some time on their hand that wants to open a PR for it (solution is already up there), feel free to open it.

caferlop commented 3 years ago

Thank you very much!

Replicated the changes, and now works properly!

The reason we tried with v2 is because v3 is labeled as under heavy development, so we decided to aim to a more stable version, just in case. As well, v3 it is not distributed through CocoaPods, which is the dependency manager we use. If that would change, we could give a try.

Regarding the PR, tried to push a branch to create the PR but got an access denied.

surpher commented 3 years ago

Good to hear. I'm not really happy with using IP addresses instead of localhost, and I haven't figured out why this is happening now with the limited time I've given it to do research. Also note that the pact-ruby-standalone installed via Homebrew was broken until yesterday. I've fixed that too so the latest 1.88.37 can be used. I've checked with pact-ruby-standalone maintainers and they said there were no changes to how self-signed certs work.

For the PRs on this repo you would've needed to fork it first, make changes on your fork and then open a PR from your fork back into this repository. We could update CHANGELOG.md with better guidance on how to open PRs.

Fair enough regarding PactSwift and heavy development note. It should be stable enough to be used in order to raise bugs or suggestions if nothing else. This way we could get the higher level of confidence faster. Next task on my list is exposing it as XCFramework, but behaviourally it should be all good to be used. I'm really not a fan of CocoaPods so it will not be on "my" to-do list. Anywhere I do any work we're super focused on SPM only. But again, that project is also an open-source project and if anyone is willing to set up the mechanism to (semi automatically) distribute it that way, I'll be happy to accept the PR(s).

surpher commented 3 years ago

@caferlop I have updated the PactMacOSExample project to demonstrate how to set up a few Pact tests using latest tools at our disposal (macOS 11, Xcode 12.4).

There should be minimal differences in setting up a project using CocoaPods as a dependency manager I would imagine. Again, I have strong feelings about CocoaPods and haven't used it a project in about 4-5 years.

The update to URLSessionDelegate to allow 127.0.0.1 is in the repo already and available on master. But in order to get those changes through CocoaPods it would require a new release I believe. I would imagine v0.10.1 is imminent. @andrewspinks?

andrewspinks commented 3 years ago

@caferlop @surpher 0.10.1 has been released to cocoapods

CaferlopAgileContent commented 3 years ago

Thank you very much @surpher @andrewspinks !!!

ghost commented 3 years ago

Just wanted to note that I was hitting a brick wall having the same issue on version 0.10.1 (macOS 11.2, Xcode 12.2). Using 127.0.0.1 instead of localhost fixed the issue for me.