Change storage key name from email to uuid #1903
Conversation
|
this does not break password reset ? Does it need to be UUID ? why cant it be a random string ? |
|
It does not break password reset, mate @degeri. We discussed this and figured uuid would be enough, since it is already a public data. We also need a way to associate the random string to the user, so that he can retrieve his key from the storage properly. This means that even if it were a random string, it would need to be associated with some user data on the storage, which would defeat the initial purpose of it. Bringing this problem up to the backend is not ideal imo, since we would'nt gain much from it being a random string. |
|
just reiterating matrix chat here. We have decided to replace email with username to store the keyvalue during the initial setup (before first login) . After that it will be replaced with UUID. |
There was a problem hiding this comment.
Hey @thi4go, this PR looks pretty good. I have only one concern regarding the "Permanent Logout" feature that was added by this diff. From what I can see, the local storage keeps storing the session keys (I guess it's due to the usage of the removeItem from localforage's API. Check my inline comment).
I would expect that both keys and values get removed and completely erased from Local Storage.
@degeri @tiagoalvesdulce @amassarwi what do you think?
|
@victorgcramos good catch mate. This was actually involving the way we used to clear the |
|
This removes all the users keys or only the logged in user ? |
|
Only the logged in user @degeri. Thought it would be best, since deleting all user data would involve others identities and drafts that might not be backed up yet. Not sure that would be fair. What do you think? |
|
@thi4go I agree with your thoughts, makes sense. |
|
Sounds fine I am good with it. |
|
@thi4go can you rebase? |
Bug (or issue) description
The storage key name was disclosing a personal user information, the email. This diff changes the key name to be his public uuid instead of email. This is valid both for the identity key and for local storage draft records.
This diff also adds a logout modal with the option to permanently logout from the app. This will delete his identity key and his draft entries.
Depends on decred/politeia#1186
Closes #1842 and #1843
Solution description
Straightforward change on the storage key name we use to set/get a user key. A workaround had to be done for a freshly registered user. Regarding this:
Problem: we need the user key to make the
/register (cms)and/user/new (pi)calls on the backend. Also for verifying his email. At this stage, the frontend does not have the user's uuid to set the appropriate storage key name.Solution: use the username as storage key name temporarily, until the user makes his first login on the app. When he does, we check if there's any local key saved under his username, and if so, we change it to be his uuid. This requires a minor change in the backend (PR decred/politeia#1186) so that a user's username is included in the verify email URL. This way we can retrieve his signing key before making the request.