Preemptively send authentication credentials in WMSHttpHelper #1284
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Before moving from commons-httpclient to httpcomponents in 8beede1, WMSHttpHelper was sending authentication credentials preemptively. httpcomponents doesn't seem to have any mechanism to force preemptive authentication except for explicitly setting the Authorization header, which is what this commit does.
|
@ianturton you worked on the HTTP components upgrade. Any observation about the preemptive authentication change? |
|
I am not sure what to with this PR, do we have a test or a downstream integration test or any way to ensure this does something? |
|
I can't really comment on the change - it looks fine but I can't think of how to test it for real unless we have an authenticated server to test against. |
|
A test can be written using wiremock, stubbing expected requests and responses. Example here: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While trying to cache a WMS layer from another GeoServer instance that requires authentication, I noticed that GeoWebCache didn't manage to perform any successful requests to that upstream GeoServer.
While digging into that issue, I noticed that httpcomponents will only send credentials when challenged by the server.
Since that GeoServer had publicly available layers, it would just respond with 404 instead of 401, so
WMSHttpHelperwouldn't try it again with credentials.Before moving from commons-httpclient to httpcomponents in 8beede1,
WMSHttpHelperwas sending authentication credentials preemptively. This behaviour changed after the move.In fact, httpcomponents doesn't seem to have any mechanism to force preemptive authentication except for explicitly setting the Authorization header. So that's exactly what this PR does.
Just as a side note,
BasicScheme.authenticatedoesn't actually ever throwAuthenticationException, that's why it's re-thrown as anAssertionErrorhere.