? Pending

User tests: Successful: Unsuccessful:

avatar zero-24
zero-24
18 Sep 2021

Summary of Changes

Make sure only non blocked users can reciver actionlogs emails

Testing Instructions

create an user
Setup actionlogs notifications for that user
trigger notifications
disable / block the user
trigger more notifications
notice that the now blocked user does not get the notifications any more

Actual result BEFORE applying this Pull Request

Even blocked users get notifications

Expected result AFTER applying this Pull Request

Blocked users does not get notifications anymore

Documentation Changes Required

None

avatar zero-24 zero-24 - open - 18 Sep 2021
avatar zero-24 zero-24 - change - 18 Sep 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 18 Sep 2021
Category Administration
avatar alikon alikon - test_item - 18 Sep 2021 - Tested successfully
avatar alikon
alikon - comment - 18 Sep 2021

I have tested this item successfully on bdb7e4b


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35591.

avatar zero-24 zero-24 - change - 18 Sep 2021
Title
Make sure only non blocked users can reciver actionlogs emails
Make sure only non blocked users can recive actionlogs emails
avatar zero-24 zero-24 - edited - 18 Sep 2021
avatar PhilETaylor
PhilETaylor - comment - 18 Sep 2021

Conceptually this is wrong... Blocking a user is about authentication. It's whether a user can login or not. It is not a flag to enable/disable other features. Its a flag to prevent login.

There is a valid use case where support@example.com has an account, which wishes to receive action log emails, but doesnt have permission to login to the website. This is a real world example.

We also have a sendEmail attribute of a user which seems more appropriate for this.

However, as in all things Joomla, there is historical prescient in that Joomla Update Notification emails dont get set to super Admins if they are blocked, and when sendMail is set to 0 - so yeah... the calls for a Joomla Architect Role in the project cannot come soon enough.

avatar zero-24 zero-24 - change - 18 Sep 2021
Labels Added: ?
avatar zero-24
zero-24 - comment - 18 Sep 2021

We also have a sendEmail attribute of a user which seems more appropriate for this.

That was used in the past for "system mails" like activate users.

Conceptually this is wrong... Blocking a user is about authentication. It's whether a user can login or not. It is not a flag to enable/disable other features. Its a flag to prevent login.

Its a flag about whether a user is enabled or not which might has impact on other features as well like when you disable an article in com_content?

However, as in all things Joomla, there is historical prescient in that Joomla Update Notification emails dont get set to super Admins if they are blocked, and when sendMail is set to 0 - so yeah... the calls for a Joomla Architect Role in the project cannot come soon enough.

https://github.com/joomla/joomla-cms/blob/3.10-dev/plugins/system/updatenotification/updatenotification.php#L355-L366

The update mail is send to super users not blocked but have sendEmail (where sendEmail is Receive System Emails) set to 1?

There is a valid use case where support@example.com has an account, which wishes to receive action log emails, but doesnt have permission to login to the website. This is a real world example.

Just go and create an enabled users with the role "Guest" and it should be good to go too?

avatar PhilETaylor
PhilETaylor - comment - 18 Sep 2021

Thanks for basically repeating everything I said.

Unless you are calling this an official security issue, this is a backward compatibility change that effects real world sites as I have demonstrated and therefore cannot be merged to 3.10.x - merging this would/could effect behaviour

Sending User Action Logs to a Guest User? You've lost the plot...

avatar zero-24
zero-24 - comment - 18 Sep 2021

Unless you are calling this an official security issue, this is a backward compatibility change that effects real world sites as I have demonstrated and therefore cannot be merged to 3.10.x - merging this would/could effect behaviour

Well when you depend on issues we are fixing in later versions you will be affected by many changes we do right?

I dont think fixing such obvius issues and inconsistencies with other parts of the code should fall under backward compatibility and therefore be moved to J5. Given that by than this would be called a feature by others in the first place.

avatar PhilETaylor
PhilETaylor - comment - 18 Sep 2021

well as "block" is undocumented (that I can find) then I guess its open to interpretation - just like much of Joomla...

I interpret it as a way to stop someone logging in. Not as a general flag to prevent other features.

avatar zero-24
zero-24 - comment - 18 Sep 2021

I interpret it as a way to stop someone logging in. Not as a general flag to prevent other features.

I interpret it the same way (stop someone logging in) but not limited to that. Similiar to what other disable settings of stuff in Joomla does, like deny to get update emails or dont list them in the user list to be secected as author.

So also right now that setting already impact other features similiar to what is proposed here.

avatar PhilETaylor
PhilETaylor - comment - 18 Sep 2021

So "User phones helpdesk and says his wallet was stolen with his 2fa usb key in it, Super Admin blocks the users account" means that this user cannot be added as an author to an article by his fellow work buddies???.... yeah talk about cross-over of feature roles here...

avatar zero-24
zero-24 - comment - 18 Sep 2021

So "User phones helpdesk and says his wallet was stolen with his 2fa usb key in it, Super Admin blocks the users account" means that this user cannot be added as an author to an article by his fellow work buddies???.... yeah talk about cross-over of feature roles here...

When I would be that super admin I would not block the user in the first place but directly coordinate with him to reset the PW and 2FA -> Done.
An blocked user can not recover its access without an super admin anyway so I dont see a point in just blocking him when I have him on the call and go and set a new inital PW and remove the 2FA setting? Yes I know I also need to check thats actually the user and get prove about that before removing the 2FA setup and share a new PW.

But yes right now for the time that the user would be blocked the user can no longer be added as author for new articles nor get system mails for Joomla Core updates etc.

avatar PhilETaylor
PhilETaylor - comment - 23 Sep 2021

I'll mark this as a successful test - however I have made my point, and in the absence of any actual architecture lead in Joomla, and without accurate documentation on what the block/sendMails features should/should not do, its hard to disagree.

avatar PhilETaylor PhilETaylor - test_item - 23 Sep 2021 - Tested successfully
avatar PhilETaylor
PhilETaylor - comment - 23 Sep 2021

I have tested this item successfully on 3d50dfd


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35591.

avatar PhilETaylor
PhilETaylor - comment - 23 Sep 2021

This branch is out-of-date with the base branch

avatar zero-24 zero-24 - change - 21 Nov 2021
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-11-21 13:21:59
Closed_By zero-24
avatar zero-24 zero-24 - close - 21 Nov 2021
avatar zero-24 zero-24 - merge - 21 Nov 2021
avatar zero-24
zero-24 - comment - 21 Nov 2021

Merging this now. Thanks

Add a Comment

Login with GitHub to post a comment