Create "refresh token" action filter

Question

I am using idenity server 3 authentication. I have to implement refresh token logic. AccessTokenLifetime is 1 hour, and after that time I want to update access token with refresh token.

I want do this only once, by using filter in mvc. My idea is to check if token has expired, and if yes that get new token.

This is my code, I want to know if this is best practice or you maybe have better idea?

  public class RefreshTokenActionFilter : FilterAttribute, IActionFilter
        {
            void IActionFilter.OnActionExecuted(ActionExecutedContext filterContext)
            {
                var user = filterContext.HttpContext.User as ClaimsPrincipal;
                var request = filterContext.HttpContext.Request;
                RefreshToken(user, request);
            }

            void IActionFilter.OnActionExecuting(ActionExecutingContext filterContext)
            {
            }

            private  void RefreshToken(ClaimsPrincipal User, HttpRequestBase Request)
            {
                var claims = ClaimsPrincipal.Current.Claims;
                var refreshToken = claims.Where(c => c.Type == AuthenticationValues.RefreshTokenKey).Select(c => c.Value).SingleOrDefault();
                long epoch = Convert.ToInt64((claims.Where(c => c.Type == AuthenticationValues.ExpiresAtEpochKey).Select(c => c.Value).SingleOrDefault()));

                // 60 seconds is tolerance
                if (refreshToken != null && epoch - 60 < DateTime.UtcNow.ToEpochTime())
                {
                    var tokenClient = new TokenClient(ApplicationConfiguration.IdentityServerToken,
                                      AuthenticationController.ClientApplicationId,
                                      AuthenticationController.ClientApplicationSecret);

                    var response = (tokenClient.RequestRefreshTokenAsync(refreshToken)).Result;
                    var user = User as ClaimsPrincipal;
                    var identity = user.Identity as ClaimsIdentity;

                    //Remove old tokens:
                    var oldRefreshToken = user.Claims.Single(c => c.Type == AuthenticationValues.RefreshTokenKey);
                    var oldAccessToken = user.Claims.Single(c => c.Type == AuthenticationValues.AccessTokenKey);
                    var oldExpiresAt = user.Claims.Single(c => c.Type == AuthenticationValues.ExpiresAtKey);
                    var oldEpoch = user.Claims.Single(c => c.Type == AuthenticationValues.ExpiresAtEpochKey);
                    identity.RemoveClaim(oldRefreshToken);
                    identity.RemoveClaim(oldAccessToken);
                    identity.RemoveClaim(oldExpiresAt);
                    identity.RemoveClaim(oldEpoch);

                    //Add new tokens:
                    identity.AddClaim(new Claim(AuthenticationValues.AccessTokenKey, response.AccessToken));
                    identity.AddClaim(new Claim(AuthenticationValues.RefreshTokenKey, response.RefreshToken));
                    identity.AddClaim(new Claim(AuthenticationValues.ExpiresAtKey, (DateTime.UtcNow.ToEpochTime() + response.ExpiresIn).ToDateTimeFromEpoch().ToString()));
                    identity.AddClaim(new Claim(AuthenticationValues.ExpiresAtEpochKey, (DateTime.UtcNow.ToEpochTime() + response.ExpiresIn).ToString()));

                    var authenticationManager = Request.GetOwinContext().Authentication;

                    authenticationManager.AuthenticationResponseGrant = new AuthenticationResponseGrant(
                                                                                       new ClaimsPrincipal(identity),
                                                                                       new AuthenticationProperties { IsPersistent = true });
                }
            }
        }

Answer 1

I can see repetitive code which you can fix easily..

Before

//Remove old tokens:
var oldRefreshToken = user.Claims.Single(c => c.Type == AuthenticationValues.RefreshTokenKey);
var oldAccessToken = user.Claims.Single(c => c.Type == AuthenticationValues.AccessTokenKey);
var oldExpiresAt = user.Claims.Single(c => c.Type == AuthenticationValues.ExpiresAtKey);
var oldEpoch = user.Claims.Single(c => c.Type == AuthenticationValues.ExpiresAtEpochKey);
identity.RemoveClaim(oldRefreshToken);
identity.RemoveClaim(oldAccessToken);
identity.RemoveClaim(oldExpiresAt);
identity.RemoveClaim(oldEpoch);

After

var tokens = user.Claims
    .Where(c =>
        c.Type == AuthenticationValues.RefreshTokenKey ||
        c.Type == AuthenticationValues.AccessTokenKey ||
        c.Type == AuthenticationValues.ExpiresAtKey ||
        c.Type == AuthenticationValues.ExpiresAtEpochKey);

foreach (var token in tokens)
{
    identity.RemoveClaim(token);
}

---

You have filter claims by its type many times, which is similar code. You can get rid of duplicate code by introducing a method. I am not sure about the return or parameter type but it look somehow like below.

public static string GetClaimsType(string type)
{
    return ClaimsPrincipal.Current.Claims.Where(c => c.Type == type).Select(c => c.Value).SingleOrDefault();
}

Now your code will be much cleaner and shorter..

Before

var claims = ClaimsPrincipal.Current.Claims;
var refreshToken = claims.Where(c => c.Type == AuthenticationValues.RefreshTokenKey).Select(c => c.Value).SingleOrDefault();
long epoch = Convert.ToInt64((claims.Where(c => c.Type == AuthenticationValues.ExpiresAtEpochKey).Select(c => c.Value).SingleOrDefault()));

After

var refreshToken = GetClaimsType(AuthenticationValues.RefreshTokenKey);
long epoch = Convert.ToInt64(GetClaimsType(AuthenticationValues.ExpiresAtEpochKey));