Uploaded image for project: 'Spring Framework'
  1. Spring Framework
  2. SPR-15764

ExchangeFilterFunctions Explicit Model For Basic Authentication Credentials

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Complete
    • Affects Version/s: None
    • Fix Version/s: 5.0 RC4
    • Component/s: Web:Client
    • Labels:
      None
    • Last commented by a User:
      false

      Description

      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.

        Activity

        Show
        arjen.poutsma Arjen Poutsma added a comment - Implemented in https://github.com/spring-projects/spring-framework/commit/1d86c9c3d128f64afa51460e3ed7900963755cfd
        Hide
        arjen.poutsma Arjen Poutsma added a comment -

        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 method{{basicAuthenticationCredentials}}.

        Show
        arjen.poutsma Arjen Poutsma added a comment - 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 method{{basicAuthenticationCredentials}}.
        Hide
        rwinch Rob Winch added a comment -

        This works great! Thanks

        Show
        rwinch Rob Winch added a comment - This works great! Thanks

          People

          • Assignee:
            arjen.poutsma Arjen Poutsma
            Reporter:
            rwinch Rob Winch
            Last updater:
            St├ęphane Nicoll
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:
              Days since last comment:
              16 weeks, 1 day ago