[#32540] - [4.0] Click Saving error of two-factor alternate of profile in PHP 8
- Fixed in Code Base
- 10 Mar 2021
- Medium
- Build: 4.0-dev
- # 32540
- Diff
- himanshu007-creator:himanshu007-creator-patch-1
User tests: Successful: Unsuccessful:
Summary of Changes
If a user makes no changes to his profile and tries to click on Save or Save & Close button, an ERROR Message pops up.
Testing Instructions
USE PHP 8
- click on
User Menuoption at the top-right. - select
Edit Accountoption - without making any changes to it, click on
SaveorSave & Closebuttons.
error message pops up
- now apply this patch and repeat steps 1-3
NO ERROR MESSAGE POPS NOW
- Go to
System/Plugins - Search for
'Auth' - Enable any of the Two Factor Authentication Pluggin
-
> Two Factor Authentication - YubiKey -
> Two Factor Authentication - Google Authenticator
- repeat steps 1-3
- NO ERROR MESSAGE POPS NOW .
Actual result BEFORE applying this Pull Request
Expected result AFTER applying this Pull Request
Documentation Changes Required
No
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_admin Layout |
@himanshu007-creator Could you switch on "Debug System" and set "Error Reporting" to "Maximum" in Global Configuration in backend? This should then tell you where the error happens. Maybe it should be fixed at that place instead of like you propose it with this PR?
i changed these files
- / layouts / joomla / edit / fieldset.php
- / administrator / components / com_admin / tmpl / profile / edit.php
I have tested this item
I have tested the patch
before applying the patch error was coming after applying it there was no error.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32540.
@himanshu007-creator Your fix is wrong. From my quick look, you can change this line of code https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_users/src/Model/UserModel.php#L260 to one of the two options below:
if (array_key_exists('twofactor', $data) && is_array($data['twofactor']))OR
if (isset($data['twofactor']['method']))And the issue should be sorted.
@himanshu007-creator Your fix is wrong. From my quick look, you can change this line of code https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_users/src/Model/UserModel.php#L260 to one of the two options below:
if (array_key_exists('twofactor', $data) && is_array($data['twofactor']))OR
if (isset($data['twofactor']['method']))And the issue should be sorted.
yes! @joomdonation sir, it worked out!. So should i make changes now in my PR as asked ?
The fix is not wrong, it fixes a broken behaviour in handling automated fieldset rendering in Joomla!, where the given ignore variable is ignored...
If no 2fa is activated, nothing should be send to the server, that's why the hidden field should not be rendered. Therefor the fix.
Additional one could also fix the if. But the changes here have to be there for sure.
@himanshu007-creator could you additional add the 2nd suggestion from @joomdonation to your PR (just make the change in the same branch and push)
@himanshu007-creator could you additional add the 2nd suggestion from @joomdonation to your PR (just make the change in the same branch and push)
Sure! , working on it.
| Labels |
Added:
?
|
||
| Category | Administration com_admin Layout | ⇒ | Administration com_admin com_users Layout |
I have tested this item
Tested successfully in Beta8-dev of 3 March using Wampserver 3.2.4 and PHP 8.0.2.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32540.
I have tested this item
Tested successfully using PHP 8.0.0 and Joomla! 4.0.0-beta8-dev Development(Joomla version).
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32540.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32540.
Something seems weird here. We should be allowing users to change their 2FA settings in the com_admin view. This fixes the issue by not showing/allowing modification of the 2FA settings.
Something seems weird here. We should be allowing users to change their 2FA settings in the com_admin view. This fixes the issue by not showing/allowing modification of the 2FA settings.
@wilsonge Not sure about that. I've applied the patch here and still can change my 2FA settings in com_users. Or am I missing something?
Quy
- | Status | Ready to Commit | ⇒ | Pending |
@himanshu007-creator Could you do as suggested in the code review which you have marked as resolved, i.e. revert the changes in files administrator/components/com_admin/tmpl/profile/edit.php and layouts/joomla/edit/fieldset.php, so that only the change in file administrator/components/com_users/src/Model/UserModel.php remains? Thanks in advance.
@himanshu007-creator Could you do as suggested in the code review which you have marked as resolved, i.e. revert the changes in files
administrator/components/com_admin/tmpl/profile/edit.phpandlayouts/joomla/edit/fieldset.php, so that only the change in fileadministrator/components/com_users/src/Model/UserModel.phpremains? Thanks in advance.
Yes ! I am working on it??
| Labels |
Added:
?
|
||
| Category | Administration com_admin Layout com_users | ⇒ | Administration com_users Layout |
| Category | Administration Layout com_users | ⇒ | Administration com_users |
Made the changes as mentioned in the review.
@himanshu007-creator Thank you very much.
@toivo @srishty-07 Could you test this PR again? It has received changes meanwhile. Thanks in advance.
I have tested this item
Tested successfully in Beta8-dev of 6 March in Wampserver 3.2.4 using PHP 8.0.2.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32540.
I have tested this item
Tested successfully using PHP 8.0.0 and Joomla! 4.0.0-beta8-dev Development(Joomla version).
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32540.
| Labels |
Removed:
?
|
||
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32540.
rdeutz
- | Labels |
Added:
?
?
|
||
| Labels |
Added:
?
Removed: ? |
||
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-03-10 19:10:15 |
| Closed_By | ⇒ | rdeutz | |
| Labels |
Added:
?
Removed: ? |
||
@himanshu007-creator Could you switch on "Debug System" and set "Error Reporting" to "Maximum" in Global Configuration in backend? This should then tell you where the error happens. Maybe it should be fixed at that place instead of like you propose it with this PR?