From 8b73844f6953c5d606cf791d0002f65e8e90f8d8 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 29 Nov 2022 19:08:27 +0100 Subject: [PATCH] feat: Validate Git tokens before consuming them (#5167) * feat: Validate Git tokens before consuming them 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. * Fix requested changes --- cli/deployment/config_test.go | 2 + cli/gitaskpass.go | 2 +- coderd/gitauth/config.go | 8 ++++ coderd/gitauth/oauth.go | 9 ++++- coderd/workspaceagents.go | 56 ++++++++++++++++++++++++++ coderd/workspaceagents_test.go | 72 ++++++++++++++++++++++++++++++++++ codersdk/deploymentconfig.go | 1 + site/src/api/typesGenerated.ts | 1 + 8 files changed, 149 insertions(+), 2 deletions(-) diff --git a/cli/deployment/config_test.go b/cli/deployment/config_test.go index 0d50455023..ca1ad0eeab 100644 --- a/cli/deployment/config_test.go +++ b/cli/deployment/config_test.go @@ -179,6 +179,7 @@ func TestConfig(t *testing.T) { "CODER_GITAUTH_0_CLIENT_SECRET": "secret", "CODER_GITAUTH_0_AUTH_URL": "https://auth.com", "CODER_GITAUTH_0_TOKEN_URL": "https://token.com", + "CODER_GITAUTH_0_VALIDATE_URL": "https://validate.com", "CODER_GITAUTH_0_REGEX": "github.com", "CODER_GITAUTH_0_SCOPES": "read write", "CODER_GITAUTH_0_NO_REFRESH": "true", @@ -200,6 +201,7 @@ func TestConfig(t *testing.T) { ClientSecret: "secret", AuthURL: "https://auth.com", TokenURL: "https://token.com", + ValidateURL: "https://validate.com", Regex: "github.com", Scopes: []string{"read", "write"}, NoRefresh: true, diff --git a/cli/gitaskpass.go b/cli/gitaskpass.go index 20740be7ae..4d98b0e8da 100644 --- a/cli/gitaskpass.go +++ b/cli/gitaskpass.go @@ -62,7 +62,7 @@ func gitAskpass() *cobra.Command { if err != nil { continue } - cmd.Printf("\nYou've been authenticated with Git!\n") + cmd.Printf("You've been authenticated with Git!\n") break } } diff --git a/coderd/gitauth/config.go b/coderd/gitauth/config.go index c6c058e90b..77abea9879 100644 --- a/coderd/gitauth/config.go +++ b/coderd/gitauth/config.go @@ -28,6 +28,10 @@ type Config struct { // Some organizations have security policies that require // re-authentication for every token. NoRefresh bool + // ValidateURL ensures an access token is valid before + // returning it to the user. If omitted, tokens will + // not be validated before being returned. + ValidateURL string } // ConvertConfig converts the YAML configuration entry to the @@ -101,6 +105,9 @@ func ConvertConfig(entries []codersdk.GitAuthConfig, accessURL *url.URL) ([]*Con if entry.Scopes != nil && len(entry.Scopes) > 0 { oauth2Config.Scopes = entry.Scopes } + if entry.ValidateURL == "" { + entry.ValidateURL = validateURL[typ] + } var oauthConfig httpmw.OAuth2Config = oauth2Config // Azure DevOps uses JWT token authentication! @@ -114,6 +121,7 @@ func ConvertConfig(entries []codersdk.GitAuthConfig, accessURL *url.URL) ([]*Con Regex: regex, Type: typ, NoRefresh: entry.NoRefresh, + ValidateURL: validateURL[typ], }) } return configs, nil diff --git a/coderd/gitauth/oauth.go b/coderd/gitauth/oauth.go index f1c63515f3..055a8c4d16 100644 --- a/coderd/gitauth/oauth.go +++ b/coderd/gitauth/oauth.go @@ -29,10 +29,17 @@ var endpoint = map[codersdk.GitProvider]oauth2.Endpoint{ codersdk.GitProviderGitHub: github.Endpoint, } +// validateURL contains defaults for each provider. +var validateURL = map[codersdk.GitProvider]string{ + codersdk.GitProviderGitHub: "https://api.github.com/user", + codersdk.GitProviderGitLab: "https://gitlab.com/oauth/token/info", + codersdk.GitProviderBitBucket: "https://api.bitbucket.org/2.0/user", +} + // scope contains defaults for each Git provider. var scope = map[codersdk.GitProvider][]string{ codersdk.GitProviderAzureDevops: {"vso.code_write"}, - codersdk.GitProviderBitBucket: {"repository:write"}, + codersdk.GitProviderBitBucket: {"account", "repository:write"}, codersdk.GitProviderGitLab: {"write_repository"}, codersdk.GitProviderGitHub: {"repo"}, } diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index bf3732ee52..8170e82ee6 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "net" "net/http" "net/netip" @@ -1159,6 +1160,19 @@ func (api *API) workspaceAgentsGitAuth(rw http.ResponseWriter, r *http.Request) if gitAuthLink.OAuthExpiry.Before(database.Now()) { continue } + if gitAuthConfig.ValidateURL != "" { + valid, err := validateGitToken(ctx, gitAuthConfig.ValidateURL, gitAuthLink.OAuthAccessToken) + if err != nil { + api.Logger.Warn(ctx, "failed to validate git auth token", + slog.F("workspace_owner_id", workspace.OwnerID.String()), + slog.F("validate_url", gitAuthConfig.ValidateURL), + slog.Error(err), + ) + } + if !valid { + continue + } + } httpapi.Write(ctx, rw, http.StatusOK, formatGitAuthAccessToken(gitAuthConfig.Type, gitAuthLink.OAuthAccessToken)) return } @@ -1214,6 +1228,24 @@ func (api *API) workspaceAgentsGitAuth(rw http.ResponseWriter, r *http.Request) return } + if gitAuthConfig.ValidateURL != "" { + valid, err := validateGitToken(ctx, gitAuthConfig.ValidateURL, token.AccessToken) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to validate Git authentication token.", + Detail: err.Error(), + }) + return + } + if !valid { + // The token is no longer valid! + httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspaceAgentGitAuthResponse{ + URL: redirectURL.String(), + }) + return + } + } + if token.AccessToken != gitAuthLink.OAuthAccessToken { // Update it err = api.Database.UpdateGitAuthLink(ctx, database.UpdateGitAuthLinkParams{ @@ -1235,6 +1267,30 @@ func (api *API) workspaceAgentsGitAuth(rw http.ResponseWriter, r *http.Request) httpapi.Write(ctx, rw, http.StatusOK, formatGitAuthAccessToken(gitAuthConfig.Type, token.AccessToken)) } +// validateGitToken ensures the git token provided is valid +// against the provided URL. +func validateGitToken(ctx context.Context, validateURL, token string) (bool, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, validateURL, nil) + if err != nil { + return false, err + } + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) + res, err := http.DefaultClient.Do(req) + if err != nil { + return false, err + } + defer res.Body.Close() + if res.StatusCode == http.StatusUnauthorized { + // The token is no longer valid! + return false, nil + } + if res.StatusCode != http.StatusOK { + data, _ := io.ReadAll(res.Body) + return false, xerrors.Errorf("git token validation failed: status %d: body: %s", res.StatusCode, data) + } + return true, nil +} + // Provider types have different username/password formats. func formatGitAuthAccessToken(typ codersdk.GitProvider, token string) codersdk.WorkspaceAgentGitAuthResponse { var resp codersdk.WorkspaceAgentGitAuthResponse diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go index 13bf080a5f..15927730bd 100644 --- a/coderd/workspaceagents_test.go +++ b/coderd/workspaceagents_test.go @@ -7,6 +7,7 @@ import ( "fmt" "net" "net/http" + "net/http/httptest" "regexp" "runtime" "strconv" @@ -934,6 +935,77 @@ func TestWorkspaceAgentsGitAuth(t *testing.T) { resp = gitAuthCallback(t, "github", client) require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) }) + t.Run("ValidateURL", func(t *testing.T) { + t.Parallel() + ctx, cancelFunc := testutil.Context(t) + defer cancelFunc() + + srv := httptest.NewServer(nil) + defer srv.Close() + client := coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + GitAuthConfigs: []*gitauth.Config{{ + ValidateURL: srv.URL, + OAuth2Config: &oauth2Config{}, + ID: "github", + Regex: regexp.MustCompile(`github\.com`), + Type: codersdk.GitProviderGitHub, + }}, + }) + user := coderdtest.CreateFirstUser(t, client) + authToken := uuid.NewString() + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: echo.ProvisionComplete, + ProvisionApply: []*proto.Provision_Response{{ + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{ + Resources: []*proto.Resource{{ + Name: "example", + Type: "aws_instance", + Agents: []*proto.Agent{{ + Id: uuid.NewString(), + Auth: &proto.Agent_Token{ + Token: authToken, + }, + }}, + }}, + }, + }, + }}, + }) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + agentClient := codersdk.New(client.URL) + agentClient.SetSessionToken(authToken) + + resp := gitAuthCallback(t, "github", client) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + + // If the validation URL says unauthorized, the callback + // URL to re-authenticate should be returned. + srv.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + }) + res, err := agentClient.WorkspaceAgentGitAuth(ctx, "github.com/asd/asd", false) + require.NoError(t, err) + require.NotEmpty(t, res.URL) + + // If the validation URL gives a non-OK status code, this + // should be treated as an internal server error. + srv.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + w.Write([]byte("Something went wrong!")) + }) + _, err = agentClient.WorkspaceAgentGitAuth(ctx, "github.com/asd/asd", false) + var apiError *codersdk.Error + require.ErrorAs(t, err, &apiError) + require.Equal(t, http.StatusInternalServerError, apiError.StatusCode()) + require.Equal(t, "git token validation failed: status 403: body: Something went wrong!", apiError.Detail) + }) t.Run("ExpiredNoRefresh", func(t *testing.T) { t.Parallel() diff --git a/codersdk/deploymentconfig.go b/codersdk/deploymentconfig.go index ffe7ed4e57..74e0b58b91 100644 --- a/codersdk/deploymentconfig.go +++ b/codersdk/deploymentconfig.go @@ -126,6 +126,7 @@ type GitAuthConfig struct { ClientSecret string `json:"-" yaml:"client_secret"` AuthURL string `json:"auth_url"` TokenURL string `json:"token_url"` + ValidateURL string `json:"validate_url"` Regex string `json:"regex"` NoRefresh bool `json:"no_refresh"` Scopes []string `json:"scopes"` diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 4d54790b2c..df75f2080b 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -355,6 +355,7 @@ export interface GitAuthConfig { readonly client_id: string readonly auth_url: string readonly token_url: string + readonly validate_url: string readonly regex: string readonly no_refresh: boolean readonly scopes: string[]