claims.go: Fixed some superfluous code. Added leeway. #86
Conversation
Make the RegisterSigningMethod, GetSigningMethod, and the new RemoveSigningMethod function all thread-safe.
Forgot to include this in the previous commit.
|
Lots of stuff in here. I'll need to spend a bit more time looking over it, but it looks fine. |
| @@ -28,19 +28,19 @@ func (c StandardClaims) Valid() error { | |||
| vErr := new(ValidationError) | |||
| now := TimeFunc().Unix() | |||
|
|
|||
| // The claims below are optional, by default, so if they are set to the | |||
| // By default he claims below are, optional, so if they are set to the | |||
There was a problem hiding this comment.
Yeah. Typo. I have fat-finger syndrome :)
|
I don't agree that what you've removed is 'superfluous'. While To strengthen my argument, there are a few places in this PR where the logic is unintentionally inverted, introducing vulnerabilities. The fact that the tests pass indicates we need to add some tests around this area before 3.0 is ready. The addition of leeway, if presented without all these other changes, looks fine. If you want to open a PR with those changes, I'll review again. Otherwise, I can come through and cherry-pick when I have time. |
See: #75 (comment)