snippetcsharpMinor
Create "refresh token" action filter
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(
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
After
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.
Now your code will be much cleaner and shorter..
Before
After
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.