Fix AuthZ on endpoints that requires identity

indigo-iam · Dec 23, 2024 · 8b01a8f · 8b01a8f
1 parent 284fbeb
commit 8b01a8f
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 29 deletions.
diff --git a/...vice/src/main/java/it/infn/mw/iam/api/account/attributes/AccountAttributesController.java b/...vice/src/main/java/it/infn/mw/iam/api/account/attributes/AccountAttributesController.java
@@ -19,20 +19,19 @@
 import static java.lang.String.format;
 import static org.springframework.http.HttpStatus.NO_CONTENT;
 import static org.springframework.web.bind.annotation.RequestMethod.DELETE;
-import static org.springframework.web.bind.annotation.RequestMethod.PUT;
 
 import java.util.List;
 
-import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.HttpStatus;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.validation.BindingResult;
 import org.springframework.validation.annotation.Validated;
 import org.springframework.web.bind.annotation.ExceptionHandler;
+import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.PutMapping;
 import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RequestMapping;
-import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.bind.annotation.ResponseBody;
 import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.bind.annotation.RestController;
@@ -57,7 +56,6 @@ public class AccountAttributesController {
   final IamAccountService accountService;
   final AttributeDTOConverter converter;
 
-  @Autowired
   public AccountAttributesController(IamAccountService accountService,
       AttributeDTOConverter converter) {
     this.converter = converter;
@@ -71,7 +69,7 @@ private void handleValidationError(BindingResult result) {
     }
   }
 
-  @RequestMapping(value = "/iam/account/{id}/attributes", method = RequestMethod.GET)
+  @GetMapping(value = "/iam/account/{id}/attributes")
   @PreAuthorize("#iam.hasScope('iam:admin.read') or #iam.isUser(#id) or #iam.hasAnyDashboardRole('ROLE_ADMIN', 'ROLE_GM')")
   public List<AttributeDTO> getAttributes(@PathVariable String id) {
 
@@ -84,7 +82,7 @@ public List<AttributeDTO> getAttributes(@PathVariable String id) {
     return results;
   }
 
-  @RequestMapping(value = "/iam/account/{id}/attributes", method = PUT)
+  @PutMapping(value = "/iam/account/{id}/attributes")
   @PreAuthorize("#iam.hasScope('iam:admin.write') or #iam.hasDashboardRole('ROLE_ADMIN')")
   public void setAttribute(@PathVariable String id, @RequestBody @Validated AttributeDTO attribute,
       final BindingResult validationResult) {

diff --git a/...src/main/java/it/infn/mw/iam/api/account/group_manager/AccountGroupManagerController.java b/...src/main/java/it/infn/mw/iam/api/account/group_manager/AccountGroupManagerController.java
@@ -20,13 +20,13 @@
 
 import javax.servlet.http.HttpServletRequest;
 
-import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.HttpStatus;
 import org.springframework.security.access.prepost.PreAuthorize;
+import org.springframework.web.bind.annotation.DeleteMapping;
 import org.springframework.web.bind.annotation.ExceptionHandler;
+import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PathVariable;
-import org.springframework.web.bind.annotation.RequestMapping;
-import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.bind.annotation.RestController;
 
@@ -50,7 +50,6 @@ public class AccountGroupManagerController {
   final IamGroupRepository groupRepository;
   final UserConverter userConverter;
 
-  @Autowired
   public AccountGroupManagerController(AccountGroupManagerService service,
       IamAccountRepository accountRepo, IamGroupRepository groupRepository,
       UserConverter userConverter) {
@@ -60,9 +59,7 @@ public AccountGroupManagerController(AccountGroupManagerService service,
     this.userConverter = userConverter;
   }
 
-
-
-  @RequestMapping(value = "/iam/account/{accountId}/managed-groups", method = RequestMethod.GET)
+  @GetMapping(value = "/iam/account/{accountId}/managed-groups")
   @PreAuthorize("#iam.hasScope('iam:admin.read') or #iam.hasDashboardRole('ROLE_ADMIN') or #iam.isUser(#accountId)")
   public AccountManagedGroupsDTO getAccountManagedGroupsInformation(
       @PathVariable String accountId) {
@@ -72,8 +69,7 @@ public AccountManagedGroupsDTO getAccountManagedGroupsInformation(
     return service.getManagedGroupInfoForAccount(account);
   }
 
-  @RequestMapping(value = "/iam/account/{accountId}/managed-groups/{groupId}",
-      method = RequestMethod.POST)
+  @PostMapping(value = "/iam/account/{accountId}/managed-groups/{groupId}")
   @PreAuthorize("#iam.hasScope('iam:admin.write') or #iam.hasDashboardRole('ROLE_ADMIN')")
   @ResponseStatus(value = HttpStatus.CREATED)
   public void addManagedGroupToAccount(@PathVariable String accountId,
@@ -88,8 +84,7 @@ public void addManagedGroupToAccount(@PathVariable String accountId,
     service.addManagedGroupForAccount(account, group);
   }
 
-  @RequestMapping(value = "/iam/account/{accountId}/managed-groups/{groupId}",
-      method = RequestMethod.DELETE)
+  @DeleteMapping(value = "/iam/account/{accountId}/managed-groups/{groupId}")
   @PreAuthorize("#iam.hasScope('iam:admin.write') or #iam.hasDashboardRole('ROLE_ADMIN')")
   @ResponseStatus(value = HttpStatus.NO_CONTENT)
   public void removeManagedGroupFromAccount(@PathVariable String accountId,
@@ -104,7 +99,7 @@ public void removeManagedGroupFromAccount(@PathVariable String accountId,
     service.removeManagedGroupForAccount(account, group);
   }
 
-  @RequestMapping(value = "/iam/group/{groupId}/group-managers", method=RequestMethod.GET)
+  @GetMapping(value = "/iam/group/{groupId}/group-managers")
   @PreAuthorize("#iam.hasScope('iam:admin.read') or #iam.hasDashboardRole('ROLE_ADMIN') or #iam.isGroupManager(#groupId)")
   public List<ScimUser> getGroupManagersForGroup(@PathVariable String groupId) {
     IamGroup group = groupRepository.findByUuid(groupId)

diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/group/GroupLabelsController.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/group/GroupLabelsController.java
@@ -18,19 +18,18 @@
 import static it.infn.mw.iam.api.utils.ValidationErrorUtils.stringifyValidationError;
 import static java.lang.String.format;
 import static org.springframework.http.HttpStatus.NO_CONTENT;
-import static org.springframework.web.bind.annotation.RequestMethod.DELETE;
-import static org.springframework.web.bind.annotation.RequestMethod.GET;
-import static org.springframework.web.bind.annotation.RequestMethod.PUT;
 
 import java.util.List;
 
-import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.HttpStatus;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.validation.BindingResult;
 import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
 import org.springframework.web.bind.annotation.ExceptionHandler;
+import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.PutMapping;
 import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.ResponseBody;
@@ -58,7 +57,6 @@ public class GroupLabelsController {
   final IamGroupService service;
   final LabelDTOConverter converter;
 
-  @Autowired
   public GroupLabelsController(IamGroupService service, LabelDTOConverter converter) {
     this.service = service;
     this.converter = converter;
@@ -70,7 +68,7 @@ private void handleValidationError(BindingResult result) {
     }
   }
 
-  @RequestMapping(method = GET)
+  @GetMapping
   @PreAuthorize("hasRole('ADMIN') or #iam.isGroupManager(#id)")
   public List<LabelDTO> getLabels(@PathVariable String id) {
 
@@ -83,7 +81,7 @@ public List<LabelDTO> getLabels(@PathVariable String id) {
     return results;
   }
 
-  @RequestMapping(method = PUT)
+  @PutMapping
   public void setLabel(@PathVariable String id, @RequestBody @Validated LabelDTO label,
       BindingResult validationResult) {
     handleValidationError(validationResult);
@@ -92,7 +90,7 @@ public void setLabel(@PathVariable String id, @RequestBody @Validated LabelDTO l
     service.addLabel(group, converter.entityFromDto(label));
   }
 
-  @RequestMapping(method = DELETE)
+  @DeleteMapping
   @ResponseStatus(NO_CONTENT)
   public void deleteLabel(@PathVariable String id, @Validated LabelDTO label,
       BindingResult validationResult) {

diff --git a/...in-service/src/main/java/it/infn/mw/iam/core/expression/IamSecurityExpressionMethods.java b/...in-service/src/main/java/it/infn/mw/iam/core/expression/IamSecurityExpressionMethods.java
@@ -70,13 +70,12 @@ public boolean isGroupManager(String groupUuid) {
     boolean groupManager = authentication.getAuthorities()
       .stream()
       .anyMatch(a -> a.getAuthority().equals(ROLE_GM + groupUuid));
-    return groupManager && isRequestWithoutToken();
+    return groupManager;
   }
 
   public boolean isUser(String userUuid) {
     Optional<IamAccount> account = accountUtils.getAuthenticatedUserAccount();
-    return account.isPresent() && account.get().getUuid().equals(userUuid)
-        && isRequestWithoutToken();
+    return account.isPresent() && account.get().getUuid().equals(userUuid);
   }
 
   public boolean canManageGroupRequest(String requestId) {