Uploaded image for project: 'Atlassian Connect'
  1. Atlassian Connect
  2. AC-772

if there is a ' in the request query addon.authenticate() fails with 401

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Security Level: Public (Visible to anyone, including anonymous viewers.)
    • Labels:
      None
    • Sprint:
      1.0-m12, 1.0-m13
    • Epic Link:

      Description

      We have found that if a high comma

      '

      is part of any parameter of the query to ACE app, oauth fails with 401 - invalid signature.

      To reproduce you need to clear cookies for the ACE app host before viewing the page.

      For example

      1. clear all cookies for herokuapp.com
      2. go to https://comalatechlabs.atlassian.net/wiki/display/ds/Know%2C+Want+and+Learned - the board does not load,
      3. edit the page > edit the board > double click on first container > more .. > change description field, remove the ' (http://cl.ly/image/3t3Z103f110h)
      4. save container, board, page

      >> now the board loads.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            devdoctor Ulrich Kuhnhardt added a comment - - edited

            query that works : no high comma in the query :

            %22d%22

            query that does not work : high comma, encoded :

            %22%27d%22
            Show
            devdoctor Ulrich Kuhnhardt added a comment - - edited query that works : no high comma in the query : %22d%22 query that does not work : high comma, encoded : %22%27d%22
            Hide
            rodogu Roberto Dominguez added a comment -

            Note that to reproduce properly you MUST REMOVE your sesion cookie, otherwise the authentication would be bypassed.

            For debugging purpuses you can comment out that validation in ACEs /lib/middleware/oauth.js:

            //    if (req.session && req.session.acOauthVerified) {
            //      return next();
            //    }
            
            Show
            rodogu Roberto Dominguez added a comment - Note that to reproduce properly you MUST REMOVE your sesion cookie, otherwise the authentication would be bypassed. For debugging purpuses you can comment out that validation in ACEs /lib/middleware/oauth.js : // if (req.session && req.session.acOauthVerified) { // return next(); // }
            Hide
            rodogu Roberto Dominguez added a comment -

            Looks like Rich Manalang was playing with quotes a while back

            Show
            rodogu Roberto Dominguez added a comment - Looks like Rich Manalang was playing with quotes a while back
            Show
            rodogu Roberto Dominguez added a comment - Pull request https://bitbucket.org/atlassian/atlassian-connect-express/pull-request/8/reverting-encodedata-to-handle-single/diff
            Hide
            rmanalang Rich Manalang added a comment -

            Fixed in ACE 0.8.3. Thanks Roberto Dominguez!

            Show
            rmanalang Rich Manalang added a comment - Fixed in ACE 0.8.3. Thanks Roberto Dominguez !
            Show
            devdoctor Ulrich Kuhnhardt added a comment - I still get 401 with a-c-e 0.8.3 https://bitbucket.org/atlassian/atlassian-connect-express/src/f80273d89741cee1542696cf14e2e8e2bd6174c9/lib/internal/oauth.js?at=release/0.8.3#cl-75
            Hide
            rodogu Roberto Dominguez added a comment -

            Rich Manalang the change didin't make it to 0.8.3

            Show
            rodogu Roberto Dominguez added a comment - Rich Manalang the change didin't make it to 0.8.3
            Hide
            tpettersen Tim Pettersen added a comment -

            Sorry guys, I've just pushed out atlassian-connect-express 0.8.4 with Roberto Dominguez's fix verified and merged. Thanks for the PR!

            Show
            tpettersen Tim Pettersen added a comment - Sorry guys, I've just pushed out atlassian-connect-express 0.8.4 with Roberto Dominguez 's fix verified and merged. Thanks for the PR!

              People

              • Assignee:
                tpettersen Tim Pettersen
                Reporter:
                devdoctor Ulrich Kuhnhardt
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Who's Looking?

                    Agile