Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ClockSkew Ignored by OAuthBearerAuthenticationHandler #507

Open
DwayneNickels-tyler opened this issue Jul 10, 2023 · 3 comments
Open

ClockSkew Ignored by OAuthBearerAuthenticationHandler #507

DwayneNickels-tyler opened this issue Jul 10, 2023 · 3 comments
Milestone

Comments

@DwayneNickels-tyler
Copy link

This is a duplicate of #219, but that issue was closed and there didn't seem to be a fix for the issue.

I am using a custom JwtSecurityTokenHandler class. The ValidateToken method is overridden in the custom class. The custom JwtSecurityTokenHandler class uses a TokenValidationParameters.ClockSkew value of 5 minutes. The custom JwtSecurityTokenHandler class calls base.ValidateToken(...).

If I receive a bearer token at 9:01:00 with an expiration datetime of 9:00:00, the call to base.ValidateToken returns a ClaimsPrincipal and doesn't throw any exceptions because 9:01:00 is within the ClockSkew grace period (the ValidateToken call is successful up until 9:05:00). In that regard, the code in the custom JwtSecurityTokenHandler is working as expected. However, I'm finding that once the control flow leaves the custom JwtSecurityTokenHandler class, then something unexpected happens. The call fails with a 401 HTTP response code. OWIN logging captures the log event below:

Microsoft.Owin.Security.OAuth.OAuthBearerAuthenticationMiddleware Warning: 0 : expired bearer token received

The issue appears to be that the OAuthBearerAuthenticationHandler doesn't take into account the ClockSkew setting.

// Validate expiration time if present
DateTimeOffset currentUtc = Options.SystemClock.UtcNow;

if (ticket.Properties.ExpiresUtc.HasValue &&
    ticket.Properties.ExpiresUtc.Value < currentUtc)
{
    _logger.WriteWarning("expired bearer token received");
    return null;
}

Is this a bug in the OAuthBearerAuthenticationHandler class or a misunderstanding on my part of how JwtSecurityTokenHandler / TokenValidationParameters.ClockSkew / OAuthBearerAuthenticationHandler are supposed to collectively work together?

@Tratcher
Copy link
Member

The problem is that it doesn't know about the ClockSkew option, that's coming from an external component. At best you could skew the SystemClock abstraction by 5 minutes.

@DwayneNickels-tyler
Copy link
Author

I'm not finding an easy way to override the SystemClock implementation. It looks like UseJwtBearerAuthentication creates the OAuthBearerAuthenticationOptions instance internally. That OAuthBearerAuthenticationOptions instance contains the SystemClock property. Once the OAuthBearerAuthenticationOptions instance is built and added to the middleware collection of AppBuilder, it's not obvious to me how to obtain a reference to it in order to alter the SystemClock property. Is there an accepted way of doing such a thing?

@Tratcher Tratcher added this to the Discussions milestone Nov 16, 2023
@elements-of-boredom
Copy link

Just ran into this issue as well, took a bit to track down as we were setting the ClockSkew on the TokenValidationParameters and not seeing them respected. Adjusting the SystemClock requires you to subtract your clockskew from UtcNow which does work but it also means you end up with "now" being before your iat/nbf . For this use case its not the end of the world because this Middleware class doesn't check those properties, and is the route I went, but you could alternately simply implement your own version of OAuthBearerAuthenticationHandler (extend from AuthenticationHandler and just copy/paste the existing implementation with your tweaks). Then you just need to register the middleware itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants