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()
+    }
+  }
+}

Reply via email to