g3rg0 commented on code in PR #4774:
URL: https://github.com/apache/hive/pull/4774#discussion_r1345862690
##########
service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java:
##########
@@ -283,7 +283,7 @@ protected void doPost(HttpServletRequest request,
HttpServletResponse response)
}
// Send a 401 to the client
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
- if(isAuthTypeEnabled(request, HiveAuthConstants.AuthTypes.KERBEROS)) {
+ if(authType.isEnabled(HiveAuthConstants.AuthTypes.KERBEROS)) {
Review Comment:
@dengzhhu653 Thank you for taking care of this!
Yeah, that's a good question. I think that if Kerberos auth is enabled, we
definitely need to add the `Negotiate` challenge to the `WWW-Authenticate`
header (see https://datatracker.ietf.org/doc/html/rfc7235#section-4.1 ). If
Ldap auth is also enabled, then we should add the `Basic` challenge, too, so if
HS2 is configured with `Kerberos,Ldap`, and authentication fails, we should
respond with `WWW-Authenticate: Negotiate, Basic` and it is up to the client to
choose Kerberos or Ldap auth on the next attempt. However, I didn't want to
include support for the `Basic` challenge in this PR because I don't see the
consequences, so I don't know what component, what interaction would be broken
because of it. (e.g. The Knox code referenced above, cannot handle the case
where multiple challenges are listed in the header.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
