This bug report was previously posted to tomcat-dev as "[PATCH] JAASRealm fixes" on 24-Apr-2004. ---- These patches enable JAASRealm to work as advertised. In particular, JAASRealm can use user-specified Principal classes for both user Principals and group Principals, just like the documentation says it is supposed to be able to do. In addition, I have added support for digested passwords similar to the way the other Realm implementations work. I have patched three files: JAASRealm.java, JAASCallbackHandler.java, and LocalStrings.properties. I have unquestionably fouled up the @version Javadoc tags... one assumes the responsible parties (surely not me!) can fix these prior to committing.
Created attachment 11354 [details] JAASRealm.patch
Created attachment 11355 [details] JAASCallbackHandler.patch
Created attachment 11356 [details] LocalStrings.patch
I should make clear that the patch files I submitted affect 5.0.19, but were diffed against those from CVS HEAD, on the morning of 4/14/2004. Sorry if that was not readily apparent.
Please submit test cases for this patch if possible.
No test cases? This item may be closed if none are submitted.
Two issues preventing me from writing test cases: 1) It isn't clear how to do that. There isn't much documentation on how testing works; I was a but put off by that initially. After some investigation, I found jakarta-tomcat-catalina/tester/src/bin/tester.xml, which indeed appears to be an Ant file that performs out-of-container HTTP testing. So, sure, I can see how, if the container were configured to use my JAASRealm patch, I could re-test the authentication tests in jakarta-tomcat- catalina/tester/src/tester/org/apache/tester/Authentication0*.java. Sounds simple, but it all turns on the phrase "if the container were configured to use my JAASRealm patch." All of the tests use, by default, uses UserDatabaseRealm. How could I make Tomcat use JAASRealm instead? It appears that I would need to either: - Create a custom context descriptor for the "tester" webapp that uses JAASRealm - Edit the existing conf/server.xml file ... as well as cause the test script to bounce the server and re-load it with the new context descriptor. Aha. To do this, now I need to hack the tester.xml Ant script. I have *never* written a Tomcat test case before, so this is just my best guess on what it would take. Am I wrong? 2) I'm not sure what it would contain. Remember that JAASRealm needs an implementation of a JAAS LoginModule to function. There isn't one supplied with Tomcat, so this means I'd need to: - Create a dummy LoginModule that authenticates certain hard-coded users but not others (e.g., "Fred/ bedrock" but not "Barney/feet") - Create a "sample" LoginModule that authenticates against an existing user registry, like tomcat- users.xml. But that seems like real work to me; I might as well port MemoryRealm to JAAS while I'm at it. (Which is essentially what I'd be doing). Frankly, this seems like a lot of bother for a patch that I *know* works; is has been in production on my personal wiki for about three months. I can do it if you want to, as long as you accept that this means that there will be quite a bit of changes to certain build files, and other things. What is the best path forward? The key issue here is lack of flexibility in the test scripts, which don't permit changing Realms for exercising authentication tests.
Typical test cases submitted for Tomcat are WAR files. They contain the code to be tested along with the META-INF/context.xml file which is the Context element with the proper declarations, e.g. for a Realm. Don't bother using the Tester approach, that's wasted effort on build files. For this case a dummy LoginModule with hard-coded user names and passwords is fine. Also for this case, since the Realm has to be in a common classloader repository, a single WAR won't be possible, so maybe a separate JAR. Basically, we ask for whatever you would use to unit-test your own patch. I understand it works for you, and that's great, and if you don't unit-test that's also OK (I sometimes don't do it either, we're not perfect ;)). But we do prefer to have unit tests before committing into the core Tomcat code. I also wanted to assess your interest in this: I'm glad you responded, and quickly. If I ask for tests, give it a couple of months with no response, then ask again and get no response within a month or so, I sometimes close the bugzilla issue for lack of interest. Because you responded we can keep this open and resolve it.
After a bit (!) of a hiatus, patches against the most recent release (5.0.28), and matching test cases, are progressing. I expect to have both submitted early next week.
Created attachment 12761 [details] Small Ant project that builds a test case WAR, dummy JAAS LoginModule JAR and associated context descriptors.
Created attachment 12762 [details] JAASRealm.java (whole file, not a diff)
Created attachment 12763 [details] JAASCallbackHandler.java (whole file; not a diff)
Created attachment 12764 [details] LocalStrings.properties (whole file; not a diff)
Created attachment 12765 [details] JAASRealm.java (patch cf. tomcat 5.028 release)
Created attachment 12766 [details] JAASCallbackHandler (patch cf. tomcat 5.028 release)
Created attachment 12767 [details] LocalStrings.properties (patch cf. tomcat 5.028 release)
I've uploaded a small Ant project contains a series of test cases for the JAASRealm patches. The patches were made against the source tree for Tomcat 5.0.28. First, a short explanation of what the patch itself is all about. As several observers have noted, the current JAASRealm implementation doesn't actually deliver on all of the functionality it promises, particularly with respect to its ability to return Principals of user-specified types. In addition, the current implementation is limited by its inability to hash passwords. The patch fixes both of these issues. Files that have been modified (relative to jakarta-tomcat-catalina/catalina/src/share): org/apache/catalina/realm/JAASRealm.java org/apache/catalina/realm/LocalStrings.properties org/apache/catalina/realm/JAASCallbackHandler.java See the other attachments to this bug for the patch files, whole files, and test case tar file.
Patch applied on both TOMCAT_5_0 and HEAD (Tomcat 5.5) branches. Thanks for contributing.
It looks like the most recent commit of JAASRealm has a slight problem in it. The whole-file version I submitted has these lines, starting at line 536: if (userPrincipal == null && userClasses.contains(principalClass)) { userPrincipal = principal; if( log.isDebugEnabled() ) { log.debug(sm.getString("jaasRealm.userPrincipalSuccess", principal.getName())); } } if (roleClasses.contains(principalClass)) { roles.add(principal); if( log.isDebugEnabled() ) { log.debug(sm.getString("jaasRealm.rolePrincipalAdd", principal.getName())); } } The current file in HEAD looks like this (also starting at 536): if (userPrincipal == null && userClasses.contains(principalClass)) { userPrincipal = principal; if( log.isDebugEnabled() ) { log.debug(sm.getString("jaasRealm.userPrincipalSuccess", principal.getName())); } } if (roleClasses.contains(principalClass)) { roles.add(principal.getName()); } if (roleClasses.contains(principalClass)) { roles.add(principal); if( log.isDebugEnabled() ) { log.debug(sm.getString("jaasRealm.rolePrincipalAdd", principal.getName())); } } The middle chunk of code (3 lines) shouldn't be there. It is adding a String to what is supposed to be a collection of Principal objects. This is causing downstream calls to hasRole(Principal, String) to choke and die with a ClassCastException. The patch file I submitted also has this middle chunk marked for removal (-), so I think this was just a mistake in the commit process.
My mistake -- good catch. Thank you for pointing it out. I've fixed it on both HEAD and TOMCAT_5_0.