Skip to content

fix: enforce @RolesAllowed on microservice resources#5049

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

fix: enforce @RolesAllowed on microservice resources#5049
Ma77Ball wants to merge 19 commits into
apache:mainfrom
Ma77Ball:fix/RolesAllowedUnenforced

Conversation

@Ma77Ball
Copy link
Copy Markdown
Contributor

@Ma77Ball Ma77Ball commented May 13, 2026

What changes were proposed in this PR?

@RolesAllowed annotations on config-service, computing-unit-managing-service, and workflow-compiling-service resources were decorative because none of these services registered Jersey's RolesAllowedDynamicFeature. This PR registers that feature in each service's run(...). For workflow-compiling-service, which was not registering JWT auth at all, this PR also registers AuthDynamicFeature(JwtAuthFilter) and the SessionUser AuthValueFactoryProvider.Binder, and adds Auth as an sbt dependency for the module. access-control-service and file-service use no @RolesAllowed today and were intentionally left alone to keep the change minimal.

Any related issues, documentation, or discussions?

Closes: #4904

How was this PR tested?

Added ConfigServiceRunSpec (mirrors AccessControlServiceRunSpec) that mocks the Jersey environment and verifies RolesAllowedDynamicFeature is registered when ConfigService.run runs. The same one-line registration applies to the other two services; tests there would require either refactoring SqlServer.initConnection out of run or static-mocking the Scala SqlServer object, both of which are larger than the fix itself, so they are out of scope. Manual verification via the reproduction in the issue (low-role JWT against an annotated endpoint should now return 403; unauthenticated request to WorkflowCompilationResource should now return 401).

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

Co-authored with Claude Opus 4.7 in compliance with ASF

@github-actions github-actions Bot added dependencies Pull requests that update a dependency file fix common platform Non-amber Scala service paths labels May 13, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 13, 2026

Codecov Report

❌ Patch coverage is 56.25000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.09%. Comparing base (bf2f92c) to head (877960f).

Files with missing lines Patch % Lines
.../texera/service/ComputingUnitManagingService.scala 44.44% 5 Missing ⚠️
...ache/texera/service/WorkflowCompilingService.scala 66.66% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5049      +/-   ##
============================================
- Coverage     43.35%   43.09%   -0.26%     
- Complexity     2212     2218       +6     
============================================
  Files          1049     1045       -4     
  Lines         40560    40237     -323     
  Branches       4322     4251      -71     
============================================
- Hits          17583    17341     -242     
+ Misses        21886    21822      -64     
+ Partials       1091     1074      -17     
Flag Coverage Δ *Carryforward flag
access-control-service 39.53% <ø> (ø)
agent-service 33.64% <ø> (-0.12%) ⬇️ Carriedforward from 6e68b19
amber 43.81% <ø> (+<0.01%) ⬆️
computing-unit-managing-service 1.38% <44.44%> (+1.38%) ⬆️
config-service 19.35% <100.00%> (+19.35%) ⬆️
file-service 32.18% <ø> (ø)
frontend 33.87% <ø> (-0.75%) ⬇️ Carriedforward from 6e68b19
python 90.37% <ø> (-0.14%) ⬇️ Carriedforward from 6e68b19
workflow-compiling-service 58.39% <66.66%> (+1.57%) ⬆️

*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 @Yicong-Huang

@github-actions github-actions Bot requested a review from Yicong-Huang May 13, 2026 06:18
@Ma77Ball
Copy link
Copy Markdown
Contributor Author

@Yicong-Huang please review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a security gap where @RolesAllowed annotations on several Dropwizard microservices were not being enforced because RolesAllowedDynamicFeature was not registered with Jersey. It also wires JWT auth into workflow-compiling-service, which previously did not register JWT auth at all.

Changes:

  • Register RolesAllowedDynamicFeature in config-service, computing-unit-managing-service, and workflow-compiling-service.
  • Add JWT auth wiring (AuthDynamicFeature(JwtAuthFilter) + AuthValueFactoryProvider.Binder(SessionUser)) to workflow-compiling-service, plus add the needed module dependency.
  • Add “run() registers security features” unit tests (but two of them currently trigger real DB init paths).

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
workflow-compiling-service/src/main/scala/org/apache/texera/service/WorkflowCompilingService.scala Registers JWT auth + RolesAllowedDynamicFeature so @RolesAllowed is enforced.
workflow-compiling-service/src/test/scala/org/apache/texera/service/WorkflowCompilingServiceRunSpec.scala Adds a startup registration test (currently brittle due to DB init ordering).
workflow-compiling-service/build.sbt Adds dropwizard-auth dependency for the service.
workflow-compiling-service/LICENSE-binary Updates binary license inventory for newly pulled jars.
config-service/src/main/scala/org/apache/texera/service/ConfigService.scala Registers RolesAllowedDynamicFeature so @RolesAllowed is enforced.
config-service/src/test/scala/org/apache/texera/service/ConfigServiceRunSpec.scala Adds a startup registration test for RolesAllowedDynamicFeature.
computing-unit-managing-service/src/main/scala/org/apache/texera/service/ComputingUnitManagingService.scala Registers RolesAllowedDynamicFeature so @RolesAllowed is enforced.
computing-unit-managing-service/src/test/scala/org/apache/texera/service/ComputingUnitManagingServiceRunSpec.scala Adds a startup registration test (currently brittle due to DB init ordering).
computing-unit-managing-service/build.sbt Adds Mockito/ScalaTest test dependencies for the new spec.
build.sbt Adds Auth module dependency to WorkflowCompilingService.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@Yicong-Huang Yicong-Huang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM besides the test ordering issue.

@Ma77Ball
Copy link
Copy Markdown
Contributor Author

@Yicong-Huang, I addressed the comments. Please merge when 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 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

4 participants