-
Notifications
You must be signed in to change notification settings - Fork 31
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
fix(neon_http_client): Make authorization throttling less agressive #2582
fix(neon_http_client): Make authorization throttling less agressive #2582
Conversation
5432a51
to
d403cee
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2582 +/- ##
=======================================
Coverage 28.85% 28.86%
=======================================
Files 370 370
Lines 136528 136549 +21
=======================================
+ Hits 39396 39411 +15
- Misses 97132 97138 +6
*This pull request uses carry forward flags. Click here to find out more.
|
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.
Now we also need tests validating that:
- non json responses are not intercepted
- json responses not matching the pattern are not intercepted
...ork/packages/neon_http_client/lib/src/interceptors/authorization_throttling_interceptor.dart
Outdated
Show resolved
Hide resolved
...ork/packages/neon_http_client/lib/src/interceptors/authorization_throttling_interceptor.dart
Outdated
Show resolved
Hide resolved
...ork/packages/neon_http_client/lib/src/interceptors/authorization_throttling_interceptor.dart
Outdated
Show resolved
Hide resolved
...k/packages/neon_http_client/test/interceptors/authorization_throttling_interceptor_test.dart
Outdated
Show resolved
Hide resolved
...k/packages/neon_http_client/test/interceptors/authorization_throttling_interceptor_test.dart
Outdated
Show resolved
Hide resolved
Signed-off-by: provokateurin <[email protected]>
d403cee
to
e2ca39a
Compare
The detection flagged some responses that were fine (even though I believe nothing should really use the 401 status code except the authentication middleware), which resulted in the app becoming unusable due to the blocking.
I limited it to OCS responses for now as most requests use that format and we can correctly detect invalid credentials 100% of the time through the custom status code 997.
For non-OCS APIs and WebDAV it might be possible as well, but I think they are not so important and we can add them if ever needed.
I know there are those really ugly nested if blocks, but the alternative would be to chain null-aware casts and I don't think that is much better than this.