geronimo-scm mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From vamsic...@apache.org
Subject svn commit: r590821 - in /geronimo/server: branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/ trunk/framework/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/ trunk/mo...
Date Wed, 31 Oct 2007 20:53:32 GMT
Author: vamsic007
Date: Wed Oct 31 13:53:31 2007
New Revision: 590821

URL: http://svn.apache.org/viewvc?rev=590821&view=rev
Log:
**GERONIMO-3574 Review PropertiesFileLoginModule
 o LoginModule should not add principals when login fails. NOTE: Adding a test for this would
be good.
 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.

Modified:
    geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java
    geronimo/server/trunk/framework/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java
    geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java

Modified: geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java?rev=590821&r1=590820&r2=590821&view=diff
==============================================================================
--- geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java
(original)
+++ geronimo/server/branches/2.0/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java
Wed Oct 31 13:53:31 2007
@@ -21,8 +21,11 @@
 import java.security.Principal;
 import java.text.MessageFormat;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Hashtable;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -82,6 +85,10 @@
     private static final String ROLE_SEARCH_MATCHING = "roleSearchMatching";
     private static final String ROLE_SEARCH_SUBTREE = "roleSearchSubtree";
     private static final String USER_ROLE_NAME = "userRoleName";
+    public final static List<String> supportedOptions = Arrays.asList(INITIAL_CONTEXT_FACTORY,
CONNECTION_URL,
+            CONNECTION_USERNAME, CONNECTION_PASSWORD, CONNECTION_PROTOCOL, AUTHENTICATION,
USER_BASE,
+            USER_SEARCH_MATCHING, USER_SEARCH_SUBTREE, ROLE_BASE, ROLE_NAME, ROLE_SEARCH_MATCHING,
ROLE_SEARCH_SUBTREE,
+            USER_ROLE_NAME);
 
     private String initialContextFactory;
     private String connectionURL;
@@ -105,11 +112,18 @@
     private boolean userSearchSubtreeBool = false;
     private boolean roleSearchSubtreeBool = false;
 
-    Set<Principal> groups = new HashSet<Principal>();
+    private boolean loginSucceeded;
+    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.");
+            }
+        }
         initialContextFactory = (String) options.get(INITIAL_CONTEXT_FACTORY);
         connectionURL = (String) options.get(CONNECTION_URL);
         connectionUsername = (String) options.get(CONNECTION_USERNAME);
@@ -130,7 +144,13 @@
         roleSearchSubtreeBool = Boolean.valueOf(roleSearchSubtree);
     }
 
+    /**
+     * 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");
@@ -147,38 +167,80 @@
 
         if (cbUsername == null || "".equals(cbUsername)
                 || cbPassword == null || "".equals(cbPassword)) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
             throw new FailedLoginException();
         }
 
         try {
             boolean result = authenticate(cbUsername, cbPassword);
             if (!result) {
+                // Clear out the private state
+                cbUsername = null;
+                cbPassword = null;
+                groups.retainAll(Collections.EMPTY_SET);
                 throw new FailedLoginException();
-            } else {
-                return true;
             }
         } catch (Exception e) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
             throw (LoginException) new LoginException("LDAP Error").initCause(e);
         }
-    }
 
-    public boolean commit() throws LoginException {
-        Set<Principal> principals = subject.getPrincipals();
-        principals.add(new GeronimoUserPrincipal(cbUsername));
-        principals.addAll(groups);
+        loginSucceeded = true;
         return true;
     }
 
-    public boolean abort() throws LoginException {
+    /*
+     * @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 {
+        if(loginSucceeded) {
+            if(cbUsername != null) {
+                allPrincipals.add(new GeronimoUserPrincipal(cbUsername));
+            }
+            for(String group: groups) {
+                allPrincipals.add(new GeronimoGroupPrincipal(group));
+            }
+            subject.getPrincipals().addAll(allPrincipals);
+        }
+
+        // Clear out the private state
         cbUsername = null;
         cbPassword = null;
-        return true;
+        groups.retainAll(Collections.EMPTY_SET);
+
+        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 added by commit
+        groups.retainAll(Collections.EMPTY_SET);
+        if(!subject.isReadOnly()) {
+            // Remove principals added by this LoginModule
+            subject.getPrincipals().removeAll(allPrincipals);
+        }
+        allPrincipals.retainAll(Collections.EMPTY_SET);
         return true;
     }
 
@@ -247,7 +309,7 @@
             //if authenticated add more roles
             roles = getRoles(context, dn, username, roles);
             for (String role : roles) {
-                groups.add(new GeronimoGroupPrincipal(role));
+                groups.add(role);
             }
         } catch (CommunicationException e) {
             close(context);

Modified: geronimo/server/trunk/framework/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java?rev=590821&r1=590820&r2=590821&view=diff
==============================================================================
--- geronimo/server/trunk/framework/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java
(original)
+++ geronimo/server/trunk/framework/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java
Wed Oct 31 13:53:31 2007
@@ -21,8 +21,11 @@
 import java.security.Principal;
 import java.text.MessageFormat;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Hashtable;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -82,6 +85,10 @@
     private static final String ROLE_SEARCH_MATCHING = "roleSearchMatching";
     private static final String ROLE_SEARCH_SUBTREE = "roleSearchSubtree";
     private static final String USER_ROLE_NAME = "userRoleName";
+    public final static List<String> supportedOptions = Arrays.asList(INITIAL_CONTEXT_FACTORY,
CONNECTION_URL,
+            CONNECTION_USERNAME, CONNECTION_PASSWORD, CONNECTION_PROTOCOL, AUTHENTICATION,
USER_BASE,
+            USER_SEARCH_MATCHING, USER_SEARCH_SUBTREE, ROLE_BASE, ROLE_NAME, ROLE_SEARCH_MATCHING,
ROLE_SEARCH_SUBTREE,
+            USER_ROLE_NAME);
 
     private String initialContextFactory;
     private String connectionURL;
@@ -105,11 +112,18 @@
     private boolean userSearchSubtreeBool = false;
     private boolean roleSearchSubtreeBool = false;
 
-    Set<Principal> groups = new HashSet<Principal>();
+    private boolean loginSucceeded;
+    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.");
+            }
+        }
         initialContextFactory = (String) options.get(INITIAL_CONTEXT_FACTORY);
         connectionURL = (String) options.get(CONNECTION_URL);
         connectionUsername = (String) options.get(CONNECTION_USERNAME);
@@ -130,7 +144,13 @@
         roleSearchSubtreeBool = Boolean.valueOf(roleSearchSubtree);
     }
 
+    /**
+     * 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");
@@ -147,38 +167,80 @@
 
         if (cbUsername == null || "".equals(cbUsername)
                 || cbPassword == null || "".equals(cbPassword)) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
             throw new FailedLoginException();
         }
 
         try {
             boolean result = authenticate(cbUsername, cbPassword);
             if (!result) {
+                // Clear out the private state
+                cbUsername = null;
+                cbPassword = null;
+                groups.retainAll(Collections.EMPTY_SET);
                 throw new FailedLoginException();
-            } else {
-                return true;
             }
         } catch (Exception e) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
             throw (LoginException) new LoginException("LDAP Error").initCause(e);
         }
-    }
 
-    public boolean commit() throws LoginException {
-        Set<Principal> principals = subject.getPrincipals();
-        principals.add(new GeronimoUserPrincipal(cbUsername));
-        principals.addAll(groups);
+        loginSucceeded = true;
         return true;
     }
 
-    public boolean abort() throws LoginException {
+    /*
+     * @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 {
+        if(loginSucceeded) {
+            if(cbUsername != null) {
+                allPrincipals.add(new GeronimoUserPrincipal(cbUsername));
+            }
+            for(String group: groups) {
+                allPrincipals.add(new GeronimoGroupPrincipal(group));
+            }
+            subject.getPrincipals().addAll(allPrincipals);
+        }
+
+        // Clear out the private state
         cbUsername = null;
         cbPassword = null;
-        return true;
+        groups.retainAll(Collections.EMPTY_SET);
+
+        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 added by commit
+        groups.retainAll(Collections.EMPTY_SET);
+        if(!subject.isReadOnly()) {
+            // Remove principals added by this LoginModule
+            subject.getPrincipals().removeAll(allPrincipals);
+        }
+        allPrincipals.retainAll(Collections.EMPTY_SET);
         return true;
     }
 
@@ -247,7 +309,7 @@
             //if authenticated add more roles
             roles = getRoles(context, dn, username, roles);
             for (String role : roles) {
-                groups.add(new GeronimoGroupPrincipal(role));
+                groups.add(role);
             }
         } catch (CommunicationException e) {
             close(context);

Modified: geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java?rev=590821&r1=590820&r2=590821&view=diff
==============================================================================
--- geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java
(original)
+++ geronimo/server/trunk/modules/geronimo-security/src/main/java/org/apache/geronimo/security/realm/providers/LDAPLoginModule.java
Wed Oct 31 13:53:31 2007
@@ -21,8 +21,11 @@
 import java.security.Principal;
 import java.text.MessageFormat;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Hashtable;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -82,6 +85,10 @@
     private static final String ROLE_SEARCH_MATCHING = "roleSearchMatching";
     private static final String ROLE_SEARCH_SUBTREE = "roleSearchSubtree";
     private static final String USER_ROLE_NAME = "userRoleName";
+    public final static List<String> supportedOptions = Arrays.asList(INITIAL_CONTEXT_FACTORY,
CONNECTION_URL,
+            CONNECTION_USERNAME, CONNECTION_PASSWORD, CONNECTION_PROTOCOL, AUTHENTICATION,
USER_BASE,
+            USER_SEARCH_MATCHING, USER_SEARCH_SUBTREE, ROLE_BASE, ROLE_NAME, ROLE_SEARCH_MATCHING,
ROLE_SEARCH_SUBTREE,
+            USER_ROLE_NAME);
 
     private String initialContextFactory;
     private String connectionURL;
@@ -105,11 +112,18 @@
     private boolean userSearchSubtreeBool = false;
     private boolean roleSearchSubtreeBool = false;
 
-    Set<Principal> groups = new HashSet<Principal>();
+    private boolean loginSucceeded;
+    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.");
+            }
+        }
         initialContextFactory = (String) options.get(INITIAL_CONTEXT_FACTORY);
         connectionURL = (String) options.get(CONNECTION_URL);
         connectionUsername = (String) options.get(CONNECTION_USERNAME);
@@ -130,7 +144,13 @@
         roleSearchSubtreeBool = Boolean.valueOf(roleSearchSubtree);
     }
 
+    /**
+     * 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");
@@ -147,38 +167,80 @@
 
         if (cbUsername == null || "".equals(cbUsername)
                 || cbPassword == null || "".equals(cbPassword)) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
             throw new FailedLoginException();
         }
 
         try {
             boolean result = authenticate(cbUsername, cbPassword);
             if (!result) {
+                // Clear out the private state
+                cbUsername = null;
+                cbPassword = null;
+                groups.retainAll(Collections.EMPTY_SET);
                 throw new FailedLoginException();
-            } else {
-                return true;
             }
         } catch (Exception e) {
+            // Clear out the private state
+            cbUsername = null;
+            cbPassword = null;
+            groups.retainAll(Collections.EMPTY_SET);
             throw (LoginException) new LoginException("LDAP Error").initCause(e);
         }
-    }
 
-    public boolean commit() throws LoginException {
-        Set<Principal> principals = subject.getPrincipals();
-        principals.add(new GeronimoUserPrincipal(cbUsername));
-        principals.addAll(groups);
+        loginSucceeded = true;
         return true;
     }
 
-    public boolean abort() throws LoginException {
+    /*
+     * @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 {
+        if(loginSucceeded) {
+            if(cbUsername != null) {
+                allPrincipals.add(new GeronimoUserPrincipal(cbUsername));
+            }
+            for(String group: groups) {
+                allPrincipals.add(new GeronimoGroupPrincipal(group));
+            }
+            subject.getPrincipals().addAll(allPrincipals);
+        }
+
+        // Clear out the private state
         cbUsername = null;
         cbPassword = null;
-        return true;
+        groups.retainAll(Collections.EMPTY_SET);
+
+        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 added by commit
+        groups.retainAll(Collections.EMPTY_SET);
+        if(!subject.isReadOnly()) {
+            // Remove principals added by this LoginModule
+            subject.getPrincipals().removeAll(allPrincipals);
+        }
+        allPrincipals.retainAll(Collections.EMPTY_SET);
         return true;
     }
 
@@ -247,7 +309,7 @@
             //if authenticated add more roles
             roles = getRoles(context, dn, username, roles);
             for (String role : roles) {
-                groups.add(new GeronimoGroupPrincipal(role));
+                groups.add(role);
             }
         } catch (CommunicationException e) {
             close(context);



Mime
View raw message