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

Consider adding support for per client rate limiting #1591

Open
AndersAbel opened this issue Sep 16, 2024 · 5 comments
Open

Consider adding support for per client rate limiting #1591

AndersAbel opened this issue Sep 16, 2024 · 5 comments
Milestone

Comments

@AndersAbel
Copy link
Member

We currently have callbacks on the authorize and token endpoint validators that can be used to implement custom validation logic. But for a rate limiting scenario these runs too late - when they are called we've already done a lot of expensive storage calls.

Consider adding a new rate limiting feature that is shared across all endpoints. The rate limiting should include a context that includes source IP address (or maybe the full request object?) and extracted parameters - at least clientId and possibly a principal or the sub. Each endpoint would call the rate limiting as soon as the client id has been read from the request.

@josephdecock
Copy link
Member

Custom validation gets a sophisticated object model and the values in that model can be trusted. The rate limiting would have to run earlier and so wouldn't have that.

Instead, we would just give the raw parameters as inputs. But what would be gained by that? Would the abstraction be nicer than doing this with asp.net core middleware? I guess I see the benefit of not needing the custom middleware to look at routes.

@brockallen
Copy link
Member

Perhaps a general purpose pre-endpoint hook? IOW, we add something that gets called and knows which endpoint before the actual EP is invoked. This would allow IdentityServer endpoints to be pre-processed without the customer having to write general middleware that checks specific paths with the knowledge of IS EP paths?

@brockallen
Copy link
Member

In the past when this type of thing has been requested, we suggested using a decorator pattern on our ITokenRequestValidator to pre-process the requests. Perhaps this is the better angle on this issue -- IOW, make pre-processing these couple of validators easier?

@brockallen brockallen added this to the Future milestone Sep 17, 2024
@AndersAbel
Copy link
Member Author

@josephdecock The benefit of handing over the raw (unvalidated) parameters is that they could reuse our existing code to extract the parameters. Especially for a form body it is a performance penalty to read and parse the body in a custom middleware, just to let our middleware do it right after.

@brockallen Yes, some kind of pre-endpoint hook - but with access to raw parameters. And also the client IP address.

@brockallen
Copy link
Member

And also the client IP address.

Inject IHttpRequestAccessor?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants