Add Support to be able to verify from multiple keys #425

ritikesh · 2021-06-17T15:29:25Z

Problem -

During a security incident, if one has to rotate secrets shared with 3rd party apps, one has to ensure backward compatibility where both the old and new secrets continue to work.

Solution -
Have made changes to iterate over an array of keys while verifying signature. This can be used by returning an array of keys from the find_key block.

PS: Will add specs if changes are ok.

sourcelevel-bot · 2021-06-17T15:29:27Z

Hello, @ritikesh! This is your first Pull Request that will be reviewed by SourceLevel, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

lib/jwt/decode.rb

anakinj · 2021-06-17T19:29:26Z

Thanks for the proposal, first impression is that I think this is a pretty good idea. Have to digest it a little still if there are potential problems with this kind of behaviour. A little finetuning to get the specs passing and maybe adding a few new tests.

lib/jwt/signature.rb

ritikesh · 2021-06-17T20:36:25Z

Thanks for the proposal, first impression is that I think this is a pretty good idea. Have to digest it a little still if there are potential problems with this kind of behaviour. A little finetuning to get the specs passing and maybe adding a few new tests.

Thanks! I'll add some new tests tomorrow and fix the existing specs as well.

spec/jwk/ec_spec.rb

spec/jwt_spec.rb

sourcelevel-bot · 2021-06-22T08:10:43Z

SourceLevel has finished reviewing this Pull Request and has found:

2 possible new issues (including those that may have been commented here).
4 fixed issues! 🎉

See more details about this review.

lib/jwt/decode.rb

spec/jwt_spec.rb

anakinj · 2021-06-28T19:25:40Z

@excpt You have anything to add? I think this is a pretty neat addition. Would be ready to merge.

ritikesh · 2021-07-06T11:51:19Z

Hi @anakinj / @excpt - following up on this. Is there anything pending for this to be merged?

anakinj · 2021-07-09T07:04:59Z

From my part it looks good. Was thinking a second opinion on the feature would be valuable.

ritikesh · 2021-08-09T19:05:35Z

Hi @anakinj / @excpt - circling back on this.

ritikesh · 2021-11-03T08:32:32Z

Hi @excpt - have you had a chance to look at this? Would appreciate your thoughts on how to proceed.

anakinj · 2021-12-08T19:59:54Z

Im thinking we could merge this. Could you still be so kind and rebase to the latest master where the ruby-head build should have been fixed.

ritikesh · 2021-12-13T10:16:01Z

Hey, I've rebased my branch to the current master. Thanks.

Im thinking we could merge this. Could you still be so kind and rebase to the latest master where the ruby-head build should have been fixed.