Base64 encode the service ID in the webhook path

We don't put any restrictions on the format of the Service ID so we need to
base64 it to prevent potentially losing data when it is inserted as a path
parameter (e.g. due to weird escaping rules).

Also, only remove webhooks if there was an old service to begin with.

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

@@ -2,6 +2,7 @@ package main
 
 import (
 	"database/sql"
+	"encoding/base64"
 	"encoding/json"
 	log "github.com/Sirupsen/logrus"
 	"github.com/matrix-org/go-neb/clients"
@@ -126,8 +127,19 @@ type webhookHandler struct {
 
 func (wh *webhookHandler) handle(w http.ResponseWriter, req *http.Request) {
 	segments := strings.Split(req.URL.Path, "/")
-	// last path segment is the service ID which we will pass the incoming request to
-	srvID := segments[len(segments)-1]
+	// last path segment is the service ID which we will pass the incoming request to,
+	// but we've base64d it.
+	base64srvID := segments[len(segments)-1]
+	bytesSrvID, err := base64.StdEncoding.DecodeString(base64srvID)
+	srvID := string(bytesSrvID)
+	if err != nil {
+		log.WithError(err).WithField("base64_service_id", base64srvID).Print(
+			"Not a b64 encoded string",
+		)
+		w.WriteHeader(400)
+		return
+	}
+
 	service, err := wh.db.LoadService(srvID)
 	if err != nil {
 		log.WithError(err).WithField("service_id", srvID).Print("Failed to load service")

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

@@ -221,20 +221,22 @@ func (s *githubService) PostRegister(oldService types.Service) {
 		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
-	}
+	if oldService != nil {
+		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.
+		// 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)
+		// 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)

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

@@ -1,6 +1,7 @@
 package types
 
 import (
+	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"github.com/matrix-org/go-neb/matrix"
@@ -72,7 +73,9 @@ func CreateService(serviceID, serviceType string, serviceJSON []byte) (Service,
 	if f == nil {
 		return nil, errors.New("Unknown service type: " + serviceType)
 	}
-	webhookEndpointURL := baseURL + "services/hooks/" + serviceID
+
+	base64ServiceID := base64.StdEncoding.EncodeToString([]byte(serviceID))
+	webhookEndpointURL := baseURL + "services/hooks/" + base64ServiceID
 	service := f(serviceID, webhookEndpointURL)
 	if err := json.Unmarshal(serviceJSON, service); err != nil {
 		return nil, err