Work around bad RSAKey import #61
Conversation
jschlyter
changed the title
closes #62
|
closes #62 |
There was a problem hiding this comment.
The gist of this changeset is here:
https://github.com/IdentityPython/JWTConnect-Python-CryptoJWT/pull/61/files#diff-716790a3eab9c4e93fe7ea00a9db630fL175-R178
The calculation was wrong and resulted to an invalid key.
👍 to merge from me.
|
The fix looks good to me, but let's try to split formatting fixes from bug fixes (in different PRs preferably). |
Agree, this was due to error in |
|
I'm OK with the changes. |
|
Hello, I'm coming back to this to note a couple of things: @jschlyter I would like us to wait for all reviewers to give a thumbs up before merging. Ideally, one should not merge their own pull-requests, but somebody else that has reviewed the changes should do it. Secondly, it is nice to name and give credit to people that provided the fix. While the things we do are in the open and for the community and credits do not actually matter, it still is polite to keep it in mind and gives a boost for motivation for the effort that went into the analysis of the issue and the fix. At the same time this forces us to have proper commit messages that mention what was happening and how it got fixed. Thirdly, we are not done. This is a bug that is now breaking the library. This appeared after a dependency was upgraded. People using the current-latest version of this library will still get a breakage. We should make a new release so that users can upgrade and not get bitten by this. |
|
@c00kiemon5ter I was not aware of a review threshold > 1, hence my merge when you had approved the changes. If the policy is to require all reviewers, I'm all fine with that. Agree on the mention part, will keep that in mind. And yes, we should do a new release. |
|
Presently we have no review threshold >1. So far one is enough. |
cryptography