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.

Sign up for GitHub

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

Jump to bottom

Email notification for demoted/removed user #3264

Closed
brainwane opened this issue Mar 15, 2018 · 8 comments · Fixed by #8328
Closed

Email notification for demoted/removed user #3264

brainwane opened this issue Mar 15, 2018 · 8 comments · Fixed by #8328
Labels
feature request help needed We'd love volunteers to advise on or help fix/implement this.
Milestone
Post Legacy Shutd...

Comments

@brainwane
Copy link
Contributor

Followup to #1000: we should email the user (and possibly all the other owners/maintainers of a project as well) when they were an owner/maintainer of a project but have just been demoted or removed from the project.

if a user is removed from a [maintainer or owner] role, then that user should also be emailed (e.g., to prevent an attacker who has compromised one owner, from silently removing other owners of a package, prior to uploading a new malicious package, thereby circumventing #997).

-- @edmorley
@brainwane brainwane added help needed We'd love volunteers to advise on or help fix/implement this. feature request labels Mar 15, 2018
@brainwane brainwane added this to the 6. Post Legacy Shutdown milestone Mar 15, 2018
Mariatta added a commit to Mariatta/warehouse that referenced this issue Mar 18, 2018
@Mariatta
Email notification when user's role is changed/removed.
746fd84 
- Send email to the user whose role has been changed/removed.
- Send email to other owners of the project.
- The person who made the change will not receive the email.

Closes pypi#3264
@Mariatta Mariatta mentioned this issue Mar 18, 2018
Closed
waseem18 pushed a commit to waseem18/warehouse that referenced this issue Apr 29, 2018
@Mariatta @waseem18
Email notification when user's role is changed/removed.
33f1f98 
- Send email to the user whose role has been changed/removed.
- Send email to other owners of the project.
- The person who made the change will not receive the email.

Closes pypi#3264

Make linters happy

Improve the tests and coverage

Update email text.

Notify all owners for any role changes.

Update email subjects.
Update tests.

Since we're always sending email to all owners now when role changed,
email recipients shouldn't ever be empty.

The order matters?

Reordering list ordering issue

Minor changes

Resolved a lint issue
@waseem18 waseem18 mentioned this issue Apr 29, 2018
Closed
waseem18 pushed a commit to waseem18/warehouse that referenced this issue May 5, 2018
@Mariatta @waseem18
Email notification when user's role is changed/removed.
114b27f 
- Send email to the user whose role has been changed/removed.
- Send email to other owners of the project.
- The person who made the change will not receive the email.

Closes pypi#3264

Make linters happy

Improve the tests and coverage

Update email text.

Notify all owners for any role changes.

Update email subjects.
Update tests.

Since we're always sending email to all owners now when role changed,
email recipients shouldn't ever be empty.

The order matters?

Reordering list ordering issue

Minor changes

Resolved a lint issue

Resolved code reviews

Used {%- -%} to strip additional new lines
waseem18 pushed a commit to waseem18/warehouse that referenced this issue May 5, 2018
@Mariatta @waseem18
Email notification when user's role is changed/removed.
0baa4d0 
- Send email to the user whose role has been changed/removed.
- Send email to other owners of the project.
- The person who made the change will not receive the email.

Closes pypi#3264

Make linters happy

Improve the tests and coverage

Update email text.

Notify all owners for any role changes.

Update email subjects.
Update tests.

Since we're always sending email to all owners now when role changed,
email recipients shouldn't ever be empty.

The order matters?

Reordering list ordering issue

Minor changes

Resolved a lint issue

Resolved code reviews

Used {%- -%} to strip additional new lines

Resolved lint issue
waseem18 pushed a commit to waseem18/warehouse that referenced this issue May 8, 2018
@Mariatta @waseem18
Email notification when user's role is changed/removed.
57da335 
- Send email to the user whose role has been changed/removed.
- Send email to other owners of the project.
- The person who made the change will not receive the email.

Closes pypi#3264

Make linters happy

Improve the tests and coverage

Update email text.

Notify all owners for any role changes.

Update email subjects.
Update tests.

Since we're always sending email to all owners now when role changed,
email recipients shouldn't ever be empty.

The order matters?

Reordering list ordering issue

Minor changes

Resolved a lint issue

Resolved code reviews

Used {%- -%} to strip additional new lines

Resolved lint issue
waseem18 pushed a commit to waseem18/warehouse that referenced this issue May 26, 2018
@Mariatta @waseem18
Email notification when user's role is changed/removed.
6bcef52 
- Send email to the user whose role has been changed/removed.
- Send email to other owners of the project.
- The person who made the change will not receive the email.

Closes pypi#3264

Make linters happy

Improve the tests and coverage

Update email text.

Notify all owners for any role changes.

Update email subjects.
Update tests.

Since we're always sending email to all owners now when role changed,
email recipients shouldn't ever be empty.

The order matters?

Reordering list ordering issue

Minor changes

Resolved a lint issue

Resolved code reviews

Used {%- -%} to strip additional new lines

Resolved lint issue
@brainwane brainwane added the blocked Issues we can't or shouldn't get to yet label May 16, 2019
@brainwane
Copy link
Contributor Author

To do this the right way, we should wait till we have #5863 implemented, so we can draw on the event logging and use it to trigger this notification.

@brainwane brainwane removed the blocked Issues we can't or shouldn't get to yet label Aug 15, 2019
@rascalking
Copy link
Contributor

I finally got the code in #3853 rebased on master, and am looking at driving the notifications from the event logging. I don't see any triggers or signal plumbing in #6339. When you say the event logging should trigger the notification, are you talking about calling these notifications directly from Project.record_event and User.record_event? Or are you looking for us to use the db signals to watch for UserEvent objects being created?

@brainwane
Copy link
Contributor Author

I defer to @di and @ewdurbin and @yeraydiazdiaz on this.

@brainwane
Copy link
Contributor Author

@di do you have some feedback on @rascalking's question?

@di
Copy link
Member

di commented Jun 25, 2020

I think either could work. I think it would be probably more straightforward to just do this in *.record_event, and probably more thorough to hook into ProjectEvent/UserEvent object creation.

I think what would be even more powerful is if every Event was published to our task queue, and for every individual type of event, there was a way to resolve asynchronous actions that should happen as a result of that event, like sending an email. That's a bit more than what we're asking for here right now though.

@di
Copy link
Member

di commented Jul 15, 2020

@rascalking Any updates here? Are you still working on this?

@patelneel55 patelneel55 mentioned this issue Jul 27, 2020
Merged
@rascalking
Copy link
Contributor

Sorry, I don't think I'm going to be able to finish this.

@di di closed this as completed in #8328 Aug 4, 2020
@brainwane
Copy link
Contributor Author

Thank you, @patelneel55!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request help needed We'd love volunteers to advise on or help fix/implement this.
Projects
None yet
Milestone
Post Legacy Shutdown priorities
3 participants
@di @rascalking @brainwane