Authorization system - #210
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
There was a problem hiding this comment.
🟡 Changes recommended
Compilation failures and multiple authorization and request-handling defects currently block safe integration.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Introduces Spring Security method authorization, derives user identity from the security context, and automatically propagates identity headers to downstream services.
Changes:
- Adds authentication filtering and centralized authorization helpers.
- Applies permission annotations to controller endpoints.
- Removes explicit user IDs from service APIs and updates tests.
File summaries
| File | Description |
|---|---|
WorkspaceTest.java |
Updates directory-service expectations. |
UserIdentityTest.java |
Updates identity lookup mocks. |
SupervisionTest.java |
Updates deletion signatures. |
SingleLineDiagramTest.java |
Updates authorization expectations. |
NetworkConversionServiceTest.java |
Updates conversion signature. |
DirectoryServiceTest.java |
Updates search signature. |
MonitorTest.java |
Updates directory operation expectations. |
ExploreTest.java |
Adapts integration tests to authentication changes. |
ExploreServiceExceptionTest.java |
Updates rollback tests. |
EndpointSecurityTest.java |
Adds endpoint annotation coverage. |
DynamicMappingTest.java |
Updates permission and directory mocks. |
NetworkConversionControllerTest.java |
Updates controller signature tests. |
RestTemplateConfigTest.java |
Tests identity-header propagation. |
UserAuthentication.java |
Adds Spring Security authentication representation. |
WorkspaceService.java |
Removes explicit user propagation. |
SupervisionService.java |
Removes user IDs from deletion flow. |
StudyService.java |
Relies on centralized header propagation. |
SpreadsheetConfigService.java |
Updates deletion API. |
SpreadsheetConfigCollectionService.java |
Updates deletion API. |
SingleLineDiagramService.java |
Updates deletion API. |
ParametersService.java |
Removes explicit user headers. |
NotificationService.java |
Reads users from the security context. |
NetworkModificationService.java |
Updates deletion API. |
NetworkConversionService.java |
Removes explicit user argument. |
MonitorService.java |
Updates directory-element interface. |
IDirectoryElementsService.java |
Simplifies deletion contract. |
FilterService.java |
Removes explicit user propagation. |
ExploreService.java |
Refactors operations around security context. |
DynamicMappingService.java |
Updates deletion contract. |
DirectoryService.java |
Centralizes permission calls and header handling. |
ContingencyListService.java |
Removes explicit user headers. |
CaseService.java |
Updates deletion contract. |
AuthorizationService.java |
Adds permission-oriented authorization methods. |
ElementAttributes.java |
Derives owners from authentication. |
SupervisionController.java |
Restricts supervision to administrators. |
NetworkConversionController.java |
Adds conversion authorization. |
FilterController.java |
Adds filter read authorization. |
ExploreController.java |
Applies method-level authorization broadly. |
CaseController.java |
Marks unresolved endpoint policies. |
ActionsController.java |
Adds contingency-list read authorization. |
SpringSecurityConfig.java |
Configures the security filter chain. |
SecurityFilter.java |
Builds authentication from request headers. |
RestTemplateConfig.java |
Forwards authenticated user and roles. |
pom.xml |
Adds Spring Security test support. |
Review details
Suppressed comments (2)
src/main/java/org/gridsuite/explore/server/dto/ElementAttributes.java:71
- This constructor has the same compilation error:
this(...)must be its first statement. Inline the owner lookup into the delegated constructor call.
public ElementAttributes(UUID elementUuid, String elementName, String type, long subdirectoriesCount, String description, DirectoryElementStatus status) {
String owner = ((UserAuthentication) SecurityContextHolder.getContext().getAuthentication()).getUserId();
this(elementUuid, elementName, type, owner, subdirectoriesCount, description, null, null, status, null, null);
src/main/java/org/gridsuite/explore/server/config/RestTemplateConfig.java:72
- With
anyRequest().permitAll(), Spring can install anAnonymousAuthenticationTokenfor requests without a user header. This unconditional cast then throws for otherwise-public endpoints making outgoing REST calls. Only forward headers when the authentication is aUserAuthentication.
- Files reviewed: 44/44 changed files
- Comments generated: 9
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @PreAuthorize("@authorizationService.canDelete(#elementsUuids)") // ça ne peut pas contenir de subDirectories, car ils n'apparaissent que dans l'arbre | ||
| public ResponseEntity<Void> deleteElements(@RequestParam("elementsUuids") List<UUID> elementsUuids, |
| public ElementAttributes(UUID elementUuid, String elementName, String type, long subdirectoriesCount, String description) { | ||
| String owner = ((UserAuthentication) SecurityContextHolder.getContext().getAuthentication()).getUserId(); | ||
| this(elementUuid, elementName, type, owner, subdirectoriesCount, description, null, null, null, null, null); | ||
| } |
| if (!hasSecurityAnnotation(method, controller)) { | ||
| unsecuredEndpoints.add( | ||
| controller.getSimpleName() + "#" + method.getName() | ||
| ); | ||
| } | ||
| }); | ||
|
|
||
| assertThat(unsecuredEndpoints).isEmpty(); |
| @GetMapping(value = "/download-file/{exportUuid}") | ||
| @PreAuthorize("@authorizationService.canRead(#exportUuid)") | ||
| public ResponseEntity<Resource> downloadFile(@PathVariable("exportUuid") UUID exportUuid) { |
| @RestController | ||
| @RequestMapping(value = "/" + ExploreApi.API_VERSION + "/supervision") | ||
| @Tag(name = "Explore server - Supervision") | ||
| @PreAuthorize("hasRole('ADMIN')") |
| private HttpEntity<String> getHttpEntityWithUserHeader(String content) { | ||
|
|
||
| // TODO: HttpHeaders | ||
| HttpHeaders headers = new HttpHeaders(); |
| String path = UriComponentsBuilder.fromPath(ELEMENTS_SERVER_ROOT_PATH + "/authorized") | ||
| .queryParam(PARAM_ACCESS_TYPE, permissionType) | ||
| .queryParam(PARAM_IDS, ids) | ||
| .queryParam(PARAM_TARGET_DIRECTORY_UUID, targetDirectoryUuid) | ||
| .queryParam(PARAM_RECURSIVE_CHECK, recursiveCheck) | ||
| .buildAndExpand() | ||
| .toUriString(); | ||
| .queryParam(PARAM_ACCESS_TYPE, permissionType) | ||
| .queryParam(PARAM_IDS, elementUuids) | ||
| .queryParam(PARAM_TARGET_DIRECTORY_UUID, targetDirectoryUuid) |
# Conflicts: # src/main/java/org/gridsuite/explore/server/services/DirectoryService.java # src/main/java/org/gridsuite/explore/server/services/ExploreService.java # src/test/java/org/gridsuite/explore/server/DynamicMappingTest.java # src/test/java/org/gridsuite/explore/server/ExploreServiceExceptionTest.java # src/test/java/org/gridsuite/explore/server/MonitorTest.java # src/test/java/org/gridsuite/explore/server/SingleLineDiagramTest.java # src/test/java/org/gridsuite/explore/server/WorkspaceTest.java




PR Summary