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

feat: Implement RateLimiter into enroll_verb_handler and add unit test #1547

Merged
merged 4 commits into from
Sep 13, 2023

Conversation

sitaram-kalluri
Copy link
Member

- What I did

  • Implement Rate limiting for the enrollment requests. Limit the enrollment request per connection to the configured value in the given time frame. An exception will be thrown for the enrollment requests received beyond the limit.

- How I did it
Changes in at_server_spec

  • Create an abstract class called AtRateLimiter that includes a method called isRequestAllowed. This method should return a boolean value based on the rate limit rules.
  • Modify the InboundConnection class to implement the AtRateLimiter abstract class.

Changes in at_secondary_server

  • In the config.yaml file and at_secondary_config, introduce two new parameters: maxRequestsPerTimeFrame and timeFrameInHours to define rate limit criteria.
  • Implement the isRequestAllowed method in the InboundConnectionImpl class (which implements InboundConnection) to enforce rate limits on enrollment requests based on the criteria specified in the config.
  • In the enroll_verb_handler's _handleEnrollmentRequest method, call the isRequestAllowed method on the AtConnection object. If the method returns true, allow the enrollment request; otherwise, throw an AtThrottleLimitExceeded exception.
  • Enhance the GlobalExceptionHandler to handle the AtThrottleException. This exception should be used to send a response to the client while preventing the connection from being closed.
  • For writing function tests, when testingMode parameter in the config.yaml is set to true, allow modifications to the enrollment criteria using the config verb.
    • In the at_secondary_config, add maxRequestsPerTimeFrame and timeFrameInMills parameters to the ModifiableConfigs enum.
    • When the above values are modified through the config verb, implement listeners in the at_secondary_server_impl to capture these changes and update the values in the at_secondary_config.

- How to verify it

  • Implemented unit tests in inbound_connection_impl_test.dart to validate the rate limit functionality on the inbound connection.
  • Implemented functional tests to ensure that the rate limit is enforced on the enroll verb.

- Description for the changelog

  • Implement Rate Limiting for APKAM requests

Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you for the clear PR description @sitaram-kalluri

Once you have published at_server_spec and removed the dependency override from this PR please re-request review and I will approve and merge.

@gkc gkc merged commit a197000 into trunk Sep 13, 2023
@gkc gkc deleted the apkam_rate_limiter branch September 13, 2023 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

APKAM - Rate limit enroll requests Refactor the commit log keystore to improve time and space complexity
2 participants