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

rate limiter as access control #809

Open
johakoch opened this issue Mar 5, 2024 · 6 comments
Open

rate limiter as access control #809

johakoch opened this issue Mar 5, 2024 · 6 comments

Comments

@johakoch
Copy link
Collaborator

johakoch commented Mar 5, 2024

We could implement a rate limiter as an access control, e.g.:

  • respond with 429 if the request is blocked
  • fixed/sliding window (or more?)
  • config could be similar to rate limiting in backend
  • could be one "global" rate limiter; or one per key expression value, e.g. depending on request.<property> or a JWT claim (request.context.at.sub, if used after an "at" jwt access control)
server {
  api {
    access_control = ["global_rl", "at", "jwt_rl"]
    ...
  }
}

definitions {
  rate_limiter "global_rl" {
    period_window = "sliding" # or "fixed" or ?
    period = "1s"
    per_period = 1000
  }

  jwt "at" {
    ...
  }

  rate_limiter "jwt_rl" {
    period = "1s"
    per_period = 1
    key = request.context.at.sub
  }
}

It could be handy to add more request. sub-properties, e.g. from go Request.RemoteAddr if available.

@filex
Copy link

filex commented Mar 5, 2024

I like the idea. How would you define the order? We could inspect expressions (especially those using request.context), like we do to order request sequences. Or would you use the order of references in the access_control list?

@johakoch
Copy link
Collaborator Author

johakoch commented Mar 6, 2024

Or would you use the order of references in the access_control list?

I guess that's the order in which access controls are (already) applied.

@malud
Copy link
Collaborator

malud commented Mar 6, 2024

I would apply the ordered list - no magic reordering.

Should we reply with some additional headers like https://github.com/sashabaranov/go-openai/blob/master/ratelimit.go#L10 ? This could also be interesting for our BE limiters.

@johakoch
Copy link
Collaborator Author

johakoch commented Mar 6, 2024

@filex
Copy link

filex commented Mar 6, 2024

Let's start with a simple implementation that just uses status 429 to communicate current overuse.

@filex
Copy link

filex commented Mar 6, 2024

The key in the proposal is the "key" feature :).

Without it, rate limiting is almost useless. We could even make it a mandatory field.

Is it worth to provide a non-dynamic alternative to the key (which has to be evaluated in cty land) to be more efficient or make typical use-cases simpler? Example:

by_header = "API-Key"
by_client_ip = true

Anyways, we can start with the dynamic key and introduce those optimized shortcuts later once typical use is clear.

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