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

ACE whitelist checking doesn't match hosts with multiple parts

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Libraries – ACE
    • Labels:
      None
    • Sprint:
      1.0.3

      Description

      The whitelist regular expression for to check valid hosts does not match host names with more than one '.'

      addon.config.whitelistRegexp()0 = /^[^.]\.[^.]$/
      addon.config.whitelistRegexp()0.test("test.com") = true
      addon.config.whitelistRegexp()0.test("test.test.com") = false

      I would have expected this whitelist pattern to match all hosts including those with multiple '.'s.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            mjensen Matthew Jensen added a comment -

            Work around: explicitly set your ace version to 0.9.1:

            {
              "name": "my-addon",
              "version": "0.0.1",
              "private": true,
              "scripts": {
                "start": "node app.js"
              },
              "dependencies": {
                "express": "~3.3.4",
                "express-hbs": "*",
                "static-expiry": ">=0.0.5",
                "atlassian-connect-express": "0.9.1"
              }
            }
            
            Show
            mjensen Matthew Jensen added a comment - Work around: explicitly set your ace version to 0.9.1: { "name" : "my-addon" , "version" : "0.0.1" , " private " : true , "scripts" : { "start" : "node app.js" }, "dependencies" : { "express" : "~3.3.4" , "express-hbs" : "*" , " static -expiry" : ">=0.0.5" , "atlassian-connect-express" : "0.9.1" } }
            Hide
            pstreule Patrick Streule added a comment -

            The patterns '.','*' are only used for local development (where it's usually machinename or machinename.local. The configuration for production matches the standard multipart OD hostnames.

            Are you using a multi-part hostname for local development?

            Instead of reverting to 0.9.1, you can also just change the configuration in config.json

            Show
            pstreule Patrick Streule added a comment - The patterns ' . ','*' are only used for local development (where it's usually machinename or machinename.local . The configuration for production matches the standard multipart OD hostnames. Are you using a multi-part hostname for local development? Instead of reverting to 0.9.1, you can also just change the configuration in config.json
            Hide
            mjensen Matthew Jensen added a comment -

            This happens in local development yeah, its not Minor because AMPS, by default, picks up my host name with multiple parts.

            Instead of reverting to 0.9.1, you can also just change the configuration in config.json

            To what? Where?

            Show
            mjensen Matthew Jensen added a comment - This happens in local development yeah, its not Minor because AMPS, by default, picks up my host name with multiple parts. Instead of reverting to 0.9.1, you can also just change the configuration in config.json To what? Where?
            Hide
            pstreule Patrick Streule added a comment -

            config.json is in the root of your ACE project, and for now, you could add a whitelist entry to the development section, e.g.:

            "development": {
                ...
                "whitelist": [
                    "*.dyn.syd.atlassian.com"
                ]
            }
            
            Show
            pstreule Patrick Streule added a comment - config.json is in the root of your ACE project, and for now, you could add a whitelist entry to the development section, e.g.: "development" : { ... "whitelist" : [ "*.dyn.syd.atlassian.com" ] }
            Hide
            pstreule Patrick Streule added a comment -

            I think we should remove the whitelist check entirely for local development.

            Show
            pstreule Patrick Streule added a comment - I think we should remove the whitelist check entirely for local development.
            Hide
            rodogu Roberto Dominguez added a comment -

            I noticed that too... It's a pain in the neck the way it is done now... we use different modes too so we have to set

             "whitelist": ["*","*.*","*.*.*"]
            

            on every mode in our conflig.json... (giggle)

            Furthermore, seting AC_HOST_WHITELIST only works as a single value ...

            Patrick Streule, if you guys are going to remove the whitelist check, please disable for non production mode, this time we are covered too with our different models.

            Also, it'd be nice if you make AC_HOST_WHITELIST to handle comma-separated patterns, so it supports multiple values

            Show
            rodogu Roberto Dominguez added a comment - I noticed that too... It's a pain in the neck the way it is done now... we use different modes too so we have to set "whitelist" : [ "*" , "*.*" , "*.*.*" ] on every mode in our conflig.json... (giggle) Furthermore, seting AC_HOST_WHITELIST only works as a single value ... Patrick Streule , if you guys are going to remove the whitelist check, please disable for non production mode, this time we are covered too with our different models. Also, it'd be nice if you make AC_HOST_WHITELIST to handle comma-separated patterns, so it supports multiple values
            Hide
            pstreule Patrick Streule added a comment -

            Roberto Dominguez I think this would work for all cases:
            Make the whitelist check only if a whitelist is specified (and insist that there is a whitelist in production mode).

            We'll take the AC_HOST_WHITELIST change into account, too.

            Show
            pstreule Patrick Streule added a comment - Roberto Dominguez I think this would work for all cases: Make the whitelist check only if a whitelist is specified (and insist that there is a whitelist in production mode). We'll take the AC_HOST_WHITELIST change into account, too.
            Hide
            rodogu Roberto Dominguez added a comment -

            You're da man!

            Show
            rodogu Roberto Dominguez added a comment - You're da man!
            Hide
            pstreule Patrick Streule added a comment -

            This is the new behavior:

            • If no whitelist is specified (this is the default in `development` mode), then all hostnames are accepted
            • A whitelist can be specified as a comma-separated string using the `AC_HOST_WHITELIST` env var.
            Show
            pstreule Patrick Streule added a comment - This is the new behavior: If no whitelist is specified (this is the default in `development` mode), then all hostnames are accepted A whitelist can be specified as a comma-separated string using the `AC_HOST_WHITELIST` env var.

              People

              • Assignee:
                pstreule Patrick Streule
                Reporter:
                mjensen Matthew Jensen
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Who's Looking?

                    Agile