-
Notifications
You must be signed in to change notification settings - Fork 208
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
Refactor RSA verification: Replace jsrsasign with crypto-js #268
Conversation
7945fda
to
83bb6f2
Compare
src/jwt/__tests__/base64.spec.js
Outdated
expect(base64.padding(base64.padding('abc'))).toBe('abc='); | ||
}); | ||
|
||
it('decode to hex', function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest you reword this to use present simple describing what it should do, instead of simply repeating the function name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see other choices here. The method's responsibility is pretty dumb, TBH. Only decoding a base64 into a HEX value. What do you have in mind?
83bb6f2
to
89e847b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Changes
jsrsasign
library was not compatible with the Hermes JS engine, currently optional but still officially supported by the React Native platform for Android. This PR changes the RSA signature check implementation to use crypto-js instead.No breaking changes are introduced on this build.
References
Resolves #251
Testing
The unit tests were updated. To evaluate Hermes support, it has been tested this way:
app/build.gradle
fileChecklist