Skip to content
This repository has been archived by the owner on Sep 18, 2021. It is now read-only.

Reference Token Validation throws exception #826

Closed
TD49 opened this issue Jan 26, 2015 · 10 comments
Closed

Reference Token Validation throws exception #826

TD49 opened this issue Jan 26, 2015 · 10 comments
Assignees

Comments

@TD49
Copy link
Contributor

TD49 commented Jan 26, 2015

While we have successfully retrieved and validated self-contained JWT-based Access Tokens, I'm having issues validating a Reference Token type using the token validation endpoint (with the default in-memory TokenValidationStore).

Request to the validator (using a fake ref token for this post):
https://myidsvr3/core/connect/accesstokenvalidation?token=2fc954023e5f71286ac5d018e301f8f7 GET

It fails with a System.NullReferenceException when adding claims to the ClaimsIdentity, starting here: https://github.com/IdentityServer/Thinktecture.IdentityServer3/blob/master/source/Core/Services/Default/DefaultCustomTokenValidator.cs#L78

Stacktrace snippet:

ExceptionType: "System.NullReferenceException"
StackTrace: " at System.Security.Claims.ClaimsIdentity.SafeAddClaims(IEnumerable`1 claims) at System.Security.Claims.ClaimsIdentity..ctor(IIdentity identity, IEnumerable`1 claims, String authenticationType, String nameType, String roleType, Boolean checkAuthType) at System.Security.Claims.ClaimsIdentity..ctor(IIdentity identity, IEnumerable`1 claims, String authenticationType, String nameType, String roleType) at Thinktecture.IdentityServer.Core.Services.Default.DefaultCustomTokenValidator.<ValidateAccessTokenAsync>d__4.MoveNext() in c:\etc\Dropbox\thinktecture\IdentityServer.v3\Core\source\Core\Services\Default\DefaultCustomTokenValidator.cs:line 78 

Note that, for the above example, there are no custom claims or anything special being added or used with the Client. The token was originally retrieved through an RO grant type request. We're using IdSvr3 RTM released yesterday. Any ideas?

@TD49
Copy link
Contributor Author

TD49 commented Jan 26, 2015

Client details:

new Client
{
    Enabled = true,
    ClientName = "CustomApp",
    ClientId = "customapp",
    ClientSecrets = new List<ClientSecret> { new ClientSecret("customsecret".Sha256())},
    AccessTokenType = AccessTokenType.Reference,
    AccessTokenLifetime = 300,
    RefreshTokenUsage = TokenUsage.OneTimeOnly,
    RefreshTokenExpiration = TokenExpiration.Absolute,
    AbsoluteRefreshTokenLifetime = 86400, // one day
    SlidingRefreshTokenLifetime = 43200,
    Flow =  Flows.ResourceOwner
}

@leastprivilege
Copy link
Member

have you debugged it?

@leastprivilege
Copy link
Member

i just tried to repro it. works for me.

Try with the RO flow sample from the Clients solution (sample repo together with the core source code repo) and see if that works for you when you switch the token type to reference.

@TD49
Copy link
Contributor Author

TD49 commented Jan 26, 2015

Yes.

I was off by one line earlier, the Claims in the TokenValidationResult at https://github.com/IdentityServer/Thinktecture.IdentityServer3/blob/master/source/Core/Services/Default/DefaultCustomTokenValidator.cs#L76 are populated.

When it attempts to validate the access token in async is where it loses track here https://github.com/IdentityServer/Thinktecture.IdentityServer3/blob/master/source/Core/Validation/TokenValidator.cs#L161. I can't debug any further, as it then jumps to the Logger to log the exception listed in the earlier stacktrace above.

@TD49
Copy link
Contributor Author

TD49 commented Jan 26, 2015

Ok, just caught your comment about the samples repo. I'll give that a try.

@leastprivilege
Copy link
Member

As I said - download the source code and the client repo and debug it.

@TD49
Copy link
Contributor Author

TD49 commented Jan 26, 2015

You're correct, I couldn't repro using the IdSvr3 Core host from the source code repo (master).

The only difference on our setup is that we're using a custom user service to validate credentials, but that shouldn't cause this problem since a reference token is still being issued. The validation endpoint is able to trap it when the reference token is expired or invalid (with the "invalid_token" response). Plus I can see the claims when debugging on the lines mentioned above.

From the earlier stacktrace, I take it somehow a custom claim is being added that it can't process... I'll keep digging, at a loss for the moment.

@leastprivilege
Copy link
Member

Well - maybe you IsActiveAsync implementation does not return true...?

@TD49
Copy link
Contributor Author

TD49 commented Jan 26, 2015

Nope, I verified it returns true on debug.

However, I just noticed we had a custom IClaimsProvider type still registered that was forgotten during testing. Removing that registration to use the default provider "fixes" the problem, but I want to check the logic within the custom type to see if adding custom claims is causing this issue. Shouldn't take long...

@TD49
Copy link
Contributor Author

TD49 commented Jan 26, 2015

Ok, so our own custom IClaimsProvider type had dev code that was adding a NULL claim to the claims list. If the custom user service didn't add a "MyCustomClaim", then the claims provider can't find it and ends up adding a NULL claim per the logic below:

public override async Task<IEnumerable<Claim>> GetAccessTokenClaimsAsync(ClaimsPrincipal subject, Client client, IEnumerable<Scope> scopes, ValidatedRequest request)
    {
        var claims = await base.GetAccessTokenClaimsAsync(subject, client, scopes, request);

        var newClaims = claims.ToList();
        newClaims.Add(subject.FindFirst("MyCustomClaim"));   // BAD!

        return newClaims;
    }

So when the token is validated, it balks on that NULL. Not a very informative exception, but it's accurate. Here's a simple dev fix for this scenario:

        // newClaims.Add(subject.FindFirst("MyCustomClaim"));   // BAD!
        var newClaim = subject.FindFirst("MyCustomClaim"); 
        if (newClaim != null)
            newClaims.Add(newClaim);

This isn't a bug with IdSvr, but just something we now watch out for when messing with custom claims in the provider...

@TD49 TD49 closed this as completed Jan 27, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants