arturobernalg commented on PR #934: URL: https://github.com/apache/httpcomponents-client/pull/934#issuecomment-6740049167
> Ran the auth unit tests on [463413e](https://github.com/apache/httpcomponents-client/commit/463413e6611958fa7431426209ed1a34645027a3), they pass. A few things: > > 1. `BearerScheme` and `ScramScheme` take `challengeType` but still build the scope with `new AuthScope(host, realm, getName())`, so it's dropped. With a `BasicCredentialsProvider` holding a token under `new AuthScope(null, null, -1, null, "Bearer", ChallengeType.PROXY)`, a 401 Bearer challenge from any origin gets that token (Basic correctly refuses the same setup). Both should pass `challengeType` through. SCRAM also needs to keep it for the `COMPLETE` re-auth in `generateAuthResponse`, which calls the 3-arg `isResponseReady` and ends up untyped. > > 2. In `match`, challenge type gets +32, which outranks host (+16). Given `new AuthScope(null, null, -1, null, null, ChallengeType.TARGET)` and `new AuthScope("api.example.com", 443)`, a TARGET request to api.example.com:443 picks the wildcard. I'd give challenge type the lowest weight so it only breaks ties between otherwise equal scopes. > > 3. `SystemDefaultCredentialsProvider` is unchanged, so on a TARGET scope it still asks the `Authenticator` for SERVER then PROXY and returns the proxy credentials. That's the part #925 covers. I'll rebase it onto this once it's in. > > > Nits: the javadoc for the existing `(protocol, host, port, realm, schemeName)` and `(HttpHost, realm, schemeName)` constructors moved onto the new overloads, so the old ones lost their docs (and `@since 4.2`), and the new ones don't document `challengeType`. Also "AutScope" in the commit subject. > > Probably worth a release note: custom providers that look credentials up by `equals` on an untyped `AuthScope` will miss now that the built-in schemes send typed scopes. I agree that `ChallengeType` should be propagated consistently through Bearer and SCRAM, and that it should not take precedence over host matching. The `SystemDefaultCredentialsProvider` issue is not directly related to this change and should be addressed separately. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
