search

graphile / starter

Opinionated SaaS quick-start with pre-built user account and organization system for full-stack application development in React, Node.js, GraphQL and PostgreSQL. Powered by PostGraphile, TypeScript, Apollo Client, Graphile Worker, Graphile Migrate, GraphQL Code Generator, Ant Design and Next.js
https://graphile-starter.herokuapp.com
Other
1.75k stars 220 forks source link

Password reset : account doesn't exisit. #181

Open MP70 opened 4 years ago

MP70 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Awesome; I look forward to it :+1:

MP70 commented 3 years ago

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 commented 3 years ago

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.)