From 699b003c15bb552ce51b55c27efef77ae736f4d8 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 19 Aug 2016 17:36:58 +0100 Subject: [PATCH 1/3] Client ID is optional when creating Github services We only use it for making webhooks (to determine which GH account to load up). We need it to be optional for the global GH bot to work (which has no account and uses the account of whoever issued the !command to do the work). --- .../go-neb/services/github/github.go | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/github.com/matrix-org/go-neb/services/github/github.go b/src/github.com/matrix-org/go-neb/services/github/github.go index c2241d4..4f7dffb 100644 --- a/src/github.com/matrix-org/go-neb/services/github/github.go +++ b/src/github.com/matrix-org/go-neb/services/github/github.go @@ -26,7 +26,7 @@ type githubService struct { id string serviceUserID string webhookEndpointURL string - ClientUserID string + ClientUserID string // optional; required for webhooks RealmID string SecretToken string HandleCommands bool @@ -193,8 +193,8 @@ func (s *githubService) OnReceiveWebhook(w http.ResponseWriter, req *http.Reques w.WriteHeader(200) } func (s *githubService) Register() error { - if s.RealmID == "" || s.ClientUserID == "" { - return fmt.Errorf("RealmID and ClientUserID are required") + if s.RealmID == "" { + return fmt.Errorf("RealmID is required") } // check realm exists realm, err := database.GetServiceDB().LoadAuthRealm(s.RealmID) @@ -206,11 +206,13 @@ func (s *githubService) Register() error { return fmt.Errorf("Realm is of type '%s', not 'github'", realm.Type()) } - // In order to register the GH service, you must have authed with GH. - cli := s.githubClientFor(s.ClientUserID, false) - if cli == nil { - return fmt.Errorf( - "User %s does not have a Github auth session with realm %s.", s.ClientUserID, realm.ID()) + if s.ClientUserID != "" { + // In order to register the GH service as a client, you must have authed with GH. + cli := s.githubClientFor(s.ClientUserID, false) + if cli == nil { + return fmt.Errorf( + "User %s does not have a Github auth session with realm %s.", s.ClientUserID, realm.ID()) + } } log.Infof("%+v", s) @@ -219,6 +221,12 @@ func (s *githubService) Register() error { } func (s *githubService) PostRegister(oldService types.Service) { + // PostRegister handles creating/destroying webhooks, which is only valid if this service + // is configured on behalf of a client. + if s.ClientUserID == "" { + return + } + cli := s.githubClientFor(s.ClientUserID, false) if cli == nil { log.Errorf("PostRegister: %s does not have a github session", s.ClientUserID) From 0ad4fc6135cc608a84a21806ca7c10ccbb43956a Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 19 Aug 2016 17:43:04 +0100 Subject: [PATCH 2/3] Add TODO to parse the rest of the config to make sure no webhook config was supplied --- src/github.com/matrix-org/go-neb/services/github/github.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/github.com/matrix-org/go-neb/services/github/github.go b/src/github.com/matrix-org/go-neb/services/github/github.go index 4f7dffb..5a3e337 100644 --- a/src/github.com/matrix-org/go-neb/services/github/github.go +++ b/src/github.com/matrix-org/go-neb/services/github/github.go @@ -224,6 +224,7 @@ func (s *githubService) PostRegister(oldService types.Service) { // PostRegister handles creating/destroying webhooks, which is only valid if this service // is configured on behalf of a client. if s.ClientUserID == "" { + // TODO: We should log an error here if the service has any webhook config in Rooms return } From 569587aa6b2057d0ccf10d9f7910bdac231f659b Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 19 Aug 2016 17:45:32 +0100 Subject: [PATCH 3/3] Log for nonsensical configs --- src/github.com/matrix-org/go-neb/services/github/github.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/github.com/matrix-org/go-neb/services/github/github.go b/src/github.com/matrix-org/go-neb/services/github/github.go index 5a3e337..c21149f 100644 --- a/src/github.com/matrix-org/go-neb/services/github/github.go +++ b/src/github.com/matrix-org/go-neb/services/github/github.go @@ -224,7 +224,11 @@ func (s *githubService) PostRegister(oldService types.Service) { // PostRegister handles creating/destroying webhooks, which is only valid if this service // is configured on behalf of a client. if s.ClientUserID == "" { - // TODO: We should log an error here if the service has any webhook config in Rooms + if len(s.Rooms) != 0 { + log.WithFields(log.Fields{ + "Rooms": s.Rooms, + }).Error("Empty ClientUserID but a webhook config was supplied.") + } return }