You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@texera.apache.org by gi...@apache.org on 2026/07/29 08:52:56 UTC

(texera) branch gh-readonly-queue/main/pr-7014-6da75b2092d004b4f1662e9d1ca9c1ba8aa2901a created (now 25d2ed8239)

This is an automated email from the ASF dual-hosted git repository.

github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-7014-6da75b2092d004b4f1662e9d1ca9c1ba8aa2901a
in repository https://gitbox.apache.org/repos/asf/texera.git


      at 25d2ed8239 fix(amber): require auth on retrieveWorkflowRuntimeStatistics (#7014)

This branch includes the following new commits:

     new 25d2ed8239 fix(amber): require auth on retrieveWorkflowRuntimeStatistics (#7014)

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(texera) 01/01: fix(amber): require auth on retrieveWorkflowRuntimeStatistics (#7014)

Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7014-6da75b2092d004b4f1662e9d1ca9c1ba8aa2901a
in repository https://gitbox.apache.org/repos/asf/texera.git

commit 25d2ed82393997d7ce3b87c87782b1f03cad2701
Author: Meng Wang <me...@uci.edu>
AuthorDate: Wed Jul 29 01:52:34 2026 -0700

    fix(amber): require auth on retrieveWorkflowRuntimeStatistics (#7014)
    
    ### What changes were proposed in this PR?
    
    `retrieveWorkflowRuntimeStatistics` (`GET
    /executions/{wid}/stats/{eid}`) was the only endpoint in
    `WorkflowExecutionsResource` declaring neither `@RolesAllowed` nor an
    `@Auth` parameter — every other handler in the class carries
    `@RolesAllowed(Array("REGULAR", "ADMIN"))` + `@Auth sessionUser`, and
    there is no class-level annotation to fall back on. Auth is enforced
    per-method via `RolesAllowedDynamicFeature`, so this one handler was
    reachable without authentication.
    
    - Add `@RolesAllowed(Array("REGULAR", "ADMIN"))` and an `@Auth
    sessionUser: SessionUser` parameter, matching the sibling endpoints
    exactly.
    - Add an **auth-annotation audit test** to
    `WorkflowExecutionsResourceSpec`: it reflects over every
    `@GET/@PUT/@POST/@DELETE` handler in the class and asserts each declares
    `@RolesAllowed` and takes an `@Auth` parameter — so any future endpoint
    added without auth turns the suite red instead of shipping unprotected.
    `exportResultToLocal` is explicitly exempted with a comment: it serves a
    browser form-submit download (which can't carry an `Authorization`
    header), so its JWT arrives as a form field and is verified in-method
    via `JwtParser.parseToken`, including the role check.
    
    The frontend is unaffected: `JwtModule` attaches the `Authorization`
    header to every request (`app.module.ts`), and the same service already
    calls sibling endpoints that require auth.
    
    ### Any related issues, documentation, discussions?
    
    Closes #6977.
    
    ### How was this PR tested?
    
    - The audit test **fails before the fix**, naming exactly
    `retrieveWorkflowRuntimeStatistics` as the offender, and passes after —
    the failing run is the repro for the issue. (Its first run also flagged
    `exportResultToLocal`; on inspection that endpoint authenticates
    manually by design — see above — which is why the exemption is explicit
    and documented rather than silent.)
    - Full `WorkflowExecutionsResourceSpec` run locally: all tests pass;
    `WorkflowExecutionService/scalafmtCheck` (main + Test) passes.
    - Verified no other caller of the method exists (the added parameter
    breaks no call site), and that all sibling endpoints use the identical
    annotation pattern being applied here.
    
    ### Was this PR authored or co-authored using generative AI tooling?
    
    Generated-by: Claude Code (claude-opus-4-8)
    
    ---------
    
    Signed-off-by: Xinyuan Lin <xi...@uci.edu>
    Co-authored-by: Xinyuan Lin <xi...@uci.edu>
    Co-authored-by: Copilot Autofix powered by AI <17...@users.noreply.github.com>
---
 .../user/workflow/WorkflowExecutionsResource.scala |  6 +++-
 .../workflow/WorkflowExecutionsResourceSpec.scala  | 33 ++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
index 784d678f37..e97d64ac7f 100644
--- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
+++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
@@ -662,10 +662,14 @@ class WorkflowExecutionsResource {
   @GET
   @Produces(Array(MediaType.APPLICATION_JSON))
   @Path("/{wid}/stats/{eid}")
+  @RolesAllowed(Array("REGULAR", "ADMIN"))
   def retrieveWorkflowRuntimeStatistics(
       @PathParam("wid") wid: Integer,
-      @PathParam("eid") eid: Integer
+      @PathParam("eid") eid: Integer,
+      @Auth sessionUser: SessionUser
   ): List[WorkflowRuntimeStatistics] = {
+    validateUserCanAccessWorkflow(sessionUser.getUser.getUid, wid)
+
     // Create URI for runtime statistics
     val uriString: String = context
       .select(WORKFLOW_EXECUTIONS.RUNTIME_STATS_URI)
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResourceSpec.scala
index 04104fb065..5e8100c2f7 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResourceSpec.scala
@@ -740,5 +740,38 @@ class WorkflowExecutionsResourceSpec
     val result = WorkflowExecutionsResource invokePrivate privateMethod(testWorkflowWid, testUser)
     assert(result.isEmpty)
   }
+  // ─── new: endpoint auth-annotation audit (#6977) ──────────────────────────
+
+  "WorkflowExecutionsResource endpoints" should "all declare @RolesAllowed and take an @Auth user" in {
+    val httpAnnotations: Seq[Class[_ <: java.lang.annotation.Annotation]] =
+      Seq(
+        classOf[javax.ws.rs.GET],
+        classOf[javax.ws.rs.PUT],
+        classOf[javax.ws.rs.POST],
+        classOf[javax.ws.rs.DELETE]
+      )
+    val handlers = classOf[WorkflowExecutionsResource].getDeclaredMethods.toSeq
+      .filter(m => httpAnnotations.exists(a => m.getAnnotation(a) != null))
+    assert(handlers.nonEmpty)
+
+    // exportResultToLocal authenticates manually: it serves a browser form-submit
+    // download, which cannot carry an Authorization header, so the JWT arrives as
+    // a form field and is verified in-method via JwtParser.parseToken (including
+    // the role check). Any other handler must use the declarative annotations.
+    val manuallyAuthenticated = Set("exportResultToLocal")
+
+    val offenders = handlers.filterNot(m => manuallyAuthenticated.contains(m.getName)).filter { m =>
+      val hasRoles =
+        m.getAnnotation(classOf[javax.annotation.security.RolesAllowed]) != null
+      val hasAuthParam = m.getParameterAnnotations.exists(
+        _.exists(_.annotationType() == classOf[io.dropwizard.auth.Auth])
+      )
+      !(hasRoles && hasAuthParam)
+    }
+    assert(
+      offenders.isEmpty,
+      s"endpoints missing @RolesAllowed/@Auth: ${offenders.map(_.getName).sorted.mkString(", ")}"
+    )
+  }
 
 }