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

Store a list of IP addresses that accessed an account, notify user if a new address logged in #4490

Open
monkeytypegeorge opened this issue Jul 31, 2023 · 17 comments

Comments

@monkeytypegeorge
Copy link
Collaborator

No description provided.

@Bruception
Copy link
Member

notify as in e-mail or in-game notification?

@Miodec
Copy link
Member

Miodec commented Aug 12, 2023

notify as in e-mail or in-game notification?

Email

@Shubham10102000
Copy link

Implementing this approach could bolster the app's security.

  1. During User Registration: Upon the registration of a new user, we can seamlessly include their IP address within their respective user document.
  2. During Sign-In Post Firebase Authentication: Subsequent to Firebase authentication during sign-in, we can establish a connection to an endpoint. This endpoint would assess whether the client's IP address already exists within the user's list. In case of a novel IP address, we can promptly alert the user and subsequently augment the user's IP list in their document by appending the new IP.

And I have question here,Are notifications applicable to users with unverified email addresses?

@Miodec
Copy link
Member

Miodec commented Aug 14, 2023

Not sure, maybe? But if thye havent verified that means the email might not even exist.

@Shubham10102000
Copy link

or may be the user have not completed the verification step.

@Shubham10102000
Copy link

@Miodec Do you find the proposed approach promising? If so, am I clear to proceed with its implementation?

@Miodec
Copy link
Member

Miodec commented Aug 14, 2023

Yeah, sure

@adamgerhant
Copy link

@Shubham10102000 have you made any progress with this? The logic should be pretty simple, im just not sure where it could be implemented.

@aikooo7
Copy link
Contributor

aikooo7 commented Sep 29, 2023

I'm going to step in just for give my opinion, this should be opt-in aka turned off by yourself and user may let turn it on if he wants to do so, you know just to make more private folks happy.

@adamgerhant
Copy link

IP address are already accessible through log files. In general storing them is a gray area due to how little personal information they contain. An opt-out would be good but I dont see storing them by default to be a privacy concern. I would love to hear anybody else's opinion though since I am storing IP's on my own web app without informing the visitor, and I'm not sure if that is allowed.

@Miodec
Copy link
Member

Miodec commented Oct 2, 2023

I would love to hear anybody else's opinion though since I am storing IP's on my own web app without informing the visitor, and I'm not sure if that is allowed.

As long as you specify in your Privacy policy that you are storing user IP's, and the reason behind it - legally youre fine.

As for the privacy behind storing IPs, in my opinion, having the increased security of being able to notify the user that their account is being accessed from somewhere else is a good trade off.

@Shubham10102000
Copy link

Shubham10102000 commented Oct 3, 2023

@Miodec What is the server-side deployment strategy here ? Specifically, if Nginx is being used, will it require configuration to obtain the public IP address in the express app for tasks such as saving ip and other related actions?

@Miodec
Copy link
Member

Miodec commented Oct 3, 2023

IPs can be grabbed from the headers, for example:

return (req.headers["cf-connecting-ip"] ||

@Shubham10102000 Shubham10102000 removed their assignment Oct 21, 2023
@farkon00
Copy link
Contributor

I'm going to step in just for give my opinion, this should be opt-in aka turned off by yourself and user may let turn it on if he wants to do so, you know just to make more private folks happy.

How about this opt-out just removing the IP checking part? So every time somebody logins into the account we send them an email with an IP, but don't store it for the user, who opted-out. We can probably implement that by just not storing the IPs of those who opted-out and the rest of the logic is the same.

Miodec added a commit that referenced this issue Nov 30, 2023
@kavania2002
Copy link

Hey @Miodec, I can see that you have stored the ip address but what about the notification?
Are you working on it or else I would like to help with the same!

@Miodec
Copy link
Member

Miodec commented Dec 1, 2023

Ill be adding the email soon. Just testing the code in a production environment.

@FFUV
Copy link

FFUV commented Feb 24, 2024

@monkeytypegeorge this is outrageous with so many reasons

let me give you a SMALL list why

Privacy: Users might not want their IP addresses stored without permission.
Misuse: Storing IPs could lead to unauthorized access or tracking.
Laws: There could be legal issues with storing user data.
Accuracy: IP changes could trigger false alarms.
Annoyance: Constant notifications might irritate users.
Complexity: Adding this feature makes the app more complicated.
Resources: Storing IPs could strain server resources.
Alternatives: There are better ways to enhance security, like 2FA.
Trust: Users may question why IPs are being collected.
Focus: Adding unrelated features can confuse users.

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

No branches or pull requests

9 participants