Skip to content

Remove the autoload_psr4 before running finalisation.php#38491

Closed
nikosdion wants to merge 6 commits intojoomla:4.2-devfrom
nikosdion:fix/38486-update-remove-autoload-psr4
Closed

Remove the autoload_psr4 before running finalisation.php#38491
nikosdion wants to merge 6 commits intojoomla:4.2-devfrom
nikosdion:fix/38486-update-remove-autoload-psr4

Conversation

@nikosdion
Copy link
Copy Markdown
Contributor

@nikosdion nikosdion commented Aug 17, 2022

Pull Request for Issue #38486 .

Summary of Changes

Add code in extract.php to remove the administrator/cache/autoload_psr4.php file before loading the finalisation.php as the latter might try to use core code which depends on the former.

Disable loading all non-core and most core plugins during the task=update.finalise, task=update.cleanup and related helper tasks of com_joomlaupdate to prevent third party plugins which are not compatible with the new version being installed from getting in the way of the update (they would prevent schema update and any post-update and cleanup tasks we need to perform). They are NOT disabled in the database, just not loaded during those tasks.

Testing Instructions

You will need the System - Sabotage plugin:
plg_system_sabotage.zip

From Joomla 4.0.0

  • Install a Joomla 4.0.0 site
  • Replace administrator/components/com_joomlaupdate, components/com_joomlaupdate, media/com_joomlaupdate, and administrator/language/en-GB/com_joomlaupdate.* with the files from this PR.
  • Log into the backend
  • Install the plg_system_sabotage plugin
  • Go to Extensions, Manage, Plugins. Enable the System - Sabotage plugin and click on its title.
  • Set the sabotage type to Both and the minimum Joomla version to 4.1.9999
  • Do an md5sum of the administrator/cache/autoload_psr4.php file
  • Upgrade the site to Joomla 4.2 using the package built from this PR
  • Try to access your site's frontend
  • CHECK 1: Do an md5sum of the administrator/cache/autoload_psr4.php file
  • CHECK 2: Make sure the #__user_mfa table exists

From Joomla 4.1.5

  • Install a Joomla 4.1.5 site
  • Log into the backend
  • Install the plg_system_sabotage plugin
  • Go to Extensions, Manage, Plugins. Enable the System - Sabotage plugin and click on its title.
  • Set the sabotage type to Both and the minimum Joomla version to 4.1.9999
  • Do an md5sum of the administrator/cache/autoload_psr4.php file
  • Upgrade the site to Joomla 4.2 using the package built from this PR
  • Try to access your site's frontend
  • CHECK 1: Do an md5sum of the administrator/cache/autoload_psr4.php file
  • CHECK 2: Make sure the #__user_mfa table exists

Actual result BEFORE applying this Pull Request

Check 1: The MD5 sums are identical (the autoload_psr4.php file was not removed on update).

Check 2: The table is missing

You WILL receive errors once the update is done. That is exactly what the System - Sabotage plugin as configured in the test instructions is meant to do: throw an error with a stanza from Beastie Boys' “Sabotage” on the target Joomla version and any later version. This error got in the way of running the update finalisation which is why the schema update and cleanup did not run. It's a sabotage!

Expected result AFTER applying this Pull Request

Check 1: The MD5 sums are different (the autoload_psr4.php file was removed on update and replaced when accessing your site again).

Check 2: The table exists.

You WILL receive errors once the update is done. That is exactly what the System - Sabotage plugin as configured in the test instructions is meant to do: throw an error with a stanza from Beastie Boys' “Sabotage” on the target Joomla version and any later version. HOWEVER, this time around the sabotage plugin did not run during the update finalisation and cleanup. Therefore both the schema update and cleanup ran just fine. It's a mirage!

Documentation Changes Required

None.

@joomdonation
Copy link
Copy Markdown
Contributor

@nikosdion

I haven't tested upgrade from 4.1.5 but I tried upgrade from 4.0.4, 4.1.0 to 4.2.0 success without this PR.

I tried to upgrade from 4.0.0 to the package generated by this PR but still get the original error. So for some reasons, it does not solve the upgrade issue yet. Not sure if I missed something with the testing.

@Fedik

This comment was marked as outdated.

@Fedik
Copy link
Copy Markdown
Member

Fedik commented Aug 17, 2022

before loading the finalisation.php as the latter might try to use core code which depends on the former.

sorry, misread

@nikosdion
Copy link
Copy Markdown
Contributor Author

@joomdonation You are right about upgrading from Joomla 4.0.0 to 4.0.3 inclusive. These versions use the old Joomla Update which used restore.php (the very old version of Akeeba Restore) which doesn't clear the autoload_psr4.php file.

What I failed to mention in this PR but did mention in a comment on the original issue is that Joomla needs to publish a new version of Joomla Update. The idea is that it can be upgraded on Joomla 4.0.0 to 4.0.3 inclusive. The test in this case would be to install the Joomla 4 site, copy the entirety of the administrator/components/com_joomlaupdate to the site (replacing the existing folder) and then proceed with the rest of the test instructions.

Can you please confirm that this addresses the issue? Thank you in advance!

@joomdonation
Copy link
Copy Markdown
Contributor

@nikosdion Could you please explain what's the problem which this PR is solving so that I can understand it better? And what we need to do to see this error?

For re-generating autoload_psr4.php, we have code in administrator/components/com_joomlaupdate/finalisation.php already and it worked good for my tests as mentioned above. As I understand, we only have issue with upgrading from 4.0.0-4.0.3 to 4.2 at the moment.

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Aug 17, 2022

@nikosdion Maybe one more question about 4.0.0 do we actually have to do a joomla update release? cant we just force everyone older than 4.0.4 to go via 4.0.5 first and after that go to 4.2.0? Using that the new updater system is in place.

@roland-d
Copy link
Copy Markdown
Contributor

@nikosdion First of all, thank you for spending time on this issue. Much appreciated. If I understand you correctly, the test is to the update from Joomla 4.0.0 with the com_joomlaupdate from Joomla 4.2.0? I feel like I have the versions mixed up.

@nikosdion
Copy link
Copy Markdown
Contributor Author

@joomdonation Several people reported sites upgraded from 4.1.5 to 4.2.0 being broken and fixed when autoload_psr4.php was removed. This has happened to me too on three separate sites. Yes, the finalisation.php file has the code to remove that file BUT it is only called at the end of the finalisation. Since the finalisation may use parts of the core I'd say that it should run before the finalisation.php is loaded.

@zero-24 Back in Joomla 3.4 we had a similar problem where you could not update Joomla unless Joomla Update was updated first. At that point Joomla Update became an updateable extension to overcome this problem. Instead of making a full CMS release for a no longer maintained CMS version we can simply release a new version of Joomla Update, using the feature we already have in place for snafus like the one we are experiencing here.

@roland-d You're welcome :) There are two separate tests.

From Joomla 4.0.0

  • Install a Joomla 4.0.0 site
  • Replace administrator/components/com_joomlaupdate with the same-named folder from this PR here.
  • Log into the backend
  • Do an md5sum of the administrator/cache/autoload_psr4.php file
  • Upgrade the site to Joomla 4.2 using the package built from this PR
  • Try to access your site's frontend
  • Do an md5sum of the administrator/cache/autoload_psr4.php file

From Joomla 4.1.5

  • Install a Joomla 4.1.5 site
  • Log into the backend
  • Do an md5sum of the administrator/cache/autoload_psr4.php file
  • Upgrade the site to Joomla 4.2 using the package built from this PR
  • Try to access your site's frontend
  • Do an md5sum of the administrator/cache/autoload_psr4.php file

Why the difference?

Joomla 4.0.4 and later use the new Joomla Update which uses administrator/components/com_joomlaupdate/extract.php to extract the update ZIP file. This .php file is overwritten on update. Therefore when Joomla Update's JavaScript calls the finalise task it's talking to the new extract.php which was extracted in the previous step.

Joomla 4.0.0 to 4.0.3 inclusive uses an older version of Joomla Update which uses the administrator/components/com_joomlaupdate/restore.php file. This file is not updated during the update; it is only removed during finalisation. Therefore updates from these older Joomla versions would NOT have benefited from the changes in extract.php. The only way to address this chicken and egg problem is to first upgrade Joomla Update on those sites to use the same Joomla Update we use in newer versions (the one using extract.php), THEN run the update itself, at which point everything I said about upgrading from 4.1.5 applies.

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Aug 17, 2022

@zero-24 Back in Joomla 3.4 we had a similar problem where you could not update Joomla unless Joomla Update was updated first. At that point Joomla Update became an updateable extension to overcome this problem. Instead of making a full CMS release for a no longer maintained CMS version we can simply release a new version of Joomla Update, using the feature we already have in place for snafus like the one we are experiencing here.

I would not issue an new core release but use the first that was using the new way and worked in that point of time. As we had only issue reported now on very old versions. I would like to try to avoid issuing an standalone joomla update release which spans over so many joomla releases to avoid other issues we might get. So the plan would be 4.0.0 -> upgrade to a version that has the new updater code which support finalization -> got to 4.2.0 where this patch here comes into play and makes sure the autoload file is removed.

@roland-d
Copy link
Copy Markdown
Contributor

@nikosdion Thanks for the clear instructions. So the 4.1.5 -> 4.2.0 worked fine. The autoload_psr4.php was re-created. Going to run the 4.0.0 test now.

@nikosdion
Copy link
Copy Markdown
Contributor Author

@zero-24 If you upgrade 4.0.0 to 4.0.4 or later 4.0.x release then you can safely upgrade to 4.2.0 with this PR. However! You can't do that... The only proposed update you are given by Joomla itself on a Joomla 4.0.0 site is 4.2.0. Therefore we are guiding people to an one-click breakage of their site.

The only viable solution is releasing a new version of JUST Joomla Update. Not the entire CMS. This will also address the problems people like me had with 4.1.5 to 4.2.0 updates. It's a win-win situation.

@roland-d
Copy link
Copy Markdown
Contributor

@nikosdion I replaced the complete administrator/components/com_joomlaupdate from the package built by this PR and placed it in the 4.0.0 site. When I go to the Joomla Updater I see this error:
Call to undefined method Joomla\Component\Joomlaupdate\Administrator\Model\UpdateModel::getDatabase() which comes from line 98 $db = $this->getDatabase(); and I believe this is the new code for 4.2.0.

Can you check please?

@nikosdion
Copy link
Copy Markdown
Contributor Author

Yeah, that was changed very recently. Let me see if I can convince Joomla Update to run on Joomla 4 with some judicious use of if-blocks.

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Aug 17, 2022

@zero-24 If you upgrade 4.0.0 to 4.0.4 or later 4.0.x release then you can safely upgrade to 4.2.0 with this PR. However! You can't do that... The only proposed update you are given by Joomla itself on a Joomla 4.0.0 site is 4.2.0. Therefore we are guiding people to an one-click breakage of their site.

Yes that can be changed right now we only propose the update from 4.0.0 to 4.1.5 will prepare a PR to update the update server and point them to go via 4.0.4 first so we dont add more steps for that people affected by this.

@nikosdion
Copy link
Copy Markdown
Contributor Author

@zero-24 There were some reports a few months ago that 4.0 to 4.1 was causing the same problems for the same reason. Let's try to get the new com_joomlaupdate running on Joomla 4.0.0–4.0.4 instead.

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Aug 17, 2022

@zero-24 There were some reports a few months ago that 4.0 to 4.1 was causing the same problems for the same reason. Let's try to get the new com_joomlaupdate running on Joomla 4.0.0–4.0.4 instead.

4.0.what to 4.1? Did you found the reports? And wouldnt that kind of issues be solved by this PR? The change to the update distribution is only making sure everyone uses the new updater.

This is what i had in mind: joomla/update.joomla.org#263 (comment)

Required to make a new release for it
@nikosdion
Copy link
Copy Markdown
Contributor Author

I have updated this PR so that Joomla Update can now safely run on Joomla 4.0.0.

I have confirmed that installing the new Joomla Update (replacing administrator/components/com_joomlaupdate, media/com_joomlaupdate and administrator/language/en-GB/com_joomlaupdate*) on a Joomla 4.0.0 site works AND proposes an upgrade to 4.1.5.

Installing the upgrade to 4.1.5 DOES update the autoload_psr4.php file.

On that site, I did the same Joomla Update upgrade but it does not see the Joomla 4.2 version. No idea why. Huh. I did a manual update (Upload & Install) to Joomla 4.2.0. This worked and the upgrade to 4.2.0 DOES update the autoload_psr4.php file.

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Aug 17, 2022

On that site, I did the same Joomla Update upgrade but it does not see the Joomla 4.2 version. No idea why. Huh. I did a manual update (Upload & Install) to Joomla 4.2.0. This worked and the upgrade to 4.2.0 DOES update the autoload_psr4.php file.

Yesterday we have pulled 4.2.0 off from the update server thats the reason your site dont see it via the regular update path. :)

@nikosdion
Copy link
Copy Markdown
Contributor Author

@zero-24 Ah, good to know. So, I tested 4.0.0 to 4.2.0 after installing com_joomlaupdate from this repository (using Upload & Install, of course) and it works correctly. The autoload_psr4.php file is replaced, therefore it fixes this issue.

I just need a couple of tests and you can merge this PR and release a new standalone com_joomlaupdate update to solve all of the weird upgrade problems which ultimately are caused by autoload_psr4.php not being updated.

Post Scriptum: I am very wary of the idea that the autoload_psr4.php file is never updated unless manually deleted. What I'd like to do is have a Global Configuration option with its update frequency in minutes. Three possible defaults are 525600 (one year; suitable for production sites operated by Those Who Know What They Are Doing), 360 (6 hours; not too bad for production, not ideal either) and 1 (development mode). I would be inclined to recommend a default of 360 minutes. I have been burned too much by this file not being updated and there's no documentation I could find about it. If that sounds like something you guys would be willing to test and back I am willing to write the code.

@roland-d
Copy link
Copy Markdown
Contributor

The update from 4.0.0 to 4.2.0 worked here as well however we can have the following update path from the update servers, so users do not have to update com_joomlaupdate:

Update sequence from 3.10.x
3.10.x -> 4.0.4
4.0.4 -> 4.2.1

Update sequence from 4.0.0
4.0.0 -> 4.0.4
4.0.4 -> 4.2.1

Update sequence from 4.1.5
4.1.5 -> 4.2.1

My doubt is now if we should or should not offer the com_joomlaupdate update as well.

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Aug 17, 2022

Post Scriptum: I am very wary of the idea that the autoload_psr4.php file is never updated unless manually deleted. What I'd like to do is have a Global Configuration option with its update frequency in minutes. Three possible defaults are 525600 (one year; suitable for production sites operated by Those Who Know What They Are Doing), 360 (6 hours; not too bad for production, not ideal either) and 1 (development mode). I would be inclined to recommend a default of 360 minutes. I have been burned too much by this file not being updated and there's no documentation I could find about it. If that sounds like something you guys would be willing to test and back I am willing to write the code.

Hmm It was also updated when you installed or updated extensions. Not sure whether its worth doing it when no changes to the classes are done just on a regular basis and would that maybe even cause more issues than it solves? It is also removed as soon as you clear the cache folder. I would say lets branch that out of this PR into a dedicated issue and lets discuss that there before you have to start writing the code.

@nikosdion
Copy link
Copy Markdown
Contributor Author

@roland-d Yes, we must issue a com_joomlaupdate update as well. Otherwise the 4.0.4 to 4.2.1 MIGHT not work. I checked the site where all hell broke loose yesterday. Apparently the finalise.php script failed to execute completely. I had schema problems AND a stale autoload_psr4.php file. I am still trying to bring this site back to life (I refuse to restore it from backup and retry, I want to understand what the heck is going on).

@nikosdion
Copy link
Copy Markdown
Contributor Author

@zero-24 Agreed. I will start a discussion when I am done working on the issues I've bumped on since last night. I have horror stories to share from extension updates as well 👻

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Aug 17, 2022

@roland-d Yes, we must issue a com_joomlaupdate update as well. Otherwise the 4.0.4 to 4.2.1 MIGHT not work

That would mean we have to check any changes to com_joomlaupdate made between 4.0.0 and 4.2.0 and all supporting code and verify that they are working on 4.0.0 too. For example we would need a similiar version compare about "twofactor" vs "mfa" notjing critical but it should be checked whether there are other changes.

Until now we only did standalone releases that for one specific release when nothing else works, First I would like to understand why on some sites it might result into an error or not. We have not got any reports about issues until now.

@nikosdion
Copy link
Copy Markdown
Contributor Author

Well, I AM reporting an issue! As I said two comments above:

I checked the site where all hell broke loose yesterday. Apparently the finalise.php script failed to execute completely. I had schema problems AND a stale autoload_psr4.php file.

The site is thankfully something I am using for development but on a live, commercial VPS hosted with Rochen. That was a Joomla 4.0.3 site.

You know what is crazy? That site is practically empty! It has Joomla and SocialLogin. I am using it to develop SocialLogin plugins since most services require a live site to give me a developer account.

My business site is far more complex and it updated just fine, though it was FROM 4.1.5 TO 4.2.0 — naturally, that site is kept fully up-to-date.

I am also discovering other weird issues with WebAuthn on the special snowflake site... but that's for another issue I am composing right now.

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Aug 17, 2022

4.0.3 to 4.2.0 is confirmed to fail as that is running the outdated code of joomla update. would be interesting to know whether we have still issues using a upgrade package with this patch applied here from anything between 4.0.4 and 4.1.

@nikosdion
Copy link
Copy Markdown
Contributor Author

@zero-24 Ah, damn, I found out what happened on that special snowflake site — thank you for being my rubber ducky. Remember how I told you that I am using it to develop SocialLogin? That came with an old version of the JWT library. Joomla has a newer version. In his infinite wisdom, the library developer changed the data type of the return claims when decoding the JWT from an array of arrays to an array of objects. Since my plugin was loading the old version (returning an array of arrays) it caused plg_system_webauthn to fail because it expects an array of objects. This seems to have caused the finalisation code to fail. I did not catch the problem because the early failure of the finalisation code also means that old files were not removed, the schema was not updated and yes, the autoload_psr4.php was not removed (since upgrading from 4.0.3 won't do that, right...).

So... this is actually a bit troubling. If a system plugin fails on 4.2 we have quite the problem. While extract.php will do its job of extracting the archive and running finalise.php, running the task=update.finalise will fail.

The only solution I can possibly think is having yet another directly web accessible file with a custom WebApplication which does NOT load any plugins to run the code we currently have in task=update.finalise. Otherwise all it takes to have a broken update (and a broken site) is a system plugin which fails to work correctly.

@nikosdion
Copy link
Copy Markdown
Contributor Author

@richard67 Shoot, I fat-fingered the commit. Let me fix that.

Also add administrator/manifests/packages/pkg_search.xml to the .gitignore list of files to NOT ignore. This file needs to be in the repo. See #38491 (comment)
@nikosdion
Copy link
Copy Markdown
Contributor Author

@richard67 I fixed that and I updated .gitignore so next time I accidentally delete pkg_search.xml it will scream at me. I can't guarantee I will hear it screaming but at least it will be telling me I am removing a tracked file :p

@richard67
Copy link
Copy Markdown
Member

@richard67 I fixed that and I updated .gitignore so next time I accidentally delete pkg_search.xml it will scream at me. I can't guarantee I will hear it screaming but at least it will be telling me I am removing a tracked file :p

@nikosdion You might do that with your .gitignore locally, but the .gitignore file is not ignored itself, so it is part of the source code, and your PR now adds the "pkg_search.xml" file to the .gitignore in the sources. I don't think that's right.

@nikosdion
Copy link
Copy Markdown
Contributor Author

nikosdion commented Aug 17, 2022 via email

@richard67
Copy link
Copy Markdown
Member

Yeah, but pkg_search.xml was ALREADY in the repo despite being ignored and you asked me why I removed it. So one of the two is wrong. A. The file is in the repository. B. The file is not in the .gitignore Which one is wrong so I know what to fix?

The file is normally not touched, but when you are testing updating on a git clone, it is removed at the end of the update if search is not installed. So I think it is right as it is, the file in the sources and not in the .gitignore.

But maybe I am wrong.

@wilsonge What do you think?

@richard67
Copy link
Copy Markdown
Member

Meanwhile I’ve noticed the exclamation mark and the beginning of that line. So the .gitignore change should be ok.

@roland-d
Copy link
Copy Markdown
Contributor

My 2 cents, it is fine to track this file, so we know if it accidentally gets modified.

@roland-d
Copy link
Copy Markdown
Contributor

My brain is mush, I'd appreciate it if someone came with a good testing plan for this.

I would think we need something like this:

  • A plugin that is bad installed on a site to be updated
  • Check that the update fails
  • Apply this patch
  • Check that the update succeeds

I only do not know what the bag plugin criteria should be. Is it just a system plugin that dies or returns false somewhere? Which event should it be triggered on?

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Aug 17, 2022

I only do not know what the bag plugin criteria should be. Is it just a system plugin that dies or returns false somewhere? Which event should it be triggered on?

I would say a system plugin with an error in the constructor could do the trick. As that was an issue with all releases where we have joomlaupdate aviable already I would say we should branch out that freezing plugins patch into a dedicated PR against 4.2.2 so we can get more tests in but get the 4.2.1 release out early too.

@laoneo
Copy link
Copy Markdown
Member

laoneo commented Aug 18, 2022

What about to remove all listeners from the dispatcher during upgrade and then reinstate them again after the process is done?

@wilsonge
Copy link
Copy Markdown
Contributor

Yeah, but pkg_search.xml was ALREADY in the repo despite being ignored and you asked me why I removed it.

So one of the two is wrong.

A. The file is in the repository.

B. The file is not in the .gitignore

Which one is wrong so I know what to fix?

The file is supposed to be in the repo for people upgrading from j3 who need the update server definition to be moved to the external github page. for people starting new sites on j4 it's junk and gets removed as part of installation. In theory it's fine to add it to gitignore as long as nothing changes in the future. just in the off chance anything is intentionally changed - not sure how many of our contributors will know about force git adding

@roland-d
Copy link
Copy Markdown
Contributor

@nikosdion Do you have time to finish up this PR today? The reason I am asking is because I would like to push out the RC today.

As Tobias said, it will be good to move the plugin unloading to a separate PR as this still needs more thinking/testing/coding. Could you move that to a new PR?

With the plugin unloading out of this PR, it should be ready to do the final test and merge.

@wilsonge If something needs to change in the pkg_search.xml I am sure we can find a seasoned contributor to do or help with that :)

* @since __DEPLOY_VERSION__
*/
private const ALLOWED_PLUGINS_WHEN_FROZEN = [
'authentication' => ['cookie', 'joomla', 'ldap'],
Copy link
Copy Markdown
Contributor

@wilsonge wilsonge Aug 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to sound dumb. But if we use say a 3rd party auth plugin and have the core joomla ones disabled what's the impact?

I'm kinda going down the thought process of - we definitely should be limiting system plugins (because these are the majority of the things that screw up joomla updates) - but wondering whether we really can afford to restrict auth plugins (and maybe user plugins too - but less sure about the user plugins part).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a blatant cheat based on how we designed Joomla Update 😉

The Cookie login is used for the Remember Me feature. This normally will not be used to log you back into Joomla if your session has expired while extracting the update archive unless you have joined sessions and enabled the Remember Me feature. Having it enabled explicitly means that the user does not have to go through re-authentication to the Joomla backend which might not even work since at this point all plugins load and a broken 3PD plugin will break the login page as well.

Now, if the session did expire and the user was logged back in (manually or through cookie authentication) the anti-CSRF token we have included in the task=update.finalise URL will be invalid. At this point Joomla will ask the user to verify their username and password. The way this works it will only actually use the Joomla authentication. I am 99.5% certain it would not go through LDAP but since I have that 0.5% uncertainty I will allow it (it's core, it's updated).

Now, sure, what if they have disabled all Joomla user authentication and using their own plugin? I can't risk it because their auth plugin might not be Joomla 4.2 compatible. Also, based on my experience and the assumptions we are making in the core, you can't actually run Joomla having the joomla auth plugin disabled unless your custom plugin also somehow takes Joomla's user password, stored in the database, into account. So what happens in all the real sites I've seen is that the Joomla auth is always enabled.

Having talked about authentication with @SniperSister over email I think that we had come to the conclusion that this auth plugin was a must-have and its auth method always allowed if all else fails. But that's another question for another thread.

The best I can do to preserve our collective sanity is update the code to allow 3PD auth plugins if and only if the Joomla built-in auth plugin is not loaded. This way even the more “exotic” sites would be allowed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont we also need to whiteliste the 2fa/mfa plugins too when we need a reauth step?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Back when we had the discussion about the 3.10 -> 4.0 issues with plugins we said we can not do it because a system plugin can listen to all events and could even implement an alternative database layer?

But as long as within the few steps we dont have any database related steps we should be fine right?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont we also need to whiteliste the 2fa/mfa plugins too when we need a reauth step?

As well as webauthn? Couildn't we make it simpler and "just" disable none-core plugins?

I'm also not sure whether we can asume that one core auth plugin is up and running. For example within the joomla.org SSO setup no user actually has a PW of the site its just done via the SSO extension, I dont have details about that but @roland-d should have.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont we also need to whiteliste the 2fa/mfa plugins too when we need a reauth step?

No, because it's not a full login process. The user is already logged in. We are just revalidating the username and password. The idea is that the user is still logged in but the anti-CSRF token has changed. Will this practically happen? Mmm... maybe, if the app secret is split into two tokens (session secret and hashing secret), or if the session is somehow restarted. And yes, this revalidation will show even right after a user who was logged out during the update extraction has just logged back in and gone through the MFA process. This needs revising but that's for another day. Let's get our immediate problems addressed so we can live to code another day.

Back when we had the discussion about the 3.10 -> 4.0 issues with plugins we said we can not do it because a system plugin can listen to all events and could even implement an alternative database layer?

True, but! We still have the main DB credentials. These still have to be a DB server where we can write to and read from. The plugins you were talking about were common circa Joomla 1.5 and I have not seen them since (remember that publishing Akeeba Backup gives me a unique insight into all the weird configurations). So you had the plugin direct SELECT statements to slave servers configured in the plugin and everything else to the main, write-only server. So, removing the plugin would still have the site working, just not as efficiently.

Do keep in mind that these plugins are used for load-balancing databases, not making database connections possible. The upgrade is by its very nature a single user operation so load balancing can be safely ignored.

But as long as within the few steps we dont have any database related steps we should be fine right?

Wrong. These steps actually perform the schema update. The problem is that if a plugin kills the site the schema update never runs and we do not give the user an alternative. What's that? The Database Fix? Bollocks. I tried it on the site which broke. Yeah, it did fix the schema but it DID NOT insert new records, so all new plugins in Joomla 4.2 were not installed. I had to use Discover to install them. This also means that any other non-ALTER statements in Joomla update SQL files did not run. In the 4.2.0 upgrade the is safe, in a bigger upgrade (e.g. 3.10 to 4.x or 4.x to 5.x) skipping these SQL statements will most certainly result in borkage.

In the end of the day it is FAR more productive to issue upgrade guidance letting all of three people using a system plugin for database load balancing that this plugin will NOT be available while upgrading than have thousands of sites break because of third party plugins. The all of three people using those system plugins can figure out their upgrade strategy. The thousands of people with broken sites are left restoring backups (and asking me panicked how to restore them) or leaving Joomla because the update bricked their site.

A MAJOR concern for EVERY SINGLE SOFTWARE UPGRADE is how to mitigate the most common problems, minimise the possibility for adverse effect and, if possible, isolate them in edge cases handled by experienced people with the knowledge and means to successfully work around them. The second most major concern is documenting the upgrade process and its caveats so that the latter group can take appropriate action. You guys did neither for the 3.10 to 4.x upgrade. I tried telling you that your approach was wrong but you were unwilling to listen to me. So currently we have Joomla Update giving false and useless advise about plugins and extensions and people have learned to ignore it, bringing us back to square one with the additional burden of maintaining complex code which doesn't work. Sorry to be so direct but I've been very outspoken about this for over two years and I've frankly had enough of it :)

@nikosdion
Copy link
Copy Markdown
Contributor Author

@laoneo The problem is not just dispatching events but also creating the plugin object. For example, my plugin's constructor was loading the autoload.php of a private Composer vendor directory which had an older version of the same JWT library we ow have, in a newer version, in Joomla itself. The plugin was an integration with the Login with Apple service which uses JWT. Joomla 4.0 and 4.1 needed the extra library as it wasn't available in Joomla at the time. The old version returned an array of arrays (array[]) for the JWT claims, the new version returns an array of objects (stdClass[]) for the JWT claims. The WebAuthn system plugin has an array_map with a callback that uses concrete type hinting for its parameter as object, therefore the mere fact of my plugin loading was breaking Joomla 4.2. Oops. As a result I figured that pausing the loading of plugins makes far more sense.

@roland-d I was at the beach, sorry. I needed a break after working my butt off yesterday. I will conjure a system plugin which is deliberately sabotaged for Joomla 4.2 and update testing instructions.

@laoneo
Copy link
Copy Markdown
Member

laoneo commented Aug 18, 2022

Now I understand.

Side not: Due some class loading conflicts with other extensions in the past, I stopped loading vendor in constructor and only when the respective listener function is called, in my plugins. Looks like I'm safe then here as well 😏

@nikosdion
Copy link
Copy Markdown
Contributor Author

@laoneo I had the opposite problem. Some dudes copied my vendor folder wholesale in their plugins, causing my plugins to fail if they tried to load vendor's autoloader in the event handlers. So I had to follow the opposite direction to yours. I understand it's a uniquely “me” problem (part of the burden of having my code being the missing reference for building Joomla extensions), I am just explaining the method to my madness 😉

But only if plg_authentication_joomla is not published.
@nikosdion
Copy link
Copy Markdown
Contributor Author

I have updated the PR and the testing instructions.

PS: I hope you appreciate my taste in music 🤣

@zero-24 zero-24 added this to the Joomla 4.2.1 milestone Aug 18, 2022
@roland-d
Copy link
Copy Markdown
Contributor

@nikosdion

@roland-d I was at the beach, sorry. I needed a break after working my butt off yesterday. I will conjure a system plugin which is deliberately sabotaged for Joomla 4.2 and update testing instructions.

Of course, take your time on the beach, that is why I waited some hours from the morning :) I really appreciate your contributions. Joomla really benefits from your work.

So I have been following this further and I see a lot of discussion going on regarding the plugin handling and I really would like to see that moved to a separate PR so we can discuss/develop/test this further.

The part regarding the autoload_psr4.php deletion is tested and working fine and I want to merge that for the 4.2.1 RC release.

Could you do that please?

@nikosdion
Copy link
Copy Markdown
Contributor Author

@roland-d Yup, will do. Let me fix the CLI application first and I'll come back to split this PR into two.

@nikosdion
Copy link
Copy Markdown
Contributor Author

@roland-d I am making two PRs to replace this to avoid Git merge screw-ups.

The PR for the small change (removing autoload_psr4.php) is now live: #38525

I will post back when I create the second PR.

@roland-d
Copy link
Copy Markdown
Contributor

@nikosdion I understand, thank you very much.

@nikosdion
Copy link
Copy Markdown
Contributor Author

Second PR: #38526

I am closing this PR as it's been superseded by the two split PRs.

@richard67 richard67 removed this from the Joomla! 4.2.1 milestone Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants