Re: [PROPOSAL] Remove Realm from GenericPrincipal
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=revrevision=791900 https://issues.apache.org/bugzilla/show_bug.cgi?id=39231 Best regards, Konstantin Kolinko - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PROPOSAL] Remove Realm from GenericPrincipal
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=revrevision=791900 https://issues.apache.org/bugzilla/show_bug.cgi?id=39231 Best regards, Konstantin Kolinko - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PROPOSAL] Remove Realm from GenericPrincipal
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=revrevision=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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PROPOSAL] Remove Realm from GenericPrincipal
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=revrevision=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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PROPOSAL] Remove Realm from GenericPrincipal
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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
