Skip to content

fix: complete @RolesAllowed enforcement across microservices#5198

Open
Ma77Ball wants to merge 27 commits into
apache:mainfrom
Ma77Ball:fix/RolesAllowedUnenforced
Open

fix: complete @RolesAllowed enforcement across microservices#5198
Ma77Ball wants to merge 27 commits into
apache:mainfrom
Ma77Ball:fix/RolesAllowedUnenforced

Conversation

@Ma77Ball
Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Finishes wiring JAX-RS role enforcement across every Texera microservice. Each service's Jersey environment now registers the standard three-feature stack:

  1. AuthDynamicFeature(JwtAuthFilter) - authentication (reads the JWT, attaches a SessionUser).
  2. AuthValueFactoryProvider.Binder(SessionUser) - injects @Auth user into resource methods.
  3. RolesAllowedDynamicFeature - authorization (enforces @RolesAllowed / @PermitAll / @DenyAll).

#5049 added this trio to three services; this PR extends it to file-service and access-control-service, fixes the filter priority on the shared JwtAuthFilter so authentication runs before authorization, and tags every endpoint with the right policy.

What this PR adds, and where

  • common/auth/JwtAuthFilter - @Priority(Priorities.AUTHENTICATION) so it runs before RolesAllowedRequestFilter. Added jakarta.annotation-api dependency.
  • FileService.run and AccessControlService.run - register RolesAllowedDynamicFeature; the existing @RolesAllowed annotations in those services become live.
  • @PermitAll on endpoints that must serve unauthenticated traffic: ConfigResource bootstrap endpoints, the six public-dataset endpoints on DatasetResource, and AccessControlResource (the Envoy ExtAuth callback).
  • @RolesAllowed("REGULAR","ADMIN") on the LiteLLM copilot proxies.
  • AuthResource.register - new local signups now default to REGULAR instead of RESTRICTED, so registered users can immediately use role-gated endpoints.
  • FileService.run reorder - Jersey registration moved before infra init, mirroring ConfigService.run and making the registrations unit-testable.

Implementation details

  • Method-level annotations beat class-level; unannotated methods default to open (so unannotated HealthCheckResources stay public for k8s probes).
  • getDatasetCover is @PermitAll because it accepts optional auth; the method body re-checks access.
  • WebSocket endpoints are not affected — they use a separate query-param token path.

Tests added

JwtAuthFilterSpec, ConfigResourceSpec, AccessControlResourcePermissionsSpec, DatasetResourcePermissionsSpec, FileServiceRunSpec, plus an added assertion in AccessControlServiceRunSpec. Together, they pin the priority annotation, the public/private policy to every changed endpoint, and the registration of RolesAllowedDynamicFeature in the two newly enforced services.

Any related issues, documentation, or discussions?

Closes: #4904
Fixes issue found and addressed in: #5173 and #5049

How was this PR tested?

Added six specs covering the filter priority, the @PermitAll/@RolesAllowed tags on each affected resource, and RolesAllowedDynamicFeature registration in both newly-enforced services.

Manually verified end-to-end

Was this PR authored or co-authored using generative AI tooling?

Co-authored with Claude Opus 4.7 in compliance with ASF Generative Tooling Guidance.

Ma77Ball added 22 commits May 12, 2026 23:08
@github-actions github-actions Bot added engine dependencies Pull requests that update a dependency file fix common platform Non-amber Scala service paths labels May 25, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 25, 2026

Codecov Report

❌ Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.80%. Comparing base (c435aa7) to head (8e03082).

Files with missing lines Patch % Lines
...apache/texera/web/resource/auth/AuthResource.scala 0.00% 1 Missing ⚠️
.../scala/org/apache/texera/service/FileService.scala 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5198      +/-   ##
============================================
- Coverage     47.13%   45.80%   -1.34%     
+ Complexity     2344     2342       -2     
============================================
  Files          1042     1046       +4     
  Lines         39989    40030      +41     
  Branches       4260     4258       -2     
============================================
- Hits          18849    18335     -514     
- Misses        20015    20584     +569     
+ Partials       1125     1111      -14     
Flag Coverage Δ *Carryforward flag
access-control-service 39.88% <100.00%> (+0.34%) ⬆️
agent-service 33.74% <ø> (-0.03%) ⬇️ Carriedforward from 07be263
amber 50.29% <0.00%> (-0.04%) ⬇️
computing-unit-managing-service 0.00% <ø> (ø)
config-service 0.00% <ø> (ø)
file-service 32.10% <0.00%> (-0.09%) ⬇️
frontend 34.62% <ø> (-3.20%) ⬇️ Carriedforward from 07be263
python 90.50% <ø> (ø) Carriedforward from 07be263
workflow-compiling-service 56.81% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Ma77Ball
Copy link
Copy Markdown
Contributor Author

/request-review @aglinxinyuan @Yicong-Huang

@Ma77Ball
Copy link
Copy Markdown
Contributor Author

@carloea2 please also review and test if available

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common dependencies Pull requests that update a dependency file engine fix platform Non-amber Scala service paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Authorization bypass: @RolesAllowed unenforced on microservices, workflow-compiling-service requires no token at all

2 participants