Merge pull request #13 from matrix-org/kegan/github-webhook-creation

Create webhooks in the Github Service

src/github.com/matrix-org/go-neb/api.go

@@ -213,6 +213,8 @@ func (s *configureServiceHandler) OnIncomingRequest(req *http.Request) (interfac
 		return nil, &errors.HTTPError{err, "Error storing service", 500}
 	}
 
+	service.PostRegister(oldService)
+
 	return &struct {
 		ID        string
 		Type      string

src/github.com/matrix-org/go-neb/services/echo/echo.go

@@ -14,11 +14,12 @@ type echoService struct {
 	Rooms  []string
 }
 
-func (e *echoService) ServiceUserID() string { return e.UserID }
-func (e *echoService) ServiceID() string     { return e.id }
-func (e *echoService) ServiceType() string   { return "echo" }
-func (e *echoService) RoomIDs() []string     { return e.Rooms }
-func (e *echoService) Register() error       { return nil }
+func (e *echoService) ServiceUserID() string          { return e.UserID }
+func (e *echoService) ServiceID() string              { return e.id }
+func (e *echoService) ServiceType() string            { return "echo" }
+func (e *echoService) RoomIDs() []string              { return e.Rooms }
+func (e *echoService) Register() error                { return nil }
+func (e *echoService) PostRegister(old types.Service) {}
 func (e *echoService) Plugin(roomID string) plugin.Plugin {
 	return plugin.Plugin{
 		Commands: []plugin.Command{

src/github.com/matrix-org/go-neb/services/github/github.go

@@ -22,11 +22,17 @@ import (
 var ownerRepoIssueRegex = regexp.MustCompile("([A-z0-9-_]+)/([A-z0-9-_]+)#([0-9]+)")
 
 type githubService struct {
-	id           string
-	BotUserID    string
-	GithubUserID string
-	RealmID      string
-	WebhookRooms map[string][]string // room_id => ["push","issue","pull_request"]
+	id             string
+	BotUserID      string
+	ClientUserID   string
+	RealmID        string
+	SecretToken    string
+	WebhookBaseURI string
+	Rooms          map[string]struct { // room_id => {}
+		Repos map[string]struct { // owner/repo => { events: ["push","issue","pull_request"] }
+			Events []string
+		}
+	}
 }
 
 func (s *githubService) ServiceUserID() string { return s.BotUserID }
@@ -34,7 +40,7 @@ func (s *githubService) ServiceID() string     { return s.id }
 func (s *githubService) ServiceType() string   { return "github" }
 func (s *githubService) RoomIDs() []string {
 	var keys []string
-	for k := range s.WebhookRooms {
+	for k := range s.Rooms {
 		keys = append(keys, k)
 	}
 	return keys
@@ -124,39 +130,45 @@ func (s *githubService) Plugin(roomID string) plugin.Plugin {
 	}
 }
 func (s *githubService) OnReceiveWebhook(w http.ResponseWriter, req *http.Request, cli *matrix.Client) {
-	evType, repo, msg, err := webhook.OnReceiveRequest(req, "")
+	evType, repo, msg, err := webhook.OnReceiveRequest(req, s.SecretToken)
 	if err != nil {
 		w.WriteHeader(err.Code)
 		return
 	}
 
-	for roomID, notif := range s.WebhookRooms {
-		notifyRoom := false
-		for _, notifyType := range notif {
-			if evType == notifyType {
-				notifyRoom = true
-				break
+	for roomID, roomConfig := range s.Rooms {
+		for ownerRepo, repoConfig := range roomConfig.Repos {
+			if !strings.EqualFold(*repo.FullName, ownerRepo) {
+				continue
 			}
-		}
-		if notifyRoom {
-			log.WithFields(log.Fields{
-				"type":    evType,
-				"msg":     msg,
-				"repo":    repo,
-				"room_id": roomID,
-			}).Print("Sending notification to room")
-			_, e := cli.SendMessageEvent(roomID, "m.room.message", msg)
-			if e != nil {
-				log.WithError(e).WithField("room_id", roomID).Print(
-					"Failed to send notification to room.")
+
+			notifyRoom := false
+			for _, notifyType := range repoConfig.Events {
+				if evType == notifyType {
+					notifyRoom = true
+					break
+				}
+			}
+			if notifyRoom {
+				log.WithFields(log.Fields{
+					"type":    evType,
+					"msg":     msg,
+					"repo":    repo,
+					"room_id": roomID,
+				}).Print("Sending notification to room")
+				_, e := cli.SendMessageEvent(roomID, "m.room.message", msg)
+				if e != nil {
+					log.WithError(e).WithField("room_id", roomID).Print(
+						"Failed to send notification to room.")
+				}
 			}
 		}
 	}
 	w.WriteHeader(200)
 }
 func (s *githubService) Register() error {
-	if s.RealmID == "" || s.BotUserID == "" {
-		return fmt.Errorf("RealmID and BotUserID are required")
+	if s.RealmID == "" || s.ClientUserID == "" || s.BotUserID == "" {
+		return fmt.Errorf("RealmID, BotUserID and ClientUserID are required")
 	}
 	// check realm exists
 	realm, err := database.GetServiceDB().LoadAuthRealm(s.RealmID)
@@ -167,9 +179,96 @@ func (s *githubService) Register() error {
 	if realm.Type() != "github" {
 		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.", s.ClientUserID)
+	}
+
 	return nil
 }
 
+func (s *githubService) PostRegister(oldService types.Service) {
+	cli := s.githubClientFor(s.ClientUserID, false)
+	if cli == nil {
+		log.Errorf("PostRegister: %s does not have a github session", s.ClientUserID)
+		return
+	}
+	old, ok := oldService.(*githubService)
+	if !ok {
+		log.Error("PostRegister: Provided old service is not of type GithubService")
+		return
+	}
+
+	// TODO: We should be adding webhooks in Register() then removing old hooks in PostRegister()
+	//
+	// By doing both operations in PostRegister(), if some of the requests fail we can end up in
+	// an inconsistent state. It is a lot simpler and easy to reason about this way though, so
+	// for now it will do.
+
+	// remove any existing webhooks this service created on the user's behalf
+	modifyWebhooks(old, cli, true)
+
+	// make new webhooks according to service config
+	modifyWebhooks(s, cli, false)
+}
+
+func modifyWebhooks(s *githubService, cli *github.Client, removeHooks bool) {
+	// TODO: This makes assumptions about how Go-NEB maps services to webhook endpoints.
+	//       We should factor this out to a function called GetWebhookEndpoint(Service) or something.
+	trailingSlash := ""
+	if !strings.HasSuffix(s.WebhookBaseURI, "/") {
+		trailingSlash = "/"
+	}
+	webhookEndpointURL := s.WebhookBaseURI + trailingSlash + "services/hooks/" + s.id
+
+	ownerRepoSet := make(map[string]bool)
+	for _, roomCfg := range s.Rooms {
+		for ownerRepo := range roomCfg.Repos {
+			// sanity check that it looks like 'owner/repo' as we'll split on / later
+			if strings.Count(ownerRepo, "/") != 1 {
+				log.WithField("owner_repo", ownerRepo).Print("Bad owner/repo value.")
+				continue
+			}
+			ownerRepoSet[ownerRepo] = true
+		}
+	}
+
+	for ownerRepo := range ownerRepoSet {
+		o := strings.Split(ownerRepo, "/")
+		owner := o[0]
+		repo := o[1]
+		logger := log.WithFields(log.Fields{
+			"owner": owner,
+			"repo":  repo,
+		})
+		if removeHooks {
+			removeHook(logger, cli, owner, repo, webhookEndpointURL)
+		} else {
+			// make a hook for all GH events since we'll filter it when we receive webhook requests
+			name := "web" // https://developer.github.com/v3/repos/hooks/#create-a-hook
+			cfg := map[string]interface{}{
+				"content_type": "json",
+				"url":          webhookEndpointURL,
+			}
+			if s.SecretToken != "" {
+				cfg["secret"] = s.SecretToken
+			}
+			events := []string{"push", "pull_request", "issues", "issue_comment", "pull_request_review_comment"}
+			_, _, err := cli.Repositories.CreateHook(owner, repo, &github.Hook{
+				Name:   &name,
+				Config: cfg,
+				Events: events,
+			})
+			if err != nil {
+				logger.WithError(err).Print("Failed to create webhook")
+				// continue as others may succeed
+			}
+		}
+	}
+}
+
 func (s *githubService) githubClientFor(userID string, allowUnauth bool) *github.Client {
 	token, err := getTokenForUser(s.RealmID, userID)
 	if err != nil {
@@ -206,6 +305,9 @@ func getTokenForUser(realmID, userID string) (string, error) {
 	if !ok {
 		return "", fmt.Errorf("Session is not a github session: %s", session.ID())
 	}
+	if ghSession.AccessToken == "" {
+		return "", fmt.Errorf("Github auth session for %s has not been completed.", userID)
+	}
 	return ghSession.AccessToken, nil
 }
 
@@ -238,6 +340,40 @@ func ownerRepoNumberFromText(ownerRepoNumberText string) (string, string, int, e
 	return groups[1], groups[2], num, nil
 }
 
+func removeHook(logger *log.Entry, cli *github.Client, owner, repo, webhookEndpointURL string) {
+	// Get a list of webhooks for this owner/repo and find the one which has the
+	// same endpoint URL which is what github uses to determine equivalence.
+	hooks, _, err := cli.Repositories.ListHooks(owner, repo, nil)
+	if err != nil {
+		logger.WithError(err).Print("Failed to list hooks")
+		return
+	}
+	var hook *github.Hook
+	for _, h := range hooks {
+		if h.Config["url"] == nil {
+			logger.Print("Ignoring nil config.url")
+			continue
+		}
+		hookURL, ok := h.Config["url"].(string)
+		if !ok {
+			logger.Print("Ignoring non-string config.url")
+			continue
+		}
+		if hookURL == webhookEndpointURL {
+			hook = h
+			break
+		}
+	}
+	if hook == nil {
+		return // couldn't find it
+	}
+
+	_, err = cli.Repositories.DeleteHook(owner, repo, *hook.ID)
+	if err != nil {
+		logger.WithError(err).Print("Failed to delete hook")
+	}
+}
+
 func init() {
 	types.RegisterService(func(serviceID string) types.Service {
 		return &githubService{id: serviceID}

src/github.com/matrix-org/go-neb/services/github/webhook/webhook.go

@@ -53,6 +53,13 @@ func OnReceiveRequest(r *http.Request, secretToken string) (string, *github.Repo
 		"signature":  signatureSHA1,
 	}).Print("Received Github event")
 
+	if eventType == "ping" {
+		// Github will send a "ping" event when the webhook is first created. We need
+		// to return a 200 in order for the webhook to be marked as "up" (this doesn't
+		// affect delivery, just the tick/cross status flag).
+		return "", nil, nil, &errors.HTTPError{nil, "pong", 200}
+	}
+
 	htmlStr, repo, err := parseGithubEvent(eventType, content)
 	if err != nil {
 		log.WithError(err).Print("Failed to parse github event")

src/github.com/matrix-org/go-neb/types/types.go

@@ -36,6 +36,7 @@ type Service interface {
 	Plugin(roomID string) plugin.Plugin
 	OnReceiveWebhook(w http.ResponseWriter, req *http.Request, cli *matrix.Client)
 	Register() error
+	PostRegister(oldService Service)
 }
 
 var servicesByType = map[string]func(string) Service{}