Return-Path: Delivered-To: apmail-geronimo-scm-archive@www.apache.org Received: (qmail 43851 invoked from network); 31 Oct 2007 20:16:16 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 31 Oct 2007 20:16:16 -0000 Received: (qmail 74572 invoked by uid 500); 31 Oct 2007 20:16:03 -0000 Delivered-To: apmail-geronimo-scm-archive@geronimo.apache.org Received: (qmail 74524 invoked by uid 500); 31 Oct 2007 20:16:03 -0000 Mailing-List: contact scm-help@geronimo.apache.org; run by ezmlm Precedence: bulk list-help: list-unsubscribe: List-Post: Reply-To: dev@geronimo.apache.org List-Id: Delivered-To: mailing list scm@geronimo.apache.org Received: (qmail 74513 invoked by uid 99); 31 Oct 2007 20:16:03 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 31 Oct 2007 13:16:03 -0700 X-ASF-Spam-Status: No, hits=-100.0 required=10.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.3] (HELO eris.apache.org) (140.211.11.3) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 31 Oct 2007 20:16:14 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id 5DFDA1A9832; Wed, 31 Oct 2007 13:15:54 -0700 (PDT) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r590807 - 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... Date: Wed, 31 Oct 2007 20:15:51 -0000 To: scm@geronimo.apache.org From: vamsic007@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20071031201554.5DFDA1A9832@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: vamsic007 Date: Wed Oct 31 13:15:48 2007 New Revision: 590807 URL: http://svn.apache.org/viewvc?rev=590807&view=rev Log: **GERONIMO-3571 Review PropertiesFileLoginModule o LoginModule should not add principals when login fails. Added a test to detect the same. o Other changes to bring PropertiesFileLoginModule 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/LoginPropertiesFileAdvancedTest.java (with props) geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java (with props) geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java (with props) Modified: geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java geronimo/server/trunk/framework/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java 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?rev=590807&r1=590806&r2=590807&view=diff ============================================================================== --- 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 Wed Oct 31 13:15:48 2007 @@ -23,9 +23,12 @@ import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.security.Principal; +import java.util.Arrays; +import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Properties; import java.util.Set; @@ -66,6 +69,7 @@ public final static String GROUPS_URI = "groupsURI"; public final static String DIGEST = "digest"; public final static String ENCODING = "encoding"; + public final static List supportedOptions = Arrays.asList(USERS_URI, GROUPS_URI, DIGEST, ENCODING); private static Log log = LogFactory.getLog(PropertiesFileLoginModule.class); final Properties users = new Properties(); @@ -73,14 +77,21 @@ private String digest; private String encoding; + private boolean loginSucceeded; private Subject subject; private CallbackHandler handler; private String username; private String password; + private final Set allPrincipals = new HashSet(); 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."); + } + } try { ServerInfo serverInfo = (ServerInfo) options.get(JaasLoginModuleUse.SERVERINFO_LM_OPTION); final String users = (String) options.get(USERS_URI); @@ -152,7 +163,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"); @@ -167,6 +184,9 @@ assert callbacks.length == 2; username = ((NameCallback) callbacks[0]).getName(); if (username == null || username.equals("")) { + // Clear out the private state + username = null; + password = null; throw new FailedLoginException(); } String realPassword = users.getProperty(username); @@ -177,41 +197,65 @@ char[] entered = ((PasswordCallback) callbacks[1]).getPassword(); password = entered == null ? null : new String(entered); if (!checkPassword(realPassword, password)) { + // Clear out the private state + username = null; + password = null; throw new FailedLoginException(); } + + 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 principals = subject.getPrincipals(); - - principals.add(new GeronimoUserPrincipal(username)); - - for (Map.Entry> entry : groups.entrySet()) { - String groupName = entry.getKey(); - Set users = entry.getValue(); - for (String user : users) { - if (username.equals(user)) { - principals.add(new GeronimoGroupPrincipal(groupName)); - break; + if(loginSucceeded) { + if(username != null) { + allPrincipals.add(new GeronimoUserPrincipal(username)); + } + for (Map.Entry> entry : groups.entrySet()) { + String groupName = entry.getKey(); + Set users = entry.getValue(); + for (String user : users) { + if (username.equals(user)) { + allPrincipals.add(new GeronimoGroupPrincipal(groupName)); + break; + } } } + subject.getPrincipals().addAll(allPrincipals); } + // Clear out the private state + username = null; + password = null; - return true; + return loginSucceeded; } public boolean abort() throws LoginException { - username = null; - password = null; - - return true; + if(loginSucceeded) { + // Clear out the private state + username = null; + password = null; + allPrincipals.retainAll(Collections.EMPTY_SET); + } + return loginSucceeded; } public boolean logout() throws LoginException { + // Clear out the private state + loginSucceeded = false; username = null; password = null; - //todo: should remove principals added by commit + if(!subject.isReadOnly()) { + // Remove principals 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/LoginPropertiesFileAdvancedTest.java URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java?rev=590807&view=auto ============================================================================== --- geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java (added) +++ geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java Wed Oct 31 13:15:48 2007 @@ -0,0 +1,123 @@ +/** + * 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.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 PropertiesFileLoginModule does not add any principals when login fails + * @version $Rev$ $Date$ + */ +public class LoginPropertiesFileAdvancedTest extends AbstractTest { + protected AbstractName clientCE; + protected AbstractName testCE; + protected AbstractName testRealm; + protected AbstractName neverFailModule; + + public void setUp() throws Exception { + needServerInfo = true; + needLoginConfiguration = true; + super.setUp(); + + 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", "PropertiesLoginModule", LoginModuleGBean.getGBeanInfo()); + testCE = gbean.getAbstractName(); + gbean.setAttribute("loginModuleClass", "org.apache.geronimo.security.realm.providers.PropertiesFileLoginModule"); + Map props = new HashMap(); + props.put("usersURI", new File(BASEDIR, "src/test/data/data/users.properties").toURI().toString()); + props.put("groupsURI", new File(BASEDIR, "src/test/data/data/groups.properties").toURI().toString()); + gbean.setAttribute("options", props); + gbean.setAttribute("loginDomainName", "TestProperties"); + gbean.setAttribute("wrapPrincipals", Boolean.TRUE); + kernel.loadGBean(gbean, LoginModuleGBean.class.getClassLoader()); + kernel.startGBean(testCE); + + gbean = buildGBeanData("name", "PropertiesLoginModuleUse", JaasLoginModuleUse.getGBeanInfo()); + AbstractName propsUseName = gbean.getAbstractName(); + gbean.setAttribute("controlFlag", LoginModuleControlFlag.OPTIONAL); + gbean.setReferencePattern("LoginModule", testCE); + kernel.loadGBean(gbean, JaasLoginModuleUse.class.getClassLoader()); + kernel.startGBean(propsUseName); + + gbean = buildGBeanData("name", "NeverFailLoginModuleUse", JaasLoginModuleUse.getGBeanInfo()); + AbstractName neverFailUseName = gbean.getAbstractName(); + gbean.setAttribute("controlFlag", LoginModuleControlFlag.REQUIRED); + gbean.setReferencePattern("LoginModule", neverFailModule); + gbean.setReferencePattern("Next", propsUseName); + kernel.loadGBean(gbean, JaasLoginModuleUse.class.getClassLoader()); + kernel.startGBean(neverFailUseName); + + gbean = buildGBeanData("name", "PropertiesSecurityRealm", GenericSecurityRealm.getGBeanInfo()); + testRealm = gbean.getAbstractName(); + gbean.setAttribute("realmName", "properties-realm"); + gbean.setAttribute("wrapPrincipals", Boolean.TRUE); + gbean.setReferencePattern("LoginModuleConfiguration", neverFailUseName); + gbean.setReferencePattern("ServerInfo", serverInfo); + kernel.loadGBean(gbean, GenericSecurityRealm.class.getClassLoader()); + + kernel.startGBean(loginConfiguration); + kernel.startGBean(testRealm); + } + + public void tearDown() throws Exception { + kernel.stopGBean(testRealm); + kernel.stopGBean(loginConfiguration); + kernel.stopGBean(testCE); + kernel.stopGBean(neverFailModule); + kernel.stopGBean(serverInfo); + + kernel.unloadGBean(testRealm); + kernel.unloadGBean(loginConfiguration); + kernel.unloadGBean(testCE); + kernel.unloadGBean(neverFailModule); + kernel.unloadGBean(serverInfo); + + super.tearDown(); + } + + public void testBadPasswordLogin() throws Exception { + LoginContext context = new LoginContext("properties-realm", new UsernamePasswordCallback("alan", "bad")); + + context.login(); + Subject subject = context.getSubject(); + assertTrue("expected non-null subject", subject != null); + assertTrue("expected zero principals", subject.getPrincipals().size() == 0); + context.logout(); + } +} Propchange: geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java ------------------------------------------------------------------------------ svn:eol-style = native Propchange: geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java ------------------------------------------------------------------------------ svn:keywords = Date Revision Propchange: geronimo/server/branches/2.0/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java ------------------------------------------------------------------------------ svn:mime-type = text/plain Modified: geronimo/server/trunk/framework/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java URL: http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java?rev=590807&r1=590806&r2=590807&view=diff ============================================================================== --- geronimo/server/trunk/framework/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java (original) +++ geronimo/server/trunk/framework/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java Wed Oct 31 13:15:48 2007 @@ -23,9 +23,12 @@ import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.security.Principal; +import java.util.Arrays; +import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Properties; import java.util.Set; @@ -66,6 +69,7 @@ public final static String GROUPS_URI = "groupsURI"; public final static String DIGEST = "digest"; public final static String ENCODING = "encoding"; + public final static List supportedOptions = Arrays.asList(USERS_URI, GROUPS_URI, DIGEST, ENCODING); private static Log log = LogFactory.getLog(PropertiesFileLoginModule.class); final Properties users = new Properties(); @@ -73,14 +77,21 @@ private String digest; private String encoding; + private boolean loginSucceeded; private Subject subject; private CallbackHandler handler; private String username; private String password; + private final Set allPrincipals = new HashSet(); 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."); + } + } try { ServerInfo serverInfo = (ServerInfo) options.get(JaasLoginModuleUse.SERVERINFO_LM_OPTION); final String users = (String) options.get(USERS_URI); @@ -152,7 +163,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"); @@ -167,6 +184,9 @@ assert callbacks.length == 2; username = ((NameCallback) callbacks[0]).getName(); if (username == null || username.equals("")) { + // Clear out the private state + username = null; + password = null; throw new FailedLoginException(); } String realPassword = users.getProperty(username); @@ -177,41 +197,65 @@ char[] entered = ((PasswordCallback) callbacks[1]).getPassword(); password = entered == null ? null : new String(entered); if (!checkPassword(realPassword, password)) { + // Clear out the private state + username = null; + password = null; throw new FailedLoginException(); } + + 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 principals = subject.getPrincipals(); - - principals.add(new GeronimoUserPrincipal(username)); - - for (Map.Entry> entry : groups.entrySet()) { - String groupName = entry.getKey(); - Set users = entry.getValue(); - for (String user : users) { - if (username.equals(user)) { - principals.add(new GeronimoGroupPrincipal(groupName)); - break; + if(loginSucceeded) { + if(username != null) { + allPrincipals.add(new GeronimoUserPrincipal(username)); + } + for (Map.Entry> entry : groups.entrySet()) { + String groupName = entry.getKey(); + Set users = entry.getValue(); + for (String user : users) { + if (username.equals(user)) { + allPrincipals.add(new GeronimoGroupPrincipal(groupName)); + break; + } } } + subject.getPrincipals().addAll(allPrincipals); } + // Clear out the private state + username = null; + password = null; - return true; + return loginSucceeded; } public boolean abort() throws LoginException { - username = null; - password = null; - - return true; + if(loginSucceeded) { + // Clear out the private state + username = null; + password = null; + allPrincipals.retainAll(Collections.EMPTY_SET); + } + return loginSucceeded; } public boolean logout() throws LoginException { + // Clear out the private state + loginSucceeded = false; username = null; password = null; - //todo: should remove principals added by commit + if(!subject.isReadOnly()) { + // Remove principals 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/LoginPropertiesFileAdvancedTest.java URL: http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java?rev=590807&view=auto ============================================================================== --- geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java (added) +++ geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java Wed Oct 31 13:15:48 2007 @@ -0,0 +1,123 @@ +/** + * 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.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 PropertiesFileLoginModule does not add any principals when login fails + * @version $Rev$ $Date$ + */ +public class LoginPropertiesFileAdvancedTest extends AbstractTest { + protected AbstractName clientCE; + protected AbstractName testCE; + protected AbstractName testRealm; + protected AbstractName neverFailModule; + + public void setUp() throws Exception { + needServerInfo = true; + needLoginConfiguration = true; + super.setUp(); + + 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", "PropertiesLoginModule", LoginModuleGBean.getGBeanInfo()); + testCE = gbean.getAbstractName(); + gbean.setAttribute("loginModuleClass", "org.apache.geronimo.security.realm.providers.PropertiesFileLoginModule"); + Map props = new HashMap(); + props.put("usersURI", new File(BASEDIR, "src/test/data/data/users.properties").toURI().toString()); + props.put("groupsURI", new File(BASEDIR, "src/test/data/data/groups.properties").toURI().toString()); + gbean.setAttribute("options", props); + gbean.setAttribute("loginDomainName", "TestProperties"); + gbean.setAttribute("wrapPrincipals", Boolean.TRUE); + kernel.loadGBean(gbean, LoginModuleGBean.class.getClassLoader()); + kernel.startGBean(testCE); + + gbean = buildGBeanData("name", "PropertiesLoginModuleUse", JaasLoginModuleUse.getGBeanInfo()); + AbstractName propsUseName = gbean.getAbstractName(); + gbean.setAttribute("controlFlag", LoginModuleControlFlag.OPTIONAL); + gbean.setReferencePattern("LoginModule", testCE); + kernel.loadGBean(gbean, JaasLoginModuleUse.class.getClassLoader()); + kernel.startGBean(propsUseName); + + gbean = buildGBeanData("name", "NeverFailLoginModuleUse", JaasLoginModuleUse.getGBeanInfo()); + AbstractName neverFailUseName = gbean.getAbstractName(); + gbean.setAttribute("controlFlag", LoginModuleControlFlag.REQUIRED); + gbean.setReferencePattern("LoginModule", neverFailModule); + gbean.setReferencePattern("Next", propsUseName); + kernel.loadGBean(gbean, JaasLoginModuleUse.class.getClassLoader()); + kernel.startGBean(neverFailUseName); + + gbean = buildGBeanData("name", "PropertiesSecurityRealm", GenericSecurityRealm.getGBeanInfo()); + testRealm = gbean.getAbstractName(); + gbean.setAttribute("realmName", "properties-realm"); + gbean.setAttribute("wrapPrincipals", Boolean.TRUE); + gbean.setReferencePattern("LoginModuleConfiguration", neverFailUseName); + gbean.setReferencePattern("ServerInfo", serverInfo); + kernel.loadGBean(gbean, GenericSecurityRealm.class.getClassLoader()); + + kernel.startGBean(loginConfiguration); + kernel.startGBean(testRealm); + } + + public void tearDown() throws Exception { + kernel.stopGBean(testRealm); + kernel.stopGBean(loginConfiguration); + kernel.stopGBean(testCE); + kernel.stopGBean(neverFailModule); + kernel.stopGBean(serverInfo); + + kernel.unloadGBean(testRealm); + kernel.unloadGBean(loginConfiguration); + kernel.unloadGBean(testCE); + kernel.unloadGBean(neverFailModule); + kernel.unloadGBean(serverInfo); + + super.tearDown(); + } + + public void testBadPasswordLogin() throws Exception { + LoginContext context = new LoginContext("properties-realm", new UsernamePasswordCallback("alan", "bad")); + + context.login(); + Subject subject = context.getSubject(); + assertTrue("expected non-null subject", subject != null); + assertTrue("expected zero principals", subject.getPrincipals().size() == 0); + context.logout(); + } +} Propchange: geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java ------------------------------------------------------------------------------ svn:eol-style = native Propchange: geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java ------------------------------------------------------------------------------ svn:keywords = Date Revision Propchange: geronimo/server/trunk/framework/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java ------------------------------------------------------------------------------ svn:mime-type = text/plain Modified: geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java?rev=590807&r1=590806&r2=590807&view=diff ============================================================================== --- geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java (original) +++ geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/PropertiesFileLoginModule.java Wed Oct 31 13:15:48 2007 @@ -23,9 +23,12 @@ import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.security.Principal; +import java.util.Arrays; +import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Properties; import java.util.Set; @@ -66,6 +69,7 @@ public final static String GROUPS_URI = "groupsURI"; public final static String DIGEST = "digest"; public final static String ENCODING = "encoding"; + public final static List supportedOptions = Arrays.asList(USERS_URI, GROUPS_URI, DIGEST, ENCODING); private static Log log = LogFactory.getLog(PropertiesFileLoginModule.class); final Properties users = new Properties(); @@ -73,14 +77,21 @@ private String digest; private String encoding; + private boolean loginSucceeded; private Subject subject; private CallbackHandler handler; private String username; private String password; + private final Set allPrincipals = new HashSet(); 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."); + } + } try { ServerInfo serverInfo = (ServerInfo) options.get(JaasLoginModuleUse.SERVERINFO_LM_OPTION); final String users = (String) options.get(USERS_URI); @@ -152,7 +163,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"); @@ -167,6 +184,9 @@ assert callbacks.length == 2; username = ((NameCallback) callbacks[0]).getName(); if (username == null || username.equals("")) { + // Clear out the private state + username = null; + password = null; throw new FailedLoginException(); } String realPassword = users.getProperty(username); @@ -177,41 +197,65 @@ char[] entered = ((PasswordCallback) callbacks[1]).getPassword(); password = entered == null ? null : new String(entered); if (!checkPassword(realPassword, password)) { + // Clear out the private state + username = null; + password = null; throw new FailedLoginException(); } + + 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 principals = subject.getPrincipals(); - - principals.add(new GeronimoUserPrincipal(username)); - - for (Map.Entry> entry : groups.entrySet()) { - String groupName = entry.getKey(); - Set users = entry.getValue(); - for (String user : users) { - if (username.equals(user)) { - principals.add(new GeronimoGroupPrincipal(groupName)); - break; + if(loginSucceeded) { + if(username != null) { + allPrincipals.add(new GeronimoUserPrincipal(username)); + } + for (Map.Entry> entry : groups.entrySet()) { + String groupName = entry.getKey(); + Set users = entry.getValue(); + for (String user : users) { + if (username.equals(user)) { + allPrincipals.add(new GeronimoGroupPrincipal(groupName)); + break; + } } } + subject.getPrincipals().addAll(allPrincipals); } + // Clear out the private state + username = null; + password = null; - return true; + return loginSucceeded; } public boolean abort() throws LoginException { - username = null; - password = null; - - return true; + if(loginSucceeded) { + // Clear out the private state + username = null; + password = null; + allPrincipals.retainAll(Collections.EMPTY_SET); + } + return loginSucceeded; } public boolean logout() throws LoginException { + // Clear out the private state + loginSucceeded = false; username = null; password = null; - //todo: should remove principals added by commit + if(!subject.isReadOnly()) { + // Remove principals 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/LoginPropertiesFileAdvancedTest.java URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java?rev=590807&view=auto ============================================================================== --- geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java (added) +++ geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java Wed Oct 31 13:15:48 2007 @@ -0,0 +1,123 @@ +/** + * 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.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 PropertiesFileLoginModule does not add any principals when login fails + * @version $Rev$ $Date$ + */ +public class LoginPropertiesFileAdvancedTest extends AbstractTest { + protected AbstractName clientCE; + protected AbstractName testCE; + protected AbstractName testRealm; + protected AbstractName neverFailModule; + + public void setUp() throws Exception { + needServerInfo = true; + needLoginConfiguration = true; + super.setUp(); + + 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", "PropertiesLoginModule", LoginModuleGBean.getGBeanInfo()); + testCE = gbean.getAbstractName(); + gbean.setAttribute("loginModuleClass", "org.apache.geronimo.security.realm.providers.PropertiesFileLoginModule"); + Map props = new HashMap(); + props.put("usersURI", new File(BASEDIR, "src/test/data/data/users.properties").toURI().toString()); + props.put("groupsURI", new File(BASEDIR, "src/test/data/data/groups.properties").toURI().toString()); + gbean.setAttribute("options", props); + gbean.setAttribute("loginDomainName", "TestProperties"); + gbean.setAttribute("wrapPrincipals", Boolean.TRUE); + kernel.loadGBean(gbean, LoginModuleGBean.class.getClassLoader()); + kernel.startGBean(testCE); + + gbean = buildGBeanData("name", "PropertiesLoginModuleUse", JaasLoginModuleUse.getGBeanInfo()); + AbstractName propsUseName = gbean.getAbstractName(); + gbean.setAttribute("controlFlag", LoginModuleControlFlag.OPTIONAL); + gbean.setReferencePattern("LoginModule", testCE); + kernel.loadGBean(gbean, JaasLoginModuleUse.class.getClassLoader()); + kernel.startGBean(propsUseName); + + gbean = buildGBeanData("name", "NeverFailLoginModuleUse", JaasLoginModuleUse.getGBeanInfo()); + AbstractName neverFailUseName = gbean.getAbstractName(); + gbean.setAttribute("controlFlag", LoginModuleControlFlag.REQUIRED); + gbean.setReferencePattern("LoginModule", neverFailModule); + gbean.setReferencePattern("Next", propsUseName); + kernel.loadGBean(gbean, JaasLoginModuleUse.class.getClassLoader()); + kernel.startGBean(neverFailUseName); + + gbean = buildGBeanData("name", "PropertiesSecurityRealm", GenericSecurityRealm.getGBeanInfo()); + testRealm = gbean.getAbstractName(); + gbean.setAttribute("realmName", "properties-realm"); + gbean.setAttribute("wrapPrincipals", Boolean.TRUE); + gbean.setReferencePattern("LoginModuleConfiguration", neverFailUseName); + gbean.setReferencePattern("ServerInfo", serverInfo); + kernel.loadGBean(gbean, GenericSecurityRealm.class.getClassLoader()); + + kernel.startGBean(loginConfiguration); + kernel.startGBean(testRealm); + } + + public void tearDown() throws Exception { + kernel.stopGBean(testRealm); + kernel.stopGBean(loginConfiguration); + kernel.stopGBean(testCE); + kernel.stopGBean(neverFailModule); + kernel.stopGBean(serverInfo); + + kernel.unloadGBean(testRealm); + kernel.unloadGBean(loginConfiguration); + kernel.unloadGBean(testCE); + kernel.unloadGBean(neverFailModule); + kernel.unloadGBean(serverInfo); + + super.tearDown(); + } + + public void testBadPasswordLogin() throws Exception { + LoginContext context = new LoginContext("properties-realm", new UsernamePasswordCallback("alan", "bad")); + + context.login(); + Subject subject = context.getSubject(); + assertTrue("expected non-null subject", subject != null); + assertTrue("expected zero principals", subject.getPrincipals().size() == 0); + context.logout(); + } +} Propchange: geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java ------------------------------------------------------------------------------ svn:eol-style = native Propchange: geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java ------------------------------------------------------------------------------ svn:keywords = Date Revision Propchange: geronimo/server/trunk/modules/geronimo-security/src/test/java/org/apache/geronimo/security/jaas/LoginPropertiesFileAdvancedTest.java ------------------------------------------------------------------------------ svn:mime-type = text/plain