JSPWiki
  1. JSPWiki
  2. JSPWIKI-464

JSPWiki authentication support for TextOutputCallback (display login messages on Login.jsp)

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0
    • Fix Version/s: 3.0
    • Labels:
      None
    • Environment:

      JSPWiki 3.0

      Description

      The current version of the JSPWiki JAAS implementation does not support TextOutputCallback's.
      JAAS offers several types of Callbacks, JSPWiki's CallbackHandler currently only uses the NameCallback and PasswordCallback.
      As a result the following scenario:
      Users try to login, the login fails but the user is not told for what reason.

      I have had lots of complaints about this behavior, especially from users who do not login very often but use the wiki mostly for reading.
      When they try to login, it fails, but the Login.jsp does not tell anything at all, not even that is has failed (C.M.A.).
      In most cases because either the userid has become inactive, is revoked, or the password is expired. The net effect is that the wiki is often not usable for updates.

      Now I know that giving this information (the failure reason) to the user is often considered a security trade off.
      But in an intranet environment this is very acceptable.

      I will attach a patch that solves this in the following way :

      • AuthenticationManager keeps a Hashtable of last loginMessages for each user.
      • The WikiCallbackHandler now also handles TextOutputCallbacks and sets the login result
      • If the login fails, the LoginActionBean first reads the loginMessage for the user, if it is not null, it is displayed, else you get the old behavior.

      The exploitation of TextOutputCallbacks is optional, the default LoginModule (supplied with JSPWiki) does not use them, and therefore it's behavior is unchanged.
      The installer has to supply a LoginModule that uses the TextOutputCallback to store the loginResult. (And off course we have one that uses it).

      (Andrew), can we take this patch in the trunk ?

      regards,
      Harry

      1. JSPWIKI-464.patch
        3 kB
        Harry Metske
      2. jspwiki-login-3.0.patch
        6 kB
        Harry Metske

        Issue Links

          Activity

          Hide
          Harry Metske added a comment -

          Fixed in 3.0.0-svn-42
          The LoginException thrown by the AuthenticationManager is now properly handled by the LoginActionBean (instead of showing the default localized login.error message).

          Show
          Harry Metske added a comment - Fixed in 3.0.0-svn-42 The LoginException thrown by the AuthenticationManager is now properly handled by the LoginActionBean (instead of showing the default localized login.error message).
          Hide
          Harry Metske added a comment -

          Andrew,

          I haven't checked in which versions of Tomcat this works (I run 6.0.16).
          I specified the following for the JAVA_OPTS:

          export JAVA_OPTS="
          -Dorg.apache.jasper.Constants.JSP_PACKAGE_NAME=org.apache.jzp
          -Dlog4j.debug=false -Xmx256m -Dcom.sun.management.jmxremote.port=5001
          -Dcom.sun.management.jmxremote.ssl=false
          -Dcom.sun.management.jmxremote.authenticate=false"

          It works, and if I look at the tomcat work directory I can see a directory
          org/apache/jzp.
          This is a code snippet from org.apache.jasper.Constants:

          *
          /**

          • The default package name for compiled jsp pages.
            */
            public static final String JSP_PACKAGE_NAME =
            System.getProperty("org.apache.jasper.Constants.JSP_PACKAGE_NAME",
            "org.apache.jsp");

          *I will also file a separate JSPWIKI JIRA, so we can keep the progress, and
          documentation together.

          But first some ice skating with the kids

          regards,
          Harry

          2009/1/2 Andrew Jaquith (JIRA) <jira@apache.org>

          Show
          Harry Metske added a comment - Andrew, I haven't checked in which versions of Tomcat this works (I run 6.0.16). I specified the following for the JAVA_OPTS: export JAVA_OPTS=" -Dorg.apache.jasper.Constants.JSP_PACKAGE_NAME=org.apache.jzp -Dlog4j.debug=false -Xmx256m -Dcom.sun.management.jmxremote.port=5001 -Dcom.sun.management.jmxremote.ssl=false -Dcom.sun.management.jmxremote.authenticate=false" It works, and if I look at the tomcat work directory I can see a directory org/apache/jzp. This is a code snippet from org.apache.jasper.Constants: * /** The default package name for compiled jsp pages. */ public static final String JSP_PACKAGE_NAME = System.getProperty("org.apache.jasper.Constants.JSP_PACKAGE_NAME", "org.apache.jsp"); *I will also file a separate JSPWIKI JIRA, so we can keep the progress, and documentation together. But first some ice skating with the kids regards, Harry 2009/1/2 Andrew Jaquith (JIRA) <jira@apache.org>
          Hide
          Andrew Jaquith added a comment -

          I've filed a bug with the Tomcat project. Here's the URL:

          https://issues.apache.org/bugzilla/show_bug.cgi?id=46462

          Show
          Andrew Jaquith added a comment - I've filed a bug with the Tomcat project. Here's the URL: https://issues.apache.org/bugzilla/show_bug.cgi?id=46462
          Hide
          Andrew Jaquith added a comment -

          I am personally happy to use the system properties override, so this isn't a big issue for me. My point was that, at the moment, that JSPWiki will not "just work" with Tomcat until the bug is fixed. We have expended tremendous developer energy working to eliminate system properties dependencies (remember "-Djava.security.policy"?).

          I could not get the workaround (setting the system property org.apache.jasper.Constants.JSP_PACKAGE_NAME) to work. In fact, I am 99% sure that there is does not. If anyone has success, please let me know what you did!

          Unless I am mistaken, it means we have no workaround, and our only remedy is a bug fix.

          Show
          Andrew Jaquith added a comment - I am personally happy to use the system properties override, so this isn't a big issue for me . My point was that, at the moment, that JSPWiki will not "just work" with Tomcat until the bug is fixed. We have expended tremendous developer energy working to eliminate system properties dependencies (remember "-Djava.security.policy"?). I could not get the workaround (setting the system property org.apache.jasper.Constants.JSP_PACKAGE_NAME) to work. In fact, I am 99% sure that there is does not. If anyone has success, please let me know what you did! Unless I am mistaken, it means we have no workaround, and our only remedy is a bug fix.
          Hide
          Janne Jalkanen added a comment -

          Besides, this is no worse than having an entire class of Debian boxes out there which can't run JSPWIki because they turn on security manager by default. There is a valid workaround by defining a system property until it is fixed in the mainline code. Since there is a fairly easy workaround this does not make it a showstopper and can be dealt with in documentation.

          Show
          Janne Jalkanen added a comment - Besides, this is no worse than having an entire class of Debian boxes out there which can't run JSPWIki because they turn on security manager by default. There is a valid workaround by defining a system property until it is fixed in the mainline code. Since there is a fairly easy workaround this does not make it a showstopper and can be dealt with in documentation.
          Hide
          Janne Jalkanen added a comment -

          Why would "Apache JSPWiki" reside in the "wiki" -package? In addition, if we change the project name, we need to go through entire name change process, namecheck and legal stuff, not to mention that we lose the "jspwiki" brand that has been accumulate over the years.

          This is clearly a bug in Jasper and needs to be fixed on their part. Please raise a bug in the Jasper bug tracker. I think it can be fixed fairly fast, if need be.

          I remember in Tomcat 4.x there was a problem with us calling our template "default", since it resulted in package names which were illegal. It was fixed, and we didn't have to change anything.

          I don't think we should change the project name because of one buggy implementation. We should direct people to use Jetty or some other container instead.

          We don't have to worry about the "market". Ain't nobody getting any money out of this. We need to do the right thing.

          Show
          Janne Jalkanen added a comment - Why would "Apache JSPWiki" reside in the "wiki" -package? In addition, if we change the project name, we need to go through entire name change process, namecheck and legal stuff, not to mention that we lose the "jspwiki" brand that has been accumulate over the years. This is clearly a bug in Jasper and needs to be fixed on their part. Please raise a bug in the Jasper bug tracker. I think it can be fixed fairly fast, if need be. I remember in Tomcat 4.x there was a problem with us calling our template "default", since it resulted in package names which were illegal. It was fixed, and we didn't have to change anything. I don't think we should change the project name because of one buggy implementation. We should direct people to use Jetty or some other container instead. We don't have to worry about the "market". Ain't nobody getting any money out of this. We need to do the right thing.
          Hide
          Harry Metske added a comment -

          Sorry, my comment showed up here as a result of an email reply to Janne, so the comments are a bit out of order.

          The answer is yes, org.apache.wiki would work.

          Show
          Harry Metske added a comment - Sorry, my comment showed up here as a result of an email reply to Janne, so the comments are a bit out of order. The answer is yes, org.apache.wiki would work.
          Hide
          Andrew Jaquith added a comment -

          I didn't understand your comment, Harry... org.apache.wiki would work, wouldn't it?

          Show
          Andrew Jaquith added a comment - I didn't understand your comment, Harry... org.apache.wiki would work, wouldn't it?
          Hide
          Harry Metske added a comment -

          it's even simpler, have a look at the code snippet, it is just checking

          if( !name.startsWith(Constants.JSP_PACKAGE_NAME) )

          and Constants.JSP_PACKAGE_NAME has the value org.apache.jsp (with no
          trailing dot).

          Any good suggestions for our new package names

          I can file a separate JIRA for this issue.

          regards,
          Harry

          2009/1/1 Janne Jalkanen (JIRA) <jira@apache.org>

          Show
          Harry Metske added a comment - it's even simpler, have a look at the code snippet, it is just checking if( !name.startsWith(Constants.JSP_PACKAGE_NAME) ) and Constants.JSP_PACKAGE_NAME has the value org.apache.jsp (with no trailing dot). Any good suggestions for our new package names I can file a separate JIRA for this issue. regards, Harry 2009/1/1 Janne Jalkanen (JIRA) <jira@apache.org>
          Hide
          Andrew Jaquith added a comment -

          While we could certaInly ask the Tomcat developers to change this, I don't think it makes sense to do so, for two reasons:

          • It's likely to take 3-6 months to get the bug fixed, and we need to get an Apache release out sooner.
          • Even if it's fixed in a future version of Tomcat, the vast majority of people will still have older (i.e., not bleeding edge) versions of Tomcat. Telling 99% of the "addressable market" that they must upgrade to the latest version just to use JSPWiki is a non-starter.

          For these reasons, we should consider a different package name scheme, like "org.apache.wiki." Of course, Janne is really the person who would need to approve this, because it would be such a break <sniff> from tradition.

          Show
          Andrew Jaquith added a comment - While we could certaInly ask the Tomcat developers to change this, I don't think it makes sense to do so, for two reasons: It's likely to take 3-6 months to get the bug fixed, and we need to get an Apache release out sooner. Even if it's fixed in a future version of Tomcat, the vast majority of people will still have older (i.e., not bleeding edge) versions of Tomcat. Telling 99% of the "addressable market" that they must upgrade to the latest version just to use JSPWiki is a non-starter. For these reasons, we should consider a different package name scheme, like "org.apache.wiki." Of course, Janne is really the person who would need to approve this, because it would be such a break <sniff> from tradition.
          Hide
          Harry Metske added a comment -

          sorry, here is a better readable version of the snippet :

          {
          
                  if( !name.startsWith(Constants.JSP_PACKAGE_NAME) ) {
                      // Class is not in org.apache.jsp, therefore, have our
                      // parent load it
                      clazz = parent.loadClass(name);            
                      if( resolve )
                          resolveClass(clazz);
                      return clazz;
                  }
          
                  return findClass(name);
              }
          
          
          
          Show
          Harry Metske added a comment - sorry, here is a better readable version of the snippet : { if( !name.startsWith(Constants.JSP_PACKAGE_NAME) ) { // Class is not in org.apache.jsp, therefore, have our // parent load it clazz = parent.loadClass(name); if( resolve ) resolveClass(clazz); return clazz; } return findClass(name); }
          Hide
          Harry Metske added a comment -

          The following code snippet from the JasperLoader (http://www.docjar.com/html/api/org/apache/jasper/servlet/JasperLoader.java.html) explains this.
          The JSP_PACKAGE_NAME does not contain a trailing dot, so org.apache.jsp is the same as org.apache.jspwiki. The parent classloader off course canot find the class

          if( !name.startsWith(Constants.JSP_PACKAGE_NAME) )

          { // Class is not in org.apache.jsp, therefore, have our // parent load it clazz = parent.loadClass(name); if( resolve ) resolveClass(clazz); return clazz; }

          return findClass(name);
          }

          We could ask the tomcat developers to change this, but still then it would take a long time before this is out in the field, what do we do ?

          Show
          Harry Metske added a comment - The following code snippet from the JasperLoader ( http://www.docjar.com/html/api/org/apache/jasper/servlet/JasperLoader.java.html ) explains this. The JSP_PACKAGE_NAME does not contain a trailing dot, so org.apache.jsp is the same as org.apache.jspwiki. The parent classloader off course canot find the class if( !name.startsWith(Constants.JSP_PACKAGE_NAME) ) { // Class is not in org.apache.jsp, therefore, have our // parent load it clazz = parent.loadClass(name); if( resolve ) resolveClass(clazz); return clazz; } return findClass(name); } We could ask the tomcat developers to change this, but still then it would take a long time before this is out in the field, what do we do ?
          Hide
          Janne Jalkanen added a comment -

          Sounds like a regexp bug in Jasper - perhaps they are checking against "org.apache.jsp" instead of "org.apache.jsp."?

          Show
          Janne Jalkanen added a comment - Sounds like a regexp bug in Jasper - perhaps they are checking against "org.apache.jsp" instead of "org.apache.jsp."?
          Hide
          Harry Metske added a comment -

          I think I can do a test tomorrow.

          About the Classloading issue, bad news and good news :

          We seem to have a naming collision between our org.apache.jspwiki package and the Jasper JSP Compiler.
          The default package name for servlets compiled from JSP's which is org.apache.jsp. This causes the classloading problem.
          I will dig further for the how, why and what.

          The good news is that you can change this package name, so there is a workaround, use the following Java System Property for Tomcat :

          -Dorg.apache.jasper.Constants.JSP_PACKAGE_NAME=org.apache.jzp

          Have a happy and healthy 2009

          Show
          Harry Metske added a comment - I think I can do a test tomorrow. About the Classloading issue, bad news and good news : We seem to have a naming collision between our org.apache.jspwiki package and the Jasper JSP Compiler. The default package name for servlets compiled from JSP's which is org.apache.jsp. This causes the classloading problem. I will dig further for the how, why and what. The good news is that you can change this package name, so there is a workaround, use the following Java System Property for Tomcat : -Dorg.apache.jasper.Constants.JSP_PACKAGE_NAME=org.apache.jzp Have a happy and healthy 2009
          Hide
          Andrew Jaquith added a comment -

          Fix committed in 3.0.0-svn 41. Harry, could you test this?

          Show
          Andrew Jaquith added a comment - Fix committed in 3.0.0-svn 41. Harry, could you test this?
          Hide
          Harry Metske added a comment -

          Glad I'm not the only one anymore having the classloading problem
          I have tried everything I could think off, one thing left is try to recreate in Glassfish ( I already did Tomcat and Geronimo, but I think both use the same JSP engine).
          I can only work around it by moving WikiPage to the old package again.

          About the generalization, I did that this morning already, it involves a bunch of try catch clauses in the AuthenticationManager .

          regards,
          Harry

          Show
          Harry Metske added a comment - Glad I'm not the only one anymore having the classloading problem I have tried everything I could think off, one thing left is try to recreate in Glassfish ( I already did Tomcat and Geronimo, but I think both use the same JSP engine). I can only work around it by moving WikiPage to the old package again. About the generalization, I did that this morning already, it involves a bunch of try catch clauses in the AuthenticationManager . regards, Harry
          Hide
          Andrew Jaquith added a comment -

          Harry,

          This is a much better patch; thanks. After looking at it, though, I think we ought to make it more generalizable so that it works with all LoginModules, not just our own. So, let me think a bit more about the approach. Shouldn't take me too long, though.

          I'm also stuck on 3.0 because of the same damned classpath/classloader problem you have, and it's driving me crazy. It started happening just after we moved WikiPage to the org.apache.jspwiki.api package. I can't seem to fix it, no matter what I do.

          Show
          Andrew Jaquith added a comment - Harry, This is a much better patch; thanks. After looking at it, though, I think we ought to make it more generalizable so that it works with all LoginModules, not just our own. So, let me think a bit more about the approach. Shouldn't take me too long, though. I'm also stuck on 3.0 because of the same damned classpath/classloader problem you have, and it's driving me crazy. It started happening just after we moved WikiPage to the org.apache.jspwiki.api package. I can't seem to fix it, no matter what I do.
          Hide
          Harry Metske added a comment -

          Well, that is indeed a better approach, thanks
          Attached a new patch.

          AuthenticationManager has several calls to doJAASLogin(), they don't handle the WikiSecurityException, so I now only throw this exception if the LoginModule is not an instance of AbstractLoginModule.

          I also included an update to the CoreResources.properties :

          login.error=Login failed:

          {2}

          If I understand the Stripes docs well, the {2}

          should be replaced with the exception text.

          I have tested only on 2.8.2 (required some extra patching on Login.jsp).
          I can't test the ActionBean on 3.0, because I'm still stuck with http://mail-archives.apache.org/mod_mbox/incubator-jspwiki-dev/200812.mbox/%3C3a6c97f00812221259l60281190le9ea59fa6b7c8b6@mail.gmail.com%3E

          Let me know what you think.

          regards,
          Harry

          Show
          Harry Metske added a comment - Well, that is indeed a better approach, thanks Attached a new patch. AuthenticationManager has several calls to doJAASLogin(), they don't handle the WikiSecurityException, so I now only throw this exception if the LoginModule is not an instance of AbstractLoginModule. I also included an update to the CoreResources.properties : login.error=Login failed: {2} If I understand the Stripes docs well, the {2} should be replaced with the exception text. I have tested only on 2.8.2 (required some extra patching on Login.jsp). I can't test the ActionBean on 3.0, because I'm still stuck with http://mail-archives.apache.org/mod_mbox/incubator-jspwiki-dev/200812.mbox/%3C3a6c97f00812221259l60281190le9ea59fa6b7c8b6@mail.gmail.com%3E Let me know what you think. regards, Harry
          Hide
          Andrew Jaquith added a comment -

          Sorry, I should have said "patching WikiCallbackHandler" not "extending."

          Show
          Andrew Jaquith added a comment - Sorry, I should have said "patching WikiCallbackHandler" not "extending."
          Hide
          Andrew Jaquith added a comment -

          I think there is a better way to do this. I do not want to add a holding-tank Map AuthenticationManager for messages that should be propagated immediately back to the client. As proposed, the patch is a recipe for a memory leak, and we have enough of those already.

          I'd recommend extending WikiCallBackHandler to look for TextOutputCallbacks. If the message type is of type ERROR, it should throw a LoginException (which is what normally signifies login failure). The LoginModule should pass this back through to AuthenticationManager.

          The LoginException itself should be caught by doJAASLogin(), and rethrown as a WikiSecurityException, which our LoginActionBean already catches (without modification to existing code...).

          Harry, can you re-work?

          Show
          Andrew Jaquith added a comment - I think there is a better way to do this. I do not want to add a holding-tank Map AuthenticationManager for messages that should be propagated immediately back to the client. As proposed, the patch is a recipe for a memory leak, and we have enough of those already. I'd recommend extending WikiCallBackHandler to look for TextOutputCallbacks. If the message type is of type ERROR, it should throw a LoginException (which is what normally signifies login failure). The LoginModule should pass this back through to AuthenticationManager. The LoginException itself should be caught by doJAASLogin(), and rethrown as a WikiSecurityException, which our LoginActionBean already catches (without modification to existing code...). Harry, can you re-work?
          Hide
          Janne Jalkanen added a comment -

          You could use WikiEngine.setAttribute() to communicate the messages to avoid adding yet another message queue; but otherwise this looks good to me.

          Show
          Janne Jalkanen added a comment - You could use WikiEngine.setAttribute() to communicate the messages to avoid adding yet another message queue; but otherwise this looks good to me.

            People

            • Assignee:
              Harry Metske
              Reporter:
              Harry Metske
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development