You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Mark Thomas <ma...@apache.org> on 2009/07/17 02:16:21 UTC
[PROPOSAL] Remove Realm from GenericPrincipal
As a result of looking into
https://issues.apache.org/bugzilla/show_bug.cgi?id=40881, I discovered
that the only use made of the Realm attribute of GenericPrincipal is to
control whether or not a debug message is logged in RealmBase.hasRole()
Given that the Realm is the reason that GenericPrincipal is not
Serializable, I'd like to propose the following changes for Tomcat 7.
1. Remove the Realm from GenericPrincipal
2. Make GenericPrincipal Serializable
3. Take advantage of this to simplify the Cluster code
As a by product, this should also address bug 40881 by allowing any
Realm that uses any Serializable Principal to work with clustering.
Thoughts?
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PROPOSAL] Remove Realm from GenericPrincipal
Posted by David Jencks <da...@yahoo.com>.
On Jul 16, 2009, at 5:16 PM, Mark Thomas wrote:
> As a result of looking into
> https://issues.apache.org/bugzilla/show_bug.cgi?id=40881, I discovered
> that the only use made of the Realm attribute of GenericPrincipal is
> to
> control whether or not a debug message is logged in
> RealmBase.hasRole()
>
> Given that the Realm is the reason that GenericPrincipal is not
> Serializable, I'd like to propose the following changes for Tomcat 7.
>
> 1. Remove the Realm from GenericPrincipal
> 2. Make GenericPrincipal Serializable
> 3. Take advantage of this to simplify the Cluster code
>
> As a by product, this should also address bug 40881 by allowing any
> Realm that uses any Serializable Principal to work with clustering.
>
> Thoughts?
>
I'm not sure exactly how the GenericPrincipal fits into tomcat
security, but you might want to consider that jaspic requires that
whatever Principal is set up by the authentication context (and
communicated to the server through the somewhat bizarre mechanism of a
callback handler) must be the principal returned from
getUserPrincipal. My conclusion from this is that a reasonable
architecture involves some kind of UserIdentity object that contains
the identity info including the principal but that trying to enforce
usage of a particular principal class is not a good idea. cf the
jaspic integration I mentioned the other day.
thanks
david jencks
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
Re: [PROPOSAL] Remove Realm from GenericPrincipal
Posted by Mark Thomas <ma...@apache.org>.
Konstantin Kolinko wrote:
> 2009/7/17 Mark Thomas <ma...@apache.org>:
>> As a result of looking into
>> https://issues.apache.org/bugzilla/show_bug.cgi?id=40881, I discovered
>> that the only use made of the Realm attribute of GenericPrincipal is to
>> control whether or not a debug message is logged in RealmBase.hasRole()
>
> Looking at
> http://svn.apache.org/viewvc?view=rev&revision=301601
>
> That revision comment says about switch to commons-logging, but
> besides just that the
> "if (!(gp.getRealm() == this))" block was changed from returning false
> to just logging a message.
> I am not sure, that it was intentional.
The commit message indicates that it was intentional.
> Or there was really a bug with jaas, that this change fixed?
I think it is safe to assume Costin wasn't making this up.
> Is there a use case, besides replication, where a Principal is
> processed in context of some other realm, some other realm instance,
> different from the one that created it?
Single Sign On uses a shared Principal
>> Given that the Realm is the reason that GenericPrincipal is not
>> Serializable, I'd like to propose the following changes for Tomcat 7.
>>
>> 1. Remove the Realm from GenericPrincipal
>> 2. Make GenericPrincipal Serializable
>> 3. Take advantage of this to simplify the Cluster code
>
> I think "3." includes removing the
> o.a.c.cluster.session.SerializablePrincipal class
Yep :)
>> As a by product, this should also address bug 40881 by allowing any
>> Realm that uses any Serializable Principal to work with clustering.
>>
>
> I attached a patch to that issue, see comment #12:
> https://issues.apache.org/bugzilla/show_bug.cgi?id=40881#c12
> It is to allow a serializable Principal to be written out and read in
> during replication.
>
> Cannot test it, though, as I do not have relevant configuration now.
I see where you are going with that. That could be a potential backport
route for 6.0.x and 5.5.x but for 7.0.x I can't see a good reason to
include the Realm in the Principal and several for getting rid of it.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PROPOSAL] Remove Realm from GenericPrincipal
Posted by Konstantin Kolinko <kn...@gmail.com>.
2009/7/17 Mark Thomas <ma...@apache.org>:
> As a result of looking into
> https://issues.apache.org/bugzilla/show_bug.cgi?id=40881, I discovered
> that the only use made of the Realm attribute of GenericPrincipal is to
> control whether or not a debug message is logged in RealmBase.hasRole()
Looking at
http://svn.apache.org/viewvc?view=rev&revision=301601
That revision comment says about switch to commons-logging, but
besides just that the
"if (!(gp.getRealm() == this))" block was changed from returning false
to just logging a message.
I am not sure, that it was intentional.
Or there was really a bug with jaas, that this change fixed?
Is there a use case, besides replication, where a Principal is
processed in context of some other realm, some other realm instance,
different from the one that created it?
>
> Given that the Realm is the reason that GenericPrincipal is not
> Serializable, I'd like to propose the following changes for Tomcat 7.
>
> 1. Remove the Realm from GenericPrincipal
> 2. Make GenericPrincipal Serializable
> 3. Take advantage of this to simplify the Cluster code
I think "3." includes removing the
o.a.c.cluster.session.SerializablePrincipal class
>
> As a by product, this should also address bug 40881 by allowing any
> Realm that uses any Serializable Principal to work with clustering.
>
I attached a patch to that issue, see comment #12:
https://issues.apache.org/bugzilla/show_bug.cgi?id=40881#c12
It is to allow a serializable Principal to be written out and read in
during replication.
Cannot test it, though, as I do not have relevant configuration now.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PROPOSAL] Remove Realm from GenericPrincipal
Posted by Mark Thomas <ma...@apache.org>.
Konstantin Kolinko wrote:
> 2009/7/17 Mark Thomas <ma...@apache.org>:
>> As a result of looking into
>> https://issues.apache.org/bugzilla/show_bug.cgi?id=40881, I discovered
>> that the only use made of the Realm attribute of GenericPrincipal is to
>> control whether or not a debug message is logged in RealmBase.hasRole()
>>
>> Given that the Realm is the reason that GenericPrincipal is not
>> Serializable, I'd like to propose the following changes for Tomcat 7.
>>
>> 1. Remove the Realm from GenericPrincipal
>> 2. Make GenericPrincipal Serializable
>> 3. Take advantage of this to simplify the Cluster code
>>
>> As a by product, this should also address bug 40881 by allowing any
>> Realm that uses any Serializable Principal to work with clustering.
>>
>> Thoughts?
>>
>> Mark
>>
>
> One more note to this:
> since rev.791900 GenericPrincipal stores a reference to LoginContext,
> and LoginContext is not serializable.
Yeah, when I got around to progressing this, I was going to make it
transient.
> At least it means that that field will be transient, and probably
> that issue 39231 that that commit (with some followups) fixes
> won't/cannot be fixed for clusters. (I suppose so, though I may be
> wrong).
I think you're right although, assuming sticky sessions are being used,
this is a very narrow edge case I think we can live with. I'll add a
note to the bugzilla issue.
Mark
> http://svn.apache.org/viewvc?view=rev&revision=791900
> https://issues.apache.org/bugzilla/show_bug.cgi?id=39231
>
> Best regards,
> Konstantin Kolinko
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PROPOSAL] Remove Realm from GenericPrincipal
Posted by Konstantin Kolinko <kn...@gmail.com>.
2009/7/17 Mark Thomas <ma...@apache.org>:
> As a result of looking into
> https://issues.apache.org/bugzilla/show_bug.cgi?id=40881, I discovered
> that the only use made of the Realm attribute of GenericPrincipal is to
> control whether or not a debug message is logged in RealmBase.hasRole()
>
> Given that the Realm is the reason that GenericPrincipal is not
> Serializable, I'd like to propose the following changes for Tomcat 7.
>
> 1. Remove the Realm from GenericPrincipal
> 2. Make GenericPrincipal Serializable
> 3. Take advantage of this to simplify the Cluster code
>
> As a by product, this should also address bug 40881 by allowing any
> Realm that uses any Serializable Principal to work with clustering.
>
> Thoughts?
>
> Mark
>
One more note to this:
since rev.791900 GenericPrincipal stores a reference to LoginContext,
and LoginContext is not serializable.
At least it means that that field will be transient, and probably
that issue 39231 that that commit (with some followups) fixes
won't/cannot be fixed for clusters. (I suppose so, though I may be
wrong).
http://svn.apache.org/viewvc?view=rev&revision=791900
https://issues.apache.org/bugzilla/show_bug.cgi?id=39231
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org