-
Notifications
You must be signed in to change notification settings - Fork 795
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
3078 rate limit login #3216
3078 rate limit login #3216
Conversation
6c69668
to
6451284
Compare
6451284
to
bf64672
Compare
@@ -123,8 +129,6 @@ def get_agent_binary(self, operating_system: OperatingSystem) -> bytes: | |||
@handle_authentication_token_expiration | |||
def get_otp(self) -> str: | |||
response = self._http_client.get("/agent-otp") | |||
if response.status_code == HTTPStatus.TOO_MANY_REQUESTS: | |||
raise IslandAPIRequestLimitExceededError("Too many requests to get OTP.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't the IslandAPIAgentOTPProvider
still expect this error to be raised?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see, it's now handled by the HTTPClient decorator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It gets raised from self._http_client.get()
in the handle_island_errors()
decorator.
This test fails against our Windows Island due to network latency issues. This is being removed until we can resolve the issue.
What does this PR do?
Issue #3078
Adds rate limiting to authentication-related endpoints
PR Checklist
Testing Checklist
If applicable, add screenshots or log transcripts of the feature working