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 <[email protected]> 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 <[email protected]> Co-authored-by: Xinyuan Lin <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> --- .../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(", ")}" + ) + } }
