IAM 892 - login handler and authentication middleware implementation according to spec ID036

BarcoMasile commented 1 week ago

Description

This PR introduces the middleware functionalities to authenticate invocations via either a bearer token (access token only, no refresh) or ID and access token (with possible refresh in case one of the tokens is expired and a refresh token is available and valid). This PR also changes the behavior of the callback endpoint which previously returned a json response with three tokens, now redirects to the content of UIPrefix as a URL. This needs to be fixed/improved, and I opened the issue to track this - https://github.com/canonical/identity-platform-admin-ui/issues/337.

Middleware request authentication flow

if the request is against an endpoint whose prefix is allowlisted, then no checks are run, otherwise...
check for bearer token in the Authorization header (CLI use case) 2.a if available, we rely on that as an access token: if validated we go on, otherwise return 401
check for both ID and access tokens (browser user use case) 3.a if both valid, we validate them and if all good we serve the request, otherwise 3.b if one of them is not valid and the error is oidc.TokenExpiredError we try to refresh 3.b.1 if refresh is fine then cookies get set again and the request is served 3.c if one of them is not valid and the error is not a token expired error then return 401
if all went well set the Principal in the context and serve the request
if anything went wrong return 401 and delete all cookies

NB: all encrypted token cookies now rely on the "refresh token" expiration (set via OAUTH2_REFRESHTOKEN_COOKIE_TTL_SECONDS env var): reason is that we need id token and access token be set as cookie for longer than their expiration, otherwise we would never get TokenExpiredError which is needed to trigger the refresh flow. If for example id token cookie expiration is equal to the token expiration, the backend would not be able to receive the expired token (in the just expired cookie), so the backend would just return a 401 since no cookie would be found on the incoming request.

About cookie attributes, some of them must be explicitly set, that's why we have this:

    Name:     name,
    Value:    encrypted,
    Path:     path,
    Domain:   "",
    Expires:  expires,
    MaxAge:   int(ttl.Seconds()),
    Secure:   true,
    HttpOnly: true,
    SameSite: http.SameSiteLaxMode,

Also, both expires and max-age are set to support most browser implementations.

Changes

introduced OAuth2UserSessionCookieTTLSeconds config
added methods on Cookie manager with tests to cover them
Principal object has been expanded on to take advantage of claims coming from the ID token, and also the orignal raw id token, access token and refresh token are kept in the same Principal structure that they generated
middleware has been expanded along with test coverage

BarcoMasile commented 1 week ago

I went ahead and squashed the last commit

BarcoMasile commented 6 days ago

A couple of comments/questions, some may be off:

Why store the access token in a cookie if it is not used? I suppose it is there to implement userinfo authentication, but we could instead use the id token on the introspection endpoint.

Is it sensible to store the tokens in different cookies? It complicates the flow, by making it harder to reason (when examining the security of the flow, eg what happens if I mix and match cookies from different users?)

I replied to the first point in one of the comments, about the other point: unfortunately we have to rely on multiple cookies since the RFC(s) impose a 4096 bytes limit for cookie size, attributes included. A typical encrypted cookie of ours is more than 3k bytes, so it would not be possible to merge them as they are.

canonical / identity-platform-admin-ui