allow user to pass gh-app-key directly instead of filename (#1706)

* allow user to pass gh-app-key directly instead of filename * start fixing tests * fix make test * update website with info about new flag * Apply suggestions from code review Co-authored-by: Roberto Hidalgo <[email protected]> Co-authored-by: Roberto Hidalgo <[email protected]>
runatlantis · Aug 18, 2021 · 01b8718 · 01b8718
1 parent fe31bc6
commit 01b8718
Show file tree

Hide file tree

Showing 9 changed files with 73 additions and 38 deletions.
diff --git a/cmd/server.go b/cmd/server.go
@@ -65,6 +65,7 @@ const (
 	GHTokenFlag                = "gh-token"
 	GHUserFlag                 = "gh-user"
 	GHAppIDFlag                = "gh-app-id"
+	GHAppKeyFlag               = "gh-app-key"
 	GHAppKeyFileFlag           = "gh-app-key-file"
 	GHAppSlugFlag              = "gh-app-slug"
 	GHOrganizationFlag         = "gh-org"
@@ -192,6 +193,10 @@ var stringFlags = map[string]stringFlag{
 	GHTokenFlag: {
 		description: "GitHub token of API user. Can also be specified via the ATLANTIS_GH_TOKEN environment variable.",
 	},
+	GHAppKeyFlag: {
+		description:  "The GitHub App's private key",
+		defaultValue: "",
+	},
 	GHAppKeyFileFlag: {
 		description:  "A path to a file containing the GitHub App's private key",
 		defaultValue: "",
@@ -634,12 +639,19 @@ func (s *ServerCmd) validate(userConfig server.UserConfig) error {
 
 	// The following combinations are valid.
 	// 1. github user and token set
-	// 2. gitlab user and token set
-	// 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, 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 == "")) {
+	// 2. github app ID and (key file set or key set)
+	// 3. gitlab user and token set
+	// 4. bitbucket user and token set
+	// 5. azuredevops user and token set
+	// 6. any combination of the above
+	vcsErr := fmt.Errorf("--%s/--%s or --%s/--%s or --%s/--%s or --%s/--%s or --%s/--%s or --%s/--%s must be set", GHUserFlag, GHTokenFlag, GHAppIDFlag, GHAppKeyFileFlag, GHAppIDFlag, GHAppKeyFlag, GitlabUserFlag, GitlabTokenFlag, BitbucketUserFlag, BitbucketTokenFlag, ADUserFlag, ADTokenFlag)
+	if ((userConfig.GithubUser == "") != (userConfig.GithubToken == "")) || ((userConfig.GitlabUser == "") != (userConfig.GitlabToken == "")) || ((userConfig.BitbucketUser == "") != (userConfig.BitbucketToken == "")) || ((userConfig.AzureDevopsUser == "") != (userConfig.AzureDevopsToken == "")) {
+		return vcsErr
+	}
+	if (userConfig.GithubAppID != 0) && ((userConfig.GithubAppKey == "") && (userConfig.GithubAppKeyFile == "")) {
+		return vcsErr
+	}
+	if (userConfig.GithubAppID == 0) && ((userConfig.GithubAppKey != "") || (userConfig.GithubAppKeyFile != "")) {
 		return vcsErr
 	}
 	// At this point, we know that there can't be a single user/token without

diff --git a/cmd/server_test.go b/cmd/server_test.go
@@ -24,6 +24,7 @@ import (
 
 	homedir "github.com/mitchellh/go-homedir"
 	"github.com/runatlantis/atlantis/server"
+	"github.com/runatlantis/atlantis/server/events/vcs/fixtures"
 	"github.com/runatlantis/atlantis/server/logging"
 	. "github.com/runatlantis/atlantis/testing"
 	"github.com/spf13/cobra"
@@ -75,6 +76,7 @@ var testFlags = map[string]interface{}{
 	GHTokenFlag:                "token",
 	GHUserFlag:                 "user",
 	GHAppIDFlag:                int64(0),
+	GHAppKeyFlag:               "",
 	GHAppKeyFileFlag:           "",
 	GHAppSlugFlag:              "atlantis",
 	GHOrganizationFlag:         "",
@@ -350,7 +352,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-file 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 --gh-app-id/--gh-app-key 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{}
@@ -404,12 +406,19 @@ func TestExecute_ValidateVCSConfig(t *testing.T) {
 			true,
 		},
 		{
-			"just github app key set",
+			"just github app key file set",
 			map[string]interface{}{
 				GHAppKeyFileFlag: "key.pem",
 			},
 			true,
 		},
+		{
+			"just github app key set",
+			map[string]interface{}{
+				GHAppKeyFlag: fixtures.GithubPrivateKey,
+			},
+			true,
+		},
 		{
 			"just gitlab user set",
 			map[string]interface{}{
@@ -464,13 +473,21 @@ func TestExecute_ValidateVCSConfig(t *testing.T) {
 			false,
 		},
 		{
-			"github app and key set and should be successful",
+			"github app and key file set and should be successful",
 			map[string]interface{}{
 				GHAppIDFlag:      "1",
 				GHAppKeyFileFlag: "key.pem",
 			},
 			false,
 		},
+		{
+			"github app and key set and should be successful",
+			map[string]interface{}{
+				GHAppIDFlag:  "1",
+				GHAppKeyFlag: fixtures.GithubPrivateKey,
+			},
+			false,
+		},
 		{
 			"gitlab user and gitlab token set and should be successful",
 			map[string]interface{}{
@@ -572,7 +589,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{}{
-		GHAppKeyFileFlag:  "key.pem",
+		GHAppKeyFlag:      fixtures.GithubPrivateKey,
 		GHAppIDFlag:       "1",
 		RepoAllowlistFlag: "*",
 	}, t)

diff --git a/runatlantis.io/docs/access-credentials.md b/runatlantis.io/docs/access-credentials.md
@@ -45,7 +45,7 @@ Available in Atlantis versions **newer** than 0.13.0.
 - 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 <your id> --gh-app-key-file atlantis-app-key.pem --gh-webhook-secret <your secret> --write-git-creds --repo-allowlist '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).
+  NOTE: Instead of using a file for the GitHub App Key you can also pass the key value directly using `--gh-app-key`. 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
@@ -342,6 +342,16 @@ Values are chosen in this order:
   ```
   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/).
 
+- ### `--gh-app-key`
+  ```bash
+  atlantis server --gh-app-key="-----BEGIN RSA PRIVATE KEY-----(...)" 
+  ```
+  The PEM encoded private key for the GitHub App.
+
+  ::: warning SECURITY WARNING
+  The contents of the private key will be visible by anyone that can run `ps` or look at the shell history of the machine where Atlantis is running. Use `--gh-app-key-file` to mitigate that risk.
+  :::
+
 * ### `--gitlab-hostname`
   ```bash
   atlantis server --gitlab-hostname="my.gitlab.enterprise.com"

diff --git a/server/events/github_app_working_dir_test.go b/server/events/github_app_working_dir_test.go
@@ -31,19 +31,14 @@ func TestClone_GithubAppNoneExisting(t *testing.T) {
 		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),
+			Key:      []byte(fixtures.GithubPrivateKey),
 			AppID:    1,
 			Hostname: testServer,
 		},

diff --git a/server/events/vcs/github_credentials.go b/server/events/vcs/github_credentials.go
@@ -68,7 +68,7 @@ func (c *GithubUserCredentials) GetToken() (string, error) {
 // GithubAppCredentials implements GithubCredentials for github app installation token flow.
 type GithubAppCredentials struct {
 	AppID          int64
-	KeyPath        string
+	Key            []byte
 	Hostname       string
 	apiURL         *url.URL
 	installationID int64
@@ -128,7 +128,7 @@ func (c *GithubAppCredentials) getInstallationID() (int64, error) {
 
 	tr := http.DefaultTransport
 	// A non-installation transport
-	t, err := ghinstallation.NewAppsTransportKeyFromFile(tr, c.AppID, c.KeyPath)
+	t, err := ghinstallation.NewAppsTransport(tr, c.AppID, c.Key)
 	if err != nil {
 		return 0, err
 	}
@@ -163,7 +163,7 @@ func (c *GithubAppCredentials) transport() (*ghinstallation.Transport, error) {
 	}
 
 	tr := http.DefaultTransport
-	itr, err := ghinstallation.NewKeyFromFile(tr, c.AppID, installationID, c.KeyPath)
+	itr, err := ghinstallation.New(tr, c.AppID, installationID, c.Key)
 	if err == nil {
 		apiURL := c.getAPIURL()
 		itr.BaseURL = strings.TrimSuffix(apiURL.String(), "/")

diff --git a/server/events/vcs/github_credentials_test.go b/server/events/vcs/github_credentials_test.go
@@ -1,7 +1,6 @@
 package vcs_test
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/runatlantis/atlantis/server/events/vcs"
@@ -21,15 +20,9 @@ func TestGithubClient_GetUser_AppSlug(t *testing.T) {
 	tempSecrets, err := anonClient.ExchangeCode("good-code")
 	Ok(t, err)
 
-	tmpDir, cleanup := DirStructure(t, map[string]interface{}{
-		"key.pem": tempSecrets.Key,
-	})
-	defer cleanup()
-	keyPath := fmt.Sprintf("%v/key.pem", tmpDir)
-
 	appCreds := &vcs.GithubAppCredentials{
 		AppID:    tempSecrets.ID,
-		KeyPath:  keyPath,
+		Key:      []byte(fixtures.GithubPrivateKey),
 		Hostname: testServer,
 		AppSlug:  "some-app",
 	}
@@ -51,15 +44,9 @@ func TestGithubClient_AppAuthentication(t *testing.T) {
 	tempSecrets, err := anonClient.ExchangeCode("good-code")
 	Ok(t, err)
 
-	tmpDir, cleanup := DirStructure(t, map[string]interface{}{
-		"key.pem": tempSecrets.Key,
-	})
-	defer cleanup()
-	keyPath := fmt.Sprintf("%v/key.pem", tmpDir)
-
 	appCreds := &vcs.GithubAppCredentials{
 		AppID:    tempSecrets.ID,
-		KeyPath:  keyPath,
+		Key:      []byte(fixtures.GithubPrivateKey),
 		Hostname: testServer,
 	}
 	_, err = vcs.NewGithubClient(testServer, appCreds, logging.NewNoopLogger(t))

diff --git a/server/server.go b/server/server.go
@@ -20,6 +20,7 @@ import (
 	"encoding/json"
 	"flag"
 	"fmt"
+	"io/ioutil"
 	"log"
 	"net/http"
 	"net/url"
@@ -156,10 +157,22 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
 				User:  userConfig.GithubUser,
 				Token: userConfig.GithubToken,
 			}
-		} else if userConfig.GithubAppID != 0 {
+		} else if userConfig.GithubAppID != 0 && userConfig.GithubAppKeyFile != "" {
+			privateKey, err := ioutil.ReadFile(userConfig.GithubAppKeyFile)
+			if err != nil {
+				return nil, err
+			}
+			githubCredentials = &vcs.GithubAppCredentials{
+				AppID:    userConfig.GithubAppID,
+				Key:      privateKey,
+				Hostname: userConfig.GithubHostname,
+				AppSlug:  userConfig.GithubAppSlug,
+			}
+			githubAppEnabled = true
+		} else if userConfig.GithubAppID != 0 && userConfig.GithubAppKey != "" {
 			githubCredentials = &vcs.GithubAppCredentials{
 				AppID:    userConfig.GithubAppID,
-				KeyPath:  userConfig.GithubAppKey,
+				Key:      []byte(userConfig.GithubAppKey),
 				Hostname: userConfig.GithubHostname,
 				AppSlug:  userConfig.GithubAppSlug,
 			}

diff --git a/server/user_config.go b/server/user_config.go
@@ -36,7 +36,8 @@ 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-file"`
+	GithubAppKey               string `mapstructure:"gh-app-key"`
+	GithubAppKeyFile           string `mapstructure:"gh-app-key-file"`
 	GithubAppSlug              string `mapstructure:"gh-app-slug"`
 	GitlabHostname             string `mapstructure:"gitlab-hostname"`
 	GitlabToken                string `mapstructure:"gitlab-token"`