Regarding JAAS logout patch (for bug 39231)

[Konstantin Kolinko <kn...@gmail.com>]

The following patch is currently in voting for 5.5 and 6.0

* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=39231
  http://svn.apache.org/viewvc?rev=791900&view=rev
  http://svn.apache.org/viewvc?rev=792446&view=rev (Filip's suggestion)
  JAAS LoginContext expects a call to logout()

I have some comments on it.

1. It changes signature of JAASRealm.createPrincipal(), adding the
third argument,
and that will break classes that override that method.

While evaluating this case I stumbled upon a library that will be
broken by this change:
JOSSO (Java Open Single Sign-On Project).

Here is their instruction for configuring it on Tomcat 6:
http://www.josso.org/confluence/display/JOSSO1/Tomcat+6.0

Here are the sources:
https://josso.svn.sourceforge.net/svnroot/josso/josso1/trunk/agents/josso-tomcat60-agent/src/main/java/org/josso/tc60/agent/jaas/
- see the CatalinaJAASRealm and CatalinaSSOUser classes.

I am not sure that JAASRealm is part of our official API, but if we
can avoid breakage, it is better to do so.

The way that I see to implement our change and to be compatible with
their code is to add a setter to GenericPrincipal that will accept
LoginContext,  instead of passing that argument to the constructor.


2. This logout feature, that this patch implements, - it s actually
more like a cleanup.

All the LoginModule s that I've seen do on logout() is clearing
assigned roles and destroying credentials. That is good, because it
won't have side effects. (If there were any, this feature would have
to be configurable). Though "logout" is a bit too loud name for it.

Documentation on implementing LoginModule#logout() is here:
http://java.sun.com/javase/6/docs/technotes/guides/security/jaas/JAASLMDevGuide.html#logout


Best regards,
Konstantin Kolinko

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

Re: Regarding JAAS logout patch (for bug 39231)

[Mark Thomas <ma...@apache.org>]

Konstantin Kolinko wrote:
> The following patch is currently in voting for 5.5 and 6.0
> 
> * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=39231
>   http://svn.apache.org/viewvc?rev=791900&view=rev
>   http://svn.apache.org/viewvc?rev=792446&view=rev (Filip's suggestion)
>   JAAS LoginContext expects a call to logout()
> 
> I have some comments on it.
> 
> 1. It changes signature of JAASRealm.createPrincipal(), adding the
> third argument,
> and that will break classes that override that method.
> 
> While evaluating this case I stumbled upon a library that will be
> broken by this change:
> JOSSO (Java Open Single Sign-On Project).

That is a good enough reason for a veto in my book. I'll fix the patch.

Mark



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