[ 
http://issues.apache.org/jira/browse/GERONIMO-677?page=comments#action_12317015 
] 

Kevan Miller commented on GERONIMO-677:
---------------------------------------

The problem lies in org.apache.geronimo.security.realm.providers.SQLLoginModule 
(that's why David wasn't able to reproduce using Properties File-based login.

SQLLoginModule.login() adds GroupPrincipals to a "groups" HashSet. The 
GroupPrincipals from "groups" are then retrieved from the HashSet during 
commit() processing and added to the Subject. The problem is that "groups" is 
never reset between logins. So, any new login will get all preceding 
GroupPrincipals for this LoginModule instance... 8-{ 

In Ivan's example, "user" logs in and the user principal is added to "groups". 
This user principal is added to the Subject during commit() processing. When 
"manager" logs in, the manager principal is added to "groups". When commit() is 
called both the "user" and "manager" principals are added to the Subject...

The following changes to SQLLoginModule would seem to address the problem:

Index: src/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java
===================================================================
--- src/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java   
(revision 225640)
+++ src/java/org/apache/geronimo/security/realm/providers/SQLLoginModule.java   
(working copy)
@@ -170,12 +170,15 @@
             principals.add(iter.next());
         }
 
+        groups.clear();
+
         return true;
     }
 
     public boolean abort() throws LoginException {
         cbUsername = null;
         cbPassword = null;
+        groups.clear();
 
         return true;
     }

Note that this is simply addressing the problem at hand. I'm not familiar with 
JAAS. So, it's possible that I don't fully grok (e.g. perhaps the same 
LoginModule shouldn't be invoked for these separate logins, or groups should be 
cleared at some other time, etc.). Also, I'm not at all convinced that 
SQLLoginModule is behaving properly wrt logout(). I'm certain that it's not 
very efficient (e.g. iterating over all users during login()). Ah, I see this 
inefficiency is listed as a "Future Change" in the Security section of the Wiki 
(http://wiki.apache.org/geronimo/Security)

> Repeated login (after session invalidation) with different credentials 
> results in incorrect role set.
> -----------------------------------------------------------------------------------------------------
>
>          Key: GERONIMO-677
>          URL: http://issues.apache.org/jira/browse/GERONIMO-677
>      Project: Geronimo
>         Type: Bug
>   Components: security
>     Versions: 1.0-M4
>     Reporter: Ivan Dubrov
>     Assignee: David Jencks
>     Priority: Critical
>      Fix For: 1.0-M5
>  Attachments: db_create.sql, geronimo-application.xml, my-changes.patch, 
> test.zip
>
> Consider we have two users, "user" with role "user" and "manager" with role 
> "manager" and two secured areas /user/* and /manager/*, so only "user"'s can 
> access pages with URL /user/* and only "manager"'s can access pages with URL 
> /manager/*.
> If we log in as "user", we can access only /user/* pages, "403 Forbidden" if 
> we try to access /manager/* pages. It is OK. 
> Now, if we clean the session (request.getSession().invalidate()), we will be 
> logged out, so we cannot access nor /user/*, nor /manager/* pages - server 
> redirects to the login page. It is OK.
> But if we login second time, as a "manager", we can access both page sets - 
> /user/* and /manager/*! It means that authenticated user owns both roles 
> "user" and "manager", but this is impossible combination!

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira

Reply via email to