Kuali Rice Development
  1. Kuali Rice Development
  2. KULRICE-8417

Make it so that you can add link text to the question framework

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.1.3
    • Component/s: Development
    • Security Level: Public (Public: Anyone can view)
    • Labels:
      None
    • Similar issues:
      KULRICE-148make it so folks can add multiple buttons to transactional docs
      KULRICE-5437UIF Framework - Add standard markup for table semantics
      KULRICE-8917Iterate on validation framework UI
      KULRICE-4536Look into using a standard markup language for question text
      KULRICE-5822Make RemoteFieldsHolder so it can be used as a collection
      KULRICE-6136Exercise the Kuali Question Framework in the sample application
      KULRICE-1460Implement best of breed for maint and transactional doc frameworks
      KULRICE-4709Text based button generation
      KULRICE-6435Make it so that Rice 2.0 can shut down cleanly
      KULRICE-1343Question framework doesn't always return to where the user would want after the user has answered the question
    • Rice Module:
      KNS
    • Application Requirement:
      KFS
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Review Completed

      Description

      Right now you can pass text to a url for the question framework. When adding links however, this would cause issues if we allowed it to be passed via URL because of the potential for XSS. See the following conversation below for a suggested fix to allow this ability to be added to question framework:

      From: Smith, James K.
      Sent: Monday, December 19, 2011 9:36 AM
      To: Warren Liang
      Cc: Jonathan Keller; Westfall, Eric Curtis; Bennett III, James William; Stapleton, Heather J
      Subject: Re: FW: security question

      That makes sense to me. Thanks Warren!

      On 12/18/2011 7:42 PM, Warren Liang wrote:
      Hey James,

      You are probably right about concurrency issues w/ the automatic removal. Here's another idea,
      add the String with the UserSession.addObject() object, which will return a unique "object key"
      every time something's added. Then you won't have to bother making the SHA hash.

      W
      On Fri, Dec 16, 2011 at 7:20 AM, James Smith wrote:
      Would this raise concurrency issues? Let's say a user is doing similar documents in two different
      tabs of their browser, hitting the same question screen - first in one tab, then in the next. In that
      case...well, the String would have to be held in either a ThreadLocal Map (in which case, I begin
      to wonder if we need a map at all); or the hash could have a timestamp as part of what it hashed
      so every key is basically guaranteed to be unique; or...well, that's why I was suggesting that
      question texts get evicted if it isn't used for a "reasonable" period of time... Even if the question
      text was put in a session with a hash as a key, this would be a problem; you'd need to generate a
      unique guid I'm guessing. Anyway: guess my idea still has problems!

      On 12/15/2011 4:48 PM, Warren Liang wrote:
      I like the solution.

      If fixes a second (and I think more important than XSS) issue: that of text injection. If you're
      worried about the map becoming too large, you can just remove the map entry once it's been
      used.

      Warren
      On Thu, Dec 15, 2011 at 1:00 PM, Smith, James K. wrote:
      Hi all. Ailish, Heather, and I just had a meeting where we were discussing the question text
      security issue. UA had run into an issue with this earlier while I was consulting so I had thought
      of a solution (but they didn�t have time to implement); Ailish liked it though and she wanted me
      to share.

      First, Jonathan is correct in that at some point, questions may just be lightboxes and this whole
      stupid problem would just go away. Yay! For the time being though, we looked at some pre
      rules and Jonathan�s solution won�t work: we can�t pass property names because some question
      texts come from properties, some from parameters, and in my (and UA�s!) favorite: from the
      VendorType BO. So we can�t pass around a property name and have that work.

      My solution was that the pre rules would interpolate the text and pass that to, say,
      askOrAnalyzeYesNoQuestion. askOrAnalyzeYesNoQuestion would NOT simply pass the text
      on as a parameter. Instead, it would find, say, the SHA hash of the message and then go to a
      special map in GlobalVariables where it would put the question text as the value with the hash as
      the key. Then it would pass the hash as a parameter. When the question displayed, it would take
      the hash, go back to GlobalVariables, retrieve the actual question text and then display it. I had
      concerns about growth of that question text Map over time but certainly it could be a special map
      which evicted entries after a certain reasonable period of time.

      Does this seem like an amenable solution?

        Activity

        Hide
        Eric Westfall added a comment -

        Marked as pending KTI review because this should be reviewed by the KTI as well as a discussion on what if anything needs to be done about this in terms of KRAD.

        Show
        Eric Westfall added a comment - Marked as pending KTI review because this should be reviewed by the KTI as well as a discussion on what if anything needs to be done about this in terms of KRAD.
        Hide
        Eric Westfall added a comment -

        Link to related IU jira for those who can see it...

        Show
        Eric Westfall added a comment - Link to related IU jira for those who can see it... https://uisapp2.iu.edu/jira-prd/browse/EN-2287
        Hide
        Jerry Neal (Inactive) added a comment -

        Just a note to the review for KRAD. In KRAD we display the questions in a lightbox on the same page. Nothing is passed via a URL.

        Show
        Jerry Neal (Inactive) added a comment - Just a note to the review for KRAD. In KRAD we display the questions in a lightbox on the same page. Nothing is passed via a URL.
        Hide
        Eric Westfall added a comment - - edited

        Switching KTI Review Status to "Review Completed". This was reviewed in the KTI on November 7, 2012.

        Initial discussion was to determine whether it was really necessary to change the model to use User Session storage for the question. After some investigation and discussion with the group, the proposed plan is as follows:

        1. Leave the KNS question framework as is (where questionText is passed as a request parameter)
        2. Add support in WebUtils.filterHtmlAndReplaceRiceMarkup for a syntax like [a href] where href would be an unquoted link
        3. If the href contains the word "javascript", do not convert/replace it. This will prevent XSS attacks if someone where to fashion question text like the following:
        http://.../kr/questionPrompt.do?methodToCall=start&questionType=confirmationQuestion&questionText=This is my text with a link [a javascript:alert('Stealing your cookie!')]my link text[/a]
        Show
        Eric Westfall added a comment - - edited Switching KTI Review Status to "Review Completed". This was reviewed in the KTI on November 7, 2012. Initial discussion was to determine whether it was really necessary to change the model to use User Session storage for the question. After some investigation and discussion with the group, the proposed plan is as follows: Leave the KNS question framework as is (where questionText is passed as a request parameter) Add support in WebUtils.filterHtmlAndReplaceRiceMarkup for a syntax like [a href] where href would be an unquoted link If the href contains the word "javascript", do not convert/replace it. This will prevent XSS attacks if someone where to fashion question text like the following: http://.../kr/questionPrompt.do?methodToCall=start&questionType=confirmationQuestion&questionText=This is my text with a link [a javascript:alert('Stealing your cookie!')]my link text[/a]
        Hide
        Greg Patterson (Inactive) added a comment -

        Changes committed, changeset: 35688 (https://fisheye.kuali.org/changelog/rice?cs=35688)

        As noted in the commit message, a URL can now be passed and processed properly if done like this:
        "This is an example question which would link to [a http://www.google.com]Google.com[/a]".

        Show
        Greg Patterson (Inactive) added a comment - Changes committed, changeset: 35688 ( https://fisheye.kuali.org/changelog/rice?cs=35688 ) As noted in the commit message, a URL can now be passed and processed properly if done like this: "This is an example question which would link to [a http://www.google.com] Google.com[/a]".
        Hide
        Peter Giles (Inactive) added a comment -

        This work was committed some time ago, I'm going to go ahead and resolve it. Greg, if there is anything still pending on this Jira please reopen and let me know what it is. Thanks!

        Show
        Peter Giles (Inactive) added a comment - This work was committed some time ago, I'm going to go ahead and resolve it. Greg, if there is anything still pending on this Jira please reopen and let me know what it is. Thanks!

          People

          • Assignee:
            Greg Patterson (Inactive)
            Reporter:
            Eric Westfall
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Structure Helper Panel