Merge pull request #162 from matrix-org/kegan/jsonresponse

Use util.JSONResponse

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

@@ -40,36 +40,43 @@ type RequestAuthSession struct {
 //  {
 //      // AuthRealm-specific information
 //  }
-func (h *RequestAuthSession) OnIncomingRequest(req *http.Request) (interface{}, *util.HTTPError) {
+func (h *RequestAuthSession) OnIncomingRequest(req *http.Request) util.JSONResponse {
+	logger := util.GetLogger(req.Context())
 	if req.Method != "POST" {
-		return nil, &util.HTTPError{nil, "Unsupported Method", 405}
+		return util.MessageResponse(405, "Unsupported Method")
 	}
 	var body api.RequestAuthSessionRequest
 	if err := json.NewDecoder(req.Body).Decode(&body); err != nil {
-		return nil, &util.HTTPError{err, "Error parsing request JSON", 400}
+		return util.MessageResponse(400, "Error parsing request JSON")
 	}
-	log.WithFields(log.Fields{
+	logger.WithFields(log.Fields{
 		"realm_id": body.RealmID,
 		"user_id":  body.UserID,
 	}).Print("Incoming auth session request")
 
 	if err := body.Check(); err != nil {
-		return nil, &util.HTTPError{err, err.Error(), 400}
+		logger.WithError(err).Info("Failed Check")
+		return util.MessageResponse(400, err.Error())
 	}
 
 	realm, err := h.Db.LoadAuthRealm(body.RealmID)
 	if err != nil {
-		return nil, &util.HTTPError{err, "Unknown RealmID", 400}
+		logger.WithError(err).Info("Failed to LoadAuthRealm")
+		return util.MessageResponse(400, "Unknown RealmID")
 	}
 
 	response := realm.RequestAuthSession(body.UserID, body.Config)
 	if response == nil {
-		return nil, &util.HTTPError{nil, "Failed to request auth session", 500}
+		logger.WithField("body", body).Error("Failed to RequestAuthSession")
+		return util.MessageResponse(500, "Failed to request auth session")
 	}
 
 	metrics.IncrementAuthSession(realm.Type())
 
-	return response, nil
+	return util.JSONResponse{
+		Code: 200,
+		JSON: response,
+	}
 }
 
 // RemoveAuthSession represents an HTTP handler capable of processing /admin/removeAuthSession requests.
@@ -90,36 +97,41 @@ type RemoveAuthSession struct {
 // Response:
 //  HTTP/1.1 200 OK
 //  {}
-func (h *RemoveAuthSession) OnIncomingRequest(req *http.Request) (interface{}, *util.HTTPError) {
+func (h *RemoveAuthSession) OnIncomingRequest(req *http.Request) util.JSONResponse {
+	logger := util.GetLogger(req.Context())
 	if req.Method != "POST" {
-		return nil, &util.HTTPError{nil, "Unsupported Method", 405}
+		return util.MessageResponse(405, "Unsupported Method")
 	}
 	var body struct {
 		RealmID string
 		UserID  string
 	}
 	if err := json.NewDecoder(req.Body).Decode(&body); err != nil {
-		return nil, &util.HTTPError{err, "Error parsing request JSON", 400}
+		return util.MessageResponse(400, "Error parsing request JSON")
 	}
-	log.WithFields(log.Fields{
+	logger.WithFields(log.Fields{
 		"realm_id": body.RealmID,
 		"user_id":  body.UserID,
 	}).Print("Incoming remove auth session request")
 
 	if body.UserID == "" || body.RealmID == "" {
-		return nil, &util.HTTPError{nil, `Must supply a "UserID", a "RealmID"`, 400}
+		return util.MessageResponse(400, `Must supply a "UserID", a "RealmID"`)
 	}
 
 	_, err := h.Db.LoadAuthRealm(body.RealmID)
 	if err != nil {
-		return nil, &util.HTTPError{err, "Unknown RealmID", 400}
+		return util.MessageResponse(400, "Unknown RealmID")
 	}
 
 	if err := h.Db.RemoveAuthSession(body.RealmID, body.UserID); err != nil {
-		return nil, &util.HTTPError{err, "Failed to remove auth session", 500}
+		logger.WithError(err).Error("Failed to RemoveAuthSession")
+		return util.MessageResponse(500, "Failed to remove auth session")
 	}
 
-	return []byte(`{}`), nil
+	return util.JSONResponse{
+		Code: 200,
+		JSON: struct{}{},
+	}
 }
 
 // RealmRedirect represents an HTTP handler which can process incoming redirects for auth realms.
@@ -186,39 +198,44 @@ type ConfigureAuthRealm struct {
 //          // New auth realm config information
 //      },
 //  }
-func (h *ConfigureAuthRealm) OnIncomingRequest(req *http.Request) (interface{}, *util.HTTPError) {
+func (h *ConfigureAuthRealm) OnIncomingRequest(req *http.Request) util.JSONResponse {
+	logger := util.GetLogger(req.Context())
 	if req.Method != "POST" {
-		return nil, &util.HTTPError{nil, "Unsupported Method", 405}
+		return util.MessageResponse(405, "Unsupported Method")
 	}
 	var body api.ConfigureAuthRealmRequest
 	if err := json.NewDecoder(req.Body).Decode(&body); err != nil {
-		return nil, &util.HTTPError{err, "Error parsing request JSON", 400}
+		return util.MessageResponse(400, "Error parsing request JSON")
 	}
 
 	if err := body.Check(); err != nil {
-		return nil, &util.HTTPError{err, err.Error(), 400}
+		return util.MessageResponse(400, err.Error())
 	}
 
 	realm, err := types.CreateAuthRealm(body.ID, body.Type, body.Config)
 	if err != nil {
-		return nil, &util.HTTPError{err, "Error parsing config JSON", 400}
+		return util.MessageResponse(400, "Error parsing config JSON")
 	}
 
 	if err = realm.Register(); err != nil {
-		return nil, &util.HTTPError{err, "Error registering auth realm", 400}
+		return util.MessageResponse(400, "Error registering auth realm")
 	}
 
 	oldRealm, err := h.Db.StoreAuthRealm(realm)
 	if err != nil {
-		return nil, &util.HTTPError{err, "Error storing realm", 500}
+		logger.WithError(err).Error("Failed to StoreAuthRealm")
+		return util.MessageResponse(500, "Error storing realm")
 	}
 
-	return &struct {
-		ID        string
-		Type      string
-		OldConfig types.AuthRealm
-		NewConfig types.AuthRealm
-	}{body.ID, body.Type, oldRealm, realm}, nil
+	return util.JSONResponse{
+		Code: 200,
+		JSON: struct {
+			ID        string
+			Type      string
+			OldConfig types.AuthRealm
+			NewConfig types.AuthRealm
+		}{body.ID, body.Type, oldRealm, realm},
+	}
 }
 
 // GetSession represents an HTTP handler capable of processing /admin/getSession requests.
@@ -252,35 +269,43 @@ type GetSession struct {
 //  {
 //      "Authenticated": false
 //  }
-func (h *GetSession) OnIncomingRequest(req *http.Request) (interface{}, *util.HTTPError) {
+func (h *GetSession) OnIncomingRequest(req *http.Request) util.JSONResponse {
+	logger := util.GetLogger(req.Context())
 	if req.Method != "POST" {
-		return nil, &util.HTTPError{nil, "Unsupported Method", 405}
+		return util.MessageResponse(405, "Unsupported Method")
 	}
 	var body struct {
 		RealmID string
 		UserID  string
 	}
 	if err := json.NewDecoder(req.Body).Decode(&body); err != nil {
-		return nil, &util.HTTPError{err, "Error parsing request JSON", 400}
+		return util.MessageResponse(400, "Error parsing request JSON")
 	}
 
 	if body.RealmID == "" || body.UserID == "" {
-		return nil, &util.HTTPError{nil, `Must supply a "RealmID" and "UserID"`, 400}
+		return util.MessageResponse(400, `Must supply a "RealmID" and "UserID"`)
 	}
 
 	session, err := h.Db.LoadAuthSessionByUser(body.RealmID, body.UserID)
 	if err != nil && err != sql.ErrNoRows {
-		return nil, &util.HTTPError{err, `Failed to load session`, 500}
+		logger.WithError(err).WithField("body", body).Error("Failed to LoadAuthSessionByUser")
+		return util.MessageResponse(500, `Failed to load session`)
 	}
 	if err == sql.ErrNoRows {
-		return &struct {
-			Authenticated bool
-		}{false}, nil
+		return util.JSONResponse{
+			Code: 200,
+			JSON: struct {
+				Authenticated bool
+			}{false},
+		}
 	}
 
-	return &struct {
-		ID            string
-		Authenticated bool
-		Info          interface{}
-	}{session.ID(), session.Authenticated(), session.Info()}, nil
+	return util.JSONResponse{
+		Code: 200,
+		JSON: struct {
+			ID            string
+			Authenticated bool
+			Info          interface{}
+		}{session.ID(), session.Authenticated(), session.Info()},
+	}
 }

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

@@ -39,27 +39,31 @@ type ConfigureClient struct {
 //         // The new api.ClientConfig
 //       }
 //  }
-func (s *ConfigureClient) OnIncomingRequest(req *http.Request) (interface{}, *util.HTTPError) {
+func (s *ConfigureClient) OnIncomingRequest(req *http.Request) util.JSONResponse {
 	if req.Method != "POST" {
-		return nil, &util.HTTPError{nil, "Unsupported Method", 405}
+		return util.MessageResponse(405, "Unsupported Method")
 	}
 
 	var body api.ClientConfig
 	if err := json.NewDecoder(req.Body).Decode(&body); err != nil {
-		return nil, &util.HTTPError{err, "Error parsing request JSON", 400}
+		return util.MessageResponse(400, "Error parsing request JSON")
 	}
 
 	if err := body.Check(); err != nil {
-		return nil, &util.HTTPError{err, "Error parsing client config", 400}
+		return util.MessageResponse(400, "Error parsing client config")
 	}
 
 	oldClient, err := s.Clients.Update(body)
 	if err != nil {
-		return nil, &util.HTTPError{err, "Error storing token", 500}
+		util.GetLogger(req.Context()).WithError(err).WithField("body", body).Error("Failed to Clients.Update")
+		return util.MessageResponse(500, "Error storing token")
 	}
 
-	return &struct {
-		OldClient api.ClientConfig
-		NewClient api.ClientConfig
-	}{oldClient, body}, nil
+	return util.JSONResponse{
+		Code: 200,
+		JSON: struct {
+			OldClient api.ClientConfig
+			NewClient api.ClientConfig
+		}{oldClient, body},
+	}
 }

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

@@ -26,6 +26,9 @@ type Heartbeat struct{}
 // Response:
 //  HTTP/1.1 200 OK
 //  {}
-func (*Heartbeat) OnIncomingRequest(req *http.Request) (interface{}, *util.HTTPError) {
-	return &struct{}{}, nil
+func (*Heartbeat) OnIncomingRequest(req *http.Request) util.JSONResponse {
+	return util.JSONResponse{
+		Code: 200,
+		JSON: struct{}{},
+	}
 }

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

@@ -76,16 +76,17 @@ func (s *ConfigureService) getMutexForServiceID(serviceID string) *sync.Mutex {
 //          // new service-specific config information
 //      },
 //  }
-func (s *ConfigureService) OnIncomingRequest(req *http.Request) (interface{}, *util.HTTPError) {
+func (s *ConfigureService) OnIncomingRequest(req *http.Request) util.JSONResponse {
 	if req.Method != "POST" {
-		return nil, &util.HTTPError{nil, "Unsupported Method", 405}
+		return util.MessageResponse(405, "Unsupported Method")
 	}
 
 	service, httpErr := s.createService(req)
 	if httpErr != nil {
-		return nil, httpErr
+		return *httpErr
 	}
-	log.WithFields(log.Fields{
+	logger := util.GetLogger(req.Context())
+	logger.WithFields(log.Fields{
 		"service_id":      service.ServiceID(),
 		"service_type":    service.ServiceType(),
 		"service_user_id": service.ServiceUserID(),
@@ -98,32 +99,34 @@ func (s *ConfigureService) OnIncomingRequest(req *http.Request) (interface{}, *u
 
 	old, err := s.db.LoadService(service.ServiceID())
 	if err != nil && err != sql.ErrNoRows {
-		return nil, &util.HTTPError{err, "Error loading old service", 500}
+		logger.WithError(err).Error("Failed to LoadService")
+		return util.MessageResponse(500, "Error loading old service")
 	}
 
 	client, err := s.clients.Client(service.ServiceUserID())
 	if err != nil {
-		return nil, &util.HTTPError{err, "Unknown matrix client", 400}
+		return util.MessageResponse(400, "Unknown matrix client")
 	}
 
 	if err := checkClientForService(service, client); err != nil {
-		return nil, &util.HTTPError{err, err.Error(), 400}
+		return util.MessageResponse(400, err.Error())
 	}
 
 	if err = service.Register(old, client); err != nil {
-		return nil, &util.HTTPError{err, "Failed to register service: " + err.Error(), 500}
+		return util.MessageResponse(500, "Failed to register service: "+err.Error())
 	}
 
 	oldService, err := s.db.StoreService(service)
 	if err != nil {
-		return nil, &util.HTTPError{err, "Error storing service", 500}
+		logger.WithError(err).Error("Failed to StoreService")
+		return util.MessageResponse(500, "Error storing service")
 	}
 
 	// Start any polling NOW because they may decide to stop it in PostRegister, and we want to make
 	// sure we'll actually stop.
 	if _, ok := service.(types.Poller); ok {
 		if err := polling.StartPolling(service); err != nil {
-			log.WithFields(log.Fields{
+			logger.WithFields(log.Fields{
 				"service_id": service.ServiceID(),
 				log.ErrorKey: err,
 			}).Error("Failed to start poll loop.")
@@ -133,27 +136,33 @@ func (s *ConfigureService) OnIncomingRequest(req *http.Request) (interface{}, *u
 	service.PostRegister(old)
 	metrics.IncrementConfigureService(service.ServiceType())
 
-	return &struct {
-		ID        string
-		Type      string
-		OldConfig types.Service
-		NewConfig types.Service
-	}{service.ServiceID(), service.ServiceType(), oldService, service}, nil
+	return util.JSONResponse{
+		Code: 200,
+		JSON: struct {
+			ID        string
+			Type      string
+			OldConfig types.Service
+			NewConfig types.Service
+		}{service.ServiceID(), service.ServiceType(), oldService, service},
+	}
 }
 
-func (s *ConfigureService) createService(req *http.Request) (types.Service, *util.HTTPError) {
+func (s *ConfigureService) createService(req *http.Request) (types.Service, *util.JSONResponse) {
 	var body api.ConfigureServiceRequest
 	if err := json.NewDecoder(req.Body).Decode(&body); err != nil {
-		return nil, &util.HTTPError{err, "Error parsing request JSON", 400}
+		res := util.MessageResponse(400, "Error parsing request JSON")
+		return nil, &res
 	}
 
 	if err := body.Check(); err != nil {
-		return nil, &util.HTTPError{err, err.Error(), 400}
+		res := util.MessageResponse(400, err.Error())
+		return nil, &res
 	}
 
 	service, err := types.CreateService(body.ID, body.Type, body.UserID, body.Config)
 	if err != nil {
-		return nil, &util.HTTPError{err, "Error parsing config JSON", 400}
+		res := util.MessageResponse(400, "Error parsing config JSON")
+		return nil, &res
 	}
 	return service, nil
 }
@@ -182,34 +191,38 @@ type GetService struct {
 //          // service-specific config information
 //      }
 //  }
-func (h *GetService) OnIncomingRequest(req *http.Request) (interface{}, *util.HTTPError) {
+func (h *GetService) OnIncomingRequest(req *http.Request) util.JSONResponse {
 	if req.Method != "POST" {
-		return nil, &util.HTTPError{nil, "Unsupported Method", 405}
+		return util.MessageResponse(405, "Unsupported Method")
 	}
 	var body struct {
 		ID string
 	}
 	if err := json.NewDecoder(req.Body).Decode(&body); err != nil {
-		return nil, &util.HTTPError{err, "Error parsing request JSON", 400}
+		return util.MessageResponse(400, "Error parsing request JSON")
 	}
 
 	if body.ID == "" {
-		return nil, &util.HTTPError{nil, `Must supply a "ID"`, 400}
+		return util.MessageResponse(400, `Must supply a "ID"`)
 	}
 
 	srv, err := h.Db.LoadService(body.ID)
 	if err != nil {
 		if err == sql.ErrNoRows {
-			return nil, &util.HTTPError{err, `Service not found`, 404}
+			return util.MessageResponse(404, `Service not found`)
 		}
-		return nil, &util.HTTPError{err, `Failed to load service`, 500}
+		util.GetLogger(req.Context()).WithError(err).Error("Failed to LoadService")
+		return util.MessageResponse(500, `Failed to load service`)
 	}
 
-	return &struct {
-		ID     string
-		Type   string
-		Config types.Service
-	}{srv.ServiceID(), srv.ServiceType(), srv}, nil
+	return util.JSONResponse{
+		Code: 200,
+		JSON: struct {
+			ID     string
+			Type   string
+			Config types.Service
+		}{srv.ServiceID(), srv.ServiceType(), srv},
+	}
 }
 
 func checkClientForService(service types.Service, client *gomatrix.Client) error {

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

@@ -21,14 +21,15 @@ import (
 // matrix message to send, along with parsed repo information.
 // The secretToken, if supplied, will be used to verify the request is from
 // Github. If it isn't, an error is returned.
-func OnReceiveRequest(r *http.Request, secretToken string) (string, *github.Repository, *gomatrix.HTMLMessage, *util.HTTPError) {
+func OnReceiveRequest(r *http.Request, secretToken string) (string, *github.Repository, *gomatrix.HTMLMessage, *util.JSONResponse) {
 	// Verify the HMAC signature if NEB was configured with a secret token
 	eventType := r.Header.Get("X-GitHub-Event")
 	signatureSHA1 := r.Header.Get("X-Hub-Signature")
 	content, err := ioutil.ReadAll(r.Body)
 	if err != nil {
 		log.WithError(err).Print("Failed to read Github webhook body")
-		return "", nil, nil, &util.HTTPError{nil, "Failed to parse body", 400}
+		resErr := util.MessageResponse(400, "Failed to parse body")
+		return "", nil, nil, &resErr
 	}
 	// Verify request if a secret token has been supplied.
 	if secretToken != "" {
@@ -38,14 +39,16 @@ func OnReceiveRequest(r *http.Request, secretToken string) (string, *github.Repo
 		if err != nil {
 			log.WithError(err).WithField("X-Hub-Signature", sigHex).Print(
 				"Failed to decode signature as hex.")
-			return "", nil, nil, &util.HTTPError{nil, "Failed to decode signature", 400}
+			resErr := util.MessageResponse(400, "Failed to decode signature")
+			return "", nil, nil, &resErr
 		}
 
 		if !checkMAC([]byte(content), sigBytes, []byte(secretToken)) {
 			log.WithFields(log.Fields{
 				"X-Hub-Signature": signatureSHA1,
 			}).Print("Received Github event which failed MAC check.")
-			return "", nil, nil, &util.HTTPError{nil, "Bad signature", 403}
+			resErr := util.MessageResponse(403, "Bad signature")
+			return "", nil, nil, &resErr
 		}
 	}
 
@@ -58,13 +61,15 @@ func OnReceiveRequest(r *http.Request, secretToken string) (string, *github.Repo
 		// 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, &util.HTTPError{nil, "pong", 200}
+		res := util.MessageResponse(200, "pong")
+		return "", nil, nil, &res
 	}
 
 	htmlStr, repo, refinedType, err := parseGithubEvent(eventType, content)
 	if err != nil {
 		log.WithError(err).Print("Failed to parse github event")
-		return "", nil, nil, &util.HTTPError{nil, "Failed to parse github event", 500}
+		resErr := util.MessageResponse(500, "Failed to parse github event")
+		return "", nil, nil, &resErr
 	}
 
 	msg := gomatrix.GetHTMLMessage("m.notice", htmlStr)

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

@@ -271,8 +271,8 @@ func (s *Service) Expansions(cli *gomatrix.Client) []types.Expansion {
 func (s *Service) OnReceiveWebhook(w http.ResponseWriter, req *http.Request, cli *gomatrix.Client) {
 	eventProjectKey, event, httpErr := webhook.OnReceiveRequest(req)
 	if httpErr != nil {
-		log.WithError(httpErr).Print("Failed to handle JIRA webhook")
-		w.WriteHeader(500)
+		log.Print("Failed to handle JIRA webhook")
+		w.WriteHeader(httpErr.Code)
 		return
 	}
 	// grab base jira url

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

@@ -65,19 +65,19 @@ func RegisterHook(jrealm *jira.Realm, projects []string, userID, webhookEndpoint
 		logger.WithError(err).Print("No JIRA client exists")
 		return err // no OAuth token on this JIRA endpoint
 	}
-	wh, httpErr := getWebhook(cli, webhookEndpointURL)
-	if httpErr != nil {
-		if httpErr.Code != 403 {
-			logger.WithError(httpErr).Print("Failed to GET webhook")
-			return httpErr
+	wh, forbidden, err := getWebhook(cli, webhookEndpointURL)
+	if err != nil {
+		if !forbidden {
+			logger.WithError(err).Print("Failed to GET webhook")
+			return err
 		}
 		// User is not a JIRA admin (cannot GET webhooks)
 		// The only way this is going to end well for this request is if all the projects
 		// are PUBLIC. That is, they can be accessed directly without an access token.
-		httpErr = checkProjectsArePublic(jrealm, projects, userID)
-		if httpErr != nil {
-			logger.WithError(httpErr).Print("Failed to assert that all projects are public")
-			return httpErr
+		err = checkProjectsArePublic(jrealm, projects, userID)
+		if err != nil {
+			logger.WithError(err).Print("Failed to assert that all projects are public")
+			return err
 		}
 
 		// All projects that wish to be tracked are public, but the user cannot create
@@ -101,17 +101,19 @@ func RegisterHook(jrealm *jira.Realm, projects []string, userID, webhookEndpoint
 
 // OnReceiveRequest is called when JIRA hits NEB with an update.
 // Returns the project key and webhook event, or an error.
-func OnReceiveRequest(req *http.Request) (string, *Event, *util.HTTPError) {
+func OnReceiveRequest(req *http.Request) (string, *Event, *util.JSONResponse) {
 	// extract the JIRA webhook event JSON
 	defer req.Body.Close()
 	var whe Event
 	err := json.NewDecoder(req.Body).Decode(&whe)
 	if err != nil {
-		return "", nil, &util.HTTPError{err, "Failed to parse request JSON", 400}
+		resErr := util.MessageResponse(400, "Failed to parse request JSON")
+		return "", nil, &resErr
 	}
 
 	if err != nil {
-		return "", nil, &util.HTTPError{err, "Failed to parse JIRA URL", 400}
+		resErr := util.MessageResponse(400, "Failed to parse JIRA URL")
+		return "", nil, &resErr
 	}
 	projKey := strings.Split(whe.Issue.Key, "-")[0]
 	projKey = strings.ToUpper(projKey)
@@ -153,22 +155,21 @@ func createWebhook(jrealm *jira.Realm, webhookEndpointURL, userID string) error
 	return err
 }
 
-func getWebhook(cli *gojira.Client, webhookEndpointURL string) (*jiraWebhook, *util.HTTPError) {
+// Get an existing JIRA webhook. Returns the hook if it exists, or an error along with a bool
+// which indicates if the request to retrieve the hook is not 2xx. If it is not 2xx, it is
+// forbidden (different JIRA deployments return different codes ranging from 401/403/404/500).
+func getWebhook(cli *gojira.Client, webhookEndpointURL string) (*jiraWebhook, bool, error) {
 	req, err := cli.NewRequest("GET", "rest/webhooks/1.0/webhook", nil)
 	if err != nil {
-		return nil, &util.HTTPError{err, "Failed to prepare webhook request", 500}
+		return nil, false, fmt.Errorf("Failed to prepare webhook request")
 	}
 	var webhookList []jiraWebhook
 	res, err := cli.Do(req, &webhookList)
 	if err != nil {
-		return nil, &util.HTTPError{err, "Failed to query webhooks", 502}
+		return nil, false, fmt.Errorf("Failed to query webhooks")
 	}
 	if res.StatusCode < 200 || res.StatusCode >= 300 {
-		return nil, &util.HTTPError{
-			err,
-			fmt.Sprintf("Querying webhook returned HTTP %d", res.StatusCode),
-			403,
-		}
+		return nil, true, fmt.Errorf("Querying webhook returned HTTP %d", res.StatusCode)
 	}
 	log.Print("Retrieved ", len(webhookList), " webhooks")
 	var nebWH *jiraWebhook
@@ -178,26 +179,26 @@ func getWebhook(cli *gojira.Client, webhookEndpointURL string) (*jiraWebhook, *u
 			break
 		}
 	}
-	return nebWH, nil
+	return nebWH, false, nil
 }
 
-func checkProjectsArePublic(jrealm *jira.Realm, projects []string, userID string) *util.HTTPError {
+func checkProjectsArePublic(jrealm *jira.Realm, projects []string, userID string) error {
 	publicCli, err := jrealm.JIRAClient("", true)
 	if err != nil {
-		return &util.HTTPError{err, "Cannot create public JIRA client", 500}
+		return fmt.Errorf("Cannot create public JIRA client")
 	}
 	for _, projectKey := range projects {
 		// check you can query this project with a public client
 		req, err := publicCli.NewRequest("GET", "rest/api/2/project/"+projectKey, nil)
 		if err != nil {
-			return &util.HTTPError{err, "Failed to create project URL", 500}
+			return fmt.Errorf("Failed to create project URL for project %s", projectKey)
 		}
 		res, err := publicCli.Do(req, nil)
 		if err != nil {
-			return &util.HTTPError{err, fmt.Sprintf("Failed to query project %s", projectKey), 500}
+			return fmt.Errorf("Failed to query project %s", projectKey)
 		}
 		if res.StatusCode < 200 || res.StatusCode >= 300 {
-			return &util.HTTPError{err, fmt.Sprintf("Project %s is not public. (HTTP %d)", projectKey, res.StatusCode), 403}
+			return fmt.Errorf("Project %s is not public. (HTTP %d)", projectKey, res.StatusCode)
 		}
 	}
 	return nil

vendor/manifest

@@ -150,7 +150,7 @@
 		{
 			"importpath": "github.com/matrix-org/util",
 			"repository": "https://github.com/matrix-org/util",
-			"revision": "9b44af331cdd83d702e4f16433e47341d983c23b",
+			"revision": "ccef6dc7c24a7c896d96b433a9107b7c47ecf828",
 			"branch": "master"
 		},
 		{

vendor/src/github.com/matrix-org/util/context.go

@@ -0,0 +1,35 @@
+package util
+
+import (
+	"context"
+
+	log "github.com/Sirupsen/logrus"
+)
+
+// contextKeys is a type alias for string to namespace Context keys per-package.
+type contextKeys string
+
+// ctxValueRequestID is the key to extract the request ID for an HTTP request
+const ctxValueRequestID = contextKeys("requestid")
+
+// GetRequestID returns the request ID associated with this context, or the empty string
+// if one is not associated with this context.
+func GetRequestID(ctx context.Context) string {
+	id := ctx.Value(ctxValueRequestID)
+	if id == nil {
+		return ""
+	}
+	return id.(string)
+}
+
+// ctxValueLogger is the key to extract the logrus Logger.
+const ctxValueLogger = contextKeys("logger")
+
+// GetLogger retrieves the logrus logger from the supplied context. Returns nil if there is no logger.
+func GetLogger(ctx context.Context) *log.Entry {
+	l := ctx.Value(ctxValueLogger)
+	if l == nil {
+		return nil
+	}
+	return l.(*log.Entry)
+}

vendor/src/github.com/matrix-org/util/error.go

@@ -1,18 +0,0 @@
-package util
-
-import "fmt"
-
-// HTTPError An HTTP Error response, which may wrap an underlying native Go Error.
-type HTTPError struct {
-	WrappedError error
-	Message      string
-	Code         int
-}
-
-func (e HTTPError) Error() string {
-	var wrappedErrMsg string
-	if e.WrappedError != nil {
-		wrappedErrMsg = e.WrappedError.Error()
-	}
-	return fmt.Sprintf("%s: %d: %s", e.Message, e.Code, wrappedErrMsg)
-}

vendor/src/github.com/matrix-org/util/json.go

@@ -3,50 +3,75 @@ package util
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 	"math/rand"
 	"net/http"
 	"runtime/debug"
+	"time"
 
 	log "github.com/Sirupsen/logrus"
 )
 
-// ContextKeys is a type alias for string to namespace Context keys per-package.
-type ContextKeys string
+// JSONResponse represents an HTTP response which contains a JSON body.
+type JSONResponse struct {
+	// HTTP status code.
+	Code int
+	// JSON represents the JSON that should be serialized and sent to the client
+	JSON interface{}
+	// Headers represent any headers that should be sent to the client
+	Headers map[string]string
+}
 
-// CtxValueLogger is the key to extract the logrus Logger.
-const CtxValueLogger = ContextKeys("logger")
+// Is2xx returns true if the Code is between 200 and 299.
+func (r JSONResponse) Is2xx() bool {
+	return r.Code/100 == 2
+}
 
-// JSONRequestHandler represents an interface that must be satisfied in order to respond to incoming
-// HTTP requests with JSON. The interface returned will be marshalled into JSON to be sent to the client,
-// unless the interface is []byte in which case the bytes are sent to the client unchanged.
-// If an error is returned, a JSON error response will also be returned, unless the error code
-// is a 302 REDIRECT in which case a redirect is sent based on the Message field.
-type JSONRequestHandler interface {
-	OnIncomingRequest(req *http.Request) (interface{}, *HTTPError)
+// RedirectResponse returns a JSONResponse which 302s the client to the given location.
+func RedirectResponse(location string) JSONResponse {
+	headers := make(map[string]string)
+	headers["Location"] = location
+	return JSONResponse{
+		Code:    302,
+		JSON:    struct{}{},
+		Headers: headers,
+	}
+}
+
+// MessageResponse returns a JSONResponse with a 'message' key containing the given text.
+func MessageResponse(code int, msg string) JSONResponse {
+	return JSONResponse{
+		Code: code,
+		JSON: struct {
+			Message string `json:"message"`
+		}{msg},
+	}
 }
 
-// JSONError represents a JSON API error response
-type JSONError struct {
-	Message string `json:"message"`
+// ErrorResponse returns an HTTP 500 JSONResponse with the stringified form of the given error.
+func ErrorResponse(err error) JSONResponse {
+	return MessageResponse(500, err.Error())
+}
+
+// JSONRequestHandler represents an interface that must be satisfied in order to respond to incoming
+// HTTP requests with JSON.
+type JSONRequestHandler interface {
+	OnIncomingRequest(req *http.Request) JSONResponse
 }
 
 // Protect panicking HTTP requests from taking down the entire process, and log them using
 // the correct logger, returning a 500 with a JSON response rather than abruptly closing the
-// connection. The http.Request MUST have a CtxValueLogger.
+// connection. The http.Request MUST have a ctxValueLogger.
 func Protect(handler http.HandlerFunc) http.HandlerFunc {
 	return func(w http.ResponseWriter, req *http.Request) {
 		defer func() {
 			if r := recover(); r != nil {
-				logger := req.Context().Value(CtxValueLogger).(*log.Entry)
+				logger := req.Context().Value(ctxValueLogger).(*log.Entry)
 				logger.WithFields(log.Fields{
 					"panic": r,
 				}).Errorf(
 					"Request panicked!\n%s", debug.Stack(),
 				)
-				jsonErrorResponse(
-					w, req, &HTTPError{nil, "Internal Server Error", 500},
-				)
+				respond(w, req, MessageResponse(500, "Internal Server Error"))
 			}
 		}()
 		handler(w, req)
@@ -55,72 +80,67 @@ func Protect(handler http.HandlerFunc) http.HandlerFunc {
 
 // MakeJSONAPI creates an HTTP handler which always responds to incoming requests with JSON responses.
 // Incoming http.Requests will have a logger (with a request ID/method/path logged) attached to the Context.
-// This can be accessed via the const CtxValueLogger. The type of the logger is *log.Entry from github.com/Sirupsen/logrus
+// This can be accessed via GetLogger(Context).
 func MakeJSONAPI(handler JSONRequestHandler) http.HandlerFunc {
 	return Protect(func(w http.ResponseWriter, req *http.Request) {
-		// Set a Logger on the context
-		ctx := context.WithValue(req.Context(), CtxValueLogger, log.WithFields(log.Fields{
+		reqID := RandomString(12)
+		// Set a Logger and request ID on the context
+		ctx := context.WithValue(req.Context(), ctxValueLogger, log.WithFields(log.Fields{
 			"req.method": req.Method,
 			"req.path":   req.URL.Path,
-			"req.id":     RandomString(12),
+			"req.id":     reqID,
 		}))
+		ctx = context.WithValue(ctx, ctxValueRequestID, reqID)
 		req = req.WithContext(ctx)
 
-		logger := req.Context().Value(CtxValueLogger).(*log.Entry)
+		logger := req.Context().Value(ctxValueLogger).(*log.Entry)
 		logger.Print("Incoming request")
 
-		res, httpErr := handler.OnIncomingRequest(req)
+		res := handler.OnIncomingRequest(req)
 
 		// Set common headers returned regardless of the outcome of the request
 		w.Header().Set("Content-Type", "application/json")
 		SetCORSHeaders(w)
 
-		if httpErr != nil {
-			jsonErrorResponse(w, req, httpErr)
-			return
-		}
-
-		// if they've returned bytes as the response, then just return them rather than marshalling as JSON.
-		// This gives handlers an escape hatch if they want to return cached bytes.
-		var resBytes []byte
-		resBytes, ok := res.([]byte)
-		if !ok {
-			r, err := json.Marshal(res)
-			if err != nil {
-				jsonErrorResponse(w, req, &HTTPError{nil, "Failed to serialise response as JSON", 500})
-				return
-			}
-			resBytes = r
-		}
-		logger.Print(fmt.Sprintf("Responding (%d bytes)", len(resBytes)))
-		w.Write(resBytes)
+		respond(w, req, res)
 	})
 }
 
-func jsonErrorResponse(w http.ResponseWriter, req *http.Request, httpErr *HTTPError) {
-	logger := req.Context().Value(CtxValueLogger).(*log.Entry)
-	if httpErr.Code == 302 {
-		logger.WithField("err", httpErr.Error()).Print("Redirecting")
-		http.Redirect(w, req, httpErr.Message, 302)
-		return
-	}
-	logger.WithFields(log.Fields{
-		log.ErrorKey: httpErr,
-	}).Print("Responding with error")
+func respond(w http.ResponseWriter, req *http.Request, res JSONResponse) {
+	logger := req.Context().Value(ctxValueLogger).(*log.Entry)
 
-	w.WriteHeader(httpErr.Code) // Set response code
+	// Set custom headers
+	if res.Headers != nil {
+		for h, val := range res.Headers {
+			w.Header().Set(h, val)
+		}
+	}
 
-	r, err := json.Marshal(&JSONError{
-		Message: httpErr.Message,
-	})
+	// Marshal JSON response into raw bytes to send as the HTTP body
+	resBytes, err := json.Marshal(res.JSON)
 	if err != nil {
-		// We should never fail to marshal the JSON error response, but in this event just skip
-		// marshalling altogether
-		logger.Warn("Failed to marshal error response")
-		w.Write([]byte(`{}`))
-		return
+		logger.WithError(err).Error("Failed to marshal JSONResponse")
+		// this should never fail to be marshalled so drop err to the floor
+		res = MessageResponse(500, "Internal Server Error")
+		resBytes, _ = json.Marshal(res.JSON)
+	}
+
+	// Set status code and write the body
+	w.WriteHeader(res.Code)
+	logger.WithField("code", res.Code).Infof("Responding (%d bytes)", len(resBytes))
+	w.Write(resBytes)
+}
+
+// WithCORSOptions intercepts all OPTIONS requests and responds with CORS headers. The request handler
+// is not invoked when this happens.
+func WithCORSOptions(handler http.HandlerFunc) http.HandlerFunc {
+	return func(w http.ResponseWriter, req *http.Request) {
+		if req.Method == "OPTIONS" {
+			SetCORSHeaders(w)
+			return
+		}
+		handler(w, req)
 	}
-	w.Write(r)
 }
 
 // SetCORSHeaders sets unrestricted origin Access-Control headers on the response writer
@@ -140,3 +160,7 @@ func RandomString(n int) string {
 	}
 	return string(b)
 }
+
+func init() {
+	rand.Seed(time.Now().UTC().UnixNano())
+}

vendor/src/github.com/matrix-org/util/json_test.go

@@ -2,6 +2,7 @@ package util
 
 import (
 	"context"
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -10,10 +11,10 @@ import (
 )
 
 type MockJSONRequestHandler struct {
-	handler func(req *http.Request) (interface{}, *HTTPError)
+	handler func(req *http.Request) JSONResponse
 }
 
-func (h *MockJSONRequestHandler) OnIncomingRequest(req *http.Request) (interface{}, *HTTPError) {
+func (h *MockJSONRequestHandler) OnIncomingRequest(req *http.Request) JSONResponse {
 	return h.handler(req)
 }
 
@@ -24,22 +25,27 @@ type MockResponse struct {
 func TestMakeJSONAPI(t *testing.T) {
 	log.SetLevel(log.PanicLevel) // suppress logs in test output
 	tests := []struct {
-		Return     interface{}
-		Err        *HTTPError
+		Return     JSONResponse
 		ExpectCode int
 		ExpectJSON string
 	}{
-		{nil, &HTTPError{nil, "Everything is broken", 500}, 500, `{"message":"Everything is broken"}`},         // Error return values
-		{nil, &HTTPError{nil, "Not here", 404}, 404, `{"message":"Not here"}`},                                 // With different status codes
-		{&MockResponse{"yep"}, nil, 200, `{"foo":"yep"}`},                                                      // Success return values
-		{[]MockResponse{{"yep"}, {"narp"}}, nil, 200, `[{"foo":"yep"},{"foo":"narp"}]`},                        // Top-level array success values
-		{[]byte(`actually bytes`), nil, 200, `actually bytes`},                                                 // raw []byte escape hatch
-		{func(cannotBe, marshalled string) {}, nil, 500, `{"message":"Failed to serialise response as JSON"}`}, // impossible marshal
+		// MessageResponse return values
+		{MessageResponse(500, "Everything is broken"), 500, `{"message":"Everything is broken"}`},
+		// interface return values
+		{JSONResponse{500, MockResponse{"yep"}, nil}, 500, `{"foo":"yep"}`},
+		// Error JSON return values which fail to be marshalled should fallback to text
+		{JSONResponse{500, struct {
+			Foo interface{} `json:"foo"`
+		}{func(cannotBe, marshalled string) {}}, nil}, 500, `{"message":"Internal Server Error"}`},
+		// With different status codes
+		{JSONResponse{201, MockResponse{"narp"}, nil}, 201, `{"foo":"narp"}`},
+		// Top-level array success values
+		{JSONResponse{200, []MockResponse{{"yep"}, {"narp"}}, nil}, 200, `[{"foo":"yep"},{"foo":"narp"}]`},
 	}
 
 	for _, tst := range tests {
-		mock := MockJSONRequestHandler{func(req *http.Request) (interface{}, *HTTPError) {
-			return tst.Return, tst.Err
+		mock := MockJSONRequestHandler{func(req *http.Request) JSONResponse {
+			return tst.Return
 		}}
 		mockReq, _ := http.NewRequest("GET", "http://example.com/foo", nil)
 		mockWriter := httptest.NewRecorder()
@@ -55,10 +61,38 @@ func TestMakeJSONAPI(t *testing.T) {
 	}
 }
 
+func TestMakeJSONAPICustomHeaders(t *testing.T) {
+	mock := MockJSONRequestHandler{func(req *http.Request) JSONResponse {
+		headers := make(map[string]string)
+		headers["Custom"] = "Thing"
+		headers["X-Custom"] = "Things"
+		return JSONResponse{
+			Code:    200,
+			JSON:    MockResponse{"yep"},
+			Headers: headers,
+		}
+	}}
+	mockReq, _ := http.NewRequest("GET", "http://example.com/foo", nil)
+	mockWriter := httptest.NewRecorder()
+	handlerFunc := MakeJSONAPI(&mock)
+	handlerFunc(mockWriter, mockReq)
+	if mockWriter.Code != 200 {
+		t.Errorf("TestMakeJSONAPICustomHeaders wanted HTTP status 200, got %d", mockWriter.Code)
+	}
+	h := mockWriter.Header().Get("Custom")
+	if h != "Thing" {
+		t.Errorf("TestMakeJSONAPICustomHeaders wanted header 'Custom: Thing' , got 'Custom: %s'", h)
+	}
+	h = mockWriter.Header().Get("X-Custom")
+	if h != "Things" {
+		t.Errorf("TestMakeJSONAPICustomHeaders wanted header 'X-Custom: Things' , got 'X-Custom: %s'", h)
+	}
+}
+
 func TestMakeJSONAPIRedirect(t *testing.T) {
 	log.SetLevel(log.PanicLevel) // suppress logs in test output
-	mock := MockJSONRequestHandler{func(req *http.Request) (interface{}, *HTTPError) {
-		return nil, &HTTPError{nil, "https://matrix.org", 302}
+	mock := MockJSONRequestHandler{func(req *http.Request) JSONResponse {
+		return RedirectResponse("https://matrix.org")
 	}}
 	mockReq, _ := http.NewRequest("GET", "http://example.com/foo", nil)
 	mockWriter := httptest.NewRecorder()
@@ -73,12 +107,74 @@ func TestMakeJSONAPIRedirect(t *testing.T) {
 	}
 }
 
+func TestMakeJSONAPIError(t *testing.T) {
+	log.SetLevel(log.PanicLevel) // suppress logs in test output
+	mock := MockJSONRequestHandler{func(req *http.Request) JSONResponse {
+		err := errors.New("oops")
+		return ErrorResponse(err)
+	}}
+	mockReq, _ := http.NewRequest("GET", "http://example.com/foo", nil)
+	mockWriter := httptest.NewRecorder()
+	handlerFunc := MakeJSONAPI(&mock)
+	handlerFunc(mockWriter, mockReq)
+	if mockWriter.Code != 500 {
+		t.Errorf("TestMakeJSONAPIError wanted HTTP status 500, got %d", mockWriter.Code)
+	}
+	actualBody := mockWriter.Body.String()
+	expect := `{"message":"oops"}`
+	if actualBody != expect {
+		t.Errorf("TestMakeJSONAPIError wanted body '%s', got '%s'", expect, actualBody)
+	}
+}
+
+func TestIs2xx(t *testing.T) {
+	tests := []struct {
+		Code   int
+		Expect bool
+	}{
+		{200, true},
+		{201, true},
+		{299, true},
+		{300, false},
+		{199, false},
+		{0, false},
+		{500, false},
+	}
+	for _, test := range tests {
+		j := JSONResponse{
+			Code: test.Code,
+		}
+		actual := j.Is2xx()
+		if actual != test.Expect {
+			t.Errorf("TestIs2xx wanted %t, got %t", test.Expect, actual)
+		}
+	}
+}
+
+func TestGetLogger(t *testing.T) {
+	log.SetLevel(log.PanicLevel) // suppress logs in test output
+	entry := log.WithField("test", "yep")
+	mockReq, _ := http.NewRequest("GET", "http://example.com/foo", nil)
+	ctx := context.WithValue(mockReq.Context(), ctxValueLogger, entry)
+	mockReq = mockReq.WithContext(ctx)
+	ctxLogger := GetLogger(mockReq.Context())
+	if ctxLogger != entry {
+		t.Errorf("TestGetLogger wanted logger '%v', got '%v'", entry, ctxLogger)
+	}
+
+	noLoggerInReq, _ := http.NewRequest("GET", "http://example.com/foo", nil)
+	ctxLogger = GetLogger(noLoggerInReq.Context())
+	if ctxLogger != nil {
+		t.Errorf("TestGetLogger wanted nil logger, got '%v'", ctxLogger)
+	}
+}
+
 func TestProtect(t *testing.T) {
 	log.SetLevel(log.PanicLevel) // suppress logs in test output
 	mockWriter := httptest.NewRecorder()
 	mockReq, _ := http.NewRequest("GET", "http://example.com/foo", nil)
 	mockReq = mockReq.WithContext(
-		context.WithValue(mockReq.Context(), CtxValueLogger, log.WithField("test", "yep")),
+		context.WithValue(mockReq.Context(), ctxValueLogger, log.WithField("test", "yep")),
 	)
 	h := Protect(func(w http.ResponseWriter, req *http.Request) {
 		panic("oh noes!")
@@ -97,3 +193,47 @@ func TestProtect(t *testing.T) {
 		t.Errorf("TestProtect wanted body %s, got %s", expectBody, actualBody)
 	}
 }
+
+func TestWithCORSOptions(t *testing.T) {
+	log.SetLevel(log.PanicLevel) // suppress logs in test output
+	mockWriter := httptest.NewRecorder()
+	mockReq, _ := http.NewRequest("OPTIONS", "http://example.com/foo", nil)
+	h := WithCORSOptions(func(w http.ResponseWriter, req *http.Request) {
+		w.WriteHeader(200)
+		w.Write([]byte("yep"))
+	})
+	h(mockWriter, mockReq)
+	if mockWriter.Code != 200 {
+		t.Errorf("TestWithCORSOptions wanted HTTP status 200, got %d", mockWriter.Code)
+	}
+
+	origin := mockWriter.Header().Get("Access-Control-Allow-Origin")
+	if origin != "*" {
+		t.Errorf("TestWithCORSOptions wanted Access-Control-Allow-Origin header '*', got '%s'", origin)
+	}
+
+	// OPTIONS request shouldn't hit the handler func
+	expectBody := ""
+	actualBody := mockWriter.Body.String()
+	if actualBody != expectBody {
+		t.Errorf("TestWithCORSOptions wanted body %s, got %s", expectBody, actualBody)
+	}
+}
+
+func TestGetRequestID(t *testing.T) {
+	log.SetLevel(log.PanicLevel) // suppress logs in test output
+	reqID := "alphabetsoup"
+	mockReq, _ := http.NewRequest("GET", "http://example.com/foo", nil)
+	ctx := context.WithValue(mockReq.Context(), ctxValueRequestID, reqID)
+	mockReq = mockReq.WithContext(ctx)
+	ctxReqID := GetRequestID(mockReq.Context())
+	if reqID != ctxReqID {
+		t.Errorf("TestGetRequestID wanted request ID '%s', got '%s'", reqID, ctxReqID)
+	}
+
+	noReqIDInReq, _ := http.NewRequest("GET", "http://example.com/foo", nil)
+	ctxReqID = GetRequestID(noReqIDInReq.Context())
+	if ctxReqID != "" {
+		t.Errorf("TestGetRequestID wanted empty request ID, got '%s'", ctxReqID)
+	}
+}