gitserver: Add oauth2 username to GitHub clone url by pjlast · Pull Request #45137 · sourcegraph/sourcegraph-public-snapshot

pjlast · 2022-12-05T08:20:23Z

Based on this Stackoverflow thread: https://stackoverflow.com/questions/42148841/github-clone-with-oauth-access-token/66156992#66156992 it seems that GitHub fine-grained Personal Access Tokens don't work without the oauth2 username. This PR adds the username when generating the GitHub clone URL.

This is how other code hosts work as well, for example the clone URL for GitLab uses the oauth2 username as well: https://sourcegraph.com/github.com/sourcegraph/sourcegraph@ce477a4ad027d3dfcf74d4246d7f4c4a0eec3876/-/blob/internal/repos/clone_url.go?L216&subtree=true

I also confirmed that, with this change, the classic PATs still work.

Test plan

Tested repository cloning with both classic PATs and fine-grained PATs after the oauth2 username was added.

sourcegraph-bot · 2022-12-05T08:21:57Z

Codenotify: Notifying subscribers in CODENOTIFY files for diff e14b9c8...70877e0.

Notify	File(s)
@indradhanush	internal/repos/clone_url.go internal/repos/clone_url_test.go
@ryanslade	internal/repos/clone_url.go internal/repos/clone_url_test.go
@sashaostrikov	internal/repos/clone_url.go internal/repos/clone_url_test.go
@sourcegraph/delivery	doc/admin/external_service/github.md

mrnugget · 2022-12-05T11:07:09Z

 		u.User = url.UserPassword("x-access-token", cfg.Token)
 	} else {
-		u.User = url.User(cfg.Token)
+		u.User = url.UserPassword("oauth2", cfg.Token)


What if someone uses the legacy token? Does that still work?

In the description of the PR:

I also confirmed that, with this change, the classic PATs still work.

:)

Sorry, that's on me :|

ryanslade · 2022-12-06T13:01:01Z

+
+> NOTE: Fine-grained personal access token suport is still experimental. Some functionality may not yet work.
+
+To sync repositories using fine-grained personal access tokens read-only access on the Content permission is required for repositories.


Suggested change

To sync repositories using fine-grained personal access tokens read-only access on the Content permission is required for repositories.

To sync repositories using fine-grained personal access tokens read-only access on the `Content` permission is required for repositories.

ryanslade · 2022-12-06T13:01:49Z

+
+To sync repositories using fine-grained personal access tokens read-only access on the Content permission is required for repositories.
+
+It should also be noted that fine-grained personal access tokens work a bit different than normal personal access tokens. For example, a classic personal access token can access all repositories of the owner, as well as repositories of organizations that the owner belongs to. However, for fine-grained personal access tokens, access is restricted to the owner of the token. If access to a GitHub organization's repositories is required, the GitHub organization has to be the owner of the fine-grained personal access token.


Suggested change

It should also be noted that fine-grained personal access tokens work a bit different than normal personal access tokens. For example, a classic personal access token can access all repositories of the owner, as well as repositories of organizations that the owner belongs to. However, for fine-grained personal access tokens, access is restricted to the owner of the token. If access to a GitHub organization's repositories is required, the GitHub organization has to be the owner of the fine-grained personal access token.

It should also be noted that fine-grained personal access tokens work a bit differently than normal personal access tokens. For example, a classic personal access token can access all repositories of the owner, as well as repositories of organizations that the owner belongs to. However, for fine-grained personal access tokens, access is restricted to the owner of the token. If access to a GitHub organization's repositories is required, the GitHub organization has to be the owner of the fine-grained personal access token.

Pretty sure it's differently :)

(EDIT: @ryanslade I fixed it)

It also looked wrong to me but spell-checker didn't kick in, so I assumed I made a mistake. It showed the red line under it only when I went back to edit it... weird.

mrnugget · 2022-12-06T13:05:32Z


+### Fine-grained personal access tokens
+
+> NOTE: Fine-grained personal access token suport is still experimental. Some functionality may not yet work.


We do have an experimental badge in use in docs. Use that please too.

mrnugget · 2022-12-06T13:05:57Z

+
+To sync repositories using fine-grained personal access tokens read-only access on the Content permission is required for repositories.
+
+It should also be noted that fine-grained personal access tokens work a bit different than normal personal access tokens. For example, a classic personal access token can access all repositories of the owner, as well as repositories of organizations that the owner belongs to. However, for fine-grained personal access tokens, access is restricted to the owner of the token. If access to a GitHub organization's repositories is required, the GitHub organization has to be the owner of the fine-grained personal access token.


Pretty sure it's differently :)

(EDIT: @ryanslade I fixed it)

kopancek · 2022-12-06T13:34:25Z

 - An issue causing certain kinds of queries to behave inconsistently in Code Insights. [#44917](https://github.com/sourcegraph/sourcegraph/pull/44917)
 - When the setting `batchChanges.enforceForks` is enabled, Batch Changes will now prefix the name of the fork repo it creates with the original repo's namespace name in order to prevent repo name collisions. [#43681](https://github.com/sourcegraph/sourcegraph/pull/43681), [#44458](https://github.com/sourcegraph/sourcegraph/pull/44458), [#44548](https://github.com/sourcegraph/sourcegraph/pull/44548), [#44924](https://github.com/sourcegraph/sourcegraph/pull/44924)
 - Code Insights: fixed an issue where certain queries matching sequential whitespace characters would overcount. [#44969](https://github.com/sourcegraph/sourcegraph/pull/44969)
+- GitHub fine-grained Personal Access Tokens can now clone repositories correctly. [#45137](https://github.com/sourcegraph/sourcegraph/pull/45137)


This is confusing. We say in changelog that they can clone repositories and we say in the docs that they are not yet supported. Which one is it then? :)

I mean, they can clone repositories but they aren't supported 😄

Maybe I'll add "but are not yet officially supported"

Add oauth2 username to github clone url

2dff9c4

cla-bot Bot added the cla-signed label Dec 5, 2022

github-actions Bot added the team/iam label Dec 5, 2022

Changelog entry

201697c

pjlast requested review from a team December 5, 2022 08:21

Add oauth2 username to test

05b31dd

ryanslade approved these changes Dec 5, 2022

View reviewed changes

Add doc entry and go fmt correctly

1f4397c

pjlast mentioned this pull request Dec 5, 2022

Confirm support of new GitHub Fine-Grain access tokens and update documentation #45073

Closed

2 tasks

Add explanation of required permissions

a3fe562

mrnugget reviewed Dec 5, 2022

View reviewed changes

mrnugget approved these changes Dec 5, 2022

View reviewed changes

Update notes on fine-grained personal access tokens

bbc56d6

ryanslade reviewed Dec 6, 2022

View reviewed changes

mrnugget reviewed Dec 6, 2022

View reviewed changes

Change docs to make clear that fine-grained tokens aren't supported

d8159d1

kopancek approved these changes Dec 6, 2022

View reviewed changes

kopancek reviewed Dec 6, 2022

View reviewed changes

Clarify changelog entry

70877e0

pjlast enabled auto-merge (squash) December 6, 2022 13:59

pjlast merged commit daeedc4 into main Dec 6, 2022

pjlast deleted the pjlast/45136-add-oauth2-username-to-github-clone-url branch December 6, 2022 14:03


		> NOTE: Fine-grained personal access token suport is still experimental. Some functionality may not yet work.

		To sync repositories using fine-grained personal access tokens read-only access on the Content permission is required for repositories.


		To sync repositories using fine-grained personal access tokens read-only access on the Content permission is required for repositories.

		It should also be noted that fine-grained personal access tokens work a bit different than normal personal access tokens. For example, a classic personal access token can access all repositories of the owner, as well as repositories of organizations that the owner belongs to. However, for fine-grained personal access tokens, access is restricted to the owner of the token. If access to a GitHub organization's repositories is required, the GitHub organization has to be the owner of the fine-grained personal access token.


		### Fine-grained personal access tokens

		> NOTE: Fine-grained personal access token suport is still experimental. Some functionality may not yet work.

Conversation

pjlast commented Dec 5, 2022

Test plan

Uh oh!

sourcegraph-bot commented Dec 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanslade Dec 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrnugget Dec 6, 2022 • edited by ryanslade Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrnugget Dec 6, 2022 • edited by ryanslade Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sourcegraph-bot commented Dec 5, 2022 •

edited

Loading

ryanslade Dec 6, 2022 •

edited

Loading

mrnugget Dec 6, 2022 •

edited by ryanslade

Loading

mrnugget Dec 6, 2022 •

edited by ryanslade

Loading