From 875abf1362a54b44e1fc815fb1f70334859e0ea2 Mon Sep 17 00:00:00 2001 From: Luke Kysow <1034429+lkysow@users.noreply.github.com> Date: Fri, 19 Jun 2020 11:41:44 -0700 Subject: [PATCH] Refactoring to github app code - rename gh-app-key to gh-app-key-file for clarity - change git credentials writer to append a line if there is an existing .git-credentials file and in the case of the github app to replace the old github app line - removed automatically setting --write-git-creds to true when using a github app and instead requiring this is set specifically --- cmd/server.go | 6 +- cmd/server_test.go | 12 +-- runatlantis.io/docs/access-credentials.md | 11 ++- runatlantis.io/docs/server-configuration.md | 6 +- server/events/git_cred_writer.go | 89 +++++++++++++++----- server/events/git_cred_writer_test.go | 50 ++++++++--- server/events/github_app_working_dir.go | 47 +++++++++++ server/events/github_app_working_dir_test.go | 57 +++++++++++++ server/events/vcs/github_client.go | 16 ++-- server/events/vcs/github_credentials.go | 26 +++--- server/events/working_dir.go | 37 -------- server/events/working_dir_test.go | 47 ----------- server/server.go | 2 +- server/user_config.go | 2 +- server/web_templates.go | 2 +- 15 files changed, 254 insertions(+), 156 deletions(-) create mode 100644 server/events/github_app_working_dir.go create mode 100644 server/events/github_app_working_dir_test.go diff --git a/cmd/server.go b/cmd/server.go index 32aaf7b96..7e7d1b22d 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -58,7 +58,7 @@ const ( GHTokenFlag = "gh-token" GHUserFlag = "gh-user" GHAppIDFlag = "gh-app-id" - GHAppKeyFlag = "gh-app-key" + GHAppKeyFileFlag = "gh-app-key-file" GHOrganizationFlag = "gh-org" GHWebhookSecretFlag = "gh-webhook-secret" // nolint: gosec GitlabHostnameFlag = "gitlab-hostname" @@ -168,7 +168,7 @@ var stringFlags = map[string]stringFlag{ GHTokenFlag: { description: "GitHub token of API user. Can also be specified via the ATLANTIS_GH_TOKEN environment variable.", }, - GHAppKeyFlag: { + GHAppKeyFileFlag: { description: "A path to a file containing the GitHub App's private key", defaultValue: "", }, @@ -565,7 +565,7 @@ func (s *ServerCmd) validate(userConfig server.UserConfig) error { // 3. bitbucket user and token set // 4. azuredevops user and token set // 5. any combination of the above - vcsErr := fmt.Errorf("--%s/--%s or --%s/--%s or --%s/--%s or --%s/--%s or --%s/--%s must be set", GHUserFlag, GHTokenFlag, GHAppIDFlag, GHAppKeyFlag, GitlabUserFlag, GitlabTokenFlag, BitbucketUserFlag, BitbucketTokenFlag, ADUserFlag, ADTokenFlag) + vcsErr := fmt.Errorf("--%s/--%s or --%s/--%s or --%s/--%s or --%s/--%s or --%s/--%s must be set", GHUserFlag, GHTokenFlag, GHAppIDFlag, GHAppKeyFileFlag, GitlabUserFlag, GitlabTokenFlag, BitbucketUserFlag, BitbucketTokenFlag, ADUserFlag, ADTokenFlag) if ((userConfig.GithubUser == "") != (userConfig.GithubToken == "")) || ((userConfig.GithubAppID == 0) != (userConfig.GithubAppKey == "")) || ((userConfig.GitlabUser == "") != (userConfig.GitlabToken == "")) || ((userConfig.BitbucketUser == "") != (userConfig.BitbucketToken == "")) || ((userConfig.AzureDevopsUser == "") != (userConfig.AzureDevopsToken == "")) { return vcsErr } diff --git a/cmd/server_test.go b/cmd/server_test.go index 1d674fc27..9d99855d4 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -70,7 +70,7 @@ var testFlags = map[string]interface{}{ GHTokenFlag: "token", GHUserFlag: "user", GHAppIDFlag: int64(0), - GHAppKeyFlag: "", + GHAppKeyFileFlag: "", GHOrganizationFlag: "", GHWebhookSecretFlag: "secret", GitlabHostnameFlag: "gitlab-hostname", @@ -350,7 +350,7 @@ func TestExecute_ValidateSSLConfig(t *testing.T) { } func TestExecute_ValidateVCSConfig(t *testing.T) { - expErr := "--gh-user/--gh-token or --gh-app-id/--gh-app-key or --gitlab-user/--gitlab-token or --bitbucket-user/--bitbucket-token or --azuredevops-user/--azuredevops-token must be set" + expErr := "--gh-user/--gh-token or --gh-app-id/--gh-app-key-file or --gitlab-user/--gitlab-token or --bitbucket-user/--bitbucket-token or --azuredevops-user/--azuredevops-token must be set" cases := []struct { description string flags map[string]interface{} @@ -406,7 +406,7 @@ func TestExecute_ValidateVCSConfig(t *testing.T) { { "just github app key set", map[string]interface{}{ - GHAppKeyFlag: "key.pem", + GHAppKeyFileFlag: "key.pem", }, true, }, @@ -466,8 +466,8 @@ func TestExecute_ValidateVCSConfig(t *testing.T) { { "github app and key set and should be successful", map[string]interface{}{ - GHAppIDFlag: "1", - GHAppKeyFlag: "key.pem", + GHAppIDFlag: "1", + GHAppKeyFileFlag: "key.pem", }, false, }, @@ -572,7 +572,7 @@ func TestExecute_GithubUser(t *testing.T) { 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", + GHAppKeyFileFlag: "key.pem", GHAppIDFlag: "1", RepoWhitelistFlag: "*", }) diff --git a/runatlantis.io/docs/access-credentials.md b/runatlantis.io/docs/access-credentials.md index ba518c831..43e2efd0f 100644 --- a/runatlantis.io/docs/access-credentials.md +++ b/runatlantis.io/docs/access-credentials.md @@ -33,10 +33,13 @@ Your Atlantis user must also have "Write permissions" (for repos in an organizat ::: ### GitHub app -- Start Atlantis with fake github username and token (`--gh-user fake --gh-token fake`) -- Visit `$ATLANTIS_HOST/github-app/setup` and click on **Setup** to create the app on Github. You'll be redirected back to Atlantis -- A link to install your app, along its secrets, will be shown on the screen. Record your app's credentials and install your app for your user/org by following said link. -- Restart Atlantis +- Start Atlantis with fake github username and token (`atlantis server --gh-user fake --gh-token fake --repo-whitelist 'github.com/your-org/*' --atlantis-url https://$ATLANTIS_HOST`) +- Visit `https://$ATLANTIS_HOST/github-app/setup` and click on **Setup** to create the app on Github. You'll be redirected back to Atlantis +- A link to install your app, along with its secrets, will be shown on the screen. Record your app's credentials and install your app for your user/org by following said link. +- Create a file with the contents of the GitHub App Key, e.g. `atlantis-app-key.pem` +- Restart Atlantis with new flags: `atlantis server --gh-app-id --gh-app-key-file atlantis-app-key.pem --gh-webhook-secret --write-git-creds --repo-whitelist 'github.com/your-org/*' --atlantis-url https://$ATLANTIS_HOST`. + + NOTE: You can also create a config file instead of using flags. See [Server Configuration](/docs/server-configuration.html#config-file). ::: warning Only a single installation per Github App is supported at the moment. diff --git a/runatlantis.io/docs/server-configuration.md b/runatlantis.io/docs/server-configuration.md index b2afcb529..78ff63b4b 100644 --- a/runatlantis.io/docs/server-configuration.md +++ b/runatlantis.io/docs/server-configuration.md @@ -265,12 +265,12 @@ Values are chosen in this order: $(hostname)/github-app/exchange-code?code=some-code ``` - After which Atlantis will display your new app's credentials: your app's ID, its generated `--gh-webhook-secret` and the contents of the file for `--gh-app-key`. Update your Atlantis config accordingly, and restart the server. + After which Atlantis will display your new app's credentials: your app's ID, its generated `--gh-webhook-secret` and the contents of the file for `--gh-app-key-file`. Update your Atlantis config accordingly, and restart the server. ::: -- ### `--gh-app-key` +- ### `--gh-app-key-file` ```bash - atlantis server --gh-app-key="path/to/app-key.pem" + atlantis server --gh-app-key-file="path/to/app-key.pem" ``` Path to a GitHub App PEM encoded private key file. If set, GitHub authentication will be performed as [an installation](https://developer.github.com/v3/apps/installations/). diff --git a/server/events/git_cred_writer.go b/server/events/git_cred_writer.go index 0feae8c19..ec27ade0a 100644 --- a/server/events/git_cred_writer.go +++ b/server/events/git_cred_writer.go @@ -15,34 +15,44 @@ 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, ignoreExisting bool) error { +// If ghAccessToken is true we will look for a line starting with https://x-access-token and ending with gitHostname and replace it. +func WriteGitCreds(gitUser string, gitToken string, gitHostname string, home string, logger *logging.SimpleLogger, ghAccessToken bool) error { const credsFilename = ".git-credentials" credsFile := filepath.Join(home, credsFilename) - credsFileContents := `https://%s:%s@%s` - config := fmt.Sprintf(credsFileContents, gitUser, gitToken, gitHostname) + credsFileContentsPattern := `https://%s:%s@%s` + config := fmt.Sprintf(credsFileContentsPattern, gitUser, gitToken, gitHostname) - // 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 && !ignoreExisting { - currContents, err := ioutil.ReadFile(credsFile) // nolint: gosec + // If the file doesn't exist, write it. + if _, err := os.Stat(credsFile); err != nil { + if err := ioutil.WriteFile(credsFile, []byte(config), 0600); err != nil { + return errors.Wrapf(err, "writing generated %s file with user, token and hostname to %s", credsFilename, credsFile) + } + logger.Info("wrote git credentials to %s", credsFile) + } else { + hasLine, err := fileHasLine(config, credsFile) if err != nil { - return errors.Wrapf(err, "trying to read %s to ensure we're not overwriting it", credsFile) + return err } - if config != string(currContents) { - return fmt.Errorf("can't write git-credentials to %s because that file has contents that would be overwritten", credsFile) + if hasLine { + logger.Debug("git credentials file has expected contents, not modifying") + return nil } - // Otherwise we don't need to write the file because it already has - // what we need. - return nil - } - if err := ioutil.WriteFile(credsFile, []byte(config), 0600); err != nil { - return errors.Wrapf(err, "writing generated %s file with user, token and hostname to %s", credsFilename, credsFile) + if ghAccessToken { + // Need to replace the line. + if err := fileLineReplace(config, gitUser, gitHostname, credsFile); err != nil { + return errors.Wrap(err, "replacing git credentials line for github app") + } + logger.Info("updated git app credentials in %s", credsFile) + } else { + // Otherwise we need to append the line. + if err := fileAppend(config, credsFile); err != nil { + return err + } + logger.Info("wrote git credentials to %s", credsFile) + } } - logger.Info("wrote git credentials to %s", credsFile) - credentialCmd := exec.Command("git", "config", "--global", "credential.helper", "store") if out, err := credentialCmd.CombinedOutput(); err != nil { return errors.Wrapf(err, "There was an error running %s: %s", strings.Join(credentialCmd.Args, " "), string(out)) @@ -56,3 +66,44 @@ func WriteGitCreds(gitUser string, gitToken string, gitHostname string, home str logger.Info("successfully ran %s", strings.Join(urlCmd.Args, " ")) return nil } + +func fileHasLine(line string, filename string) (bool, error) { + currContents, err := ioutil.ReadFile(filename) // nolint: gosec + if err != nil { + return false, errors.Wrapf(err, "reading %s", filename) + } + for _, l := range strings.Split(string(currContents), "\n") { + if l == line { + return true, nil + } + } + return false, nil +} + +func fileAppend(line string, filename string) error { + currContents, err := ioutil.ReadFile(filename) // nolint: gosec + if err != nil { + return err + } + if len(currContents) > 0 && !strings.HasSuffix(string(currContents), "\n") { + line = "\n" + line + } + return ioutil.WriteFile(filename, []byte(string(currContents)+line), 0600) +} + +func fileLineReplace(line, user, host, filename string) error { + currContents, err := ioutil.ReadFile(filename) // nolint: gosec + if err != nil { + return err + } + prevLines := strings.Split(string(currContents), "\n") + var newLines []string + for _, l := range prevLines { + if strings.HasPrefix(l, "https://"+user) && strings.HasSuffix(l, host) { + newLines = append(newLines, line) + } else { + newLines = append(newLines, l) + } + } + return ioutil.WriteFile(filename, []byte(strings.Join(newLines, "\n")), 0600) +} diff --git a/server/events/git_cred_writer_test.go b/server/events/git_cred_writer_test.go index 7aa8b30dd..de025ee51 100644 --- a/server/events/git_cred_writer_test.go +++ b/server/events/git_cred_writer_test.go @@ -29,9 +29,9 @@ func TestWriteGitCreds_WriteFile(t *testing.T) { Equals(t, expContents, string(actContents)) } -// Test that if the file already exists and its contents will be modified if -// we write our config that we error out -func TestWriteGitCreds_WillNotOverwrite(t *testing.T) { +// Test that if the file already exists and it doesn't have the line we would +// have written, we write it. +func TestWriteGitCreds_Appends(t *testing.T) { tmp, cleanup := TempDir(t) defer cleanup() @@ -39,25 +39,49 @@ func TestWriteGitCreds_WillNotOverwrite(t *testing.T) { err := ioutil.WriteFile(credsFile, []byte("contents"), 0600) Ok(t, err) - 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) + err = events.WriteGitCreds("user", "token", "hostname", tmp, logger, false) + Ok(t, err) + + expContents := "contents\nhttps://user:token@hostname" + actContents, err := ioutil.ReadFile(filepath.Join(tmp, ".git-credentials")) + Ok(t, err) + Equals(t, expContents, string(actContents)) } -// Test that if the file already exists and its contents will NOT be modified if -// we write our config that we don't error. -func TestWriteGitCreds_NoErrIfContentsSame(t *testing.T) { +// Test that if the file already exists and it already has the line expected +// we do nothing. +func TestWriteGitCreds_NoModification(t *testing.T) { tmp, cleanup := TempDir(t) defer cleanup() credsFile := filepath.Join(tmp, ".git-credentials") - contents := `https://user:token@hostname` - + contents := "line1\nhttps://user:token@hostname\nline2" err := ioutil.WriteFile(credsFile, []byte(contents), 0600) Ok(t, err) err = events.WriteGitCreds("user", "token", "hostname", tmp, logger, false) Ok(t, err) + actContents, err := ioutil.ReadFile(filepath.Join(tmp, ".git-credentials")) + Ok(t, err) + Equals(t, contents, string(actContents)) +} + +// Test that the github app credentials get replaced. +func TestWriteGitCreds_ReplaceApp(t *testing.T) { + tmp, cleanup := TempDir(t) + defer cleanup() + + credsFile := filepath.Join(tmp, ".git-credentials") + contents := "line1\nhttps://x-access-token:v1.87dddddddddddddddd@github.com\nline2" + err := ioutil.WriteFile(credsFile, []byte(contents), 0600) + Ok(t, err) + + err = events.WriteGitCreds("x-access-token", "token", "github.com", tmp, logger, true) + Ok(t, err) + expContets := "line1\nhttps://x-access-token:token@github.com\nline2" + actContents, err := ioutil.ReadFile(filepath.Join(tmp, ".git-credentials")) + Ok(t, err) + Equals(t, expContets, string(actContents)) } // Test that if we can't read the existing file to see if the contents will be @@ -70,9 +94,9 @@ func TestWriteGitCreds_ErrIfCannotRead(t *testing.T) { err := ioutil.WriteFile(credsFile, []byte("can't see me!"), 0000) Ok(t, err) - expErr := fmt.Sprintf("trying to read %s to ensure we're not overwriting it: open %s: permission denied", credsFile, credsFile) + expErr := fmt.Sprintf("open %s: permission denied", credsFile) actErr := events.WriteGitCreds("user", "token", "hostname", tmp, logger, false) - ErrEquals(t, expErr, actErr) + ErrContains(t, expErr, actErr) } // Test that if we can't write, we error out. diff --git a/server/events/github_app_working_dir.go b/server/events/github_app_working_dir.go new file mode 100644 index 000000000..2d91fc529 --- /dev/null +++ b/server/events/github_app_working_dir.go @@ -0,0 +1,47 @@ +package events + +import ( + "fmt" + "strings" + + "github.com/mitchellh/go-homedir" + "github.com/pkg/errors" + "github.com/runatlantis/atlantis/server/events/models" + "github.com/runatlantis/atlantis/server/events/vcs" + "github.com/runatlantis/atlantis/server/logging" +) + +// GithubAppWorkingDir implements WorkingDir. +// It acts as a proxy to an instance of WorkingDir that refreshes the app's token +// before every clone, given Github App tokens expire quickly +type GithubAppWorkingDir struct { + WorkingDir + Credentials vcs.GithubCredentials + GithubHostname string +} + +// Clone writes a fresh token for Github App authentication +func (g *GithubAppWorkingDir) Clone(log *logging.SimpleLogger, baseRepo models.Repo, headRepo models.Repo, p models.PullRequest, workspace string) (string, bool, error) { + + log.Info("Refreshing git tokens for Github App") + + token, err := g.Credentials.GetToken() + if err != nil { + return "", false, errors.Wrap(err, "getting github token") + } + + home, err := homedir.Dir() + if err != nil { + return "", false, errors.Wrap(err, "getting home dir to write ~/.git-credentials file") + } + + // 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, true); err != nil { + return "", false, err + } + + authURL := fmt.Sprintf("://x-access-token:%s", token) + baseRepo.CloneURL = strings.Replace(baseRepo.CloneURL, "://:", authURL, 1) + headRepo.CloneURL = strings.Replace(headRepo.CloneURL, "://:", authURL, 1) + return g.WorkingDir.Clone(log, baseRepo, headRepo, p, workspace) +} diff --git a/server/events/github_app_working_dir_test.go b/server/events/github_app_working_dir_test.go new file mode 100644 index 000000000..b229d0200 --- /dev/null +++ b/server/events/github_app_working_dir_test.go @@ -0,0 +1,57 @@ +package events_test + +import ( + "fmt" + "testing" + + "github.com/runatlantis/atlantis/server/events" + "github.com/runatlantis/atlantis/server/events/models" + "github.com/runatlantis/atlantis/server/events/vcs" + "github.com/runatlantis/atlantis/server/events/vcs/fixtures" + . "github.com/runatlantis/atlantis/testing" +) + +// 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) + defer cleanup() + expCommit := runCmd(t, repoDir, "git", "rev-parse", "HEAD") + + dataDir, cleanup2 := TempDir(t) + defer cleanup2() + + wd := &events.FileWorkspace{ + DataDir: dataDir, + CheckoutMerge: false, + TestingOverrideHeadCloneURL: fmt.Sprintf("file://%s", repoDir), + } + + tmpDir, cleanup3 := DirStructure(t, map[string]interface{}{ + "key.pem": fixtures.GithubPrivateKey, + }) + defer cleanup3() + + defer disableSSLVerification()() + testServer, err := fixtures.GithubAppTestServer(t) + Ok(t, err) + + gwd := &events.GithubAppWorkingDir{ + WorkingDir: wd, + Credentials: &vcs.GithubAppCredentials{ + KeyPath: fmt.Sprintf("%v/key.pem", tmpDir), + AppID: 1, + Hostname: testServer, + }, + GithubHostname: testServer, + } + + cloneDir, _, err := gwd.Clone(nil, models.Repo{}, models.Repo{}, models.PullRequest{ + HeadBranch: "branch", + }, "default") + Ok(t, err) + + // Use rev-parse to verify at correct commit. + actCommit := runCmd(t, cloneDir, "git", "rev-parse", "HEAD") + Equals(t, expCommit, actCommit) +} diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index cd75328ad..9ca0a066a 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -41,25 +41,25 @@ type GithubClient struct { logger *logging.SimpleLogger } -// GithubAppTemporarySecrets hold app credentials obtained from github after creation +// GithubAppTemporarySecrets holds app credentials obtained from github after creation. type GithubAppTemporarySecrets struct { - // The app ID + // ID is the app id. ID int64 - // The app's PEM-encoded key - Key string + // Key is the app's PEM-encoded key. + Key string + // Name is the app name. Name string - // The generated webhook secret for this app + // WebhookSecret is the generated webhook secret for this app. WebhookSecret string - // A link to the app, like https://github.com/apps/octoapp + // URL is a link to the app, like https://github.com/apps/octoapp. URL string } // NewGithubClient returns a valid GitHub client. func NewGithubClient(hostname string, credentials GithubCredentials, logger *logging.SimpleLogger) (*GithubClient, error) { - transport, err := credentials.Client() if err != nil { - return nil, errors.Wrap(err, "Error initializing github authentication transport") + return nil, errors.Wrap(err, "error initializing github authentication transport") } var graphqlURL string diff --git a/server/events/vcs/github_credentials.go b/server/events/vcs/github_credentials.go index b941c6ed1..802097c60 100644 --- a/server/events/vcs/github_credentials.go +++ b/server/events/vcs/github_credentials.go @@ -12,39 +12,39 @@ import ( "github.com/pkg/errors" ) -// GithubCredentials handles creating http.Clients that authenticate +// GithubCredentials handles creating http.Clients that authenticate. type GithubCredentials interface { Client() (*http.Client, error) GetToken() (string, error) GetUser() string } -// GithubAnonymousCredentials expose no credentials +// GithubAnonymousCredentials expose no credentials. type GithubAnonymousCredentials struct{} -// Client returns a client with no credentials +// Client returns a client with no credentials. func (c *GithubAnonymousCredentials) Client() (*http.Client, error) { tr := http.DefaultTransport return &http.Client{Transport: tr}, nil } -// GetUser returns the username for these credentials +// GetUser returns the username for these credentials. func (c *GithubAnonymousCredentials) GetUser() string { return "anonymous" } -// GetToken returns an empty token +// GetToken returns an empty token. func (c *GithubAnonymousCredentials) GetToken() (string, error) { return "", nil } -// GithubUserCredentials implements GithubCredentials for the personal auth token flow +// GithubUserCredentials implements GithubCredentials for the personal auth token flow. type GithubUserCredentials struct { User string Token string } -// Client returns a client for basic auth user credentials +// Client returns a client for basic auth user credentials. func (c *GithubUserCredentials) Client() (*http.Client, error) { tr := &github.BasicAuthTransport{ Username: strings.TrimSpace(c.User), @@ -53,17 +53,17 @@ func (c *GithubUserCredentials) Client() (*http.Client, error) { return tr.Client(), nil } -// GetUser returns the username for these credentials +// GetUser returns the username for these credentials. func (c *GithubUserCredentials) GetUser() string { return c.User } -// GetToken returns the user token +// GetToken returns the user token. func (c *GithubUserCredentials) GetToken() (string, error) { return c.Token, nil } -// GithubAppCredentials implements GithubCredentials for github app installation token flow +// GithubAppCredentials implements GithubCredentials for github app installation token flow. type GithubAppCredentials struct { AppID int64 KeyPath string @@ -73,7 +73,7 @@ type GithubAppCredentials struct { tr *ghinstallation.Transport } -// Client returns a github app installation client +// Client returns a github app installation client. func (c *GithubAppCredentials) Client() (*http.Client, error) { itr, err := c.transport() if err != nil { @@ -82,12 +82,12 @@ func (c *GithubAppCredentials) Client() (*http.Client, error) { return &http.Client{Transport: itr}, nil } -// GetUser returns the username for these credentials +// GetUser returns the username for these credentials. func (c *GithubAppCredentials) GetUser() string { return "" } -// GetToken returns a fresh installation token +// GetToken returns a fresh installation token. func (c *GithubAppCredentials) GetToken() (string, error) { tr, err := c.transport() if err != nil { diff --git a/server/events/working_dir.go b/server/events/working_dir.go index 20158f147..5811055f0 100644 --- a/server/events/working_dir.go +++ b/server/events/working_dir.go @@ -21,10 +21,8 @@ import ( "strconv" "strings" - "github.com/mitchellh/go-homedir" "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/events/models" - "github.com/runatlantis/atlantis/server/events/vcs" "github.com/runatlantis/atlantis/server/logging" ) @@ -285,38 +283,3 @@ func (w *FileWorkspace) sanitizeGitCredentials(s string, base models.Repo, head baseReplaced := strings.Replace(s, base.CloneURL, base.SanitizedCloneURL, -1) return strings.Replace(baseReplaced, head.CloneURL, head.SanitizedCloneURL, -1) } - -// GithubAppWorkingDir implements WorkingDir. -// It acts as a proxy to an instance of WorkingDir that refreshes the app's token -// before every clone, given Github App tokens expire quickly -type GithubAppWorkingDir struct { - WorkingDir - Credentials vcs.GithubCredentials - GithubHostname string -} - -// Clone writes a fresh token for Github App authentication -func (g *GithubAppWorkingDir) Clone(log *logging.SimpleLogger, baseRepo models.Repo, headRepo models.Repo, p models.PullRequest, workspace string) (string, bool, error) { - - log.Info("Refreshing git tokens for Github App") - - token, err := g.Credentials.GetToken() - if err != nil { - return "", false, errors.Wrap(err, "getting github token") - } - - home, err := homedir.Dir() - if err != nil { - return "", false, errors.Wrap(err, "getting home dir to write ~/.git-credentials file") - } - - // 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, true); err != nil { - return "", false, err - } - - authURL := fmt.Sprintf("://x-access-token:%s", token) - baseRepo.CloneURL = strings.Replace(baseRepo.CloneURL, "://:", authURL, 1) - 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 f3e7ee48c..c4f49e837 100644 --- a/server/events/working_dir_test.go +++ b/server/events/working_dir_test.go @@ -10,8 +10,6 @@ import ( "github.com/runatlantis/atlantis/server/events" "github.com/runatlantis/atlantis/server/events/models" - "github.com/runatlantis/atlantis/server/events/vcs" - "github.com/runatlantis/atlantis/server/events/vcs/fixtures" . "github.com/runatlantis/atlantis/testing" ) @@ -52,51 +50,6 @@ 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 with a github app. -func TestClone_GithubAppNoneExisting(t *testing.T) { - // Initialize the git repo. - repoDir, cleanup := initRepo(t) - defer cleanup() - expCommit := runCmd(t, repoDir, "git", "rev-parse", "HEAD") - - dataDir, cleanup2 := TempDir(t) - defer cleanup2() - - wd := &events.FileWorkspace{ - DataDir: dataDir, - CheckoutMerge: false, - TestingOverrideHeadCloneURL: fmt.Sprintf("file://%s", repoDir), - } - - tmpDir, cleanup3 := DirStructure(t, map[string]interface{}{ - "key.pem": fixtures.GithubPrivateKey, - }) - defer cleanup3() - - defer disableSSLVerification()() - testServer, err := fixtures.GithubAppTestServer(t) - Ok(t, err) - - gwd := &events.GithubAppWorkingDir{ - WorkingDir: wd, - Credentials: &vcs.GithubAppCredentials{ - KeyPath: fmt.Sprintf("%v/key.pem", tmpDir), - AppID: 1, - Hostname: testServer, - }, - GithubHostname: testServer, - } - - cloneDir, _, err := gwd.Clone(nil, models.Repo{}, models.Repo{}, models.PullRequest{ - HeadBranch: "branch", - }, "default") - Ok(t, err) - - // Use rev-parse to verify at correct commit. - actCommit := runCmd(t, cloneDir, "git", "rev-parse", "HEAD") - Equals(t, expCommit, actCommit) -} - // Test that if we don't have any existing files, we check out the repo // successfully when we're using the merge method. func TestClone_CheckoutMergeNoneExisting(t *testing.T) { diff --git a/server/server.go b/server/server.go index f4cd5e7fc..a1b0cd781 100644 --- a/server/server.go +++ b/server/server.go @@ -268,7 +268,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { // provide fresh tokens before clone from the GitHub Apps integration, proxy workingDir if githubAppEnabled { if !userConfig.WriteGitCreds { - logger.Warn("Github App requires --write-git-creds, overriding") + return nil, errors.New("Github App requires --write-git-creds to support cloning") } workingDir = &events.GithubAppWorkingDir{ WorkingDir: workingDir, diff --git a/server/user_config.go b/server/user_config.go index 507e8f742..8064ca168 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -30,7 +30,7 @@ type UserConfig struct { GithubWebhookSecret string `mapstructure:"gh-webhook-secret"` GithubOrg string `mapstructure:"gh-org"` GithubAppID int64 `mapstructure:"gh-app-id"` - GithubAppKey string `mapstructure:"gh-app-key"` + GithubAppKey string `mapstructure:"gh-app-key-file"` GitlabHostname string `mapstructure:"gitlab-hostname"` GitlabToken string `mapstructure:"gitlab-token"` GitlabUser string `mapstructure:"gitlab-user"` diff --git a/server/web_templates.go b/server/web_templates.go index a4570039c..5cf710e2c 100644 --- a/server/web_templates.go +++ b/server/web_templates.go @@ -314,7 +314,7 @@ var githubAppSetupTemplate = template.Must(template.New("github-app.html.tmpl"). {{ end }}