Skip to content

Allow ISO Date encoded timestamp fields in JWT tokens. #6187

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

Closed
greyfairer opened this issue Nov 29, 2018 · 3 comments
Closed

Allow ISO Date encoded timestamp fields in JWT tokens. #6187

greyfairer opened this issue Nov 29, 2018 · 3 comments
Labels
status: duplicate A duplicate of another issue

Comments

@greyfairer
Copy link

Summary

Both Auth0 and OneLogin identity providers use ISO Date strings for the 'updated_at' claim in JWT Tokens, even if the spec says these should be numeric (unix timestamp). https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims

If this happens, the StandardClaimAccessor#getUpdatedAt throws exceptions.

Actual Behavior

If you use an OpenID Connector that writes the 'updated_at' claim in a JWT token in ISO Date format instead of a numerical unix timestamp, the getters on OidcUserInfo throw errors, so you cannot use e.g. jackson to serialize this user info.

Expected Behavior

In general, getters should not throw exceptions. If the field is invalid, the constructor or the setter should throw an exception, or the field should be null.

In this case, it would be nice to accept ISO Date formatted timestamps as valid values, since apparently this is used by at least two major OIDC vendors.

Version

5.1.1.RELEASE

Sample

@Controller
public class MyCollaboratorInfoController {
    @RequestMapping(value = "/myCollaboratorInfo.json")
    @ResponseBody
    public Principal currentUser(Principal principal) {
        return principal; //throws java.lang.IllegalArgumentException: Unable to convert claim 'updated_at' of type 'class java.lang.String' to Instant.
    }

    @RequestMapping(value = "/myCollaboratorInfo2.json")
    @ResponseBody
    public Map<String, Object> currentUser2(Principal principal) {
        return ((OAuth2AuthenticationToken)principal).getPrincipal().getAttributes(); //workaround that avoids the ClaimAccessor issue.
    }
@greyfairer
Copy link
Author

See #5250 (comment)
in #5250 by @sirianni

@jgrandja jgrandja self-assigned this Nov 29, 2018
@jgrandja jgrandja added type: enhancement A general enhancement in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) labels Nov 29, 2018
@jgrandja jgrandja added this to the 5.2.0.M1 milestone Nov 29, 2018
@jgrandja
Copy link
Contributor

@greyfairer Agreed...

getters should not throw exceptions

I'll apply some improvements here.

@jgrandja
Copy link
Contributor

jgrandja commented Dec 6, 2018

@greyfairer After giving this some further thought, we need to apply some more involved changes to ClaimAccessor to ensure it's more robust going forward. I have logged #6245 detailing the required changes. We're going to target this improvement for the 5.2.0 release.

Given this, I'm going to close this issue and associated PR as ClaimAccessor needs a bigger improvement compared to what is in the PR. I apologize for the back-and-forth and really appreciate your efforts and bringing up this issue so we can address it for the next release.

@jgrandja jgrandja closed this as completed Dec 6, 2018
@jgrandja jgrandja added status: duplicate A duplicate of another issue and removed type: enhancement A general enhancement in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) labels Dec 6, 2018
@jgrandja jgrandja removed their assignment Dec 6, 2018
@jgrandja jgrandja removed this from the 5.1.3 milestone Dec 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

3 participants