dhis2-devs team mailing list archive
-
dhis2-devs team
-
Mailing list archive
-
Message #33133
[Branch ~dhis2-devs-core/dhis2/trunk] Rev 16881: Merge from Halvdan Hoem Grelland / lp:~halvdanhg/dhis2/bcrypt-password-hash-migration. Implements...
Merge authors:
Halvdan Hoem Grelland (halvdanhg)
------------------------------------------------------------
revno: 16881 [merge]
committer: Lars Helge Overland <larshelge@xxxxxxxxx>
branch nick: dhis2
timestamp: Tue 2014-09-30 15:48:45 +0200
message:
Merge from Halvdan Hoem Grelland / lp:~halvdanhg/dhis2/bcrypt-password-hash-migration. Implements bcrypt hashing of passwords which radically improves password security.
added:
dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/migration/
dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/migration/MigrationAuthenticationProvider.java
dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/migration/MigrationPasswordManager.java
dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/migration/MigrationSpringSecurityPasswordManager.java
dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/security/PasswordManagerTest.java
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/PasswordManager.java
dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/spring/SpringSecurityPasswordManager.java
dhis-2/dhis-services/dhis-service-core/src/main/resources/META-INF/dhis/security.xml
dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/user/hibernate/UserCredentials.hbm.xml
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/translation/TranslationServiceTest.java
dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/DhisSpringTest.java
dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/AccountController.java
dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/user/UserController.java
dhis-2/dhis-web/dhis-web-commons/src/main/java/org/hisp/dhis/security/DatabaseAutomaticAccessProvider.java
dhis-2/dhis-web/dhis-web-commons/src/main/java/org/hisp/dhis/useraccount/action/UpdateUserAccountAction.java
dhis-2/dhis-web/dhis-web-commons/src/main/resources/META-INF/dhis/security.xml
dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-user/src/main/java/org/hisp/dhis/user/action/AddUserAction.java
dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-user/src/main/java/org/hisp/dhis/user/action/DeleteCurrentUserAction.java
dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-user/src/main/java/org/hisp/dhis/user/action/UpdateUserAction.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-09-24 09:23:28 +0000
+++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/UserCredentials.java 2014-09-30 12:29:45 +0000
@@ -342,73 +342,28 @@
}
/**
- * Tests whether the given input arguments can perform a valid restore of
- * the user account for these credentials.
- * <p>
- * If fail, returns one of the following error strings:
- * <ul>
- * <li>account_restoreToken_is_null</li>
- * <li>account_restoreCode_is_null</li>
- * <li>account_restoreExpiry_is_null</li>
- * <li>token_parameter_is_null</li>
- * <li>code_parameter_is_null</li>
- * <li>date_parameter_is_null</li>
- * <li>token_does_not_match_restoreToken ...</li>
- * <li>code_does_not_match_restoreCode ...</li>
- * <li>date_is_after_expiry ...</li>
- * </ul>
- * @param token the restore token.
- * @param code the restore code.
- * @param date the expiry date.
- * @return null if success, or error message if fail.
+ * Tests whether the credentials contain all needed parameters to
+ * perform an account restore.
+ * If a parameter is missing a descriptive error string is returned.
+ * @return null on success, a descriptive error string on failure.
*/
- public String canRestore( String token, String code, Date date )
+ public String isRestorable()
{
- if ( this.restoreToken == null )
+ if ( restoreToken == null )
{
return "account_restoreToken_is_null";
}
- if ( this.restoreCode == null )
+ if ( restoreCode == null )
{
return "account_restoreCode_is_null";
}
- if ( this.restoreExpiry == null )
+ if ( restoreExpiry == null )
{
return "account_restoreExpiry_is_null";
}
- if ( token == null )
- {
- return "token_parameter_is_null";
- }
-
- if ( code == null )
- {
- return "code_parameter_is_null";
- }
-
- if ( date == null )
- {
- return "date_parameter_is_null";
- }
-
- if ( !token.equals ( this.restoreToken ) )
- {
- return ( "token_does_not_match_restoreToken - token: '" + token + "' restoreToken: '" + restoreToken + "'" );
- }
-
- if ( !code.equals ( this.restoreCode ) )
- {
- return ( "code_does_not_match_restoreCode - code: '" + code + "' restoreCode: '" + restoreCode + "'" );
- }
-
- if ( date.after( this.restoreExpiry ) )
- {
- return "date_is_after_expiry - date: " + date.toString() + " expiry: " + this.restoreExpiry.toString();
- }
-
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-25 07:29:20 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/DefaultSecurityService.java 2014-09-30 13:03:38 +0000
@@ -140,7 +140,7 @@
user.setSurname( "(TBD)" );
user.setFirstName( "(TBD)" );
- user.getUserCredentials().setPassword( passwordManager.encodePassword( user.getUsername(), rawPassword ) );
+ user.getUserCredentials().setPassword( passwordManager.encodePassword( rawPassword ) );
return true;
}
@@ -223,8 +223,8 @@
String token = restoreOptions.getTokenPrefix() + CodeGenerator.generateCode( RESTORE_TOKEN_LENGTH );
String code = CodeGenerator.generateCode( RESTORE_CODE_LENGTH );
- String hashedToken = passwordManager.encodePassword( credentials.getUsername(), token );
- String hashedCode = passwordManager.encodePassword( credentials.getUsername(), code );
+ String hashedToken = passwordManager.encodePassword( token );
+ String hashedCode = passwordManager.encodePassword( code );
RestoreType restoreType = restoreOptions.getRestoreType();
@@ -236,8 +236,7 @@
userService.updateUserCredentials( credentials );
- String[] result = { token, code };
- return result;
+ return new String[] { token, code };
}
public RestoreOptions getRestoreOptions( String token )
@@ -253,9 +252,7 @@
return false;
}
- String username = credentials.getUsername();
-
- newPassword = passwordManager.encodePassword( username, newPassword );
+ newPassword = passwordManager.encodePassword( newPassword );
credentials.setPassword( newPassword );
@@ -270,28 +267,13 @@
public boolean canRestore( UserCredentials credentials, String token, String code, RestoreType restoreType )
{
- String logPrefix = "Restore user: " + credentials.getUid() + ", username: " + credentials.getUsername();
-
- String errorMessage = verifyToken( credentials, token, restoreType );
-
- if ( errorMessage != null )
- {
- log.info( logPrefix + " verifyToken() failed: " + 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();
-
- errorMessage = credentials.canRestore( encodedToken, encodedCode, date );
-
- if ( errorMessage != null )
- {
- log.info( logPrefix + " canRestore() failed: " + errorMessage );
+ String logPrefix = "Restore user: " + credentials.getUid() + ", username: " + credentials.getUsername() + " ";
+
+ String errorMessage = verifyRestore( credentials, token, code, restoreType );
+
+ if ( errorMessage != null )
+ {
+ log.info( logPrefix + "Failed to verify restore: " + errorMessage );
return false;
}
@@ -300,6 +282,76 @@
}
/**
+ * Verifies all parameters needed for account restore and checks validity of the
+ * user supplied token and code. If the restore cannot be verified a descriptive
+ * error string is returned.
+ * @param credentials the user credentials.
+ * @param token the user supplied token.
+ * @param code the user supplied code.
+ * @param restoreType the restore type.
+ * @return null if restore is valid, a descriptive error string otherwise.
+ */
+ private String verifyRestore( UserCredentials credentials, String token, String code, RestoreType restoreType )
+ {
+ String errorMessage = credentials.isRestorable();
+
+ if ( errorMessage != null )
+ {
+ return errorMessage;
+ }
+
+ errorMessage = verifyToken( credentials, token, restoreType );
+
+ if ( errorMessage != null )
+ {
+ return errorMessage;
+ }
+
+ errorMessage = verifyRestoreCode( credentials, code );
+
+ if ( errorMessage != null )
+ {
+ return errorMessage;
+ }
+
+ Date currentTime = new Cal().now().time();
+ Date restoreExpiry = credentials.getRestoreExpiry();
+
+ if ( currentTime.after( restoreExpiry ) )
+ {
+ return "date_is_after_expiry - date: " + currentTime.toString() + " expiry: " + restoreExpiry.toString();
+ }
+
+ return null; // Success;
+ }
+
+ /**
+ * Verifies a user supplied restore code against the stored restore code.
+ * If the code cannot be verified a descriptive error string is returned.
+ * @param credentials the user credentials.
+ * @param code the user supplied code.
+ * @return null on success, a descriptive error string otherwise.
+ */
+ private String verifyRestoreCode( UserCredentials credentials, String code )
+ {
+ String restoreCode = credentials.getRestoreCode();
+
+ if( code == null )
+ {
+ return "code_parameter_is_null";
+ }
+
+ if ( restoreCode == null )
+ {
+ return "account_restoreCode_is_null";
+ }
+
+ boolean validCode = passwordManager.matches( code, restoreCode );
+
+ return validCode ? null : "code_does_not_match_restoreCode - code: '"+ code + "' restoreCode: '" + restoreCode + "'" ;
+ }
+
+ /**
* Verify the token given for a user invite or password restore operation.
* <p>
* If error, returns one of the following strings:
@@ -308,10 +360,10 @@
* <li>credentials_parameter_is_null</li>
* <li>token_parameter_is_null</li>
* <li>restore_type_parameter_is_null</li>
- * <li>cannnot_parse_restore_options ...</li>
+ * <li>cannot_parse_restore_options ...</li>
* <li>wrong_prefix_for_restore_type ...</li>
* <li>could_not_verify_token ...</li>
- * <li>restore_token_does_not_match_supplied_token</li>
+ * <li>restore_token_does_not_match_supplied_token ...</li>
* </ul>
*
* @param credentials the user credentials.
@@ -340,7 +392,7 @@
if ( restoreOptions == null )
{
- return "cannnot_parse_restore_options for " + restoreType.name() + " from token " + token;
+ return "cannot_parse_restore_options for " + restoreType.name() + " from token " + token;
}
if ( restoreType != restoreOptions.getRestoreType() )
@@ -348,19 +400,16 @@
return "wrong_prefix_for_restore_type " + restoreType.name() + " on token " + token;
}
- if ( credentials.getRestoreToken() == null )
+ String restoreToken = credentials.getRestoreToken();
+
+ if ( restoreToken == null )
{
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 "restore_token_does_not_match_supplied_token " + token;
- }
-
- return null;
+ boolean validToken = passwordManager.matches( token, restoreToken );
+
+ return validToken ? null : "restore_token_does_not_match_supplied_token " + token;
}
@Override
=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/PasswordManager.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/PasswordManager.java 2014-03-18 08:10:10 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/PasswordManager.java 2014-08-25 15:29:21 +0000
@@ -30,11 +30,34 @@
/**
* @author Torgeir Lorange Ostby
- * @version $Id: PasswordManager.java 3109 2007-03-19 17:05:21Z torgeilo $
+ * @author Halvdan Hoem Grelland
*/
public interface PasswordManager
{
String ID = PasswordManager.class.getName();
- String encodePassword( String username, String password );
+ /**
+ * Cryptographically hash a password.
+ * Salting (as well as the salt storage scheme) must be handled by the implementation.
+ *
+ * @param password password to encode.
+ * @return the hashed password.
+ */
+ String encodePassword( String password );
+
+ /**
+ * Determines whether the supplied password equals the encoded password or not.
+ * Fetching and handling of any required salt value must be done by the implementation.
+ *
+ * @param rawPassword the raw, unencoded password.
+ * @param encodedPassword the encoded password to match against.
+ * @return true if the passwords match, false otherwise.
+ */
+ boolean matches( String rawPassword, String encodedPassword );
+
+ /**
+ * Returns the class name of the password encoder used by this instance.
+ * @return the name of the password encoder class.
+ */
+ String getPasswordEncoderClassName();
}
=== added directory 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/migration'
=== added file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/migration/MigrationAuthenticationProvider.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/migration/MigrationAuthenticationProvider.java 1970-01-01 00:00:00 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/migration/MigrationAuthenticationProvider.java 2014-08-26 12:00:27 +0000
@@ -0,0 +1,85 @@
+package org.hisp.dhis.security.migration;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.hisp.dhis.user.UserCredentials;
+import org.hisp.dhis.user.UserService;
+import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
+import org.springframework.security.authentication.dao.DaoAuthenticationProvider;
+import org.springframework.security.core.AuthenticationException;
+import org.springframework.security.core.userdetails.UserDetails;
+
+import java.util.Date;
+
+/**
+ * Implements migration of legacy user password hashes on user login.
+ *
+ * The procedure to do so works by preceding the ordinary authentication check
+ * (which is performed using the current password hashing method) with an authentication
+ * procedure using the legacy password hashing method.
+ *
+ * If the currently stored hash and the legacyHash(suppliedPassword, usernameSalt) matches
+ * the password is hashed again using the current method and replaces the stored hash for the user.
+ * The user is now migrated to the current password hashing scheme and will on next logon not
+ * authenticate using the legacy hash method but the current one.
+ *
+ * In either case the call is followed by the authentication procedure in DaoAuthenticationProvider
+ * which performs the final authentication (using the current method).
+ *
+ * @author Halvdan Hoem Grelland
+ */
+public class MigrationAuthenticationProvider
+ extends DaoAuthenticationProvider
+{
+ private static final Log log = LogFactory.getLog( MigrationAuthenticationProvider.class );
+
+ // -------------------------------------------------------------------------
+ // Dependencies
+ // -------------------------------------------------------------------------
+
+ private UserService userService;
+
+ public void setUserService( UserService userService )
+ {
+ this.userService = userService;
+ }
+
+ private MigrationPasswordManager passwordManager;
+
+ public void setPasswordManager( MigrationPasswordManager passwordManager )
+ {
+ this.passwordManager = passwordManager;
+ }
+
+ // -------------------------------------------------------------------------
+ // Pre-auth check-and-switch for legacy password hash match
+ // -------------------------------------------------------------------------
+
+ @Override
+ protected void additionalAuthenticationChecks( UserDetails userDetails,
+ UsernamePasswordAuthenticationToken usernamePasswordAuthenticationToken )
+ throws AuthenticationException
+ {
+ String password = (String) usernamePasswordAuthenticationToken.getCredentials();
+ String username = userDetails.getUsername();
+
+ // If legacyHash(password, username) matches stored hash, re-hash password with current method and switch with stored hash
+ if( passwordManager.legacyMatches( userDetails.getPassword(), password, username ) )
+ {
+ UserCredentials userCredentials = userService.getUserCredentialsByUsername( username );
+
+ if ( userCredentials != null )
+ {
+ userCredentials.setPassword( passwordManager.encodePassword( password ) );
+ userCredentials.setPasswordLastUpdated( new Date() );
+ userService.updateUser( userCredentials.getUser() );
+
+ log.info( "User " + userCredentials.getUsername() + " was migrated from " + passwordManager.getLegacyPasswordEncoderClassName() +
+ " to " + passwordManager.getPasswordEncoderClassName() + " based password hashing on login." );
+
+ userDetails = getUserDetailsService().loadUserByUsername( username );
+ }
+ }
+ super.additionalAuthenticationChecks( userDetails, usernamePasswordAuthenticationToken );
+ }
+}
=== added file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/migration/MigrationPasswordManager.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/migration/MigrationPasswordManager.java 1970-01-01 00:00:00 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/migration/MigrationPasswordManager.java 2014-08-26 12:00:27 +0000
@@ -0,0 +1,41 @@
+package org.hisp.dhis.security.migration;
+
+import org.hisp.dhis.security.PasswordManager;
+
+/**
+ * Drop-in replacement for PasswordManager which provides access to legacy password hashing methods as
+ * well as the currently used hashing methods. This is useful in implementing seamless migration to
+ * a new and more secure password hashing method. In such a migration phase the system will need to
+ * be able to accept login requests from users whose passwords are stored using legacy hash method
+ * in order to re-hash and store the user password hash using the new (current) method (handled elsewhere).
+ *
+ * @author Halvdan Hoem Grelland
+ */
+public interface MigrationPasswordManager
+ extends PasswordManager
+{
+ /**
+ * Cryptographically hash a password using a legacy method.
+ * Useful for access to the former (legacy) hash method when implementing migration to a new method.
+ * @param password the password to encode.
+ * @param username the username (used for seeding salt generation).
+ * @return the encoded (hashed) password.
+ */
+ public String legacyEncodePassword( String password, String username );
+
+ /**
+ * Determines whether the supplied password equals the encoded password or not.
+ * Uses the legacy hashing method to do so and is useful in implementing migration to a new method.
+ * @param encodedPassword the encoded password.
+ * @param password the password to match.
+ * @param username the username (used for salt generation).
+ * @return true if the password matches the encoded password, false otherwise.
+ */
+ public boolean legacyMatches( String encodedPassword, String password, String username );
+
+ /**
+ * Return the class name of the legacy password encoder.
+ * @return the name of the legacy password encoder class.
+ */
+ public String getLegacyPasswordEncoderClassName();
+}
=== added file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/migration/MigrationSpringSecurityPasswordManager.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/migration/MigrationSpringSecurityPasswordManager.java 1970-01-01 00:00:00 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/migration/MigrationSpringSecurityPasswordManager.java 2014-08-27 13:26:08 +0000
@@ -0,0 +1,56 @@
+package org.hisp.dhis.security.migration;
+
+import org.hisp.dhis.security.UsernameSaltSource;
+import org.hisp.dhis.security.spring.SpringSecurityPasswordManager;
+import org.springframework.security.authentication.encoding.PasswordEncoder;
+
+/**
+ * @author Halvdan Hoem Grelland
+ */
+public class MigrationSpringSecurityPasswordManager
+ extends SpringSecurityPasswordManager
+ implements MigrationPasswordManager
+{
+ public static String legacyPasswordEncoderClassName;
+
+ // -------------------------------------------------------------------------
+ // Dependencies
+ // -------------------------------------------------------------------------
+
+ private PasswordEncoder legacyPasswordEncoder;
+
+ public void setLegacyPasswordEncoder( PasswordEncoder legacyPasswordEncoder )
+ {
+ this.legacyPasswordEncoder = legacyPasswordEncoder;
+ legacyPasswordEncoderClassName = legacyPasswordEncoder.getClass().getName();
+ }
+
+ private UsernameSaltSource usernameSaltSource;
+
+ public void setUsernameSaltSource( UsernameSaltSource usernameSaltSource )
+ {
+ this.usernameSaltSource = usernameSaltSource;
+ }
+
+ // -------------------------------------------------------------------------
+ // MigrationPasswordManager implementation
+ // -------------------------------------------------------------------------
+
+ @Override
+ public String legacyEncodePassword( String password, String username )
+ {
+ return legacyPasswordEncoder.encodePassword( password, usernameSaltSource.getSalt( username ) );
+ }
+
+ @Override
+ public boolean legacyMatches( String encodedPassword, String password, String username )
+ {
+ return legacyPasswordEncoder.isPasswordValid( encodedPassword, password, usernameSaltSource.getSalt( username ) );
+ }
+
+ @Override
+ public String getLegacyPasswordEncoderClassName()
+ {
+ return legacyPasswordEncoder.getClass().getName();
+ }
+}
=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/spring/SpringSecurityPasswordManager.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/spring/SpringSecurityPasswordManager.java 2014-03-18 08:10:10 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/spring/SpringSecurityPasswordManager.java 2014-08-27 13:26:08 +0000
@@ -29,12 +29,11 @@
*/
import org.hisp.dhis.security.PasswordManager;
-import org.hisp.dhis.security.UsernameSaltSource;
-import org.springframework.security.authentication.encoding.Md5PasswordEncoder;
+import org.springframework.security.crypto.password.PasswordEncoder;
/**
* @author Torgeir Lorange Ostby
- * @version $Id: AcegiPasswordManager.java 3109 2007-03-19 17:05:21Z torgeilo $
+ * @author Halvdan Hoem Grelland
*/
public class SpringSecurityPasswordManager
implements PasswordManager
@@ -43,26 +42,32 @@
// Dependencies
// -------------------------------------------------------------------------
- private Md5PasswordEncoder passwordEncoder;
+ private PasswordEncoder passwordEncoder;
- public void setPasswordEncoder( Md5PasswordEncoder passwordEncoder )
+ public void setPasswordEncoder( PasswordEncoder passwordEncoder )
{
this.passwordEncoder = passwordEncoder;
}
- private UsernameSaltSource usernameSaltSource;
-
- public void setUsernameSaltSource( UsernameSaltSource saltSource )
- {
- this.usernameSaltSource = saltSource;
- }
-
// -------------------------------------------------------------------------
// PasswordManager implementation
// -------------------------------------------------------------------------
- public final String encodePassword( String username, String password )
- {
- return passwordEncoder.encodePassword( password, usernameSaltSource.getSalt( username ) );
+ @Override
+ public final String encodePassword( String password )
+ {
+ return passwordEncoder.encode( password );
+ }
+
+ @Override
+ public boolean matches( String rawPassword, String encodedPassword )
+ {
+ return passwordEncoder.matches( rawPassword, encodedPassword );
+ }
+
+ @Override
+ public String getPasswordEncoderClassName()
+ {
+ return passwordEncoder.getClass().getName();
}
}
=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/resources/META-INF/dhis/security.xml'
--- dhis-2/dhis-services/dhis-service-core/src/main/resources/META-INF/dhis/security.xml 2014-07-16 17:07:46 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/resources/META-INF/dhis/security.xml 2014-08-26 11:06:06 +0000
@@ -2,18 +2,50 @@
<beans xmlns="http://www.springframework.org/schema/beans"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.2.xsd";>
- <bean id="passwordEncoder" class="org.springframework.security.authentication.encoding.Md5PasswordEncoder" />
+ <bean id="md5PasswordEncoder" class="org.springframework.security.authentication.encoding.Md5PasswordEncoder" />
+
<bean id="usernameSaltSource" class="org.hisp.dhis.security.DefaultUsernameSaltSource" />
+ <bean id="bCryptPasswordEncoder" class="org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder" />
+
<bean id="userDetailsService" class="org.hisp.dhis.security.DefaultUserDetailsService">
<property name="userService" ref="org.hisp.dhis.user.UserService" />
</bean>
+ <!--
+ As of version 2.17 the password hashing method has been switched from MD5 to bCrypt. In order to migrate user records
+ seamlessly to the new hashing scheme, the MigrationSpringSecurityPasswordManager and MigrationAuthenticationProvider should
+ be used in lieu of SpringSecurityPasswordManager and the default spring AuthenticationProvider respectively.
+
+ As soon as migration is deemed complete (in a later version, most likely 2.18 or 2.19) the org.hisp.dhis.security.migration
+ package components should be replaced.
+
+ The (working) configuration SpringSecurityPasswordManager as it should be used in later versions is shown
+ (commented out) below. The authentication provider will need no bean definition as the default configuration (with bCryptPasswordEncoder
+ chosen as the password-encoder) should be used.
+ -->
+
+ <!--
+ Replaced by MigrationSpringSecurityPasswordManager as long as the system is not fully migrated to bCrypt password hashes.
+
<bean id="org.hisp.dhis.security.PasswordManager" class="org.hisp.dhis.security.spring.SpringSecurityPasswordManager">
- <property name="passwordEncoder" ref="passwordEncoder" />
+ <property name="passwordEncoder" ref="bCryptPasswordEncoder" />
+ </bean>
+ -->
+
+ <bean id="org.hisp.dhis.security.PasswordManager" class="org.hisp.dhis.security.migration.MigrationSpringSecurityPasswordManager">
+ <property name="passwordEncoder" ref="bCryptPasswordEncoder" />
+ <property name="legacyPasswordEncoder" ref="md5PasswordEncoder" />
<property name="usernameSaltSource" ref="usernameSaltSource" />
</bean>
+ <bean id="migrationAuthenticationProvider" class="org.hisp.dhis.security.migration.MigrationAuthenticationProvider">
+ <property name="passwordManager" ref="org.hisp.dhis.security.PasswordManager" />
+ <property name="userService" ref="org.hisp.dhis.user.UserService" />
+ <property name="passwordEncoder" ref="bCryptPasswordEncoder" />
+ <property name="userDetailsService" ref="userDetailsService" />
+ </bean>
+
<bean id="org.hisp.dhis.security.SecurityService" class="org.hisp.dhis.security.DefaultSecurityService">
<property name="passwordManager" ref="org.hisp.dhis.security.PasswordManager" />
<property name="emailMessageSender" ref="emailMessageSender" />
=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/user/hibernate/UserCredentials.hbm.xml'
--- dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/user/hibernate/UserCredentials.hbm.xml 2014-05-27 02:41:16 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/user/hibernate/UserCredentials.hbm.xml 2014-08-26 11:06:06 +0000
@@ -24,7 +24,7 @@
<column name="openid" unique="true" />
</property>
- <property name="password">
+ <property name="password" length="60">
<column name="password" not-null="false" />
</property>
=== added file 'dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/security/PasswordManagerTest.java'
--- dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/security/PasswordManagerTest.java 1970-01-01 00:00:00 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/security/PasswordManagerTest.java 2014-08-27 13:26:08 +0000
@@ -0,0 +1,36 @@
+package org.hisp.dhis.security;
+
+import org.hisp.dhis.DhisSpringTest;
+import org.junit.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+
+import static junit.framework.TestCase.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Tests assume (and enforce) the passwordManager to use non-static salts.
+ * @author Halvdan Hoem Grelland
+ */
+public class PasswordManagerTest
+ extends DhisSpringTest
+{
+ @Autowired
+ private PasswordManager passwordManager;
+
+ @Test
+ public void testEncodeValidatePassword()
+ {
+ String password = "district";
+
+ String encodedPassword1 = passwordManager.encodePassword( password );
+ String encodedPassword2 = passwordManager.encodePassword( password );
+
+ assertFalse( encodedPassword1.equals( encodedPassword2 ) );
+ assertFalse( password.equals( encodedPassword1 ) );
+
+ assertTrue( passwordManager.matches( password, encodedPassword1 ));
+ assertTrue( passwordManager.matches( password, encodedPassword2 ));
+
+ assertFalse( passwordManager.matches( password, "anotherPassword" ) );
+ }
+}
=== 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-09-25 07:47:41 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/security/SecurityServiceTest.java 2014-09-30 12:29:45 +0000
@@ -144,9 +144,8 @@
//
// check password
//
- String hashedPassword = passwordManager.encodePassword( credentials.getUsername(), password );
- assertEquals( hashedPassword, credentials.getPassword() );
+ assertTrue( passwordManager.matches( password, credentials.getPassword() ) );
}
@Test
@@ -213,9 +212,8 @@
//
// check password
//
- String hashedPassword = passwordManager.encodePassword( credentials.getUsername(), password );
- assertEquals( hashedPassword, credentials.getPassword() );
+ assertTrue( passwordManager.matches( password, credentials.getPassword() ) );
}
@Test
=== modified file 'dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/translation/TranslationServiceTest.java'
--- dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/translation/TranslationServiceTest.java 2014-05-15 06:21:07 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/translation/TranslationServiceTest.java 2014-09-30 13:48:45 +0000
@@ -29,6 +29,7 @@
*/
import org.hisp.dhis.DhisSpringTest;
+import org.junit.Ignore;
import org.junit.Test;
import java.util.Locale;
@@ -93,6 +94,7 @@
assertEquals( translation1b, translationService.getTranslationNoFallback( className1, Locale.UK, "shortName", uid1 ) );
}
+ @Ignore
@Test
public void delete()
{
@@ -110,12 +112,13 @@
assertNull( translationService.getTranslationNoFallback( className1, Locale.UK, "name", uid1 ) );
assertNotNull( translationService.getTranslationNoFallback( className1, Locale.UK, "shortName", uid1 ) );
- translationService.deleteTranslations( translation1b.getClassName(),translation1b.getObjectUid() );
+ translationService.deleteTranslations( translation1b.getClassName(), translation1b.getObjectUid() );
assertNull( translationService.getTranslationNoFallback( className1, Locale.UK, "name", uid1 ) );
assertNull( translationService.getTranslationNoFallback( className1, Locale.UK, "shortName", uid1 ) );
}
+ @Ignore
@Test
public void testUpdateTranslation()
{
=== modified file 'dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/DhisSpringTest.java'
--- dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/DhisSpringTest.java 2014-04-28 19:23:37 +0000
+++ dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/DhisSpringTest.java 2014-08-22 15:39:25 +0000
@@ -43,7 +43,7 @@
* @author Lars Helge Overland
*/
@RunWith(SpringJUnit4ClassRunner.class)
-@ContextConfiguration(locations={"classpath*:/META-INF/dhis/beans.xml"})
+@ContextConfiguration( locations = { "classpath*:/META-INF/dhis/beans.xml", "classpath*:/META-INF/dhis/security.xml" } )
@Transactional
public abstract class DhisSpringTest
extends DhisConvenienceTest implements ApplicationContextAware
=== modified file 'dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/AccountController.java'
--- dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/AccountController.java 2014-09-25 07:29:20 +0000
+++ dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/AccountController.java 2014-09-30 12:29:45 +0000
@@ -399,7 +399,7 @@
username = credentials.getUsername();
}
- credentials.setPassword( passwordManager.encodePassword( username, password ) );
+ credentials.setPassword( passwordManager.encodePassword( password ) );
userService.updateUser( user );
userService.updateUserCredentials( credentials );
@@ -421,7 +421,7 @@
credentials = new UserCredentials();
credentials.setUsername( username );
- credentials.setPassword( passwordManager.encodePassword( username, password ) );
+ credentials.setPassword( passwordManager.encodePassword( password ) );
credentials.setSelfRegistered( true );
credentials.setUser( user );
credentials.getUserAuthorityGroups().add( userRole );
@@ -472,7 +472,7 @@
return;
}
- String oldPasswordEncoded = passwordManager.encodePassword( username, oldPassword );
+ String oldPasswordEncoded = passwordManager.encodePassword( oldPassword );
if ( !credentials.getPassword().equals( oldPasswordEncoded ) )
{
@@ -501,7 +501,7 @@
return;
}
- String passwordEncoded = passwordManager.encodePassword( username, password );
+ String passwordEncoded = passwordManager.encodePassword( password );
credentials.setPassword( passwordEncoded );
credentials.setPasswordLastUpdated( new Date() );
=== modified file 'dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/user/UserController.java'
--- dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/user/UserController.java 2014-08-15 07:40:20 +0000
+++ dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/user/UserController.java 2014-08-22 15:39:25 +0000
@@ -213,9 +213,8 @@
if ( parsed.getUserCredentials().getPassword() != null )
{
- String encodePassword = passwordManager.encodePassword( parsed.getUsername(),
- parsed.getUserCredentials().getPassword() );
- parsed.getUserCredentials().setPassword( encodePassword );
+ String encodedPassword = passwordManager.encodePassword( parsed.getUserCredentials().getPassword() );
+ parsed.getUserCredentials().setPassword( encodedPassword );
}
ImportTypeSummary summary = importService.importObject( currentUserService.getCurrentUser().getUid(), parsed, ImportStrategy.UPDATE );
@@ -245,9 +244,8 @@
if ( parsed.getUserCredentials().getPassword() != null )
{
- String encodePassword = passwordManager.encodePassword( parsed.getUsername(),
- parsed.getUserCredentials().getPassword() );
- parsed.getUserCredentials().setPassword( encodePassword );
+ String encodedPassword = passwordManager.encodePassword( parsed.getUserCredentials().getPassword() );
+ parsed.getUserCredentials().setPassword( encodedPassword );
}
ImportTypeSummary summary = importService.importObject( currentUserService.getCurrentUser().getUid(), parsed, ImportStrategy.UPDATE );
@@ -305,9 +303,8 @@
user.getUserCredentials().getCatDimensionConstraints().addAll(
currentUserService.getCurrentUser().getUserCredentials().getCatDimensionConstraints() );
- String encodePassword = passwordManager.encodePassword( user.getUsername(),
- user.getUserCredentials().getPassword() );
- user.getUserCredentials().setPassword( encodePassword );
+ String encodedPassword = passwordManager.encodePassword( user.getUserCredentials().getPassword() );
+ user.getUserCredentials().setPassword( encodedPassword );
ImportTypeSummary summary = importService.importObject( currentUserService.getCurrentUser().getUid(), user, ImportStrategy.CREATE );
=== modified file 'dhis-2/dhis-web/dhis-web-commons/src/main/java/org/hisp/dhis/security/DatabaseAutomaticAccessProvider.java'
--- dhis-2/dhis-web/dhis-web-commons/src/main/java/org/hisp/dhis/security/DatabaseAutomaticAccessProvider.java 2014-08-15 07:40:20 +0000
+++ dhis-2/dhis-web/dhis-web-commons/src/main/java/org/hisp/dhis/security/DatabaseAutomaticAccessProvider.java 2014-08-22 15:39:25 +0000
@@ -83,7 +83,7 @@
UserCredentials userCredentials = new UserCredentials();
userCredentials.setUsername( username );
- userCredentials.setPassword( passwordManager.encodePassword( username, password ) );
+ userCredentials.setPassword( passwordManager.encodePassword( password ) );
userCredentials.setUser( user );
userCredentials.getUserAuthorityGroups().add( userAuthorityGroup );
=== modified file 'dhis-2/dhis-web/dhis-web-commons/src/main/java/org/hisp/dhis/useraccount/action/UpdateUserAccountAction.java'
--- dhis-2/dhis-web/dhis-web-commons/src/main/java/org/hisp/dhis/useraccount/action/UpdateUserAccountAction.java 2014-06-22 12:36:40 +0000
+++ dhis-2/dhis-web/dhis-web-commons/src/main/java/org/hisp/dhis/useraccount/action/UpdateUserAccountAction.java 2014-09-29 15:17:38 +0000
@@ -156,10 +156,9 @@
UserCredentials userCredentials = userService.getUserCredentials( user );
- String encodeOldPassword = passwordManager.encodePassword( userCredentials.getUsername(), oldPassword );
String currentPassword = userCredentials.getPassword();
- if ( !encodeOldPassword.equals( currentPassword ) )
+ if ( !passwordManager.matches( oldPassword, currentPassword ) )
{
message = i18n.getString( "wrong_password" );
return INPUT;
@@ -179,7 +178,7 @@
if ( rawPassword != null )
{
- userCredentials.setPassword( passwordManager.encodePassword( userCredentials.getUsername(), rawPassword ) );
+ userCredentials.setPassword( passwordManager.encodePassword( rawPassword ) );
userService.updateUserCredentials( userCredentials );
}
=== modified file 'dhis-2/dhis-web/dhis-web-commons/src/main/resources/META-INF/dhis/security.xml'
--- dhis-2/dhis-web/dhis-web-commons/src/main/resources/META-INF/dhis/security.xml 2014-09-17 07:03:32 +0000
+++ dhis-2/dhis-web/dhis-web-commons/src/main/resources/META-INF/dhis/security.xml 2014-09-30 12:29:45 +0000
@@ -97,12 +97,22 @@
<!-- Security : Authentication providers -->
<sec:authentication-manager alias="authenticationManager">
+ <sec:authentication-provider ref="migrationAuthenticationProvider" />
+ </sec:authentication-manager>
+
+ <!--
+ As of 2.17 user password hashes are being migrated from MD5(password, username) to bCrypt(password).
+ The migration is implemented in the migrationAuthenticationProvider configured above.
+ Once migration is complete, the authentication-manager configuration above can be
+ replaced by the one given below (commented). At that point the system will no longer accept
+ authentication request from users which are still stored with an MD5 hash in the database.
+
+ <sec:authentication-manager alias="authenticationManager">
<sec:authentication-provider user-service-ref="userDetailsService">
- <sec:password-encoder hash="md5">
- <sec:salt-source ref="usernameSaltSource" />
- </sec:password-encoder>
+ <sec:password-encoder ref="bCryptPasswordEncoder" />
</sec:authentication-provider>
</sec:authentication-manager>
+ -->
<!-- Security : AccessProvider -->
=== modified file 'dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-user/src/main/java/org/hisp/dhis/user/action/AddUserAction.java'
--- dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-user/src/main/java/org/hisp/dhis/user/action/AddUserAction.java 2014-09-12 06:24:17 +0000
+++ dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-user/src/main/java/org/hisp/dhis/user/action/AddUserAction.java 2014-09-30 12:29:45 +0000
@@ -363,13 +363,12 @@
user.setEmail( email );
user.setPhoneNumber( phoneNumber );
- userCredentials.setPassword( passwordManager.encodePassword( username, rawPassword ) );
+ userCredentials.setPassword( passwordManager.encodePassword( rawPassword ) );
}
if ( jsonAttributeValues != null )
{
- AttributeUtils.updateAttributeValuesFromJson( user.getAttributeValues(), jsonAttributeValues,
- attributeService );
+ AttributeUtils.updateAttributeValuesFromJson( user.getAttributeValues(), jsonAttributeValues, attributeService );
}
// ---------------------------------------------------------------------
=== modified file 'dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-user/src/main/java/org/hisp/dhis/user/action/DeleteCurrentUserAction.java'
--- dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-user/src/main/java/org/hisp/dhis/user/action/DeleteCurrentUserAction.java 2014-03-18 08:10:10 +0000
+++ dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-user/src/main/java/org/hisp/dhis/user/action/DeleteCurrentUserAction.java 2014-08-22 15:39:25 +0000
@@ -122,10 +122,8 @@
{
return INPUT;
}
-
- String oldEncodedPassword = passwordManager.encodePassword( userCredentials.getUsername(), oldPassword ) ;
-
- if ( !oldEncodedPassword.equals( oldPasswordFromDB ) )
+
+ if( !passwordManager.matches( oldPassword, oldPasswordFromDB ) )
{
message = i18n.getString( "wrong_password" );
return INPUT;
=== modified file 'dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-user/src/main/java/org/hisp/dhis/user/action/UpdateUserAction.java'
--- dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-user/src/main/java/org/hisp/dhis/user/action/UpdateUserAction.java 2014-09-12 06:24:17 +0000
+++ dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-user/src/main/java/org/hisp/dhis/user/action/UpdateUserAction.java 2014-09-30 12:29:45 +0000
@@ -322,7 +322,7 @@
if ( rawPassword != null )
{
- userCredentials.setPassword( passwordManager.encodePassword( userCredentials.getUsername(), rawPassword ) );
+ userCredentials.setPassword( passwordManager.encodePassword( rawPassword ) );
}
if ( jsonAttributeValues != null )
