Some changes

[Remy Maucherat <re...@apache.org>]

I will propose making some changes:

- Add Jan's patch to have an ISE thrown for Session.getId if the session 
is expired. However, it is important for container internal components 
to be able to call getId, even if the session is invalidated. As a 
result, I propose adding a Session.getIdInternal method (or propose 
another name) which would do the same as the old getId

- I reported some mess with the JAAS realm some time earlier, which has 
to maintain a map of principals, which is messy and makes code more 
complex (as well as needlessly leaking memory). We do have the exact 
same issue in JBoss, as we use JAAS as well. Scott Stark proposed 
storing the user principal to be returned by Request.getUserPrincipal 
inside the GenericPrincipal itself, while the regular GenericPrincipal 
would be used for calls to hasRole (removing the need for the JAAS realm 
to override the method).
This would mean adding a new constructor to GenericPrincipal:
     /**
      * Construct a new Principal, associated with the specified Realm, 
for the
      * specified username and password, with the specified role names
      * (as Strings).
      *
      * @param realm The Realm that owns this principal
      * @param name The username of the user represented by this Principal
      * @param password Credentials used to authenticate this user
      * @param roles List of roles (must be Strings) possessed by this user
      * @param userPrincipal - the principal to be returned from the 
request getUserPrincipal call if not null.
      */
     public GenericPrincipal(Realm realm, String name, String password,
                             List roles, Principal userPrincipal)

That's all the proposals I have for now.

Rémy

---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org

Re: Some changes

[Remy Maucherat <re...@apache.org>]

Remy Maucherat wrote:
> I will propose making some changes:
> 
> - Add Jan's patch to have an ISE thrown for Session.getId if the session 
> is expired. However, it is important for container internal components 
> to be able to call getId, even if the session is invalidated. As a 
> result, I propose adding a Session.getIdInternal method (or propose 
> another name) which would do the same as the old getId
> 
> - I reported some mess with the JAAS realm some time earlier, which has 
> to maintain a map of principals, which is messy and makes code more 
> complex (as well as needlessly leaking memory). We do have the exact 
> same issue in JBoss, as we use JAAS as well. Scott Stark proposed 
> storing the user principal to be returned by Request.getUserPrincipal 
> inside the GenericPrincipal itself, while the regular GenericPrincipal 
> would be used for calls to hasRole (removing the need for the JAAS realm 
> to override the method).
> This would mean adding a new constructor to GenericPrincipal:
>     /**
>      * Construct a new Principal, associated with the specified Realm, 
> for the
>      * specified username and password, with the specified role names
>      * (as Strings).
>      *
>      * @param realm The Realm that owns this principal
>      * @param name The username of the user represented by this Principal
>      * @param password Credentials used to authenticate this user
>      * @param roles List of roles (must be Strings) possessed by this user
>      * @param userPrincipal - the principal to be returned from the 
> request getUserPrincipal call if not null.
>      */
>     public GenericPrincipal(Realm realm, String name, String password,
>                             List roles, Principal userPrincipal)
> 
> That's all the proposals I have for now.

Ok, so nobody cares ;) I'll commit the changes then.

Another thing:
The purpose of DataSourceRealm.authenticate overriding seems to be to 
use only one "connection" for the full authentication process. This 
seems to me like a rather pointless optimization, and straight 
delegation to the RealmBase should be used instead.
I added this myself as part of an omnibus patch which was submitted to 
fix a very serious problem (I was anxious to commit it ...), so I'm 
partially vetoing myself here :)
The change would be to remove the two DataSourceRealm.authenticate methods.

Comments ?

Rémy

---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org