search

hmellor / auction-website

An open-source auction hosting system
https://hmellor.github.io/auction-website/
MIT License
85 stars 41 forks source link

Fix sign out button #78

Closed overwatcheddude closed 3 weeks ago

overwatcheddude commented 1 month ago

Currently, the "Sign out" button doesn't actually log the user out. It merely sets text.

This PR allows the user to log out using Firebase auth.

hmellor commented 1 month ago

This was actually intentional, but the comment explaining it (if there ever was one) has been lost to refactoring.

The idea here is that we don't want the user to be able to change their uid, which happens every time an anonymous user signs out and signs in again. Users are actually anonymously signed in as soon as they open the page by AutoSignIn:

https://github.com/hmellor/auction-website/blob/efccd7ff7674958251fac9be2ad718612f2b1f2e/src/firebase/AutoSignIn.jsx#L6-L34

Then, when a user signs up we, actually just set their user account's displayName and their user documents name in handleSignUp:

https://github.com/hmellor/auction-website/blob/efccd7ff7674958251fac9be2ad718612f2b1f2e/src/components/Modal.jsx#L177-L187

If I accept this PR then it means that a new account is created every every time a user signs out (I believe AutoSignIn will create one as soon as their auth state changes when signing out), meaning that one person might end up with many accounts. This could be a potential attack vector if a malicious actor wanted to create thousands of users and run up your Firebase bill.

What do you think?

overwatcheddude commented 1 month ago

Hello Harry,

Thank you for the auction website, it is the only viable free and open source solution available. Also, your detailed explanation on the PR is appreciated.

I initially thought that this would be a simple fix, similar to the cancel button fix, but I guess it is a bit more than that.

Indeed, this PR will allow malicious actors to easily create multiple accounts and run up the Firebase bill, as you mentioned. However, breaking the "Sign out" button to prevent abuse may not be the best solution.

If you reject this PR, then: