Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Joomla 4 compatibility to CRM_Utils_System_Joomla::loadUser() #31601

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add Joomla 4 compatibility to CRM_Utils_System_Joomla::loadUser() #31601

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
85b6def
Add Joomla 4 compatibility to CRM_Utils_System_Joomla::loadUser()
webmaster-cses-org-uk Dec 15, 2024
612b12a
Fix typographical issues
webmaster-cses-org-uk Dec 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion CRM/Utils/System/Joomla.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,29 @@ public function loadUser($username, $password = NULL) {
}
$contactID = CRM_Core_BAO_UFMatch::getContactId($uid);
if (!empty($password)) {
$instance = JFactory::getApplication('site');
if (version_compare(JVERSION, '4.0.0', 'ge')) {
// Joomla 4 compatible way of getting the SiteApplication object, not the generic global application object
// we need this because only the specific site application object has the login() method
// the below is adapted from <JOOMLA_ROOT>/includes/app.php on Joomla 4.4.9
$joomlaBase = self::getBasePath();
require_once $joomlaBase . '/plugins/authentication/joomla/src/Extension/Joomla.php';
Copy link
Contributor

Choose a reason for hiding this comment

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

@webmaster-cses-org-uk I'm thinking this is probably sensible in a cron.php context but does it still make sense if Joomla has already been booted does it cause any issues you think?

@demeritcowboy @totten you folks might now if we do require_once on files that might have already been included by joomla would there be warnings/issues emitted by PHP?

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't see how it would give a warning.
Regarding the timing of it I don't know enough (anything) about joomla to comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@seamuslee001 I guess we shouldn't really be calling cron.php if Joomla is already booted but we could put some sort of condition round it to guard against double-initialisation (e.g. check for _JEXEC defined already or something like that). What I would say is this whole file is going to require a major rewrite for Joomla 5, all I've done here is a quick fix, but I could look at putting an extra condition round lines 499-512.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry forgot to mention... I did check and as far as I could tell this method is only called from cron.php, but that's not a reason/excuse not to make it robust like you suggest!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK 3rd reply then I must leave this for a bit!

Having had a closer look, there is already some similar code in loadBootStrap() in the same file, which looks like it may be called before loadUser() in cli.class.php. However that loads either the ConsoleApplication (which does not have the login() method - hence why cli.php fails) or the AdministratorApplication (which does have login() but for whatever reason we seem to want the site here - maybe this is wrong / not necessary?).

So with this change we actually would be overwriting the initialisation done in loadBootStrap() - although it seems to work fine - so I would suggest that the actual fix is to harmonise these somehow so that the correct application object is available when needed.

require_once $joomlaBase . '/plugins/authentication/cookie/src/Extension/Cookie.php';
require_once $joomlaBase . '/plugins/user/token/src/Extension/Token.php';
require_once $joomlaBase . '/plugins/user/joomla/src/Extension/Joomla.php';
$container = \Joomla\CMS\Factory::getContainer();
$container->alias('session.web', 'session.web.site')
->alias('session', 'session.web.site')
->alias('JSession', 'session.web.site')
->alias(\Joomla\CMS\Session\Session::class, 'session.web.site')
->alias(\Joomla\Session\Session::class, 'session.web.site')
->alias(\Joomla\Session\SessionInterface::class, 'session.web.site');
$app = $container->get(\Joomla\CMS\Application\SiteApplication::class);
\Joomla\CMS\Factory::$application = $app;
$instance = JFactory::getApplication();
}
else {
$instance = JFactory::getApplication('site');
}
$params = [
'username' => $username,
'password' => $password,
Expand Down