From 7580d930427523a59062956b6096b2d9575e965d Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 4 Nov 2016 12:57:22 +0000 Subject: [PATCH 1/2] 400 if a service which needs a syncing client is configured without one This catches a common configuration error and fails early. --- .../matrix-org/go-neb/api/handlers/service.go | 21 +++++++++++++++++++ .../matrix-org/go-neb/clients/clients.go | 5 +++-- .../matrix-org/go-neb/matrix/matrix.go | 7 +++++-- .../matrix-org/go-neb/services/echo/echo.go | 2 +- .../matrix-org/go-neb/services/giphy/giphy.go | 2 +- .../go-neb/services/github/github.go | 7 +++---- .../matrix-org/go-neb/services/guggy/guggy.go | 2 +- .../matrix-org/go-neb/services/jira/jira.go | 4 ++-- .../matrix-org/go-neb/types/service.go | 8 +++---- 9 files changed, 41 insertions(+), 17 deletions(-) diff --git a/src/github.com/matrix-org/go-neb/api/handlers/service.go b/src/github.com/matrix-org/go-neb/api/handlers/service.go index 4b32605..f33bd97 100644 --- a/src/github.com/matrix-org/go-neb/api/handlers/service.go +++ b/src/github.com/matrix-org/go-neb/api/handlers/service.go @@ -3,6 +3,7 @@ package handlers import ( "database/sql" "encoding/json" + "fmt" "net/http" "sync" @@ -11,6 +12,7 @@ import ( "github.com/matrix-org/go-neb/clients" "github.com/matrix-org/go-neb/database" "github.com/matrix-org/go-neb/errors" + "github.com/matrix-org/go-neb/matrix" "github.com/matrix-org/go-neb/metrics" "github.com/matrix-org/go-neb/polling" "github.com/matrix-org/go-neb/types" @@ -103,6 +105,10 @@ func (s *ConfigureService) OnIncomingRequest(req *http.Request) (interface{}, *e return nil, &errors.HTTPError{err, "Unknown matrix client", 400} } + if err := checkClientForService(service, client); err != nil { + return nil, &errors.HTTPError{err, err.Error(), 400} + } + if err = service.Register(old, client); err != nil { return nil, &errors.HTTPError{err, "Failed to register service: " + err.Error(), 500} } @@ -204,3 +210,18 @@ func (h *GetService) OnIncomingRequest(req *http.Request) (interface{}, *errors. Config types.Service }{srv.ServiceID(), srv.ServiceType(), srv}, nil } + +func checkClientForService(service types.Service, client *matrix.Client) error { + // If there are any commands or expansions for this Service then the service user ID + // MUST be a syncing client or else the Service will never get the incoming command/expansion! + cmds := service.Commands(client) + expans := service.Expansions(client) + if len(cmds) > 0 || len(expans) > 0 { + if !client.ClientConfig.Sync { + return fmt.Errorf( + "Service type '%s' requires a syncing client", service.ServiceType(), + ) + } + } + return nil +} diff --git a/src/github.com/matrix-org/go-neb/clients/clients.go b/src/github.com/matrix-org/go-neb/clients/clients.go index 5408928..0b4287a 100644 --- a/src/github.com/matrix-org/go-neb/clients/clients.go +++ b/src/github.com/matrix-org/go-neb/clients/clients.go @@ -201,11 +201,11 @@ func (c *Clients) onMessageEvent(client *matrix.Client, event *matrix.Event) { args = strings.Split(body[1:], " ") } - if response := runCommandForService(service.Commands(client, event.RoomID), event, args); response != nil { + if response := runCommandForService(service.Commands(client), event, args); response != nil { responses = append(responses, response) } } else { // message isn't a command, it might need expanding - expansions := runExpansionsForService(service.Expansions(client, event.RoomID), event, body) + expansions := runExpansionsForService(service.Expansions(client), event, body) responses = append(responses, expansions...) } } @@ -345,6 +345,7 @@ func (c *Clients) newClient(config api.ClientConfig) (*matrix.Client, error) { client := matrix.NewClient(c.httpClient, homeserverURL, config.AccessToken, config.UserID) client.NextBatchStorer = nextBatchStore{c.db} + client.ClientConfig = config // TODO: Check that the access token is valid for the userID by peforming // a request against the server. diff --git a/src/github.com/matrix-org/go-neb/matrix/matrix.go b/src/github.com/matrix-org/go-neb/matrix/matrix.go index bae4edb..1834ca2 100644 --- a/src/github.com/matrix-org/go-neb/matrix/matrix.go +++ b/src/github.com/matrix-org/go-neb/matrix/matrix.go @@ -15,8 +15,6 @@ import ( "bytes" "encoding/json" "fmt" - log "github.com/Sirupsen/logrus" - "github.com/matrix-org/go-neb/errors" "io" "io/ioutil" "net/http" @@ -25,6 +23,10 @@ import ( "strconv" "sync" "time" + + log "github.com/Sirupsen/logrus" + "github.com/matrix-org/go-neb/api" + "github.com/matrix-org/go-neb/errors" ) var ( @@ -58,6 +60,7 @@ type Client struct { httpClient *http.Client filterID string NextBatchStorer NextBatchStorer + ClientConfig api.ClientConfig } func (cli *Client) buildURL(urlPath ...string) string { diff --git a/src/github.com/matrix-org/go-neb/services/echo/echo.go b/src/github.com/matrix-org/go-neb/services/echo/echo.go index 939441f..6630bef 100644 --- a/src/github.com/matrix-org/go-neb/services/echo/echo.go +++ b/src/github.com/matrix-org/go-neb/services/echo/echo.go @@ -19,7 +19,7 @@ type Service struct { // Commands supported: // !echo some message // Responds with a notice of "some message". -func (e *Service) Commands(cli *matrix.Client, roomID string) []types.Command { +func (e *Service) Commands(cli *matrix.Client) []types.Command { return []types.Command{ types.Command{ Path: []string{"echo"}, diff --git a/src/github.com/matrix-org/go-neb/services/giphy/giphy.go b/src/github.com/matrix-org/go-neb/services/giphy/giphy.go index c82333d..13b64ed 100644 --- a/src/github.com/matrix-org/go-neb/services/giphy/giphy.go +++ b/src/github.com/matrix-org/go-neb/services/giphy/giphy.go @@ -50,7 +50,7 @@ type Service struct { // Commands supported: // !giphy some search query without quotes // Responds with a suitable GIF into the same room as the command. -func (s *Service) Commands(client *matrix.Client, roomID string) []types.Command { +func (s *Service) Commands(client *matrix.Client) []types.Command { return []types.Command{ types.Command{ Path: []string{"giphy"}, 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 7945dd7..a3d7208 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 @@ -30,8 +30,7 @@ var ownerRepoRegex = regexp.MustCompile(`^([A-z0-9-_]+)/([A-z0-9-_]+)$`) // Service contains the Config fields for the Github service. // -// Before you can set up a Github Service, you need to set up a Github Realm. This -// service requires a syncing client. +// Before you can set up a Github Service, you need to set up a Github Realm. // // You can set a "default repository" for a Matrix room by sending a `m.room.bot.options` state event // which has the following `content`: @@ -155,7 +154,7 @@ func (s *Service) expandIssue(roomID, userID, owner, repo string, issueNum int) // Responds with the outcome of the issue creation request. This command requires // a Github account to be linked to the Matrix user ID issuing the command. If there // is no link, it will return a Starter Link instead. -func (s *Service) Commands(cli *matrix.Client, roomID string) []types.Command { +func (s *Service) Commands(cli *matrix.Client) []types.Command { return []types.Command{ types.Command{ Path: []string{"github", "create"}, @@ -172,7 +171,7 @@ func (s *Service) Commands(cli *matrix.Client, roomID string) []types.Command { // it will also expand strings of the form: // #12 // using the default repository. -func (s *Service) Expansions(cli *matrix.Client, roomID string) []types.Expansion { +func (s *Service) Expansions(cli *matrix.Client) []types.Expansion { return []types.Expansion{ types.Expansion{ Regexp: ownerRepoIssueRegex, diff --git a/src/github.com/matrix-org/go-neb/services/guggy/guggy.go b/src/github.com/matrix-org/go-neb/services/guggy/guggy.go index fa4b5a4..e661fa3 100644 --- a/src/github.com/matrix-org/go-neb/services/guggy/guggy.go +++ b/src/github.com/matrix-org/go-neb/services/guggy/guggy.go @@ -47,7 +47,7 @@ type Service struct { // Commands supported: // !guggy some search query without quotes // Responds with a suitable GIF into the same room as the command. -func (s *Service) Commands(client *matrix.Client, roomID string) []types.Command { +func (s *Service) Commands(client *matrix.Client) []types.Command { return []types.Command{ types.Command{ Path: []string{"guggy"}, diff --git a/src/github.com/matrix-org/go-neb/services/jira/jira.go b/src/github.com/matrix-org/go-neb/services/jira/jira.go index ea59805..9c1c743 100644 --- a/src/github.com/matrix-org/go-neb/services/jira/jira.go +++ b/src/github.com/matrix-org/go-neb/services/jira/jira.go @@ -238,7 +238,7 @@ func (s *Service) expandIssue(roomID, userID string, issueKeyGroups []string) in // same project key, which project is chosen is undefined. If there // is no JIRA account linked to the Matrix user ID, it will return a Starter Link // if there is a known public project with that project key. -func (s *Service) Commands(cli *matrix.Client, roomID string) []types.Command { +func (s *Service) Commands(cli *matrix.Client) []types.Command { return []types.Command{ types.Command{ Path: []string{"jira", "create"}, @@ -255,7 +255,7 @@ func (s *Service) Commands(cli *matrix.Client, roomID string) []types.Command { // to map the project key to a realm, and subsequently the JIRA endpoint to hit. // If there are multiple projects with the same project key in the Service Config, one will // be chosen arbitrarily. -func (s *Service) Expansions(cli *matrix.Client, roomID string) []types.Expansion { +func (s *Service) Expansions(cli *matrix.Client) []types.Expansion { return []types.Expansion{ types.Expansion{ Regexp: issueKeyRegex, diff --git a/src/github.com/matrix-org/go-neb/types/service.go b/src/github.com/matrix-org/go-neb/types/service.go index e6e6611..e776cc8 100644 --- a/src/github.com/matrix-org/go-neb/types/service.go +++ b/src/github.com/matrix-org/go-neb/types/service.go @@ -34,8 +34,8 @@ type Service interface { ServiceID() string // Return the type of service. This string MUST NOT change. ServiceType() string - Commands(cli *matrix.Client, roomID string) []Command - Expansions(cli *matrix.Client, roomID string) []Expansion + Commands(cli *matrix.Client) []Command + Expansions(cli *matrix.Client) []Expansion OnReceiveWebhook(w http.ResponseWriter, req *http.Request, cli *matrix.Client) // A lifecycle function which is invoked when the service is being registered. The old service, if one exists, is provided, // along with a Client instance for ServiceUserID(). If this function returns an error, the service will not be registered @@ -82,12 +82,12 @@ func (s *DefaultService) ServiceType() string { } // Commands returns no commands. -func (s *DefaultService) Commands(cli *matrix.Client, roomID string) []Command { +func (s *DefaultService) Commands(cli *matrix.Client) []Command { return []Command{} } // Expansions returns no expansions. -func (s *DefaultService) Expansions(cli *matrix.Client, roomID string) []Expansion { +func (s *DefaultService) Expansions(cli *matrix.Client) []Expansion { return []Expansion{} } From be3b4bc22c5c078c400279139432badd53e9f0a3 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 4 Nov 2016 13:45:46 +0000 Subject: [PATCH 2/2] Mention 'syncing client' --- src/github.com/matrix-org/go-neb/api/api.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/github.com/matrix-org/go-neb/api/api.go b/src/github.com/matrix-org/go-neb/api/api.go index cbb8687..183dfd1 100644 --- a/src/github.com/matrix-org/go-neb/api/api.go +++ b/src/github.com/matrix-org/go-neb/api/api.go @@ -61,8 +61,8 @@ type ClientConfig struct { HomeserverURL string // The matrix access token to authenticate the requests with. AccessToken string - // True to start a sync stream for this user. If false, no /sync goroutine will be - // created and this client won't listen for new events from Matrix. For services + // True to start a sync stream for this user, making this a "syncing client". If false, no + // /sync goroutine will be created and this client won't listen for new events from Matrix. For services // which only SEND events into Matrix, it may be desirable to set Sync to false to reduce the // number of goroutines Go-NEB has to maintain. For services which respond to !commands, // Sync MUST be set to true in order to receive those commands.