Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExchangeFilterFunctions Explicit Model For Basic Authentication Credentials [SPR-15764] #20319

Closed
spring-issuemaster opened this issue Jul 12, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Jul 12, 2017

Rob Winch opened SPR-15764 and commented

Currently ExchangeFilterFunctions provides a USERNAME_ATTRIBUTE and a PASSWORD_ATTRIBUTE and will populate the credentials in a Basic authentication header if the attributes are found.

I think this will lead to issues if we provide other ways to authenticate. Consider a client that adds both basic authentication and digest authentication (if this is eventually supported). The user wants to specify to use basic authentication so the attributes are provided. However, digest authentication overrides the basic authentication header.

Instead, it might be better to use a richer domain model for the attributes to ensure the users choice is clear.

The domain model might even provide a way to add itself to read / write the model to the attributes. For example, something like this:

public class BasicAuthenticationCredential implements Consumer<Map<String,Object>> {
	private static String ATTRIBUTE_NAME = BasicAuthenticationCredential.class.getName().concat(".ATTRIBUTE_NAME");

	private final String username;
	private final String password;


	BasicAuthenticationCredential(String username, String password) {
		this.username = username;
		this.password = password;
	}

	public String getUsername() {
		return username;
	}

	public String getPassword() {
		return password;
	}

	@Override
	public void accept(Map<String, Object> attributes) {
		attributes.put(ATTRIBUTE_NAME, this);
	}

	public static BasicAuthenticationCredential get(Map<String,Object> attributes) {
		return (BasicAuthenticationCredential) attributes.get(ATTRIBUTE_NAME);
	}
}

Consumers would then be able to use:

client.get()
		.uri("/messages/20")
		// perhaps add static factory method for BasicAuthenticationCredential
		.attributes(new BasicAuthenticationCredential("joe", "joe"))

Comparing that vs:

client.get()
	.uri("/messages/1")
	.attribute(ExchangeFilterFunctions.USERNAME_ATTRIBUTE, "rob")
	.attribute(ExchangeFilterFunctions.PASSWORD_ATTRIBUTE, "rob")

Plus now we would be able to differentiate between the two credential types because we would add a DigestAuthenticationCredential if we supported it later on.


Referenced from: commits 1d86c9c

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 27, 2017

Arjen Poutsma commented

Implemented in 1d86c9c

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 27, 2017

Arjen Poutsma commented

I chose to take a slightly different path than suggested: rather than having a credentials object specific to Basic Authentication, we have a generic Credentials object stored under a BA-specific key. Digest authentication would use the same Credentials class, but under a different key. As a consequence, it also does not implement Consumer<Map<String,Object>>, but does expose a Consumer with the static methodbasicAuthenticationCredentials.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 27, 2017

Rob Winch commented

This works great! Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.