GFlisch / Arc4u

Apache License 2.0
23 stars 18 forks source link

(Arc4u 6+ ??) RefreshTokenProvider doesn't handle access_token refresh properly except for the first time. #89

Closed vvdb-architecture closed 7 months ago

vvdb-architecture commented 11 months ago

When an access token is expired, it's up to StandardCookieEvents to handle the refresh.

Ultimately, this will end up calling ITokenRefreshProvider and therefore RefreshTokenProvider, the RefreshTokenProvider assumes that the return value will always contain a refresh_token. On ADFS 2019, this will never contain a refresh token, only an access_token.

So the next time the RefreshTokenProvider is called, it will fail because the refresh token will be null.

The easy solution is to check if a refresh token is present, and only replace the TokenRefreshInfo.TokenRefresh if so, while being smart about the expiration date. See https://github.com/GFlisch/Arc4u/pull/88.

The code below is an alternative example that does just that and as an added bonus avoids constructing the grant request manually, but use the standardTokenClient instead. But this needs a link to Microsoft's IdentityModel library, Therefore, the code is for illustration and not for a pull request:

using Arc4u;
using Arc4u.Dependency.Attribute;
using Arc4u.Diagnostics;
using Arc4u.OAuth2.Token;
using IdentityModel.Client;
using Microsoft.AspNetCore.Authentication.OpenIdConnect;
using Microsoft.Extensions.Options;
using System.Diagnostics;

/// <summary>
/// The purpose of this token provider is to be used by Oidc one.
/// It will refresh a token based on a refresh token and update the scoped TokenRefreshInfo.
/// The Oidc token provider is responsible to get back an access token.
/// </summary>
[Export(typeof(ITokenRefreshProvider))]
public class RefreshTokenProvider : ITokenRefreshProvider
{
    public const string ProviderName = "Refresh";

    public RefreshTokenProvider(TokenRefreshInfo refreshInfo,
                                IOptionsMonitor<OpenIdConnectOptions> openIdConnectOptions,
                                IActivitySourceFactory activitySourceFactory,
                                ILogger<RefreshTokenProvider> logger)
    {
        _tokenRefreshInfo = refreshInfo;
        _openIdConnectOptions = openIdConnectOptions;
        _logger = logger;
        _activitySource = activitySourceFactory?.Get("Arc4u");
    }

    private readonly TokenRefreshInfo _tokenRefreshInfo;
    private readonly IOptionsMonitor<OpenIdConnectOptions> _openIdConnectOptions;
    private readonly ILogger<RefreshTokenProvider> _logger;
    private readonly ActivitySource? _activitySource;
    private static readonly HttpClient _httpClient = new();

    public async Task<TokenInfo> GetTokenAsync(IKeyValueSettings settings, object platformParameters)
    {
        using var activity = _activitySource?.StartActivity("Get access_token using refresh_token", ActivityKind.Producer);

        // Check if the token refresh is not expired. 
        // if yes => we have to log this and return a Unauthorized!
        if (DateTime.UtcNow > _tokenRefreshInfo.RefreshToken.ExpiresOnUtc)
        {
            _logger.Technical().LogError($"Refresh token is expired: {_tokenRefreshInfo.RefreshToken.ExpiresOnUtc}.");
            throw new InvalidOperationException("Refreshing the token is impossible, validity date is expired.");
        }

        var options = _openIdConnectOptions.Get(OpenIdConnectDefaults.AuthenticationScheme);
        var metadata = await options!.ConfigurationManager!.GetConfigurationAsync(CancellationToken.None).ConfigureAwait(false);

        // Use the token client for standardized access. 
        var tokenClient = new TokenClient(_httpClient, new TokenClientOptions { Address = metadata.TokenEndpoint, ClientId = options.ClientId, ClientSecret = options.ClientSecret });
        var tokenResponse = await tokenClient.RequestRefreshTokenAsync(_tokenRefreshInfo.RefreshToken.Token);
        // check for error while renewing - any error will trigger a new login.
        if (tokenResponse.IsError)
        {
            if (tokenResponse.Exception is not null)
                _logger.Technical().LogException(tokenResponse.Exception);
            else
                _logger.Technical().LogError($"{tokenResponse.ErrorType}: {tokenResponse.Error} {tokenResponse.ErrorDescription}");
            throw new InvalidOperationException("An error occurred refreshing the token");
        }

        if (tokenResponse.ExpiresIn > 0)
        {
            var expirationAt = DateTimeOffset.UtcNow.AddSeconds(tokenResponse.ExpiresIn).DateTime.ToUniversalTime();
            _tokenRefreshInfo.AccessToken = new TokenInfo("access_token", tokenResponse.AccessToken, expirationAt);
            // don't replace the refresh token if none was provided as a result
            if (!string.IsNullOrEmpty(tokenResponse.RefreshToken))
                _tokenRefreshInfo.RefreshToken = new TokenInfo("refresh_token", tokenResponse.RefreshToken, expirationAt);
        }
        else
        {
            _tokenRefreshInfo.AccessToken = new TokenInfo("access_token", tokenResponse.AccessToken);
            // don't replace the refresh token if none was provided as a result
            if (!string.IsNullOrEmpty(tokenResponse.RefreshToken))
                _tokenRefreshInfo.RefreshToken = new TokenInfo("refresh_token", tokenResponse.RefreshToken, _tokenRefreshInfo.RefreshToken.ExpiresOnUtc);
        }

        return _tokenRefreshInfo.AccessToken;
    }

    public ValueTask SignOutAsync(IKeyValueSettings settings, CancellationToken cancellationToken)
    {
        // there is no Signout on a provider for the token refresh...
        throw new NotImplementedException();
    }
}
rdarko commented 7 months ago

In order to fix this problem, people have to upgrate to at least version 6.1.18