[#41952] - [4.4] Fix "group by" in com_users users model for strict SQL syntax
- Closed
- 28 Sep 2023
- Medium
- Build: 4.4-dev
- # 41952
- Diff
- richard67:4.4-dev-fix-group-by-user-mfa
- Pending continuous-integration/drone/pr Build is running Details
User tests: Successful: Unsuccessful:
Pull Request for Issue #41951 .
Summary of Changes
For correct SQL syntax of "GROUP BY" statements, all columns in the result set either must be part of the "GROUP BY" clause or be used with an aggregate function.
Examples:
SELECT id, name, MAX(column1), MIN(column2) FROM sometable GROUP BY id, name is correct.
SELECT id, name, column1, MIN(column2) FROM sometable GROUP BY id, name is not correct.
SELECT id, name, column1, MIN(column2) FROM sometable GROUP BY id, name, column1 is correct.
This is the case for all RDBMS which I know (PostgreSQL, Oracle, MS SQL Server), only MySQL and MariaDB were tolerant with this in past. MySQL 8 might be less tolerant when in strict SQL mode beginning at a particular version.
This PR fixes the SQL in the "getListQuery" method of the "Users" model of com_users to fix the SQL error shown e.g. on PostgreSQL when MFA is enabled.
Testing Instructions
If possible and available, test with PostgreSQL, MariaDB, MySQL 8 (whatever of these you have) and report back which database(s) you have used.
- Have some users with MFA enabled and some others without.
- In the administrator side menu, select "Users" -> "Groups".
- Click on any entry on the "Enabled Users" or "Blocked Users" column.
- Check the log file of your database server.
Actual result BEFORE applying this Pull Request
In the database server's log file:
An error has occurred.
500 42803, 7, ERROR: column "mfa.mfaRecords" must appear in the GROUP BY clause or be used in an aggregate function LINE 1: SELECT a.,"mfa"."mfaRecords" ^ 42803, 7, ERROR: column "mfa.mfaRecords" must appear in the GROUP BY clause or be used in an aggregate function LINE 1: SELECT a.,"mfa"."mfaRecords"
Expected result AFTER applying this Pull Request
It shows a list of enabled or blocked users with a group.
There is no error like mentioned above in the database server's log file.
Link to documentations
Please select:
-
No documentation changes for docs.joomla.org needed
-
No documentation changes for manual.joomla.org needed
joomla-cms-bot
- | Category | ⇒ | Administration com_users |
| Status | New | ⇒ | Pending |
| Title |
|
||||||
hold on
i'm doing something wrong
Hmm it seems there is really something wrong, system tests for com_user failing. If they do again, I will put this PR into draft mode until I was able to test it myself.
Here are my modifications which work with PostgreSQL and MySQL. But I only did some simple tests on 4.3.4 installations. No errors appeared.
--- administrator/components/com_users/src/Model/UsersModel.php.orig 2023-08-23 02:21:56.966929868 -0400
+++ administrator/components/com_users/src/Model/UsersModel.php 2023-09-27 19:42:35.213056111 -0400
@@ -365,10 +365,7 @@
$groups = $this->getState('filter.groups');
if ($groupId || isset($groups)) {
- $query->join('LEFT', '#__user_usergroup_map AS map2 ON map2.user_id = a.id')
- ->group(
- $db->quoteName(
- [
+ $group_by = [
'a.id',
'a.name',
'a.username',
@@ -385,9 +382,13 @@
'a.otpKey',
'a.otep',
'a.requireReset',
- ]
- )
- );
+ ];
+ if (PluginHelper::isEnabled('multifactorauth')) {
+ $group_by[] = 'mfa.mfaRecords';
+ }
+
+ $query->join('LEFT', '#__user_usergroup_map AS map2 ON map2.user_id = a.id')
+ ->group($db->quoteName($group_by));
if ($groupId) {
$groupId = (int) $groupId;
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-09-28 15:20:31 |
| Closed_By | ⇒ | richard67 | |
| Labels |
Added:
bug
PR-4.4-dev
|
||
I have tested this item ✅ successfully on b582410
on postgres
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41952.