search

Andrewnt219 / rent-near-me

https://rent-near-me.vercel.app/
1 stars 1 forks source link

Handle changing user password #63

Closed vitokhangnguyen closed 3 years ago

vitokhangnguyen commented 3 years ago

Attention: This is a draft PR

It will be ready when the Draft preffix is removed. Apparently, The built-in Draft PR feature is not available for free private repo on Github.

Works on #55

vercel[bot] commented 3 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/andrewnt219/rent-near-me/5m1ir2HM74B8dYzeGUMbHe8bXe6V
✅ Preview: https://rent-near-me-git-vitokhangnguyen-issue55-andrewnt219.vercel.app

Andrewnt219 commented 3 years ago

Made it public.

vitokhangnguyen commented 3 years ago

Made it public.

Agree

vitokhangnguyen commented 3 years ago

Hmm, so apparently even with public status, the repo hs to belong to an organization to have extra feature. Let's create an organization for this 😄

Andrewnt219 commented 3 years ago

Sure, we can use VAS organization as well. The only note is that the deployment version depends on the forked repo. Deploying organization repo needs a team tier.

vitokhangnguyen commented 3 years ago

Sure, we can use VAS organization as well. The only note is that the deployment version depends on the forked repo. Deploying organization repo needs a team tier.

No hard feelings but I don't really want this project to be tied to VAS 😂 Let me create a RentNearMe organization.

The deployment depinding onn the fork is fine... We can deploy on your fork until we afford Vercel team tier 😏

vitokhangnguyen commented 3 years ago

I created the RentNearMe organization and invited you. Whenever you have time, please trasnfer it in there

vitokhangnguyen commented 3 years ago

Back to the main issue...

We won't be able to have a route (i.e.: /user/changePassword) in the backend to handle all the password changing logic. The main reason is that Firebase Admin SDK does not support verification of the current password.

Here're the alternatives based on what I found:

  1. We perform password verification and changing of the user's password on the frontend (the Firebase Client SDK is designed to do this) and then we make a request to our backend to perform excess tasks such as updating passwordLastUpdatedTime. <== I personally prefer this since it requires less efffort.

  2. We use Firebase Client SDK on the server-side alongside with the Firebase Admin SDK which sounds crazy but is actually recommended by the Firebase team: https://github.com/firebase/firebase-admin-node/issues/1141. Again, the Client SDK has a method to do exactly just this. <== I personally want to avoid this because it seems like an abomination.

  3. Google API has an endpoint to verify a user's password but we would need to manually make request to this endpoint. (Edit 1: I start not to like this option as well since the endpoint seems to be used by Google internally and might change without notice)

Let me know what you think.

Edit 2: I went with the option 1 for now

vitokhangnguyen commented 3 years ago

This PR is ready for review.

Note I renamed the following files