add a basic rate limiter to magic attach #14840
Conversation
Demo
Jenkins 
demos.haus 
webapp/decorators.py
Outdated
| import flask | ||
| from webapp.login import user_info | ||
|
|
||
| rate_limits = defaultdict(deque) |
There was a problem hiding this comment.
Having a global dictionary for rate limiting is simple but it has its pitfalls which you should be aware of:
- This is per process memory and each instance of app is going to have independent memory
- It isn't thread safe
- There is no way to reclaim the memory being used by this dictionary, it will keep growing indefinitely unless the process is stopped
- Also, these limits will reset as soon as the process stops. Users will be able to make requests again.
I would suggest that you go with a different approach that caters to the above problems that will arise
Redis comes to my mind as a solution.
There was a problem hiding this comment.
- That is not going to be a problem because the limits are not a hard limit
- The order of the array does not matter in this case. Thread safety should not affect the functioning of the limiter.
- It removes older timestamps when there is a repeated request.
- Yes that is what is expected as this is going to be in memory
Redis deployment will take time as it will need to requested from IS and this is something temporary that does the job as we don't have a database or anything to add stateful features to our application.
That being said this is not a final implementation as I am implementing a leaky bucket. Hence this PR is in draft and not yet ready for review. That would add a process to remove older requests from the queue at a steady rate.
There was a problem hiding this comment.
Could you please let me know how many instances of the app are running at a given time?
Is this system running behind K8s?
There was a problem hiding this comment.
Yes the website is on k8s, there are multiple pods in prod and staging and only one in demos running that specific app instance.
Also it is a moot point now because I am storing it in session which means it is stored in the user's end as a signed cookie. So this means the only way to tamper with it is to invalidate the session which means they need to login again. Since the goal was to stop misuse, this should work
Co-authored-by: Muhammad Ali <its.muhammad.ali.mughal@gmail.com>
…html Co-authored-by: Muhammad Ali <its.muhammad.ali.mughal@gmail.com>
…html Co-authored-by: Muhammad Ali <its.muhammad.ali.mughal@gmail.com>
…html Co-authored-by: Muhammad Ali <its.muhammad.ali.mughal@gmail.com>
…html Co-authored-by: Muhammad Ali <its.muhammad.ali.mughal@gmail.com>
…html Co-authored-by: Muhammad Ali <its.muhammad.ali.mughal@gmail.com>
* fix(performance): Add API token for the perf-checks CI * Replace api_key with key
* add form on cancel subscription
* Exclude static case studies from ubuntu.com/engage * Bumped discourse module version * format python
suggesting the addition of esm-infra-legacy pockets to cves and usns webpages so we can have the hyperlink to Ubuntu Pro and the note about this pocket. It is not a new pocket, it is already known in the ubuntu-com-security-api and it is being pushed to our websites already, we are just missing the information as we have for esm-infra Signed-off-by: Rodrigo Figueiredo Zaiden <rodrigo.zaiden@canonical.com>
reformating with 'dotrun format-python' to fix lint python report. Signed-off-by: Rodrigo Figueiredo Zaiden <rodrigo.zaiden@canonical.com>
…s/build-an-image' (#14846)
…ic-attach-endpoint
abhigyanghosh30
force-pushed
the
WD-20031-add-auth-based-rate-limiting-to-magic-attach-endpoint
branch
from
ab4c2fc to
86308bd
Compare
Done
QA
./run serveordotrunIssue / Card
Fixes #
Screenshots
Help
QA steps - Commit guidelines