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.

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

Fix "email changed" activity email check #34135

Merged

Conversation

julien-nc
Copy link
Member

@julien-nc julien-nc commented Sep 19, 2022

Check disable_activity.email_address_changed_by_admin when email is changed by admin via the OCS API and not just only when set via OCC.

This setting value was never checked if there was an actor (meaning it was done by the API) but if the actor is different than the target user, we can safely assume it is an admin and apply the settings which prevent notifications for this event.

If you want to test that, here are the 2 methods to change a user email address as an admin:
OCC:

occ user:setting USER_ID settings email 'NEW_EMAIL'

OCS API:

curl -H "ocs-apirequest: true" \
    -u ADMIN_USER_ID:ADMIN_PASSWORD \
    -X PUT \
    -d 'key=email&value=URL_ENCODED_NEW_EMAIL' \
    https://my.nc.org/ocs/v1.php/cloud/users/USER_ID

How to toggle the setting:

# DISABLE the notification
occ config:app:set settings disable_activity.email_address_changed_by_admin --value yes
# ENABLE it
occ config:app:set settings disable_activity.email_address_changed_by_admin --value no

With this PR, there is no more email notification when changing the email via the OCS request when the user making the request is different than the target user.

This could be backported to 24 and 23.

@julien-nc
Copy link
Member Author

/backport to stable23

@julien-nc
Copy link
Member Author

/backport to stable24

@blizzz blizzz mentioned this pull request Sep 20, 2022
@blizzz blizzz mentioned this pull request Sep 22, 2022
2 tasks
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022
@julien-nc
Copy link
Member Author

/backport to stable25

Copy link
Member

@marcelklehr marcelklehr left a comment

Choose a reason for hiding this comment

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

Changes make sense ♥️

@julien-nc julien-nc force-pushed the fix/noid/disable_activity.email_address_changed_by_admin branch from db00d57 to 9362fd7 Compare October 7, 2022 12:45
@julien-nc julien-nc force-pushed the fix/noid/disable_activity.email_address_changed_by_admin branch from 9362fd7 to 390a3f8 Compare October 13, 2022 13:16
…hanged by admin via the OCS API

Signed-off-by: Julien Veyssier <eneiluj@posteo.net>
@julien-nc julien-nc force-pushed the fix/noid/disable_activity.email_address_changed_by_admin branch from 390a3f8 to 08a7731 Compare October 14, 2022 07:54
@julien-nc
Copy link
Member Author

Drone failure is unrelated

@julien-nc julien-nc merged commit a8101ec into master Oct 14, 2022
@julien-nc julien-nc deleted the fix/noid/disable_activity.email_address_changed_by_admin branch October 14, 2022 08:55
@blizzz blizzz added the pending documentation This pull request needs an associated documentation update label Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug feature: activity and notification pending documentation This pull request needs an associated documentation update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants