geronimo-scm mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ke...@apache.org
Subject svn commit: r565936 [1/2] - in /geronimo/server/branches/2.0: 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/s...
Date Tue, 14 Aug 2007 21:54:52 GMT
Author: kevan
Date: Tue Aug 14 14:54:50 2007
New Revision: 565936

URL: http://svn.apache.org/viewvc?view=rev&rev=565936
Log:
GERONIMO-3407 GERONIMO-3404 Merge fixes from trunk. Fix security problems in LoginModules related to null or null string username checks. Also, merge changes to SubjectRegistrationLoginModule processing

Removed:
    geronimo/server/branches/2.0/applications/console/geronimo-console-core/src/main/java/org/apache/geronimo/console/core/security/PropertiesFileLoginModuleNoCache.java
Modified:
    geronimo/server/branches/2.0/applications/console/geronimo-console-standard/src/main/java/org/apache/geronimo/console/securitymanager/realm/SecurityRealmPortlet.java
    geronimo/server/branches/2.0/modules/geronimo-client/src/main/java/org/apache/geronimo/client/AppClientContainer.java
    geronimo/server/branches/2.0/modules/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/security/CallerIdentityPasswordCredentialLoginModule.java
    geronimo/server/branches/2.0/modules/geronimo-corba/src/main/java/org/apache/geronimo/corba/security/config/tss/TSSGSSUPMechConfig.java
    geronimo/server/branches/2.0/modules/geronimo-jetty6/src/main/java/org/apache/geronimo/jetty6/InternalJAASJettyRealm.java
    geronimo/server/branches/2.0/modules/geronimo-jmx-remoting/src/main/java/org/apache/geronimo/jmxremoting/Authenticator.java
    geronimo/server/branches/2.0/modules/geronimo-jmx-remoting/src/test/java/org/apache/geronimo/jmxremoting/AuthenticatorTest.java
    geronimo/server/branches/2.0/modules/geronimo-openejb/src/main/java/org/apache/geronimo/openejb/GeronimoSecurityService.java
    geronimo/server/branches/2.0/modules/geronimo-openejb/src/main/java/org/apache/geronimo/openejb/OpenejbRemoteLoginModule.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/ContextManager.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/credentialstore/SimpleCredentialStoreImpl.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/ConfiguredIdentityNamedUsernamePasswordLoginModule.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/NamedUPCredentialLoginModule.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/SubjectRegistrationLoginModule.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/UPCredentialLoginModule.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/GenericSecurityRealm.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificateChainLoginModule.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificatePropertiesFileLoginModule.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/FileAuditLoginModule.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPasswordCredentialLoginModule.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPropertiesFileMappedPasswordCredentialLoginModule.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/RepeatedFailureLockoutLoginModule.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/ConfigurationEntryTest.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginKerberosTest.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileTest.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLTest.java
    geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/TimeoutTest.java
    geronimo/server/branches/2.0/modules/geronimo-tomcat6/src/main/java/org/apache/geronimo/tomcat/realm/TomcatGeronimoRealm.java
    geronimo/server/branches/2.0/modules/geronimo-tomcat6/src/main/java/org/apache/geronimo/tomcat/realm/TomcatJAASRealm.java
    geronimo/server/branches/2.0/testsuite/enterprise-testsuite/sec-tests/sec-ejb/src/main/java/org/apache/geronimo/itest/TestLoginModule.java

Modified: geronimo/server/branches/2.0/applications/console/geronimo-console-standard/src/main/java/org/apache/geronimo/console/securitymanager/realm/SecurityRealmPortlet.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/applications/console/geronimo-console-standard/src/main/java/org/apache/geronimo/console/securitymanager/realm/SecurityRealmPortlet.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/applications/console/geronimo-console-standard/src/main/java/org/apache/geronimo/console/securitymanager/realm/SecurityRealmPortlet.java (original)
+++ geronimo/server/branches/2.0/applications/console/geronimo-console-standard/src/main/java/org/apache/geronimo/console/securitymanager/realm/SecurityRealmPortlet.java Tue Aug 14 14:54:50 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/branches/2.0/modules/geronimo-client/src/main/java/org/apache/geronimo/client/AppClientContainer.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-client/src/main/java/org/apache/geronimo/client/AppClientContainer.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-client/src/main/java/org/apache/geronimo/client/AppClientContainer.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-client/src/main/java/org/apache/geronimo/client/AppClientContainer.java Tue Aug 14 14:54:50 2007
@@ -130,13 +130,7 @@
 
             if (callbackHandlerClass != null) {
                 callbackHandler = (CallbackHandler) holder.newInstance(callbackHandlerClass, classLoader, componentContext);
-                loginContext = new LoginContext(realmName, callbackHandler);
-                try {
-                    loginContext.login();
-                } catch (LoginException e) {
-                    loginContext = null;
-                    throw e;
-                }
+                loginContext = ContextManager.login(realmName, callbackHandler);
                 clientSubject = loginContext.getSubject();
             }
             ContextManager.setCallers(clientSubject, clientSubject);
@@ -220,7 +214,7 @@
             holder.destroyInstance(callbackHandler);
         }
         if (loginContext != null) {
-            loginContext.logout();
+            ContextManager.logout(loginContext);
         }
         jndiContext.stopClient(appClientModuleName);
     }

Modified: geronimo/server/branches/2.0/modules/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/security/CallerIdentityPasswordCredentialLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/security/CallerIdentityPasswordCredentialLoginModule.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/security/CallerIdentityPasswordCredentialLoginModule.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/security/CallerIdentityPasswordCredentialLoginModule.java Tue Aug 14 14:54:50 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$
  *
@@ -76,27 +81,30 @@
         resourcePrincipalName = ((NameCallback) callbacks[0]).getName();
         userName = ((NameCallback) callbacks[0]).getName();
         password = ((PasswordCallback) callbacks[1]).getPassword();
-        return resourcePrincipalName != null && userName != null && password != null;
+        return false;
     }
 
     public boolean commit() throws LoginException {
+        if (resourcePrincipalName == null || userName == null || password == null) {
+            return false;
+        }
         subject.getPrincipals().add(new ResourcePrincipal(resourcePrincipalName));
         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/branches/2.0/modules/geronimo-corba/src/main/java/org/apache/geronimo/corba/security/config/tss/TSSGSSUPMechConfig.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-corba/src/main/java/org/apache/geronimo/corba/security/config/tss/TSSGSSUPMechConfig.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-corba/src/main/java/org/apache/geronimo/corba/security/config/tss/TSSGSSUPMechConfig.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-corba/src/main/java/org/apache/geronimo/corba/security/config/tss/TSSGSSUPMechConfig.java Tue Aug 14 14:54:50 2007
@@ -105,11 +105,10 @@
                 if (!targetName.equals(tokenTargetName)) throw new SASException(2);
                 String userName = Util.extractUserNameFromScopedName(token.username);
 
-                LoginContext context = new LoginContext(tokenTargetName,
+                LoginContext context = ContextManager.login(tokenTargetName,
                         new UsernamePasswordCallback(userName,
                                 new String(token.password, "UTF8").toCharArray()));
-                context.login();
-                result = ContextManager.getServerSideSubject(context.getSubject());
+                result = context.getSubject();
             }
         } catch (UnsupportedEncodingException e) {
             throw new SASException(1, e);

Modified: geronimo/server/branches/2.0/modules/geronimo-jetty6/src/main/java/org/apache/geronimo/jetty6/InternalJAASJettyRealm.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-jetty6/src/main/java/org/apache/geronimo/jetty6/InternalJAASJettyRealm.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-jetty6/src/main/java/org/apache/geronimo/jetty6/InternalJAASJettyRealm.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-jetty6/src/main/java/org/apache/geronimo/jetty6/InternalJAASJettyRealm.java Tue Aug 14 14:54:50 2007
@@ -88,12 +88,10 @@
                 }
 
                 //set up the login context
-                LoginContext loginContext = new LoginContext(securityRealmName, callbackHandler);
-                loginContext.login();
+                LoginContext loginContext = ContextManager.login(securityRealmName, callbackHandler);
                 callbackHandler.clear();
 
                 Subject subject = ContextManager.getServerSideSubject(loginContext.getSubject());
-                //TODO use the run-as subject as nextCaller
                 ContextManager.setCallers(subject, subject);
 
                 //login success

Modified: geronimo/server/branches/2.0/modules/geronimo-jmx-remoting/src/main/java/org/apache/geronimo/jmxremoting/Authenticator.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-jmx-remoting/src/main/java/org/apache/geronimo/jmxremoting/Authenticator.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-jmx-remoting/src/main/java/org/apache/geronimo/jmxremoting/Authenticator.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-jmx-remoting/src/main/java/org/apache/geronimo/jmxremoting/Authenticator.java Tue Aug 14 14:54:50 2007
@@ -35,12 +35,13 @@
 public class Authenticator implements JMXAuthenticator, NotificationListener {
     private final String configName;
     private final ClassLoader cl;
-    private ThreadLocal threadContext = new ThreadLocal();
-    private Map contextMap = Collections.synchronizedMap(new HashMap());
+    private ThreadLocal<LoginContext> threadContext = new ThreadLocal<LoginContext>();
+    private Map<String, LoginContext> contextMap = Collections.synchronizedMap(new HashMap<String, LoginContext>());
 
     /**
      * Constructor indicating which JAAS Application Configuration Entry to use.
      * @param configName the JAAS config name
+     * @param cl classloader to use as TCCL for operations
      */
     public Authenticator(String configName, ClassLoader cl) {
         this.configName = configName;
@@ -48,7 +49,7 @@
     }
 
     public Subject authenticate(Object o) throws SecurityException {
-        if (o instanceof String[] == false) {
+        if (!(o instanceof String[])) {
             throw new IllegalArgumentException("Expected String[2], got " + o == null ? null : o.getClass().getName());
         }
         String[] params = (String[]) o;
@@ -61,6 +62,8 @@
         Credentials credentials = new Credentials(params[0], params[1]);
         try {
             thread.setContextClassLoader(cl);
+            //TODO consider using ContextManager for login and checking a permission against the ACC
+            //to do e.g. deployments.
             LoginContext context = new LoginContext(configName, credentials);
             context.login();
             threadContext.set(context);
@@ -80,11 +83,11 @@
             String type = cxNotification.getType();
             String connectionId = cxNotification.getConnectionId();
             if (JMXConnectionNotification.OPENED.equals(type)) {
-                LoginContext context = (LoginContext) threadContext.get();
+                LoginContext context = threadContext.get();
                 threadContext.set(null);
                 contextMap.put(connectionId, context);
             } else {
-                LoginContext context = (LoginContext) contextMap.remove(connectionId);
+                LoginContext context = contextMap.remove(connectionId);
                 if (context != null) {
                     try {
                         context.logout();

Modified: geronimo/server/branches/2.0/modules/geronimo-jmx-remoting/src/test/java/org/apache/geronimo/jmxremoting/AuthenticatorTest.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-jmx-remoting/src/test/java/org/apache/geronimo/jmxremoting/AuthenticatorTest.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-jmx-remoting/src/test/java/org/apache/geronimo/jmxremoting/AuthenticatorTest.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-jmx-remoting/src/test/java/org/apache/geronimo/jmxremoting/AuthenticatorTest.java Tue Aug 14 14:54:50 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,16 @@
                 username = nameCallback.getName();
                 String password = (String) options.get(username);
                 if (password == null) {
-                    return false;
+                    throw new FailedLoginException();
                 }
-                return password.equals(new String(passwordCallback.getPassword()));
+                if (password.equals(new String(passwordCallback.getPassword()))) {
+                    return true;
+                }
+                throw new FailedLoginException();
             } catch (java.io.IOException e) {
-                return false;
+                throw new FailedLoginException();
             } catch (UnsupportedCallbackException e) {
-                return false;
+                throw new FailedLoginException();
             }
         }
 
@@ -125,7 +129,7 @@
         }
 
         public boolean logout() throws LoginException {
-            return false;
+            return true;
         }
 
         public boolean abort() throws LoginException {

Modified: geronimo/server/branches/2.0/modules/geronimo-openejb/src/main/java/org/apache/geronimo/openejb/GeronimoSecurityService.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-openejb/src/main/java/org/apache/geronimo/openejb/GeronimoSecurityService.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-openejb/src/main/java/org/apache/geronimo/openejb/GeronimoSecurityService.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-openejb/src/main/java/org/apache/geronimo/openejb/GeronimoSecurityService.java Tue Aug 14 14:54:50 2007
@@ -49,18 +49,16 @@
     }
 
     public Object login(String securityRealm, String user, String pass) throws LoginException {
-        LoginContext context = new LoginContext(securityRealm, new UsernamePasswordCallbackHandler(user, pass));
-        context.login();
+        LoginContext context = ContextManager.login(securityRealm, new UsernamePasswordCallbackHandler(user, pass));
 
         Subject subject = context.getSubject();
-        SubjectId subjectId = ContextManager.registerSubject(subject);
-        return subjectId;
+        return ContextManager.getSubjectId(subject);
     }
 
-    public void logout(Object securityIdentity) {
-        Subject subject = ContextManager.getRegisteredSubject((SubjectId) securityIdentity);
-        ContextManager.unregisterSubject(subject);
-    }
+//    public void logout(Object securityIdentity) {
+//        Subject subject = ContextManager.getRegisteredSubject((SubjectId) securityIdentity);
+//        ContextManager.unregisterSubject(subject);
+//    }
 
     public void associate(Object securityIdentity) throws LoginException {
         if (securityIdentity == null) {

Modified: geronimo/server/branches/2.0/modules/geronimo-openejb/src/main/java/org/apache/geronimo/openejb/OpenejbRemoteLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-openejb/src/main/java/org/apache/geronimo/openejb/OpenejbRemoteLoginModule.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-openejb/src/main/java/org/apache/geronimo/openejb/OpenejbRemoteLoginModule.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-openejb/src/main/java/org/apache/geronimo/openejb/OpenejbRemoteLoginModule.java Tue Aug 14 14:54:50 2007
@@ -38,7 +38,20 @@
 import org.apache.geronimo.security.SubjectId;
 
 /**
- * @version $Rev:$ $Date:$
+<<<<<<< .working
+ * @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$
+>>>>>>> .merge-right.r565912
  */
 public class OpenejbRemoteLoginModule implements LoginModule {
     private static final String SECURITY_REALM_KEY = "org.apache.geronimo.openejb.OpenejbRemoteLoginModule.RemoteSecurityRealm";
@@ -83,6 +96,6 @@
 
     public boolean logout() throws LoginException {
         //TODO what?
-        return false;
+        return true;
     }
 }

Modified: geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/ContextManager.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/ContextManager.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/ContextManager.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/ContextManager.java Tue Aug 14 14:54:50 2007
@@ -35,6 +35,9 @@
 import javax.crypto.SecretKey;
 import javax.crypto.spec.SecretKeySpec;
 import javax.security.auth.Subject;
+import javax.security.auth.callback.CallbackHandler;
+import javax.security.auth.login.LoginContext;
+import javax.security.auth.login.LoginException;
 import javax.security.jacc.EJBRoleRefPermission;
 
 import org.apache.geronimo.security.realm.providers.GeronimoCallerPrincipal;
@@ -66,6 +69,22 @@
     static {
         EMPTY.setReadOnly();
         registerSubject(EMPTY);
+    }
+
+    public static LoginContext login(String realm, CallbackHandler callbackHandler) throws LoginException {
+        Subject subject = new Subject();
+        LoginContext loginContext = new LoginContext(realm, subject, callbackHandler);
+        loginContext.login();
+        SubjectId id = ContextManager.registerSubject(subject);
+        IdentificationPrincipal principal = new IdentificationPrincipal(id);
+        subject.getPrincipals().add(principal);
+        return loginContext;
+    }
+
+    public static void logout(LoginContext loginContext) throws LoginException {
+        Subject subject = loginContext.getSubject();
+        ContextManager.unregisterSubject(subject);
+        loginContext.logout();
     }
 
 

Modified: geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/credentialstore/SimpleCredentialStoreImpl.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/credentialstore/SimpleCredentialStoreImpl.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/credentialstore/SimpleCredentialStoreImpl.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/credentialstore/SimpleCredentialStoreImpl.java Tue Aug 14 14:54:50 2007
@@ -36,7 +36,7 @@
 import org.apache.geronimo.security.ContextManager;
 
 /**
- * @version $Rev:$ $Date:$
+ * @version $Rev$ $Date$
  */
 public class SimpleCredentialStoreImpl implements CredentialStore {
 
@@ -78,8 +78,7 @@
         if (callbackInfos == null) {
             throw new LoginException("Unknown id: " + id + " in realm: " + realm);
         }
-        Subject subject = new Subject();
-        LoginContext loginContext = new LoginContext(realm, subject, new CallbackHandler() {
+        LoginContext loginContext = ContextManager.login(realm, new CallbackHandler() {
 
             public void handle(Callback[] callbacks) throws UnsupportedCallbackException {
                 for (Callback callback: callbacks) {
@@ -91,8 +90,7 @@
                 }
             }
         });
-        loginContext.login();
-        return ContextManager.getServerSideSubject(subject);
+        return loginContext.getSubject();
     }
 
     public void addEntry(String realm, String id, Map<String, SingleCallbackHandler> callbackInfos) {

Modified: geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/ConfiguredIdentityNamedUsernamePasswordLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/ConfiguredIdentityNamedUsernamePasswordLoginModule.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/ConfiguredIdentityNamedUsernamePasswordLoginModule.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/ConfiguredIdentityNamedUsernamePasswordLoginModule.java Tue Aug 14 14:54:50 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/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/NamedUPCredentialLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/NamedUPCredentialLoginModule.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/NamedUPCredentialLoginModule.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/NamedUPCredentialLoginModule.java Tue Aug 14 14:54:50 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/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/SubjectRegistrationLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/SubjectRegistrationLoginModule.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/SubjectRegistrationLoginModule.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/SubjectRegistrationLoginModule.java Tue Aug 14 14:54:50 2007
@@ -32,7 +32,16 @@
 import org.apache.geronimo.security.IdentificationPrincipal;
 
 /**
- * @version $Rev:$ $Date:$
+<<<<<<< .working
+ * @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$
+>>>>>>> .merge-right.r565912
  */
 public class SubjectRegistrationLoginModule implements LoginModule {
 
@@ -43,22 +52,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/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/UPCredentialLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/UPCredentialLoginModule.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/UPCredentialLoginModule.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/jaas/UPCredentialLoginModule.java Tue Aug 14 14:54:50 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 {
@@ -80,11 +72,30 @@
         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;
 
         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/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/GenericSecurityRealm.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/GenericSecurityRealm.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/GenericSecurityRealm.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/GenericSecurityRealm.java Tue Aug 14 14:54:50 2007
@@ -17,7 +17,6 @@
 package org.apache.geronimo.security.realm;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Properties;
@@ -32,7 +31,6 @@
 import org.apache.geronimo.security.jaas.ConfigurationEntryFactory;
 import org.apache.geronimo.security.jaas.JaasLoginModuleChain;
 import org.apache.geronimo.security.jaas.JaasLoginModuleUse;
-import org.apache.geronimo.security.jaas.SubjectRegistrationLoginModule;
 import org.apache.geronimo.system.serverinfo.ServerInfo;
 
 
@@ -81,7 +79,6 @@
 
         if (loginModuleUse != null) {
             loginModuleUse.configure(domainNames, loginModuleConfigurations, realmName, kernel, serverInfo, classLoader);
-            loginModuleConfigurations.add(new AppConfigurationEntry(SubjectRegistrationLoginModule.class.getName(), AppConfigurationEntry.LoginModuleControlFlag.REQUIRED, Collections.<String, Object>emptyMap()));
         }
 
         domains = domainNames.toArray(new String[domainNames.size()]);

Modified: geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificateChainLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificateChainLoginModule.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificateChainLoginModule.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificateChainLoginModule.java Tue Aug 14 14:54:50 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/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificatePropertiesFileLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificatePropertiesFileLoginModule.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificatePropertiesFileLoginModule.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/CertificatePropertiesFileLoginModule.java Tue Aug 14 14:54:50 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/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/FileAuditLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/FileAuditLoginModule.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/FileAuditLoginModule.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/FileAuditLoginModule.java Tue Aug 14 14:54:50 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/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPasswordCredentialLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPasswordCredentialLoginModule.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPasswordCredentialLoginModule.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPasswordCredentialLoginModule.java Tue Aug 14 14:54:50 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/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPropertiesFileMappedPasswordCredentialLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPropertiesFileMappedPasswordCredentialLoginModule.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPropertiesFileMappedPasswordCredentialLoginModule.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/GeronimoPropertiesFileMappedPasswordCredentialLoginModule.java Tue Aug 14 14:54:50 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/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java Tue Aug 14 14:54:50 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/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java Tue Aug 14 14:54:50 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/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/RepeatedFailureLockoutLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/RepeatedFailureLockoutLoginModule.java?view=diff&rev=565936&r1=565935&r2=565936
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/RepeatedFailureLockoutLoginModule.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/RepeatedFailureLockoutLoginModule.java Tue Aug 14 14:54:50 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;



Mime
View raw message