geronimo-scm mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From djen...@apache.org
Subject svn commit: r565599 - in /geronimo/server/trunk: applications/console/geronimo-console-core/src/main/java/org/apache/geronimo/console/core/security/ applications/console/geronimo-console-standard/src/main/java/org/apache/geronimo/console/securitymanage...
Date Tue, 14 Aug 2007 01:21:57 GMT
Author: djencks
Date: Mon Aug 13 18:21:55 2007
New Revision: 565599

URL: http://svn.apache.org/viewvc?view=rev&rev=565599
Log:
GERONIMO-3404 GERONIMO-3406  Fix the return values from login module lifecycle methods and add some javadoc and a little code cleanup

Removed:
    geronimo/server/trunk/applications/console/geronimo-console-core/src/main/java/org/apache/geronimo/console/core/security/PropertiesFileLoginModuleNoCache.java
Modified:
    geronimo/server/trunk/applications/console/geronimo-console-standard/src/main/java/org/apache/geronimo/console/securitymanager/realm/SecurityRealmPortlet.java
    geronimo/server/trunk/modules/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/security/CallerIdentityPasswordCredentialLoginModule.java
    geronimo/server/trunk/modules/geronimo-jmx-remoting/src/test/java/org/apache/geronimo/jmxremoting/AuthenticatorTest.java
    geronimo/server/trunk/modules/geronimo-openejb/src/main/java/org/apache/geronimo/openejb/OpenejbRemoteLoginModule.java
    geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/ConfiguredIdentityNamedUsernamePasswordLoginModule.java
    geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/NamedUPCredentialLoginModule.java
    geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/SubjectRegistrationLoginModule.java
    geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/UPCredentialLoginModule.java
    geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificateChainLoginModule.java
    geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificatePropertiesFileLoginModule.java
    geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/FileAuditLoginModule.java
    geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPasswordCredentialLoginModule.java
    geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPropertiesFileMappedPasswordCredentialLoginModule.java
    geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java
    geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java
    geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/RepeatedFailureLockoutLoginModule.java
    geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java

Modified: geronimo/server/trunk/applications/console/geronimo-console-standard/src/main/java/org/apache/geronimo/console/securitymanager/realm/SecurityRealmPortlet.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/applications/console/geronimo-console-standard/src/main/java/org/apache/geronimo/console/securitymanager/realm/SecurityRealmPortlet.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/applications/console/geronimo-console-standard/src/main/java/org/apache/geronimo/console/securitymanager/realm/SecurityRealmPortlet.java (original)
+++ geronimo/server/trunk/applications/console/geronimo-console-standard/src/main/java/org/apache/geronimo/console/securitymanager/realm/SecurityRealmPortlet.java Mon Aug 13 18:21:55 2007
@@ -57,7 +57,6 @@
 import javax.portlet.RenderResponse;
 import javax.portlet.WindowState;
 import javax.security.auth.Subject;
-import javax.security.auth.login.AppConfigurationEntry;
 import javax.security.auth.spi.LoginModule;
 import javax.xml.namespace.QName;
 
@@ -91,10 +90,10 @@
 import org.apache.geronimo.management.geronimo.JCAManagedConnectionFactory;
 import org.apache.geronimo.security.jaas.JaasLoginModuleChain;
 import org.apache.geronimo.security.jaas.JaasLoginModuleUse;
-import org.apache.geronimo.security.jaas.LoginModuleSettings;
-import org.apache.geronimo.security.jaas.NamedUPCredentialLoginModule;
 import org.apache.geronimo.security.jaas.LoginModuleControlFlag;
 import org.apache.geronimo.security.jaas.LoginModuleControlFlagEditor;
+import org.apache.geronimo.security.jaas.LoginModuleSettings;
+import org.apache.geronimo.security.jaas.NamedUPCredentialLoginModule;
 import org.apache.geronimo.security.realm.SecurityRealm;
 import org.apache.geronimo.security.realm.providers.FileAuditLoginModule;
 import org.apache.geronimo.security.realm.providers.GeronimoPasswordCredentialLoginModule;

Modified: geronimo/server/trunk/modules/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/security/CallerIdentityPasswordCredentialLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/security/CallerIdentityPasswordCredentialLoginModule.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/modules/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/security/CallerIdentityPasswordCredentialLoginModule.java (original)
+++ geronimo/server/trunk/modules/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/security/CallerIdentityPasswordCredentialLoginModule.java Mon Aug 13 18:21:55 2007
@@ -32,7 +32,12 @@
 import javax.security.auth.spi.LoginModule;
 
 /**
+ * CallerIdentityPasswordCredentialLoginModule uses the username and password from the CallbackHandler
+ * and a ManagedConnectionFactory from the Options to construct a j2ca PasswordCredential that can be
+ * used for j2ca container managed security.
  *
+ * This login module does not check credentials so it should never be able to cause a login to succeed.
+ * Therefore the lifecycle methods must return false to indicate success or throw a LoginException to indicate failure.
  *
  * @version $Rev$ $Date$
  *
@@ -84,19 +89,19 @@
         PasswordCredential passwordCredential = new PasswordCredential(userName, password);
         passwordCredential.setManagedConnectionFactory(managedConnectionFactory);
         subject.getPrivateCredentials().add(passwordCredential);
-        return true;
+        return false;
     }
 
     public boolean abort() throws LoginException {
         userName = null;
         password = null;
-        return true;
+        return false;
     }
 
     public boolean logout() throws LoginException {
         subject = null;
         userName = null;
         password = null;
-        return true;
+        return false;
     }
 }

Modified: geronimo/server/trunk/modules/geronimo-jmx-remoting/src/test/java/org/apache/geronimo/jmxremoting/AuthenticatorTest.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-jmx-remoting/src/test/java/org/apache/geronimo/jmxremoting/AuthenticatorTest.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/modules/geronimo-jmx-remoting/src/test/java/org/apache/geronimo/jmxremoting/AuthenticatorTest.java (original)
+++ geronimo/server/trunk/modules/geronimo-jmx-remoting/src/test/java/org/apache/geronimo/jmxremoting/AuthenticatorTest.java Mon Aug 13 18:21:55 2007
@@ -29,6 +29,7 @@
 import javax.security.auth.login.AppConfigurationEntry;
 import javax.security.auth.login.Configuration;
 import javax.security.auth.login.LoginException;
+import javax.security.auth.login.FailedLoginException;
 import javax.security.auth.spi.LoginModule;
 
 import junit.framework.TestCase;
@@ -109,13 +110,13 @@
                 username = nameCallback.getName();
                 String password = (String) options.get(username);
                 if (password == null) {
-                    return false;
+                    throw new FailedLoginException();
                 }
                 return password.equals(new String(passwordCallback.getPassword()));
             } catch (java.io.IOException e) {
-                return false;
+                throw new FailedLoginException();
             } catch (UnsupportedCallbackException e) {
-                return false;
+                throw new FailedLoginException();
             }
         }
 
@@ -125,7 +126,7 @@
         }
 
         public boolean logout() throws LoginException {
-            return false;
+            return true;
         }
 
         public boolean abort() throws LoginException {

Modified: geronimo/server/trunk/modules/geronimo-openejb/src/main/java/org/apache/geronimo/openejb/OpenejbRemoteLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-openejb/src/main/java/org/apache/geronimo/openejb/OpenejbRemoteLoginModule.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/modules/geronimo-openejb/src/main/java/org/apache/geronimo/openejb/OpenejbRemoteLoginModule.java (original)
+++ geronimo/server/trunk/modules/geronimo-openejb/src/main/java/org/apache/geronimo/openejb/OpenejbRemoteLoginModule.java Mon Aug 13 18:21:55 2007
@@ -38,7 +38,16 @@
 import org.apache.geronimo.security.SubjectId;
 
 /**
- * @version $Rev:$ $Date:$
+ * OpenejbRemoteLoginModule uses the openejb protocol to communicate with the server to be used for ejbs and try to
+ * login on that server. If login succeeds an identity token is added to the private credentials of the Subject
+ * that can be used on further calls to identify the client.  Note this should only be used on secure networks or
+ * with secured communication with openejb, as sniffing the identity token gives you all the permissions of the user you
+ * sniffed.
+ *
+ * This login module checks security credentials so the lifecycle methods must return true to indicate success
+ * or throw LoginException to indicate failure.
+ *
+ * @version $Rev$ $Date$
  */
 public class OpenejbRemoteLoginModule implements LoginModule {
     private static final String SECURITY_REALM_KEY = "org.apache.geronimo.openejb.OpenejbRemoteLoginModule.RemoteSecurityRealm";
@@ -83,6 +92,6 @@
 
     public boolean logout() throws LoginException {
         //TODO what?
-        return false;
+        return true;
     }
 }

Modified: geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/ConfiguredIdentityNamedUsernamePasswordLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/ConfiguredIdentityNamedUsernamePasswordLoginModule.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/ConfiguredIdentityNamedUsernamePasswordLoginModule.java (original)
+++ geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/ConfiguredIdentityNamedUsernamePasswordLoginModule.java Mon Aug 13 18:21:55 2007
@@ -25,6 +25,18 @@
 import javax.security.auth.callback.CallbackHandler;
 
 /**
+ * ConfiguredIdentityNamedUsernamePasswordLoginModule adds a geronimo-specific NamedUsernamePasswordCredential
+ * to the subject constructed from the configured username, password, and credential name.  This is useful in
+ * supplying fixed credentials to e.g. web service calls.
+ *
+ * Note that this places passwords to external services in configuration information.  It may be more appropriate
+ * to use the GeronimoPropertiesFileMappedPasswordCredentialLoginModule or a run-as subject with a
+ * NamedUPCredentialLoginModule although the latter solution may put a credential in a
+ * credential store configuration.
+ *
+ * This login module does not check credentials so it should never be able to cause a login to succeed.
+ * Therefore the lifecycle methods must return false to indicate success or throw a LoginException to indicate failure.
+ *
  * @version $Rev$ $Date$
  */
 public class ConfiguredIdentityNamedUsernamePasswordLoginModule implements LoginModule {
@@ -44,7 +56,7 @@
     }
 
     public boolean login() throws LoginException {
-        return true;
+        return false;
     }
 
     public boolean commit() throws LoginException {
@@ -56,7 +68,7 @@
         if (namedUsernamePasswordCredential != null && !pvtCreds.contains(namedUsernamePasswordCredential)) {
             pvtCreds.add(namedUsernamePasswordCredential);
         }
-        return true;
+        return false;
     }
 
     public boolean abort() throws LoginException {
@@ -65,7 +77,7 @@
 
     public boolean logout() throws LoginException {
         if (namedUsernamePasswordCredential == null) {
-            return true;
+            return false;
         }
 
         Set pvtCreds = subject.getPrivateCredentials(UsernamePasswordCredential.class);
@@ -80,6 +92,6 @@
         }
         namedUsernamePasswordCredential = null;
 
-        return true;
+        return false;
     }
 }

Modified: geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/NamedUPCredentialLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/NamedUPCredentialLoginModule.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/NamedUPCredentialLoginModule.java (original)
+++ geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/NamedUPCredentialLoginModule.java Mon Aug 13 18:21:55 2007
@@ -36,6 +36,9 @@
  * If either the username or password is not passed in the callback handler,
  * then the credential is not placed into the Subject.
  *
+ * This login module does not check credentials so it should never be able to cause a login to succeed.
+ * Therefore the lifecycle methods must return false to indicate success or throw a LoginException to indicate failure.
+ *
  * @version $Revision$ $Date$
  */
 public class NamedUPCredentialLoginModule implements LoginModule {
@@ -47,23 +50,11 @@
     private CallbackHandler callbackHandler;
     private NamedUsernamePasswordCredential nupCredential;
 
-    public boolean abort() throws LoginException {
-
-        return logout();
-    }
-
-    public boolean commit() throws LoginException {
-
-        if (subject.isReadOnly()) {
-            throw new LoginException("Subject is ReadOnly");
-        }
-
-        Set pvtCreds = subject.getPrivateCredentials();
-        if (nupCredential != null && !pvtCreds.contains(nupCredential)) {
-            pvtCreds.add(nupCredential);
-        }
+    public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) {
 
-        return true;
+        this.subject = subject;
+        this.callbackHandler = callbackHandler;
+        this.name = (String) options.get(CREDENTIAL_NAME);
     }
 
     public boolean login() throws LoginException {
@@ -83,16 +74,35 @@
         String username = ((NameCallback) callbacks[0]).getName();
         char[] password = ((PasswordCallback) callbacks[1]).getPassword();
 
-        if (username == null || password == null) return true;
+        if (username == null || password == null) return false;
 
         nupCredential = new NamedUsernamePasswordCredential(username, password, name);
 
-        return true;
+        return false;
+    }
+
+    public boolean commit() throws LoginException {
+
+        if (subject.isReadOnly()) {
+            throw new LoginException("Subject is ReadOnly");
+        }
+
+        Set pvtCreds = subject.getPrivateCredentials();
+        if (nupCredential != null && !pvtCreds.contains(nupCredential)) {
+            pvtCreds.add(nupCredential);
+        }
+
+        return false;
+    }
+
+    public boolean abort() throws LoginException {
+
+        return logout();
     }
 
     public boolean logout() throws LoginException {
 
-        if (nupCredential == null) return true;
+        if (nupCredential == null) return false;
 
         Set pvtCreds = subject.getPrivateCredentials(NamedUsernamePasswordCredential.class);
         if (pvtCreds.contains(nupCredential)) {
@@ -106,13 +116,7 @@
         }
         nupCredential = null;
 
-        return true;
+        return false;
     }
 
-    public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) {
-
-        this.subject = subject;
-        this.callbackHandler = callbackHandler;
-        this.name = (String) options.get(CREDENTIAL_NAME);
-    }
 }

Modified: geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/SubjectRegistrationLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/SubjectRegistrationLoginModule.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/SubjectRegistrationLoginModule.java (original)
+++ geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/SubjectRegistrationLoginModule.java Mon Aug 13 18:21:55 2007
@@ -32,7 +32,12 @@
 import org.apache.geronimo.security.IdentificationPrincipal;
 
 /**
- * @version $Rev:$ $Date:$
+ * SubjectRegistrationLoginModule registers the Subject with geronimo and adds an identification principal.
+ *
+ * This login module does not check credentials so it should never be able to cause a login to succeed.
+ * Therefore the lifecycle methods must return false to indicate success or throw a LoginException to indicate failure.
+ *
+ * @version $Rev$ $Date$
  */
 public class SubjectRegistrationLoginModule implements LoginModule {
 
@@ -43,22 +48,22 @@
     }
 
     public boolean login() throws LoginException {
-        return true;
+        return false;
     }
 
     public boolean commit() throws LoginException {
         SubjectId id = ContextManager.registerSubject(subject);
         IdentificationPrincipal principal = new IdentificationPrincipal(id);
         subject.getPrincipals().add(principal);
-        return true;
+        return false;
     }
 
     public boolean abort() throws LoginException {
-        return true;
+        return false;
     }
 
     public boolean logout() throws LoginException {
         ContextManager.unregisterSubject(subject);
-        return true;
+        return false;
     }
 }

Modified: geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/UPCredentialLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/UPCredentialLoginModule.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/UPCredentialLoginModule.java (original)
+++ geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/UPCredentialLoginModule.java Mon Aug 13 18:21:55 2007
@@ -31,11 +31,16 @@
 
 
 /**
+ *
+ *
  * Inserts Username/Password credential into private credentials of Subject.
  * <p/>
  * If either the username or password is not passed in the callback handler,
  * then the credential is not placed into the Subject.
  *
+ * This login module does not check credentials so it should never be able to cause a login to succeed.
+ * Therefore the lifecycle methods must return false to indicate success or throw a LoginException to indicate failure.
+ *
  * @version $Revision$ $Date$
  */
 public class UPCredentialLoginModule implements LoginModule {
@@ -44,23 +49,10 @@
     private CallbackHandler callbackHandler;
     private UsernamePasswordCredential upCredential;
 
-    public boolean abort() throws LoginException {
-
-        return logout();
-    }
-
-    public boolean commit() throws LoginException {
-
-        if (subject.isReadOnly()) {
-            throw new LoginException("Subject is ReadOnly");
-        }
-
-        Set pvtCreds = subject.getPrivateCredentials();
-        if (upCredential != null && !pvtCreds.contains(upCredential)) {
-            pvtCreds.add(upCredential);
-        }
+    public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) {
 
-        return true;
+        this.subject = subject;
+        this.callbackHandler = callbackHandler;
     }
 
     public boolean login() throws LoginException {
@@ -84,7 +76,26 @@
 
         upCredential = new UsernamePasswordCredential(username, password);
 
-        return true;
+        return false;
+    }
+
+    public boolean commit() throws LoginException {
+
+        if (subject.isReadOnly()) {
+            throw new LoginException("Subject is ReadOnly");
+        }
+
+        Set pvtCreds = subject.getPrivateCredentials();
+        if (upCredential != null && !pvtCreds.contains(upCredential)) {
+            pvtCreds.add(upCredential);
+        }
+
+        return false;
+    }
+
+    public boolean abort() throws LoginException {
+
+        return logout();
     }
 
     public boolean logout() throws LoginException {
@@ -103,12 +114,7 @@
         }
         upCredential = null;
 
-        return true;
+        return false;
     }
 
-    public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) {
-
-        this.subject = subject;
-        this.callbackHandler = callbackHandler;
-    }
 }

Modified: geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificateChainLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificateChainLoginModule.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificateChainLoginModule.java (original)
+++ geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificateChainLoginModule.java Mon Aug 13 18:21:55 2007
@@ -26,6 +26,7 @@
 import javax.security.auth.callback.Callback;
 import javax.security.auth.callback.CallbackHandler;
 import javax.security.auth.callback.UnsupportedCallbackException;
+import javax.security.auth.login.FailedLoginException;
 import javax.security.auth.login.LoginException;
 import javax.security.auth.spi.LoginModule;
 import javax.security.auth.x500.X500Principal;
@@ -47,32 +48,22 @@
  * The groupsURI property file should have lines of the form group=token1,token2,...
  * where the tokens were associated to the certificate names in the usersURI properties file.
  *
+ * This login module checks security credentials so the lifecycle methods must return true to indicate success
+ * or throw LoginException to indicate failure.
+ *
  * @version $Rev$ $Date$
  */
 public class CertificateChainLoginModule implements LoginModule {
-    private static Log log = LogFactory.getLog(CertificateChainLoginModule.class);
 
-    Subject subject;
-    CallbackHandler handler;
-    X500Principal principal;
+    private Subject subject;
+    private CallbackHandler handler;
+    private X500Principal principal;
 
     public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) {
         this.subject = subject;
         this.handler = callbackHandler;
-//        try {
-//            Kernel kernel = KernelRegistry.getKernel((String)options.get(JaasLoginModuleUse.KERNEL_LM_OPTION));
-//            ServerInfo serverInfo = (ServerInfo) options.get(JaasLoginModuleUse.SERVERINFO_LM_OPTION);
-//            URI usersURI = new URI((String)options.get(CREDENTIALS_URI));
-//            URI groupsURI = new URI((String)options.get(GROUPS_URI));
-//            loadProperties(kernel, serverInfo, usersURI, groupsURI);
-//        } catch (Exception e) {
-//            log.error(e);
-//            throw new IllegalArgumentException("Unable to configure properties file login module: "+e);
-//        }
     }
 
-
-
     public boolean login() throws LoginException {
         Callback[] callbacks = new Callback[1];
 
@@ -87,10 +78,10 @@
         assert callbacks.length == 1;
         Certificate[] certificateChain = ((CertificateChainCallback)callbacks[0]).getCertificateChain();
         if (certificateChain == null || certificateChain.length == 0) {
-            return false;
+            throw new FailedLoginException();
         }
         if (!(certificateChain[0] instanceof X509Certificate)) {
-            return false;
+            throw new FailedLoginException();
         }
         //TODO actually validate chain
         principal = ((X509Certificate)certificateChain[0]).getSubjectX500Principal();
@@ -117,14 +108,6 @@
         principal = null;
 
         return true;
-    }
-
-    /**
-     * Gets the names of all principal classes that may be populated into
-     * a Subject.
-     */
-    public String[] getPrincipalClassNames() {
-        return new String[]{GeronimoUserPrincipal.class.getName()};
     }
 
 }

Modified: geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificatePropertiesFileLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificatePropertiesFileLoginModule.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificatePropertiesFileLoginModule.java (original)
+++ geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificatePropertiesFileLoginModule.java Mon Aug 13 18:21:55 2007
@@ -58,18 +58,22 @@
  * The groupsURI property file should have lines of the form group=token1,token2,...
  * where the tokens were associated to the certificate names in the usersURI properties file.
  *
+ * This login module checks security credentials so the lifecycle methods must return true to indicate success
+ * or throw LoginException to indicate failure.
+ *
  * @version $Rev$ $Date$
  */
 public class CertificatePropertiesFileLoginModule implements LoginModule {
+    private static Log log = LogFactory.getLog(CertificatePropertiesFileLoginModule.class);
     public final static String USERS_URI = "usersURI";
     public final static String GROUPS_URI = "groupsURI";
-    private static Log log = LogFactory.getLog(CertificatePropertiesFileLoginModule.class);
+
     private final Map users = new HashMap();
     final Map groups = new HashMap();
 
-    Subject subject;
-    CallbackHandler handler;
-    X500Principal principal;
+    private Subject subject;
+    private CallbackHandler handler;
+    private X500Principal principal;
 
     public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) {
         this.subject = subject;
@@ -142,7 +146,7 @@
         assert callbacks.length == 1;
         X509Certificate certificate = ((CertificateCallback)callbacks[0]).getCertificate();
         if (certificate == null) {
-            return false;
+            throw new FailedLoginException();
         }
         principal = certificate.getSubjectX500Principal();
 
@@ -188,29 +192,4 @@
         return true;
     }
 
-    /**
-     * Gets the names of all principal classes that may be populated into
-     * a Subject.
-     */
-    public String[] getPrincipalClassNames() {
-        return new String[]{GeronimoUserPrincipal.class.getName(), GeronimoGroupPrincipal.class.getName()};
-    }
-
-    /**
-     * Gets a list of all the principals of a particular type (identified by
-     * the principal class).  These are available for manual role mapping.
-     */
-    public String[] getPrincipalsOfClass(String className) {
-        Collection s;
-        if(className.equals(GeronimoGroupPrincipal.class.getName())) {
-            s = groups.keySet();
-        } else if(className.equals(GeronimoUserPrincipal.class.getName())) {
-            s = users.values();
-        } else if(className.equals(X500Principal.class.getName())) {
-            s = users.keySet();
-        } else {
-            throw new IllegalArgumentException("No such principal class "+className);
-        }
-        return (String[]) s.toArray(new String[s.size()]);
-    }
 }

Modified: geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/FileAuditLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/FileAuditLoginModule.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/FileAuditLoginModule.java (original)
+++ geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/FileAuditLoginModule.java Mon Aug 13 18:21:55 2007
@@ -47,6 +47,9 @@
  * To enable this login module, set your primary login module to REQUIRED or
  * OPTIONAL, and list this module after it (with any setting).
  *
+ * This login module does not check credentials so it should never be able to cause a login to succeed.
+ * Therefore the lifecycle methods must return false to indicate success or throw a LoginException to indicate failure.
+ *
  * @version $Rev$ $Date$
  */
 public class FileAuditLoginModule implements LoginModule {
@@ -79,7 +82,7 @@
         username = user.getName();
         writeToFile("Authentication attempt");
 
-        return true;
+        return false;
     }
 
     private synchronized void writeToFile(String action) {
@@ -102,7 +105,7 @@
 
     public boolean commit() throws LoginException {
         writeToFile("Authentication succeeded");
-        return true;
+        return false;
     }
 
     public boolean abort() throws LoginException {
@@ -110,12 +113,12 @@
             writeToFile("Authentication failed");
             username = null;
         }
-        return true;
+        return false;
     }
 
     public boolean logout() throws LoginException {
         writeToFile("Explicit logout");
         username = null;
-        return true;
+        return false;
     }
 }

Modified: geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPasswordCredentialLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPasswordCredentialLoginModule.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPasswordCredentialLoginModule.java (original)
+++ geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPasswordCredentialLoginModule.java Mon Aug 13 18:21:55 2007
@@ -29,6 +29,13 @@
 
 
 /**
+ * GeronimoPasswordCredentialLoginModule stores the user name and password in a GeronimoPasswordCredential.
+ * This allows an application to  retrieve the subject through jacc or the geronimo specific ContextManager and
+ * find out what the password was.  I can't think of any other reason to use it right now.
+ *
+ * This login module does not check credentials so it should never be able to cause a login to succeed.
+ * Therefore the lifecycle methods must return false to indicate success or throw a LoginException to indicate failure.
+ *
  * @version $Rev$ $Date$
  */
 public class GeronimoPasswordCredentialLoginModule implements LoginModule {
@@ -51,26 +58,27 @@
         try {
             callbackHandler.handle(callbacks);
         } catch (java.io.IOException e) {
+            throw (LoginException) new LoginException("Could not determine username and password").initCause(e);
         } catch (UnsupportedCallbackException e) {
             throw (LoginException) new LoginException("Unlikely UnsupportedCallbackException").initCause(e);
         }
         geronimoPasswordCredential = new GeronimoPasswordCredential(((NameCallback) callbacks[0]).getName(),
                                                                     ((PasswordCallback) callbacks[1]).getPassword());
-        return true;
+        return false;
     }
 
     public boolean commit() throws LoginException {
         subject.getPrivateCredentials().add(geronimoPasswordCredential);
-        return true;
+        return false;
     }
 
     public boolean abort() throws LoginException {
         geronimoPasswordCredential = null;
-        return true;
+        return false;
     }
 
     public boolean logout() throws LoginException {
         geronimoPasswordCredential = null;
-        return true;
+        return false;
     }
 }

Modified: geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPropertiesFileMappedPasswordCredentialLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPropertiesFileMappedPasswordCredentialLoginModule.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPropertiesFileMappedPasswordCredentialLoginModule.java (original)
+++ geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPropertiesFileMappedPasswordCredentialLoginModule.java Mon Aug 13 18:21:55 2007
@@ -45,6 +45,22 @@
 import org.apache.geronimo.system.serverinfo.ServerInfo;
 
 /**
+ * GeronimoPropertiesFileMappedPasswordCredentialLoginModule adds NamedUsernamePasswordCredentials to the Subject.
+ * The NamedUsernamePasswordCredential are specified in a properties file specified in the options. Each line of the
+ * properties file is of the form:
+ *
+ * username=credentials
+ *
+ * where credentials is a comma-separated list of credentials and a credential is of the form
+ * name:username=password
+ *
+ * Thus a typical line would be:
+ *
+ * whee=foo:bar=baz,foo2:bar2=baz2
+ *
+ * This login module does not check credentials so it should never be able to cause a login to succeed.
+ * Therefore the lifecycle methods must return false to indicate success or throw a LoginException to indicate failure.
+ *
  * @version $Rev$ $Date$
  */
 public class GeronimoPropertiesFileMappedPasswordCredentialLoginModule implements LoginModule {
@@ -103,7 +119,7 @@
         if (unparsedCredentials != null) {
             parseCredentials(unparsedCredentials, passwordCredentials);
         }
-        return true;
+        return false;
     }
 
     void parseCredentials(String unparsedCredentials, Set<NamedUsernamePasswordCredential> passwordCredentials) {
@@ -119,16 +135,16 @@
 
     public boolean commit() throws LoginException {
         subject.getPrivateCredentials().addAll(passwordCredentials);
-        return true;
+        return false;
     }
 
     public boolean abort() throws LoginException {
         passwordCredentials.clear();
-        return true;
+        return false;
     }
 
     public boolean logout() throws LoginException {
         passwordCredentials.clear();
-        return true;
+        return false;
     }
 }

Modified: geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java (original)
+++ geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java Mon Aug 13 18:21:55 2007
@@ -18,16 +18,16 @@
 package org.apache.geronimo.security.realm.providers;
 
 import java.io.IOException;
+import java.security.Principal;
 import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Hashtable;
-import java.util.Iterator;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
 import javax.naming.AuthenticationException;
+import javax.naming.CommunicationException;
 import javax.naming.Context;
 import javax.naming.Name;
 import javax.naming.NameParser;
@@ -37,7 +37,6 @@
 import javax.naming.directory.Attributes;
 import javax.naming.directory.DirContext;
 import javax.naming.directory.InitialDirContext;
-import javax.naming.CommunicationException;
 import javax.naming.directory.SearchControls;
 import javax.naming.directory.SearchResult;
 import javax.security.auth.Subject;
@@ -46,18 +45,20 @@
 import javax.security.auth.callback.NameCallback;
 import javax.security.auth.callback.PasswordCallback;
 import javax.security.auth.callback.UnsupportedCallbackException;
-import javax.security.auth.login.LoginException;
 import javax.security.auth.login.FailedLoginException;
+import javax.security.auth.login.LoginException;
 import javax.security.auth.spi.LoginModule;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
-import org.apache.geronimo.security.realm.providers.GeronimoGroupPrincipal;
-import org.apache.geronimo.security.realm.providers.GeronimoUserPrincipal;
-
 
 /**
+ * LDAPLoginModule is a login module using ldap as an authentication store.
+ * <p/>
+ * This login module checks security credentials so the lifecycle methods must return true to indicate success
+ * or throw LoginException to indicate failure.
+ *
  * @version $Rev$ $Date$
  */
 public class LDAPLoginModule implements LoginModule {
@@ -89,13 +90,8 @@
     private String connectionProtocol;
     private String authentication;
     private String userBase;
-    private String userSearchMatching;
-    private String userPassword;
     private String roleBase;
     private String roleName;
-    private String roleSearchMatching;
-    private String userSearchSubtree;
-    private String roleSearchSubtree;
     private String userRoleName;
 
     private String cbUsername;
@@ -109,7 +105,7 @@
     private boolean userSearchSubtreeBool = false;
     private boolean roleSearchSubtreeBool = false;
 
-    Set groups = new HashSet();
+    Set<Principal> groups = new HashSet<Principal>();
 
     public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) {
         this.subject = subject;
@@ -121,17 +117,17 @@
         connectionProtocol = (String) options.get(CONNECTION_PROTOCOL);
         authentication = (String) options.get(AUTHENTICATION);
         userBase = (String) options.get(USER_BASE);
-        userSearchMatching = (String) options.get(USER_SEARCH_MATCHING);
-        userSearchSubtree = (String) options.get(USER_SEARCH_SUBTREE);
+        String userSearchMatching = (String) options.get(USER_SEARCH_MATCHING);
+        String userSearchSubtree = (String) options.get(USER_SEARCH_SUBTREE);
         roleBase = (String) options.get(ROLE_BASE);
         roleName = (String) options.get(ROLE_NAME);
-        roleSearchMatching = (String) options.get(ROLE_SEARCH_MATCHING);
-        roleSearchSubtree = (String) options.get(ROLE_SEARCH_SUBTREE);
+        String roleSearchMatching = (String) options.get(ROLE_SEARCH_MATCHING);
+        String roleSearchSubtree = (String) options.get(ROLE_SEARCH_SUBTREE);
         userRoleName = (String) options.get(USER_ROLE_NAME);
         userSearchMatchingFormat = new MessageFormat(userSearchMatching);
         roleSearchMatchingFormat = new MessageFormat(roleSearchMatching);
-        userSearchSubtreeBool = new Boolean(userSearchSubtree).booleanValue();
-        roleSearchSubtreeBool = new Boolean(roleSearchSubtree).booleanValue();
+        userSearchSubtreeBool = Boolean.valueOf(userSearchSubtree);
+        roleSearchSubtreeBool = Boolean.valueOf(roleSearchSubtree);
     }
 
     public boolean login() throws LoginException {
@@ -150,13 +146,13 @@
         cbPassword = new String(((PasswordCallback) callbacks[1]).getPassword());
 
         if (cbUsername == null || "".equals(cbUsername)
-            || cbPassword == null || "".equals(cbPassword)) {
-            return false;
+                || cbPassword == null || "".equals(cbPassword)) {
+            throw new FailedLoginException();
         }
 
         try {
             boolean result = authenticate(cbUsername, cbPassword);
-            if(!result) {
+            if (!result) {
                 throw new FailedLoginException();
             } else {
                 return true;
@@ -166,20 +162,10 @@
         }
     }
 
-    public boolean logout() throws LoginException {
-        cbUsername = null;
-        cbPassword = null;
-        //todo: should remove principals added by commit
-        return true;
-    }
-
     public boolean commit() throws LoginException {
-        Set principals = subject.getPrincipals();
+        Set<Principal> principals = subject.getPrincipals();
         principals.add(new GeronimoUserPrincipal(cbUsername));
-        Iterator iter = groups.iterator();
-        while (iter.hasNext()) {
-            principals.add(iter.next());
-        }
+        principals.addAll(groups);
         return true;
     }
 
@@ -189,6 +175,13 @@
         return true;
     }
 
+    public boolean logout() throws LoginException {
+        cbUsername = null;
+        cbPassword = null;
+        //todo: should remove principals added by commit
+        return true;
+    }
+
     protected void close(DirContext context) {
         try {
             context.close();
@@ -199,8 +192,7 @@
 
     protected boolean authenticate(String username, String password) throws Exception {
 
-        DirContext context = null;
-        context = open();
+        DirContext context = open();
 
         try {
 
@@ -213,12 +205,12 @@
             }
 
             //setup attributes
-            ArrayList list = new ArrayList();
-            if (userRoleName != null) {
-                list.add(userRoleName);
+            String[] attribs;
+            if (userRoleName == null) {
+                attribs = new String[]{};
+            } else {
+                attribs = new String[]{userRoleName};
             }
-            String[] attribs = new String[list.size()];
-            list.toArray(attribs);
             constraints.setReturningAttributes(attribs);
 
 
@@ -245,38 +237,33 @@
             if (attrs == null) {
                 return false;
             }
-            ArrayList roles = null;
+            ArrayList<String> roles = null;
             if (userRoleName != null) {
                 roles = addAttributeValues(userRoleName, attrs, roles);
             }
 
             //check the credentials by binding to server
-            if (bindUser(context, dn, password)) {
-                //if authenticated add more roles
-                roles = getRoles(context, dn, username, roles);
-                for (int i = 0; i < roles.size(); i++) {
-                    groups.add(new GeronimoGroupPrincipal((String) roles.get(i)));
-                }
-            } else {
-                return false;
+            bindUser(context, dn, password);
+            //if authenticated add more roles
+            roles = getRoles(context, dn, username, roles);
+            for (String role : roles) {
+                groups.add(new GeronimoGroupPrincipal(role));
             }
         } catch (CommunicationException e) {
-
+            close(context);
+            throw (LoginException) new FailedLoginException().initCause(e);
         } catch (NamingException e) {
-            if (context != null) {
-                close(context);
-            }
-            return false;
+            close(context);
+            throw (LoginException) new FailedLoginException().initCause(e);
         }
 
 
         return true;
     }
 
-    protected ArrayList getRoles(DirContext context, String dn, String username, ArrayList currentRoles) throws NamingException {
-        ArrayList list = currentRoles;
+    protected ArrayList<String> getRoles(DirContext context, String dn, String username, ArrayList<String> list) throws NamingException {
         if (list == null) {
-            list = new ArrayList();
+            list = new ArrayList<String>();
         }
         if (roleName == null || "".equals(roleName)) {
             return list;
@@ -309,19 +296,19 @@
         for (int i = 0; i < inputString.length(); i++) {
             char c = inputString.charAt(i);
             switch (c) {
-                case '\\':
+                case'\\':
                     buf.append("\\5c");
                     break;
-                case '*':
+                case'*':
                     buf.append("\\2a");
                     break;
-                case '(':
+                case'(':
                     buf.append("\\28");
                     break;
-                case ')':
+                case')':
                     buf.append("\\29");
                     break;
-                case '\0':
+                case'\0':
                     buf.append("\\00");
                     break;
                 default:
@@ -332,69 +319,41 @@
         return buf.toString();
     }
 
-    protected boolean bindUser(DirContext context, String dn, String password) throws NamingException {
-        boolean isValid = false;
-        Attributes attr;
+    protected void bindUser(DirContext context, String dn, String password) throws NamingException, FailedLoginException {
 
         context.addToEnvironment(Context.SECURITY_PRINCIPAL, dn);
         context.addToEnvironment(Context.SECURITY_CREDENTIALS, password);
         try {
-            attr = context.getAttributes("", null);
-            isValid = true;
+            context.getAttributes("", null);
         } catch (AuthenticationException e) {
-            isValid = false;
             log.debug("Authentication failed for dn=" + dn);
-        }
-
-        if (connectionUsername != null) {
-            context.addToEnvironment(Context.SECURITY_PRINCIPAL,
-                                     connectionUsername);
-        } else {
-            context.removeFromEnvironment(Context.SECURITY_PRINCIPAL);
-        }
+            throw new FailedLoginException();
+        } finally {
 
-        if (connectionPassword != null) {
-            context.addToEnvironment(Context.SECURITY_CREDENTIALS,
-                                     connectionPassword);
-        } else {
-            context.removeFromEnvironment(Context.SECURITY_CREDENTIALS);
-        }
-
-        return isValid;
-    }
-
-    private String getAttributeValue(String attrId, Attributes attrs)
-            throws NamingException {
-
-        if (attrId == null || attrs == null) {
-            return null;
-        }
+            if (connectionUsername != null) {
+                context.addToEnvironment(Context.SECURITY_PRINCIPAL,
+                        connectionUsername);
+            } else {
+                context.removeFromEnvironment(Context.SECURITY_PRINCIPAL);
+            }
 
-        Attribute attr = attrs.get(attrId);
-        if (attr == null) {
-            return (null);
-        }
-        Object value = attr.get();
-        if (value == null) {
-            return (null);
-        }
-        String valueString = null;
-        if (value instanceof byte[]) {
-            valueString = new String((byte[]) value);
-        } else {
-            valueString = value.toString();
+            if (connectionPassword != null) {
+                context.addToEnvironment(Context.SECURITY_CREDENTIALS,
+                        connectionPassword);
+            } else {
+                context.removeFromEnvironment(Context.SECURITY_CREDENTIALS);
+            }
         }
-        return valueString;
     }
 
-    private ArrayList addAttributeValues(String attrId, Attributes attrs, ArrayList values)
+    private ArrayList<String> addAttributeValues(String attrId, Attributes attrs, ArrayList<String> values)
             throws NamingException {
 
         if (attrId == null || attrs == null) {
             return values;
         }
         if (values == null) {
-            values = new ArrayList();
+            values = new ArrayList<String>();
         }
         Attribute attr = attrs.get(attrId);
         if (attr == null) {
@@ -414,7 +373,7 @@
         }
 
         try {
-            Hashtable env = new Hashtable();
+            Hashtable<String, String> env = new Hashtable<String, String>();
             env.put(Context.INITIAL_CONTEXT_FACTORY, initialContextFactory);
             if (connectionUsername != null || !"".equals(connectionUsername)) {
                 env.put(Context.SECURITY_PRINCIPAL, connectionUsername);

Modified: geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java (original)
+++ geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java Mon Aug 13 18:21:55 2007
@@ -17,43 +17,46 @@
 
 package org.apache.geronimo.security.realm.providers;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-import org.apache.geronimo.common.GeronimoSecurityException;
-import org.apache.geronimo.security.jaas.JaasLoginModuleUse;
-import org.apache.geronimo.system.serverinfo.ServerInfo;
-import org.apache.geronimo.util.encoders.Base64;
-import org.apache.geronimo.util.encoders.HexTranslator;
-import org.apache.geronimo.util.SimpleEncryption;
-
-
-import javax.security.auth.Subject;
-import javax.security.auth.callback.Callback;
-import javax.security.auth.callback.CallbackHandler;
-import javax.security.auth.callback.NameCallback;
-import javax.security.auth.callback.PasswordCallback;
-import javax.security.auth.callback.UnsupportedCallbackException;
-import javax.security.auth.login.FailedLoginException;
-import javax.security.auth.login.LoginException;
-import javax.security.auth.spi.LoginModule;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URI;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
 
+import javax.security.auth.Subject;
+import javax.security.auth.callback.Callback;
+import javax.security.auth.callback.CallbackHandler;
+import javax.security.auth.callback.NameCallback;
+import javax.security.auth.callback.PasswordCallback;
+import javax.security.auth.callback.UnsupportedCallbackException;
+import javax.security.auth.login.FailedLoginException;
+import javax.security.auth.login.LoginException;
+import javax.security.auth.spi.LoginModule;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.geronimo.common.GeronimoSecurityException;
+import org.apache.geronimo.security.jaas.JaasLoginModuleUse;
+import org.apache.geronimo.system.serverinfo.ServerInfo;
+import org.apache.geronimo.util.SimpleEncryption;
+import org.apache.geronimo.util.encoders.Base64;
+import org.apache.geronimo.util.encoders.HexTranslator;
+
 
 /**
  * A LoginModule that reads a list of credentials and group from files on disk.  The
  * files should be formatted using standard Java properties syntax.  Expects
  * to be run by a GenericSecurityRealm (doesn't work on its own).
+ * <p/>
+ * This login module checks security credentials so the lifecycle methods must return true to indicate success
+ * or throw LoginException to indicate failure.
  *
  * @version $Rev$ $Date$
  */
@@ -65,47 +68,50 @@
 
     private static Log log = LogFactory.getLog(PropertiesFileLoginModule.class);
     final Properties users = new Properties();
-    final Map groups = new HashMap();
+    final Map<String, Set<String>> groups = new HashMap<String, Set<String>>();
     private String digest;
-    private String encoding;    
+    private String encoding;
 
-    Subject subject;
-    CallbackHandler handler;
-    String username;
-    String password;
+    private Subject subject;
+    private CallbackHandler handler;
+    private String username;
+    private String password;
 
     public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) {
         this.subject = subject;
         this.handler = callbackHandler;
         try {
             ServerInfo serverInfo = (ServerInfo) options.get(JaasLoginModuleUse.SERVERINFO_LM_OPTION);
-            final String users = (String)options.get(USERS_URI);
-            final String groups = (String)options.get(GROUPS_URI);
+            final String users = (String) options.get(USERS_URI);
+            final String groups = (String) options.get(GROUPS_URI);
             digest = (String) options.get(DIGEST);
             encoding = (String) options.get(ENCODING);
 
-            if(digest != null && !digest.equals("")) {
+            if (digest != null && !digest.equals("")) {
                 // Check if the digest algorithm is available
                 try {
                     MessageDigest.getInstance(digest);
-                } catch(NoSuchAlgorithmException e) {
-                    log.error("Initialization failed. Digest algorithm "+digest+" is not available.", e);
-                    throw new IllegalArgumentException("Unable to configure properties file login module: "+e.getMessage(), e);
+                } catch (NoSuchAlgorithmException e) {
+                    log.error("Initialization failed. Digest algorithm " + digest + " is not available.", e);
+                    throw new IllegalArgumentException(
+                            "Unable to configure properties file login module: " + e.getMessage(), e);
                 }
-                if(encoding != null && !"hex".equalsIgnoreCase(encoding) && !"base64".equalsIgnoreCase(encoding)) {
-                    log.error("Initialization failed. Digest Encoding "+encoding+" is not supported.");
-                    throw new IllegalArgumentException("Unable to configure properties file login module. Digest Encoding "+encoding+" not supported.");
+                if (encoding != null && !"hex".equalsIgnoreCase(encoding) && !"base64".equalsIgnoreCase(encoding)) {
+                    log.error("Initialization failed. Digest Encoding " + encoding + " is not supported.");
+                    throw new IllegalArgumentException(
+                            "Unable to configure properties file login module. Digest Encoding " + encoding + " not supported.");
                 }
             }
-            if(users == null || groups == null) {
-                throw new IllegalArgumentException("Both "+USERS_URI+" and "+GROUPS_URI+" must be provided!");
+            if (users == null || groups == null) {
+                throw new IllegalArgumentException("Both " + USERS_URI + " and " + GROUPS_URI + " must be provided!");
             }
             URI usersURI = new URI(users);
             URI groupsURI = new URI(groups);
             loadProperties(serverInfo, usersURI, groupsURI);
         } catch (Exception e) {
             log.error("Initialization failed", e);
-            throw new IllegalArgumentException("Unable to configure properties file login module: "+e.getMessage(), e);
+            throw new IllegalArgumentException("Unable to configure properties file login module: " + e.getMessage(),
+                    e);
         }
     }
 
@@ -128,14 +134,13 @@
                 String groupName = (String) e.nextElement();
                 String[] userList = ((String) temp.get(groupName)).split(",");
 
-                Set userset = (Set) groups.get(groupName);
+                Set<String> userset = groups.get(groupName);
                 if (userset == null) {
-                    userset = new HashSet();
+                    userset = new HashSet<String>();
                     groups.put(groupName, userset);
                 }
-
-                for (int i = 0; i < userList.length; i++) {
-                    userset.add(userList[i]);
+                for (String user : userList) {
+                    userset.add(user);
                 }
             }
 
@@ -160,8 +165,8 @@
         }
         assert callbacks.length == 2;
         username = ((NameCallback) callbacks[0]).getName();
-        if(username == null || username.equals("")) {
-            return false;
+        if (username == null || username.equals("")) {
+            throw new FailedLoginException();
         }
         String realPassword = users.getProperty(username);
         // Decrypt the password if needed, so we can compare it with the supplied one
@@ -172,26 +177,21 @@
         }
         char[] entered = ((PasswordCallback) callbacks[1]).getPassword();
         password = entered == null ? null : new String(entered);
-        boolean result = (realPassword == null && password == null) ||
-                (realPassword != null && password != null && checkPassword(realPassword, password));
-        if(!result) {
+        if (!checkPassword(realPassword, password)) {
             throw new FailedLoginException();
         }
         return true;
     }
 
     public boolean commit() throws LoginException {
-        Set principals = subject.getPrincipals();
+        Set<Principal> principals = subject.getPrincipals();
 
         principals.add(new GeronimoUserPrincipal(username));
 
-        Iterator e = groups.keySet().iterator();
-        while (e.hasNext()) {
-            String groupName = (String) e.next();
-            Set users = (Set) groups.get(groupName);
-            Iterator iter = users.iterator();
-            while (iter.hasNext()) {
-                String user = (String) iter.next();
+        for (Map.Entry<String, Set<String>> entry : groups.entrySet()) {
+            String groupName = entry.getKey();
+            Set<String> users = entry.getValue();
+            for (String user : users) {
                 if (username.equals(user)) {
                     principals.add(new GeronimoGroupPrincipal(groupName));
                     break;
@@ -217,37 +217,22 @@
     }
 
     /**
-     * Gets the names of all principal classes that may be populated into
-     * a Subject.
-     */
-    public String[] getPrincipalClassNames() {
-        return new String[]{GeronimoUserPrincipal.class.getName(), GeronimoGroupPrincipal.class.getName()};
-    }
-
-    /**
-     * Gets a list of all the principals of a particular type (identified by
-     * the principal class).  These are available for manual role mapping.
-     */
-    public String[] getPrincipalsOfClass(String className) {
-        Set s;
-        if(className.equals(GeronimoGroupPrincipal.class.getName())) {
-            s = groups.keySet();
-        } else if(className.equals(GeronimoUserPrincipal.class.getName())) {
-            s = users.keySet();
-        } else {
-            throw new IllegalArgumentException("No such principal class "+className);
-        }
-        return (String[]) s.toArray(new String[s.size()]);
-    }
-
-    /**
      * This method checks if the provided password is correct.  The original password may have been digested.
-     * @param real      Original password in digested form if applicable
-     * @param provided  User provided password in clear text
+     *
+     * @param real     Original password in digested form if applicable
+     * @param provided User provided password in clear text
      * @return true     If the password is correct
      */
-    private boolean checkPassword(String real, String provided){
-        if(digest == null || digest.equals("")) {
+    private boolean checkPassword(String real, String provided) {
+        if (real == null && provided == null) {
+            return true;
+        }
+        if (real == null || provided == null) {
+            return false;
+        }
+
+        //both non-null
+        if (digest == null || digest.equals("")) {
             // No digest algorithm is used
             return real.equals(provided);
         }
@@ -255,14 +240,14 @@
             // Digest the user provided password
             MessageDigest md = MessageDigest.getInstance(digest);
             byte[] data = md.digest(provided.getBytes());
-            if(encoding == null || "hex".equalsIgnoreCase(encoding)) {
+            if (encoding == null || "hex".equalsIgnoreCase(encoding)) {
                 // Convert bytes to hex digits
                 byte[] hexData = new byte[data.length * 2];
                 HexTranslator ht = new HexTranslator();
                 ht.encode(data, 0, data.length, hexData, 0);
                 // Compare the digested provided password with the actual one
                 return real.equalsIgnoreCase(new String(hexData));
-            } else if("base64".equalsIgnoreCase(encoding)) {
+            } else if ("base64".equalsIgnoreCase(encoding)) {
                 return real.equals(new String(Base64.encode(data)));
             }
         } catch (NoSuchAlgorithmException e) {

Modified: geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/RepeatedFailureLockoutLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/RepeatedFailureLockoutLoginModule.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/RepeatedFailureLockoutLoginModule.java (original)
+++ geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/RepeatedFailureLockoutLoginModule.java Mon Aug 13 18:21:55 2007
@@ -53,13 +53,16 @@
  *                                    failurePeriodSecs.</li>
  * </ul>
  *
+ * This login module does not check credentials so it should never be able to cause a login to succeed.
+ * Therefore the lifecycle methods must return false to indicate success or throw a LoginException to indicate failure.
+ *
  * @version $Rev$ $Date$
  */
 public class RepeatedFailureLockoutLoginModule implements LoginModule {
     public static final String FAILURE_COUNT_OPTION = "failureCount";
     public static final String FAILURE_PERIOD_OPTION = "failurePeriodSecs";
     public static final String LOCKOUT_DURATION_OPTION = "lockoutDurationSecs";
-    private static final HashMap userData = new HashMap();
+    private static final HashMap<String, LoginHistory> userData = new HashMap<String, LoginHistory>();
     private CallbackHandler handler;
     private String username;
     private int failureCount = 5;
@@ -114,23 +117,21 @@
         if(username != null) {
             LoginHistory history;
             synchronized (userData) {
-                history = (LoginHistory) userData.get(username);
+                history = userData.get(username);
             }
             if(history != null && !history.isLoginAllowed(lockoutDuration, failurePeriod, failureCount)) {
                 username = null;
                 throw new FailedLoginException("Maximum login failures exceeded; try again later");
             }
-        } else {
-            return false; // it's a fake login, ignore this module
         }
-        return true;
+        return false;
     }
 
     /**
      * This module does nothing if a login succeeds.
      */
     public boolean commit() throws LoginException {
-        return username != null;
+        return false;
     }
 
     /**
@@ -141,7 +142,7 @@
         if(username != null) { //work around initial "fake" login
             LoginHistory history;
             synchronized (userData) {
-                history = (LoginHistory) userData.get(username);
+                history = userData.get(username);
                 if(history == null) {
                     history = new LoginHistory(username);
                     userData.put(username, history);
@@ -149,11 +150,9 @@
             }
             history.addFailure();
             username = null;
-            return true;
-        } else {
-            return false;
         }
-    }
+        return false;
+     }
 
     /**
      * This module does nothing on a logout.
@@ -161,7 +160,7 @@
     public boolean logout() throws LoginException {
         username = null;
         handler = null;
-        return true;
+        return false;
     }
 
     /**
@@ -169,8 +168,10 @@
      * status and expiry, etc.
      */
     private static class LoginHistory implements Serializable {
+        private static final long serialVersionUID = 7792298296084531182L;
+
         private String user;
-        private LinkedList data = new LinkedList();
+        private LinkedList<Long> data = new LinkedList<Long>();
         private long lockExpires = -1;
 
         public LoginHistory(String user) {
@@ -192,7 +193,7 @@
                 return false;
             }
             if(data.size() >= maxFailures) {
-                lockExpires = ((Long)data.getLast()).longValue() + lockoutLengthMillis;
+                lockExpires = data.getLast() + lockoutLengthMillis;
                 if(lockExpires > now) {
                     return false;
                 }
@@ -204,7 +205,7 @@
          * Notes that a failure occured.
          */
         public synchronized void addFailure() {
-            data.add(new Long(System.currentTimeMillis()));
+            data.add(System.currentTimeMillis());
         }
 
         /**
@@ -214,7 +215,7 @@
         public synchronized void cleanup(long ignoreOlderThan) {
             for (Iterator it = data.iterator(); it.hasNext();) {
                 Long time = (Long) it.next();
-                if(time.longValue() < ignoreOlderThan) {
+                if(time < ignoreOlderThan) {
                     it.remove();
                 } else {
                     break;

Modified: geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java?view=diff&rev=565599&r1=565598&r2=565599
==============================================================================
--- geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java (original)
+++ geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java Mon Aug 13 18:21:55 2007
@@ -20,16 +20,17 @@
 import java.io.IOException;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
 import java.sql.Connection;
 import java.sql.Driver;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
+
 import javax.security.auth.Subject;
 import javax.security.auth.callback.Callback;
 import javax.security.auth.callback.CallbackHandler;
@@ -58,21 +59,24 @@
 /**
  * A login module that loads security information from a SQL database.  Expects
  * to be run by a GenericSecurityRealm (doesn't work on its own).
- * <p>
+ * <p/>
  * This requires database connectivity information (either 1: a dataSourceName and
  * optional dataSourceApplication or 2: a JDBC driver, URL, username, and password)
  * and 2 SQL queries.
- * <p>
+ * <p/>
  * The userSelect query should return 2 values, the username and the password in
  * that order.  It should include one PreparedStatement parameter (a ?) which
  * will be filled in with the username.  In other words, the query should look
  * like: <tt>SELECT user, password FROM credentials WHERE username=?</tt>
- * <p>
+ * <p/>
  * The groupSelect query should return 2 values, the username and the group name in
  * that order (but it may return multiple rows, one per group).  It should include
  * one PreparedStatement parameter (a ?) which will be filled in with the username.
  * In other words, the query should look like:
  * <tt>SELECT user, role FROM user_roles WHERE username=?</tt>
+ * <p/>
+ * This login module checks security credentials so the lifecycle methods must return true to indicate success
+ * or throw LoginException to indicate failure.
  *
  * @version $Rev$ $Date$
  */
@@ -95,13 +99,13 @@
     private String userSelect;
     private String groupSelect;
     private String digest;
-    private String encoding;    
+    private String encoding;
 
     private Subject subject;
     private CallbackHandler handler;
     private String cbUsername;
     private String cbPassword;
-    private final Set groups = new HashSet();
+    private final Set<Principal> groups = new HashSet<Principal>();
 
     public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) {
         this.subject = subject;
@@ -111,41 +115,41 @@
 
         digest = (String) options.get(DIGEST);
         encoding = (String) options.get(ENCODING);
-        if(digest != null && !digest.equals("")) {
+        if (digest != null && !digest.equals("")) {
             // Check if the digest algorithm is available
             try {
                 MessageDigest.getInstance(digest);
-            } catch(NoSuchAlgorithmException e) {
-                log.error("Initialization failed. Digest algorithm "+digest+" is not available.", e);
-                throw new IllegalArgumentException("Unable to configure SQL login module: "+e.getMessage(), e);
+            } catch (NoSuchAlgorithmException e) {
+                log.error("Initialization failed. Digest algorithm " + digest + " is not available.", e);
+                throw new IllegalArgumentException("Unable to configure SQL login module: " + e.getMessage(), e);
             }
-            if(encoding != null && !"hex".equalsIgnoreCase(encoding) && !"base64".equalsIgnoreCase(encoding)) {
-                log.error("Initialization failed. Digest Encoding "+encoding+" is not supported.");
-                throw new IllegalArgumentException("Unable to configure SQL login module. Digest Encoding "+encoding+" not supported.");
+            if (encoding != null && !"hex".equalsIgnoreCase(encoding) && !"base64".equalsIgnoreCase(encoding)) {
+                log.error("Initialization failed. Digest Encoding " + encoding + " is not supported.");
+                throw new IllegalArgumentException(
+                        "Unable to configure SQL login module. Digest Encoding " + encoding + " not supported.");
             }
         }
 
         String dataSourceName = (String) options.get(DATABASE_POOL_NAME);
-        if(dataSourceName != null) {
+        if (dataSourceName != null) {
             dataSourceName = dataSourceName.trim();
             String dataSourceAppName = (String) options.get(DATABASE_POOL_APP_NAME);
-            if(dataSourceAppName == null || dataSourceAppName.trim().equals("")) {
+            if (dataSourceAppName == null || dataSourceAppName.trim().equals("")) {
                 dataSourceAppName = "null";
             } else {
                 dataSourceAppName = dataSourceAppName.trim();
             }
             String kernelName = (String) options.get(JaasLoginModuleUse.KERNEL_NAME_LM_OPTION);
             Kernel kernel = KernelRegistry.getKernel(kernelName);
-            Set set = kernel.listGBeans(new AbstractNameQuery(JCAManagedConnectionFactory.class.getName()));
+            Set<AbstractName> set = kernel.listGBeans(new AbstractNameQuery(JCAManagedConnectionFactory.class.getName()));
             JCAManagedConnectionFactory factory;
-            for (Iterator it = set.iterator(); it.hasNext();) {
-                AbstractName name = (AbstractName) it.next();
-                if(name.getName().get(NameFactory.J2EE_APPLICATION).equals(dataSourceAppName) &&
-                    name.getName().get(NameFactory.J2EE_NAME).equals(dataSourceName)) {
+            for (AbstractName name : set) {
+                if (name.getName().get(NameFactory.J2EE_APPLICATION).equals(dataSourceAppName) &&
+                        name.getName().get(NameFactory.J2EE_NAME).equals(dataSourceName)) {
                     try {
                         factory = (JCAManagedConnectionFactory) kernel.getGBean(name);
                         String type = factory.getConnectionFactoryInterface();
-                        if(type.equals(DataSource.class.getName())) {
+                        if (type.equals(DataSource.class.getName())) {
                             this.factory = factory;
                             break;
                         }
@@ -157,19 +161,23 @@
         } else {
             connectionURL = (String) options.get(CONNECTION_URL);
             properties = new Properties();
-            if(options.get(USER) != null) {
+            if (options.get(USER) != null) {
                 properties.put("user", options.get(USER));
             }
-            if(options.get(PASSWORD) != null) {
+            if (options.get(PASSWORD) != null) {
                 properties.put("password", options.get(PASSWORD));
             }
             ClassLoader cl = (ClassLoader) options.get(JaasLoginModuleUse.CLASSLOADER_LM_OPTION);
             try {
                 driver = (Driver) cl.loadClass((String) options.get(DRIVER)).newInstance();
             } catch (ClassNotFoundException e) {
-                throw new IllegalArgumentException("Driver class " + options.get(DRIVER) + " is not available.  Perhaps you need to add it as a dependency in your deployment plan?", e);
+                throw new IllegalArgumentException("Driver class " + options.get(
+                        DRIVER) + " is not available.  Perhaps you need to add it as a dependency in your deployment plan?",
+                        e);
             } catch (Exception e) {
-                throw new IllegalArgumentException("Unable to load, instantiate, register driver " + options.get(DRIVER) + ": " + e.getMessage(), e);
+                throw new IllegalArgumentException(
+                        "Unable to load, instantiate, register driver " + options.get(DRIVER) + ": " + e.getMessage(),
+                        e);
             }
         }
     }
@@ -189,15 +197,14 @@
         assert callbacks.length == 2;
         cbUsername = ((NameCallback) callbacks[0]).getName();
         if (cbUsername == null || cbUsername.equals("")) {
-            return false;
+            throw new FailedLoginException();
         }
         char[] provided = ((PasswordCallback) callbacks[1]).getPassword();
         cbPassword = provided == null ? null : new String(provided);
 
-        boolean found = false;
         try {
             Connection conn;
-            if(factory != null) {
+            if (factory != null) {
                 DataSource ds = (DataSource) factory.getConnectionFactory();
                 conn = ds.getConnection();
             } else {
@@ -208,8 +215,8 @@
                 PreparedStatement statement = conn.prepareStatement(userSelect);
                 try {
                     int count = countParameters(userSelect);
-                    for(int i=0; i<count; i++) {
-                        statement.setObject(i+1, cbUsername);
+                    for (int i = 0; i < count; i++) {
+                        statement.setObject(i + 1, cbUsername);
                     }
                     ResultSet result = statement.executeQuery();
 
@@ -219,8 +226,9 @@
                             String userPassword = result.getString(2);
 
                             if (cbUsername.equals(userName)) {
-                                found = (cbPassword == null && userPassword == null) ||
-                                        (cbPassword != null && userPassword != null && checkPassword(userPassword, cbPassword));
+                                if (!checkPassword(userPassword, cbPassword)) {
+                                    throw new FailedLoginException();
+                                }
                                 break;
                             }
                         }
@@ -231,15 +239,11 @@
                     statement.close();
                 }
 
-                if (!found) {
-                    throw new FailedLoginException();
-                }
-
                 statement = conn.prepareStatement(groupSelect);
                 try {
                     int count = countParameters(groupSelect);
-                    for(int i=0; i<count; i++) {
-                        statement.setObject(i+1, cbUsername);
+                    for (int i = 0; i < count; i++) {
+                        statement.setObject(i + 1, cbUsername);
                     }
                     ResultSet result = statement.executeQuery();
 
@@ -271,12 +275,9 @@
     }
 
     public boolean commit() throws LoginException {
-        Set principals = subject.getPrincipals();
+        Set<Principal> principals = subject.getPrincipals();
         principals.add(new GeronimoUserPrincipal(cbUsername));
-        Iterator iter = groups.iterator();
-        while (iter.hasNext()) {
-            principals.add(iter.next());
-        }
+        principals.addAll(groups);
 
         return true;
     }
@@ -298,7 +299,7 @@
     private static int countParameters(String sql) {
         int count = 0;
         int pos = -1;
-        while((pos = sql.indexOf('?', pos+1)) != -1) {
+        while ((pos = sql.indexOf('?', pos + 1)) != -1) {
             ++count;
         }
         return count;
@@ -306,12 +307,21 @@
 
     /**
      * This method checks if the provided password is correct.  The original password may have been digested.
-     * @param real      Original password in digested form if applicable
-     * @param provided  User provided password in clear text
+     *
+     * @param real     Original password in digested form if applicable
+     * @param provided User provided password in clear text
      * @return true     If the password is correct
      */
-    private boolean checkPassword(String real, String provided){
-        if(digest == null || digest.equals("")) {
+    private boolean checkPassword(String real, String provided) {
+        if (real == null && provided == null) {
+            return true;
+        }
+        if (real == null || provided == null) {
+            return false;
+        }
+
+        //both are non-null
+        if (digest == null || digest.equals("")) {
             // No digest algorithm is used
             return real.equals(provided);
         }
@@ -319,14 +329,14 @@
             // Digest the user provided password
             MessageDigest md = MessageDigest.getInstance(digest);
             byte[] data = md.digest(provided.getBytes());
-            if(encoding == null || "hex".equalsIgnoreCase(encoding)) {
+            if (encoding == null || "hex".equalsIgnoreCase(encoding)) {
                 // Convert bytes to hex digits
                 byte[] hexData = new byte[data.length * 2];
                 HexTranslator ht = new HexTranslator();
                 ht.encode(data, 0, data.length, hexData, 0);
                 // Compare the digested provided password with the actual one
                 return real.equalsIgnoreCase(new String(hexData));
-            } else if("base64".equalsIgnoreCase(encoding)) {
+            } else if ("base64".equalsIgnoreCase(encoding)) {
                 return real.equals(new String(Base64.encode(data)));
             }
         } catch (NoSuchAlgorithmException e) {



Mime
View raw message