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

Claim-based rate limiting #1587

Open
impratikpatel opened this issue Jul 18, 2022 · 6 comments
Open

Claim-based rate limiting #1587

impratikpatel opened this issue Jul 18, 2022 · 6 comments
Assignees
Labels
accepted Bug or feature would be accepted as a PR or is being worked on Authentication Ocelot feature: Authentication feature A new feature proposal Proposal for a new functionality in Ocelot Rate Limiting Ocelot feature: Rate Limiting

Comments

@impratikpatel
Copy link

impratikpatel commented Jul 18, 2022

New Feature

Extracting claims from the token and use as a Client ID in the rate limiting.
Suggesting a new feature to implement the rate limiting based on the claims in the token.

Motivation for New Feature

Current rate limiting is based on the Client ID passed in the header from the request. There are chances that anyone can manipulate the request by updating headers and using APIs without any restriction.

So, instead of depending on the consumer to provide the Client ID in the header, we can use the claims from the token. Which is more secure and not modifiable. Considering this, a rate limit will be applicable for authenticated endpoints only.

Specifications

  • Version: 18.1.0
  • Platform: .NET 6
@impratikpatel impratikpatel changed the title Extracting claims from the token and use as a Client ID in the rate limiting Claim base rate limiting: Extracting claims from the token and use as a Client ID in the rate limiting Jul 18, 2022
@raman-m
Copy link
Member

raman-m commented Dec 4, 2023

@ArchPratik commented on Jul 18, 2022

Hi Pratik!
Welcome to the Ocelot world! 🐅

After team discussion we've decided to accept the issue! We agree that current Header solution is ugly.

We must switch to Remote IP to detect client's identity for anonymous routes...

I have no idea when we will start development, I hope in January'24. We still have a couple of professional senior developers who write high quality code... So, any your contributions are really welcomed!

@raman-m raman-m added bug Identified as a potential bug proposal Proposal for a new functionality in Ocelot accepted Bug or feature would be accepted as a PR or is being worked on Jan'24 January 2024 release Rate Limiting Ocelot feature: Rate Limiting labels Dec 4, 2023
@evilz
Copy link

evilz commented Feb 14, 2024

Hello all,
Does someone work on this ?

What is the specification for the configuration if we want to use a claim value as clientId ?

Let me know if I should work on this ;p

@impratikpatel
Copy link
Author

Hi Raman,

Thank you for accepting this proposal. I Apologise, for the delay in reply.

I am ready to work on this improvement, but let's clear the scope of this feature first.

I was suggested to implement claim-based rate-limiting for authenticated routes. For anonymous/unauthenticated routs we can relay on Remote IP as you suggested. Let me know if any changes in these requirements.

@evilz
Copy link

evilz commented Feb 14, 2024

I think we should add a new property in RateLimitOptions like
ClientIdClaim or just Claim
If this property is set, it will try to get the value from Authorization header to override current clientId.

Seems good to you ?

@raman-m raman-m removed the Jan'24 January 2024 release label Feb 22, 2024
@raman-m
Copy link
Member

raman-m commented Feb 22, 2024

@ArchPratik You are assigned! Please start development!

I was suggested to implement claim-based rate-limiting for authenticated routes. For anonymous/unauthenticated routs we can relay on Remote IP as you suggested. Let me know if any changes in these requirements.

Correct! But why not do discuss more?..
What about options model?
Also, a callback for Rate Limiter will be a good design and extensibility approach...
And because the community worries about security, let's brainstorm security topics more. I can invite a rest of the team here...

@ThreeMammals ThreeMammals deleted a comment from ks1990cn Feb 22, 2024
@ThreeMammals ThreeMammals deleted a comment from ks1990cn Feb 22, 2024
@raman-m raman-m changed the title Claim base rate limiting: Extracting claims from the token and use as a Client ID in the rate limiting Claim-based rate limiting Feb 22, 2024
@raman-m raman-m added Authentication Ocelot feature: Authentication feature A new feature and removed bug Identified as a potential bug labels Feb 22, 2024
@raman-m
Copy link
Member

raman-m commented Jun 15, 2024

Dear @ArchPratik,
Do you work on the POC to show your design and ready to show us your code in a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Bug or feature would be accepted as a PR or is being worked on Authentication Ocelot feature: Authentication feature A new feature proposal Proposal for a new functionality in Ocelot Rate Limiting Ocelot feature: Rate Limiting
Projects
None yet
Development

No branches or pull requests

4 participants