diff --git a/cmd/server_test.go b/cmd/server_test.go index 8722350672..91e687d326 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -395,6 +395,20 @@ func TestExecute_ValidateVCSConfig(t *testing.T) { }, true, }, + { + "just github app set", + map[string]interface{}{ + GHAppIDFlag: "1", + }, + true, + }, + { + "just github app key set", + map[string]interface{}{ + GHAppKeyFlag: "key.pem", + }, + true, + }, { "just gitlab user set", map[string]interface{}{ @@ -448,6 +462,14 @@ func TestExecute_ValidateVCSConfig(t *testing.T) { }, false, }, + { + "github app and key set and should be successful", + map[string]interface{}{ + GHAppIDFlag: "1", + GHAppKeyFlag: "key.pem", + }, + false, + }, { "gitlab user and gitlab token set and should be successful", map[string]interface{}{ @@ -546,6 +568,19 @@ func TestExecute_GithubUser(t *testing.T) { Equals(t, "user", passedConfig.GithubUser) } +func TestExecute_GithubApp(t *testing.T) { + t.Log("Should remove the @ from the github username if it's passed.") + c := setup(map[string]interface{}{ + GHAppKeyFlag: "key.pem", + GHAppIDFlag: "1", + RepoWhitelistFlag: "*", + }) + err := c.Execute() + Ok(t, err) + + Equals(t, int64(1), passedConfig.GithubAppID) +} + func TestExecute_GitlabUser(t *testing.T) { t.Log("Should remove the @ from the gitlab username if it's passed.") c := setup(map[string]interface{}{ diff --git a/server/events/git_cred_writer.go b/server/events/git_cred_writer.go index bef7a900a5..0feae8c19e 100644 --- a/server/events/git_cred_writer.go +++ b/server/events/git_cred_writer.go @@ -15,7 +15,7 @@ import ( // WriteGitCreds generates a .git-credentials file containing the username and token // used for authenticating with git over HTTPS // It will create the file in home/.git-credentials -func WriteGitCreds(gitUser string, gitToken string, gitHostname string, home string, logger *logging.SimpleLogger) error { +func WriteGitCreds(gitUser string, gitToken string, gitHostname string, home string, logger *logging.SimpleLogger, ignoreExisting bool) error { const credsFilename = ".git-credentials" credsFile := filepath.Join(home, credsFilename) credsFileContents := `https://%s:%s@%s` @@ -24,7 +24,7 @@ func WriteGitCreds(gitUser string, gitToken string, gitHostname string, home str // If there is already a .git-credentials file and its contents aren't exactly // what we would have written to it, then we error out because we don't // want to overwrite anything - if _, err := os.Stat(credsFile); err == nil { + if _, err := os.Stat(credsFile); err == nil && !ignoreExisting { currContents, err := ioutil.ReadFile(credsFile) // nolint: gosec if err != nil { return errors.Wrapf(err, "trying to read %s to ensure we're not overwriting it", credsFile) diff --git a/server/events/git_cred_writer_test.go b/server/events/git_cred_writer_test.go index db1c121f74..7aa8b30dd8 100644 --- a/server/events/git_cred_writer_test.go +++ b/server/events/git_cred_writer_test.go @@ -19,7 +19,7 @@ func TestWriteGitCreds_WriteFile(t *testing.T) { tmp, cleanup := TempDir(t) defer cleanup() - err := events.WriteGitCreds("user", "token", "hostname", tmp, logger) + err := events.WriteGitCreds("user", "token", "hostname", tmp, logger, false) Ok(t, err) expContents := `https://user:token@hostname` @@ -39,7 +39,7 @@ func TestWriteGitCreds_WillNotOverwrite(t *testing.T) { err := ioutil.WriteFile(credsFile, []byte("contents"), 0600) Ok(t, err) - actErr := events.WriteGitCreds("user", "token", "hostname", tmp, logger) + actErr := events.WriteGitCreds("user", "token", "hostname", tmp, logger, false) expErr := fmt.Sprintf("can't write git-credentials to %s because that file has contents that would be overwritten", tmp+"/.git-credentials") ErrEquals(t, expErr, actErr) } @@ -56,7 +56,7 @@ func TestWriteGitCreds_NoErrIfContentsSame(t *testing.T) { err := ioutil.WriteFile(credsFile, []byte(contents), 0600) Ok(t, err) - err = events.WriteGitCreds("user", "token", "hostname", tmp, logger) + err = events.WriteGitCreds("user", "token", "hostname", tmp, logger, false) Ok(t, err) } @@ -71,7 +71,7 @@ func TestWriteGitCreds_ErrIfCannotRead(t *testing.T) { Ok(t, err) expErr := fmt.Sprintf("trying to read %s to ensure we're not overwriting it: open %s: permission denied", credsFile, credsFile) - actErr := events.WriteGitCreds("user", "token", "hostname", tmp, logger) + actErr := events.WriteGitCreds("user", "token", "hostname", tmp, logger, false) ErrEquals(t, expErr, actErr) } @@ -79,7 +79,7 @@ func TestWriteGitCreds_ErrIfCannotRead(t *testing.T) { func TestWriteGitCreds_ErrIfCannotWrite(t *testing.T) { credsFile := "/this/dir/does/not/exist/.git-credentials" expErr := fmt.Sprintf("writing generated .git-credentials file with user, token and hostname to %s: open %s: no such file or directory", credsFile, credsFile) - actErr := events.WriteGitCreds("user", "token", "hostname", "/this/dir/does/not/exist", logger) + actErr := events.WriteGitCreds("user", "token", "hostname", "/this/dir/does/not/exist", logger, false) ErrEquals(t, expErr, actErr) } @@ -88,7 +88,7 @@ func TestWriteGitCreds_ConfigureGitCredentialHelper(t *testing.T) { tmp, cleanup := TempDir(t) defer cleanup() - err := events.WriteGitCreds("user", "token", "hostname", tmp, logger) + err := events.WriteGitCreds("user", "token", "hostname", tmp, logger, false) Ok(t, err) expOutput := `store` @@ -102,7 +102,7 @@ func TestWriteGitCreds_ConfigureGitUrlOverride(t *testing.T) { tmp, cleanup := TempDir(t) defer cleanup() - err := events.WriteGitCreds("user", "token", "hostname", tmp, logger) + err := events.WriteGitCreds("user", "token", "hostname", tmp, logger, false) Ok(t, err) expOutput := `ssh://git@hostname` diff --git a/server/events/vcs/fixtures/fixtures.go b/server/events/vcs/fixtures/fixtures.go index 78e37fd069..53aac029fa 100644 --- a/server/events/vcs/fixtures/fixtures.go +++ b/server/events/vcs/fixtures/fixtures.go @@ -292,49 +292,45 @@ var githubConversionJSON = `{ "pem": "%s" }` -var githubAppInfoJSON = `{ - "id": 1, - "slug": "atlantis", - "node_id": "MDExOkludGVncmF0aW9uMQ==", - "owner": { - "login": "github", - "id": 1, - "node_id": "MDEyOk9yZ2FuaXphdGlvbjE=", - "url": "https://api.github.com/orgs/github", - "repos_url": "https://api.github.com/orgs/github/repos", - "events_url": "https://api.github.com/orgs/github/events", - "hooks_url": "https://api.github.com/orgs/github/hooks", - "issues_url": "https://api.github.com/orgs/github/issues", - "members_url": "https://api.github.com/orgs/github/members{/member}", - "public_members_url": "https://api.github.com/orgs/github/public_members{/member}", - "avatar_url": "https://github.com/images/error/octocat_happy.gif", - "description": "A great organization" - }, - "name": "Atlantis", - "description": "atlantis", - "external_url": "https://atlantis.example.com", - "html_url": "https://github.com/apps/atlantis", - "created_at": "2017-07-08T16:18:44-04:00", - "updated_at": "2017-07-08T16:18:44-04:00", - "permissions":{ - "checks": "write", - "contents": "write", - "issues": "write", - "pull_requests": "write", - "repository_hooks": "write", - "statuses": "write" - }, - "events": [ - "check_run", - "create", - "delete", - "pull_request", - "push", - "issues" - ], - "installations_count": 1 -}` +var githubAppInstallationJSON = `[ + { + "id": 1, + "account": { + "login": "github", + "id": 1, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjE=", + "url": "https://api.github.com/orgs/github", + "repos_url": "https://api.github.com/orgs/github/repos", + "events_url": "https://api.github.com/orgs/github/events", + "hooks_url": "https://api.github.com/orgs/github/hooks", + "issues_url": "https://api.github.com/orgs/github/issues", + "members_url": "https://api.github.com/orgs/github/members{/member}", + "public_members_url": "https://api.github.com/orgs/github/public_members{/member}", + "avatar_url": "https://github.com/images/error/octocat_happy.gif", + "description": "A great organization" + }, + "access_tokens_url": "https://api.github.com/installations/1/access_tokens", + "repositories_url": "https://api.github.com/installation/repositories", + "html_url": "https://github.com/organizations/github/settings/installations/1", + "app_id": 1, + "target_id": 1, + "target_type": "Organization", + "permissions": { + "metadata": "read", + "contents": "read", + "issues": "write", + "single_file": "write" + }, + "events": [ + "push", + "pull_request" + ], + "single_file_name": "config.yml", + "repository_selection": "selected" + } +]` +// nolint: gosec var githubAppTokenJSON = `{ "token": "v1.1f699f1069f60xx%d", "expires_at": "2050-01-01T00:00:00Z", @@ -488,27 +484,26 @@ func GithubAppTestServer(t *testing.T) (string, error) { testServer := httptest.NewTLSServer( http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.RequestURI { - // https://developer.github.com/v3/apps/#get-the-authenticated-github-app - case "/api/v3/app": + case "/api/v3/app-manifests/good-code/conversions": + encodedKey := strings.Join(strings.Split(GithubPrivateKey, "\n"), "\\n") + appInfo := fmt.Sprintf(githubConversionJSON, encodedKey) + w.Write([]byte(appInfo)) // nolint: errcheck + // https://developer.github.com/v3/apps/#list-installations + case "/api/v3/app/installations": token := strings.Replace(r.Header.Get("Authorization"), "Bearer ", "", 1) if err := validateGithubToken(token); err != nil { w.WriteHeader(403) - w.Write([]byte("Invalid token")) + w.Write([]byte("Invalid token")) // nolint: errcheck return } - w.Write([]byte(githubAppInfoJSON)) // nolint: errcheck + w.Write([]byte(githubAppInstallationJSON)) // nolint: errcheck return - case "/api/v3/app-manifests/good-code/conversions": - encodedKey := strings.Join(strings.Split(GithubPrivateKey, "\n"), "\\n") - appInfo := fmt.Sprintf(githubConversionJSON, encodedKey) - w.Write([]byte(appInfo)) // nolint: errcheck case "/api/v3/app/installations/1/access_tokens": - case "/api/v3//app/installations/1/access_tokens": token := strings.Replace(r.Header.Get("Authorization"), "Bearer ", "", 1) if err := validateGithubToken(token); err != nil { w.WriteHeader(403) - w.Write([]byte("Invalid token")) + w.Write([]byte("Invalid token")) // nolint: errcheck return } diff --git a/server/events/vcs/github_credentials.go b/server/events/vcs/github_credentials.go index e2391c2f48..a945717c0a 100644 --- a/server/events/vcs/github_credentials.go +++ b/server/events/vcs/github_credentials.go @@ -2,13 +2,14 @@ package vcs import ( "context" - "log" + "fmt" "net/http" "net/url" "strings" "github.com/bradleyfalzon/ghinstallation" "github.com/google/go-github/v30/github" + "github.com/pkg/errors" ) // GithubCredentials handles creating http.Clients that authenticate @@ -69,7 +70,7 @@ type GithubAppCredentials struct { Hostname string apiURL *url.URL installationID int64 - token *github.InstallationToken + tr *ghinstallation.Transport } // Client returns a github app installation client @@ -88,38 +89,12 @@ func (c *GithubAppCredentials) GetUser() string { // GetToken returns a fresh installation token func (c *GithubAppCredentials) GetToken() (string, error) { - if c.token != nil { - log.Println(c.token.GetExpiresAt()) - return c.token.GetToken(), nil - } - - transport, err := c.Client() - if err != nil { - return "", err - } - - apiURL := c.getAPIURL() - var client *github.Client - - if apiURL.Hostname() == "github.com" { - client = github.NewClient(transport) - } else { - client, _ = github.NewEnterpriseClient(apiURL.String(), apiURL.String(), transport) - } - - installationID, err := c.getInstallationID() + tr, err := c.transport() if err != nil { - return "", err + return "", errors.Wrap(err, "transport failed") } - token, _, err := client.Apps.CreateInstallationToken(context.Background(), installationID, &github.InstallationTokenOptions{}) - - if err != nil { - return "", err - } - - c.token = token - return token.GetToken(), nil + return tr.Token(context.Background()) } func (c *GithubAppCredentials) getInstallationID() (int64, error) { @@ -139,16 +114,25 @@ func (c *GithubAppCredentials) getInstallationID() (int64, error) { client := github.NewClient(&http.Client{Transport: t}) client.BaseURL = c.getAPIURL() ctx := context.Background() - app, _, err := client.Apps.Get(ctx, "") + + installations, _, err := client.Apps.ListInstallations(ctx, nil) if err != nil { return 0, err } - c.installationID = app.GetID() + if len(installations) != 1 { + return 0, fmt.Errorf("wrong number of installations, expected 1, found %d", len(installations)) + } + + c.installationID = installations[0].GetID() return c.installationID, nil } func (c *GithubAppCredentials) transport() (*ghinstallation.Transport, error) { + if c.tr != nil { + return c.tr, nil + } + installationID, err := c.getInstallationID() if err != nil { return nil, err @@ -157,7 +141,9 @@ func (c *GithubAppCredentials) transport() (*ghinstallation.Transport, error) { tr := http.DefaultTransport itr, err := ghinstallation.NewKeyFromFile(tr, c.AppID, installationID, c.KeyPath) if err == nil { - itr.BaseURL = c.getAPIURL().String() + apiURL := c.getAPIURL() + itr.BaseURL = strings.TrimSuffix(apiURL.String(), "/") + c.tr = itr } return itr, err } diff --git a/server/events/vcs/github_credentials_test.go b/server/events/vcs/github_credentials_test.go index a629173b77..867880e28a 100644 --- a/server/events/vcs/github_credentials_test.go +++ b/server/events/vcs/github_credentials_test.go @@ -41,6 +41,6 @@ func TestGithubClient_AppAuthentication(t *testing.T) { Ok(t, err) if token != newToken { - t.Errorf("app token was not cached") + t.Errorf("app token was not cached: %q != %q", token, newToken) } } diff --git a/server/events/working_dir.go b/server/events/working_dir.go index 7adbdd8ac0..91130e07e8 100644 --- a/server/events/working_dir.go +++ b/server/events/working_dir.go @@ -227,7 +227,8 @@ func (w *FileWorkspace) forceClone(log *logging.SimpleLogger, "GIT_COMMITTER_NAME=atlantis", }...) - cmdStr := w.sanitizeGitCredentials(strings.Join(cmd.Args, " "), p.BaseRepo, headRepo) + // cmdStr := w.sanitizeGitCredentials(strings.Join(cmd.Args, " "), p.BaseRepo, headRepo) + cmdStr := strings.Join(cmd.Args, " ") output, err := cmd.CombinedOutput() sanitizedOutput := w.sanitizeGitCredentials(string(output), p.BaseRepo, headRepo) if err != nil { @@ -308,9 +309,16 @@ func (g *GithubAppWorkingDir) Clone(log *logging.SimpleLogger, baseRepo models.R } // https://developer.github.com/apps/building-github-apps/authenticating-with-github-apps/#http-based-git-access-by-an-installation - if err := WriteGitCreds("x-access-token", token, g.GithubHostname, home, log); err != nil { + if err := WriteGitCreds("x-access-token", token, g.GithubHostname, home, log, true); err != nil { return "", false, err } + authURL := fmt.Sprintf("://x-access-token:%s", token) + if baseRepo.CloneURL != "" { + baseRepo.CloneURL = strings.Replace(baseRepo.CloneURL, "://:", authURL, 1) + } + if headRepo.CloneURL != "" { + headRepo.CloneURL = strings.Replace(headRepo.CloneURL, "://:", authURL, 1) + } return g.WorkingDir.Clone(log, baseRepo, headRepo, p, workspace) } diff --git a/server/events/working_dir_test.go b/server/events/working_dir_test.go index 462fa0f102..f3e7ee48c3 100644 --- a/server/events/working_dir_test.go +++ b/server/events/working_dir_test.go @@ -52,7 +52,7 @@ func TestClone_NoneExisting(t *testing.T) { Equals(t, expCommit, actCommit) } -// Test that if we don't have any existing files, we check out the repo. +// Test that if we don't have any existing files, we check out the repo with a github app. func TestClone_GithubAppNoneExisting(t *testing.T) { // Initialize the git repo. repoDir, cleanup := initRepo(t) @@ -84,6 +84,7 @@ func TestClone_GithubAppNoneExisting(t *testing.T) { AppID: 1, Hostname: testServer, }, + GithubHostname: testServer, } cloneDir, _, err := gwd.Clone(nil, models.Repo{}, models.Repo{}, models.PullRequest{ diff --git a/server/server.go b/server/server.go index cb13fce8a3..34154a83a2 100644 --- a/server/server.go +++ b/server/server.go @@ -187,12 +187,12 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { return nil, errors.Wrap(err, "getting home dir to write ~/.git-credentials file") } if userConfig.GithubUser != "" { - if err := events.WriteGitCreds(userConfig.GithubUser, userConfig.GithubToken, userConfig.GithubHostname, home, logger); err != nil { + if err := events.WriteGitCreds(userConfig.GithubUser, userConfig.GithubToken, userConfig.GithubHostname, home, logger, false); err != nil { return nil, err } } if userConfig.GitlabUser != "" { - if err := events.WriteGitCreds(userConfig.GitlabUser, userConfig.GitlabToken, userConfig.GitlabHostname, home, logger); err != nil { + if err := events.WriteGitCreds(userConfig.GitlabUser, userConfig.GitlabToken, userConfig.GitlabHostname, home, logger, false); err != nil { return nil, err } } @@ -203,12 +203,12 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { if bitbucketBaseURL == "https://api.bitbucket.org" { bitbucketBaseURL = "bitbucket.org" } - if err := events.WriteGitCreds(userConfig.BitbucketUser, userConfig.BitbucketToken, bitbucketBaseURL, home, logger); err != nil { + if err := events.WriteGitCreds(userConfig.BitbucketUser, userConfig.BitbucketToken, bitbucketBaseURL, home, logger, false); err != nil { return nil, err } } if userConfig.AzureDevopsUser != "" { - if err := events.WriteGitCreds(userConfig.AzureDevopsUser, userConfig.AzureDevopsToken, "https://dev.azure.com/", home, logger); err != nil { + if err := events.WriteGitCreds(userConfig.AzureDevopsUser, userConfig.AzureDevopsToken, "https://dev.azure.com/", home, logger, false); err != nil { return nil, err } } @@ -263,7 +263,10 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { CheckoutMerge: userConfig.CheckoutStrategy == "merge", } // provide fresh tokens before clone from the GitHub Apps integration, proxy workingDir - if githubAppEnabled && userConfig.WriteGitCreds { + if githubAppEnabled { + if !userConfig.WriteGitCreds { + logger.Warn("Github App requires --write-git-creds, overriding") + } workingDir = &events.GithubAppWorkingDir{ WorkingDir: workingDir, Credentials: githubCredentials,