Excluding logged in users from rate-limiting

pglombardo / PasswordPusher

🔐 Securely share sensitive information with automatic expiration & deletion after a set number of views or duration. Track who, what and when with full audit logs.

https://docs.pwpush.com

Apache License 2.0

2.06k stars 356 forks source link

Excluding logged in users from rate-limiting #2686

Open olegbliaher opened 2 hours ago

olegbliaher commented 2 hours ago

Checklist

[x] I've searched the project's issues.
[x] I've searched the project's discussions.

❓ Question

Is it possible to exclude logged in users from rate-limits?

📎 Additional context

Running pwpush 1.47.1 with Caddy.

I also noticed I can configure safelist_ip-addresses via config/initializers/rack_attack.rb. Would be nice if it would be possible to specify those IP-addresses as an environment variable.

github-actions[bot] commented 2 hours ago

Hello @olegbliaher, thanks for contributing to the Password Pusher community! We will respond as soon as possible.

pglombardo commented 1 hour ago

Hi @olegbliaher - I can do that. But if you want it to apply to all logged in users, there might be a better solution rather than by IP:

throttle("req/minute/ip", limit: 120, period: 1.minute) do |req|
  req.ip if req.session[:warden] && req.session[:warden]["user.user.id"].nil? && !req.path.start_with?("/assets")
end

Needs testing though.

olegbliaher commented 1 hour ago

Hi @pglombardo, thank you for the very swift response! Personally, I'd prefer excluding all logged-in users, instead of dealing with IP-addresses and ranges.

Your suggestion looks perfect.

pglombardo commented 28 minutes ago

Sounds good - I have a couple other things in progress right but hopefully I can add this over the weekend. I'll update once the addition is out. Thanks for pointing this out!