You are viewing a plain text version of this content. The canonical link for it is here.
Posted to scm@geronimo.apache.org by va...@apache.org on 2007/10/31 19:26:29 UTC

svn commit: r590777 - in /geronimo/server: branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/ branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/ trunk/framework/module...

Author: vamsic007
Date: Wed Oct 31 11:26:28 2007
New Revision: 590777

URL: http://svn.apache.org/viewvc?rev=590777&view=rev
Log:
**GERONIMO-3570 Review SQLLoginModule
 o LoginModule should not add principals when login fails.  Added a test to detect the same.
 o Other changes to bring SQLLoginModule in line with http://java.sun.com/j2se/1.5.0/docs/guide/security/jaas/JAASLMDevGuide.html
**: This fix can use a thorough review.

Added:
    geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java   (with props)
    geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java   (with props)
    geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java   (with props)
    geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java   (with props)
    geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java   (with props)
    geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java   (with props)
Modified:
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java
    geronimo/server/trunk/framework/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java
    geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java

Modified: geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java?rev=590777&r1=590776&r2=590777&view=diff
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java (original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java Wed Oct 31 11:26:28 2007
@@ -26,7 +26,10 @@
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
@@ -92,6 +95,9 @@
     public final static String DATABASE_POOL_APP_NAME = "dataSourceApplication";
     public final static String DIGEST = "digest";
     public final static String ENCODING = "encoding";
+    public final static List<String> supportedOptions = Arrays.asList(USER_SELECT, GROUP_SELECT, CONNECTION_URL,
+            USER, PASSWORD, DRIVER, DATABASE_POOL_NAME, DATABASE_POOL_APP_NAME, DIGEST, ENCODING);
+
     private String connectionURL;
     private Properties properties;
     private Driver driver;
@@ -101,15 +107,22 @@
     private String digest;
     private String encoding;
 
+    private boolean loginSucceeded;
     private Subject subject;
     private CallbackHandler handler;
     private String cbUsername;
     private String cbPassword;
-    private final Set<Principal> groups = new HashSet<Principal>();
+    private final Set<String> groups = new HashSet<String>();
+    private final Set<Principal> allPrincipals = new HashSet<Principal>();
 
     public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) {
         this.subject = subject;
         this.handler = callbackHandler;
+        for(Object option: options.keySet()) {
+            if(!supportedOptions.contains(option)) {
+                log.warn("Ignoring option: "+option+". Not supported.");
+            }
+        }
         userSelect = (String) options.get(USER_SELECT);
         groupSelect = (String) options.get(GROUP_SELECT);
 
@@ -182,7 +195,13 @@
         }
     }
 
+    /**
+     * This LoginModule is not to be ignored.  So, this method should never return false.
+     * @return true if authentication succeeds, or throw a LoginException such as FailedLoginException
+     *         if authentication fails
+     */
     public boolean login() throws LoginException {
+        loginSucceeded = false;
         Callback[] callbacks = new Callback[2];
 
         callbacks[0] = new NameCallback("User name");
@@ -259,7 +278,7 @@
                             String groupName = result.getString(2);
 
                             if (cbUsername.equals(userName)) {
-                                groups.add(new GeronimoGroupPrincipal(groupName));
+                                groups.add(groupName);
                             }
                         }
                     } finally {
@@ -272,35 +291,75 @@
                 conn.close();
             }
         } catch (LoginException e) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
             throw e;
         } catch (SQLException sqle) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
             throw (LoginException) new LoginException("SQL error").initCause(sqle);
         } catch (Exception e) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
             throw (LoginException) new LoginException("Could not access datasource").initCause(e);
         }
 
+        loginSucceeded = true;
         return true;
     }
 
+    /*
+     * @exception LoginException if login succeeded but commit failed.
+     *
+     * @return true if login succeeded and commit succeeded, or false if login failed but commit succeeded.
+     */
     public boolean commit() throws LoginException {
-        Set<Principal> principals = subject.getPrincipals();
-        principals.add(new GeronimoUserPrincipal(cbUsername));
-        principals.addAll(groups);
-
-        return true;
-    }
+        if(loginSucceeded) {
+            if(cbUsername != null) {
+                allPrincipals.add(new GeronimoUserPrincipal(cbUsername));
+            }
+            for(String group: groups) {
+                allPrincipals.add(new GeronimoGroupPrincipal(group));
+            }
+            subject.getPrincipals().addAll(allPrincipals);
+        }
 
-    public boolean abort() throws LoginException {
+        // Clear out the private state
         cbUsername = null;
         cbPassword = null;
+        groups.retainAll(Collections.EMPTY_SET);
 
-        return true;
+        return loginSucceeded;
+    }
+
+    public boolean abort() throws LoginException {
+        if(loginSucceeded) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
+            allPrincipals.retainAll(Collections.EMPTY_SET);
+        }
+        return loginSucceeded;
     }
 
     public boolean logout() throws LoginException {
+        // Clear out the private state
+        loginSucceeded = false;
         cbUsername = null;
         cbPassword = null;
-        //todo: should remove principals put in by commit
+        groups.retainAll(Collections.EMPTY_SET);
+        if(!subject.isReadOnly()) {
+            // Remove principles added by this LoginModule
+            subject.getPrincipals().removeAll(allPrincipals);
+        }
+        allPrincipals.retainAll(Collections.EMPTY_SET);
         return true;
     }
 

Added: geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java?rev=590777&view=auto
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java (added)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java Wed Oct 31 11:26:28 2007
@@ -0,0 +1,167 @@
+/**
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package org.apache.geronimo.security.jaas;
+
+import java.io.File;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.security.auth.Subject;
+import javax.security.auth.login.LoginContext;
+
+import org.apache.geronimo.gbean.AbstractName;
+import org.apache.geronimo.gbean.GBeanData;
+import org.apache.geronimo.security.AbstractTest;
+import org.apache.geronimo.security.realm.GenericSecurityRealm;
+
+
+/**
+ * This test makes sure that SQLLoginModule does not add any principals when login fails
+ * @version $Rev$ $Date$
+ */
+public class LoginSQLAdvancedTest extends AbstractTest {
+    private File basedir = new File(System.getProperty("basedir"));
+    private String hsqldbURL = "jdbc:hsqldb:" + new File(basedir, "target/database/LoginSQLTest");
+    
+    protected AbstractName sqlRealm;
+    protected AbstractName sqlModule;
+    protected AbstractName neverFailModule;
+
+    public void setUp() throws Exception {
+        super.setUp();
+
+        DriverManager.registerDriver(new org.hsqldb.jdbcDriver());
+
+        Connection conn = DriverManager.getConnection(hsqldbURL, "sa", "");
+
+
+        try {
+            conn.prepareStatement("CREATE USER loginmodule PASSWORD password ADMIN;").executeUpdate();
+        } catch (SQLException e) {
+            //ignore, for some reason user already exists.
+        }
+
+        conn.prepareStatement("CREATE TABLE Users(UserName VARCHAR(16), Password VARCHAR(16));").executeUpdate();
+        conn.prepareStatement("CREATE TABLE Groups(GroupName VARCHAR(16), UserName VARCHAR(16));").executeUpdate();
+
+        conn.prepareStatement("GRANT SELECT ON Users TO loginmodule;").executeUpdate();
+        conn.prepareStatement("GRANT SELECT ON Groups TO loginmodule;").executeUpdate();
+
+        conn.prepareStatement("INSERT INTO Users VALUES ('izumi', 'violin');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Users VALUES ('alan', 'starcraft');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Users VALUES ('george', 'bone');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Users VALUES ('gracie', 'biscuit');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Users VALUES ('metro', 'mouse');").executeUpdate();
+
+        conn.prepareStatement("INSERT INTO Groups VALUES ('manager', 'izumi');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('it', 'alan');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('pet', 'george');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('pet', 'gracie');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('pet', 'metro');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('dog', 'george');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('dog', 'gracie');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('cat', 'metro');").executeUpdate();
+
+        conn.close();
+
+        GBeanData gbean;
+        gbean = buildGBeanData("name", "NeverFailLoginModule", LoginModuleGBean.getGBeanInfo());
+        neverFailModule = gbean.getAbstractName();
+        gbean.setAttribute("loginModuleClass", "org.apache.geronimo.security.jaas.NeverFailLoginModule");
+        gbean.setAttribute("options", null);
+        gbean.setAttribute("loginDomainName", "NeverFailDomain");
+        gbean.setAttribute("wrapPrincipals", Boolean.TRUE);
+        kernel.loadGBean(gbean, LoginModuleGBean.class.getClassLoader());
+        kernel.startGBean(neverFailModule);
+
+        gbean = buildGBeanData("name", "SQLLoginModule", LoginModuleGBean.getGBeanInfo());
+        sqlModule = gbean.getAbstractName();
+        gbean.setAttribute("loginModuleClass", "org.apache.geronimo.security.realm.providers.SQLLoginModule");
+        Map<String, Object> props = new HashMap<String, Object>();
+        props.put("jdbcURL", hsqldbURL);
+        props.put("jdbcDriver", "org.hsqldb.jdbcDriver");
+        props.put("jdbcUser", "loginmodule");
+        props.put("jdbcPassword", "password");
+        props.put("userSelect", "SELECT UserName, Password FROM Users where UserName = ?");
+        props.put("groupSelect", "SELECT UserName, GroupName FROM Groups where UserName = ?");
+        gbean.setAttribute("options", props);
+        gbean.setAttribute("loginDomainName", "SQLDomain");
+        gbean.setAttribute("wrapPrincipals", Boolean.TRUE);
+        kernel.loadGBean(gbean, LoginModuleGBean.class.getClassLoader());
+        kernel.startGBean(sqlModule);
+
+        gbean = buildGBeanData("name", "SQLLoginModuleUse", JaasLoginModuleUse.getGBeanInfo());
+        AbstractName sqlUseName = gbean.getAbstractName();
+        gbean.setAttribute("controlFlag", LoginModuleControlFlag.OPTIONAL);
+        gbean.setReferencePattern("LoginModule", sqlModule);
+        kernel.loadGBean(gbean, JaasLoginModuleUse.class.getClassLoader());
+        kernel.startGBean(sqlUseName);
+
+        gbean = buildGBeanData("name", "NeverFailLoginModuleUse", JaasLoginModuleUse.getGBeanInfo());
+        AbstractName neverFailUseName = gbean.getAbstractName();
+        gbean.setAttribute("controlFlag", LoginModuleControlFlag.REQUIRED);
+        gbean.setReferencePattern("LoginModule", neverFailModule);
+        gbean.setReferencePattern("Next", sqlUseName);
+        kernel.loadGBean(gbean, JaasLoginModuleUse.class.getClassLoader());
+        kernel.startGBean(neverFailUseName);
+        
+        gbean = buildGBeanData("name", "SQLSecurityRealm", GenericSecurityRealm.getGBeanInfo());
+        sqlRealm = gbean.getAbstractName();
+        gbean.setAttribute("realmName", "sql-realm");
+        gbean.setReferencePattern("LoginModuleConfiguration", neverFailUseName);
+        kernel.loadGBean(gbean, GenericSecurityRealm.class.getClassLoader());
+        kernel.startGBean(sqlRealm);
+
+    }
+
+    public void tearDown() throws Exception {
+        kernel.stopGBean(sqlRealm);
+        kernel.stopGBean(sqlModule);
+        kernel.stopGBean(neverFailModule);
+        kernel.unloadGBean(sqlRealm);
+        kernel.unloadGBean(sqlModule);
+        kernel.unloadGBean(neverFailModule);
+
+        super.tearDown();
+
+        Connection conn = DriverManager.getConnection(hsqldbURL, "sa", "");
+
+        try {
+            conn.prepareStatement("DROP USER loginmodule;").executeUpdate();
+
+            conn.prepareStatement("DROP TABLE Users;").executeUpdate();
+            conn.prepareStatement("DROP TABLE Groups;").executeUpdate();
+        } catch (SQLException e) {
+            //who knows??
+        }
+
+    }
+
+    public void testBadPasswordLogin() throws Exception {
+        LoginContext context = new LoginContext("sql-realm", new UsernamePasswordCallback("alan", "bad"));
+
+        context.login();
+        Subject subject = context.getSubject();
+        assertTrue("expected non-null subject", subject != null);
+        assertTrue(subject.getPrincipals().size() == 0);
+        context.logout();
+    }
+}

Propchange: geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java
------------------------------------------------------------------------------
    svn:keywords = Date Revision

Propchange: geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Added: geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java?rev=590777&view=auto
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java (added)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java Wed Oct 31 11:26:28 2007
@@ -0,0 +1,52 @@
+/**
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package org.apache.geronimo.security.jaas;
+
+import java.util.Map;
+
+import javax.security.auth.Subject;
+import javax.security.auth.callback.CallbackHandler;
+import javax.security.auth.login.LoginException;
+import javax.security.auth.spi.LoginModule;
+
+
+/**
+ * A LoginModule that never fails login.
+ *
+ * @version $Rev$ $Date$
+ */
+public class NeverFailLoginModule implements LoginModule {
+    public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) {
+    }
+
+    public boolean login() throws LoginException {
+        return true;
+    }
+
+    public boolean commit() throws LoginException {
+        return true;
+    }
+
+    public boolean abort() throws LoginException {
+        return true;
+    }
+
+    public boolean logout() throws LoginException {
+        return true;
+    }
+}

Propchange: geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java
------------------------------------------------------------------------------
    svn:keywords = Date Revision

Propchange: geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Modified: geronimo/server/trunk/framework/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java?rev=590777&r1=590776&r2=590777&view=diff
==============================================================================
--- geronimo/server/trunk/framework/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java (original)
+++ geronimo/server/trunk/framework/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java Wed Oct 31 11:26:28 2007
@@ -26,7 +26,10 @@
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
@@ -92,6 +95,9 @@
     public final static String DATABASE_POOL_APP_NAME = "dataSourceApplication";
     public final static String DIGEST = "digest";
     public final static String ENCODING = "encoding";
+    public final static List<String> supportedOptions = Arrays.asList(USER_SELECT, GROUP_SELECT, CONNECTION_URL,
+            USER, PASSWORD, DRIVER, DATABASE_POOL_NAME, DATABASE_POOL_APP_NAME, DIGEST, ENCODING);
+
     private String connectionURL;
     private Properties properties;
     private Driver driver;
@@ -101,15 +107,22 @@
     private String digest;
     private String encoding;
 
+    private boolean loginSucceeded;
     private Subject subject;
     private CallbackHandler handler;
     private String cbUsername;
     private String cbPassword;
-    private final Set<Principal> groups = new HashSet<Principal>();
+    private final Set<String> groups = new HashSet<String>();
+    private final Set<Principal> allPrincipals = new HashSet<Principal>();
 
     public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) {
         this.subject = subject;
         this.handler = callbackHandler;
+        for(Object option: options.keySet()) {
+            if(!supportedOptions.contains(option)) {
+                log.warn("Ignoring option: "+option+". Not supported.");
+            }
+        }
         userSelect = (String) options.get(USER_SELECT);
         groupSelect = (String) options.get(GROUP_SELECT);
 
@@ -182,7 +195,13 @@
         }
     }
 
+    /**
+     * This LoginModule is not to be ignored.  So, this method should never return false.
+     * @return true if authentication succeeds, or throw a LoginException such as FailedLoginException
+     *         if authentication fails
+     */
     public boolean login() throws LoginException {
+        loginSucceeded = false;
         Callback[] callbacks = new Callback[2];
 
         callbacks[0] = new NameCallback("User name");
@@ -259,7 +278,7 @@
                             String groupName = result.getString(2);
 
                             if (cbUsername.equals(userName)) {
-                                groups.add(new GeronimoGroupPrincipal(groupName));
+                                groups.add(groupName);
                             }
                         }
                     } finally {
@@ -272,35 +291,75 @@
                 conn.close();
             }
         } catch (LoginException e) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
             throw e;
         } catch (SQLException sqle) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
             throw (LoginException) new LoginException("SQL error").initCause(sqle);
         } catch (Exception e) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
             throw (LoginException) new LoginException("Could not access datasource").initCause(e);
         }
 
+        loginSucceeded = true;
         return true;
     }
 
+    /*
+     * @exception LoginException if login succeeded but commit failed.
+     *
+     * @return true if login succeeded and commit succeeded, or false if login failed but commit succeeded.
+     */
     public boolean commit() throws LoginException {
-        Set<Principal> principals = subject.getPrincipals();
-        principals.add(new GeronimoUserPrincipal(cbUsername));
-        principals.addAll(groups);
-
-        return true;
-    }
+        if(loginSucceeded) {
+            if(cbUsername != null) {
+                allPrincipals.add(new GeronimoUserPrincipal(cbUsername));
+            }
+            for(String group: groups) {
+                allPrincipals.add(new GeronimoGroupPrincipal(group));
+            }
+            subject.getPrincipals().addAll(allPrincipals);
+        }
 
-    public boolean abort() throws LoginException {
+        // Clear out the private state
         cbUsername = null;
         cbPassword = null;
+        groups.retainAll(Collections.EMPTY_SET);
 
-        return true;
+        return loginSucceeded;
+    }
+
+    public boolean abort() throws LoginException {
+        if(loginSucceeded) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
+            allPrincipals.retainAll(Collections.EMPTY_SET);
+        }
+        return loginSucceeded;
     }
 
     public boolean logout() throws LoginException {
+        // Clear out the private state
+        loginSucceeded = false;
         cbUsername = null;
         cbPassword = null;
-        //todo: should remove principals put in by commit
+        groups.retainAll(Collections.EMPTY_SET);
+        if(!subject.isReadOnly()) {
+            // Remove principles added by this LoginModule
+            subject.getPrincipals().removeAll(allPrincipals);
+        }
+        allPrincipals.retainAll(Collections.EMPTY_SET);
         return true;
     }
 

Added: geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java?rev=590777&view=auto
==============================================================================
--- geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java (added)
+++ geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java Wed Oct 31 11:26:28 2007
@@ -0,0 +1,167 @@
+/**
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package org.apache.geronimo.security.jaas;
+
+import java.io.File;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.security.auth.Subject;
+import javax.security.auth.login.LoginContext;
+
+import org.apache.geronimo.gbean.AbstractName;
+import org.apache.geronimo.gbean.GBeanData;
+import org.apache.geronimo.security.AbstractTest;
+import org.apache.geronimo.security.realm.GenericSecurityRealm;
+
+
+/**
+ * This test makes sure that SQLLoginModule does not add any principals when login fails
+ * @version $Rev$ $Date$
+ */
+public class LoginSQLAdvancedTest extends AbstractTest {
+    private File basedir = new File(System.getProperty("basedir"));
+    private String hsqldbURL = "jdbc:hsqldb:" + new File(basedir, "target/database/LoginSQLTest");
+    
+    protected AbstractName sqlRealm;
+    protected AbstractName sqlModule;
+    protected AbstractName neverFailModule;
+
+    public void setUp() throws Exception {
+        super.setUp();
+
+        DriverManager.registerDriver(new org.hsqldb.jdbcDriver());
+
+        Connection conn = DriverManager.getConnection(hsqldbURL, "sa", "");
+
+
+        try {
+            conn.prepareStatement("CREATE USER loginmodule PASSWORD password ADMIN;").executeUpdate();
+        } catch (SQLException e) {
+            //ignore, for some reason user already exists.
+        }
+
+        conn.prepareStatement("CREATE TABLE Users(UserName VARCHAR(16), Password VARCHAR(16));").executeUpdate();
+        conn.prepareStatement("CREATE TABLE Groups(GroupName VARCHAR(16), UserName VARCHAR(16));").executeUpdate();
+
+        conn.prepareStatement("GRANT SELECT ON Users TO loginmodule;").executeUpdate();
+        conn.prepareStatement("GRANT SELECT ON Groups TO loginmodule;").executeUpdate();
+
+        conn.prepareStatement("INSERT INTO Users VALUES ('izumi', 'violin');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Users VALUES ('alan', 'starcraft');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Users VALUES ('george', 'bone');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Users VALUES ('gracie', 'biscuit');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Users VALUES ('metro', 'mouse');").executeUpdate();
+
+        conn.prepareStatement("INSERT INTO Groups VALUES ('manager', 'izumi');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('it', 'alan');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('pet', 'george');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('pet', 'gracie');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('pet', 'metro');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('dog', 'george');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('dog', 'gracie');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('cat', 'metro');").executeUpdate();
+
+        conn.close();
+
+        GBeanData gbean;
+        gbean = buildGBeanData("name", "NeverFailLoginModule", LoginModuleGBean.getGBeanInfo());
+        neverFailModule = gbean.getAbstractName();
+        gbean.setAttribute("loginModuleClass", "org.apache.geronimo.security.jaas.NeverFailLoginModule");
+        gbean.setAttribute("options", null);
+        gbean.setAttribute("loginDomainName", "NeverFailDomain");
+        gbean.setAttribute("wrapPrincipals", Boolean.TRUE);
+        kernel.loadGBean(gbean, LoginModuleGBean.class.getClassLoader());
+        kernel.startGBean(neverFailModule);
+
+        gbean = buildGBeanData("name", "SQLLoginModule", LoginModuleGBean.getGBeanInfo());
+        sqlModule = gbean.getAbstractName();
+        gbean.setAttribute("loginModuleClass", "org.apache.geronimo.security.realm.providers.SQLLoginModule");
+        Map<String, Object> props = new HashMap<String, Object>();
+        props.put("jdbcURL", hsqldbURL);
+        props.put("jdbcDriver", "org.hsqldb.jdbcDriver");
+        props.put("jdbcUser", "loginmodule");
+        props.put("jdbcPassword", "password");
+        props.put("userSelect", "SELECT UserName, Password FROM Users where UserName = ?");
+        props.put("groupSelect", "SELECT UserName, GroupName FROM Groups where UserName = ?");
+        gbean.setAttribute("options", props);
+        gbean.setAttribute("loginDomainName", "SQLDomain");
+        gbean.setAttribute("wrapPrincipals", Boolean.TRUE);
+        kernel.loadGBean(gbean, LoginModuleGBean.class.getClassLoader());
+        kernel.startGBean(sqlModule);
+
+        gbean = buildGBeanData("name", "SQLLoginModuleUse", JaasLoginModuleUse.getGBeanInfo());
+        AbstractName sqlUseName = gbean.getAbstractName();
+        gbean.setAttribute("controlFlag", LoginModuleControlFlag.OPTIONAL);
+        gbean.setReferencePattern("LoginModule", sqlModule);
+        kernel.loadGBean(gbean, JaasLoginModuleUse.class.getClassLoader());
+        kernel.startGBean(sqlUseName);
+
+        gbean = buildGBeanData("name", "NeverFailLoginModuleUse", JaasLoginModuleUse.getGBeanInfo());
+        AbstractName neverFailUseName = gbean.getAbstractName();
+        gbean.setAttribute("controlFlag", LoginModuleControlFlag.REQUIRED);
+        gbean.setReferencePattern("LoginModule", neverFailModule);
+        gbean.setReferencePattern("Next", sqlUseName);
+        kernel.loadGBean(gbean, JaasLoginModuleUse.class.getClassLoader());
+        kernel.startGBean(neverFailUseName);
+        
+        gbean = buildGBeanData("name", "SQLSecurityRealm", GenericSecurityRealm.getGBeanInfo());
+        sqlRealm = gbean.getAbstractName();
+        gbean.setAttribute("realmName", "sql-realm");
+        gbean.setReferencePattern("LoginModuleConfiguration", neverFailUseName);
+        kernel.loadGBean(gbean, GenericSecurityRealm.class.getClassLoader());
+        kernel.startGBean(sqlRealm);
+
+    }
+
+    public void tearDown() throws Exception {
+        kernel.stopGBean(sqlRealm);
+        kernel.stopGBean(sqlModule);
+        kernel.stopGBean(neverFailModule);
+        kernel.unloadGBean(sqlRealm);
+        kernel.unloadGBean(sqlModule);
+        kernel.unloadGBean(neverFailModule);
+
+        super.tearDown();
+
+        Connection conn = DriverManager.getConnection(hsqldbURL, "sa", "");
+
+        try {
+            conn.prepareStatement("DROP USER loginmodule;").executeUpdate();
+
+            conn.prepareStatement("DROP TABLE Users;").executeUpdate();
+            conn.prepareStatement("DROP TABLE Groups;").executeUpdate();
+        } catch (SQLException e) {
+            //who knows??
+        }
+
+    }
+
+    public void testBadPasswordLogin() throws Exception {
+        LoginContext context = new LoginContext("sql-realm", new UsernamePasswordCallback("alan", "bad"));
+
+        context.login();
+        Subject subject = context.getSubject();
+        assertTrue("expected non-null subject", subject != null);
+        assertTrue(subject.getPrincipals().size() == 0);
+        context.logout();
+    }
+}

Propchange: geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java
------------------------------------------------------------------------------
    svn:keywords = Date Revision

Propchange: geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Added: geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java?rev=590777&view=auto
==============================================================================
--- geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java (added)
+++ geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java Wed Oct 31 11:26:28 2007
@@ -0,0 +1,52 @@
+/**
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package org.apache.geronimo.security.jaas;
+
+import java.util.Map;
+
+import javax.security.auth.Subject;
+import javax.security.auth.callback.CallbackHandler;
+import javax.security.auth.login.LoginException;
+import javax.security.auth.spi.LoginModule;
+
+
+/**
+ * A LoginModule that never fails login.
+ *
+ * @version $Rev$ $Date$
+ */
+public class NeverFailLoginModule implements LoginModule {
+    public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) {
+    }
+
+    public boolean login() throws LoginException {
+        return true;
+    }
+
+    public boolean commit() throws LoginException {
+        return true;
+    }
+
+    public boolean abort() throws LoginException {
+        return true;
+    }
+
+    public boolean logout() throws LoginException {
+        return true;
+    }
+}

Propchange: geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java
------------------------------------------------------------------------------
    svn:keywords = Date Revision

Propchange: geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain

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?rev=590777&r1=590776&r2=590777&view=diff
==============================================================================
--- 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 Wed Oct 31 11:26:28 2007
@@ -26,7 +26,10 @@
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
@@ -92,6 +95,9 @@
     public final static String DATABASE_POOL_APP_NAME = "dataSourceApplication";
     public final static String DIGEST = "digest";
     public final static String ENCODING = "encoding";
+    public final static List<String> supportedOptions = Arrays.asList(USER_SELECT, GROUP_SELECT, CONNECTION_URL,
+            USER, PASSWORD, DRIVER, DATABASE_POOL_NAME, DATABASE_POOL_APP_NAME, DIGEST, ENCODING);
+
     private String connectionURL;
     private Properties properties;
     private Driver driver;
@@ -101,15 +107,22 @@
     private String digest;
     private String encoding;
 
+    private boolean loginSucceeded;
     private Subject subject;
     private CallbackHandler handler;
     private String cbUsername;
     private String cbPassword;
-    private final Set<Principal> groups = new HashSet<Principal>();
+    private final Set<String> groups = new HashSet<String>();
+    private final Set<Principal> allPrincipals = new HashSet<Principal>();
 
     public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) {
         this.subject = subject;
         this.handler = callbackHandler;
+        for(Object option: options.keySet()) {
+            if(!supportedOptions.contains(option)) {
+                log.warn("Ignoring option: "+option+". Not supported.");
+            }
+        }
         userSelect = (String) options.get(USER_SELECT);
         groupSelect = (String) options.get(GROUP_SELECT);
 
@@ -182,7 +195,13 @@
         }
     }
 
+    /**
+     * This LoginModule is not to be ignored.  So, this method should never return false.
+     * @return true if authentication succeeds, or throw a LoginException such as FailedLoginException
+     *         if authentication fails
+     */
     public boolean login() throws LoginException {
+        loginSucceeded = false;
         Callback[] callbacks = new Callback[2];
 
         callbacks[0] = new NameCallback("User name");
@@ -259,7 +278,7 @@
                             String groupName = result.getString(2);
 
                             if (cbUsername.equals(userName)) {
-                                groups.add(new GeronimoGroupPrincipal(groupName));
+                                groups.add(groupName);
                             }
                         }
                     } finally {
@@ -272,35 +291,75 @@
                 conn.close();
             }
         } catch (LoginException e) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
             throw e;
         } catch (SQLException sqle) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
             throw (LoginException) new LoginException("SQL error").initCause(sqle);
         } catch (Exception e) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
             throw (LoginException) new LoginException("Could not access datasource").initCause(e);
         }
 
+        loginSucceeded = true;
         return true;
     }
 
+    /*
+     * @exception LoginException if login succeeded but commit failed.
+     *
+     * @return true if login succeeded and commit succeeded, or false if login failed but commit succeeded.
+     */
     public boolean commit() throws LoginException {
-        Set<Principal> principals = subject.getPrincipals();
-        principals.add(new GeronimoUserPrincipal(cbUsername));
-        principals.addAll(groups);
-
-        return true;
-    }
+        if(loginSucceeded) {
+            if(cbUsername != null) {
+                allPrincipals.add(new GeronimoUserPrincipal(cbUsername));
+            }
+            for(String group: groups) {
+                allPrincipals.add(new GeronimoGroupPrincipal(group));
+            }
+            subject.getPrincipals().addAll(allPrincipals);
+        }
 
-    public boolean abort() throws LoginException {
+        // Clear out the private state
         cbUsername = null;
         cbPassword = null;
+        groups.retainAll(Collections.EMPTY_SET);
 
-        return true;
+        return loginSucceeded;
+    }
+
+    public boolean abort() throws LoginException {
+        if(loginSucceeded) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
+            allPrincipals.retainAll(Collections.EMPTY_SET);
+        }
+        return loginSucceeded;
     }
 
     public boolean logout() throws LoginException {
+        // Clear out the private state
+        loginSucceeded = false;
         cbUsername = null;
         cbPassword = null;
-        //todo: should remove principals put in by commit
+        groups.retainAll(Collections.EMPTY_SET);
+        if(!subject.isReadOnly()) {
+            // Remove principles added by this LoginModule
+            subject.getPrincipals().removeAll(allPrincipals);
+        }
+        allPrincipals.retainAll(Collections.EMPTY_SET);
         return true;
     }
 

Added: geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java?rev=590777&view=auto
==============================================================================
--- geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java (added)
+++ geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java Wed Oct 31 11:26:28 2007
@@ -0,0 +1,167 @@
+/**
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package org.apache.geronimo.security.jaas;
+
+import java.io.File;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.security.auth.Subject;
+import javax.security.auth.login.LoginContext;
+
+import org.apache.geronimo.gbean.AbstractName;
+import org.apache.geronimo.gbean.GBeanData;
+import org.apache.geronimo.security.AbstractTest;
+import org.apache.geronimo.security.realm.GenericSecurityRealm;
+
+
+/**
+ * This test makes sure that SQLLoginModule does not add any principals when login fails
+ * @version $Rev$ $Date$
+ */
+public class LoginSQLAdvancedTest extends AbstractTest {
+    private File basedir = new File(System.getProperty("basedir"));
+    private String hsqldbURL = "jdbc:hsqldb:" + new File(basedir, "target/database/LoginSQLTest");
+    
+    protected AbstractName sqlRealm;
+    protected AbstractName sqlModule;
+    protected AbstractName neverFailModule;
+
+    public void setUp() throws Exception {
+        super.setUp();
+
+        DriverManager.registerDriver(new org.hsqldb.jdbcDriver());
+
+        Connection conn = DriverManager.getConnection(hsqldbURL, "sa", "");
+
+
+        try {
+            conn.prepareStatement("CREATE USER loginmodule PASSWORD password ADMIN;").executeUpdate();
+        } catch (SQLException e) {
+            //ignore, for some reason user already exists.
+        }
+
+        conn.prepareStatement("CREATE TABLE Users(UserName VARCHAR(16), Password VARCHAR(16));").executeUpdate();
+        conn.prepareStatement("CREATE TABLE Groups(GroupName VARCHAR(16), UserName VARCHAR(16));").executeUpdate();
+
+        conn.prepareStatement("GRANT SELECT ON Users TO loginmodule;").executeUpdate();
+        conn.prepareStatement("GRANT SELECT ON Groups TO loginmodule;").executeUpdate();
+
+        conn.prepareStatement("INSERT INTO Users VALUES ('izumi', 'violin');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Users VALUES ('alan', 'starcraft');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Users VALUES ('george', 'bone');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Users VALUES ('gracie', 'biscuit');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Users VALUES ('metro', 'mouse');").executeUpdate();
+
+        conn.prepareStatement("INSERT INTO Groups VALUES ('manager', 'izumi');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('it', 'alan');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('pet', 'george');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('pet', 'gracie');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('pet', 'metro');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('dog', 'george');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('dog', 'gracie');").executeUpdate();
+        conn.prepareStatement("INSERT INTO Groups VALUES ('cat', 'metro');").executeUpdate();
+
+        conn.close();
+
+        GBeanData gbean;
+        gbean = buildGBeanData("name", "NeverFailLoginModule", LoginModuleGBean.getGBeanInfo());
+        neverFailModule = gbean.getAbstractName();
+        gbean.setAttribute("loginModuleClass", "org.apache.geronimo.security.jaas.NeverFailLoginModule");
+        gbean.setAttribute("options", null);
+        gbean.setAttribute("loginDomainName", "NeverFailDomain");
+        gbean.setAttribute("wrapPrincipals", Boolean.TRUE);
+        kernel.loadGBean(gbean, LoginModuleGBean.class.getClassLoader());
+        kernel.startGBean(neverFailModule);
+
+        gbean = buildGBeanData("name", "SQLLoginModule", LoginModuleGBean.getGBeanInfo());
+        sqlModule = gbean.getAbstractName();
+        gbean.setAttribute("loginModuleClass", "org.apache.geronimo.security.realm.providers.SQLLoginModule");
+        Map<String, Object> props = new HashMap<String, Object>();
+        props.put("jdbcURL", hsqldbURL);
+        props.put("jdbcDriver", "org.hsqldb.jdbcDriver");
+        props.put("jdbcUser", "loginmodule");
+        props.put("jdbcPassword", "password");
+        props.put("userSelect", "SELECT UserName, Password FROM Users where UserName = ?");
+        props.put("groupSelect", "SELECT UserName, GroupName FROM Groups where UserName = ?");
+        gbean.setAttribute("options", props);
+        gbean.setAttribute("loginDomainName", "SQLDomain");
+        gbean.setAttribute("wrapPrincipals", Boolean.TRUE);
+        kernel.loadGBean(gbean, LoginModuleGBean.class.getClassLoader());
+        kernel.startGBean(sqlModule);
+
+        gbean = buildGBeanData("name", "SQLLoginModuleUse", JaasLoginModuleUse.getGBeanInfo());
+        AbstractName sqlUseName = gbean.getAbstractName();
+        gbean.setAttribute("controlFlag", LoginModuleControlFlag.OPTIONAL);
+        gbean.setReferencePattern("LoginModule", sqlModule);
+        kernel.loadGBean(gbean, JaasLoginModuleUse.class.getClassLoader());
+        kernel.startGBean(sqlUseName);
+
+        gbean = buildGBeanData("name", "NeverFailLoginModuleUse", JaasLoginModuleUse.getGBeanInfo());
+        AbstractName neverFailUseName = gbean.getAbstractName();
+        gbean.setAttribute("controlFlag", LoginModuleControlFlag.REQUIRED);
+        gbean.setReferencePattern("LoginModule", neverFailModule);
+        gbean.setReferencePattern("Next", sqlUseName);
+        kernel.loadGBean(gbean, JaasLoginModuleUse.class.getClassLoader());
+        kernel.startGBean(neverFailUseName);
+        
+        gbean = buildGBeanData("name", "SQLSecurityRealm", GenericSecurityRealm.getGBeanInfo());
+        sqlRealm = gbean.getAbstractName();
+        gbean.setAttribute("realmName", "sql-realm");
+        gbean.setReferencePattern("LoginModuleConfiguration", neverFailUseName);
+        kernel.loadGBean(gbean, GenericSecurityRealm.class.getClassLoader());
+        kernel.startGBean(sqlRealm);
+
+    }
+
+    public void tearDown() throws Exception {
+        kernel.stopGBean(sqlRealm);
+        kernel.stopGBean(sqlModule);
+        kernel.stopGBean(neverFailModule);
+        kernel.unloadGBean(sqlRealm);
+        kernel.unloadGBean(sqlModule);
+        kernel.unloadGBean(neverFailModule);
+
+        super.tearDown();
+
+        Connection conn = DriverManager.getConnection(hsqldbURL, "sa", "");
+
+        try {
+            conn.prepareStatement("DROP USER loginmodule;").executeUpdate();
+
+            conn.prepareStatement("DROP TABLE Users;").executeUpdate();
+            conn.prepareStatement("DROP TABLE Groups;").executeUpdate();
+        } catch (SQLException e) {
+            //who knows??
+        }
+
+    }
+
+    public void testBadPasswordLogin() throws Exception {
+        LoginContext context = new LoginContext("sql-realm", new UsernamePasswordCallback("alan", "bad"));
+
+        context.login();
+        Subject subject = context.getSubject();
+        assertTrue("expected non-null subject", subject != null);
+        assertTrue(subject.getPrincipals().size() == 0);
+        context.logout();
+    }
+}

Propchange: geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java
------------------------------------------------------------------------------
    svn:keywords = Date Revision

Propchange: geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginSQLAdvancedTest.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Added: geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java?rev=590777&view=auto
==============================================================================
--- geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java (added)
+++ geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java Wed Oct 31 11:26:28 2007
@@ -0,0 +1,52 @@
+/**
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package org.apache.geronimo.security.jaas;
+
+import java.util.Map;
+
+import javax.security.auth.Subject;
+import javax.security.auth.callback.CallbackHandler;
+import javax.security.auth.login.LoginException;
+import javax.security.auth.spi.LoginModule;
+
+
+/**
+ * A LoginModule that never fails login.
+ *
+ * @version $Rev$ $Date$
+ */
+public class NeverFailLoginModule implements LoginModule {
+    public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) {
+    }
+
+    public boolean login() throws LoginException {
+        return true;
+    }
+
+    public boolean commit() throws LoginException {
+        return true;
+    }
+
+    public boolean abort() throws LoginException {
+        return true;
+    }
+
+    public boolean logout() throws LoginException {
+        return true;
+    }
+}

Propchange: geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java
------------------------------------------------------------------------------
    svn:keywords = Date Revision

Propchange: geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/NeverFailLoginModule.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain