Skip to content
This repository has been archived by the owner on Feb 27, 2023. It is now read-only.

optional date claims #220

Merged
merged 3 commits into from
Feb 26, 2019
Merged

optional date claims #220

merged 3 commits into from
Feb 26, 2019

Conversation

nelz9999
Copy link
Contributor

Solves #219

@CLAassistant
Copy link

CLAassistant commented Feb 25, 2019

CLA assistant check
All committers have signed the CLA.

@nelz9999 nelz9999 changed the base branch from master to v2 February 25, 2019 04:35
@nelz9999
Copy link
Contributor Author

Aww jeez, t.Run(...) doesn't exist pre-1.7. 😢 I'll have to tidy that up later tonight.

@mcpherrinm
Copy link
Contributor

1.7 is pretty old at this point. Seems fine to just bump the requirements up to something more recent.

@mcpherrinm mcpherrinm closed this Feb 25, 2019
@mcpherrinm mcpherrinm reopened this Feb 25, 2019
@mcpherrinm
Copy link
Contributor

sorry, I accidentally pressed "close and comment" instead of just "comment"

@nelz9999
Copy link
Contributor Author

Bumping to 1.7 seems like a bigger breaking change than I feel empowered to make. I'll probably just make my tests work in pre-1.7, and let the low-version policy decision be handled elsewhere.

@csstaub
Copy link
Collaborator

csstaub commented Feb 25, 2019

Go 1.7 came out in August 2016, seems reasonable to bump that requirement.

Here's a PR for it to rebase on: #221

@nelz9999
Copy link
Contributor Author

Rebased, and it seems to have passed!

@csstaub
Copy link
Collaborator

csstaub commented Feb 26, 2019

This looks good, thank you @nelz9999!

One concern I have that I'd like input on: some callers might want to enforce the presence of an expiration in the token. This was previously enforced (though not on purpose I suppose) but now isn't. It's probably sufficient to make this clear in the release notes, but maybe this is something we'd want to support in the API. Any thoughts or opinions on this?

@csstaub csstaub merged commit 628223f into square:v2 Feb 26, 2019
@nelz9999
Copy link
Contributor Author

some callers might want to enforce the presence of an expiration in the token

This is a little related to the scenario I'm programming against right now. My feeling is that much like non-standard claims, if the caller wants to verify things surpassing the requirements of the spec, they should do that on their own. And I believe the JSONWebToken.Claims(...) decoding into arbitrary structs is a fine affordance for that.

@nelz9999
Copy link
Contributor Author

And now I'm gonna be a little annoying... D'you have any idea when this might be released? 😇

@csstaub
Copy link
Collaborator

csstaub commented Feb 26, 2019

Soon, I just have to type up release notes and such

@csstaub
Copy link
Collaborator

csstaub commented Feb 27, 2019

Voilà: https://github.com/square/go-jose/releases/tag/v2.3.0

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

Successfully merging this pull request may close these issues.

4 participants