[Proposal] Clarify authentication method interfaces

User Story

As a developer I want clean interfaces to work to for adding authentication methods, so if/when we're asked to add more it can be done easily and securely without breaking core assumptions of our security model

Any solution to this should be documented as part of the ADR system and will likely supersede some of ADR-0015

The Problem

At the moment new Authentication methods (e.g. OIDC, User accounts, Token Passthrough) are added by implementing the PrincipalGetter interface:

type PrincipalGetter interface {
  Principal(r *http.Request) (*UserPrincipal, error)
}

pkg/auth/jwt.go

PrincipalGetters for the enabled authentication methods are then added to to a MultiAuthPrincipal and iterated over, calling Principal(request), to find one that succeeds. From this pattern it is unclear that Principal is also expected to authenticate the request (rather than just return a UserPrincipal).

There is also no uniform method for configuring authentication methods. This is currently carried out via an ad-hoc collection of CLI flags and by reading kubernetes secrets.

A potential solution

(This is just my thoughts on one way to improve this, feel free to disregard etc)

The PrincipalGetter interface is replaced with an AuthenticationMethod interface, something like:

type PrincipalGetter interface {
    Initialise(l logr.logger, k ctrlclient.Client) error
    IsAuthenticated(r *http.Request) (bool, error)
    Principal(r *http.Request) (*UserPrincipal, error)
}

IsAuthenticated explicitly checks that the user is authenticated whereas Principal returns user information. Often times these will overlap in what they do (e.g. parsing cookies etc) but by separating them we can make explicit the two tasks we currently expect Principal to carry out.

The Initialise method is expected to set up any configuration that the authentication method needs (e.g. calling the OIDC /.well-known endpoints). To reduce the number of CLI arguments the Initialise function would be expected to read configuration from environment variables or from Kubernetes.

weaveworks / weave-gitops