This is an automated email from the ASF dual-hosted git repository.
Yicong-Huang pushed a commit to branch release/v1.2
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/release/v1.2 by this push:
new 24e5b70187 fix(auth): require REGULAR/ADMIN role on LiteLLM proxy
endpoints (#5421)
24e5b70187 is described below
commit 24e5b70187d36b25bd04552e555cccaaf11d912e
Author: Yicong Huang <[email protected]>
AuthorDate: Sun Jun 7 19:14:09 2026 +0000
fix(auth): require REGULAR/ADMIN role on LiteLLM proxy endpoints (#5421)
### What changes were proposed in this PR?
`LiteLLMProxyResource` (`/chat/*`) and `LiteLLMModelsResource`
(`/models`) in `access-control-service` were `@PermitAll` — they contain
no auth check beyond a `guiWorkflowWorkspaceCopilotEnabled` feature
flag, so any anonymous caller could spend the deployment's LiteLLM
credits whenever Copilot was on. PR #5404 left the annotations in place
to preserve pre-eager-filter behavior and deferred the hardening
decision; this picks it back up.
Both classes now use `@RolesAllowed(Array("REGULAR", "ADMIN"))`. The
JwtAuthFilter from #5404 runs at `Priorities.AUTHENTICATION`, so missing
tokens fall to a 401 from the filter and bad-role tokens fall through to
a 403 from Jersey's role check.
`access-control-service` was the one microservice that hadn't picked up
`RolesAllowedDynamicFeature` when #5199 added it elsewhere, so this PR
also registers the feature in `AccessControlService.scala` — otherwise
the annotations would have been decorative.
`AccessControlServiceRunSpec` is updated to pin the registration so a
future refactor can't quietly drop it and send us back to
anonymous-LLM-access.
### Any related issues, documentation, discussions?
Closes #5420. Follow-up hardening for #5404.
### How was this PR tested?
`sbt AccessControlService/test` (22 cases) and `sbt scalafmtCheckAll`
clean. Manual verification against a running access-control-service:
anonymous `curl http://localhost:8081/api/models` returns 401 with
`WWW-Authenticate: Bearer realm="texera"` instead of the pre-PR 200; a
REGULAR-role bearer token gets through to LiteLLM as expected.
### Was this PR authored or co-authored using generative AI tooling?
(backported from commit afc5f98c8e512c1e90f8fee86dd0a596e58d08d9)
Generated-by: Claude Code (Opus 4.7)
---
.../texera/service/AccessControlService.scala | 4 +
.../service/resource/AccessControlResource.scala | 57 ++--
.../service/AccessControlServiceRunSpec.scala | 3 +
.../service/resource/LiteLLMProxyAuthSpec.scala | 299 +++++++++++++++++++++
4 files changed, 345 insertions(+), 18 deletions(-)
diff --git
a/access-control-service/src/main/scala/org/apache/texera/service/AccessControlService.scala
b/access-control-service/src/main/scala/org/apache/texera/service/AccessControlService.scala
index 0cb5738419..e262b80900 100644
---
a/access-control-service/src/main/scala/org/apache/texera/service/AccessControlService.scala
+++
b/access-control-service/src/main/scala/org/apache/texera/service/AccessControlService.scala
@@ -39,6 +39,7 @@ import org.apache.texera.service.resource.{
LiteLLMProxyResource
}
import org.eclipse.jetty.server.session.SessionHandler
+import org.glassfish.jersey.server.filter.RolesAllowedDynamicFeature
import java.nio.file.Path
class AccessControlService extends
Application[AccessControlServiceConfiguration] with LazyLogging {
@@ -84,6 +85,9 @@ class AccessControlService extends
Application[AccessControlServiceConfiguration
new
io.dropwizard.auth.AuthValueFactoryProvider.Binder(classOf[SessionUser])
)
+ // Required for @RolesAllowed on resources to be enforced.
+ environment.jersey.register(classOf[RolesAllowedDynamicFeature])
+
// Record USER_LAST_ACTIVE_TIME on every matched, completed request.
// Lives only in this service because authenticated client sessions
// contact access-control-service often enough to capture activity
diff --git
a/access-control-service/src/main/scala/org/apache/texera/service/resource/AccessControlResource.scala
b/access-control-service/src/main/scala/org/apache/texera/service/resource/AccessControlResource.scala
index 259b9f1f50..0c90a6ce31 100644
---
a/access-control-service/src/main/scala/org/apache/texera/service/resource/AccessControlResource.scala
+++
b/access-control-service/src/main/scala/org/apache/texera/service/resource/AccessControlResource.scala
@@ -20,7 +20,7 @@ package org.apache.texera.service.resource
import com.fasterxml.jackson.databind.ObjectMapper
import com.fasterxml.jackson.module.scala.DefaultScalaModule
import com.typesafe.scalalogging.LazyLogging
-import jakarta.annotation.security.PermitAll
+import jakarta.annotation.security.{PermitAll, RolesAllowed}
import jakarta.ws.rs.client.{Client, ClientBuilder, Entity}
import jakarta.ws.rs.core._
import jakarta.ws.rs.{Consumes, DELETE, GET, POST, Path, Produces}
@@ -243,20 +243,27 @@ class AccessControlResource extends LazyLogging {
}
}
-// LiteLLM proxy: gates on `guiWorkflowWorkspaceCopilotEnabled`, not on
-// JWT. Preserve pre-eager-filter behavior (anonymous access permitted when
-// the feature flag is on) by opting out of the filter's eager 401. Whether
-// /chat/* should require an authenticated user is a separate hardening
-// decision tracked outside this PR.
+// Forwards chat completions to LiteLLM with the server's master key, so
+// only authenticated users may call it.
@Path("/chat")
-@PermitAll
+@RolesAllowed(Array("REGULAR", "ADMIN"))
@Produces(Array(MediaType.APPLICATION_JSON))
@Consumes(Array(MediaType.APPLICATION_JSON))
-class LiteLLMProxyResource extends LazyLogging {
+class LiteLLMProxyResource(
+ copilotEnabled: Boolean,
+ litellmBaseUrl: String,
+ litellmApiKey: String
+) extends LazyLogging {
+
+ // No-arg constructor for Jersey reflection. Tests use the param-ful form.
+ def this() =
+ this(
+ GuiConfig.guiWorkflowWorkspaceCopilotEnabled,
+ LLMConfig.baseUrl,
+ LLMConfig.masterKey
+ )
private val client: Client = ClientBuilder.newClient()
- private val litellmBaseUrl: String = LLMConfig.baseUrl
- private val litellmApiKey: String = LLMConfig.masterKey
@POST
@Path("/{path:.*}")
@@ -265,10 +272,10 @@ class LiteLLMProxyResource extends LazyLogging {
@Context headers: HttpHeaders,
body: String
): Response = {
- if (!GuiConfig.guiWorkflowWorkspaceCopilotEnabled) {
+ if (!copilotEnabled) {
return Response
.status(Response.Status.FORBIDDEN)
- .entity("""{"error": "Copilot feature is disabled"}""")
+ .entity(LiteLLMProxyResource.CopilotDisabledBody)
.build()
}
@@ -321,21 +328,35 @@ class LiteLLMProxyResource extends LazyLogging {
}
}
+object LiteLLMProxyResource {
+ val CopilotDisabledBody: String = """{"error": "Copilot feature is
disabled"}"""
+}
+
@Path("/models")
-@PermitAll
+@RolesAllowed(Array("REGULAR", "ADMIN"))
@Produces(Array(MediaType.APPLICATION_JSON))
-class LiteLLMModelsResource extends LazyLogging {
+class LiteLLMModelsResource(
+ copilotEnabled: Boolean,
+ litellmBaseUrl: String,
+ litellmApiKey: String
+) extends LazyLogging {
+
+ // No-arg constructor for Jersey reflection. Tests use the param-ful form.
+ def this() =
+ this(
+ GuiConfig.guiWorkflowWorkspaceCopilotEnabled,
+ LLMConfig.baseUrl,
+ LLMConfig.masterKey
+ )
private val client: Client = ClientBuilder.newClient()
- private val litellmBaseUrl: String = LLMConfig.baseUrl
- private val litellmApiKey: String = LLMConfig.masterKey
@GET
def getModels: Response = {
- if (!GuiConfig.guiWorkflowWorkspaceCopilotEnabled) {
+ if (!copilotEnabled) {
return Response
.status(Response.Status.FORBIDDEN)
- .entity("""{"error": "Copilot feature is disabled"}""")
+ .entity(LiteLLMProxyResource.CopilotDisabledBody)
.build()
}
diff --git
a/access-control-service/src/test/scala/org/apache/texera/service/AccessControlServiceRunSpec.scala
b/access-control-service/src/test/scala/org/apache/texera/service/AccessControlServiceRunSpec.scala
index 96bfd104d9..2460d18b45 100644
---
a/access-control-service/src/test/scala/org/apache/texera/service/AccessControlServiceRunSpec.scala
+++
b/access-control-service/src/test/scala/org/apache/texera/service/AccessControlServiceRunSpec.scala
@@ -25,6 +25,7 @@ import io.dropwizard.jetty.MutableServletContextHandler
import io.dropwizard.jetty.setup.ServletEnvironment
import org.apache.texera.auth.UnauthorizedExceptionMapper
import org.apache.texera.service.activity.UserActivityEventListener
+import org.glassfish.jersey.server.filter.RolesAllowedDynamicFeature
import org.mockito.ArgumentMatchers.isA
import org.mockito.Mockito.{mock, verify, when}
import org.scalatest.flatspec.AnyFlatSpec
@@ -46,6 +47,8 @@ class AccessControlServiceRunSpec extends AnyFlatSpec with
Matchers {
verify(jersey).register(isA(classOf[UserActivityEventListener]))
verify(jersey).register(classOf[UnauthorizedExceptionMapper])
+ // Without this feature Jersey ignores @RolesAllowed on the LiteLLM
proxies.
+ verify(jersey).register(classOf[RolesAllowedDynamicFeature])
verify(jersey).setUrlPattern("/api/*")
}
}
diff --git
a/access-control-service/src/test/scala/org/apache/texera/service/resource/LiteLLMProxyAuthSpec.scala
b/access-control-service/src/test/scala/org/apache/texera/service/resource/LiteLLMProxyAuthSpec.scala
new file mode 100644
index 0000000000..4d8d271c7d
--- /dev/null
+++
b/access-control-service/src/test/scala/org/apache/texera/service/resource/LiteLLMProxyAuthSpec.scala
@@ -0,0 +1,299 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.service.resource
+
+import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.module.scala.DefaultScalaModule
+import com.sun.net.httpserver.{HttpExchange, HttpHandler, HttpServer}
+import io.dropwizard.jackson.Jackson
+import io.dropwizard.testing.junit5.ResourceExtension
+import jakarta.ws.rs.client.Entity
+import jakarta.ws.rs.core.MediaType
+import org.apache.texera.auth.{JwtAuth, JwtAuthFilter,
UnauthorizedExceptionMapper}
+import org.apache.texera.dao.jooq.generated.enums.UserRoleEnum
+import org.apache.texera.dao.jooq.generated.tables.pojos.User
+import org.glassfish.jersey.server.filter.RolesAllowedDynamicFeature
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+import java.net.InetSocketAddress
+import java.nio.charset.StandardCharsets
+
+// Wires the LiteLLM proxy resources through the same Jersey auth pipeline
+// production uses and fires HTTP requests with no / wrong-role / right-role
+// Bearer tokens. The @RolesAllowed annotations are only enforced when
+// AccessControlService registers RolesAllowedDynamicFeature; this spec is
+// the regression guard that the annotation, the registration, and the
+// JwtAuthFilter priority continue to compose into the expected status codes.
+class LiteLLMProxyAuthSpec extends AnyFlatSpec with Matchers with
BeforeAndAfterAll {
+
+ private val testMapper: ObjectMapper =
+ Jackson.newObjectMapper().registerModule(DefaultScalaModule)
+
+ // Inject copilotEnabled = true and a guaranteed-unreachable upstream so
+ // requests that pass the auth + role gates fall straight into the
+ // resource's "proxy attempt failed" branch — that 502 confirms the auth
+ // pipeline didn't short-circuit before the resource body ran.
+ private val unreachableLiteLLM = "http://127.0.0.1:1"
+
+ private val resources: ResourceExtension = ResourceExtension
+ .builder()
+ .setMapper(testMapper)
+ .addProvider(classOf[JwtAuthFilter])
+ .addProvider(classOf[UnauthorizedExceptionMapper])
+ .addProvider(classOf[RolesAllowedDynamicFeature])
+ .addResource(
+ new LiteLLMProxyResource(
+ copilotEnabled = true,
+ litellmBaseUrl = unreachableLiteLLM,
+ litellmApiKey = "test"
+ )
+ )
+ .addResource(
+ new LiteLLMModelsResource(
+ copilotEnabled = true,
+ litellmBaseUrl = unreachableLiteLLM,
+ litellmApiKey = "test"
+ )
+ )
+ .build()
+
+ // Second resource extension with copilotEnabled = false, used to exercise
+ // the resource's early-exit branch that returns "Copilot feature is
disabled".
+ private val resourcesCopilotDisabled: ResourceExtension = ResourceExtension
+ .builder()
+ .setMapper(testMapper)
+ .addProvider(classOf[JwtAuthFilter])
+ .addProvider(classOf[UnauthorizedExceptionMapper])
+ .addProvider(classOf[RolesAllowedDynamicFeature])
+ .addResource(
+ new LiteLLMProxyResource(
+ copilotEnabled = false,
+ litellmBaseUrl = unreachableLiteLLM,
+ litellmApiKey = "test"
+ )
+ )
+ .addResource(
+ new LiteLLMModelsResource(
+ copilotEnabled = false,
+ litellmBaseUrl = unreachableLiteLLM,
+ litellmApiKey = "test"
+ )
+ )
+ .build()
+
+ // An in-process HTTP server stands in for LiteLLM so the resources' success
+ // path (header / body / status forwarding) is exercised end-to-end without
+ // a network dependency. Bound to port 0 to pick any free port.
+ private val mockChatBody =
"""{"id":"mock-chat","choices":[{"message":{"content":"hi"}}]}"""
+ private val mockModelsBody = """{"data":[{"id":"mock-gpt"}]}"""
+
+ private val mockLiteLLM: HttpServer = HttpServer.create(new
InetSocketAddress(0), 0)
+ mockLiteLLM.createContext("/chat/completions", respondWith(200,
mockChatBody))
+ mockLiteLLM.createContext("/models", respondWith(200, mockModelsBody))
+
+ private def respondWith(status: Int, body: String): HttpHandler =
+ (exchange: HttpExchange) => {
+ val bytes = body.getBytes(StandardCharsets.UTF_8)
+ exchange.getResponseHeaders.add("Content-Type",
MediaType.APPLICATION_JSON)
+ exchange.sendResponseHeaders(status, bytes.length.toLong)
+ val os = exchange.getResponseBody
+ try os.write(bytes)
+ finally os.close()
+ }
+
+ private def mockBaseUrl: String =
s"http://127.0.0.1:${mockLiteLLM.getAddress.getPort}"
+
+ // Third extension: copilot on, upstream reachable. Resource is built lazily
+ // because litellmBaseUrl depends on the mock server's bound port.
+ private lazy val resourcesMockLiteLLM: ResourceExtension = ResourceExtension
+ .builder()
+ .setMapper(testMapper)
+ .addProvider(classOf[JwtAuthFilter])
+ .addProvider(classOf[UnauthorizedExceptionMapper])
+ .addProvider(classOf[RolesAllowedDynamicFeature])
+ .addResource(
+ new LiteLLMProxyResource(
+ copilotEnabled = true,
+ litellmBaseUrl = mockBaseUrl,
+ litellmApiKey = "test"
+ )
+ )
+ .addResource(
+ new LiteLLMModelsResource(
+ copilotEnabled = true,
+ litellmBaseUrl = mockBaseUrl,
+ litellmApiKey = "test"
+ )
+ )
+ .build()
+
+ override protected def beforeAll(): Unit = {
+ mockLiteLLM.start()
+ resources.before()
+ resourcesCopilotDisabled.before()
+ resourcesMockLiteLLM.before()
+ }
+ override protected def afterAll(): Unit = {
+ resourcesMockLiteLLM.after()
+ resourcesCopilotDisabled.after()
+ resources.after()
+ mockLiteLLM.stop(0)
+ }
+
+ private def token(role: UserRoleEnum): String = {
+ val u = new User()
+ u.setUid(1)
+ u.setName("test")
+ u.setEmail("[email protected]")
+ u.setGoogleId(null)
+ u.setRole(role)
+ JwtAuth.jwtToken(JwtAuth.jwtClaims(u, expireInDays = 1))
+ }
+
+ private val chatBody = """{"model":"gpt-4o-mini","messages":[]}"""
+
+ "POST /chat/completions without an Authorization header" should "return 401
with a Bearer challenge" in {
+ val response = resources
+ .target("/chat/completions")
+ .request(MediaType.APPLICATION_JSON)
+ .post(Entity.json(chatBody))
+ response.getStatus shouldBe 401
+ response.getHeaderString("WWW-Authenticate") shouldBe
JwtAuthFilter.BearerChallenge
+ }
+
+ it should "return 403 with an INACTIVE-role token" in {
+ val response = resources
+ .target("/chat/completions")
+ .request(MediaType.APPLICATION_JSON)
+ .header("Authorization", s"Bearer ${token(UserRoleEnum.INACTIVE)}")
+ .post(Entity.json(chatBody))
+ response.getStatus shouldBe 403
+ }
+
+ it should "return 502 when the upstream LiteLLM call fails" in {
+ // Exercises the resource's catch branch via a connect-refused upstream.
+ val response = resources
+ .target("/chat/completions")
+ .request(MediaType.APPLICATION_JSON)
+ .header("Authorization", s"Bearer ${token(UserRoleEnum.REGULAR)}")
+ .post(Entity.json(chatBody))
+ response.getStatus shouldBe 502
+ }
+
+ it should "return the resource's Copilot-disabled response when copilot is
off" in {
+ // 403 alone is ambiguous (could be from RolesAllowedDynamicFeature);
+ // matching the entity to the same constant the resource emits proves the
+ // role check passed and the resource's own early-exit branch fired.
+ val response = resourcesCopilotDisabled
+ .target("/chat/completions")
+ .request(MediaType.APPLICATION_JSON)
+ .header("Authorization", s"Bearer ${token(UserRoleEnum.REGULAR)}")
+ .post(Entity.json(chatBody))
+ response.getStatus shouldBe 403
+ response.readEntity(classOf[String]) shouldBe
LiteLLMProxyResource.CopilotDisabledBody
+ }
+
+ "GET /models without an Authorization header" should "return 401 with a
Bearer challenge" in {
+ val response =
resources.target("/models").request(MediaType.APPLICATION_JSON).get()
+ response.getStatus shouldBe 401
+ response.getHeaderString("WWW-Authenticate") shouldBe
JwtAuthFilter.BearerChallenge
+ }
+
+ it should "return 403 with an INACTIVE-role token" in {
+ val response = resources
+ .target("/models")
+ .request(MediaType.APPLICATION_JSON)
+ .header("Authorization", s"Bearer ${token(UserRoleEnum.INACTIVE)}")
+ .get()
+ response.getStatus shouldBe 403
+ }
+
+ it should "return 502 when the upstream LiteLLM call fails" in {
+ val response = resources
+ .target("/models")
+ .request(MediaType.APPLICATION_JSON)
+ .header("Authorization", s"Bearer ${token(UserRoleEnum.ADMIN)}")
+ .get()
+ response.getStatus shouldBe 502
+ }
+
+ it should "return the resource's Copilot-disabled response when copilot is
off" in {
+ val response = resourcesCopilotDisabled
+ .target("/models")
+ .request(MediaType.APPLICATION_JSON)
+ .header("Authorization", s"Bearer ${token(UserRoleEnum.ADMIN)}")
+ .get()
+ response.getStatus shouldBe 403
+ response.readEntity(classOf[String]) shouldBe
LiteLLMProxyResource.CopilotDisabledBody
+ }
+
+ "POST /chat/completions" should "forward the upstream response when copilot
is on and upstream is reachable" in {
+ val response = resourcesMockLiteLLM
+ .target("/chat/completions")
+ .request(MediaType.APPLICATION_JSON)
+ .header("Authorization", s"Bearer ${token(UserRoleEnum.REGULAR)}")
+ .post(Entity.json(chatBody))
+ response.getStatus shouldBe 200
+ response.readEntity(classOf[String]) shouldBe mockChatBody
+ }
+
+ "GET /models" should "forward the upstream response when copilot is on and
upstream is reachable" in {
+ val response = resourcesMockLiteLLM
+ .target("/models")
+ .request(MediaType.APPLICATION_JSON)
+ .header("Authorization", s"Bearer ${token(UserRoleEnum.ADMIN)}")
+ .get()
+ response.getStatus shouldBe 200
+ response.readEntity(classOf[String]) shouldBe mockModelsBody
+ }
+
+ // Regression guard for the no-arg auxiliary constructor that Jersey
+ // reflection picks at production startup. Jersey resolves constructors in
+ // descending parameter count and skips any whose parameters are not
+ // @Context / HK2 injectable; LiteLLMProxyResource's 3-arg ctor takes plain
+ // Boolean / String values, so the no-arg form must exist and be picked.
+ // addResource(classOf[...]) (vs. addResource(new ...)) exercises that path.
+ "Jersey reflection" should "instantiate both LiteLLM resources via their
no-arg constructors" in {
+ val reflective = ResourceExtension
+ .builder()
+ .setMapper(testMapper)
+ .addProvider(classOf[JwtAuthFilter])
+ .addProvider(classOf[UnauthorizedExceptionMapper])
+ .addProvider(classOf[RolesAllowedDynamicFeature])
+ .addResource(classOf[LiteLLMProxyResource])
+ .addResource(classOf[LiteLLMModelsResource])
+ .build()
+ reflective.before()
+ try {
+ val chat =
reflective.target("/chat/completions").request(MediaType.APPLICATION_JSON).get()
+ // Unauthenticated GET on the POST-only chat path: we just need any
+ // response that proves Jersey wired the resource (4xx is fine; an
+ // instantiation failure surfaces as 500 or a test setup error).
+ chat.getStatus should (be >= 400 and be < 500)
+
+ val models =
reflective.target("/models").request(MediaType.APPLICATION_JSON).get()
+ models.getStatus shouldBe 401
+ } finally {
+ reflective.after()
+ }
+ }
+}