Skip to content

Commit

Permalink
[KYUUBI #5289] RESTful API should always print audit log
Browse files Browse the repository at this point in the history
### _Why are the changes needed?_

Previously, the RESTful audit log did not contain HTTP requests that threw non-AuthenticationException during the process.

### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

- [x] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request

### _Was this patch authored or co-authored using generative AI tooling?_

No.

Closes #5289 from pan3793/auth-log.

Closes #5289

9e29279 [Cheng Pan] RESTful API should always print audit log

Authored-by: Cheng Pan <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
  • Loading branch information
pan3793 committed Sep 14, 2023
1 parent f6e3225 commit 137dcf1
Showing 1 changed file with 22 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import javax.security.sasl.AuthenticationException
import javax.servlet.{Filter, FilterChain, FilterConfig, ServletException, ServletRequest, ServletResponse}
import javax.servlet.http.{HttpServletRequest, HttpServletResponse}

import scala.collection.mutable.HashMap
import scala.collection.mutable

import org.apache.kyuubi.Logging
import org.apache.kyuubi.config.KyuubiConf
Expand All @@ -35,7 +35,8 @@ class AuthenticationFilter(conf: KyuubiConf) extends Filter with Logging {
import AuthenticationHandler._
import AuthSchemes._

private[authentication] val authSchemeHandlers = new HashMap[AuthScheme, AuthenticationHandler]()
private[authentication] val authSchemeHandlers =
new mutable.HashMap[AuthScheme, AuthenticationHandler]()

private[authentication] def addAuthHandler(authHandler: AuthenticationHandler): Unit = {
authHandler.init(conf)
Expand Down Expand Up @@ -109,32 +110,31 @@ class AuthenticationFilter(conf: KyuubiConf) extends Filter with Logging {
HTTP_PROXY_HEADER_CLIENT_IP_ADDRESS.set(
httpRequest.getHeader(conf.get(FRONTEND_PROXY_HTTP_CLIENT_IP_HEADER)))

if (matchedHandler == null) {
debug(s"No auth scheme matched for url: ${httpRequest.getRequestURL}")
httpResponse.setStatus(HttpServletResponse.SC_UNAUTHORIZED)
AuthenticationAuditLogger.audit(httpRequest, httpResponse)
httpResponse.sendError(
HttpServletResponse.SC_UNAUTHORIZED,
s"No auth scheme matched for $authorization")
} else {
HTTP_AUTH_TYPE.set(matchedHandler.authScheme.toString)
try {
try {
if (matchedHandler == null) {
debug(s"No auth scheme matched for url: ${httpRequest.getRequestURL}")
httpResponse.setStatus(HttpServletResponse.SC_UNAUTHORIZED)
httpResponse.sendError(
HttpServletResponse.SC_UNAUTHORIZED,
s"No auth scheme matched for $authorization")
} else {
HTTP_AUTH_TYPE.set(matchedHandler.authScheme.toString)
val authUser = matchedHandler.authenticate(httpRequest, httpResponse)
if (authUser != null) {
HTTP_CLIENT_USER_NAME.set(authUser)
doFilter(filterChain, httpRequest, httpResponse)
}
AuthenticationAuditLogger.audit(httpRequest, httpResponse)
} catch {
case e: AuthenticationException =>
httpResponse.setStatus(HttpServletResponse.SC_FORBIDDEN)
AuthenticationAuditLogger.audit(httpRequest, httpResponse)
HTTP_CLIENT_USER_NAME.remove()
HTTP_CLIENT_IP_ADDRESS.remove()
HTTP_PROXY_HEADER_CLIENT_IP_ADDRESS.remove()
HTTP_AUTH_TYPE.remove()
httpResponse.sendError(HttpServletResponse.SC_FORBIDDEN, e.getMessage)
}
} catch {
case e: AuthenticationException =>
httpResponse.setStatus(HttpServletResponse.SC_FORBIDDEN)
HTTP_CLIENT_USER_NAME.remove()
HTTP_CLIENT_IP_ADDRESS.remove()
HTTP_PROXY_HEADER_CLIENT_IP_ADDRESS.remove()
HTTP_AUTH_TYPE.remove()
httpResponse.sendError(HttpServletResponse.SC_FORBIDDEN, e.getMessage)
} finally {
AuthenticationAuditLogger.audit(httpRequest, httpResponse)
}
}

Expand Down

0 comments on commit 137dcf1

Please sign in to comment.