Struts 1
  1. Struts 1
  2. STR-1609

Extend TokenProcessor to handle a list of transaction tokens

    Details

    • Type: Improvement Improvement
    • Status: Reopened
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 1.1.0
    • Fix Version/s: 1.4.0
    • Component/s: Core
    • Labels:
      None
    • Environment:
      Operating System: All
      Platform: All

      Description

      The TokenProcessor currently supports a single token per user session. This can
      be used to guarantee that a user will only submit a form once. However, since
      only one token is supported, a user can not have multiple browser windows open
      and submit forms from all of them. Only the most recent form would have a valid
      token.

      This enhancement suggests supporting a list (say 20) of tokens so the user could
      have a fair number of browser windows open and all would have a valid token.

      A suggested patch is available.

        Issue Links

          Activity

          Art Snowdon created issue -
          Hide
          Art Snowdon added a comment -

          Created an attachment (id=7315)
          Multiple Tokens patch to TokenProcessor.java

          Show
          Art Snowdon added a comment - Created an attachment (id=7315) Multiple Tokens patch to TokenProcessor.java
          Hide
          Vic Cekvenich added a comment -

          I think this would be confusing.
          I sugest not fixing.
          .V

          Show
          Vic Cekvenich added a comment - I think this would be confusing. I sugest not fixing. .V
          Hide
          Rob Leland added a comment -

          This is a useful functionality
          Duane K. Fields, which is where the token class came from originally,
          said that at on etime they had a complex system with different types of tokens,
          and lists of tokens. Now they just have one type of token that needs to
          match one of the outstanding tokens. Also there needs to be a way to override
          the method used for hashing the token.

          Show
          Rob Leland added a comment - This is a useful functionality Duane K. Fields, which is where the token class came from originally, said that at on etime they had a complex system with different types of tokens, and lists of tokens. Now they just have one type of token that needs to match one of the outstanding tokens. Also there needs to be a way to override the method used for hashing the token.
          Hide
          David Graham added a comment -

          Can't you override TokenProcessor.generateToken()?

          Show
          David Graham added a comment - Can't you override TokenProcessor.generateToken()?
          Hide
          Ted Husted added a comment -

          There's a factory method in Action, getInstance, that anyone can override to
          instantiate a different TokenProcessor. I could see offering an alternate
          TokenProcessor, and maybe even making it easier to configure which class to use,
          but I'd be concerned with explaining how to use it to people. We'd need some
          type of example to demonstrate how to use it, or at least a HOWTO article. But
          given some more documentation, this could be a good candidate for an alternate
          class.

          Show
          Ted Husted added a comment - There's a factory method in Action, getInstance, that anyone can override to instantiate a different TokenProcessor. I could see offering an alternate TokenProcessor, and maybe even making it easier to configure which class to use, but I'd be concerned with explaining how to use it to people. We'd need some type of example to demonstrate how to use it, or at least a HOWTO article. But given some more documentation, this could be a good candidate for an alternate class.
          Hide
          Erik van Oosten added a comment -

          A HOWTO is not necessary IMHO as the interface does not change and the behaviour
          is backward compatible.

          The example implementation stores the next token in the session context. This is
          potentially unsafe when the same user request multiple pages simulteneously.

          I propose to put the next token in the request context only and change the
          html:form tag so that it gets the next token from the request context instead of
          from the session context.

          Show
          Erik van Oosten added a comment - A HOWTO is not necessary IMHO as the interface does not change and the behaviour is backward compatible. The example implementation stores the next token in the session context. This is potentially unsafe when the same user request multiple pages simulteneously. I propose to put the next token in the request context only and change the html:form tag so that it gets the next token from the request context instead of from the session context.
          Hide
          Jean-Baptiste Nizet added a comment -

          I've posted another potential mechanism to solve this issue in
          http://issues.apache.org/bugzilla/show_bug.cgi?id=30295
          Sorry for the duplication.

          BTW, storing the next token in the session is not correct, as Erik mentions it,
          but storing it in the request won't work either (as is). You need to make sure
          to get the next token from the request parameters or the request attributes in
          the form tag. Indeed, if the form tag gets it from the request attribute only,
          the following scenario won't work:

          • request
          • generate token and put it in the request
          • display the form: get the token from the request attribute and put it in a
            hidden field
          • post the form
          • validate fails: forward to input
          • display the form: the token is not in the request attribute anymore, so no
            token hidden field is generated.
          Show
          Jean-Baptiste Nizet added a comment - I've posted another potential mechanism to solve this issue in http://issues.apache.org/bugzilla/show_bug.cgi?id=30295 Sorry for the duplication. BTW, storing the next token in the session is not correct, as Erik mentions it, but storing it in the request won't work either (as is). You need to make sure to get the next token from the request parameters or the request attributes in the form tag. Indeed, if the form tag gets it from the request attribute only, the following scenario won't work: request generate token and put it in the request display the form: get the token from the request attribute and put it in a hidden field post the form validate fails: forward to input display the form: the token is not in the request attribute anymore, so no token hidden field is generated.
          Hide
          Erik van Oosten added a comment -

          Jean-Baptiste has a good point. We have seen this also in the solution we
          currently use (proprietary unfortunately). We solved this be letting the
          tokenprocessor allow any page that does not have any token. Upon a forward the
          token is removed from the request. This will make Jean-Baptiste's scenario work.

          Show
          Erik van Oosten added a comment - Jean-Baptiste has a good point. We have seen this also in the solution we currently use (proprietary unfortunately). We solved this be letting the tokenprocessor allow any page that does not have any token. Upon a forward the token is removed from the request. This will make Jean-Baptiste's scenario work.
          Don Brown made changes -
          Field Original Value New Value
          issue.field.bugzillaimportkey 21624 26900
          Paul Benedict made changes -
          Assignee Struts Developers [ dev@struts.apache.org ]
          Fix Version/s Future [ 21701 ]
          Paul Benedict made changes -
          Link This issue is duplicated by STR-2144 [ STR-2144 ]
          Paul Benedict made changes -
          Fix Version/s 1.4.0 [ 21795 ]
          Fix Version/s Future [ 21701 ]
          Jeff Turner made changes -
          Project Import Mon Feb 01 01:03:21 UTC 2010 [ 1264986201992 ]
          Mark Thomas made changes -
          Workflow jira [ 12490891 ] Default workflow, editable Closed status [ 12546182 ]
          Mark Thomas made changes -
          Workflow Default workflow, editable Closed status [ 12546182 ] jira [ 12549467 ]
          Mark Thomas made changes -
          Workflow jira [ 12549467 ] Default workflow, editable Closed status [ 12559549 ]
          Mark Thomas made changes -
          Workflow Default workflow, editable Closed status [ 12559549 ] jira [ 12588024 ]

            People

            • Assignee:
              Unassigned
              Reporter:
              Art Snowdon
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:

                Development