fix(GithubEventSource): GitHub Enterprise auth logic broken (#2494) (#…

…2495)

Signed-off-by: Petri Kivikangas <[email protected]>

eventsources/sources/github/appauth.go

@@ -21,14 +21,19 @@ import (
 
 type AppsAuthStrategy struct {
 	AppID          int64
+	BaseURL        string
 	InstallationID int64
 	PrivateKey     string
 	Transport      http.RoundTripper
 }
 
 // AuthTransport implements the AuthStrategy interface.
 func (t *AppsAuthStrategy) AuthTransport() (http.RoundTripper, error) {
-	return ghinstallation.New(t.transport(), t.AppID, t.InstallationID, []byte(t.PrivateKey))
+	appTransport, err := ghinstallation.New(t.transport(), t.AppID, t.InstallationID, []byte(t.PrivateKey))
+	if appTransport != nil && t.BaseURL != "" {
+		appTransport.BaseURL = t.BaseURL
+	}
+	return appTransport, err
 }
 
 func (t *AppsAuthStrategy) transport() http.RoundTripper {

eventsources/sources/github/hook_util.go

@@ -1,7 +1,7 @@
 package github
 
 import (
-	gh "github.com/google/go-github/v31/github"
+	gh "github.com/google/go-github/v50/github"
 
 	"github.com/argoproj/argo-events/common"
 )

eventsources/sources/github/hook_util_test.go

@@ -3,7 +3,7 @@ package github
 import (
 	"testing"
 
-	gh "github.com/google/go-github/v31/github"
+	gh "github.com/google/go-github/v50/github"
 	"github.com/stretchr/testify/assert"
 )
 

eventsources/sources/github/start.go

@@ -23,9 +23,10 @@ import (
 	"math/big"
 	"net/http"
 	"net/url"
+	"strings"
 	"time"
 
-	gh "github.com/google/go-github/v31/github"
+	gh "github.com/google/go-github/v50/github"
 	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
 
@@ -88,6 +89,7 @@ func (router *Router) getGithubAppAuthStrategy() (*AppsAuthStrategy, error) {
 
 	return &AppsAuthStrategy{
 		AppID:          appCreds.AppID,
+		BaseURL:        router.githubEventSource.GithubBaseURL,
 		InstallationID: appCreds.InstallationID,
 		PrivateKey:     githubAppPrivateKey.secret,
 	}, nil
@@ -239,11 +241,21 @@ func (el *EventListener) StartListening(ctx context.Context, dispatch func([]byt
 		// create webhooks
 
 		// In order to successfully setup a GitHub hook for the given repository,
-		// 1. Get the GitHub auth credentials and Webhook secret from K8s secrets
-		// 2. Configure the hook with url, content type, ssl etc.
-		// 3. Set up a GitHub client
-		// 4. Set the base and upload url for the client
-		// 5. Create the hook if one doesn't exist already. If exists already, then use that one.
+		// 1. Parse and validate base and upload url if provided
+		// 2. Get the GitHub auth credentials and Webhook secret from K8s secrets
+		// 3. Configure the hook with url, content type, ssl etc.
+		// 4. Set up a GitHub client
+		// 5. Set the base and upload url for the client
+		// 6. Create the hook if one doesn't exist already. If exists already, then use that one.
+
+		baseURL, err := parseUrlWithSlash(&githubEventSource.GithubBaseURL)
+		if err != nil {
+			return fmt.Errorf("failed to parse github base url. err: %v", err)
+		}
+		uploadURL, err := parseUrlWithSlash(&githubEventSource.GithubUploadURL)
+		if err != nil {
+			return fmt.Errorf("failed to parse github upload url. err: %v", err)
+		}
 
 		logger.Info("choosing github auth strategy...")
 		authStrategy, err := router.chooseAuthStrategy()
@@ -276,24 +288,13 @@ func (el *EventListener) StartListening(ctx context.Context, dispatch func([]byt
 
 		logger.Info("setting up client for GitHub...")
 		client := gh.NewClient(&http.Client{Transport: authTransport})
-
-		logger.Info("setting up base url for GitHub client...")
-		if githubEventSource.GithubBaseURL != "" {
-			baseURL, err := url.Parse(githubEventSource.GithubBaseURL)
-			if err != nil {
-				return fmt.Errorf("failed to parse github base url. err: %v", err)
-			}
+		if baseURL != nil && uploadURL != nil {
+			logger.Info("setting up client for GitHub Enterprise...")
 			client.BaseURL = baseURL
-		}
-
-		logger.Info("setting up the upload url for GitHub client...")
-		if githubEventSource.GithubUploadURL != "" {
-			uploadURL, err := url.Parse(githubEventSource.GithubUploadURL)
-			if err != nil {
-				return fmt.Errorf("failed to parse github upload url. err: %v", err)
-			}
 			client.UploadURL = uploadURL
 		}
+		logger.Infof("client set for baseURL=[%s] uploadURL=[%s]", client.BaseURL, client.UploadURL)
+
 		router.githubClient = client
 		router.repoHookIDs = make(map[string]int64)
 		router.orgHookIDs = make(map[string]int64)
@@ -397,3 +398,14 @@ func parseValidateRequest(r *http.Request, secret []byte) ([]byte, error) {
 	}
 	return json.Marshal(payload)
 }
+
+// parseUrlWithSlash parses URL and enforces trailing slash expected by GitHub client
+func parseUrlWithSlash(urlStr *string) (*url.URL, error) {
+	if *urlStr == "" {
+		return nil, nil
+	}
+	if !strings.HasSuffix(*urlStr, "/") {
+		*urlStr += "/"
+	}
+	return url.Parse(*urlStr)
+}

eventsources/sources/github/types.go

@@ -18,7 +18,7 @@ package github
 import (
 	"net/http"
 
-	"github.com/google/go-github/v31/github"
+	"github.com/google/go-github/v50/github"
 
 	"github.com/argoproj/argo-events/eventsources/common/webhook"
 	"github.com/argoproj/argo-events/metrics"

eventsources/sources/github/validate.go

@@ -46,5 +46,16 @@ func validate(githubEventSource *v1alpha1.GithubEventSource) error {
 			return fmt.Errorf("content type must be \"json\" or \"form\"")
 		}
 	}
+
+	// in order to avoid requests ending accidentally to public GitHub,
+	// make sure that both are set if either one is provided
+	if githubEventSource.GithubBaseURL != "" || githubEventSource.GithubUploadURL != "" {
+		if githubEventSource.GithubBaseURL == "" {
+			return fmt.Errorf("githubBaseURL is required when githubUploadURL is set")
+		}
+		if githubEventSource.GithubUploadURL == "" {
+			return fmt.Errorf("githubUploadURL is required when githubBaseURL is set")
+		}
+	}
 	return webhook.ValidateWebhookContext(githubEventSource.Webhook)
 }

go.mod

@@ -23,7 +23,7 @@ require (
 	github.com/argoproj/pkg v0.13.6
 	github.com/aws/aws-sdk-go v1.44.209
 	github.com/blushft/go-diagrams v0.0.0-20201006005127-c78c821223d9
-	github.com/bradleyfalzon/ghinstallation/v2 v2.1.0
+	github.com/bradleyfalzon/ghinstallation/v2 v2.2.0
 	github.com/cloudevents/sdk-go/v2 v2.13.0
 	github.com/colinmarc/hdfs v1.1.4-0.20180802165501-48eb8d6c34a9
 	github.com/eclipse/paho.mqtt.golang v1.4.1
@@ -41,7 +41,7 @@ require (
 	github.com/gogo/protobuf v1.3.2
 	github.com/golang/protobuf v1.5.2
 	github.com/google/go-cmp v0.5.9
-	github.com/google/go-github/v31 v31.0.0
+	github.com/google/go-github/v50 v50.1.0
 	github.com/google/uuid v1.3.0
 	github.com/gorilla/mux v1.8.0
 	github.com/grpc-ecosystem/grpc-gateway v1.16.0
@@ -113,7 +113,6 @@ require (
 	github.com/golang-jwt/jwt v3.2.2+incompatible // indirect
 	github.com/google/gnostic v0.5.7-v3refs // indirect
 	github.com/google/go-github/v41 v41.0.0 // indirect
-	github.com/google/go-github/v45 v45.2.0 // indirect
 	github.com/googleapis/enterprise-certificate-proxy v0.2.1 // indirect
 	github.com/gregdel/pushover v1.1.0 // indirect
 	github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c // indirect
@@ -195,7 +194,7 @@ require (
 	github.com/go-openapi/swag v0.22.3 // indirect
 	github.com/go-openapi/validate v0.22.0 // indirect
 	github.com/gobuffalo/flect v0.2.3 // indirect
-	github.com/golang-jwt/jwt/v4 v4.4.2 // indirect
+	github.com/golang-jwt/jwt/v4 v4.5.0 // indirect
 	github.com/golang/glog v1.0.0 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/snappy v0.0.4 // indirect

go.sum

@@ -212,8 +212,8 @@ github.com/blushft/go-diagrams v0.0.0-20201006005127-c78c821223d9 h1:mV+hh0rMjzr
 github.com/blushft/go-diagrams v0.0.0-20201006005127-c78c821223d9/go.mod h1:nDeXEIaeDV+mAK1gBD3/RJH67DYPC0GdaznWN7sB07s=
 github.com/bmatcuk/doublestar v1.1.1/go.mod h1:UD6OnuiIn0yFxxA2le/rnRU1G4RaI4UvFv1sNto9p6w=
 github.com/bmizerany/perks v0.0.0-20141205001514-d9a9656a3a4b/go.mod h1:ac9efd0D1fsDb3EJvhqgXRbFx7bs2wqZ10HQPeU8U/Q=
-github.com/bradleyfalzon/ghinstallation/v2 v2.1.0 h1:5+NghM1Zred9Z078QEZtm28G/kfDfZN/92gkDlLwGVA=
-github.com/bradleyfalzon/ghinstallation/v2 v2.1.0/go.mod h1:Xg3xPRN5Mcq6GDqeUVhFbjEWMb4JHCyWEeeBGEYQoTU=
+github.com/bradleyfalzon/ghinstallation/v2 v2.2.0 h1:AVvVU33rE8wdTS1aNnenwpigEBA9mvzI5OhjhZfH/LU=
+github.com/bradleyfalzon/ghinstallation/v2 v2.2.0/go.mod h1:xo3iIfK0lDKECe0s19nbxT0KKvk7LsrGc4NxR5ckKMA=
 github.com/bwmarrin/discordgo v0.19.0/go.mod h1:O9S4p+ofTFwB02em7jkpkV8M3R0/PUVOwN61zSZ0r4Q=
 github.com/cenkalti/backoff v2.1.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM=
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
@@ -479,9 +479,8 @@ github.com/golang-jwt/jwt v3.2.2+incompatible h1:IfV12K8xAKAnZqdXVzCZ+TOjboZ2keL
 github.com/golang-jwt/jwt v3.2.2+incompatible/go.mod h1:8pz2t5EyA70fFQQSrl6XZXzqecmYZeUEB8OUGHkxJ+I=
 github.com/golang-jwt/jwt/v4 v4.0.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg=
 github.com/golang-jwt/jwt/v4 v4.2.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg=
-github.com/golang-jwt/jwt/v4 v4.4.1/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
-github.com/golang-jwt/jwt/v4 v4.4.2 h1:rcc4lwaZgFMCZ5jxF9ABolDcIHdBytAFgqFPbSJQAYs=
-github.com/golang-jwt/jwt/v4 v4.4.2/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
+github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg=
+github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
 github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
 github.com/golang/glog v1.0.0 h1:nfP3RFugxnNRyKgeWd4oI1nYvXpxrx8ck8ZrcizshdQ=
 github.com/golang/glog v1.0.0/go.mod h1:EWib/APOK0SL3dFbYqvxE3UYd8E6s1ouQ7iEp/0LWV4=
@@ -545,15 +544,12 @@ github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8/DtOE=
-github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
 github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
-github.com/google/go-github/v31 v31.0.0 h1:JJUxlP9lFK+ziXKimTCprajMApV1ecWD4NB6CCb0plo=
-github.com/google/go-github/v31 v31.0.0/go.mod h1:NQPZol8/1sMoWYGN2yaALIBytu17gAWfhbweiEed3pM=
 github.com/google/go-github/v41 v41.0.0 h1:HseJrM2JFf2vfiZJ8anY2hqBjdfY1Vlj/K27ueww4gg=
 github.com/google/go-github/v41 v41.0.0/go.mod h1:XgmCA5H323A9rtgExdTcnDkcqp6S30AVACCBDOonIxg=
-github.com/google/go-github/v45 v45.2.0 h1:5oRLszbrkvxDDqBCNj2hjDZMKmvexaZ1xw/FCD+K3FI=
-github.com/google/go-github/v45 v45.2.0/go.mod h1:FObaZJEDSTa/WGCzZ2Z3eoCDXWJKMenWWTrd8jrta28=
+github.com/google/go-github/v50 v50.1.0 h1:hMUpkZjklC5GJ+c3GquSqOP/T4BNsB7XohaPhtMOzRk=
+github.com/google/go-github/v50 v50.1.0/go.mod h1:Ev4Tre8QoKiolvbpOSG3FIi4Mlon3S2Nt9W5JYqKiwA=
 github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck=
 github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8=
 github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU=