=== modified file 'dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/UserCredentials.java' --- dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/UserCredentials.java 2014-08-15 07:40:20 +0000 +++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/UserCredentials.java 2014-09-24 06:56:36 +0000 @@ -345,33 +345,63 @@ * Tests whether the given input arguments can perform a valid restore of the * user account for these credentials. Returns false if any of the input arguments * are null, or any of the properties on the credentials are null. Returns false - * if the expiry date arguement is after the expiry date of the credentials. + * if the expiry date argument is after the expiry date of the credentials. * Returns false if any of the given token or code arguments are not equal to * the respective properties the the credentials. Returns true otherwise. * * @param token the restore token. * @param code the restore code. * @param date the expiry date. - * @return true or false. + * @return null if success, or error message if fail. */ - public boolean canRestore( String token, String code, Date date ) + public String canRestore( String token, String code, Date date ) { - if ( this.restoreToken == null || this.restoreCode == null || this.restoreExpiry == null ) - { - return false; - } - - if ( token == null || code == null || date == null ) - { - return false; + if ( this.restoreToken == null ) + { + return "account restoreToken is null"; + } + + if ( this.restoreCode == null ) + { + return "account restoreCode is null"; + } + + if ( this.restoreExpiry == null ) + { + return "account restoreExpiry is null"; + } + + if ( token == null ) + { + return "canRestore() token parameter is null"; + } + + if ( code == null ) + { + return "canRestore() code parameter is null"; + } + + if ( date == null ) + { + return "canRestore() date parameter is null"; + } + + if ( !token.equals ( this.restoreToken ) ) + { + return ( "token '" + token + "' does not match restoreToken '" + restoreToken + "'" ); + } + + if ( !code.equals ( this.restoreCode ) ) + { + return ( "code '" + code + "' does not match restoreCode '" + restoreCode + "'" ); } if ( date.after( this.restoreExpiry ) ) { - return false; + return "date " + date.toString() + " is after " + this.restoreExpiry.toString(); } - return token.equals( this.restoreToken ) && code.equals( this.restoreCode ); + return null; // Success. } /** === modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/DefaultSecurityService.java' --- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/DefaultSecurityService.java 2014-09-08 03:44:30 +0000 +++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/DefaultSecurityService.java 2014-09-24 06:56:36 +0000 @@ -145,9 +145,9 @@ return true; } - public boolean validateRestore( UserCredentials credentials, RestoreOptions restoreOptions ) + public boolean sendRestoreMessage( UserCredentials credentials, String rootPath, RestoreOptions restoreOptions ) { - if ( credentials == null || restoreOptions == null ) + if ( credentials == null || rootPath == null ) { return false; } @@ -174,24 +174,7 @@ if ( credentials.hasAnyAuthority( Arrays.asList( UserAuthorityGroup.CRITICAL_AUTHS ) ) ) { - log.info( "Not allowed to " + restoreType.name() + " users with critical authorities" ); - return false; - } - - return true; - } - - public boolean sendRestoreMessage( UserCredentials credentials, String rootPath, RestoreOptions restoreOptions ) - { - if ( credentials == null || rootPath == null || restoreOptions == null ) - { - return false; - } - - RestoreType restoreType = restoreOptions.getRestoreType(); - - if ( validateRestore( credentials, restoreOptions ) == false ) - { + log.info( "Not allowed to " + restoreType.name() + " users with critical authorities" ); return false; } @@ -287,51 +270,74 @@ public boolean canRestoreNow( UserCredentials credentials, String token, String code, RestoreType restoreType ) { - if ( !verifyToken( credentials, token, restoreType ) ) - { + String errorMessage = verifyToken( credentials, token, restoreType ); + + if ( errorMessage == null ) + { + String username = credentials.getUsername(); + + String encodedToken = passwordManager.encodePassword( username, token ); + String encodedCode = passwordManager.encodePassword( username, code ); + + Date date = new Cal().now().time(); + + errorMessage = credentials.canRestore( encodedToken, encodedCode, date ); + } + + String messageInfo = "Restore User " + credentials.getUid() + " " + credentials.getUsername(); + + if ( errorMessage != null ) + { + log.info( messageInfo + " fail because " + errorMessage + "." ); return false; } - String username = credentials.getUsername(); - - String encodedToken = passwordManager.encodePassword( username, token ); - String encodedCode = passwordManager.encodePassword( username, code ); - - Date date = new Cal().now().time(); - - return credentials.canRestore( encodedToken, encodedCode, date ); + log.info( messageInfo + " success." ); + return true; } - public boolean verifyToken( UserCredentials credentials, String token, RestoreType restoreType ) + public String verifyToken( UserCredentials credentials, String token, RestoreType restoreType ) { - if ( credentials == null || token == null || restoreType == null ) - { - return false; + if ( credentials == null ) + { + return "verifyToken() - credentials parameter is null"; + } + + if ( token == null ) + { + return "verifyToken() - token parameter is null"; + } + + if ( restoreType == null ) + { + return "verifyToken() - restoreType parameter is null"; } RestoreOptions restoreOptions = RestoreOptions.getRestoreOptions( token ); if ( restoreOptions == null ) { - log.info( "Can't parse restore options for " + restoreType.name() + " from token " + token + " for user " + credentials ); - return false; + return "can't parse restore options for " + restoreType.name() + " from token " + token; } if ( restoreType != restoreOptions.getRestoreType() ) { - log.info( "Wrong prefix for restore type " + restoreType.name() + " on token " + token + " for user " + credentials ); - return false; + return "wrong prefix for restore type " + restoreType.name() + " on token " + token; } if ( credentials.getRestoreToken() == null ) { - log.info( "Could not verify token for " + restoreType.name() + " as user has no token: " + credentials ); - return false; - } - - token = passwordManager.encodePassword( credentials.getUsername(), token ); - - return credentials.getRestoreToken().equals( token ); + return "could not verify token for " + restoreType.name() + " because user has no token"; + } + + String encodedToken = passwordManager.encodePassword( credentials.getUsername(), token ); + + if ( !credentials.getRestoreToken().equals( encodedToken ) ) + { + return "supplied token " + token + " does not mach account restoreToken"; + } + + return null; // Success. } @Override === modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/SecurityService.java' --- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/SecurityService.java 2014-09-08 03:44:30 +0000 +++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/SecurityService.java 2014-09-24 06:56:36 +0000 @@ -45,14 +45,6 @@ * @return true if the invitation was sent, otherwise false. */ boolean prepareUserForInvite( User user ); - - /** - * Validates whether a restore is allowed. - * - * @param credentials the credentials for the user to send restore message. - * @param restoreOptions restore options, including type of restore. - */ - boolean validateRestore( UserCredentials credentials, RestoreOptions restoreOptions ); /** * Invokes the initRestore method and dispatches email messages with @@ -128,11 +120,11 @@ * * @param credentials the user credentials. * @param token the token. - * @return false if any of the arguments are null or if the user credentials - * identified by the user name does not exist, true if the arguments - * are valid. + * @return error message if any of the arguments are null or if the user + * credentials identified by the user name does not exist, null if + * the arguments are valid. */ - boolean verifyToken( UserCredentials credentials, String token, RestoreType restoreType ); + String verifyToken( UserCredentials credentials, String token, RestoreType restoreType ); /** * Checks whether current user has read access to object. === modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/user/hibernate/HibernateUserCredentialsStore.java' --- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/user/hibernate/HibernateUserCredentialsStore.java 2014-08-15 07:40:20 +0000 +++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/user/hibernate/HibernateUserCredentialsStore.java 2014-09-24 06:56:36 +0000 @@ -92,6 +92,8 @@ User persistedUser = userService.getUser( userCredentials.getUser().getUid() ); if ( persistedUser != null && persistedUser.getUserCredentials() != null + && persistedUser.getUserCredentials().getPassword() != null + && userCredentials.getPassword() != null && !persistedUser.getUserCredentials().getPassword().equals( userCredentials.getPassword() ) ) { userCredentials.setPasswordLastUpdated( new Date() ); === modified file 'dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/security/SecurityServiceTest.java' --- dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/security/SecurityServiceTest.java 2014-03-18 08:10:10 +0000 +++ dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/security/SecurityServiceTest.java 2014-09-24 06:56:36 +0000 @@ -105,13 +105,13 @@ // // verifyToken() // - assertFalse( securityService.verifyToken( otherCredentials, token, RestoreType.RECOVER_PASSWORD ) ); - - assertFalse( securityService.verifyToken( credentials, "wrongToken", RestoreType.RECOVER_PASSWORD ) ); - - assertFalse( securityService.verifyToken( credentials, token, RestoreType.INVITE ) ); - - assertTrue( securityService.verifyToken( credentials, token, RestoreType.RECOVER_PASSWORD ) ); + assertNotNull( securityService.verifyToken( otherCredentials, token, RestoreType.RECOVER_PASSWORD ) ); + + assertNotNull( securityService.verifyToken( credentials, "wrongToken", RestoreType.RECOVER_PASSWORD ) ); + + assertNotNull( securityService.verifyToken( credentials, token, RestoreType.INVITE ) ); + + assertNull( securityService.verifyToken( credentials, token, RestoreType.RECOVER_PASSWORD ) ); // // canRestoreNow() @@ -174,13 +174,13 @@ // // verifyToken() // - assertFalse( securityService.verifyToken( otherCredentials, token, RestoreType.INVITE ) ); - - assertFalse( securityService.verifyToken( credentials, "wrongToken", RestoreType.INVITE ) ); - - assertFalse( securityService.verifyToken( credentials, token, RestoreType.RECOVER_PASSWORD ) ); - - assertTrue( securityService.verifyToken( credentials, token, RestoreType.INVITE ) ); + assertNotNull( securityService.verifyToken( otherCredentials, token, RestoreType.INVITE ) ); + + assertNotNull( securityService.verifyToken( credentials, "wrongToken", RestoreType.INVITE ) ); + + assertNotNull( securityService.verifyToken( credentials, token, RestoreType.RECOVER_PASSWORD ) ); + + assertNull( securityService.verifyToken( credentials, token, RestoreType.INVITE ) ); // // canRestoreNow() === modified file 'dhis-2/dhis-web/dhis-web-commons/src/main/java/org/hisp/dhis/useraccount/action/IsInviteTokenValidAction.java' --- dhis-2/dhis-web/dhis-web-commons/src/main/java/org/hisp/dhis/useraccount/action/IsInviteTokenValidAction.java 2014-03-18 08:10:10 +0000 +++ dhis-2/dhis-web/dhis-web-commons/src/main/java/org/hisp/dhis/useraccount/action/IsInviteTokenValidAction.java 2014-09-24 06:56:36 +0000 @@ -140,8 +140,8 @@ usernameChoice = Boolean.toString( restoreOptions.isUsernameChoice() ); } - boolean verified = securityService.verifyToken( userCredentials, token, RestoreType.INVITE ); + String errorMessage = securityService.verifyToken( userCredentials, token, RestoreType.INVITE ); - return verified ? SUCCESS : ERROR; + return errorMessage == null ? SUCCESS : ERROR; } } === modified file 'dhis-2/dhis-web/dhis-web-commons/src/main/java/org/hisp/dhis/useraccount/action/IsRestoreTokenValidAction.java' --- dhis-2/dhis-web/dhis-web-commons/src/main/java/org/hisp/dhis/useraccount/action/IsRestoreTokenValidAction.java 2014-03-18 08:10:10 +0000 +++ dhis-2/dhis-web/dhis-web-commons/src/main/java/org/hisp/dhis/useraccount/action/IsRestoreTokenValidAction.java 2014-09-24 06:56:36 +0000 @@ -88,9 +88,9 @@ { return ERROR; } - - boolean verified = securityService.verifyToken( credentials, token, RestoreType.RECOVER_PASSWORD ); - - return verified ? SUCCESS : ERROR; + + String errorMessage = securityService.verifyToken( credentials, token, RestoreType.RECOVER_PASSWORD ); + + return errorMessage == null ? SUCCESS : ERROR; } }