Ensure that password isn't stored in AsyncStorage by default

[jamonholmgren]

In the AuthenticationStore, the password prop is a normal text type:

ignite/boilerplate/app/models/AuthenticationStore.ts

Line 8 in 01c2a11

authPassword: types.optional(types.string, ""),

Which means it's persisted into AsyncStorage as plain text:

ignite/boilerplate/app/models/helpers/setupRootStore.ts

Line 43 in 7832f96

_disposer = onSnapshot(rootStore, (snapshot) => storage.save(ROOT_STATE_STORAGE_KEY, snapshot))

We should make sure it's not. I don't know for sure that it's a major problem, but it doesn't feel right.

[jamonholmgren]

While we're in there, any types.optional(types.string, "") could be shortened to just an empty string, which is the same thing.

- authPassword: types.optional(types.string, ""), 
+ authPassword: "", 

[joshuayoes]

I know that on the web, there isn't really a way to store passwords or secure tokens client side. The only way to do so is through a cookie, but that is because client side javascript cannot access a cookie.

The alternative is storing a refresh token, which can be used to make a request to the server to get an access token and make auth requests.

Is AsyncStorage accessible by other apps?

[joshuayoes]

Here are some relevant docs about storing sensitive info from the official React Native docs: https://reactnative.dev/docs/security#storing-sensitive-info

[kateinkim]

Just opened a PR to address this. Do we want to add a way to store sensitive info like @joshuayoes mentioned? We used https://docs.expo.dev/versions/latest/sdk/securestore/ in our client project. I can open a separate ticket to file this as an issue if so. @jamonholmgren

[lodev09]

@kateinkim I checked the PR for this but it doesn't really solve the issue. Correct me if I'm wrong but I think preProcessSnapshot is called before a snapshot is loaded into an instance. This means that the auth credentials were already in the storage to begin with. The PR will only do harm than good for the demo since it will present the login screen every time you close & open the app.

I believe the solution is to clear these "before saving" into storage. Or maybe refactor the AuthenticationStore model and move it out of the RootStore then use different storing strategy for it?

[kateinkim]

@lodev09 I am very sorry for the late reply. IR took a week off over the holidays and just realized that I should have replied back to you sooner!

As I mentioned in the PR, I think we do actually want postProcessSnapshot instead of preProcessSnapshot. Maybe @jamonholmgren can clarify that?

I also think you're right about always presenting the login screen with this update which is why I was suggesting something like https://docs.expo.dev/versions/latest/sdk/securestore/ so we can store the login credentials in the keychain instead of keeping it in the state tree at all.

[lodev09]

No worries @kateinkim . Happy new year!

I think the easiest and safest solution is to just omit authPassword from the model and put it in a state on the login screen 😄. Just pass the password to login api and then it's gone after authentication

[StefanWallin]

Just stumbled on this issue now with v8.5.1, the demo app didn't work the way I expected it to. I think the proper solution would be to store login with https://www.npmjs.com/package/react-native-keychain#setinternetcredentialsserver-username-password--accesscontrol-accessible-accessgroup-securitylevel- and to store the authToken with https://www.npmjs.com/package/react-native-keychain#setgenericpasswordusername-password--accesscontrol-accessible-accessgroup-service-securitylevel-

[kateinkim]

Hello all!
First of all, @lodev09 thank you so much for opening a PR for this. We took a look at your PR and it will be approved soon. So sorry it took a while for us to get back to you 🥲

I talked to IR folks internally and we agreed that adding any sort of credential storing is beyond Ignite demo scope. If anybody needs a way to store this, they can add any necessary packages in their own projects and that might be the way going forward so that Ignite doesn't impose a particular solution here.

Thanks for your contributions and suggestions 😄

[infinitered-circleci]

🎉 This issue has been resolved in version 8.5.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀