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

Remove rate limit for root user #83

Open
KAIYOHUGO opened this issue Sep 3, 2024 · 6 comments
Open

Remove rate limit for root user #83

KAIYOHUGO opened this issue Sep 3, 2024 · 6 comments
Assignees
Labels
C-Backend Relate to backend K-Feature Feature request P-Middle Middle priority
Milestone

Comments

@KAIYOHUGO
Copy link
Contributor

Root user should not be rate limited (So we can use tools to do something such as create problem in batch)

@KAIYOHUGO KAIYOHUGO added P-Middle Middle priority C-Backend Relate to backend K-Feature Feature request labels Sep 3, 2024
@KAIYOHUGO KAIYOHUGO added this to the 0.1.0 milestone Sep 4, 2024
@Eason0729
Copy link
Contributor

Isn't your usecase for migration tools(from QMOJ to MDOJ)?

I prefers implementing alternative authentication method(service account?), because it's easier to maintain two rate-limiting method.

@Eason0729
Copy link
Contributor

Eason0729 commented Sep 4, 2024

Maybe we could use alternative http header for service account, and always rate limit service account with ip address, so that blacklist feature that current rate-limiting implementation provided can continue to work.

@KAIYOHUGO
Copy link
Contributor Author

Isn't your usecase for migration tools(from QMOJ to MDOJ)?

Yes

I prefers implementing alternative authentication method(service account?), because it's easier to maintain two rate-limiting method.

Maybe we could use alternative http header for service account, and always rate limit service account with ip address, so that blacklist feature that current rate-limiting implementation provided can continue to work.

You mean something like API key?

@Eason0729
Copy link
Contributor

Yes, and it's likely that we would put API key in header.

If you don't prefer that, I would just prefix normal token and API key.

@KAIYOHUGO
Copy link
Contributor Author

I think we can let Role::Root able to create api key and use it by add api-token metadata.

We can add Api service

message CreateApiRequest {
  // name of the api token
  required string name = 1;
  optional uint64 expiry = 2;
}

message ApiTokenInfo {
  required string token = 1;
  required google.protobuf.Timestamp expiry = 2;
}

service Api {
  rpc Create(CreateApiRequest) returns (ApiTokenInfo);

  // and some list/delete method, there is no refresh method
}

for all service will prefer use token instead of api-token if both exists. (for the sake of performance)

@Eason0729
Copy link
Contributor

I am thinking about removing rate limit entirely for ip address in a CIDR submask, so we could implement them faster.

And I also think that a correct solution because we can't make a new functionality without enlarging attack surface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Backend Relate to backend K-Feature Feature request P-Middle Middle priority
Projects
None yet
Development

No branches or pull requests

2 participants