Skip to content

Commit

Permalink
[KYUUBI #6619][FOLLOWUP] Do not log auth audit log for thrift http co…
Browse files Browse the repository at this point in the history
…okie auth

# 🔍 Description
## Issue References 🔗

Follow up for #6619, I found that, there are many duplicate auth audit logs for thrift http.

We shall only log when make authentication with auth handler, and skip it for cookie auth.
## Describe Your Solution 🔧

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

## Types of changes 🔖

- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests

---

# Checklist 📝

- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

**Be nice. Be informative.**

Closes #6632 from turboFei/need_to_auth.

Closes #6619

3a25a3d [Wang, Fei] do auth

Authored-by: Wang, Fei <[email protected]>
Signed-off-by: Wang, Fei <[email protected]>
  • Loading branch information
turboFei committed Aug 29, 2024
1 parent 4c71415 commit c9b4512
Showing 1 changed file with 7 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class ThriftHttpServlet(
override def doPost(request: HttpServletRequest, response: HttpServletResponse): Unit = {
var clientUserName: String = null
var requireNewCookie: Boolean = false
var doAuth: Boolean = false
try {
if (conf.get(KyuubiConf.FRONTEND_THRIFT_HTTP_XSRF_FILTER_ENABLED)) {
val continueProcessing = doXsrfFilter(request, response)
Expand Down Expand Up @@ -111,6 +112,7 @@ class ThriftHttpServlet(
// If the cookie based authentication is not enabled or the request does not have a valid
// cookie, use authentication depending on the server setup.
if (clientUserName == null) {
doAuth = true
clientUserName = authenticate(request, response)
}

Expand Down Expand Up @@ -143,7 +145,7 @@ class ThriftHttpServlet(
error("Error: ", e)
throw e
} finally {
AuthenticationAuditLogger.audit(request, response)
if (doAuth) AuthenticationAuditLogger.audit(request, response)
AuthenticationFilter.HTTP_CLIENT_USER_NAME.remove()
AuthenticationFilter.HTTP_CLIENT_IP_ADDRESS.remove()
AuthenticationFilter.HTTP_PROXY_HEADER_CLIENT_IP_ADDRESS.remove()
Expand Down Expand Up @@ -275,8 +277,10 @@ class ThriftHttpServlet(

private def authenticate(request: HttpServletRequest, response: HttpServletResponse): String = {
val authorization = request.getHeader(AUTHORIZATION_HEADER)
authenticationFilter.getMatchedHandler(authorization).map(
_.authenticate(request, response)).orNull
authenticationFilter.getMatchedHandler(authorization).map { authHandler =>
AuthenticationFilter.HTTP_AUTH_TYPE.set(authHandler.authScheme.toString)
authHandler.authenticate(request, response)
}.orNull
}

private def getDoAsQueryParam(queryString: String): String = {
Expand Down

0 comments on commit c9b4512

Please sign in to comment.