Merge pull request #31 from indigo-iam/fix/INDIAM-131

INDIAM-131: Email forgot endpoint is confused by some emails

iam-login-service/pom.xml

@@ -32,7 +32,7 @@
         </exclusion>
       </exclusions>
     </dependency>
-    
+
     <!-- Web Jars -->
     <dependency>
       <groupId>org.webjars</groupId>
@@ -274,9 +274,7 @@
         </exclusion>
       </exclusions>
     </dependency>
-
   </dependencies>
-
   <build>
     <finalName>iam-login-service</finalName>
     <resources>
@@ -295,6 +293,9 @@
       <plugin>
         <groupId>org.springframework.boot</groupId>
         <artifactId>spring-boot-maven-plugin</artifactId>
+        <configuration>
+          <mainClass>it.infn.mw.iam.IamLoginService</mainClass>
+        </configuration>
         <executions>
           <execution>
             <goals>

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

@@ -1,5 +1,6 @@
 package it.infn.mw.iam;
 
+import org.mitre.openid.connect.web.RootController;
 import org.mitre.openid.connect.web.UserInfoEndpoint;
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
@@ -36,7 +37,9 @@
     "org.mitre.discovery.view"},
 excludeFilters = {
     @ComponentScan.Filter(type=FilterType.ASSIGNABLE_TYPE,
-        value=UserInfoEndpoint.class)
+        value=UserInfoEndpoint.class),
+    @ComponentScan.Filter(type=FilterType.ASSIGNABLE_TYPE,
+    value=RootController.class),
 })
 // @formatter:on
 

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

@@ -8,7 +8,6 @@
 import org.springframework.security.crypto.password.PasswordEncoder;
 import org.springframework.stereotype.Service;
 
-import it.infn.mw.iam.api.scim.exception.ScimResourceNotFoundException;
 import it.infn.mw.iam.notification.NotificationService;
 import it.infn.mw.iam.persistence.model.IamAccount;
 import it.infn.mw.iam.persistence.repository.IamAccountRepository;
@@ -32,33 +31,38 @@ public class DefaultPasswordResetService implements PasswordResetService {
   private PasswordEncoder passwordEncoder;
 
   @Override
-  public Boolean checkResetKey(String resetKey) {
+  public void validateResetToken(String resetToken) {
 
-    IamAccount account = accountRepository.findByResetKey(resetKey)
-      .orElseThrow(() -> new ScimResourceNotFoundException(
-          String.format("No account found for reset_key [%s]", resetKey)));
+    IamAccount account = accountRepository.findByResetKey(resetToken)
+      .orElseThrow(() -> new InvalidPasswordResetTokenError(
+          String.format("No account found for reset_key [%s]", resetToken)));
 
-    return isAccountEnabled(account);
+    if (!accountActiveAndEmailVerified(account)) {
+      throw new InvalidPasswordResetTokenError(
+          "The user account is not active or is linked to an email that has not been verified");
+    }
   }
 
   @Override
-  public void changePassword(String resetKey, String password) {
+  public void resetPassword(String resetToken, String password) {
 
-    if (checkResetKey(resetKey)) {
-      IamAccount account = accountRepository.findByResetKey(resetKey).get();
-      account.setPassword(passwordEncoder.encode(password));
-      account.setResetKey(null);
+    validateResetToken(resetToken);
 
-      accountRepository.save(account);
-    }
+    IamAccount account = accountRepository.findByResetKey(resetToken).get();
+
+    account.setPassword(passwordEncoder.encode(password));
+    account.setResetKey(null);
+
+    accountRepository.save(account);
   }
 
+
   @Override
-  public void forgotPassword(String email) {
+  public void createPasswordResetToken(String email) {
     try {
       IamAccount account = accountRepository.findByEmail(email).get();
 
-      if (isAccountEnabled(account)) {
+      if (accountActiveAndEmailVerified(account)) {
         String resetKey = tokenGenerator.generateToken();
         account.setResetKey(resetKey);
         accountRepository.save(account);
@@ -71,7 +75,7 @@ public void forgotPassword(String email) {
   }
 
 
-  private boolean isAccountEnabled(IamAccount account) {
+  private boolean accountActiveAndEmailVerified(IamAccount account) {
     return account.isActive() && (account.getUserInfo().getEmailVerified() != null
         && account.getUserInfo().getEmailVerified());
   }

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

@@ -0,0 +1,23 @@
+package it.infn.mw.iam.api.account;
+
+import javax.validation.constraints.NotNull;
+
+import org.hibernate.validator.constraints.Email;
+
+public class EmailDTO {
+
+  @Email(message = "please specify a valid email address")
+  @NotNull(message = "please specify an email address")
+  private String email;
+
+  public EmailDTO() {}
+
+  public String getEmail() {
+    return email;
+  }
+
+  public void setEmail(String email) {
+    this.email = email;
+  }
+
+}

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

@@ -0,0 +1,15 @@
+package it.infn.mw.iam.api.account;
+
+public class InvalidEmailAddressError extends RuntimeException {
+
+
+  /**
+   * 
+   */
+  private static final long serialVersionUID = 1L;
+
+  public InvalidEmailAddressError(String message) {
+    super(message);
+  }
+
+}

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

@@ -0,0 +1,14 @@
+package it.infn.mw.iam.api.account;
+
+public class InvalidPasswordResetTokenError extends RuntimeException {
+
+  /**
+   * 
+   */
+  private static final long serialVersionUID = 1L;
+
+  public InvalidPasswordResetTokenError(String message) {
+    super(message);
+  }
+
+}

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

@@ -1,34 +1,63 @@
 package it.infn.mw.iam.api.account;
 
+import javax.servlet.http.HttpServletRequest;
+import javax.validation.Valid;
+
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
 import org.springframework.stereotype.Controller;
 import org.springframework.ui.Model;
+import org.springframework.validation.BindingResult;
+import org.springframework.web.bind.annotation.ExceptionHandler;
 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.RequestParam;
 import org.springframework.web.bind.annotation.ResponseBody;
-
-import it.infn.mw.iam.api.scim.exception.ScimResourceNotFoundException;
+import org.springframework.web.bind.annotation.ResponseStatus;
 
 @Controller
+@RequestMapping(PasswordResetController.BASE_RESOURCE)
 public class PasswordResetController {
 
+  public static final String BASE_RESOURCE = "/iam/password-reset";
+  public static final String BASE_TOKEN_URL = BASE_RESOURCE + "/token";
+
   @Autowired
   private PasswordResetService service;
 
-  private static final String PLAIN_TEXT = "plain/text";
+  @RequestMapping(value = "/token", method = RequestMethod.POST,
+      produces = MediaType.TEXT_PLAIN_VALUE)
+  @ResponseBody
+  public void createPasswordResetToken(@Valid EmailDTO emailDTO, BindingResult validationResult) {
+
+    if (validationResult.hasErrors()) {
+      throw new InvalidEmailAddressError(
+          "validation error: " + validationResult.getFieldError("email").getDefaultMessage());
+    }
 
+    service.createPasswordResetToken(emailDTO.getEmail());
+    return;
+  }
+
+  @RequestMapping(value = "/token/{token}", method = RequestMethod.HEAD)
+  @ResponseBody
+  public String validateResetToken(@PathVariable("token") String token) {
+    service.validateResetToken(token);
+    return "ok";
+  }
 
-  @RequestMapping(value = "/iam/password-reset/{token}", method = RequestMethod.GET)
-  public String resetPassword(Model model, @PathVariable("token") String token) {
+  @RequestMapping(value = "/token/{token}", method = RequestMethod.GET)
+  public String resetPasswordPage(Model model, @PathVariable("token") String token) {
     String message = null;
+
     try {
-      if (!service.checkResetKey(token)) {
-        message = "This account is not active or email is not verified. Cannot reset password!";
-      }
-    } catch (ScimResourceNotFoundException e) {
-      message = "Invalid reset key: " + e.getMessage();
+
+      service.validateResetToken(token);
+
+    } catch (InvalidPasswordResetTokenError e) {
+      message = e.getMessage();
     }
 
     model.addAttribute("errorMessage", message);
@@ -37,35 +66,26 @@ public String resetPassword(Model model, @PathVariable("token") String token) {
     return "iam/resetPassword";
   }
 
-  @RequestMapping(value = "/iam/password/reset-key/{token}", method = RequestMethod.GET)
+  @RequestMapping(value = {"", "/"}, method = RequestMethod.POST)
   @ResponseBody
-  public Boolean checkResetKey(Model model, @PathVariable("token") String token) {
-    return service.checkResetKey(token);
-  }
+  public void resetPassword(@RequestParam(required = true, name = "token") String token,
+      @RequestParam(required = true, name = "password") String password) {
 
+    service.resetPassword(token, password);
+  }
 
-  @RequestMapping(value = "/iam/password-change", method = RequestMethod.POST,
-      produces = PLAIN_TEXT)
+  @ResponseStatus(value = HttpStatus.BAD_REQUEST)
+  @ExceptionHandler(InvalidEmailAddressError.class)
   @ResponseBody
-  public String changePassword(@RequestParam(required = true, name = "resetkey") String resetKey,
-      @RequestParam(required = true, name = "password") String password) {
-    String retval = null;
-    try {
-      service.changePassword(resetKey, password);
-      retval = "ok";
-    } catch (Exception e) {
-      retval = "err";
-    }
-    return retval;
+  public String emailValidationError(HttpServletRequest req, Exception ex) {
+    return ex.getMessage();
   }
 
-
-  @RequestMapping(value = "/iam/password-forgot/{email:.+}", method = RequestMethod.GET,
-      produces = PLAIN_TEXT)
+  @ResponseStatus(value = HttpStatus.NOT_FOUND)
+  @ExceptionHandler(InvalidPasswordResetTokenError.class)
   @ResponseBody
-  public String forgotPassword(@PathVariable("email") String email) {
-    service.forgotPassword(email);
-    return "ok";
+  public String invalidPasswordRequestTokenError(HttpServletRequest req, Exception ex) {
+    return ex.getMessage();
   }
 
 }

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

@@ -1,11 +1,38 @@
 package it.infn.mw.iam.api.account;
 
+/**
+ * 
+ * The IAM password reset service
+ *
+ */
 public interface PasswordResetService {
 
-  public Boolean checkResetKey(String resetKey);
 
-  public void changePassword(String resetKey, String password);
+  /**
+   * Validates a password reset token.
+   * 
+   * @param resetToken the password reset token to be validated
+   * 
+   * @throws InvalidPasswordResetTokenError if the password reset token is not valid
+   */
+  public void validateResetToken(String resetToken);
 
-  public void forgotPassword(String email);
+  /**
+   * Resets the password for an account, given a valid password reset token
+   * 
+   * @param resetToken the password reset token
+   * 
+   * @param password the password to be set
+   * 
+   * @throws InvalidPasswordResetTokenError if the password reset token is not valid
+   */
+  public void resetPassword(String resetToken, String password);
+
+  /**
+   * Creates a password reset token for the account linked with the email passed as argument.
+   * 
+   * @param email the email linked to the account for which the password must be reset
+   */
+  public void createPasswordResetToken(String email);
 
 }

iam-login-service/src/main/java/it/infn/mw/iam/config/TaskConfig.java

@@ -51,7 +51,7 @@ public void clearExpiredSites() {
 
   @Scheduled(fixedDelayString = "${notification.taskDelay}", initialDelay = 60000)
   public void sendNotifications() {
-    notificationService.sendPendingNotification();
+    notificationService.sendPendingNotifications();
   }
 
   @Scheduled(fixedDelay = 30000, initialDelay = 60000)

iam-login-service/src/main/java/it/infn/mw/iam/core/web/IamRootController.java

@@ -0,0 +1,24 @@
+package it.infn.mw.iam.core.web;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.springframework.security.access.prepost.PreAuthorize;
+import org.springframework.stereotype.Controller;
+import org.springframework.ui.ModelMap;
+import org.springframework.web.bind.annotation.RequestMapping;
+
+@Controller
+public class IamRootController {
+
+  @RequestMapping({"", "home", "index"})
+  public String home(HttpServletRequest request) {
+    return "home";
+  }
+
+  @PreAuthorize("hasRole('ROLE_USER')")
+  @RequestMapping("manage/**")
+  public String manage(ModelMap m) {
+    return "manage";
+  }
+
+}

iam-login-service/src/main/java/it/infn/mw/iam/dashboard/DashboardController.java

@@ -16,22 +16,23 @@
 @RequestMapping(value = "/dashboard")
 @Transactional
 public class DashboardController {
-  
+
   @Autowired
   LoginPageConfiguration loginPageConfiguration;
 
-  @PreAuthorize("hasRole('USER')")
+  @PreAuthorize("hasRole('ADMIN')")
   @RequestMapping(method = RequestMethod.GET)
   public ModelAndView showDashboard(HttpServletRequest request) {
-    
+
     ModelAndView dashboard = new ModelAndView("iam/dashboard");
-    dashboard.getModelMap().addAttribute("isRegistrationEnabled", loginPageConfiguration.isRegistrationEnabled());
+    dashboard.getModelMap().addAttribute("isRegistrationEnabled",
+        loginPageConfiguration.isRegistrationEnabled());
     return dashboard;
   }
 
   @RequestMapping(method = RequestMethod.GET, value = "/expiredsession")
   public ModelAndView showExpiredSession(HttpServletRequest request) {
-    
+
     return new ModelAndView("iam/expiredsession");
 
   }