From 236e94a75d4fe61ddd3929b36098fe77d12208f9 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Thu, 24 Nov 2022 10:24:26 +0000 Subject: [PATCH 1/2] 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. --- cli/deployment/config_test.go | 2 + cli/gitaskpass.go | 2 +- coderd/gitauth/config.go | 8 ++++ coderd/gitauth/oauth.go | 9 ++++- coderd/workspaceagents.go | 49 ++++++++++++++++++++++++ coderd/workspaceagents_test.go | 69 ++++++++++++++++++++++++++++++++++ codersdk/deploymentconfig.go | 1 + site/src/api/typesGenerated.ts | 1 + 8 files changed, 139 insertions(+), 2 deletions(-) diff --git a/cli/deployment/config_test.go b/cli/deployment/config_test.go index 515851439ee84..6b4f26759fafe 100644 --- a/cli/deployment/config_test.go +++ b/cli/deployment/config_test.go @@ -165,6 +165,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", @@ -186,6 +187,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 20740be7ae3bf..4d98b0e8da535 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 c6c058e90bc5f..77abea9879195 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 f1c63515f32a8..055a8c4d166c0 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 21b6d83b599c3..311b07c9a059f 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "net" "net/http" "net/netip" @@ -1158,6 +1159,12 @@ func (api *API) workspaceAgentsGitAuth(rw http.ResponseWriter, r *http.Request) if gitAuthLink.OAuthExpiry.Before(database.Now()) { continue } + if gitAuthConfig.ValidateURL != "" { + valid, _ := validateGitToken(ctx, gitAuthConfig.ValidateURL, gitAuthLink.OAuthAccessToken) + if !valid { + continue + } + } httpapi.Write(ctx, rw, http.StatusOK, formatGitAuthAccessToken(gitAuthConfig.Type, gitAuthLink.OAuthAccessToken)) return } @@ -1213,6 +1220,24 @@ func (api *API) workspaceAgentsGitAuth(rw http.ResponseWriter, r *http.Request) return } + if gitAuthConfig.ValidateURL != "" { + valid, err := validateGitToken(r.Context(), gitAuthConfig.ValidateURL, token.AccessToken) + if err != nil { + httpapi.Write(r.Context(), 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{ @@ -1234,6 +1259,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("body: %s", 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 13bf080a5f307..ff519b5386ab6 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,74 @@ 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() + 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(context.Background(), "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(context.Background(), "github.com/asd/asd", false) + var apiError *codersdk.Error + require.ErrorAs(t, err, &apiError) + require.Equal(t, http.StatusInternalServerError, apiError.StatusCode()) + require.Equal(t, "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 806cd59b76cc1..588b4c30ed3ed 100644 --- a/codersdk/deploymentconfig.go +++ b/codersdk/deploymentconfig.go @@ -125,6 +125,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 2347d8f680369..166e8b6233aa4 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[] From b9a517ea0f0fad4d8de26a859caf03afdb09aeb8 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 29 Nov 2022 14:29:10 +0000 Subject: [PATCH 2/2] Fix requested changes --- coderd/workspaceagents.go | 15 +++++++++++---- coderd/workspaceagents_test.go | 9 ++++++--- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index 311b07c9a059f..e1551adc8ac84 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -1160,7 +1160,14 @@ func (api *API) workspaceAgentsGitAuth(rw http.ResponseWriter, r *http.Request) continue } if gitAuthConfig.ValidateURL != "" { - valid, _ := validateGitToken(ctx, gitAuthConfig.ValidateURL, gitAuthLink.OAuthAccessToken) + 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 } @@ -1221,9 +1228,9 @@ func (api *API) workspaceAgentsGitAuth(rw http.ResponseWriter, r *http.Request) } if gitAuthConfig.ValidateURL != "" { - valid, err := validateGitToken(r.Context(), gitAuthConfig.ValidateURL, token.AccessToken) + valid, err := validateGitToken(ctx, gitAuthConfig.ValidateURL, token.AccessToken) if err != nil { - httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{ + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Failed to validate Git authentication token.", Detail: err.Error(), }) @@ -1278,7 +1285,7 @@ func validateGitToken(ctx context.Context, validateURL, token string) (bool, err } if res.StatusCode != http.StatusOK { data, _ := io.ReadAll(res.Body) - return false, xerrors.Errorf("body: %s", data) + return false, xerrors.Errorf("git token validation failed: status %d: body: %s", res.StatusCode, data) } return true, nil } diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go index ff519b5386ab6..15927730bd915 100644 --- a/coderd/workspaceagents_test.go +++ b/coderd/workspaceagents_test.go @@ -937,6 +937,9 @@ func TestWorkspaceAgentsGitAuth(t *testing.T) { }) 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{ @@ -987,7 +990,7 @@ func TestWorkspaceAgentsGitAuth(t *testing.T) { srv.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusUnauthorized) }) - res, err := agentClient.WorkspaceAgentGitAuth(context.Background(), "github.com/asd/asd", false) + res, err := agentClient.WorkspaceAgentGitAuth(ctx, "github.com/asd/asd", false) require.NoError(t, err) require.NotEmpty(t, res.URL) @@ -997,11 +1000,11 @@ func TestWorkspaceAgentsGitAuth(t *testing.T) { w.WriteHeader(http.StatusForbidden) w.Write([]byte("Something went wrong!")) }) - _, err = agentClient.WorkspaceAgentGitAuth(context.Background(), "github.com/asd/asd", false) + _, 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, "Something went wrong!", apiError.Detail) + require.Equal(t, "git token validation failed: status 403: body: Something went wrong!", apiError.Detail) }) t.Run("ExpiredNoRefresh", func(t *testing.T) {