Sliding Cookies not working for implicit flow in IdentityServer4 v4.x

[leendertdommicent]

Issue

When using the implicit flow, the AspNetCore Identity sliding cookie functionality does not work. On the authorize endpoint, the cookie is not refreshed when necessary.

Cause

When rendering the response of the authorize endpoint, the function AddClientAsync is called.

protected async Task ProcessResponseAsync(HttpContext context)
{
    if (!Response.IsError)
    {
        // success response -- track client authorization for sign-out
        //_logger.LogDebug("Adding client {0} to client list cookie for subject {1}", request.ClientId, request.Subject.GetSubjectId());
        await _userSession.AddClientIdAsync(Response.Request.ClientId);
    }

    await RenderAuthorizeResponseAsync(context);
}

This method in DefaultUserSession.cs adds the ClientId to the AuthenticationTicket that is used by the Cookie middleware. This is done by calling the method SignInAsync with the properties of the existing ticket, which includes the expiration time.

public virtual async Task AddClientIdAsync(string clientId)
{
    if (clientId == null) throw new ArgumentNullException(nameof(clientId));

    await AuthenticateAsync();
    Properties?.AddClientId(clientId);
    await UpdateSessionCookie();
}

private async Task UpdateSessionCookie()
{
    await AuthenticateAsync();

    if (Principal == null || Properties == null) throw new InvalidOperationException("User is not currently authenticated");

    var scheme = await HttpContext.GetCookieAuthenticationSchemeAsync();
    await HttpContext.SignInAsync(scheme, Principal, Properties);
}

However the AspNetCore Identity CookieAuthenticationHandler will not refresh the cookie in a scope where the function SignInAsync is called. But since the existing ticket properties are passed to the function, the expire of the cookie is not changed. In the implicit flow where the client only calls the authorize endpoint, the cookie is never refreshed.

In IdentityServer4 v3.1 this does work because the cookie is only updated when the ClientId is not already in the cookie. In v4 the ClientId is only added to the properties when it is not already present, but the cookie is always recreated.

Equivalent code in IdentityServer4 v3.1:

public virtual async Task AddClientIdAsync(string clientId)
{
    if (clientId == null) throw new ArgumentNullException(nameof(clientId));

    var clients = await GetClientListAsync();
    if (!clients.Contains(clientId))
    {
        var update = clients.ToList();
        update.Add(clientId);

        await SetClientsAsync(update);
    }
}

The relevant code snippets of CookieAuthenticationHandler. _signInCalled is set to true, which causes the refresh logic to return immediately.

protected async override Task HandleSignInAsync(ClaimsPrincipal user, AuthenticationProperties? properties)
{
    if (user == null)
    {
        throw new ArgumentNullException(nameof(user));
    }

    properties = properties ?? new AuthenticationProperties();

    _signInCalled = true;

    ...
}

protected virtual async Task FinishResponseAsync()
{
    // Only renew if requested, and neither sign in or sign out was called
    if (!_shouldRefresh || _signInCalled || _signOutCalled)
    {
        return;
    }

    var ticket = _refreshTicket;

    ...
}

A possible solution is to again only recreate the cookie when a new ClientId is added and not every time. This is also currently our workaround with a custom IUserSession implementation.

Other solution could be to clear the expiration timestamp from the ticket properties before calling SignInAsync. However this should only be done when sliding cookies are enabled.

[brockallen]

Confirmed and PR added to only re-issue cookie as needed (same as it used to be).

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.