fix(amber, v1.2): require auth on retrieveWorkflowRuntimeStatistics - #7054
Merged
Yicong-Huang merged 2 commits intoJul 30, 2026
Merged
Conversation
### 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)
---------
(backported from commit 0635737)
Signed-off-by: Xinyuan Lin <xinyual3@uci.edu>
Co-authored-by: Xinyuan Lin <xinyual3@uci.edu>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
Author
|
The cherry-pick applied cleanly but the backported tree failed its pre-merge build. Fix the build on this branch, then mark this PR ready for review. |
Contributor
There was a problem hiding this comment.
Pull request overview
Backports the security fix from #7014 onto release/v1.2 by ensuring WorkflowExecutionsResource.retrieveWorkflowRuntimeStatistics is protected by the same declarative authentication/authorization pattern used by the rest of the resource, and adds a regression-style audit test to prevent future unauthenticated endpoints from being introduced.
Changes:
- Require
@RolesAllowed(Array("REGULAR", "ADMIN"))and an@Auth sessionUserparameter onGET /executions/{wid}/stats/{eid}. - Add an auth-annotation audit test that reflects over
WorkflowExecutionsResourceHTTP handlers and asserts each declares@RolesAllowedand accepts an@Authparameter (with an explicit exemption forexportResultToLocal, which authenticates manually).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala | Secures retrieveWorkflowRuntimeStatistics with roles + injected auth user, and enforces workflow access check before returning runtime stats. |
| amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResourceSpec.scala | Adds a reflection-based audit test to ensure all HTTP endpoints in WorkflowExecutionsResource declare @RolesAllowed and take an @Auth parameter (excluding the known manual-auth download endpoint). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…veworkflowruntimes-v1.2
Contributor
Author
Automated Reviewer SuggestionsBased on the
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/v1.2 #7054 +/- ##
==================================================
- Coverage 52.49% 52.48% -0.02%
+ Complexity 2498 2494 -4
==================================================
Files 1077 1077
Lines 42296 42297 +1
Branches 4551 4551
==================================================
- Hits 22203 22198 -5
- Misses 18782 18787 +5
- Partials 1311 1312 +1
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
@xuang7 PTAL |
xuang7
approved these changes
Jul 30, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to Branch Protection failures
Jul 30, 2026
You're not authorized to push to this branch. Visit "About protected branches" for more information.
github-merge-queue
Bot
removed this pull request from the merge queue due to no response for status checks
Jul 30, 2026
Yicong-Huang
deleted the
backport/7014-require-auth-on-retrieveworkflowruntimes-v1.2
branch
July 30, 2026 18:09
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What changes were proposed in this PR?
Automated backport of #7014 to
release/v1.2.Source: 0635737 · automation run
Any related issues, documentation, discussions?
Backport of #7014. Originally linked #6977.
How was this PR tested?
Release-branch CI runs on this branch once the build is fixed and this PR is marked ready for review.
Was this PR authored or co-authored using generative AI tooling?
No.