Feature #25445
closedunify email notifications
0%
Description
Under certain circumstances, separate mail notifications are sent to users with different roles (e.g. one mail with To:Assignee, Author and one with no To and Cc:Watcher). We'd prefer to have a unified mail primarily to let the recipients know who else got the same notification.
I attach a patch to fix this issue. (I couldn't make the code less awkward than it was due to those seemingly useless yields in the called methods.)
Files
Updated by Holger Just over 7 years ago
The code as it is written in Redmine was chosen explicitly to ensure that people only see data in notifications that they are allowed to see. This includes:
- the ability to see other users in the To and CC headers (as enforced by the user visibility setting on roles)
- the ability to see custom fields (as specified by visibility options on the custom fields)
Your patch removes these distinctions and send all the data to all users. Since the visible details in the notification mail are determined by the first recipient (in Mailer#issue_edit
), this would lead to unintended data leaks. The code as it is today ensures that the minimal number of mails is sent while ensuring that each mail shows the exact correct amount of data without leaking too much or omitting any data.
Given this, your patch can't be used without compromising existing visibility permissions.
Updated by Holger Just over 7 years ago
See #1005 for details about this feature, especially #1005#note-36 ff.
Updated by Greg T over 7 years ago
Thanks for the updates.
Holger Just wrote:
- the ability to see other users in the To and CC headers (as enforced by the user visibility setting on roles)
I don't quite understand this, it seems a complex problem. When I added a watcher who is not a project member, no notification was sent to him and he couldn't access the issue. Can you describe a working example?
- the ability to see custom fields (as specified by visibility options on the custom fields)
Yes, that made a difference in our case. Though, the visibility was disabled only to separate concerns, not based on confidentiality. And there was no change in those fields.
Your patch removes these distinctions and send all the data to all users. Since the visible details in the notification mail are determined by the first recipient (in
Mailer#issue_edit
), this would lead to unintended data leaks.
I didn't know that and I didn't expect that: it doesn't look to me like a clear design.
The code as it is today ensures that the minimal number of mails is sent while ensuring that each mail shows the exact correct amount of data without leaking too much or omitting any data.
I understand that and I've no idea how to solve this. But we will probably forward these mails when we don't see the other party in the TO/CC list, so we'll "leak" information anyway. Technically, it won't be Redmine's fault.
A related problem: is there a standard way to leave out the superfluous (for me) unchanged fields and description from these notifications so we can minimize the manual information leak and have less verbosity?
Updated by Holger Just over 7 years ago
Holger Just wrote:
- the ability to see other users in the To and CC headers (as enforced by the user visibility setting on roles)
I don't quite understand this, it seems a complex problem. When I added a watcher who is not a project member, no notification was sent to him and he couldn't access the issue. Can you describe a working example?
You can define for each role which users members with this role should be able to see:
- any users
- only users which are members of any of the same project as the user
This can be used to make sure that certain users can not learn anything about the existence of other users in Redmine, which is e.g. a requirement if you add your customers to Redmine as users and they should not learn from each other. This feature was added in #11724. Please see there and in #5159 for a rational for these features.
- the ability to see custom fields (as specified by visibility options on the custom fields)
Yes, that made a difference in our case. Though, the visibility was disabled only to separate concerns, not based on confidentiality. And there was no change in those fields.
One could argue that you could merge together groups of people if no different confidential data is included in the mail (e.g. in your case if the custom field was not changed). However, this would add a lot of knowledge from the views (i.e. the rendered mails) into the controller and would make the decision matrix for which users to group together much bigger and thus more complex and error-prone.
As such, I think it is still a good idea to continue grouping users strictly by their common visibility rules to ensure that the view doesn't have to be extremely careful which data to render.
Your patch removes these distinctions and send all the data to all users. Since the visible details in the notification mail are determined by the first recipient (in
Mailer#issue_edit
), this would lead to unintended data leaks.I didn't know that and I didn't expect that: it doesn't look to me like a clear design.
Well, this works well as long as you ensure that all recipients have the same visibility permissions, which is exactly what happens here. With this ensured, it's quite okay to use the first user of the list since all of them will follow the same rules. The only alternative would have need to send a single mail per recipient (which is currently not done for various reasons).
The code as it is today ensures that the minimal number of mails is sent while ensuring that each mail shows the exact correct amount of data without leaking too much or omitting any data.
I understand that and I've no idea how to solve this. But we will probably forward these mails when we don't see the other party in the TO/CC list, so we'll "leak" information anyway. Technically, it won't be Redmine's fault.
Well, forwarding of mails is not a problem of Redmine. We as a project however need to ensure that no data is leaked inside of Redmine. By sending a single mail in all cases, this would not be the case as explained above.
A related problem: is there a standard way to leave out the superfluous (for me) unchanged fields and description from these notifications so we can minimize the manual information leak and have less verbosity?
You can update the mailer views. But that would be outside the scope of this issue.
Updated by Greg T over 7 years ago
Holger Just wrote:
I understood that. Still, I had the same problem as in #5159#note-15. I had toYou can define for each role which users members with this role should be able to see:
- any users
- only users which are members of any of the same project as the user
- make the project public (which may be OK)
AND - add Non member users group to the project as a member with a role (which is awkward)
to make the non-member watcher able to access the issue. It doesn't seem right to be able to add a watcher who can't access the issue.
As such, I think it is still a good idea to continue grouping users strictly by their common visibility rules to ensure that the view doesn't have to be extremely careful which data to render.
OK. As what I wanted can't be done, you may close this issue.
Updated by Toshi MARUYAMA over 7 years ago
- Tracker changed from Patch to Feature
- Status changed from New to Closed
- Resolution set to Invalid