Set gRPC security interceptor to highest priority#5940
Set gRPC security interceptor to highest priority#5940aparajita31pandey wants to merge 4 commits intoopensearch-project:mainfrom
Conversation
Signed-off-by: Aparajita Pandey <aparajita31pandey@gmail.com>
|
@aparajita31pandey Can we add a CHANGELOG entry for this? |
Signed-off-by: Aparajita Pandey <aparajita31pandey@gmail.com>
|
Thankyou @cwperks for quick review. Added CHANGELOG entry for this change |
|
it seems that we have integration test failures |
|
It looks like this change is colliding with the TestUserInjectionInterceptorProvider injected into the gRPC transport in the integration test suite. This class mocks a gRPC interceptor which might perform user injection by reading the "test-user-injection" header and stashing it in the thread context under Do we necessarily want the AuthNGrpcInterceptor to always execute first in the chain? I'm wondering if we want to reserve space for these user injection use cases, or maybe something like a custom compression/decompression handler which may want to execute before we handle auth. |
|
I cannot really comment on most of the points, as I was not really involved in the architecture. Just a few cents:
For HTTP, it is important to be able to do authn before decompression is applied to avoid unauthenticated DDOS attacks. I do not know so much about gRPC, but that might be a similar consideration there, right? |
I think my compression example was not great but maybe some other use cases which would need to execute before security plugin could be:
By making the gRPC filter max prio we remove the above use cases. Is the idea to protect future plugin developers from accidentally registering an insecure intercepter or doing unnecessary work ahead of the grpc security filter? |
Description
Prioritize security gRPC interceptor with minimum order value
SecurityGrpcFilterIssues Resolved
[#5812]
Do these changes introduce new permission(s) to be displayed in the static dropdown on the front-end? If so, please open a draft PR in the security dashboards plugin and link the draft PR here
Testing
[Please provide details of testing done: unit testing, integration testing and manual testing]
Unit Test Added
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.