Skip to content

feat: Validate Git tokens before consuming them #5167

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

Merged
merged 2 commits into from
Nov 29, 2022
Merged

feat: Validate Git tokens before consuming them #5167

merged 2 commits into from
Nov 29, 2022

Conversation

kylecarbs
Copy link
Member

This works the exact same way that the Git credential manager does. It ensures the user token is valid before returning it to the client.

It's been manually tested on GitHub, GitLab, and BitBucket.

@kylecarbs kylecarbs requested a review from a team as a code owner November 24, 2022 10:24
@kylecarbs kylecarbs self-assigned this Nov 24, 2022
@kylecarbs kylecarbs requested review from code-asher and removed request for a team November 24, 2022 10:24
@kylecarbs
Copy link
Member Author

@ericpaulsen when you get a shot, we should make some docs that explain how to set Git authentication up for providers like BitBucket, GitHub Enterprise, and GitLab self-hosted. This will involve setting the following values:

# GitHub Enterprise
CODER_GITAUTH_0_VALIDATE_URL=https://<hostname>/api/v3/user
CODER_GITAUTH_0_AUTH_URL=https://<hostname>/login/oauth/authorize
CODER_GITAUTH_0_TOKEN_URL=https://<hostname>/login/oauth/access_token

We should do this for the other providers and manually test that it works as well when adding docs. The validate URL is added in this PR and is required.

This works the exact same way that the Git credential manager does. It ensures the user token is valid before returning it to the client.

It's been manually tested on GitHub, GitLab, and BitBucket.
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Some minor stuff, looks good in general. 👍🏻

Copy link
Member

@deansheather deansheather left a comment

Choose a reason for hiding this comment

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

LGTM other than Mathias' comments

@kylecarbs kylecarbs merged commit 8b73844 into main Nov 29, 2022
@kylecarbs kylecarbs deleted the gitvalid branch November 29, 2022 18:08
@github-actions github-actions bot locked and limited conversation to collaborators Nov 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants