Apache HttpClient 5.0 followup

[cachescrubber]

@gregturn Thanks for the quick merge of #1356. There are some issued we need to address. I already spotted a Bug in HttpComponents5MessageSender#afterPropertiesSet. This is fixed in this MR.

In my original MR I noted incompatibilities in the HttpClient Api regarding configuring Authentication on the HttpClient.
Previously, AuthScope has been AuthScope.ANY which is removed in httpclient 5.x. So basically this way to configure preemptive http authentication will no longer work as before (or at all, not sure). At least I updated the Javadoc to reflect the change.

Another Idea would be to remove credentials and authScope altogether and guide users to handle authentication in a client interceptor. WDYT?

[cachescrubber]

caused by #1164

[gregturn]

We aren't adopting this plugin. Essentially, I am using https://github.com/spring-projects/spring-data-build/tree/main/etc/ide as the basis for any/all polishing going forwarda.

[gregturn]

On top of the specific comments, in general we try to keep polishing commits separate from actual coded commits. It makes it easier to sort through real changes while still making format changes that meets our standards.

[gregturn]

We aren't adopting this plugin. Essentially, I am using https://github.com/spring-projects/spring-data-build/tree/main/etc/ide as the basis for any/all polishing going forward.

[cachescrubber]

No offense, I just accidentally committed the pom ...

[gregturn]

Another Idea would be to remove credentials and authScope altogether and guide users to handle authentication in a client interceptor. WDYT?

I would favor this approach. If we can make security a separate task, I think that would simplify this part.

[cachescrubber]

How to proceed? Should we supply/sketch an Interceptor as a documentation snippet? Oder provide actual Code?

public class BasicAuthClientInterceptor extends ClientInterceptorAdapter {
	private final String credentials;

	public BasicAuthClientInterceptor(String username, String password) {
		this.credentials = basicAuth(username, password);
	}

	private static String basicAuth(String username, String password) {
		return Base64.getEncoder().encodeToString(String.format("%s:%s", username, password).getBytes());
	}

	@Override
	public boolean handleRequest(MessageContext messageContext) throws WebServiceClientException {
		try {
			TransportContext context = TransportContextHolder.getTransportContext();
			if (context.getConnection() instanceof AbstractHttpSenderConnection httpSenderConnection) {
				httpSenderConnection.addRequestHeader("Authorization", "Basic " + credentials);
			} else {
				logger.warn("web service connection is not http; back-off");
			}
		} catch (IOException e) {
			throw new WebServiceIOException("BasicAuthHeaderInterceptor " + e.getMessage(), e);
		}
		return true;
	}
}

[gregturn]

Instead of coming up with something more to maintain, I'd rather point them toward https://docs.spring.io/spring-ws/docs/current/reference/html/#security-wss4j-security-interceptor as well as https://github.com/spring-projects/spring-ws-samples/blob/main/airline/client/spring-ws/src/main/java/org/springframework/ws/samples/airline/client/sws/WsConfiguration.java#L51, an example we already maintain, that illustrates how to secure things using Wss4j. (And...I'd rather use existing interceptors so we don't hand roll it).

[cachescrubber]

Full blown Wss is a different kind of story IMO. I was thinking about this section in the client documentation: how to use Apache HfttpClient to authenticate with HTTP authentication. This might no longer work as documented using HttpClient 5. So unfortunately spring-ws would loose the ability to do simple http basic auth. If you don't want to add new code, let's just add a snippet to the documentation or add it to some example project or even release notes.

[cachescrubber]

I just checked the AuthScope source code and api doc. Another possibility would be to set the default AuthScope in HttpComponents5ClientFactory to new AuthScope(null, null, -1, null, null ); As it seems, they removed the static ANY instance, but functionality is still there and documented. We would then have a drop in replacement for HttpComponentsMessageSender.

[cachescrubber]

My last commit redefines AuthScope.ANY in HttpComponents5ClientFactory. Probably means least effort overall. WDYT?

[gregturn]

I like that approach over simply using a null.

Only thing I'd suggest is including a little extra in the JavaDocs explaining that HttpClient dropped ANY and that this value object is a flyweight replacement.

[gregturn]

Regarding docs, I think a simple example of using it for Basic Auth in the client-side is fine.

[cachescrubber]

I pushed a draft.

[gregturn]

Thanks @cachescrubber.

I squashed your changes, polished, and merged to main.