Pull requests

#54 Declined
Repository
atlassian
Branch
master

ACDEV-1174: prevent double registration when verifying installation requests.

Author
  1. Sam Day
Reviewers
Description

This is a fix for ACDEV-1174. It prevents requests to /installed if the addon already has data for a tenant.

One caveat, this means uninstalling and re-installing an addon will fail, because ACE currently doesn't clean up tenant details on uninstall (it doesn't even listen for the uninstalled lifecycle even currently). I raised AC-929 about this a while ago. I'm addressing that manually for example in my confstats addon (https://bitbucket.org/sday_atlassian/confstats-connect/src/bd75cffc151bfe123ec7ac78ecc2761fdf097d11/routes/lifecycle.js?at=master#cl-8) but I can try pushing a patch through for ACE.

Comments (6)

    1. Sam Day author

      Crazy!

      I guess we could check if we already have client info on file, and then reject only if we already have info AND the secrets don't match. I'm just trying to think if that results in possible information disclosure. I guess the secret is big enough that it shouldn't be a problem? Maybe? :P

        1. Patrick Streule

          Comalatech is doing this in their implementation, that's probably what I mentioned once.

          @sday_atlassian I agree, we should reject only if the secrets don't match and not generally. Unfortunately there are also some other cases that we need to take into account, namely the transition from OAuth to JWT, which shouldn't lead to rejections.