Fix sonar issues

iam-login-service/src/main/java/it/infn/mw/iam/api/account/AccountUtils.java

@@ -71,8 +71,7 @@ public Optional<IamAccount> getAuthenticatedUserAccount(Authentication authn) {
 
     Authentication userAuthn = authn;
 
-    if (authn instanceof OAuth2Authentication) {
-      OAuth2Authentication oauth = (OAuth2Authentication) authn;
+    if (authn instanceof OAuth2Authentication oauth) {
       if (oauth.getUserAuthentication() == null) {
         return Optional.empty();
       }

iam-login-service/src/main/java/it/infn/mw/iam/api/account/attributes/AccountAttributesController.java

@@ -18,21 +18,19 @@
 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 java.util.List;
 
 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;
 import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.bind.annotation.RestController;
 
@@ -96,7 +94,7 @@ public void setAttribute(@PathVariable String id, @RequestBody @Validated Attrib
     accountService.setAttribute(account, attr);
   }
 
-  @RequestMapping(value = "/iam/account/{id}/attributes", method = DELETE)
+  @DeleteMapping(value = "/iam/account/{id}/attributes")
   @PreAuthorize("#iam.hasScope('iam:admin.write') or #iam.hasDashboardRole('ROLE_ADMIN')")
   @ResponseStatus(value = NO_CONTENT)
   public void deleteAttribute(@PathVariable String id, @Validated AttributeDTO attribute,
@@ -112,14 +110,12 @@ public void deleteAttribute(@PathVariable String id, @Validated AttributeDTO att
 
   @ResponseStatus(code = HttpStatus.BAD_REQUEST)
   @ExceptionHandler(InvalidAttributeError.class)
-  @ResponseBody
   public ErrorDTO handleValidationError(InvalidAttributeError e) {
     return ErrorDTO.fromString(e.getMessage());
   }
 
   @ResponseStatus(code = HttpStatus.NOT_FOUND)
   @ExceptionHandler(NoSuchAccountError.class)
-  @ResponseBody
   public ErrorDTO handleNoSuchAccountError(NoSuchAccountError e) {
     return ErrorDTO.fromString(e.getMessage());
   }

iam-login-service/src/main/java/it/infn/mw/iam/api/account/group_manager/AccountGroupManagerController.java

@@ -16,7 +16,6 @@
 package it.infn.mw.iam.api.account.group_manager;
 
 import java.util.List;
-import java.util.stream.Collectors;
 
 import javax.servlet.http.HttpServletRequest;
 
@@ -108,7 +107,7 @@ public List<ScimUser> getGroupManagersForGroup(@PathVariable String groupId) {
     return service.getGroupManagersForGroup(group)
       .stream()
       .map(userConverter::dtoFromEntity)
-      .collect(Collectors.toList());
+      .toList();
 
   }
 

iam-login-service/src/main/java/it/infn/mw/iam/api/group/GroupLabelsController.java

@@ -32,7 +32,6 @@
 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;
 import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.bind.annotation.RestController;
 
@@ -101,14 +100,12 @@ public void deleteLabel(@PathVariable String id, @Validated LabelDTO label,
 
   @ResponseStatus(code = HttpStatus.BAD_REQUEST)
   @ExceptionHandler(InvalidLabelError.class)
-  @ResponseBody
   public ErrorDTO handleValidationError(InvalidLabelError e) {
     return ErrorDTO.fromString(e.getMessage());
   }
 
   @ResponseStatus(code = HttpStatus.NOT_FOUND)
   @ExceptionHandler(NoSuchGroupError.class)
-  @ResponseBody
   public ErrorDTO handleNotFoundError(NoSuchGroupError e) {
     return ErrorDTO.fromString(e.getMessage());
   }

iam-login-service/src/main/java/it/infn/mw/iam/core/expression/IamSecurityExpressionMethods.java

@@ -67,10 +67,9 @@ public enum Role {
   }
 
   public boolean isGroupManager(String groupUuid) {
-    boolean groupManager = authentication.getAuthorities()
+    return authentication.getAuthorities()
       .stream()
       .anyMatch(a -> a.getAuthority().equals(ROLE_GM + groupUuid));
-    return groupManager;
   }
 
   public boolean isUser(String userUuid) {
@@ -104,8 +103,7 @@ public boolean userCanDeleteGroupRequest(String requestId) {
 
   public boolean hasScope(String scope) {
 
-    if (authentication instanceof OAuth2Authentication) {
-      OAuth2Authentication oauth = (OAuth2Authentication) authentication;
+    if (authentication instanceof OAuth2Authentication oauth) {
       return scopeResolver.resolveScope(oauth).stream().anyMatch(s -> s.equals(scope));
     }
     return false;

iam-login-service/src/main/java/it/infn/mw/iam/core/lifecycle/cern/CernHrLifecycleHandler.java

@@ -75,10 +75,6 @@ public class CernHrLifecycleHandler implements Runnable, SchedulingConfigurer {
 
   public static final Logger LOG = LoggerFactory.getLogger(CernHrLifecycleHandler.class);
 
-  public enum CernStatus {
-    IGNORED, ERROR, EXPIRED, VO_MEMBER
-  }
-
   private final CernProperties cernProperties;
   private final IamAccountRepository accountRepo;
   private final IamAccountService accountService;

iam-login-service/src/main/java/it/infn/mw/iam/core/lifecycle/cern/CernHrLifecycleUtils.java

@@ -30,7 +30,6 @@
 import it.infn.mw.iam.api.registration.cern.dto.ParticipationDTO;
 import it.infn.mw.iam.core.lifecycle.ExpiredAccountsHandler;
 import it.infn.mw.iam.core.lifecycle.ExpiredAccountsHandler.AccountLifecycleStatus;
-import it.infn.mw.iam.core.lifecycle.cern.CernHrLifecycleHandler.CernStatus;
 import it.infn.mw.iam.persistence.model.IamAccount;
 import it.infn.mw.iam.persistence.model.IamLabel;
 

iam-login-service/src/main/java/it/infn/mw/iam/core/lifecycle/cern/CernStatus.java

@@ -0,0 +1,5 @@
+package it.infn.mw.iam.core.lifecycle.cern;
+
+public enum CernStatus {
+  IGNORED, ERROR, EXPIRED, VO_MEMBER
+}

iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/IamOAuthConfirmationController.java

@@ -46,10 +46,9 @@
 import org.springframework.security.oauth2.common.exceptions.OAuth2Exception;
 import org.springframework.security.oauth2.provider.AuthorizationRequest;
 import org.springframework.security.oauth2.provider.endpoint.RedirectResolver;
-import org.springframework.stereotype.Controller;
+import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.ModelAttribute;
-import org.springframework.web.bind.annotation.RequestMapping;
-import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.bind.annotation.SessionAttributes;
 import org.springframework.web.bind.support.SessionStatus;
 
@@ -64,16 +63,11 @@
 import it.infn.mw.iam.core.oauth.scope.pdp.ScopePolicyPDP;
 import it.infn.mw.iam.persistence.model.IamAccount;
 
-/**
- * @author jricher
- *
- */
 @SuppressWarnings("deprecation")
-@Controller
+@RestController
 @SessionAttributes("authorizationRequest")
 public class IamOAuthConfirmationController {
 
-
   @Autowired
   private ClientDetailsEntityService clientService;
 
@@ -116,7 +110,7 @@ public IamOAuthConfirmationController(ClientDetailsEntityService clientService)
   }
 
   @PreAuthorize("hasRole('ROLE_USER')")
-  @RequestMapping(path = "/oauth/confirm_access", method = RequestMethod.GET)
+  @GetMapping(path = "/oauth/confirm_access")
   public String confimAccess(Map<String, Object> model,
       @ModelAttribute("authorizationRequest") AuthorizationRequest authRequest,
       Authentication authUser, SessionStatus status) {
@@ -148,26 +142,7 @@ public String confimAccess(Map<String, Object> model,
 
     if (prompts.contains("none")) {
       // if we've got a redirect URI then we'll send it
-
-      String url = redirectResolver.resolveRedirect(authRequest.getRedirectUri(), client);
-
-      try {
-        URIBuilder uriBuilder = new URIBuilder(url);
-
-        uriBuilder.addParameter("error", "interaction_required");
-        if (!Strings.isNullOrEmpty(authRequest.getState())) {
-          uriBuilder.addParameter("state", authRequest.getState()); // copy the state parameter if
-                                                                    // one was given
-        }
-
-        status.setComplete();
-        return "redirect:" + uriBuilder.toString();
-
-      } catch (URISyntaxException e) {
-        logger.error("Can't build redirect URI for prompt=none, sending error instead", e);
-        model.put("code", HttpStatus.FORBIDDEN);
-        return HttpCodeView.VIEWNAME;
-      }
+      return handleRedirectOrFail(model, authRequest, status, client);
     }
 
     model.put("auth_request", authRequest);
@@ -181,22 +156,43 @@ public String confimAccess(Map<String, Object> model,
     // pre-process the scopes
     Set<SystemScope> scopes = scopeService.fromStrings(authRequest.getScope());
 
-    Set<SystemScope> sortedScopes = new LinkedHashSet<>(scopes.size());
     Set<SystemScope> systemScopes = scopeService.getAll();
 
-    // filter requested scopes according to the scope policy
-    IamAccount account = accountUtils.getAuthenticatedUserAccount(authUser)
-      .orElseThrow(() -> NoSuchAccountError.forUsername(authUser.getName()));
+    Set<String> filteredScopes = getFilteredScopes(scopes, authUser);
 
-    Set<String> filteredScopes = pdp.filterScopes(scopeService.toStrings(scopes), account);
+    Set<SystemScope> sortedScopes = getSortedScopes(systemScopes, filteredScopes);
 
-    // admin scopes are not allowed to clients approved by regular users
-    if (!accountUtils.isAdmin(authUser)) {
-      filteredScopes =
-          filteredScopes.stream().filter(s -> !adminScopes.contains(s)).collect(Collectors.toSet());
+    model.put("scopes", sortedScopes);
+
+    authRequest.setScope(scopeService.toStrings(sortedScopes));
+
+    model.put("claims", getClaimsForScopes(sortedScopes, authUser.getName()));
+
+    // client stats
+    Integer count = statsService.getCountForClientId(client.getClientId()).getApprovedSiteCount();
+    model.put("count", count);
+
+    // contacts
+    if (!client.getContacts().isEmpty()) {
+      String contacts = Joiner.on(", ").join(client.getContacts());
+      model.put("contacts", contacts);
     }
 
-    // sort scopes for display based on the inherent order of system scopes
+    // if the client is over a week old and has more than one registration, don't give such a big
+    // warning instead, tag as "Generally Recognized As Safe" (gras)
+    Date lastWeek = new Date(System.currentTimeMillis() - (60 * 60 * 24 * 7 * 1000));
+    Boolean expression =
+        count > 1 && client.getCreatedAt() != null && client.getCreatedAt().before(lastWeek);
+    model.put("gras", expression);
+
+    return "iam/approveClient";
+  }
+
+  private Set<SystemScope> getSortedScopes(Set<SystemScope> systemScopes,
+      Set<String> filteredScopes) {
+
+    Set<SystemScope> sortedScopes = new LinkedHashSet<>(systemScopes.size());
+
     for (SystemScope s : systemScopes) {
       if (scopeService.fromStrings(filteredScopes).contains(s)) {
         sortedScopes.add(s);
@@ -206,51 +202,70 @@ public String confimAccess(Map<String, Object> model,
     // add in any scopes that aren't system scopes to the end of the list
     sortedScopes.addAll(Sets.difference(scopeService.fromStrings(filteredScopes), systemScopes));
 
-    model.put("scopes", sortedScopes);
+    return sortedScopes;
+  }
 
-    authRequest.setScope(scopeService.toStrings(sortedScopes));
+  private Set<String> getFilteredScopes(Set<SystemScope> scopes, Authentication authUser)
+      throws NoSuchAccountError {
+
+    IamAccount account = accountUtils.getAuthenticatedUserAccount(authUser)
+      .orElseThrow(() -> NoSuchAccountError.forUsername(authUser.getName()));
+
+    Set<String> filteredScopes = pdp.filterScopes(scopeService.toStrings(scopes), account);
+
+    if (!accountUtils.isAdmin(authUser)) {
+      filteredScopes =
+          filteredScopes.stream().filter(s -> !adminScopes.contains(s)).collect(Collectors.toSet());
+    }
+    return filteredScopes;
+  }
+
+  private Map<String, Map<String, String>> getClaimsForScopes(Set<SystemScope> sortedScopes,
+      String username) {
+
+    UserInfo user = userInfoService.getByUsername(username);
 
-    // get the userinfo claims for each scope
-    UserInfo user = userInfoService.getByUsername(authUser.getName());
     Map<String, Map<String, String>> claimsForScopes = new HashMap<>();
-    if (user != null) {
-      JsonObject userJson = user.toJson();
+    if (user == null) {
+      return claimsForScopes;
+    }
 
-      for (SystemScope systemScope : sortedScopes) {
-        Map<String, String> claimValues = new HashMap<>();
+    JsonObject userJson = user.toJson();
+    for (SystemScope systemScope : sortedScopes) {
+      Map<String, String> claimValues = new HashMap<>();
 
-        Set<String> claims = scopeClaimTranslationService.getClaimsForScope(systemScope.getValue());
-        for (String claim : claims) {
-          if (userJson.has(claim) && userJson.get(claim).isJsonPrimitive()) {
-            claimValues.put(claim, userJson.get(claim).getAsString());
-          }
+      Set<String> claims = scopeClaimTranslationService.getClaimsForScope(systemScope.getValue());
+      for (String claim : claims) {
+        if (userJson.has(claim) && userJson.get(claim).isJsonPrimitive()) {
+          claimValues.put(claim, userJson.get(claim).getAsString());
         }
-
-        claimsForScopes.put(systemScope.getValue(), claimValues);
       }
+      claimsForScopes.put(systemScope.getValue(), claimValues);
     }
+    return claimsForScopes;
+  }
 
-    model.put("claims", claimsForScopes);
+  private String handleRedirectOrFail(Map<String, Object> model, AuthorizationRequest authRequest,
+      SessionStatus status, ClientDetailsEntity client) {
 
-    // client stats
-    Integer count = statsService.getCountForClientId(client.getClientId()).getApprovedSiteCount();
-    model.put("count", count);
+    String url = redirectResolver.resolveRedirect(authRequest.getRedirectUri(), client);
 
+    try {
+      URIBuilder uriBuilder = new URIBuilder(url);
 
-    // contacts
-    if (!client.getContacts().isEmpty()) {
-      String contacts = Joiner.on(", ").join(client.getContacts());
-      model.put("contacts", contacts);
-    }
+      uriBuilder.addParameter("error", "interaction_required");
+      if (!Strings.isNullOrEmpty(authRequest.getState())) {
+        uriBuilder.addParameter("state", authRequest.getState());
+      }
 
-    // if the client is over a week old and has more than one registration, don't give such a big
-    // warning
-    // instead, tag as "Generally Recognized As Safe" (gras)
-    Date lastWeek = new Date(System.currentTimeMillis() - (60 * 60 * 24 * 7 * 1000));
-    Boolean expression = count > 1 && client.getCreatedAt() != null && client.getCreatedAt().before(lastWeek);
-    model.put("gras", expression);
+      status.setComplete();
+      return "redirect:" + uriBuilder.toString();
 
-    return "iam/approveClient";
+    } catch (URISyntaxException e) {
+      logger.error("Can't build redirect URI for prompt=none, sending error instead", e);
+      model.put("code", HttpStatus.FORBIDDEN);
+      return HttpCodeView.VIEWNAME;
+    }
   }
 
   /**

iam-login-service/src/main/java/it/infn/mw/iam/registration/RegistrationApiController.java

@@ -42,7 +42,6 @@
 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RequestParam;
-import org.springframework.web.bind.annotation.ResponseBody;
 import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.servlet.ModelAndView;
@@ -97,7 +96,6 @@ private Optional<ExternalAuthenticationRegistrationInfo> getExternalAuthenticati
 
   @PreAuthorize("#iam.hasScope('registration:read') or hasRole('ADMIN')")
   @GetMapping(value = "/registration/list")
-  @ResponseBody
   public List<RegistrationRequestDto> listRequests(
       @RequestParam(value = "status", required = false) IamRegistrationRequestStatus status) {
 
@@ -106,7 +104,6 @@ public List<RegistrationRequestDto> listRequests(
 
   @PreAuthorize("#iam.hasScope('registration:read') or hasRole('ADMIN')")
   @GetMapping(value = "/registration/list/pending")
-  @ResponseBody
   public List<RegistrationRequestDto> listPendingRequests() {
 
     return service.listPendingRequests();