LitJWT icon indicating copy to clipboard operation
LitJWT copied to clipboard

Flexibilice token lifetime validation

Open SergioLuis opened this issue 2 years ago • 6 comments

Goals

Allowing more flexible token lifetime validation.

Existing APIs remain unchanged and do not break compatibility.

New APIs allow client code to do a flexible token lifetime validation:

  • Client code can now specify a clock skew for the lifetime validation
    • Accounting for clock skew is something recommended in JWT RFC (sections 4.1.4 and 4.1.5 on RFC 7519)
  • Client code can now provide their own lifetime validation callback.
  • Client code can now skip lifetime validation altogether.

Code examples

Specifying clock skew when validating lifetime:

string token = "...";
JwtDecoder decoder = new(...);

var tokenValidationParameters = new TokenValidationParameters<TokenModel>()
{
    ClockSkew = TimeSpan.FromMinutes(5)
};

DecodeResult result = decoder.TryDecode<TokenModel>(token, tokenValidationParameters, out TokenModel result);

Specifying your own lifetime validator function when validating lifetime:

string token = "...";
JwtDecoder decoder = new(...);

var tokenValidationParameters = new TokenValidationParameters<TokenModel>()
{
    LifetimeValidator = (before, expire, token, parameters) =>
    {
        DateTimeOffset now = DateTimeOffset.UtcNow;
        if ((now - before).Duration() > parameters.ClockSkew)
            return DecodeResult.FailedVerifyNotBefore;
        if ((expire - now).Duration() > parameters.ClockSkew)
            return DecodeResult.FailedVerifyExpire;

        return DecodeResult.Success;
    }
};

DecodeResult result = decoder.TryDecode<TokenModel>(token, tokenValidationParameters, out TokenModel result);

Skip lifetime validation altogether:

string expiredToken = "...";
JwtDecoder decoder = new(...);

var tokenValidationParameters = new TokenValidationParameters<TokenModel>()
{
    ValidateLifetime = false
};

DecodeResult result = decoder.TryDecode<TokenModel>(token, tokenValidationParameters, out TokenModel result);
// result is different than DecodeResult.FailedVerifyNotBefore and DecodeResult.FailedVerifyExpire

Work done / to review

  • Adds new TokenValidationParameters class
    • This class allows (for now, although it can be extended) how the token lifetime ("not before" and "expire" token properties) are validated
  • Adds new JwtDecoder.TryDecode overloads accepting the new TokenValidationParameters class.
    • The existing APIs are not affected by these changes, nor are their behaviours changed.
  • Removes internal delegate InternalPayloadParser
    • By removing this delegate, all TryDecode public APIs can rely on internal TryDecodeCore methods.
    • By removing this delegate, all duplicated copy-pasted implementations of TryDecode could be removed.
  • Add new test cases to cover all what was implemented.
  • Tidy up a little bit some code in the JwtDecoder class.

SergioLuis avatar Jul 30 '22 12:07 SergioLuis

Last commit 0d2f36a solves what I believe to be a security flaw in this library (or better put - something that could lead to a security flaw in client code).

I would create a separate Pull Request just for this, but the changes are pretty much in line with the rest of the work.

SergioLuis avatar Aug 01 '22 13:08 SergioLuis

Hello @neuecc (pinging you as the main contributor to this repository, hope that's OK) and the Cysharp team!

First of all, thank you for your fantastic work on Open Source libraries such as this. Having high-quality libraries that provide low-level, mission-critical functionality like JWT token generation and validation is truly fantastic. I stumbled upon this one precisely looking for something fast and with low memory usage and LitJwt blew my mind.

While integrating LitJwt within our system, I found some room for improvement. Instead of opening a new GitHub issue requesting changes, and being inspired by your work, I decided to take matters into my own hands and I filled the gaps I found, and that would make LitJwt better for my use case and hopefully for others too. I hope that that's OK with the Cysharp team.

Please let me know if this PR is up to your standards, and in case it is not, what can I change in order to get it merged and released.

Thank you again!

SergioLuis avatar Aug 02 '22 07:08 SergioLuis

Thank you. From what I have seen, I think it is very good. As for the review, I need a little more time, wait for a moment.

neuecc avatar Aug 02 '22 08:08 neuecc

Hi @neuecc Following up on this - is there anything I can do to help get this merged and released? 🙂

SergioLuis avatar Aug 22 '22 10:08 SergioLuis

Sorry for the late review. Hmmm, I think there are a lot of alterations to the original code. For example. TryDecodeCore(token.AsSpan(), InternalPayloadParser<T>, null, out payloadResult); and InternalPayloadParser<T> are undesirable. (recently improved, but there used to be an allocation)

neuecc avatar Sep 30 '22 11:09 neuecc

Also, there are too many code style modifications, so it's hard to read from the diffs. It's tough to get them to redo the code with a cleaner code that fits the project style a little better.

neuecc avatar Sep 30 '22 11:09 neuecc