Re-sync GitHub (RemoteRepository and RemoteOrganization) on webhook

[humitos]

Add member event when creating the Read the Docs' webhook on each repository
imported under Read the Docs.

This webhook will communicate back to us when a collaborator was
added/removed/changed from that repository in particular. This allow us to
trigger the re-sync method for the GitHub service on that user and
create/delete/change RemoteRepository objects for that user.

Examples,

• user is removed from having access to a repository -> remove RemoteRepository
• user is added access to a repository -> create RemoteRepository
• user is removed admin access but keep read access -> .admin=False

Related to #7185

Documentation reference: https://docs.github.com/en/developers/webhooks-and-events/webhook-events-and-payloads#member

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[humitos]

This PR is useful. I haven't had the time to update it with the newer code, but we will eventually need it.

[humitos]

This webhook will communicate back to us when a collaborator was added/removed/changed from that repository in particular

Take into account that this only works for repository collaborators. There is another webhook called membership that works for users added/removed from an organization's team: https://docs.github.com/en/developers/webhooks-and-events/webhook-events-and-payloads#membership . However, it's only available for organization webhooks and we are not making usage of them.

---

Even if we merge this PR, all the Read the Docs projects have to re-sync their webhooks (:disappointed: ) to make usage of the new member event. New projects will use it automatically.

[agjohnson]

I'd agree that celery task is the most correct here. We've had issues lately with blocking web processes and should avoid that pattern going forward.

[humitos]

We are moving away from OAuth Application to GitHub Application: #8445

If anything, we will implement the re-sync via the events we can subscribe there.

[ericholscher]

I'd love to get this working via Celery. I think it might help us keep SSO in sync.

[humitos]

I updated this PR to use a Celery task to sync but...

Note this PR does not cover exactly what we want, and I'm not sure if is worth merging it and maintaining it:

• only works for new imported projects
• old projects would need to re-sync their webhooks to subscribe to the new member event
• only works over GH collaborators on repositories --it does not work over permissions granted via teams or organizations
• when a GH webhook is received it will re-sync all the VCS connected accounts for the user

Taking into account all of these points, and considering that we added a scheduled Celery task to re-sync in #8601 it may not make sense to merge it.

[ericholscher]

@humitos I don't fully understand the issues with it, but I do think we want some kind of way to do real-time updates on permissions. Having permissions be out of sync for users for a whole day feels like bad UX, so I feel like we need to at least attempt to do this.

[ericholscher]

I think this makes sense, and doesn't seem like a lot of code to maintain?

[humitos]

@ericholscher

I don't fully understand the issues with it, but I do think we want some kind of way to do real-time updates on permissions.

The implementation of this webhook may make you feel that it solves the problem, where it does not. That's what I'm trying to say. It will keep permissions over repositories updated, but it won't for permissions granted via organizations and teams --which is the main problem we want to solve here.

That said, it may help in some cases and add more confusion in others.

The bad UX because we update permissions once a day, will still be present even with this webhook.

[ericholscher]

@humitos Good points. I think I'm OK leaving this out for now if we want, and just doing nightly updates. That seems cleanest and easiest to explain for now, until we can find a better solution. I agree this does add a good bit of confusion, and "it might work" situations, which is probably not great.

[ericholscher]

Closing this again... Seems we were correct the first time 🙃