dhis2-devs team mailing list archive
-
dhis2-devs team
-
Mailing list archive
-
Message #32991
[Branch ~dhis2-devs-core/dhis2/trunk] Rev 16787: Better error logging when user account invite response fails.
------------------------------------------------------------
revno: 16787
committer: jimgrace@xxxxxxxxx
branch nick: dhis2
timestamp: Wed 2014-09-24 08:56:36 +0200
message:
Better error logging when user account invite response fails.
modified:
dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/UserCredentials.java
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/SecurityService.java
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/test/java/org/hisp/dhis/security/SecurityServiceTest.java
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/IsRestoreTokenValidAction.java
--
lp:dhis2
https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk
Your team DHIS 2 developers is subscribed to branch lp:dhis2.
To unsubscribe from this branch go to https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk/+edit-subscription
=== 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;
}
}
