Open RChild0916 opened 1 year ago
I would assume normal usage would be to use a combination request filters / state handlers to check the existing token matches a particular format and then provide a valid token in the request to be verified. see 4 https://docs.pact.io/provider/handling_auth#4-modify-the-request-to-use-real-credentials
withCustomHeader
without looking at the code, I expect the use case would be to add a new header, rather than change an existing.
When you say you get two headers, are the different in casing?
Are you able to provide any logs?
I would assume normal usage would be to use a combination request filters / state handlers to check the existing token matches a particular format and then provide a valid token in the request to be verified. see 4 https://docs.pact.io/provider/handling_auth#4-modify-the-request-to-use-real-credentials
Yeah, this is what I am doing in the above setup by adding a new, valid token. There is the option of creating a new middleware within the API itself to check for a specific value before it hits the auth middleware, but I would ideally like to avoid this.
withCustomHeader
without looking at the code, I expect the use case would be to add a new header, rather than change an existing.When you say you get two headers, are the different in casing?
Again, I'm unsure of the exact result, but I'd wager you are correct that their are two different headers with the request.
Are you able to provide any logs? If it is needed I can try to get some logs but this would take some time to prune company data so it is shareable. If you are curious about whether or not it is adding a second header, I can look into this. What else are you hoping to see from the logs?
So it does indeed just add it (in the core), without considering others, which I assume is by design. If it is the case, maybe we need to warn, or error to the user, in the core, unless we do want to allow us to overwrite an existing error.
this is the user facing side
this is the native interop, so this is calling our rust code
ffi code for pactffi_verifier_add_custom_header
which calls add_custom_header to insert some custom_headers
Which I think gets setup here
which believe to be processed here
We may want to consider
What should happen if
Why would you want to add a custom header, without considering state
Are you able to provide any logs? If it is needed I can try to get some logs but this would take some time to prune company data so it is shareable. If you are curious about whether or not it is adding a second header, I can look into this. What else are you hoping to see from the logs?
A minimal reproducible example
You can create an example from the code in pact-net, or our example-consumer-dotnet / example-provider-dotnet repos
I want to confirm if there is indeed two headers, I can't imagine you would have two headers with the same casing, you should get one overwritten, however its possible there might be an authorization
and Authorization
header
I'm working with pact-net as a provider using pactflow which has some bi-directional testin
Just curious, are you using bi-directional, as well as consumer driven? So you have a provider OAS to verify against, and then also verifying a running provider with traditional consumer driven Pact?
Just as there traditionally wouldn't be a verification task as you show, if you use bi-directional only
Just curious, are you using bi-directional, as well as consumer driven? So you have a provider OAS to verify against, and then also verifying a running provider with traditional consumer driven Pact?
Well in this case, we are the provider, but yes, we are also using the consumer's pact json and running it against our API as the provider.
A minimal reproducible example
You can create an example from the code in pact-net, or our example-consumer-dotnet / example-provider-dotnet repos
This would take me some time to do, I'll see if I can get to this today or tomorrow. In the meantime, I ran some tests locally and can provide the requests /responses I see in Fiddler. Overall, it appears as if .WithCustomHeader does nothing if the header already exists, as I'm not seeing a duplicate header (see fiddler response below).
Our setup:
IPactVerifier pactVerifier = new PactVerifier(config); pactVerifier .ServiceProvider("App.Api", new Uri(_baseurl)) .WithPactBrokerSource( new Uri(_pacturl), options => options .TokenAuthentication(_pactflowToken) .PublishResults( _providerVersion, options => options.ProviderTags(_providerTags) .ProviderBranch(_providerBranch))) .WithProviderStateUrl(new Uri(_fixture.ServerUri, "/provider-states")) .WithCustomHeader("authorization", $"Bearer {tokenValue}") .Verify();
The consumer pact (removed some data that isn't needed for this discussion):
{ "consumer": { "name": "Consumer" }, "provider": { "name": "App.Api" }, "interactions": [ { "description": "a request to create an object", "providerStates": [ ], "request": { "method": "POST", "path": "/test", "headers": { "authorization": "Bearer Test" }, "body": { [Removed for github] }, "matchingRules": { "$.body": { "match": "type" } } }, "response": { "status": 201, "headers": { "Content-Type": "[Removed for github]" }, "body": { [Removed for github] }, "matchingRules": { "$.body": { "match": "type" } } } } ], "metadata": { "pactSpecification": { "version": "3.0.0" } } }
Fiddler Raw request:
POST http://localhost:35000/test HTTP/1.1 authorization: Bearer Test accept-encoding: gzip, deflate host: localhost:35000 content-length: 172
Response didn't give much detail but threw a 500 because the JWT wasn't parseable (since it wasn't replaced and is just 'Test')
Cheers buddy! That is useful, I'm not a pact-net maintainer, nor a pact-rust maintainer but do use the pact ffi library, so the information you've provided is useful.
Something else we or others could do, is add test cases here
This uses a mock provider so under if it actually calls the native core, I'm going to a couple of tests with the ffi directly :)
This commit
https://github.com/pact-foundation/pact-reference/commit/cd83d3e417c2be02c7efb14683329c0a102bf14e
shows the issue with php, directly using the pact_ffi libarary.
testy-mc-header
pactffi_verifier_add_custom_header($handle, 'testy-mc-header', 'some-other-header')
'testy-mc-header', 'some-header'
'testy-mc-header', 'some-other-header'
Code examples below.
This will probably want transferring to the pact-reference repository and discussing with @uglyog as to whether this is expected behaviour or a bug
consumer 1
Array
(
[testy-mc-header] => Array
(
[0] => some-header
)
[content-type] => Array
(
[0] => application/json
)
[accept] => Array
(
[0] => */*
)
[accept-encoding] => Array
(
[0] => gzip, deflate
)
[host] => Array
(
[0] => localhost
)
[content-length] => Array
(
[0] => 315
)
)
[Wed May 10 17:24:01 2023] 127.0.0.1:50965 [201]: POST /api/books
consumer 2's headers
Array
(
[content-type] => Array
(
[0] => application/json
)
[accept] => Array
(
[0] => */*
)
[testy-mc-header] => Array
(
[0] => some-other-header
)
[accept-encoding] => Array
(
[0] => gzip, deflate
)
[host] => Array
(
[0] => localhost
)
[content-length] => Array
(
[0] => 2
)
)
[Wed May 10 17:33:31 2023] 127.0.0.1:51142 [204]: PUT /api/books/fb5a885f-f7e8-4a50-950f-c1a64a94d500/generate-cover
Yeah this is a problem in the FFI rather than PactNet itself. It'll need a tracking issue
Yeah this is a problem in the FFI rather than PactNet itself. It'll need a tracking issue
Okay, should I create that? If so, what repo should that go under?
Yeah this is a problem in the FFI rather than PactNet itself. It'll need a tracking issue
Okay, should I create that? If so, what repo should that go under?
Hey @RChild0916 sorry, we should have said (or done it for you), if you could raise the issue against https://github.com/pact-foundation/pact-reference/ and mention this issue that would be great
Hello all!
I'm working with pact-net as a provider using pactflow which has some bi-directional testing, and ran into an interesting issue. We recently added authentication to our API, and needed to add an auth header using the .WithCustomHeader method like so:
`
`
The "WithCustomHeader" function seems to have an issue, which I haven't been able to discern exactly but believe it is one of two things:
To get this working, we removed the Authorization header from the pact file and everything ran clean from the provider side, but this lead to a new issue within pactflow. Pactflow will check the request headers against our API's OAS, and sees that the authorization header is missing and fails for the consumer side check.
Is it possible to get new functionality to allow users to replace existing headers?