Skip to content
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

Added Firebase sign-in implementation and error states #22

Closed
wants to merge 7 commits into from

Conversation

Sweekrit-B
Copy link
Contributor

Changes

Added sign-in implementation on Firebase and added the appropriate error states.

Testing

Tested changes through independent verification -> if email and password were incorrect, then error states were raised. If they were correct, console.log and redirected to the homepage.

Correct password, incorrect username:
image

Correct username, incorrect password
image

Email is invalid
image

After sign in
image

@Sweekrit-B Sweekrit-B linked an issue Feb 11, 2025 that may be closed by this pull request
Copy link
Collaborator

@eshaan-s18 eshaan-s18 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job on this task! Couple things I noticed that we discussed at our last dev meeting:

  1. If the email is not verified, they shouldn't be able to sign in. Maybe we can refactor the code to make it that once the email is verified it adds to the Firebase and Mongo? another option (probably easier) is that we could just add an error that says something like "Email not verified, please check your email". One issue I see with this that might happen is for example if someone creates an account and doesn't verify their email. When they try to sign in and see the error, they might not have received the email and there is no way for them to resend it, and our current implementation won't let them resend the email after they exit the resend page. I hope that makes sense. Let me
    know what you think and we can discuss this.
  2. The second thing we would want to implement is a way to determine if it is their first time signing in. If it is their first time signing in we want to take them to the intro video page, if it isn't they can go straight to the home page. This could be easy maybe you could just make a boolean field firstSignIn in Mongo. There might be a way with Firebase to do that too, but I think the field might be easiest.

Let me know if you have any questions about this, great work!

@Sweekrit-B
Copy link
Contributor Author

Changes made! Also merged with Kevin's work on debugging user authentication.

@OrigamiStarz
Copy link
Contributor

OrigamiStarz commented Feb 20, 2025

Hello, just checked out everything, and seems like the Firebase implementation and error states are working pretty well.

A few things I noticed:

  • When clicking enter after typing in a textbox, the form resets (not 100% sure if this is relevant to the PR though?)
  • Issue when validating email for account? (but it still seems to create an account, I was trying to go through the account creation process and it seems to pop up every time)
Screenshot 2025-02-19 at 11 38 21 PM - As of now, I don't see a redirect to the homepage after sign-in (this is intentional, right?), but I see the "Firebase sign in successful" and "First login updated" print statements - Interestingly, when the BE server is down, the front-end is still able to create the account, but shows this error, and also does not redirect to the "Email sent" component despite sending the email to the user. Screenshot 2025-02-19 at 11 58 13 PM

Could you see if the second bullet point is an issue on your end, and if it is, to fix it? Additionally, I'm wondering if we should block user creation entirely when the server is down.

Copy link
Contributor

@OrigamiStarz OrigamiStarz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevwu27
Copy link
Contributor

kevwu27 commented Feb 20, 2025

Fixes/Changes since Sweekrit's first comment:

  • Created an /auth route. This page will deal with email verification and password resetting (another PR to come). This is because for these operations, firebase only allows one unique link for them, so we created an /auth page as an intermediary page that will simply display 1) any errors risen throughout the process, 2) act as a "loading" screen in transition (for a split second though), and 3) direct users to the /emailverified or /resetpassword page (this page to come).
Screen.Recording.2025-02-20.at.1.01.48.AM.mov
  • More API routes created: now can query/get users by email, and can update user objects in MongoDB -- as of now, only purpose is to change the firstLogin field
  • Added a firstLogin field to user objects in MongoDB, keeps track of whether it's the user's first time logging in. If so, redirects user to intro page, else home page. Right now, it directs user to "/" for the former and "/signup" for the latter. To be replaced once those pages are created
  • Addressing @eshaan-s18 's comments: 1) when user tries to sign up but they have not verified their email, they are given opportunity to resend the verification email, 2) fixed as mentioned in previous bullet point
Screenshot 2025-02-20 at 12 57 35 AM
  • Addressing @OrigamiStarz 's comments: Not sure why pressing "enter" reloads the screen, we can probably discuss this as a whole during meeting. Fixed the error with verifying email, so that pop up should no longer appear. Also reorganized code so that if the backend is done or MongoDB does not successfully create a user, the firebase account that was created is removed, so it's as if the account was never made. The verification email is also not sent until after it is confirmed that MongoDB object creation is successful. -- changes in CreateAccountPanel.tsx

Let us know if you notice anything else that needs changing!

@kevwu27 kevwu27 closed this Feb 20, 2025
@kevwu27 kevwu27 removed a link to an issue Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants