From 019a5d0e870bf20e00662c8e4cad17a96c1e9494 Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Sat, 7 Jan 2023 04:20:04 +0100 Subject: [PATCH] Make ValidateApplicationService work with UserIDs and move to internal Signed-off-by: Sijmen Signed-off-by: Sijmen Schoon --- appservice/validate/validate.go | 163 -------------------- appservice/validate/validate_test.go | 86 ----------- clientapi/auth/login_application_service.go | 4 +- clientapi/routing/register.go | 5 +- internal/validate.go | 160 ++++++++++++++++++- internal/validate_test.go | 69 ++++++++- 6 files changed, 228 insertions(+), 259 deletions(-) delete mode 100644 appservice/validate/validate.go delete mode 100644 appservice/validate/validate_test.go diff --git a/appservice/validate/validate.go b/appservice/validate/validate.go deleted file mode 100644 index cbda5f7bb..000000000 --- a/appservice/validate/validate.go +++ /dev/null @@ -1,163 +0,0 @@ -// Copyright 2023 The Matrix.org Foundation C.I.C. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package validate - -import ( - "fmt" - "net/http" - - "github.com/matrix-org/dendrite/clientapi/jsonerror" - "github.com/matrix-org/dendrite/clientapi/userutil" - "github.com/matrix-org/dendrite/internal" - "github.com/matrix-org/dendrite/setup/config" - "github.com/matrix-org/gomatrixserverlib" - "github.com/matrix-org/util" -) - -// 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.ClientAPI, - userID string, - appservice *config.ApplicationService, -) bool { - - var local, domain, err = gomatrixserverlib.SplitID('@', userID) - if err != nil { - // Not a valid userID - return false - } - - if !cfg.Matrix.IsLocalServerName(domain) { - return false - } - - if appservice != nil { - if appservice.SenderLocalpart == local { - return true - } - - // 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(userID) { - return true - } - } - return false - } - - // Loop through all known application service's namespaces and see if any match - for _, knownAppService := range cfg.Derived.ApplicationServices { - if knownAppService.SenderLocalpart == local { - return true - } - for _, namespace := range knownAppService.NamespaceMap["users"] { - // AS namespaces are checked for validity in config - if namespace.RegexpObject.MatchString(userID) { - return true - } - } - } - return false -} - -// UsernameMatchesMultipleExclusiveNamespaces will check if a given username matches -// more than one exclusive namespace. More than one is not allowed -func UsernameMatchesMultipleExclusiveNamespaces( - cfg *config.ClientAPI, - username string, -) bool { - userID := userutil.MakeUserID(username, cfg.Matrix.ServerName) - - // Check namespaces and see if more than one match - matchCount := 0 - for _, appservice := range cfg.Derived.ApplicationServices { - if appservice.OwnsNamespaceCoveringUserId(userID) { - if matchCount++; matchCount > 1 { - return true - } - } - } - return false -} - -// UsernameMatchesExclusiveNamespaces will check if a given username matches any -// application service's exclusive users namespace -func UsernameMatchesExclusiveNamespaces( - cfg *config.ClientAPI, - username string, -) bool { - userID := userutil.MakeUserID(username, cfg.Matrix.ServerName) - return cfg.Derived.ExclusiveApplicationServicesUsernameRegexp.MatchString(userID) -} - -// validateApplicationService checks if a provided application service token -// corresponds to one that is registered. If so, then it checks if the desired -// username is within that application service's namespace. As long as these -// two requirements are met, no error will be returned. -// TODO Move somewhere better -func ValidateApplicationService( - cfg *config.ClientAPI, - username string, - accessToken string, -) (string, *util.JSONResponse) { - // Check if the token if the application service is valid with one we have - // registered in the config. - var matchedApplicationService *config.ApplicationService - for _, appservice := range cfg.Derived.ApplicationServices { - if appservice.ASToken == accessToken { - matchedApplicationService = &appservice - break - } - } - if matchedApplicationService == nil { - return "", &util.JSONResponse{ - Code: http.StatusUnauthorized, - JSON: jsonerror.UnknownToken("Supplied access_token does not match any known application service"), - } - } - - userID := userutil.MakeUserID(username, cfg.Matrix.ServerName) - - // Ensure the desired username is within at least one of the application service's namespaces. - if !UserIDIsWithinApplicationServiceNamespace(cfg, userID, matchedApplicationService) { - // If we didn't find any matches, return M_EXCLUSIVE - return "", &util.JSONResponse{ - Code: http.StatusBadRequest, - JSON: jsonerror.ASExclusive(fmt.Sprintf( - "Supplied username %s did not match any namespaces for application service ID: %s", username, matchedApplicationService.ID)), - } - } - - // Check this user does not fit multiple application service namespaces - if UsernameMatchesMultipleExclusiveNamespaces(cfg, userID) { - return "", &util.JSONResponse{ - Code: http.StatusBadRequest, - JSON: jsonerror.ASExclusive(fmt.Sprintf( - "Supplied username %s matches multiple exclusive application service namespaces. Only 1 match allowed", username)), - } - } - - // Check username application service is trying to register is valid - if err := internal.ValidateApplicationServiceUsername(username, cfg.Matrix.ServerName); err != nil { - return "", internal.UsernameResponse(err) - } - - // No errors, registration valid - return matchedApplicationService.ID, nil -} diff --git a/appservice/validate/validate_test.go b/appservice/validate/validate_test.go deleted file mode 100644 index dc3741c78..000000000 --- a/appservice/validate/validate_test.go +++ /dev/null @@ -1,86 +0,0 @@ -// Copyright 2023 The Matrix.org Foundation C.I.C. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package validate - -import ( - "regexp" - "testing" - - "github.com/matrix-org/dendrite/setup/config" -) - -// 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.Defaults(config.DefaultOpts{ - Generate: true, - Monolithic: true, - }) - fakeConfig.Global.ServerName = "localhost" - fakeConfig.ClientAPI.Derived.ApplicationServices = []config.ApplicationService{fakeApplicationService} - - // Access token is correct, user_id omitted so we are acting as SenderLocalpart - asID, resp := ValidateApplicationService(&fakeConfig.ClientAPI, fakeSenderLocalpart, "1234") - 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 - asID, resp = ValidateApplicationService(&fakeConfig.ClientAPI, fakeSenderLocalpart, "xxxx") - if resp == nil || asID == fakeID { - t.Errorf("access_token should have been marked as invalid") - } - - // Access token is correct, acting as valid user_id - asID, resp = ValidateApplicationService(&fakeConfig.ClientAPI, "_appservice_bob", "1234") - 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 - asID, resp = ValidateApplicationService(&fakeConfig.ClientAPI, "_something_else", "1234") - if resp == nil || asID == fakeID { - t.Errorf("user_id should not have been valid: @_something_else:localhost") - } -} diff --git a/clientapi/auth/login_application_service.go b/clientapi/auth/login_application_service.go index 8ffa9b867..6baf3e12e 100644 --- a/clientapi/auth/login_application_service.go +++ b/clientapi/auth/login_application_service.go @@ -17,9 +17,9 @@ package auth import ( "context" - "github.com/matrix-org/dendrite/appservice/validate" "github.com/matrix-org/dendrite/clientapi/auth/authtypes" "github.com/matrix-org/dendrite/clientapi/httputil" + "github.com/matrix-org/dendrite/internal" "github.com/matrix-org/dendrite/setup/config" "github.com/matrix-org/util" ) @@ -45,7 +45,7 @@ func (t *LoginTypeApplicationService) LoginFromJSON( return nil, nil, err } - _, err := validate.ValidateApplicationService(t.Config, r.Identifier.User, t.Token) + _, err := internal.ValidateApplicationService(t.Config, r.Identifier.User, t.Token) if err != nil { return nil, nil, err } diff --git a/clientapi/routing/register.go b/clientapi/routing/register.go index b3e617bdd..3064c2d64 100644 --- a/clientapi/routing/register.go +++ b/clientapi/routing/register.go @@ -30,7 +30,6 @@ import ( "sync" "time" - asvalidate "github.com/matrix-org/dendrite/appservice/validate" "github.com/matrix-org/dendrite/internal" "github.com/tidwall/gjson" @@ -696,7 +695,7 @@ func handleRegistrationFlow( // If an access token is provided, ignore this check this is an appservice // request and we will validate in validateApplicationService if len(cfg.Derived.ApplicationServices) != 0 && - asvalidate.UsernameMatchesExclusiveNamespaces(cfg, r.Username) { + internal.UsernameMatchesExclusiveNamespaces(cfg, r.Username) { return util.JSONResponse{ Code: http.StatusBadRequest, JSON: spec.ASExclusive("This username is reserved by an application service."), @@ -773,7 +772,7 @@ func handleApplicationServiceRegistration( // Check application service register user request is valid. // The application service's ID is returned if so. - appserviceID, err := asvalidate.ValidateApplicationService( + appserviceID, err := internal.ValidateApplicationService( cfg, r.Username, accessToken, ) if err != nil { diff --git a/internal/validate.go b/internal/validate.go index 99088f240..abe3928b5 100644 --- a/internal/validate.go +++ b/internal/validate.go @@ -20,7 +20,10 @@ import ( "net/http" "regexp" - "github.com/matrix-org/gomatrixserverlib/spec" + "github.com/matrix-org/dendrite/clientapi/jsonerror" + "github.com/matrix-org/dendrite/clientapi/userutil" + "github.com/matrix-org/dendrite/setup/config" + "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/util" ) @@ -99,11 +102,160 @@ func UsernameResponse(err error) *util.JSONResponse { } // ValidateApplicationServiceUsername returns an error if the username is invalid for an application service -func ValidateApplicationServiceUsername(localpart string, domain spec.ServerName) error { - if id := fmt.Sprintf("@%s:%s", localpart, domain); len(id) > maxUsernameLength { +func ValidateApplicationServiceUsername(localpart string, domain gomatrixserverlib.ServerName) error { + userID := userutil.MakeUserID(localpart, domain) + return ValidateApplicationServiceUserID(userID) +} + +func ValidateApplicationServiceUserID(userID string) error { + if len(userID) > maxUsernameLength { return ErrUsernameTooLong - } else if !validUsernameRegex.MatchString(localpart) { + } + + localpart, _, err := gomatrixserverlib.SplitID('@', userID) + if err != nil || !validUsernameRegex.MatchString(localpart) { return ErrUsernameInvalid } + return nil } + +// 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.ClientAPI, + userID string, + appservice *config.ApplicationService, +) bool { + var local, domain, err = gomatrixserverlib.SplitID('@', userID) + if err != nil { + // Not a valid userID + return false + } + + if !cfg.Matrix.IsLocalServerName(domain) { + return false + } + + if appservice != nil { + if appservice.SenderLocalpart == local { + return true + } + + // 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(userID) { + return true + } + } + return false + } + + // Loop through all known application service's namespaces and see if any match + for _, knownAppService := range cfg.Derived.ApplicationServices { + if knownAppService.SenderLocalpart == local { + return true + } + for _, namespace := range knownAppService.NamespaceMap["users"] { + // AS namespaces are checked for validity in config + if namespace.RegexpObject.MatchString(userID) { + return true + } + } + } + return false +} + +// usernameMatchesMultipleExclusiveNamespaces will check if a given username matches +// more than one exclusive namespace. More than one is not allowed +func userIDMatchesMultipleExclusiveNamespaces( + cfg *config.ClientAPI, + userID string, +) bool { + // Check namespaces and see if more than one match + matchCount := 0 + for _, appservice := range cfg.Derived.ApplicationServices { + if appservice.OwnsNamespaceCoveringUserId(userID) { + if matchCount++; matchCount > 1 { + return true + } + } + } + return false +} + +// UsernameMatchesExclusiveNamespaces will check if a given username matches any +// application service's exclusive users namespace +func UsernameMatchesExclusiveNamespaces( + cfg *config.ClientAPI, + username string, +) bool { + userID := userutil.MakeUserID(username, cfg.Matrix.ServerName) + return cfg.Derived.ExclusiveApplicationServicesUsernameRegexp.MatchString(userID) +} + +// validateApplicationService checks if a provided application service token +// corresponds to one that is registered. If so, then it checks if the desired +// username is within that application service's namespace. As long as these +// two requirements are met, no error will be returned. +func ValidateApplicationService( + cfg *config.ClientAPI, + username string, + accessToken string, +) (string, *util.JSONResponse) { + localpart, domain, err := userutil.ParseUsernameParam(username, cfg.Matrix) + if err != nil { + return "", &util.JSONResponse{ + Code: http.StatusUnauthorized, + JSON: jsonerror.InvalidUsername(err.Error()), + } + } + + userID := userutil.MakeUserID(localpart, domain) + + // Check if the token if the application service is valid with one we have + // registered in the config. + var matchedApplicationService *config.ApplicationService + for _, appservice := range cfg.Derived.ApplicationServices { + if appservice.ASToken == accessToken { + matchedApplicationService = &appservice + break + } + } + if matchedApplicationService == nil { + return "", &util.JSONResponse{ + Code: http.StatusUnauthorized, + JSON: jsonerror.UnknownToken("Supplied access_token does not match any known application service"), + } + } + + // Ensure the desired username is within at least one of the application service's namespaces. + if !userIDIsWithinApplicationServiceNamespace(cfg, userID, matchedApplicationService) { + // If we didn't find any matches, return M_EXCLUSIVE + return "", &util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.ASExclusive(fmt.Sprintf( + "Supplied username %s did not match any namespaces for application service ID: %s", username, matchedApplicationService.ID)), + } + } + + // Check this user does not fit multiple application service namespaces + if userIDMatchesMultipleExclusiveNamespaces(cfg, userID) { + return "", &util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.ASExclusive(fmt.Sprintf( + "Supplied username %s matches multiple exclusive application service namespaces. Only 1 match allowed", username)), + } + } + + // Check username application service is trying to register is valid + if err := ValidateApplicationServiceUserID(userID); err != nil { + return "", UsernameResponse(err) + } + + // No errors, registration valid + return matchedApplicationService.ID, nil +} diff --git a/internal/validate_test.go b/internal/validate_test.go index e3a10178f..c957a1e6d 100644 --- a/internal/validate_test.go +++ b/internal/validate_test.go @@ -3,10 +3,13 @@ package internal import ( "net/http" "reflect" + "regexp" "strings" "testing" - "github.com/matrix-org/gomatrixserverlib/spec" + "github.com/matrix-org/dendrite/clientapi/jsonerror" + "github.com/matrix-org/dendrite/setup/config" + "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/util" ) @@ -167,3 +170,67 @@ func Test_validateUsername(t *testing.T) { }) } } + +// 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.Defaults(config.DefaultOpts{ + Generate: true, + Monolithic: true, + }) + fakeConfig.Global.ServerName = "localhost" + fakeConfig.ClientAPI.Derived.ApplicationServices = []config.ApplicationService{fakeApplicationService} + + // Access token is correct, user_id omitted so we are acting as SenderLocalpart + asID, resp := ValidateApplicationService(&fakeConfig.ClientAPI, fakeSenderLocalpart, "1234") + 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 + asID, resp = ValidateApplicationService(&fakeConfig.ClientAPI, fakeSenderLocalpart, "xxxx") + if resp == nil || asID == fakeID { + t.Errorf("access_token should have been marked as invalid") + } + + // Access token is correct, acting as valid user_id + asID, resp = ValidateApplicationService(&fakeConfig.ClientAPI, "_appservice_bob", "1234") + 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 + asID, resp = ValidateApplicationService(&fakeConfig.ClientAPI, "_something_else", "1234") + if resp == nil || asID == fakeID { + t.Errorf("user_id should not have been valid: @_something_else:localhost") + } +}