Check userID instead of username from application services (#500)

* Check UserID instead of username from AS's. Tests.

* add tests to validateApplicationService

* Use some literals, organize URLs & checks

* Fix error messages and incorrect test

src/github.com/matrix-org/dendrite/clientapi/routing/register.go

@@ -39,6 +39,7 @@ import (
 	"github.com/matrix-org/dendrite/clientapi/auth/storage/devices"
 	"github.com/matrix-org/dendrite/clientapi/httputil"
 	"github.com/matrix-org/dendrite/clientapi/jsonerror"
+	"github.com/matrix-org/dendrite/clientapi/userutil"
 	"github.com/matrix-org/gomatrixserverlib"
 	"github.com/matrix-org/util"
 	"github.com/prometheus/client_golang/prometheus"
@@ -186,12 +187,12 @@ func validateUsername(username string) *util.JSONResponse {
 	} else if !validUsernameRegex.MatchString(username) {
 		return &util.JSONResponse{
 			Code: http.StatusBadRequest,
-			JSON: jsonerror.InvalidUsername("User ID can only contain characters a-z, 0-9, or '_-./'"),
+			JSON: jsonerror.InvalidUsername("Username can only contain characters a-z, 0-9, or '_-./'"),
 		}
 	} else if username[0] == '_' { // Regex checks its not a zero length string
 		return &util.JSONResponse{
 			Code: http.StatusBadRequest,
-			JSON: jsonerror.InvalidUsername("User ID cannot start with a '_'"),
+			JSON: jsonerror.InvalidUsername("Username cannot start with a '_'"),
 		}
 	}
 	return nil
@@ -207,7 +208,7 @@ func validateApplicationServiceUsername(username string) *util.JSONResponse {
 	} else if !validUsernameRegex.MatchString(username) {
 		return &util.JSONResponse{
 			Code: http.StatusBadRequest,
-			JSON: jsonerror.InvalidUsername("User ID can only contain characters a-z, 0-9, or '_-./'"),
+			JSON: jsonerror.InvalidUsername("Username can only contain characters a-z, 0-9, or '_-./'"),
 		}
 	}
 	return nil
@@ -296,20 +297,20 @@ func validateRecaptcha(
 	return nil
 }
 
-// UsernameIsWithinApplicationServiceNamespace checks to see if a username falls
-// within any of the namespaces of a given application service. If no
-// application service is given, it will check to see if it matches any
-// application service's namespace.
-func UsernameIsWithinApplicationServiceNamespace(
+// UserIDIsWithinApplicationServiceNamespace checks to see if a given userID
+// falls within any of the namespaces of a given Application Service. If no
+// Application Service is given, it will check to see if it matches any
+// Application Service's namespace.
+func UserIDIsWithinApplicationServiceNamespace(
 	cfg *config.Dendrite,
-	username string,
+	userID string,
 	appservice *config.ApplicationService,
 ) bool {
 	if appservice != nil {
 		// Loop through given application service's namespaces and see if any match
 		for _, namespace := range appservice.NamespaceMap["users"] {
 			// AS namespaces are checked for validity in config
-			if namespace.RegexpObject.MatchString(username) {
+			if namespace.RegexpObject.MatchString(userID) {
 				return true
 			}
 		}
@@ -320,7 +321,7 @@ func UsernameIsWithinApplicationServiceNamespace(
 	for _, knownAppservice := range cfg.Derived.ApplicationServices {
 		for _, namespace := range knownAppservice.NamespaceMap["users"] {
 			// AS namespaces are checked for validity in config
-			if namespace.RegexpObject.MatchString(username) {
+			if namespace.RegexpObject.MatchString(userID) {
 				return true
 			}
 		}
@@ -375,8 +376,10 @@ func validateApplicationService(
 		}
 	}
 
+	userID := userutil.MakeUserID(username, cfg.Matrix.ServerName)
+
 	// Ensure the desired username is within at least one of the application service's namespaces.
-	if !UsernameIsWithinApplicationServiceNamespace(cfg, username, matchedApplicationService) {
+	if !UserIDIsWithinApplicationServiceNamespace(cfg, userID, matchedApplicationService) {
 		// If we didn't find any matches, return M_EXCLUSIVE
 		return "", &util.JSONResponse{
 			Code: http.StatusBadRequest,
@@ -386,7 +389,7 @@ func validateApplicationService(
 	}
 
 	// Check this user does not fit multiple application service namespaces
-	if UsernameMatchesMultipleExclusiveNamespaces(cfg, username) {
+	if UsernameMatchesMultipleExclusiveNamespaces(cfg, userID) {
 		return "", &util.JSONResponse{
 			Code: http.StatusBadRequest,
 			JSON: jsonerror.ASExclusive(fmt.Sprintf(

src/github.com/matrix-org/dendrite/clientapi/routing/register_test.go

@@ -15,9 +15,13 @@
 package routing
 
 import (
+	"net/http"
+	"net/url"
+	"regexp"
 	"testing"
 
 	"github.com/matrix-org/dendrite/clientapi/auth/authtypes"
+	"github.com/matrix-org/dendrite/common/config"
 )
 
 var (
@@ -145,3 +149,84 @@ func TestEmptyCompletedFlows(t *testing.T) {
 		t.Error("Empty Completed Flow Stages should be a empty slice: returned ", ret, ". Should be []")
 	}
 }
+
+// This method tests validation of the provided Application Service token and
+// username that they're registering
+func TestValidationOfApplicationServices(t *testing.T) {
+	// Set up application service namespaces
+	regex := "@_appservice_.*"
+	regexp, err := regexp.Compile(regex)
+	if err != nil {
+		t.Errorf("Error compiling regex: %s", regex)
+	}
+
+	fakeNamespace := config.ApplicationServiceNamespace{
+		Exclusive:    true,
+		Regex:        regex,
+		RegexpObject: regexp,
+	}
+
+	// Create a fake application service
+	fakeID := "FakeAS"
+	fakeSenderLocalpart := "_appservice_bot"
+	fakeApplicationService := config.ApplicationService{
+		ID:              fakeID,
+		URL:             "null",
+		ASToken:         "1234",
+		HSToken:         "4321",
+		SenderLocalpart: fakeSenderLocalpart,
+		NamespaceMap: map[string][]config.ApplicationServiceNamespace{
+			"users": {fakeNamespace},
+		},
+	}
+
+	// Set up a config
+	fakeConfig := config.Dendrite{}
+	fakeConfig.Matrix.ServerName = "localhost"
+	fakeConfig.Derived.ApplicationServices = []config.ApplicationService{fakeApplicationService}
+
+	// Access token is correct, user_id omitted so we are acting as SenderLocalpart
+	URL, _ := url.Parse("http://localhost/register?access_token=1234")
+	fakeHTTPRequest := http.Request{
+		Method: "POST",
+		URL:    URL,
+	}
+	asID, resp := validateApplicationService(&fakeConfig, &fakeHTTPRequest, fakeSenderLocalpart)
+	if resp != nil || asID != fakeID {
+		t.Errorf("appservice should have validated and returned correct ID: %s", resp.JSON)
+	}
+
+	// Access token is incorrect, user_id omitted so we are acting as SenderLocalpart
+	URL, _ = url.Parse("http://localhost/register?access_token=xxxx")
+	fakeHTTPRequest = http.Request{
+		Method: "POST",
+		URL:    URL,
+	}
+	asID, resp = validateApplicationService(&fakeConfig, &fakeHTTPRequest, fakeSenderLocalpart)
+	if resp == nil || asID == fakeID {
+		t.Errorf("access_token should have been marked as invalid")
+	}
+
+	// Access token is correct, acting as valid user_id
+	URL, _ = url.Parse("http://localhost/register?access_token=1234&user_id=@_appservice_bob:localhost")
+	fakeHTTPRequest = http.Request{
+		Method: "POST",
+		URL:    URL,
+	}
+	asID, resp = validateApplicationService(&fakeConfig, &fakeHTTPRequest, "_appservice_bob")
+	if resp != nil || asID != fakeID {
+		t.Errorf("access_token and user_id should've been valid: %s", resp.JSON)
+	}
+
+	// Access token is correct, acting as invalid user_id
+	URL, _ = url.Parse("http://localhost/register?access_token=1234&user_id=@_something_else:localhost")
+	fakeHTTPRequest = http.Request{
+		Method: "POST",
+		URL:    URL,
+	}
+	asID, resp = validateApplicationService(&fakeConfig, &fakeHTTPRequest, "_something_else")
+	if resp == nil || asID == fakeID {
+		t.Errorf("user_id should not have been valid: %s",
+			fakeHTTPRequest.URL.Query().Get("user_id"))
+	}
+}