Move to user factory aware trait

[laoneo]

Summary of Changes

Moves MFA and authentication plugins to use the new UserFactoryAwareInterface, introduced in #40337.

Testing Instructions

• Edit an user and go to the multifactor authentication tab
• Click on "Add a new Code by Email"
• Enter the code you got by mail
• Open the generated backup codes

Run some API requests with a token.

Actual result BEFORE applying this Pull Request

All Works.

Expected result AFTER applying this Pull Request

All works.

Link to documentations

Please select:

• Documentation link for docs.joomla.org:

• No documentation changes for docs.joomla.org needed

• Pull Request link for manual.joomla.org:

• No documentation changes for manual.joomla.org needed

[laoneo]

It is hard to find testers for this pr. @nikosdion can you have a look here as I really would like to move forward with this one.

[nikosdion]

I did a preliminary code review and it looks good and straightforward, so you get a tentative green light from me.

Please allow me the long weekend to perform manual tests on copies of different sites, across different OS, to be perfectly sure nothing broke.

Nitpick: I dislike method names like setXOnObject which may or may not set X depending on whether the object implements an interface. I'd rather name them something like potentiallySetXOnObject (inspired by WordPress' maybe_do_x convention for similar functions). It is a bit gnarlier, for sure, but it avoids desensitizing developers against potential bugs such as expecting X to be set on the object and then spending an hour trying to understand why it's not set. Yes, as I said, it's nitpicking 😉

[nikosdion]

I am getting an error trying to install the auto-generated update file:

Uncaught Error: Class "Joomla\Filesystem\File" not found in /var/www/jbeta/administrator/components/com_admin/script.php:7982\nStack trace:\n#0 /var/www/jbeta/administrator/components/com_joomlaupdate/finalisation.php(71): JoomlaInstallerScript->deleteUnexistingFiles()\n#1 /var/www/jbeta/administrator/components/com_joomlaupdate/extract.php(1958): finalizeUpdate()\n#2 {main}\n thrown in /var/www/jbeta/administrator/components/com_admin/script.php on line 7982', referer: https://jbeta.local.web/administrator/index.php?option=com_joomlaupdate&task=update.install&99b08aebd02b84e14c9787f93b51fc53=1

I have tracked the problem down to a commit by @Hackwar. However, I am not going to tell you what or why, seeing that he thinks so lowly of me. Hint: you can trivially fix it by removing four characters from a single .php file. I'm more than happy to do a PR as soon as Hannes apologises for his slandering me on Twitter since @SniperSister has confirmed 14 months ago the existence of the bug Hannes stubbornly rejected 25 months ago, and as long as issue #32592 and Hannes' tweet are deleted. I understand that everyone makes mistakes and will forgive people who own up to them.

[laoneo]

Did you try to install the nightly package or through patchtester? Not sure what this issue has to do with this pr as I touched mainly MFA code and API auth plugins.

[nikosdion]

I am trying to convert an existing Joomla 4.3 site to 4.4 so I can test with a real site (a clean installation does work, as far as I can tell). Basically, I wanted to make sure that existing data wouldn't break for any reason I can't easily see just by doing code review.

For this, I was trying to use the auto-generated update file from https://ci.joomla.org/artifacts/joomla/joomla-cms/4.4-dev/40430/downloads/65057

However, there's an error in com_admin's script.php (and I confirmed it's in the 4.4-dev branch) which prevents the update from working: the last AJAX request in the updater leads to a 500 error message with the error log saying what I pasted in my previous message.

This needs to be fixed in 4.4-dev first, then you need to merge the 4.4-dev branch, then I can test with real sites. I have three sites I can readily test with: my business site, my blog, and my development site. They all use all sorts of different MFA methods.

[laoneo]

Got it, looks like we have an issue when updating from 4.3 to 4.4. Will investigate and report back when solved. Thanks for this extensive test, really appreciated.

[nikosdion]

@laoneo Look in com_joomlaupdate, finalise.php. Delete the \CMS part of namespace Joomla\Filesystem\CMS.

If you need a refresher on why this is necessary, tag me in your PR.

[Hackwar]

I have tracked the problem down to a commit by @Hackwar. However, I am not going to tell you what or why, seeing that he thinks so lowly of me. Hint: you can trivially fix it by removing four characters from a single .php file. I'm more than happy to do a PR as soon as Hannes apologises for his slandering me on Twitter since @SniperSister has confirmed 14 months ago the existence of the bug Hannes stubbornly rejected 25 months ago, and as long as issue #32592 and Hannes' tweet are deleted. I understand that everyone makes mistakes and will forgive people who own up to them.

I have no idea what you are talking about and since you blocked me on Twitter, I also have no idea which tweet you mean. I also don't intend to read 20 pages of text to find out when in the last 10 years I might have insulted you. If you are referring to me being angry about the drama you once again caused a few months ago, I stand with the internal comments I made at that time.

[nikosdion]

@Hackwar I am talking about this:

So, you stand by your "internal comment" (public tweet) that I "f**ed up" (pardon the language, I am quoting verbatim) my extension updates despite my analysis being correct and verified by another core contributor and misrepresent your temper tantrum as me causing drama. Okay. Got it.

[Hackwar]

While I'm not talking about that comment, I'm also not going to delete that tweet. You are projecting that that tweet is about you, or is it saying "Akeeba" anywhere in there? Anyway, I'm not going to discuss this further, especially here. If you have a problem with me, you know how to contact me.

[nikosdion]

Insulting people's intelligence when caught in a compromising situation is the lowest form of cowardice.

[brianteeman]

While I'm not talking about that comment, I'm also not going to delete that tweet. You are projecting that that tweet is about you, or is it saying "Akeeba" anywhere in there? Anyway, I'm not going to discuss this further, especially here. If you have a problem with me, you know how to contact me.

grow up, please

[laoneo]

@nikosdion can you open a new issue for the update problem? I tried to reproduce the issue, by running the update from 4.3.0 to the dev package of this pr (used the update and the full one), but couldn't. Despite I see the problem in the code, for me was the framework package correctly loaded. So I don't want to block this pr out.

[brianteeman]

I just tried an upload and update with the generated package from this PR.

The update log only contains the following

#
#<?php die('Forbidden.'); ?>
#Date: 2023-05-01 20:27:10 UTC
#Software: Joomla! 4.3.1-rc2-dev Development [ Bora ] 28-April-2023 16:01 GMT

#Fields: datetime	priority clientip	category	message
2023-05-01T20:27:10+00:00	INFO ::1	update	Starting installation of new version.

Apache error log contains

Mon May 01 21:28:05.356142 2023] [fcgid:warn] [pid 1652:tid 960] [client ::1:18396] mod_fcgid: stderr: PHP Fatal error:  Uncaught Error: Class "Joomla\\Filesystem\\File" not found in C:\\htdocs\\j42\\administrator\\components\\com_admin\\script.php:7945, referer: http://localhost/j42/administrator/index.php?option=com_joomlaupdate&task=update.install&53692b380aa4f44e4d07c6e78d3a41f0=1
[Mon May 01 21:28:05.356142 2023] [fcgid:warn] [pid 1652:tid 960] [client ::1:18396] mod_fcgid: stderr: Stack trace:, referer: http://localhost/j42/administrator/index.php?option=com_joomlaupdate&task=update.install&53692b380aa4f44e4d07c6e78d3a41f0=1
[Mon May 01 21:28:05.356142 2023] [fcgid:warn] [pid 1652:tid 960] [client ::1:18396] mod_fcgid: stderr: #0 C:\\htdocs\\j42\\administrator\\components\\com_joomlaupdate\\finalisation.php(71): JoomlaInstallerScript->deleteUnexistingFiles(), referer: http://localhost/j42/administrator/index.php?option=com_joomlaupdate&task=update.install&53692b380aa4f44e4d07c6e78d3a41f0=1
[Mon May 01 21:28:05.356142 2023] [fcgid:warn] [pid 1652:tid 960] [client ::1:18396] mod_fcgid: stderr: #1 C:\\htdocs\\j42\\administrator\\components\\com_joomlaupdate\\extract.php(1958): finalizeUpdate('C:\\\\htdocs\\\\j42', 'C:/htdocs/j42/a...'), referer: http://localhost/j42/administrator/index.php?option=com_joomlaupdate&task=update.install&53692b380aa4f44e4d07c6e78d3a41f0=1
[Mon May 01 21:28:05.356142 2023] [fcgid:warn] [pid 1652:tid 960] [client ::1:18396] mod_fcgid: stderr: #2 {main}, referer: http://localhost/j42/administrator/index.php?option=com_joomlaupdate&task=update.install&53692b380aa4f44e4d07c6e78d3a41f0=1
[Mon May 01 21:28:05.356142 2023] [fcgid:warn] [pid 1652:tid 960] [client ::1:18396] mod_fcgid: stderr:   thrown in C:\\htdocs\\j42\\administrator\\components\\com_admin\\script.php on line 7945, referer: http://localhost/j42/administrator/index.php?option=com_joomlaupdate&task=update.install&53692b380aa4f44e4d07c6e78d3a41f0=1

[laoneo]

Thanks. Can you then open an issue as it will very likely happen with any 4.4-dev pr? So it is not really related to this pr. Thanks...

[laoneo]

@nikosdion can you do the tests also without the updates? I really tried to reproduce the issue on my server and nothing produced the error. So to be able to move forward with this pr, it would be nice when you can do at least the other tests which do not require an update from 4.3 to 4.4?

[Hackwar]

I installed a fresh Joomla 4.2.9 and 4.3.2-dev on windows, enabled debugging and then uploaded the current package from this PR and both tests were successfull. @brianteeman can you see where our setups differ that this fails for you and works for me?

[brianteeman]

Doubt it

[Hackwar]

Ok, I can't see why this happens for you, but it will have to be related to the copy of the Filesystem package in the finalisation.php. Since the switchover from the CMS filesystem package to the framework filesystem package is not consistent yet, I will revert this for the script.php. When all core code has been moved over to the framework classes in 5.0, we can do these files in one go.

[joomdonation]

I can't see why this happens for you

@Hackwar By just code reading, I think it happens because at the end of extract process, we call deleteUnexistingFiles method from JoomlaInstallerScript https://github.com/joomla/joomla-cms/blob/4.4-dev/administrator/components/com_joomlaupdate/finalisation.php#L71

Since finalisation.php is a standalone php script, the framework filesystem package is not loaded, so the Joomla\Filesystem\File class is not available and it causes fatal error. We can try to fix the code in the file finalisation.php, follow the hint here #40430 (comment) if you don't want to revert the change in script.php of com_admin.

[Hackwar]

I've created a PR to revert the script.php change here: #40569 We will have to fix that all at once at another time. The problem is, that the Folder class has not been switched over to the framework yet and thus if we switch the classes in the finalisation.php to the other namespace, the Folder stuff will throw the next error...

[richard67]

@Hackwar Possibly there is something regarding opcache missing in the framework filesystem package which is there in the CMS filesystem library? There was a PR by Phil T. which he closed: joomla-framework/filesystem#38 . And as I observed, opcache is switched on in the default configuration of recent PHP versions.

[basd82]

I have tested this item ✅ successfully on f97f853

Test was a suc6

did 13 (lucky number) api calls

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40430.}

[MacJoom]

I did some tests and my impression is that all of the packages before #40569 throw the server error upon an update from 4.3 to 4.4. Strange thing is that if you have Display Errors=ON in your php.ini (as i had for debugging purposes) you will get no visibile error in the backend, the update seems to run without a problem - will get an error in you error.log only.
With #40569 integrated this PR installs OK