Add validation to Attribute DTO and fix Sonar issues

Author: enricovianello

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

@@ -18,23 +18,21 @@
 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 java.util.function.Supplier;
 
-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;
 import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.bind.annotation.RestController;
 
@@ -58,7 +56,6 @@ public class AccountLabelsController {
   final IamAccountService service;
   final LabelDTOConverter converter;
 
-  @Autowired
   public AccountLabelsController(IamAccountService service, LabelDTOConverter converter) {
     this.service = service;
     this.converter = converter;
@@ -74,7 +71,7 @@ private void handleValidationError(BindingResult result) {
     }
   }
 
-  @RequestMapping(method = GET)
+  @GetMapping
   @PreAuthorize("#iam.hasScope('iam:admin.read') or #iam.hasAnyDashboardRole('ROLE_ADMIN', 'ROLE_GM') or #iam.isUser(#id)")
   public List<LabelDTO> getLabels(@PathVariable String id) {
 
@@ -87,7 +84,7 @@ public List<LabelDTO> getLabels(@PathVariable String id) {
     return results;
   }
 
-  @RequestMapping(method = PUT)
+  @PutMapping
   @PreAuthorize("#iam.hasScope('iam:admin.write') or #iam.hasDashboardRole('ROLE_ADMIN')")
   public void setLabel(@PathVariable String id, @RequestBody @Validated LabelDTO label,
       BindingResult validationResult) {
@@ -97,7 +94,7 @@ public void setLabel(@PathVariable String id, @RequestBody @Validated LabelDTO l
     service.addLabel(account, converter.entityFromDto(label));
   }
 
-  @RequestMapping(method = DELETE)
+  @DeleteMapping
   @PreAuthorize("#iam.hasScope('iam:admin.write') or #iam.hasDashboardRole('ROLE_ADMIN')")
   @ResponseStatus(NO_CONTENT)
   public void deleteLabel(@PathVariable String id, @Validated LabelDTO label,
@@ -109,14 +106,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(NoSuchAccountError.class)
-  @ResponseBody
   public ErrorDTO handleNotFoundError(NoSuchAccountError e) {
     return ErrorDTO.fromString(e.getMessage());
   }

iam-login-service/src/main/java/it/infn/mw/iam/api/common/AttributeDTO.java

@@ -16,9 +16,12 @@
 package it.infn.mw.iam.api.common;
 
 import javax.annotation.Generated;
+import javax.validation.constraints.NotBlank;
 import javax.validation.constraints.Pattern;
 import javax.validation.constraints.Size;
 
+import it.infn.mw.iam.api.common.validator.NoNewLineOrCarriageReturn;
+
 
 
 public class AttributeDTO {
@@ -28,9 +31,11 @@ public class AttributeDTO {
   @Size(max = 64, message = "name cannot be longer than 64 chars")
   @Pattern(regexp = NAME_REGEXP,
       message = "invalid name (does not match with regexp: '" + NAME_REGEXP + "')")
+  @NotBlank
   private String name;
 
   @Size(max = 256, message = "value cannot be longer than 256 chars")
+  @NoNewLineOrCarriageReturn
   private String value;
 
   public AttributeDTO() {

iam-login-service/src/main/java/it/infn/mw/iam/api/common/validator/NoNewLineOrCarriageReturnValidator.java

@@ -1,3 +1,18 @@
+/**
+ * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
 package it.infn.mw.iam.api.common.validator;
 
 import javax.validation.ConstraintValidator;

iam-login-service/src/test/java/it/infn/mw/iam/test/api/account/attributes/AccountAttributesTests.java

@@ -17,6 +17,7 @@
 
 import static java.lang.String.format;
 import static java.util.Arrays.asList;
+import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.hasItem;
@@ -67,6 +68,7 @@ public class AccountAttributesTests {
   public static final ResultMatcher UNAUTHORIZED = status().isUnauthorized();
   public static final ResultMatcher FORBIDDEN = status().isForbidden();
   public static final ResultMatcher NOT_FOUND = status().isNotFound();
+  public static final ResultMatcher BAD_REQUEST = status().isBadRequest();
 
   public static final String TEST_USER = "test";
   public static final String TEST_100_USER = "test_100";
@@ -145,7 +147,7 @@ public void aUserCanListHisAttributes() throws Exception {
 
     mvc.perform(get(ACCOUNT_ATTR_URL_TEMPLATE, testAccount.getUuid())).andExpect(OK);
   }
-  
+
   @Test
   @WithMockUser(username = "test", roles = "USER")
   public void managingAttributesRequiresPrivilegedUser() throws Exception {
@@ -390,4 +392,58 @@ public void multiAttributeSetTest() throws Exception {
       attrs.forEach(a -> assertThat(results, hasItem(a)));
     }
   }
+
+  @Test
+  @WithMockUser(username = "admin", roles = "ADMIN")
+  public void attributeValidationTests() throws Exception {
+
+    AttributeDTO noNameAttribute = AttributeDTO.newInstance(null, ATTR_VALUE);
+
+    mvc
+      .perform(put(ACCOUNT_ATTR_URL_TEMPLATE, noNameAttribute).contentType(APPLICATION_JSON)
+        .content(mapper.writeValueAsString(noNameAttribute)))
+      .andExpect(BAD_REQUEST)
+      .andExpect(jsonPath("$.error", containsString("must not be blank")));
+
+    final String SOME_INVALID_NAMES[] =
+        {"-pippo", "/ciccio/paglia", ".starts-with-dot", "carriage\nreturn", "another\rreturn"};
+
+    for (String name : SOME_INVALID_NAMES) {
+      AttributeDTO invalidAttribute = AttributeDTO.newInstance(name, ATTR_VALUE);
+      mvc
+        .perform(put(ACCOUNT_ATTR_URL_TEMPLATE, invalidAttribute).contentType(APPLICATION_JSON)
+          .content(mapper.writeValueAsString(invalidAttribute)))
+        .andExpect(BAD_REQUEST)
+        .andExpect(jsonPath("$.error", containsString("invalid name (does not match with regexp")));
+    }
+
+    final String SOME_INVALID_VALES[] = {"carriage\nreturn", "another\rreturn"};
+
+    for (String value : SOME_INVALID_VALES) {
+      AttributeDTO invalidAttribute = AttributeDTO.newInstance(ATTR_NAME, value);
+      mvc
+        .perform(put(ACCOUNT_ATTR_URL_TEMPLATE, invalidAttribute).contentType(APPLICATION_JSON)
+          .content(mapper.writeValueAsString(invalidAttribute)))
+        .andExpect(BAD_REQUEST)
+        .andExpect(jsonPath("$.error",
+            containsString("The string must not contain any new line or carriage return")));
+    }
+
+    AttributeDTO longNameAttribute = AttributeDTO.newInstance(randomAlphabetic(65), ATTR_VALUE);
+
+    mvc
+      .perform(put(ACCOUNT_ATTR_URL_TEMPLATE, longNameAttribute).contentType(APPLICATION_JSON)
+        .content(mapper.writeValueAsString(longNameAttribute)))
+      .andExpect(BAD_REQUEST)
+      .andExpect(jsonPath("$.error", containsString("name cannot be longer than 64 chars")));
+
+
+    AttributeDTO longValueAttribute = AttributeDTO.newInstance(ATTR_NAME, randomAlphabetic(257));
+
+    mvc
+      .perform(put(ACCOUNT_ATTR_URL_TEMPLATE, longValueAttribute).contentType(APPLICATION_JSON)
+        .content(mapper.writeValueAsString(longValueAttribute)))
+      .andExpect(BAD_REQUEST)
+      .andExpect(jsonPath("$.error", containsString("value cannot be longer than 256 chars")));
+  }
 }