diff --git a/cmd/server.go b/cmd/server.go index 2ae1bab24..101d7fd43 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -61,6 +61,7 @@ const ( GHUserFlag = "gh-user" GHAppIDFlag = "gh-app-id" GHAppKeyFileFlag = "gh-app-key-file" + GHAppSlugFlag = "gh-app-slug" GHOrganizationFlag = "gh-org" GHWebhookSecretFlag = "gh-webhook-secret" // nolint: gosec GitlabHostnameFlag = "gitlab-hostname" @@ -181,6 +182,9 @@ var stringFlags = map[string]stringFlag{ description: "A path to a file containing the GitHub App's private key", defaultValue: "", }, + GHAppSlugFlag: { + description: "The Github app slug (ie. the URL-friendly name of your GitHub App)", + }, GHOrganizationFlag: { description: "The name of the GitHub organization to use during the creation of a Github App for Atlantis", defaultValue: "", diff --git a/cmd/server_test.go b/cmd/server_test.go index d60f8bffe..e22b372ad 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -71,6 +71,7 @@ var testFlags = map[string]interface{}{ GHUserFlag: "user", GHAppIDFlag: int64(0), GHAppKeyFileFlag: "", + GHAppSlugFlag: "atlantis", GHOrganizationFlag: "", GHWebhookSecretFlag: "secret", GitlabHostnameFlag: "gitlab-hostname", diff --git a/server/events/vcs/fixtures/fixtures.go b/server/events/vcs/fixtures/fixtures.go index 0f9f2c331..5bd935ba4 100644 --- a/server/events/vcs/fixtures/fixtures.go +++ b/server/events/vcs/fixtures/fixtures.go @@ -332,7 +332,7 @@ var githubAppInstallationJSON = `[ // nolint: gosec var githubAppTokenJSON = `{ - "token": "v1.1f699f1069f60xx%d", + "token": "some-token", "expires_at": "2050-01-01T00:00:00Z", "permissions": { "issues": "write", @@ -452,6 +452,48 @@ var githubAppTokenJSON = `{ ] }` +var githubAppJSON = `{ + "id": 1, + "slug": "octoapp", + "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", + "avatar_url": "https://github.com/images/error/octocat_happy.gif", + "gravatar_id": "", + "html_url": "https://github.com/octocat", + "followers_url": "https://api.github.com/users/octocat/followers", + "following_url": "https://api.github.com/users/octocat/following{/other_user}", + "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}", + "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/octocat/subscriptions", + "organizations_url": "https://api.github.com/users/octocat/orgs", + "received_events_url": "https://api.github.com/users/octocat/received_events", + "type": "User", + "site_admin": true + }, + "name": "Octocat App", + "description": "", + "external_url": "https://example.com", + "html_url": "https://github.com/apps/octoapp", + "created_at": "2017-07-08T16:18:44-04:00", + "updated_at": "2017-07-08T16:18:44-04:00", + "permissions": { + "metadata": "read", + "contents": "read", + "issues": "write", + "single_file": "write" + }, + "events": [ + "push", + "pull_request" + ] + }` + func validateGithubToken(tokenString string) error { key, err := jwt.ParseRSAPrivateKeyFromPEM([]byte(GithubPrivateKey)) if err != nil { @@ -499,6 +541,17 @@ func GithubAppTestServer(t *testing.T) (string, error) { w.Write([]byte(githubAppInstallationJSON)) // nolint: errcheck return + case "/api/v3/apps/some-app": + token := strings.Replace(r.Header.Get("Authorization"), "token ", "", 1) + + // token is taken from githubAppTokenJSON + if token != "some-token" { + w.WriteHeader(403) + w.Write([]byte("Invalid installation token")) // nolint: errcheck + return + } + w.Write([]byte(githubAppJSON)) // nolint: errcheck + return case "/api/v3/app/installations/1/access_tokens": token := strings.Replace(r.Header.Get("Authorization"), "Bearer ", "", 1) if err := validateGithubToken(token); err != nil { diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index e97c863b9..476260fc9 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -97,8 +97,15 @@ func NewGithubClient(hostname string, credentials GithubCredentials, logger *log transport, graphql.WithHeader("Accept", "application/vnd.github.queen-beryl-preview+json"), ) + + user, err := credentials.GetUser() + logger.Debug("GH User: %s", user) + + if err != nil { + return nil, errors.Wrap(err, "getting user") + } return &GithubClient{ - user: credentials.GetUser(), + user: user, client: client, v4MutateClient: v4MutateClient, ctx: context.Background(), @@ -179,7 +186,7 @@ func (g *GithubClient) HidePrevPlanComments(repo models.Repo, pullNum int) error ListOptions: github.ListOptions{Page: nextPage}, }) if err != nil { - return err + return errors.Wrap(err, "listing comments") } allComments = append(allComments, comments...) if resp.NextPage == 0 { diff --git a/server/events/vcs/github_credentials.go b/server/events/vcs/github_credentials.go index 29db764e0..dc18457e0 100644 --- a/server/events/vcs/github_credentials.go +++ b/server/events/vcs/github_credentials.go @@ -18,7 +18,7 @@ import ( type GithubCredentials interface { Client() (*http.Client, error) GetToken() (string, error) - GetUser() string + GetUser() (string, error) } // GithubAnonymousCredentials expose no credentials. @@ -31,8 +31,8 @@ func (c *GithubAnonymousCredentials) Client() (*http.Client, error) { } // GetUser returns the username for these credentials. -func (c *GithubAnonymousCredentials) GetUser() string { - return "anonymous" +func (c *GithubAnonymousCredentials) GetUser() (string, error) { + return "anonymous", nil } // GetToken returns an empty token. @@ -56,8 +56,8 @@ func (c *GithubUserCredentials) Client() (*http.Client, error) { } // GetUser returns the username for these credentials. -func (c *GithubUserCredentials) GetUser() string { - return c.User +func (c *GithubUserCredentials) GetUser() (string, error) { + return c.User, nil } // GetToken returns the user token. @@ -73,6 +73,7 @@ type GithubAppCredentials struct { apiURL *url.URL installationID int64 tr *ghinstallation.Transport + AppSlug string } // Client returns a github app installation client. @@ -85,8 +86,29 @@ func (c *GithubAppCredentials) Client() (*http.Client, error) { } // GetUser returns the username for these credentials. -func (c *GithubAppCredentials) GetUser() string { - return "" +func (c *GithubAppCredentials) GetUser() (string, error) { + // Keeping backwards compatibility since this flag is optional + if c.AppSlug == "" { + return "", nil + } + client, err := c.Client() + + if err != nil { + return "", errors.Wrap(err, "initializing client") + } + + ghClient := github.NewClient(client) + ghClient.BaseURL = c.getAPIURL() + ctx := context.Background() + + app, _, err := ghClient.Apps.Get(ctx, c.AppSlug) + + if err != nil { + return "", errors.Wrap(err, "getting app details") + } + // Currently there is no way to get the bot's login info, so this is a + // hack until Github exposes that. + return fmt.Sprintf("%s[bot]", app.GetName()), nil } // GetToken returns a fresh installation token. diff --git a/server/events/vcs/github_credentials_test.go b/server/events/vcs/github_credentials_test.go index 3be20f3b2..c643c9706 100644 --- a/server/events/vcs/github_credentials_test.go +++ b/server/events/vcs/github_credentials_test.go @@ -9,6 +9,36 @@ import ( . "github.com/runatlantis/atlantis/testing" ) +func TestGithubClient_GetUser_AppSlug(t *testing.T) { + defer disableSSLVerification()() + testServer, err := fixtures.GithubAppTestServer(t) + Ok(t, err) + + anonCreds := &vcs.GithubAnonymousCredentials{} + anonClient, err := vcs.NewGithubClient(testServer, anonCreds, nil) + Ok(t, err) + 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, + Hostname: testServer, + AppSlug: "some-app", + } + + user, err := appCreds.GetUser() + Ok(t, err) + + Assert(t, user == "Octocat App[bot]", "user should not empty") +} + func TestGithubClient_AppAuthentication(t *testing.T) { defer disableSSLVerification()() testServer, err := fixtures.GithubAppTestServer(t) @@ -40,6 +70,11 @@ func TestGithubClient_AppAuthentication(t *testing.T) { newToken, err := appCreds.GetToken() Ok(t, err) + user, err := appCreds.GetUser() + Ok(t, err) + + Assert(t, user == "", "user should be empty") + if token != newToken { t.Errorf("app token was not cached: %q != %q", token, newToken) } diff --git a/server/events/vcs/mocks/matchers/ptr_to_http_client.go b/server/events/vcs/mocks/matchers/ptr_to_http_client.go index 4ec255001..74766e888 100644 --- a/server/events/vcs/mocks/matchers/ptr_to_http_client.go +++ b/server/events/vcs/mocks/matchers/ptr_to_http_client.go @@ -2,8 +2,9 @@ package matchers import ( - "reflect" "github.com/petergtz/pegomock" + "reflect" + http "net/http" ) @@ -18,3 +19,15 @@ func EqPtrToHttpClient(value *http.Client) *http.Client { var nullValue *http.Client return nullValue } + +func NotEqPtrToHttpClient(value *http.Client) *http.Client { + pegomock.RegisterMatcher(&pegomock.NotEqMatcher{Value: value}) + var nullValue *http.Client + return nullValue +} + +func PtrToHttpClientThat(matcher pegomock.ArgumentMatcher) *http.Client { + pegomock.RegisterMatcher(matcher) + var nullValue *http.Client + return nullValue +} diff --git a/server/events/vcs/mocks/mock_github_credentials.go b/server/events/vcs/mocks/mock_github_credentials.go index 2837977c4..d491009dc 100644 --- a/server/events/vcs/mocks/mock_github_credentials.go +++ b/server/events/vcs/mocks/mock_github_credentials.go @@ -63,19 +63,23 @@ func (mock *MockGithubCredentials) GetToken() (string, error) { return ret0, ret1 } -func (mock *MockGithubCredentials) GetUser() string { +func (mock *MockGithubCredentials) GetUser() (string, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockGithubCredentials().") } params := []pegomock.Param{} - result := pegomock.GetGenericMockFrom(mock).Invoke("GetUser", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem()}) + result := pegomock.GetGenericMockFrom(mock).Invoke("GetUser", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var ret0 string + var ret1 error if len(result) != 0 { if result[0] != nil { ret0 = result[0].(string) } + if result[1] != nil { + ret1 = result[1].(error) + } } - return ret0 + return ret0, ret1 } func (mock *MockGithubCredentials) VerifyWasCalledOnce() *VerifierMockGithubCredentials { @@ -85,14 +89,14 @@ func (mock *MockGithubCredentials) VerifyWasCalledOnce() *VerifierMockGithubCred } } -func (mock *MockGithubCredentials) VerifyWasCalled(invocationCountMatcher pegomock.Matcher) *VerifierMockGithubCredentials { +func (mock *MockGithubCredentials) VerifyWasCalled(invocationCountMatcher pegomock.InvocationCountMatcher) *VerifierMockGithubCredentials { return &VerifierMockGithubCredentials{ mock: mock, invocationCountMatcher: invocationCountMatcher, } } -func (mock *MockGithubCredentials) VerifyWasCalledInOrder(invocationCountMatcher pegomock.Matcher, inOrderContext *pegomock.InOrderContext) *VerifierMockGithubCredentials { +func (mock *MockGithubCredentials) VerifyWasCalledInOrder(invocationCountMatcher pegomock.InvocationCountMatcher, inOrderContext *pegomock.InOrderContext) *VerifierMockGithubCredentials { return &VerifierMockGithubCredentials{ mock: mock, invocationCountMatcher: invocationCountMatcher, @@ -100,7 +104,7 @@ func (mock *MockGithubCredentials) VerifyWasCalledInOrder(invocationCountMatcher } } -func (mock *MockGithubCredentials) VerifyWasCalledEventually(invocationCountMatcher pegomock.Matcher, timeout time.Duration) *VerifierMockGithubCredentials { +func (mock *MockGithubCredentials) VerifyWasCalledEventually(invocationCountMatcher pegomock.InvocationCountMatcher, timeout time.Duration) *VerifierMockGithubCredentials { return &VerifierMockGithubCredentials{ mock: mock, invocationCountMatcher: invocationCountMatcher, @@ -110,7 +114,7 @@ func (mock *MockGithubCredentials) VerifyWasCalledEventually(invocationCountMatc type VerifierMockGithubCredentials struct { mock *MockGithubCredentials - invocationCountMatcher pegomock.Matcher + invocationCountMatcher pegomock.InvocationCountMatcher inOrderContext *pegomock.InOrderContext timeout time.Duration } diff --git a/server/server.go b/server/server.go index 1727cca4f..fd60c40ac 100644 --- a/server/server.go +++ b/server/server.go @@ -158,6 +158,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { AppID: userConfig.GithubAppID, KeyPath: userConfig.GithubAppKey, Hostname: userConfig.GithubHostname, + AppSlug: userConfig.GithubAppSlug, } githubAppEnabled = true } diff --git a/server/user_config.go b/server/user_config.go index 82cae6a75..d138c43f5 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -33,6 +33,7 @@ type UserConfig struct { GithubOrg string `mapstructure:"gh-org"` GithubAppID int64 `mapstructure:"gh-app-id"` GithubAppKey string `mapstructure:"gh-app-key-file"` + GithubAppSlug string `mapstructure:"gh-app-slug"` GitlabHostname string `mapstructure:"gitlab-hostname"` GitlabToken string `mapstructure:"gitlab-token"` GitlabUser string `mapstructure:"gitlab-user"`