← Back to team overview

dhis2-devs team mailing list archive

[Branch ~dhis2-devs-core/dhis2/trunk] Rev 13700: SecurityServce, using UserCredentials as iinterface argument rather than username for testability...

 

------------------------------------------------------------
revno: 13700
committer: Lars Helge Øverland <larshelge@xxxxxxxxx>
branch nick: dhis2
timestamp: Mon 2014-01-13 16:16:22 +0100
message:
  SecurityServce, using UserCredentials as iinterface argument rather than username for testability purposes.
modified:
  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/test/java/org/hisp/dhis/security/SecurityServiceTest.java
  dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/api/controller/AccountController.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-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	2013-08-23 16:05:01 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/DefaultSecurityService.java	2014-01-13 15:16:22 +0000
@@ -105,18 +105,16 @@
     // SecurityService implementation
     // -------------------------------------------------------------------------
 
-    public boolean sendRestoreMessage( String username, String rootPath )
+    public boolean sendRestoreMessage( UserCredentials credentials, String rootPath )
     {
-        if ( username == null || rootPath == null )
+        if ( credentials == null || rootPath == null )
         {
             return false;
         }
 
-        UserCredentials credentials = userService.getUserCredentialsByUsername( username );
-
-        if ( credentials == null || credentials.getUser() == null || credentials.getUser().getEmail() == null )
+        if ( credentials.getUser() == null || credentials.getUser().getEmail() == null )
         {
-            log.info( "Could not send message as user does not exist or has no email: " + username );
+            log.info( "Could not send message as user does not exist or has no email: " + credentials );
             return false;
         }
 
@@ -148,7 +146,7 @@
         vars.put( "restorePath", rootPath + RESTORE_PATH );
         vars.put( "token", result[0] );
         vars.put( "code", result[1] );
-        vars.put( "username", username );
+        vars.put( "username", credentials.getUsername() );
 
         String text1 = new VelocityManager().render( vars, "restore_message1" );
         String text2 = new VelocityManager().render( vars, "restore_message2" );
@@ -179,21 +177,15 @@
         return result;
     }
 
-    public boolean restore( String username, String token, String code, String newPassword )
+    public boolean restore( UserCredentials credentials, String token, String code, String newPassword )
     {
-        if ( username == null || token == null || code == null || newPassword == null )
-        {
-            return false;
-        }
-
-        UserCredentials credentials = userService.getUserCredentialsByUsername( username );
-
-        if ( credentials == null )
-        {
-            log.info( "Could not restore as user does not exist: " + username );
-            return false;
-        }
-
+        if ( credentials == null || token == null || code == null || newPassword == null )
+        {
+            return false;
+        }
+
+        String username = credentials.getUsername();
+        
         token = passwordManager.encodePassword( username, token );
         code = passwordManager.encodePassword( username, code );
 
@@ -217,22 +209,20 @@
         return true;
     }
 
-    public boolean verifyToken( String username, String token )
+    public boolean verifyToken( UserCredentials credentials, String token )
     {
-        if ( username == null || token == null )
-        {
-            return false;
-        }
-
-        UserCredentials credentials = userService.getUserCredentialsByUsername( username );
-
-        if ( credentials == null || credentials.getRestoreToken() == null )
-        {
-            log.info( "Could not verify token as user does not exist or has no token: " + username );
-            return false;
-        }
-
-        token = passwordManager.encodePassword( username, token );
+        if ( credentials == null || token == null )
+        {
+            return false;
+        }
+
+        if ( credentials.getRestoreToken() == null )
+        {
+            log.info( "Could not verify token as user has no token: " + credentials );
+            return false;
+        }
+
+        token = passwordManager.encodePassword( credentials.getUsername(), token );
 
         return credentials.getRestoreToken().equals( token );
     }

=== 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	2013-08-23 16:05:01 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/SecurityService.java	2014-01-13 15:16:22 +0000
@@ -40,12 +40,12 @@
      * Will invoke the initiateRestore method and dispatch email messages with
      * restore information to the user.
      *
-     * @param username the user name of the user to send restore messages.
+     * @param credentials the credentials for the user to send restore message.
      * @param rootPath the root path of the request.
      * @return false if any of the arguments are null or if the user credentials
      *         identified by the user name does not exist, true otherwise.
      */
-    boolean sendRestoreMessage( String username, String rootPath );
+    boolean sendRestoreMessage( UserCredentials credentials, String rootPath );
 
     /**
      * Will populate the restoreToken and restoreCode property of the given
@@ -66,26 +66,26 @@
      * must match the ones on the credentials, and the current date must be before
      * the expiry date time of the credentials.
      *
-     * @param username    the user name.
-     * @param token       the token.
-     * @param code        the code.
+     * @param credentials the user credentials.
+     * @param token the token.
+     * @param code the code.
      * @param newPassword the proposed new password.
      * @return true or false.
      */
-    boolean restore( String username, String token, String code, String newPassword );
+    boolean restore( UserCredentials credentials, String token, String code, String newPassword );
 
     /**
      * Tests whether the given token in combination with the given user name is
      * valid, i.e. whether the hashed version of the token matches the one on the
      * user credentials identified by the given user name.
      *
-     * @param username the user name.
-     * @param token    the token.
+     * @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.
      */
-    boolean verifyToken( String username, String token );
+    boolean verifyToken( UserCredentials credentials, String token );
 
     /**
      * Checks whether current user has read access to object.

=== 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	2013-08-23 16:05:01 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/security/SecurityServiceTest.java	2014-01-13 15:16:22 +0000
@@ -79,13 +79,13 @@
         assertNotNull( credentials.getRestoreCode() );
         assertNotNull( credentials.getRestoreExpiry() );
         
-        boolean verified = securityService.verifyToken( credentials.getUsername(), result[0] );
+        boolean verified = securityService.verifyToken( credentials, result[0] );
         
         assertTrue( verified );
         
         String password = "NewPassword1";
         
-        boolean restored = securityService.restore( credentials.getUsername(), result[0], result[1], password );
+        boolean restored = securityService.restore( credentials, result[0], result[1], password );
         
         assertTrue( restored );
         

=== modified file 'dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/api/controller/AccountController.java'
--- dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/api/controller/AccountController.java	2014-01-07 18:46:22 +0000
+++ dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/api/controller/AccountController.java	2014-01-13 15:16:22 +0000
@@ -106,7 +106,7 @@
 
     @Autowired
     private SystemSettingManager systemSettingManager;
-
+    
     private ObjectMapper objectMapper = new ObjectMapper();
 
     @RequestMapping( value = "/recovery", method = RequestMethod.POST, produces = ContextUtils.CONTENT_TYPE_TEXT )
@@ -123,7 +123,15 @@
             return "Account recovery is not enabled";
         }
 
-        boolean recover = securityService.sendRestoreMessage( username, rootPath );
+        UserCredentials credentials = userService.getUserCredentialsByUsername( username );
+
+        if ( credentials == null )
+        {
+            response.setStatus( HttpServletResponse.SC_CONFLICT );
+            return "User does not exist: " + username;
+        }
+        
+        boolean recover = securityService.sendRestoreMessage( credentials, rootPath );
 
         if ( !recover )
         {
@@ -164,7 +172,15 @@
             return "Password cannot be equal to username";
         }
 
-        boolean restore = securityService.restore( username, token, code, password );
+        UserCredentials credentials = userService.getUserCredentialsByUsername( username );
+
+        if ( credentials == null )
+        {
+            response.setStatus( HttpServletResponse.SC_CONFLICT );
+            return "User does not exist: " + username;
+        }
+        
+        boolean restore = securityService.restore( credentials, token, code, password );
 
         if ( !restore )
         {