Simplication of the User Authentication

[jbee]

Intro

Login or authentication has grown into a big jungle of classes that is very hard to understand and also very hard to improve.

One such improvement that is very important is to load all the application level access control (ACL+) belonging to a user with a single SQL query. Ideally there would be one place where this happens.

Another improvement is to make sure that if one method of authentication is tested with high coverage for relevant scenarios after a successful login that the coverage equally applies to other methods of authentication, or on other words that the only difference is how the event of authentication works but from there on they are all guaranteed to be the same.

But the current reality is far from that mainly as clarity is obstructed by terrible Spring abstractions that blurs and scatters the code. The good news is that most of them are 100% optional and are not essential to the core mechanism of Spring security. So this proposal is about a better design to move towards that navigates around most of the current issues by simply not using the optional Spring APIs and replace them with straight forward DHIS2 specific solution. With those changes the DHIS2 specific model can easily be simplified to be more clear, slimmer, more accessible and easier to fetch.

What does it mean to be "authenticated"?

In Spring a successful authentication or login is indicated by putting a Authentication instance in the SecurityContext using setAuthentication.
The only key quality of the Authentication is that isAuthenticated() must return true and that getAuthorities() returns the set of granted authorities. But even that set is mostly theoretical in the DHIS2 context as DHIS2 does not use Spring mechanisms that directly would call this method to decide access (like @PreAuthorize). While the Authentication getName() should provide a unique name it has no functional role and is only informal in e.g. log messages. A logout means to clear the context and set it to null.

While not essential to Spring security itself the second role of the Authentication that is put in the SecurityContext is to hold application level data linked to the user session. This is done via the getPrincipal() object. In DHIS this is the ACL related UserDetails data. But making it a part of the Authentication this data automatically shares the same life-cycle.

Important

The above in essence is the entirety of the "mandatory" Spring security mechanism. It is crucial to understand that all the concepts detailed below are just a means to get to that very result of a Authentication being set in the SecurityContext. But in practice it does not matter how the authentication arrives there.

Where things are now

Spring API induced Confusion AuthenticationManager

The core Spring API to perform the authentication itself is AuthenticationManager which has a single method:

Authentication authenticate(Authentication authentication) throws AuthenticationException;

The first confusion is that input and output are both objects implementing Authentication but they are actually very different things.
The input Authentication captures the user input for the specific form/method of authentication.
The output Authentication on the other hand is supposed to describe the authenticated user as detailed above in the essential mechanism in terms of the getAuthorities() and the application specific object hidden behind getPrincipal().

In current code this confusion has caused the input to be extended with authorities and/or the output to drag along details from the authentication user input as if these were needed later on.

Because of this API it is never clear what data is worked with and the code has to perform many casts or instanceof checks with often no alternative other than to fail if a type assumption does not hold true.

Spring API induced Confusion AuthenticationProvider

The second confusion originates from the invisible magic of class based dispatch. One AuthenticationManager implements a particular authentication method, like HTTP basic. So there are several implementations but they do not directly indicate themselves what method they can handle. This is task is handled by a AuthenticationProvider which has a supports method:

boolean supports(Class<?> authentication);

The Class here are implementing classes of Authentication in their role of a data container for the user input needed by the method to perform the authentication.

Note

The irony of the supports based dispatch is that the code constructing the proper input (often called "token" classes) is application code, the code consuming this is also application code (in most instances). So we go through a complicated and confusing generic API just to make a method call that could have been hard coded using a concrete input and output type that enforce a very clear and specific contract and thereby exclude a lot of error surface by design.

The process or Authentication

The process in its essence is quite simple but unfortunately Spring provides countless abstractions that complicate the idea (the most central being pointed out above but there are many more).

Conceptually an authentication has these steps

1. HTTP request received by server
2. Request Filter identifies authentication method (mostly looking at headers)
3. Client provided input (credentials of sorts) are extracted and put in the appropriate data container for the authentication method (a implementation of Authentication when using AuthenticationManagers)
4. authentication is performed (AuthenticationManager.authenticate)
5. if successful a user authority descriptor is created (again a Authentication this time as returned by authenticate)
6. the descriptor is put in the SecurityContext (and some other stuff with spring level events signaling the login)

A a data transformation this look like this

HTTP request => Authentication (token/credentials) => Authentication (authorities) =>  SecurityContext

Note

As discussed above using the Spring APIs is more of an obstacle than providing a solution. These APIs provide layers of glue code to the actual implementation of the authorization method itself. But within DHIS2 most methods needed customization anyhow and so it is DHIS2 code that calls the implementation just that each step and layer is wrapped by the Spring API for the concept. At that point all the layers and wrapping is just an obstacle to clarity and source of complications and ceremony bloat.

Proposal

The main idea is to move away from using the optional Spring API layers to gain a simple, easy to follow implementation where each of the steps of an authentication as outlined above is clearly visible as a code component and where inputs and outputs are strongly typed enforcing a proper contract through types and the compiler that excludes most issues by design and which provides a common chain of execution even if the particulars of different authentication methods are vastly different.

Note

The names chosen in the code sketches will lean on the corresponding Spring APIs to make clear which part they correspond to
but in an actual implementation there surely are better more clear names to be found.

A DHIS2 specific interface details the exact contract of all authentication methods using only DHIS2 defined data types

interface AuthService {
  AuthenticatedUser authenticate(HTTPBasicParams params);
  AuthenticatedUser authenticate(JWTParams params);
  AuthenticatedUser authenticate(ApiKeyParams params);
  AuthenticatedUser authenticate(ApiKeyParams params);
  AuthenticatedUser authenticate(OAuthParams params);
  AuthenticatedUser authenticate(OIDCParams params);
}

The inputs here would be records with the essential inputs as they have been extracted from the request filters (mostly from HTTP headers) and not as they might be needed by the underlying implementation (that transformation happens within the authenticate method).
For example for HTTP basic authentication:

record HTTPBasicParams(String username, String password, String twoFactorCode) {}

The other params classes would similarly contains only what is needed in the DHIS2 case for each of the mechanisms.

The output is always a AuthenticatedUser which implements Authentication but other than that is only a wrapper around the UserDetails as the getPrincipal:

record AuthenticatedUser(UserDetails getPrincipal) implements Authentication {
  public boolean isAuthenticated() {
    return true;
  }
  //...
}

Before the AuthService returns a AuthenticatedUser it is properly set in the SecurityContext which also means there is a single point with the service where based on the user row identified by the authentication method the UserDetails are loaded and packaged into the AuthenticatedUser providing a single code path where this occurs.

Internally each of the authenticate methods performs the authentication either purely using DHIS2 fundamentals (e.g. username+password+2fa) or by providing the adapter/glue to the implementation handled by a 3rd party library. In cases where this is more complicated it seems reasonable to extract a single class for an authentication method that contains all the details of performing the authentication. For example, in case of JWT the process is much more involved than HTTP basic.

For UserDetails this means there can be a single class or record holding only the DHIS2 specific data we need and none of the input to authenticate. Also DHIS2 UserDetails should not implement Spring's UserDetails. It is a data container that purely exist to satisfy DHIS2 access control needs (mostly ACL and OU-tree based restrictions) as well as some common identifiers for the current user that are needed as user context.

Based on a attempt to refactor to such a single container it could look something like this

record UserDetails(
  String uid,
  String username,
  Long id,
  String email,
  boolean isEmailVerified,
  Integer dataViewMaxOrganisationUnitLevel,
  Set<String> authorities,
  Set<String> allRestrictions,
  Set<UID> userGroupIds,
  Set<UID> userOrgUnitIds,
  Set<UID> userDataOrgUnitIds,
  Set<UID> userSearchOrgUnitIds,
  Set<UID> userEffectiveSearchOrgUnitIds,
  Set<UID> userRoleIds,
  Set<UID> managedGroupIds
) {}

Note that I removed most of the questionable fields that have a role in performing authenticate but that are (likely) not required once the user is authenticated. I also eliminated sets of Long IDs. IDs should all be UIDs.

In this model the current SuperUser class simply becomes a constant SUPER_USER defined in the record with the appropriate values hard coded. Test ideally all use AuthService to authenticate, e.g. with username/password using a HTTPBasicParams input.

Journey

While the goal is pretty clear getting there is quite a lot of work to disentangle from the Spring ceremony and to simplify the code.

Actions identified are as follows (in order)

Actions:

• remove JSON serialize of UserDetails
• create e2e tests for all login methods
• create DHIS2 version for each spring filter (replace spring where still used)
• merge these filters into a universal auth filter handling all supported methods (the extraction of "inputs")
• move code towards the DHIS2 own API for AuthService as detailed above
• simplify UserDetails data and its loading to occur in single SQL query in a common code path in the new service