Skip to content

Commit ccd4de8

Browse files
turboFeibowenliang123
authored andcommitted
[KYUUBI #6750] [REST] Using ForbiddenException instead of NotAllowedException
# 🔍 Description ## Issue References 🔗 Seems NotAllowedException is used for method not allowed, and currently, we use false constructor, the error message we expected would not be return to client end. It only told: ``` {"message":"HTTP 405 Method Not Allowed"} ``` Because the message we used to build the NotAllowedException was treated as `allowed` method, not as `message`. ![image](https://github.com/user-attachments/assets/3199f20c-6148-4e6a-9183-7a0843913d8d) ## Describe Your Solution 🔧 We should use the ForbidenException instead, and then the error message we excepted can be visible in client end. https://github.com/apache/kyuubi/blob/85dd5a52efa790195ed46b713c53938ffd31c5d9/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/api.scala#L47-L51 ## 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 <img width="913" alt="image" src="https://github.com/user-attachments/assets/6c4e836d-a47a-485d-85a3-fd3a35a9e425"> --- # Checklist 📝 - [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) **Be nice. Be informative.** Closes #6750 from turboFei/not_allowed_exception. Closes #6750 4dd6fc1 [Wang, Fei] Using ForbiddenException instead of NotAllowedException Authored-by: Wang, Fei <[email protected]> Signed-off-by: Bowen Liang <[email protected]> (cherry picked from commit cefb98d) Signed-off-by: Bowen Liang <[email protected]>
1 parent 5a416e9 commit ccd4de8

File tree

4 files changed

+25
-25
lines changed

4 files changed

+25
-25
lines changed

kyuubi-server/src/main/scala/org/apache/kyuubi/server/KyuubiRestFrontendService.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ class KyuubiRestFrontendService(override val serverable: Serverable)
248248
} catch {
249249
case t: Throwable => throw new WebApplicationException(
250250
t.getMessage,
251-
Status.METHOD_NOT_ALLOWED)
251+
Status.FORBIDDEN)
252252
}
253253
}
254254

kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
5757
val ipAddress = fe.getIpAddress
5858
info(s"Receive refresh Kyuubi server hadoop conf request from $userName/$ipAddress")
5959
if (!fe.isAdministrator(userName)) {
60-
throw new NotAllowedException(
60+
throw new ForbiddenException(
6161
s"$userName is not allowed to refresh the Kyuubi server hadoop conf")
6262
}
6363
info(s"Reloading the Kyuubi server hadoop conf")
@@ -76,7 +76,7 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
7676
val ipAddress = fe.getIpAddress
7777
info(s"Receive refresh user defaults conf request from $userName/$ipAddress")
7878
if (!fe.isAdministrator(userName)) {
79-
throw new NotAllowedException(
79+
throw new ForbiddenException(
8080
s"$userName is not allowed to refresh the user defaults conf")
8181
}
8282
info(s"Reloading user defaults conf")
@@ -95,7 +95,7 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
9595
val ipAddress = fe.getIpAddress
9696
info(s"Receive refresh kubernetes conf request from $userName/$ipAddress")
9797
if (!fe.isAdministrator(userName)) {
98-
throw new NotAllowedException(
98+
throw new ForbiddenException(
9999
s"$userName is not allowed to refresh the kubernetes conf")
100100
}
101101
info(s"Reloading kubernetes conf")
@@ -114,7 +114,7 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
114114
val ipAddress = fe.getIpAddress
115115
info(s"Receive refresh unlimited users request from $userName/$ipAddress")
116116
if (!fe.isAdministrator(userName)) {
117-
throw new NotAllowedException(
117+
throw new ForbiddenException(
118118
s"$userName is not allowed to refresh the unlimited users")
119119
}
120120
info(s"Reloading unlimited users")
@@ -133,7 +133,7 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
133133
val ipAddress = fe.getIpAddress
134134
info(s"Receive refresh deny users request from $userName/$ipAddress")
135135
if (!fe.isAdministrator(userName)) {
136-
throw new NotAllowedException(
136+
throw new ForbiddenException(
137137
s"$userName is not allowed to refresh the deny users")
138138
}
139139
info(s"Reloading deny users")
@@ -152,7 +152,7 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
152152
val ipAddress = fe.getIpAddress
153153
info(s"Receive refresh deny ips request from $userName/$ipAddress")
154154
if (!fe.isAdministrator(userName)) {
155-
throw new NotAllowedException(
155+
throw new ForbiddenException(
156156
s"$userName is not allowed to refresh the deny ips")
157157
}
158158
info(s"Reloading deny ips")
@@ -175,7 +175,7 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
175175
val ipAddress = fe.getIpAddress
176176
info(s"Received listing all live sessions request from $userName/$ipAddress")
177177
if (!fe.isAdministrator(userName)) {
178-
throw new NotAllowedException(
178+
throw new ForbiddenException(
179179
s"$userName is not allowed to list all live sessions")
180180
}
181181
var sessions = fe.be.sessionManager.allSessions()
@@ -201,7 +201,7 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
201201
val ipAddress = fe.getIpAddress
202202
info(s"Received closing a session request from $userName/$ipAddress")
203203
if (!fe.isAdministrator(userName)) {
204-
throw new NotAllowedException(
204+
throw new ForbiddenException(
205205
s"$userName is not allowed to close the session $sessionHandleStr")
206206
}
207207
fe.be.closeSession(SessionHandle.fromUUID(sessionHandleStr))
@@ -226,7 +226,7 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
226226
val ipAddress = fe.getIpAddress
227227
info(s"Received listing all of the active operations request from $userName/$ipAddress")
228228
if (!fe.isAdministrator(userName)) {
229-
throw new NotAllowedException(
229+
throw new ForbiddenException(
230230
s"$userName is not allowed to list all the operations")
231231
}
232232
var operations = fe.be.sessionManager.operationManager.allOperations()
@@ -258,7 +258,7 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
258258
val ipAddress = fe.getIpAddress
259259
info(s"Received close an operation request from $userName/$ipAddress")
260260
if (!fe.isAdministrator(userName)) {
261-
throw new NotAllowedException(
261+
throw new ForbiddenException(
262262
s"$userName is not allowed to close the operation $operationHandleStr")
263263
}
264264
val operationHandle = OperationHandle(operationHandleStr)
@@ -371,7 +371,7 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
371371
val ipAddress = fe.getIpAddress
372372
info(s"Received list all live kyuubi servers request from $userName/$ipAddress")
373373
if (!fe.isAdministrator(userName)) {
374-
throw new NotAllowedException(
374+
throw new ForbiddenException(
375375
s"$userName is not allowed to list all live kyuubi servers")
376376
}
377377
val kyuubiConf = fe.getConf
@@ -442,7 +442,7 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
442442
val ipAddress = fe.getIpAddress
443443
info(s"Received counting batches request from $userName/$ipAddress")
444444
if (!fe.isAdministrator(userName)) {
445-
throw new NotAllowedException(
445+
throw new ForbiddenException(
446446
s"$userName is not allowed to count the batches")
447447
}
448448
val batchCount = batchService

kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/AdminResourceSuite.scala

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ class AdminResourceSuite extends KyuubiFunSuite with RestFrontendTestHelper {
9090
.request()
9191
.header(AUTHORIZATION_HEADER, HttpAuthUtils.basicAuthorizationHeader("admin002"))
9292
.post(null)
93-
assert(response.getStatus === 405)
93+
assert(response.getStatus === 403)
9494
}
9595

9696
test("refresh user defaults config of the kyuubi server") {
@@ -494,19 +494,19 @@ class AdminResourceSuite extends KyuubiFunSuite with RestFrontendTestHelper {
494494

495495
// use proxyUser
496496
val deleteEngineResponse1 = runDeleteEngine(Option(normalUser), None)
497-
assert(deleteEngineResponse1.getStatus === 405)
497+
assert(deleteEngineResponse1.getStatus === 403)
498498
val errorMessage = s"Failed to validate proxy privilege of anonymous for $normalUser"
499499
assert(deleteEngineResponse1.readEntity(classOf[String]).contains(errorMessage))
500500

501501
// it should be the same behavior as hive.server2.proxy.user
502502
val deleteEngineResponse2 = runDeleteEngine(None, Option(normalUser))
503-
assert(deleteEngineResponse2.getStatus === 405)
503+
assert(deleteEngineResponse2.getStatus === 403)
504504
assert(deleteEngineResponse2.readEntity(classOf[String]).contains(errorMessage))
505505

506506
// when both set, proxyUser takes precedence
507507
val deleteEngineResponse3 =
508508
runDeleteEngine(Option(normalUser), Option(s"${normalUser}HiveServer2"))
509-
assert(deleteEngineResponse3.getStatus === 405)
509+
assert(deleteEngineResponse3.getStatus === 403)
510510
assert(deleteEngineResponse3.readEntity(classOf[String]).contains(errorMessage))
511511
}
512512
}
@@ -745,19 +745,19 @@ class AdminResourceSuite extends KyuubiFunSuite with RestFrontendTestHelper {
745745

746746
// use proxyUser
747747
val listEngineResponse1 = runListEngine(Option(normalUser), None)
748-
assert(listEngineResponse1.getStatus === 405)
748+
assert(listEngineResponse1.getStatus === 403)
749749
val errorMessage = s"Failed to validate proxy privilege of anonymous for $normalUser"
750750
assert(listEngineResponse1.readEntity(classOf[String]).contains(errorMessage))
751751

752752
// it should be the same behavior as hive.server2.proxy.user
753753
val listEngineResponse2 = runListEngine(None, Option(normalUser))
754-
assert(listEngineResponse2.getStatus === 405)
754+
assert(listEngineResponse2.getStatus === 403)
755755
assert(listEngineResponse2.readEntity(classOf[String]).contains(errorMessage))
756756

757757
// when both set, proxyUser takes precedence
758758
val listEngineResponse3 =
759759
runListEngine(Option(normalUser), Option(s"${normalUser}HiveServer2"))
760-
assert(listEngineResponse3.getStatus === 405)
760+
assert(listEngineResponse3.getStatus === 403)
761761
assert(listEngineResponse3.readEntity(classOf[String]).contains(errorMessage))
762762
}
763763
}

kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/BatchesResourceSuite.scala

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ abstract class BatchesResourceSuiteBase extends KyuubiFunSuite
136136
.request(MediaType.APPLICATION_JSON_TYPE)
137137
.header(AUTHORIZATION_HEADER, basicAuthorizationHeader("anonymous"))
138138
.post(Entity.entity(proxyUserRequest, MediaType.APPLICATION_JSON_TYPE))
139-
assert(proxyUserResponse.getStatus === 405)
139+
assert(proxyUserResponse.getStatus === 403)
140140
var errorMessage = "Failed to validate proxy privilege of anonymous for root"
141141
assert(proxyUserResponse.readEntity(classOf[String]).contains(errorMessage))
142142

@@ -211,7 +211,7 @@ abstract class BatchesResourceSuiteBase extends KyuubiFunSuite
211211
.request(MediaType.APPLICATION_JSON_TYPE)
212212
.header(AUTHORIZATION_HEADER, basicAuthorizationHeader(batch.getId))
213213
.delete()
214-
assert(deleteBatchResponse.getStatus === 405)
214+
assert(deleteBatchResponse.getStatus === 403)
215215
errorMessage = s"Failed to validate proxy privilege of ${batch.getId} for anonymous"
216216
assert(deleteBatchResponse.readEntity(classOf[String]).contains(errorMessage))
217217

@@ -872,19 +872,19 @@ abstract class BatchesResourceSuiteBase extends KyuubiFunSuite
872872

873873
// use kyuubi.session.proxy.user
874874
val proxyUserResponse1 = runOpenBatchExecutor(Option(normalUser), None)
875-
assert(proxyUserResponse1.getStatus === 405)
875+
assert(proxyUserResponse1.getStatus === 403)
876876
val errorMessage = s"Failed to validate proxy privilege of anonymous for $normalUser"
877877
assert(proxyUserResponse1.readEntity(classOf[String]).contains(errorMessage))
878878

879879
// it should be the same behavior as hive.server2.proxy.user
880880
val proxyUserResponse2 = runOpenBatchExecutor(None, Option(normalUser))
881-
assert(proxyUserResponse2.getStatus === 405)
881+
assert(proxyUserResponse2.getStatus === 403)
882882
assert(proxyUserResponse2.readEntity(classOf[String]).contains(errorMessage))
883883

884884
// when both set, kyuubi.session.proxy.user takes precedence
885885
val proxyUserResponse3 =
886886
runOpenBatchExecutor(Option(normalUser), Option(s"${normalUser}HiveServer2"))
887-
assert(proxyUserResponse3.getStatus === 405)
887+
assert(proxyUserResponse3.getStatus === 403)
888888
assert(proxyUserResponse3.readEntity(classOf[String]).contains(errorMessage))
889889
}
890890
}

0 commit comments

Comments
 (0)