Add authentication verifier implementation #13
Conversation
Signed-off-by: Peter Nied <[email protected]>
| } | ||
|
|
||
| return Optional.empty(); | ||
| } |
There was a problem hiding this comment.
Can you briefly explain the roles of each of these two methods?
| @@ -0,0 +1,74 @@ | |||
| package org.opensearch.security.http; | |||
There was a problem hiding this comment.
this seems like a duplicate of above with the class name change
| log.info("Checking if request is authenticated:\n" + request); | ||
|
|
||
| final NettyRequestChannel requestChannel = (NettyRequestChannel) SecurityRequestFactory.from(request); | ||
| restFilter.checkAndAuthenticateRequest(requestChannel); |
There was a problem hiding this comment.
By adding a call to checkAndAuthenticateRequest here and leaving the existing call to checkAndAuthenticate request in SecurityRestFilter this will authenticate valid requests twice.
Do you think its possible to remove the need to authenticate the request twice? For external IdPs, i.e. LDAP, will that mean 2 network roundtrips per request to the LDAP auth server?
Since a side effect of checkAndAuthenticateRequest is populating the threadContext, is it possible to carry the threadcontext into the SecurityRestFilter?
peternied
force-pushed
the
headerverifier
branch
from
6b02da8 to
33aa863
Compare
|
Thanks for the reviews - closing this out as we ended up doing this as part of Craig's fix in security |
Description
This is the delta from opensearch-project#3430 to add the AuthenticationVerifer in the Security Plugin
Testing
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.