Design doc: use GitHub Apps for more granular permissions

[humitos]

Design document to communicate how to support GitHub Application for new users
while keeping OAuth Application for old users.

The main benefit is that GitHub Application allows us more granular permissions
than OAuth Application. With GitHub Application most of the permission we
request are Read-only and that's a win for everybody!

Rendered version: https://docs--8445.org.readthedocs.build/en/8445/development/design/github-application.html

[ericholscher]

This is a great overview. It definitely feels like we have some more thinking to do, but this is a great first step.

[ericholscher]

Is this for SSO, or something else? Might be useful to escalate in this case, if it lets us not request Org permissions up front.

[humitos]

This may be outdated and it's not required 😅 I already solved this problem in "Remote* models re-sync" without requiring this permission.

However, I had an idea in mind with this permission, but I didn't document it here. If we request Organization - Members: Read-only we can subscribe to related events (Member, Membership, Team and Team add) to immediately sync permissions when they are changed in GitHub.

Currently, we are re-syncing permissions at signup and on each login. So, this permission is not strictly required, but it could be better UX, tho.

We tried to implement the re-sync based on a webhook already with our current code at #7336

[ericholscher]

These seem like really important questions. The interaction with allauth in particular worries me. I really don't want to have to hack a lot of stuff, so keeping auth & repo permissions separated might be the best solution. Could even make the GH App approach optional for more security-minded people?

[humitos]

so keeping auth & repo permissions separated might be the best solution

I'm afraid that this is not possible. At the "Login" page, when the user clicks on "Login with GitHub" we need to redirect them to the URL to authorize the OAuth Application or the GitHub Application --so we have to make that decision before showing the button. I don't really know how to solve this problem yet. This will require a little more thinking and research.

Could even make the GH App approach optional for more security-minded people?

This question makes me think that we may need to show 2 different buttons: OAuth App and GH App. I know that would be terrible UX and will confuse a lot of users, tho.

I'd say that we should keep OAuth working as long as we have old users using this integration. However, new users should not have the option to use OAuth App and they must use GH App for all the good reasons we are migrating to it.

At some point, when no users using OAuth left, we should be able to delete it completely.

The most important thing to solve here is "How do we know that the person on the Login page is old or new to show them the appropriate button?"

[agjohnson]

Agreed having two buttons would be bad UX, we'd confuse users a lot here.

Is there any way we can try both, starting with one and if that attempt fails, try the other automatically? Or do either allow us to query to see if it is installed for the user?

To start, we could attempt authentication with the oauth app, and back down to the GH app -- swapping once we feel more users are using the GH app.

This could leave a single button in tact but for some users there would be an ugly wait to log in as we try to auth twice

new users should not have the option to use OAuth App and they must use GH App

👍

[humitos]

Is there any way we can try both, starting with one and if that attempt fails, try the other automatically? Or do either allow us to query to see if it is installed for the user?

No, I don't think we can do this due to how GitHub login works. After clicking in "Sign in" we redirect the unknown user (yet) to a GitHub URL where we lose control. That URL has to be the OAuth App or GH App URL (same URL with different parameters).

Example (current OAuth app): https://github.com/login/oauth/authorize?client_id=0dca71bac3403aba87c1&redirect_uri=https%3A%2F%2Freadthedocs.com%2Faccounts%2Fgithub%2Flogin%2Fcallback%2F&scope=read%3Aorg+repo+user%3Aemail+admin%3Arepo_hook&response_type=code&state=111111111

There, the user will be presented the page to authorize our OAuth application --even if they have already installed our GH app. We cannot do any kind of check at this point because we don't have control and also because we don't know who is the user.

For GH app the same URL is used. However, the parameters (in particular, client_id) are differents. See https://docs.github.com/en/developers/apps/building-github-apps/identifying-and-authorizing-users-for-github-apps#web-application-flow

Note that we could check the Request user authorization (OAuth) during installation in our GH application and make this flow happen during GH app installation; however, we will get only an access token for the organization where the GH app is installed, instead of for each of its members. So, that doesn't solve our problem either 😞

[humitos]

Honestly, I'm not finding a good way to solve this problem. I re-read the migration guide that GitHub provides and it's not super useful in this case.

I was thinking of a different approach:

1. implement all the GH application code
2. do not expose GH application to users yet, but make all the URLs to work
3. QA internally and make sure that everything works as expected
4. send emails + add a banner (with "call to action") telling organization owners to install our GH application (https://docs.github.com/en/developers/apps/getting-started-with-apps/migrating-oauth-apps-to-github-apps#direct-users-to-install-your-github-app-on-repositories)
5. send another email + add a banner communicating the EOL of the OAuth and telling them that starting on X date they must have GH App installed to use Read the Docs

As always, dropping support for something at a specific date is not good; but I'm not clearly seeing what else we can do besides forcing users to migrate. This may be doable in commercial, but probably won't in community.

Anyways, I'm open to getting some feedback and ideas on this because I ran out of ideas 😞

[agjohnson]

You might have the most knowledge and best opinions here. I'm not super familiar with the implementation at a technical level so can only take some wild shots in the dark.

The thing that I find awkward about that approach is that the user will get this notification to use the GHA method, allow the application on their user/organization, but then they need both the GHA and the oauth app installed until we do the cut over to the GHA.

It feels like we want users to have a different login entirely if they want to use the GHA implementation. Does it make sense to discuss using the beta instance for GHA in addition to the new templates?

[agjohnson]

Does it make sense to discuss using the beta instance for GHA in addition to the new templates?

This was my suggestion that I was trying to bring up in our roadmap call today. Would this approach be worth exploring?

The unknown here that I haven't thought through is what would happen when we're ready to cut everyone over to the new template instances.

cc @humitos

[humitos]

@agjohnson If I understand correctly, there won't be any difference in doing this for the beta instance than for the production one. At some point, all users will need to migrate to the new GH Application (installing it on their GH Organization / GH Project) to be able to use Read the Docs.

Another thing I just realize that may work:

• force all users to re-signing/re-signup with our new GH application (only one "Sign up/Login with Github" button)
• that would associate a new SocialAccount (with a custom provider='github-app') to the username
• at this point, they don't have to install our GH app on the organization/repositories
• if we don't delete the old GH SocialAccount (provider='github') for this user, RemoteRepositoryRelation(s) won't be deleted, so users should keep access via SSO
• we should freeze any change on permissions or "Import Project" until the user Installs our new GH application on their Organization / Project. Otherwise, it will be super confusing.
• once they install our GH application, we can delete the old SocialAccount with all old RemoteResitoryRelation(s) for this user and re-sync them with the new GH application
• at this point, the user should be on the new GH app and giving us only the permissions strictly needed for RTD to work

This may need more thinking on the UX, but I think with this we could keep "old permissions and webhooks" working until they migrate to the new GH app. However, the migration should be all or nothing because a mixture of both for the same user would be hard to communicate at a UI/UX level.

I'm also thinking that QAing this will probably be a nightmare 😄

Note that for the case where the user don't have permission to install the GH app on the organization/repository, we should force the re-signing/re-signup when we detect the GH application was installed in the organization they belong to. Otherwise, we will have some users accessing to old data and other users from the same RTD organization accessing to new data.

Anyways, this is still super complex.

[agjohnson]

At some point, all users will need to migrate to the new GH Application

Yeah, if we made the beta instance a path to using the new GH App, there are two problems:

• We have to switch everyone over at some point. We also will switch everyone over to the new templates at some point, so perhaps the UX overall could be fairly good here. That is, users switching to the new templates would go to a different page to log in, and on first log in they'd be asked to authorize the GH App -- like it's a new application entirely. I don't hate this option.
• However this brings up a new hiccup: the user can't easily switch back to using the current templates once they set up their account to use the new GitHub App instead.

That second point might make this not a great option.

Another thing I just realize that may work

We might need to diagram that out 😆 I think I understand most of that though, and it seems like maybe a workaround that would get us past the UX issues of multiple Github Allauth providers.

I'd need to think more on the UX around organization specially, this might be worth some discussion.

[agjohnson]

One of the issues that I was left thinking through here, and I think @humitos echoed it, would be if we did upgrade the connection for one of the users in an organization, do we have to upgrade all of the connections for users in the same organization?

[ericholscher]

It says the app needs to be installed on at least 1 organization. That could be it.

[agjohnson]

I don't have any strong input here, this all matches our conversations on GHA, as I understand. What are the next steps? Do you need to do more research on the remaining questions in this document?

[humitos]

What are the next steps? Do you need to do more research on the remaining questions in this document?

I put those questions mainly to not forget them and check if we can answer them before the implementation phase, but I don't think we will answer all of them. I think some of them will be tested/answered at that phase (in particular, those related to Django AllAuth).

On the other hand, this question (https://github.com/readthedocs/readthedocs.org/pull/8445/files#r700103315) is important because it's not technical, but requires more UX thinking and I'm not sure how to solve that problem. So, input there would be a good place to collaborate.

[agjohnson]

What is the first thing that we will work on to either start the implementation or to make design decisions around the implementation? I'm having trouble visualizing how this is going to be split up into digestible chunks of work, but the unanswered questions seem like the first place to start experimenting.

[benjaoming]

@humitos What a fantastic proposal! I love it - been thinking about GitHub Application support, too. Do you know if there is a migration path from the old OAuth App to GitHub App? Or are we indeed forced to either keep the old OAuth (like you suggested) or force all users to re-authorize?

[humitos]

@benjaoming

Do you know if there is a migration path from the old OAuth App to GitHub App? Or are we indeed forced to either keep the old OAuth (like you suggested) or force all users to re-authorize?

Yeah, this whole thing was a pretty complex topic. We talked about this in many opportunities and different places.

(there should be issues linked from/to this PR if you want to research a little more and read the whole history)

I wrote this 1.5 years ago, but I remember that GitHub didn't provide a clear migration path and basically was: "Change this code, and re-authorize all your users" 🤷🏼 which was a no-go for us.

Then we decided to keep both applications (OAuth and GHApp) at the same time, while we perform the migration; but that implies "knowing who the user is before log in" since the backend needs to use one app or the other. Finally, we thought that it could be a good idea to have a two-step login to know this upfront...

That's a small summary; but it's not nearly close to all we discussed 😄

[benjaoming]

Suggested change

	==================
	GitHub Application
	==================
	=======================
	GitHub Apps integration
	=======================

[benjaoming]

I wanted to quickly add a user benefit here. But I see that the design doc has a structure where it's a bit hard for me to quickly add this exact part. So I'll just leave it here for later:

With the current OAuth integration, it's difficult for projects with multiple maintainers to know who's access is used for GitHub status callbacks and webhook creation. In a recent support case, it meant that access to setup a webhook was denied for a user that expected it to work since an owner of the GitHub repository was also an owner (maintainer) of the Read the Docs project. Clearly, this situation is fixed with GitHub Apps.

Secondary note: I try to use "GitHub Apps" everywhere since that's the most accurate name. Searching out existing information was a bit tricky, so I renamed a couple of items.

[ericholscher]

This is pretty out of date, so we should update our thinking here and decide on a path to move forward with some kind of GH App. The current questions are here: #11780 (comment)

[agjohnson]

Work is continuing on #11780, I think we can close this one.