Use Keychain/Keystore to store refresh token #13
Conversation
|
Thanks for the pull request. I’ll take look tomorrow. |
| const tokens = await getAuthTokens() | ||
| if (!tokens) { | ||
| const refreshToken = await getRefreshToken() | ||
| if (!refreshToken || !accessToken) { |
There was a problem hiding this comment.
should it be !refreshToken || !token?
There was a problem hiding this comment.
Both tokens need to be checked, like before.
|
@jdanthinne Thanks again for the pull request. Would you mind writing tests for changes you made? Cheers. |
|
I'm still not convinced we need this change. The reason tokens should not be stored in the localstorage in a web environment, is because it's vulnerable to XSS attacks. In react native environments, the attack surface for XSS atacks is narrowed down a lot. |
For sure, but anyone who can have access to your phone can have access to your tokens, because they are stored unencrypted in the app files. |
|
I suppose it won’t hurt! I’ll take another look once you’ve added unit tests. If you need help with writing these tests, let me know. |
|
Being more a native (iOS) dev, I must admit I've never written any unit test for React Native. I just have to find some spare time to dive into that. I already began but I was already facing some config issue due to the fact that the previous test didn't have anything to do with the native side of things, so jest must be configured differently (with babel transform, etc.). |
|
You should mock the dependencies you added. No need to make Native changes. Unit test should simply check if dependencies are being called with the right parameters. Jest config should be alright. |
|
That's not really testing the core functionalities then, or am I wrong? |
|
I think what needs to be tested is the js part: when and under what key tokens should be stored and retrieved. I trust that the dependencies that we are now using are functioning properly. |
|
ping @jdanthinne |
|
@mvanroon Sorry, I've been away, and I have to admit I wasn't able to configure jest to run the current tests as my changes now uses react-native and the previous code was not. Being not used to jest, could you help me with the initial setup. I'd be happy to update the tests then. |
This changes the way tokens are stored.
To follow OWASP guidance, refresh token is stored securely in the keychain, and the access token is only stored in memory.
Issue raised there as well: jetbridge/axios-jwt#24