HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcsharpMinor

Create "refresh token" action filter

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
createrefreshactiontokenfilter

Problem

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 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(

Solution

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));

Code Snippets

//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);
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);
}
public static string GetClaimsType(string type)
{
    return ClaimsPrincipal.Current.Claims.Where(c => c.Type == type).Select(c => c.Value).SingleOrDefault();
}
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()));
var refreshToken = GetClaimsType(AuthenticationValues.RefreshTokenKey);
long epoch = Convert.ToInt64(GetClaimsType(AuthenticationValues.ExpiresAtEpochKey));

Context

StackExchange Code Review Q#121417, answer score: 4

Revisions (0)

No revisions yet.