Cannot activate integrations

[ProxiBlue]

Preconditions and environment

• Mage-OS Magento version 1.2.0

Create a new system->integration, and then after created, attempt to activate. Activation fails with a js error, and one cannot continue. It is thus impossible to add new integrations.

roles-tree.js:101 Uncaught TypeError: Cannot read properties of null (reading 'removeChild')
    at roles-tree.js:101:36
    at Array.forEach (<anonymous>)
    at $.<computed>.<computed>._destroy (roles-tree.js:100:26)
    at $.<computed>.<computed>._destroy (jquery-ui.js:143:25)
    at $.<computed>.<computed>.destroy (jquery-ui.js:364:8)
    at $.<computed>.<computed>.remove (jquery-ui.js:322:12)
    at HTMLDivElement.handlerProxy (jquery-ui.js:626:7)
    at HTMLDivElement.dispatch (jquery.js:5145:49)
    at elemData.handle (jquery.js:4949:47)
    at Object.trigger (jquery.js:8629:28)

I tried on two separate sites, both mage-os 1.2.0 WIP upgrades - same issue
I disabled the custom mage-os admin theme - same issue. (so is not theme)
I tried a clean vanilla mage-os install - same issue (so no 3rd party modules installed)
I tried a clean 2.4.8 install - no issues - worked out the box

integration_bug.mp4

Steps to reproduce

1. Navigate to admin, System->integrations
2. Create a new test integration
3. Try activate

Expected result

Integration should be activated.

Actual result

Js error (refer main text)

Additional information

I can bypass issue by commenting out the js code line, as shown in video attached, so there is a workaround.

Release note

No response

Triage and priority

• Severity: S0 - Affects critical data or functionality and leaves users without workaround.
• Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
• Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
• Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
• Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.

[rhoerr]

Reproduced on release/1.x.

[rhoerr]

Interestingly, it also fails if you click 'Save and Activate' while creating an integration. Rather than activating, it saves it and then initiates a redirect to the dashboard.

POST /admin/integration/save/... returns:

{"integrationId":"6","isTokenExchange":"0"}

which triggers _showPopup, which triggers an AJAX call for the form GET /admin/integration/permissionsDialog/id/6/reauthorize/0/..., which returns

{
    "_redirect": "http:\/\/mageos.test\/admin_9zbrdr1\/admin\/dashboard\/index\/key\/...\/"
}

which, after some digging, is because of this on the last request

You entered an invalid Secret Key. Please refresh the page.

But this appears to be unrelated to the JS error on the grid activate popup, which is from the close button not having a parent element at the time of execution. No recent changes in the controller or integration.js or widget.js or templates. Not obvious what the source of the discrepancy is.

[ProxiBlue]

Not obvious what the source of the discrepancy is.

yeah, I had a quick look as well, ,limited time.
Nothing seems obviously wrong, but it is 100% a mageos change/issue.

Looks like git bisect might be the tool to use to find a commit/merge that causes issue.
Just need to go back and find a commit it works, and bisect from there

[rhoerr]

I suspect it might actually be a change from post-2.4.8 that made it into our 2.4-develop branch and then release/1.x for 1.1.0 and 1.2.0. For 2.4.7 and 2.4.8, we base our release on a particular tag of the upstream develop branch -- but we have to wait until the release is made and then until Adobe adds the security-aspect fixes into the open source repository. If something was added between the commit that 2.4.8 was based on, and when the security fixes were added to 2.4-develop, those would likely have been included in our releases.

Based on that, some lines of possible investigation:

• Does the same issue occur in 2.4.9-beta1? (indirect, but would confirm the above if so)
• What differences exist between 2.4.8-p1 and Mage-OS 1.2.0? Any that do not appear related to Mage-OS or fixes thereof?

Your git bisect could also get right to the point.

[ProxiBlue]

Your git bisect could also get right to the point.

Busy gearing up for a site release / inclusive of migration to hypernode, (which s why i also found issue during testing of new build) so not likely going to get to it in next 2 weeks.

At least there is a easy workaround for anyone in a pinch

[ProxiBlue]

Note. To reproduce, one must add in an APi resource under 'custom' or, just change it to 'all'
If no API resources are selected, the issue is not appearing.

Not that one would make an integration without any API access, but, it does allow this, and then se no error as described.

[ProxiBlue]

Ok, so using git bisect to track down bad merge commit, I came across one commit that is in fact also failing build commands

01:59 $ git bisect bad
Bisecting: 3 revisions left to test after this (roughly 2 steps)
[a3f60e1bb07f2f4dc87fa73a7217d470bef33b87] Merge remote-tracking branch 'upstream/2.4-develop' into ui/admin/roles-tree

02:01 $ ./bin/magento setup:upgrade

Fatal error: Declaration of Magento\Framework\Console\Cli::getDefaultCommands() must be compatible with Symfony\Component\Console\Application::getDefaultCommands(): array in /var/www/html/lib/internal/Magento/Framework/Console/Cli.php on line 133

Call Stack:
    0.0001     485816   1. {main}() /var/www/html/bin/magento:0
    0.0189     906328   2. Composer\Autoload\ClassLoader->loadClass($class = 'Magento\\Framework\\Console\\Cli') /var/www/html/bin/magento:22
    0.0190     906440   3. Composer\Autoload\includeFile($file = '/var/www/html/vendor/composer/../../lib/internal/Magento/Framework/Console/Cli.php') /var/www/html/vendor/composer/ClassLoader.php:428
    0.0191     906816   4. include('/var/www/html/lib/internal/Magento/Framework/Console/Cli.php') /var/www/html/vendor/composer/ClassLoader.php:571

which is of interest, as this is very likely related: into ui/admin/roles-tree

which is what / where it is from (roles tree for api's?)

commit a3f60e1bb07f2f4dc87fa73a7217d470bef33b87 (HEAD)
Merge: 3aa6cd9af22 f00af583b2b
Author: Dan Wallis <mrdanwallis@gmail.com>
Date:   Fri Nov 25 11:36:50 2022 +0000

    Merge remote-tracking branch 'upstream/2.4-develop' into ui/admin/roles-tree

[ProxiBlue]

at this point git bisect fell over as I can;t build/compile test code.

marking as bad, next one as well.

I reckon this is the cause - whatever was done around these commits all by same user : Dan Wallis mrdanwallis@gmail.com

commit 7e01a64e758b34f14f6123b09ee5a63231ca8acd (HEAD)
Author: Dan Wallis <mrdanwallis@gmail.com>
Date:   Tue Nov 22 23:37:04 2022 +0000

    Appease current coding standard
    
    See also https://github.com/magento/magento-coding-standard/pull/347

commit eae7a1ff4141cf17a43dd49e86b54e442788c1dc
Author: Dan Wallis <mrdanwallis@gmail.com>
Date:   Tue Nov 22 22:07:59 2022 +0000

    Improve user experience with roles tree

[ProxiBlue]

but it did identify the likely commit, if I calle the last two bad.....

02:11 $ git bisect bad
eae7a1ff4141cf17a43dd49e86b54e442788c1dc is the first bad commit
commit eae7a1ff4141cf17a43dd49e86b54e442788c1dc
Author: Dan Wallis <mrdanwallis@gmail.com>
Date:   Tue Nov 22 22:07:59 2022 +0000

    Improve user experience with roles tree

 .../User/view/adminhtml/web/js/roles-tree.js       | 59 ++++++++++++++++++++--
 1 file changed, 54 insertions(+), 5 deletions(-)

and the description and code edit matches section where issue is...so looks like it may be.

[ProxiBlue]

[ProxiBlue]

So the code that is failing, was directly inserted by this change/merge.

Since I don;t have scope as to what these changes are all about, really not easy to fix, other than adding a check that parentNode is actually defined.

as shown by workaround, this is sufficient.

but, what is missing/why was this added? why is it broken in mage-os ?

[ProxiBlue]

seems it was already addressed upstream:

https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/User/view/adminhtml/web/js/roles-tree.js#L102

[ProxiBlue]

magento/magento2@9995ae5

[fballiano]

@rhoerr would this be automatically fixed by an upstream sync instead of merging a change ourself that will end up in a future conflict?