fix: enforce @RolesAllowed on microservice resources#5049
Conversation
Codecov Report❌ Patch coverage is
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
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/request-review @Yicong-Huang |
… fix/RolesAllowedUnenforced
|
@Yicong-Huang please review |
There was a problem hiding this comment.
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
RolesAllowedDynamicFeatureinconfig-service,computing-unit-managing-service, andworkflow-compiling-service. - Add JWT auth wiring (
AuthDynamicFeature(JwtAuthFilter)+AuthValueFactoryProvider.Binder(SessionUser)) toworkflow-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.
Yicong-Huang
left a comment
There was a problem hiding this comment.
LGTM besides the test ordering issue.
|
@Yicong-Huang, I addressed the comments. Please merge when available. |
What changes were proposed in this PR?
@RolesAllowedannotations onconfig-service,computing-unit-managing-service, andworkflow-compiling-serviceresources were decorative because none of these services registered Jersey'sRolesAllowedDynamicFeature. This PR registers that feature in each service'srun(...). Forworkflow-compiling-service, which was not registering JWT auth at all, this PR also registersAuthDynamicFeature(JwtAuthFilter)and theSessionUserAuthValueFactoryProvider.Binder, and addsAuthas an sbt dependency for the module.access-control-serviceandfile-serviceuse no@RolesAllowedtoday 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(mirrorsAccessControlServiceRunSpec) that mocks the Jersey environment and verifiesRolesAllowedDynamicFeatureis registered whenConfigService.runruns. The same one-line registration applies to the other two services; tests there would require either refactoringSqlServer.initConnectionout ofrunor static-mocking the ScalaSqlServerobject, 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 toWorkflowCompilationResourceshould 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