Allow pluggable means of acquiring the username and password for authentication

[spatula75]

Use a CredentialsProvider to pass usernames & passwords into AMQConnection with default
implementation that simply tracks a static username and password. Useful
for reconnect scenarios where the username or password might have
changed between the connect & reconnect. Doesn't break the existing API,
which uses the DefaultCredentialsProvider unless otherwise configured
with setCredentialsProvider.

Proposed Changes

Rationale: in some customized AuthC/AuthZ scenarios, especially where someone may be using auth_backend_http, a username or a password might change between the time it was first used and the time it was needed to reconnect for a connection recovery.

By passing a CredentialsProvider (in a ConnectionParameters) to AMQConnection instead of a static username and password, and by requesting the username and password of the CredentialsProvider just prior to using them to connect, it is possible to provide a custom instance which knows how to go and fetch new credentials if necessary prior to making the connection.

So for example, if a custom token-based oauth scheme is used, and the token is short-lived, this custom implementation could request a new oauth token to be provided as the 'password', which then gets validated on the other side of the request by auth_backend_http.

Without some mechanism to refresh credentials, a recovery would continue to use the original token, which may no longer be valid.

It's also conceivable that it may be useful in some cases to have a username provided to a dumb client by some API call in a manner which is semi-random or at least changeable. That, too, would be supportable by a custom CredentialsProvider (and then again handled by a custom server with auth_backend_http).

Types of Changes

What types of changes does your code introduce to this project?
Put an x in the boxes that apply

• Bugfix (non-breaking change which fixes issue #NNNN)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation (correction or otherwise)
• Cosmetics (whitespace, appearance)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask on the
mailing list. We're here to help! This is simply a reminder of what we are
going to look for before merging your code.

• I have read the CONTRIBUTING.md document
• I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
• All tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)
• Any dependent changes have been merged and published in related repositories

Further Comments

I'm still finding my way around in the test infrastructure, but I will try to add some tests for a custom CredentialsProvider as well once I figure it out. Existing tests with the new DefaultCredentialsProvider are working in my local environment, so I'm reasonably confident.

Apologies for hitting cmd-shift-O (force of habit) once in ConnectionFactory and creating some spurious differences where Eclipse did an "organize imports". I can try to revert that if it's important.

I selfishly made these changes on the 4.4 branch because we're stuck with 4.4 until we can end our Java 7 support in the wild.

[spatula75]

Unfortunately I am not able to log in to concourse-ci to see what failed a check or why it failed. If someone can copy/paste for me, I can look into it more thoroughly.

[michaelklishin]

Here's the most relevant output:

Results :

Tests in error: 
com.rabbitmq.client.test.functional.SaslMechanisms.connectionCloseAuthFailurePassword(com.rabbitmq.client.test.functional.SaslMechanisms)
  Run 1: SaslMechanisms>BrokerTestCase.tearDown:99->BrokerTestCase.openConnection:153 » AuthenticationFailure
  Run 2: SaslMechanisms>BrokerTestCase.tearDown:99->BrokerTestCase.openConnection:153 » AuthenticationFailure

com.rabbitmq.client.test.functional.SaslMechanisms.connectionCloseAuthFailureUsername(com.rabbitmq.client.test.functional.SaslMechanisms)
  Run 1: SaslMechanisms>BrokerTestCase.tearDown:99->BrokerTestCase.openConnection:153 » AuthenticationFailure
  Run 2: SaslMechanisms>BrokerTestCase.tearDown:99->BrokerTestCase.openConnection:153 » AuthenticationFailure

We can dig up a log archive if needed.

[michaelklishin]

I think it's a good idea. @acogoluegnes can this also go into 4.5.0?

[spatula75]

Thanks, @michaelklishin. Most likely I just need to update those tests, especially if they're doing anything reflectiony to get/set values. I'll look at that now.

[acogoluegnes]

@spatula75 CI failed somewhere else, here's the output:

Tests run: 401, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 1,228.41 sec <<< FAILURE! - in com.rabbitmq.client.test.server.HATests
connectionRecoveryRequestsCredentialsAgain(com.rabbitmq.client.test.functional.ConnectionRecovery)  Time elapsed: 5.167 sec  <<< FAILURE!
	
java.lang.AssertionError: expected:<2> but was:<3>	
	at com.rabbitmq.client.test.functional.ConnectionRecovery.connectionRecoveryRequestsCredentialsAgain(ConnectionRecovery.java:155)
	
Results :
Failed tests: 
com.rabbitmq.client.test.functional.ConnectionRecovery.connectionRecoveryRequestsCredentialsAgain(com.rabbitmq.client.test.functional.ConnectionRecovery)

  Run 1: ConnectionRecovery.connectionRecoveryRequestsCredentialsAgain:155 expected:<2> but was:<3>	
  Run 2: ConnectionRecovery.connectionRecoveryRequestsCredentialsAgain:155 expected:<2> but was:<3>

@michaelklishin It can go into 4.5.0, we'll release a milestone once we're done with this PR.

[acogoluegnes]

@spatula75 Failure is reproductible in NIO mode, digging further.

[acogoluegnes]

@spatula75 username is used in AMQConnection#toString, so it's likely to be called for logging (it's the case in the NIO mode). A greaterThanOrEqualTo assertion does the trick.

[acogoluegnes]

@spatula75 Thanks for this! You can have a look at the polish commit. This will be released in 4.5.0.RC2 in the next days.

[spatula75]

Thanks for fixing my bug, @acogoluegnes! I likely missed it by not running the test in NIO mode. And thanks for the amazingly fast turnaround on the PR!

[spatula75]

@acogoluegnes incidentally, I like your solution for setting the username/password in the DefaultCredentialsProvider... did a bit of a face-palm "why didn't I think of that," and I'm tucking it away in the back of my brain for use again later, so thanks for that! Makes the interface more like how I really wanted it to be (with just the getters).