Merge pull request #115 from matrix-org/kegan/fail-if-bad-user

400 if a service which needs a syncing client is configured without one

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.

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
+}

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.

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 {

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"},

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"},

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,

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"},

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,

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{}
 }