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

Password reset : account doesn't exisit. #181

Open
MP70 opened this issue Jul 27, 2020 · 5 comments
Open

Password reset : account doesn't exisit. #181

MP70 opened this issue Jul 27, 2020 · 5 comments

Comments

@MP70
Copy link

MP70 commented Jul 27, 2020

Currently if a password reset is requested and this email address isn't associated with an account we send a email saying ' request failed: you don't have a $project account'. This isn't perfect as you can send these unsolicited emails to any address from this page.

Should we change this so that it does not send any email, it just silently fails. We should then change the text on the webpage to say 'If you have an account linked to this email we have sent you a password reset'.

@benjie
Copy link
Member

benjie commented Jul 27, 2020

I don't think that's the right way to do it; but we should probably add the email to a temporary tracker (e.g. redis) to avoid emailing the person more than once every X amount of time, e.g. 24 hours, and maybe limit the number of emails sent for each IP address (client).

Silently failing is generally not a good idea, legitimate users who have many email addresses might spend a long time waiting for the email which will never come. So long as it's not too tempting a target for abuse (and the limitations should help with this), we should prioritise legitimate user experience.

@MP70
Copy link
Author

MP70 commented Aug 11, 2020

Thank you - I think that is a reasonable balance to strike between usability and security/spam/abuse control. I have adjusted to 24hrs between sending 'no account' emails to the same address on my own project and doing this with IP address rate limiting (etc) is on my to-do list. I will submit a PR when done.

@benjie
Copy link
Member

benjie commented Aug 11, 2020

Awesome; I look forward to it 👍

@MP70
Copy link
Author

MP70 commented Jun 15, 2021

Hi @benjie,

I very quickly implemented this at the time by persisting the jobs in the worker table for a set amount of time. There is then a test that checks for if <$x emails sent to this address in the last $y hours and <$z total tally. It's very light on resources and simplistic.

Elsewhere in my app, I use a token bucket algorithm (in Postgres as I didn't see the need to add another dependency). This is slightly slower, obviously.

Would you like a PR for this with either rate limit algorithm?

@benjie
Copy link
Member

benjie commented Jun 30, 2021

In general I see polling the jobs table for information as an anti-pattern, but I agree that adding more to postgres just to track this seems overkill - that's why I suggested redis. Maybe make it work with redis and if there's no redis then this protection simply won't be enabled? (We already advise using redis for sessions.)

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

2 participants