Repository navigation
feat(agent-service): authenticate requests via JWT and gateway ext_authz #9043
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Alwaysgaurav1
wants to merge
13
commits into
apache:main
Choose a base branch
from
Alwaysgaurav1:feat/5561-agent-service-auth
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+787
−162
Open
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
6e8f991
feat(agent): authenticate agent-service requests via JWT + gateway ex…
Alwaysgaurav1 f07f21c
style(agent): format agent-service and frontend files with prettier
Alwaysgaurav1 0146328
style(frontend): align agent service formatting with prettier-eslint
Alwaysgaurav1 153568a
style(frontend): format agent.service.ts with prettier-eslint
Alwaysgaurav1 a0791b6
fix(deps, ci): bump sbt/setup-sbt to v1.5.12 to restore CI (#9044)
Alwaysgaurav1 8dd84f6
Merge branch 'main' into feat/5561-agent-service-auth
Alwaysgaurav1 7eadccc
fix(agent-service): address review feedback on auth, routing, and specs
Alwaysgaurav1 75e304a
style: format Scala with scalafmt and frontend with prettier-eslint
Alwaysgaurav1 f4bf397
Merge branch 'main' into feat/5561-agent-service-auth
Alwaysgaurav1 5b899d9
Merge branch 'main' into feat/5561-agent-service-auth
Alwaysgaurav1 9d06b47
Merge branch 'main' into feat/5561-agent-service-auth
Alwaysgaurav1 a2d9418
Merge branch 'main' into feat/5561-agent-service-auth
Alwaysgaurav1 fb66377
Merge branch 'main' into feat/5561-agent-service-auth
Alwaysgaurav1 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,13 +23,13 @@ import com.typesafe.scalalogging.LazyLogging | |
| 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, PUT, Path, Produces} | ||
| import jakarta.ws.rs.{Consumes, DELETE, GET, PATCH, POST, PUT, Path, Produces} | ||
| import org.apache.texera.auth.JwtParser.parseToken | ||
| import org.apache.texera.auth.SessionUser | ||
| import org.apache.texera.auth.util.{ComputingUnitAccess, HeaderField} | ||
| import org.apache.texera.common.config.{GuiConfig, KubernetesConfig, LLMConfig} | ||
| import org.apache.texera.dao.SqlServer | ||
| import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum | ||
| import org.apache.texera.dao.jooq.generated.enums.{PrivilegeEnum, UserRoleEnum} | ||
| import org.apache.texera.dao.jooq.generated.tables.daos.{UserJupyterDao, WorkflowComputingUnitDao} | ||
|
|
||
| import java.net.{URI, URLDecoder} | ||
|
|
@@ -47,6 +47,8 @@ object AccessControlResource extends LazyLogging { | |
| private val apiExecutionsStats: Regex = """.*/api/executions/[0-9]+/stats/[0-9]+.*""".r | ||
| private val apiExecutionsResultExport: Regex = """.*/api/executions/result/export.*""".r | ||
| private val pveRoute: Regex = """^/?(?:auth/)?(?:api/|wsapi/)?pve(?:/.*)?$""".r | ||
| // Agent service: authenticate any /api/agents request (Phase 1 — see #5561). | ||
| private val apiAgents: Regex = """^/?(?:auth/)?api/agents(?:/.*)?$""".r | ||
| // Path patterns whose cuid lives in the URL path rather than the query string. | ||
| private val pvePvesCuidPath: Regex = """^/?(?:auth/)?(?:api/|wsapi/)?pve/pves/([0-9]+)$""".r | ||
| private val pvePackagesCuidPath: Regex = | ||
|
|
@@ -84,6 +86,8 @@ object AccessControlResource extends LazyLogging { | |
| case wsapiWorkflowWebsocket() | apiExecutionsStats() | apiExecutionsResultExport() | | ||
| pveRoute() => | ||
| checkComputingUnitAccess(uriInfo, headers, bodyOpt) | ||
| case apiAgents() => | ||
| checkAgentAccess(uriInfo, headers, bodyOpt) | ||
| case _ => | ||
| logger.warn(s"No authorization logic for path: $path. Denying access.") | ||
| Response.status(Response.Status.FORBIDDEN).build() | ||
|
|
@@ -125,6 +129,60 @@ object AccessControlResource extends LazyLogging { | |
| } | ||
| } | ||
|
|
||
| // Extract the bearer token from the access-token query param, the | ||
| // Authorization header, or a "token" field in the body (in that order). | ||
| private def extractBearerToken( | ||
| uriInfo: UriInfo, | ||
| headers: HttpHeaders, | ||
| bodyOpt: Option[String] | ||
| ): String = { | ||
| val qToken = Option(uriInfo.getQueryParameters().getFirst("access-token")) | ||
| .map(_.trim) | ||
| .filter(_.nonEmpty) | ||
| val hToken = Option(headers.getRequestHeader("Authorization")) | ||
| .flatMap(_.asScala.headOption) | ||
| .map(_.replaceFirst("(?i)^Bearer\\s+", "")) // case-insensitive "Bearer " | ||
| .map(_.trim) | ||
| .filter(_.nonEmpty) | ||
| val bToken = bodyOpt.flatMap(extractTokenFromBody) | ||
| qToken.orElse(hToken).orElse(bToken).getOrElse("") | ||
| } | ||
|
|
||
| // Phase 1 agent authorization: authenticate the JWT and require a | ||
| // REGULAR/ADMIN role. Any such user may reach any agent for now; per-agent | ||
| // ownership is deferred (see #5302 / #5561). On success, return the user | ||
| // identity headers for future per-agent authorization in agent-service. | ||
| private def checkAgentAccess( | ||
| uriInfo: UriInfo, | ||
| headers: HttpHeaders, | ||
| bodyOpt: Option[String] | ||
| ): Response = { | ||
| val token = extractBearerToken(uriInfo, headers, bodyOpt) | ||
| val userSession: Optional[SessionUser] = | ||
| try parseToken(token) | ||
| catch { | ||
| case e: Exception => | ||
| logger.error(s"Failed parsing token for agent request: $e") | ||
| Optional.empty() | ||
| } | ||
|
|
||
| if (userSession.isEmpty) { | ||
| return Response.status(Response.Status.UNAUTHORIZED).build() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Polish: The class comment at |
||
| } | ||
|
|
||
| val user = userSession.get() | ||
| if (!(user.isRoleOf(UserRoleEnum.REGULAR) || user.isRoleOf(UserRoleEnum.ADMIN))) { | ||
| return Response.status(Response.Status.FORBIDDEN).build() | ||
| } | ||
|
|
||
| Response | ||
| .ok() | ||
| .header(HeaderField.UserId, user.getUid.toString) | ||
| .header(HeaderField.UserName, user.getName) | ||
| .header(HeaderField.UserEmail, user.getEmail) | ||
| .build() | ||
| } | ||
|
|
||
| private def checkComputingUnitAccess( | ||
| uriInfo: UriInfo, | ||
| headers: HttpHeaders, | ||
|
|
@@ -141,16 +199,7 @@ object AccessControlResource extends LazyLogging { | |
| s"Request URI: ${uriInfo.getRequestUri} and headers: ${headers.getRequestHeaders.asScala} and queryParams: $queryParams" | ||
| ) | ||
|
|
||
| val token: String = { | ||
| val qToken = queryParams.get("access-token").filter(_.nonEmpty) | ||
| val hToken = Option(headers.getRequestHeader("Authorization")) | ||
| .flatMap(_.asScala.headOption) | ||
| .map(_.replaceFirst("(?i)^Bearer\\s+", "")) // case-insensitive "Bearer " | ||
| .map(_.trim) | ||
| .filter(_.nonEmpty) | ||
| val bToken = bodyOpt.flatMap(extractTokenFromBody) | ||
| qToken.orElse(hToken).orElse(bToken).getOrElse("") | ||
| } | ||
| val token: String = extractBearerToken(uriInfo, headers, bodyOpt) | ||
| logger.info(s"token extracted from request $token") | ||
|
|
||
| val cuid = queryParams.get("cuid").filter(_.nonEmpty).getOrElse { | ||
|
|
@@ -306,6 +355,16 @@ class AccessControlResource extends LazyLogging { | |
| AccessControlResource.authorize(uriInfo, headers, Option(body).map(_.trim).filter(_.nonEmpty)) | ||
| } | ||
|
|
||
| @PATCH | ||
| @Path("/{path:.*}") | ||
| def authorizePatch( | ||
| @Context uriInfo: UriInfo, | ||
| @Context headers: HttpHeaders, | ||
| body: String | ||
| ): Response = { | ||
| AccessControlResource.authorize(uriInfo, headers, Option(body).map(_.trim).filter(_.nonEmpty)) | ||
| } | ||
|
|
||
| @DELETE | ||
| @Path("/{path:.*}") | ||
| def authorizeDelete( | ||
|
|
||
112 changes: 112 additions & 0 deletions
112
...ntrol-service/src/test/scala/org/apache/texera/service/resource/AgentAccessAuthSpec.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| /* | ||
| * 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 jakarta.ws.rs.core.{HttpHeaders, MultivaluedHashMap, Response, UriInfo} | ||
| import org.apache.texera.auth.JwtAuth | ||
| import org.apache.texera.dao.jooq.generated.enums.UserRoleEnum | ||
| import org.apache.texera.dao.jooq.generated.tables.pojos.User | ||
| import org.mockito.Mockito.{mock, when} | ||
| import org.scalatest.flatspec.AnyFlatSpec | ||
| import org.scalatest.matchers.should.Matchers | ||
|
|
||
| import java.net.URI | ||
| import java.util.Collections | ||
|
|
||
| /** | ||
| * Exercises the `/api/agents` branch of [[AccessControlResource.authorize]] — | ||
| * the Phase 1 ext_authz gate that authenticates the JWT and requires a | ||
| * REGULAR/ADMIN role. Tokens are minted with the same [[JwtAuth]] the rest of | ||
| * Texera uses, so this is a real signature round-trip against auth.conf. | ||
| */ | ||
| class AgentAccessAuthSpec extends AnyFlatSpec with Matchers { | ||
|
|
||
| private def token(role: UserRoleEnum): String = { | ||
| val u = new User() | ||
| u.setUid(7) | ||
| u.setName("agent-user") | ||
| u.setEmail("agent-user@example.com") | ||
| u.setRole(role) | ||
| JwtAuth.jwtToken(JwtAuth.jwtClaims(u)) | ||
| } | ||
|
|
||
| private def buildMocks(path: String, authHeader: Option[String]): (UriInfo, HttpHeaders) = { | ||
| val uriInfo = mock(classOf[UriInfo]) | ||
| when(uriInfo.getPath).thenReturn(path) | ||
| when(uriInfo.getRequestUri).thenReturn(URI.create(s"http://localhost/$path")) | ||
| when(uriInfo.getQueryParameters()).thenReturn(new MultivaluedHashMap[String, String]()) | ||
|
|
||
| val headers = mock(classOf[HttpHeaders]) | ||
| when(headers.getRequestHeaders).thenReturn(new MultivaluedHashMap[String, String]()) | ||
| when(headers.getRequestHeader("Authorization")).thenReturn( | ||
| authHeader.map((h: String) => Collections.singletonList(h)).orNull | ||
| ) | ||
|
|
||
| (uriInfo, headers) | ||
| } | ||
|
|
||
| private def authorize(path: String, authHeader: Option[String]): Response = { | ||
| val (uriInfo, headers) = buildMocks(path, authHeader) | ||
| AccessControlResource.authorize(uriInfo, headers) | ||
| } | ||
|
|
||
| private val agentPath = "auth/api/agents/agent-1" | ||
|
|
||
| "authorize on /api/agents" should "return 200 for a REGULAR user" in { | ||
| authorize(agentPath, Some(s"Bearer ${token(UserRoleEnum.REGULAR)}")).getStatus shouldBe 200 | ||
| } | ||
|
|
||
| it should "return 200 for an ADMIN user" in { | ||
| authorize(agentPath, Some(s"Bearer ${token(UserRoleEnum.ADMIN)}")).getStatus shouldBe 200 | ||
| } | ||
|
|
||
| it should "forward the user identity headers on success" in { | ||
| val resp = authorize(agentPath, Some(s"Bearer ${token(UserRoleEnum.REGULAR)}")) | ||
| resp.getHeaderString("x-user-id") shouldBe "7" | ||
| resp.getHeaderString("x-user-name") shouldBe "agent-user" | ||
| resp.getHeaderString("x-user-email") shouldBe "agent-user@example.com" | ||
| } | ||
|
|
||
| it should "return 401 when no token is present" in { | ||
| authorize(agentPath, None).getStatus shouldBe 401 | ||
| } | ||
|
|
||
| it should "return 401 for a malformed / mis-signed token" in { | ||
| authorize(agentPath, Some("Bearer not-a-real-jwt")).getStatus shouldBe 401 | ||
| } | ||
|
|
||
| it should "return 403 for an INACTIVE-role user" in { | ||
| authorize(agentPath, Some(s"Bearer ${token(UserRoleEnum.INACTIVE)}")).getStatus shouldBe 403 | ||
| } | ||
|
|
||
| it should "authorize PATCH requests through authorizePatch handler" in { | ||
| val (uriInfo, headers) = | ||
| buildMocks(s"$agentPath/settings", Some(s"Bearer ${token(UserRoleEnum.REGULAR)}")) | ||
| val resource = new AccessControlResource() | ||
| resource.authorizePatch(uriInfo, headers, """{"model": "test"}""").getStatus shouldBe 200 | ||
| } | ||
|
|
||
| it should "not match dynamic wsapi routes containing api/agents and deny access" in { | ||
| authorize( | ||
| "auth/wsapi/x/api/agents", | ||
| Some(s"Bearer ${token(UserRoleEnum.REGULAR)}") | ||
| ).getStatus shouldBe 403 | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.