From 56f6ad136231e9ec20d990253d348b9f6aa887a9 Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Fri, 6 Jan 2023 23:14:36 +0100 Subject: [PATCH 01/26] Replace "m.login.password" with authtypes constant Signed-off-by: Sijmen Signed-off-by: Sijmen Schoon --- clientapi/routing/login.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/clientapi/routing/login.go b/clientapi/routing/login.go index bc38b83406..88a961ee1e 100644 --- a/clientapi/routing/login.go +++ b/clientapi/routing/login.go @@ -19,6 +19,8 @@ import ( "net/http" "github.com/matrix-org/dendrite/clientapi/auth" + "github.com/matrix-org/dendrite/clientapi/auth/authtypes" + "github.com/matrix-org/dendrite/clientapi/jsonerror" "github.com/matrix-org/dendrite/clientapi/userutil" "github.com/matrix-org/dendrite/setup/config" userapi "github.com/matrix-org/dendrite/userapi/api" @@ -42,10 +44,9 @@ type flow struct { func passwordLogin() flows { f := flows{} - s := flow{ - Type: "m.login.password", - } - f.Flows = append(f.Flows, s) + f.Flows = append(f.Flows, flow{ + Type: authtypes.LoginTypePassword, + }) return f } From 4899bdb66333d9bade2afb39014276e7422de6ce Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Fri, 6 Jan 2023 23:14:58 +0100 Subject: [PATCH 02/26] Add LoginTypeApplicationService to GET /login Signed-off-by: Sijmen Signed-off-by: Sijmen Schoon --- clientapi/routing/login.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/clientapi/routing/login.go b/clientapi/routing/login.go index 88a961ee1e..5de1105e8e 100644 --- a/clientapi/routing/login.go +++ b/clientapi/routing/login.go @@ -47,6 +47,12 @@ func passwordLogin() flows { f.Flows = append(f.Flows, flow{ Type: authtypes.LoginTypePassword, }) + + // TODO: Add config option to disable + f.Flows = append(f.Flows, flow{ + Type: authtypes.LoginTypeApplicationService, + }) + return f } From 29bf74483279cf18f329979bf4146985fb132e24 Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Sat, 7 Jan 2023 02:04:27 +0100 Subject: [PATCH 03/26] Move ValidateApplicationService to a separate package for reuse Signed-off-by: Sijmen Signed-off-by: Sijmen Schoon --- appservice/validate/validate.go | 163 +++++++++++++++++++++++++++ appservice/validate/validate_test.go | 86 ++++++++++++++ clientapi/routing/register.go | 5 +- clientapi/routing/register_test.go | 3 - 4 files changed, 252 insertions(+), 5 deletions(-) create mode 100644 appservice/validate/validate.go create mode 100644 appservice/validate/validate_test.go diff --git a/appservice/validate/validate.go b/appservice/validate/validate.go new file mode 100644 index 0000000000..cbda5f7bbd --- /dev/null +++ b/appservice/validate/validate.go @@ -0,0 +1,163 @@ +// 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 new file mode 100644 index 0000000000..dc3741c78a --- /dev/null +++ b/appservice/validate/validate_test.go @@ -0,0 +1,86 @@ +// 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/routing/register.go b/clientapi/routing/register.go index 565c415332..b3e617bdde 100644 --- a/clientapi/routing/register.go +++ b/clientapi/routing/register.go @@ -30,6 +30,7 @@ import ( "sync" "time" + asvalidate "github.com/matrix-org/dendrite/appservice/validate" "github.com/matrix-org/dendrite/internal" "github.com/tidwall/gjson" @@ -695,7 +696,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 && - UsernameMatchesExclusiveNamespaces(cfg, r.Username) { + asvalidate.UsernameMatchesExclusiveNamespaces(cfg, r.Username) { return util.JSONResponse{ Code: http.StatusBadRequest, JSON: spec.ASExclusive("This username is reserved by an application service."), @@ -772,7 +773,7 @@ func handleApplicationServiceRegistration( // Check application service register user request is valid. // The application service's ID is returned if so. - appserviceID, err := validateApplicationService( + appserviceID, err := asvalidate.ValidateApplicationService( cfg, r.Username, accessToken, ) if err != nil { diff --git a/clientapi/routing/register_test.go b/clientapi/routing/register_test.go index 2a88ec3806..c92f53fa9a 100644 --- a/clientapi/routing/register_test.go +++ b/clientapi/routing/register_test.go @@ -22,7 +22,6 @@ import ( "net/http" "net/http/httptest" "reflect" - "regexp" "strings" "testing" "time" @@ -32,8 +31,6 @@ import ( "github.com/matrix-org/dendrite/internal/caching" "github.com/matrix-org/dendrite/internal/sqlutil" "github.com/matrix-org/dendrite/roomserver" - "github.com/matrix-org/dendrite/setup/config" - "github.com/matrix-org/dendrite/setup/jetstream" "github.com/matrix-org/dendrite/test" "github.com/matrix-org/dendrite/test/testrig" "github.com/matrix-org/dendrite/userapi" From 20b0bfaac0ae0c803a20801b4e5e759b81f9d312 Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Sat, 7 Jan 2023 02:05:09 +0100 Subject: [PATCH 04/26] Implement POST /login for LoginTypeApplicationService Signed-off-by: Sijmen Signed-off-by: Sijmen Schoon --- clientapi/auth/login.go | 22 +++++++-- clientapi/auth/login_application_service.go | 55 +++++++++++++++++++++ 2 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 clientapi/auth/login_application_service.go diff --git a/clientapi/auth/login.go b/clientapi/auth/login.go index 77835614ea..2a63629fb0 100644 --- a/clientapi/auth/login.go +++ b/clientapi/auth/login.go @@ -15,7 +15,6 @@ package auth import ( - "context" "encoding/json" "io" "net/http" @@ -32,8 +31,13 @@ import ( // called after authorization has completed, with the result of the authorization. // If the final return value is non-nil, an error occurred and the cleanup function // is nil. -func LoginFromJSONReader(ctx context.Context, r io.Reader, useraccountAPI uapi.UserLoginAPI, userAPI UserInternalAPIForLogin, cfg *config.ClientAPI) (*Login, LoginCleanupFunc, *util.JSONResponse) { - reqBytes, err := io.ReadAll(r) +func LoginFromJSONReader( + req *http.Request, + useraccountAPI uapi.UserLoginAPI, + userAPI UserInternalAPIForLogin, + cfg *config.ClientAPI, +) (*Login, LoginCleanupFunc, *util.JSONResponse) { + reqBytes, err := io.ReadAll(req.Body) if err != nil { err := &util.JSONResponse{ Code: http.StatusBadRequest, @@ -65,6 +69,16 @@ func LoginFromJSONReader(ctx context.Context, r io.Reader, useraccountAPI uapi.U UserAPI: userAPI, Config: cfg, } + case authtypes.LoginTypeApplicationService: + token, err := ExtractAccessToken(req) + if err != nil { + token = "" + } + + typ = &LoginTypeApplicationService{ + Config: cfg, + Token: token, + } default: err := util.JSONResponse{ Code: http.StatusBadRequest, @@ -73,7 +87,7 @@ func LoginFromJSONReader(ctx context.Context, r io.Reader, useraccountAPI uapi.U return nil, nil, &err } - return typ.LoginFromJSON(ctx, reqBytes) + return typ.LoginFromJSON(req.Context(), reqBytes) } // UserInternalAPIForLogin contains the aspects of UserAPI required for logging in. diff --git a/clientapi/auth/login_application_service.go b/clientapi/auth/login_application_service.go new file mode 100644 index 0000000000..8ffa9b8672 --- /dev/null +++ b/clientapi/auth/login_application_service.go @@ -0,0 +1,55 @@ +// 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 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/setup/config" + "github.com/matrix-org/util" +) + +// LoginTypeApplicationService describes how to authenticate as an +// application service +type LoginTypeApplicationService struct { + Config *config.ClientAPI + Token string +} + +// Name implements Type +func (t *LoginTypeApplicationService) Name() string { + return authtypes.LoginTypeApplicationService +} + +// LoginFromJSON implements Type +func (t *LoginTypeApplicationService) LoginFromJSON( + ctx context.Context, reqBytes []byte, +) (*Login, LoginCleanupFunc, *util.JSONResponse) { + var r Login + if err := httputil.UnmarshalJSON(reqBytes, &r); err != nil { + return nil, nil, err + } + + _, err := validate.ValidateApplicationService(t.Config, r.Identifier.User, t.Token) + if err != nil { + return nil, nil, err + } + + cleanup := func(ctx context.Context, j *util.JSONResponse) {} + return &r, cleanup, nil +} From 019a5d0e870bf20e00662c8e4cad17a96c1e9494 Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Sat, 7 Jan 2023 04:20:04 +0100 Subject: [PATCH 05/26] 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 cbda5f7bbd..0000000000 --- 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 dc3741c78a..0000000000 --- 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 8ffa9b8672..6baf3e12ed 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 b3e617bdde..3064c2d640 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 99088f2403..abe3928b5a 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 e3a10178fa..c957a1e6d2 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") + } +} From 9bef2876b47da23f9709dae7658ea2915f9a224c Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Sun, 8 Jan 2023 04:09:20 +0100 Subject: [PATCH 06/26] Add tests for m.login.application_service Signed-off-by: Sijmen Schoon --- clientapi/auth/login.go | 6 +- clientapi/auth/login_test.go | 141 +++++++++++++++++++++++++---- clientapi/auth/user_interactive.go | 2 +- clientapi/routing/login.go | 2 +- 4 files changed, 128 insertions(+), 23 deletions(-) diff --git a/clientapi/auth/login.go b/clientapi/auth/login.go index 2a63629fb0..96310f0dfb 100644 --- a/clientapi/auth/login.go +++ b/clientapi/auth/login.go @@ -72,7 +72,11 @@ func LoginFromJSONReader( case authtypes.LoginTypeApplicationService: token, err := ExtractAccessToken(req) if err != nil { - token = "" + err := &util.JSONResponse{ + Code: http.StatusForbidden, + JSON: jsonerror.MissingToken(err.Error()), + } + return nil, nil, err } typ = &LoginTypeApplicationService{ diff --git a/clientapi/auth/login_test.go b/clientapi/auth/login_test.go index 93d3e2713a..b319918512 100644 --- a/clientapi/auth/login_test.go +++ b/clientapi/auth/login_test.go @@ -18,6 +18,7 @@ import ( "context" "net/http" "reflect" + "regexp" "strings" "testing" @@ -29,12 +30,49 @@ import ( "github.com/matrix-org/util" ) +var cfg = &config.ClientAPI{ + Matrix: &config.Global{ + SigningIdentity: gomatrixserverlib.SigningIdentity{ + ServerName: serverName, + }, + }, + Derived: &config.Derived{ + ApplicationServices: []config.ApplicationService{ + { + ID: "anapplicationservice", + ASToken: "astoken", + NamespaceMap: map[string][]config.ApplicationServiceNamespace{ + "users": { + { + Exclusive: true, + Regex: "@alice:example.com", + RegexpObject: regexp.MustCompile("@alice:example.com"), + }, + }, + }, + }, + }, + }, +} + +func genHTTPRequest(ctx context.Context, body string, token string) (*http.Request, error) { + req, err := http.NewRequestWithContext(ctx, "POST", "", strings.NewReader(body)) + if err != nil { + return nil, err + } + if token != "" { + req.Header.Add("Authorization", "Bearer "+token) + } + return req, err +} + func TestLoginFromJSONReader(t *testing.T) { ctx := context.Background() tsts := []struct { - Name string - Body string + Name string + Body string + Token string WantUsername string WantDeviceID string @@ -62,21 +100,45 @@ func TestLoginFromJSONReader(t *testing.T) { WantDeviceID: "adevice", WantDeletedTokens: []string{"atoken"}, }, + { + Name: "appServiceWorksUserID", + Body: `{ + "type": "m.login.application_service", + "identifier": { "type": "m.id.user", "user": "@alice:example.com" }, + "device_id": "adevice" + }`, + Token: "astoken", + + WantUsername: "@alice:example.com", + WantDeviceID: "adevice", + }, + { + Name: "appServiceWorksLocalpart", + Body: `{ + "type": "m.login.application_service", + "identifier": { "type": "m.id.user", "user": "alice" }, + "device_id": "adevice" + }`, + Token: "astoken", + + WantUsername: "alice", + WantDeviceID: "adevice", + }, } for _, tst := range tsts { t.Run(tst.Name, func(t *testing.T) { var userAPI fakeUserInternalAPI - cfg := &config.ClientAPI{ - Matrix: &config.Global{ - SigningIdentity: fclient.SigningIdentity{ - ServerName: serverName, - }, - }, - } - login, cleanup, err := LoginFromJSONReader(ctx, strings.NewReader(tst.Body), &userAPI, &userAPI, cfg) + + req, err := genHTTPRequest(ctx, tst.Body, tst.Token) if err != nil { - t.Fatalf("LoginFromJSONReader failed: %+v", err) + t.Fatalf("genHTTPRequest failed: %v", err) + } + + login, cleanup, jsonErr := LoginFromJSONReader(req, &userAPI, &userAPI, cfg) + if jsonErr != nil { + t.Fatalf("LoginFromJSONReader failed: %+v", jsonErr) } + cleanup(ctx, &util.JSONResponse{Code: http.StatusOK}) if login.Username() != tst.WantUsername { @@ -104,8 +166,9 @@ func TestBadLoginFromJSONReader(t *testing.T) { ctx := context.Background() tsts := []struct { - Name string - Body string + Name string + Body string + Token string WantErrCode spec.MatrixErrorCode }{ @@ -142,18 +205,56 @@ func TestBadLoginFromJSONReader(t *testing.T) { }`, WantErrCode: spec.ErrorInvalidParam, }, + { + Name: "noASToken", + Body: `{ + "type": "m.login.application_service", + "identifier": { "type": "m.id.user", "user": "@alice:example.com" }, + "device_id": "adevice" + }`, + WantErrCode: "M_MISSING_TOKEN", + }, + { + Name: "badASToken", + Token: "badastoken", + Body: `{ + "type": "m.login.application_service", + "identifier": { "type": "m.id.user", "user": "@alice:example.com" }, + "device_id": "adevice" + }`, + WantErrCode: "M_UNKNOWN_TOKEN", + }, + { + Name: "badASNamespace", + Token: "astoken", + Body: `{ + "type": "m.login.application_service", + "identifier": { "type": "m.id.user", "user": "@bob:example.com" }, + "device_id": "adevice" + }`, + WantErrCode: "M_EXCLUSIVE", + }, + { + Name: "badASUserID", + Token: "astoken", + Body: `{ + "type": "m.login.application_service", + "identifier": { "type": "m.id.user", "user": "@alice:wrong.example.com" }, + "device_id": "adevice" + }`, + WantErrCode: "M_INVALID_USERNAME", + }, } for _, tst := range tsts { t.Run(tst.Name, func(t *testing.T) { var userAPI fakeUserInternalAPI - cfg := &config.ClientAPI{ - Matrix: &config.Global{ - SigningIdentity: fclient.SigningIdentity{ - ServerName: serverName, - }, - }, + + req, err := genHTTPRequest(ctx, tst.Body, tst.Token) + if err != nil { + t.Fatalf("genHTTPRequest failed: %v", err) } - _, cleanup, errRes := LoginFromJSONReader(ctx, strings.NewReader(tst.Body), &userAPI, &userAPI, cfg) + + _, cleanup, errRes := LoginFromJSONReader(req, &userAPI, &userAPI, cfg) if errRes == nil { cleanup(ctx, nil) t.Fatalf("LoginFromJSONReader err: got %+v, want code %q", errRes, tst.WantErrCode) diff --git a/clientapi/auth/user_interactive.go b/clientapi/auth/user_interactive.go index 92d83ad291..9831450ccd 100644 --- a/clientapi/auth/user_interactive.go +++ b/clientapi/auth/user_interactive.go @@ -55,7 +55,7 @@ type LoginCleanupFunc func(context.Context, *util.JSONResponse) // https://matrix.org/docs/spec/client_server/r0.6.1#identifier-types type LoginIdentifier struct { Type string `json:"type"` - // when type = m.id.user + // when type = m.id.user or m.id.application_service User string `json:"user"` // when type = m.id.thirdparty Medium string `json:"medium"` diff --git a/clientapi/routing/login.go b/clientapi/routing/login.go index 5de1105e8e..b440948c42 100644 --- a/clientapi/routing/login.go +++ b/clientapi/routing/login.go @@ -68,7 +68,7 @@ func Login( JSON: passwordLogin(), } } else if req.Method == http.MethodPost { - login, cleanup, authErr := auth.LoginFromJSONReader(req.Context(), req.Body, userAPI, userAPI, cfg) + login, cleanup, authErr := auth.LoginFromJSONReader(req, userAPI, userAPI, cfg) if authErr != nil { return *authErr } From 55bc0f680786409c7b1a05230370d38cb72d683a Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Tue, 10 Jan 2023 21:49:39 +0100 Subject: [PATCH 07/26] Improve ValidateApplicationService test coverage --- internal/validate_test.go | 71 ++++++++++++++++++++++++++++----------- 1 file changed, 52 insertions(+), 19 deletions(-) diff --git a/internal/validate_test.go b/internal/validate_test.go index c957a1e6d2..582e6562a5 100644 --- a/internal/validate_test.go +++ b/internal/validate_test.go @@ -174,24 +174,16 @@ 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 + // Create a fake application service 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, + RegexpObject: regexp.MustCompile(regex), } - - // Create a fake application service - fakeID := "FakeAS" fakeSenderLocalpart := "_appservice_bot" fakeApplicationService := config.ApplicationService{ - ID: fakeID, + ID: "FakeAS", URL: "null", ASToken: "1234", HSToken: "4321", @@ -201,6 +193,25 @@ func TestValidationOfApplicationServices(t *testing.T) { }, } + // Create a second fake application service where userIDs ending in + // "_overlap" overlap with the first. + regex = "@.*_overlap" + fakeNamespace = config.ApplicationServiceNamespace{ + Exclusive: true, + Regex: regex, + RegexpObject: regexp.MustCompile(regex), + } + fakeApplicationServiceOverlap := config.ApplicationService{ + ID: "FakeASOverlap", + URL: fakeApplicationService.URL, + ASToken: fakeApplicationService.ASToken, + HSToken: fakeApplicationService.HSToken, + SenderLocalpart: "_appservice_bot_overlap", + NamespaceMap: map[string][]config.ApplicationServiceNamespace{ + "users": {fakeNamespace}, + }, + } + // Set up a config fakeConfig := &config.Dendrite{} fakeConfig.Defaults(config.DefaultOpts{ @@ -208,29 +219,51 @@ func TestValidationOfApplicationServices(t *testing.T) { Monolithic: true, }) fakeConfig.Global.ServerName = "localhost" - fakeConfig.ClientAPI.Derived.ApplicationServices = []config.ApplicationService{fakeApplicationService} + fakeConfig.ClientAPI.Derived.ApplicationServices = []config.ApplicationService{fakeApplicationService, fakeApplicationServiceOverlap} // 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 { + asID, resp := ValidateApplicationService(&fakeConfig.ClientAPI, fakeSenderLocalpart, fakeApplicationService.ASToken) + if resp != nil || asID != fakeApplicationService.ID { 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 { + if resp == nil || asID == fakeApplicationService.ID { 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 { + asID, resp = ValidateApplicationService(&fakeConfig.ClientAPI, "_appservice_bob", fakeApplicationService.ASToken) + if resp != nil || asID != fakeApplicationService.ID { 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 { + asID, resp = ValidateApplicationService(&fakeConfig.ClientAPI, "_something_else", fakeApplicationService.ASToken) + if resp == nil || asID == fakeApplicationService.ID { t.Errorf("user_id should not have been valid: @_something_else:localhost") } + + // Access token is correct, acting as user_id that matches two exclusive namespaces + asID, resp = ValidateApplicationService(&fakeConfig.ClientAPI, "_appservice_overlap", fakeApplicationService.ASToken) + if resp == nil || asID == fakeApplicationService.ID { + t.Errorf("user_id should not have been valid: @_appservice_overlap:localhost") + } + + // Access token is correct, acting as matching user_id that is too long + asID, resp = ValidateApplicationService( + &fakeConfig.ClientAPI, + "_appservice_"+strings.Repeat("a", maxUsernameLength), + fakeApplicationService.ASToken, + ) + if resp == nil || asID == fakeApplicationService.ID { + t.Errorf("user_id exceeding maxUsernameLength should not have been valid") + } + + // Access token is correct, acting as user_id that matches but is invalid + asID, resp = ValidateApplicationService(&fakeConfig.ClientAPI, "@_appservice_bob::", fakeApplicationService.ASToken) + if resp == nil || asID == fakeApplicationService.ID { + t.Errorf("user_id should not have been valid: @_appservice_bob::") + } } From aafb13f6167489cb1d1aeed370ae87bae19279bb Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Tue, 10 Jan 2023 22:32:52 +0100 Subject: [PATCH 08/26] Remove dead, untested code from userIDIsWithinApplicationServiceNamespace --- internal/validate.go | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/internal/validate.go b/internal/validate.go index abe3928b5a..2526fc1190 100644 --- a/internal/validate.go +++ b/internal/validate.go @@ -139,33 +139,18 @@ func userIDIsWithinApplicationServiceNamespace( 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 + if appservice.SenderLocalpart == local { + return true } - // Loop through all known application service's namespaces and see if any match - for _, knownAppService := range cfg.Derived.ApplicationServices { - if knownAppService.SenderLocalpart == local { + // 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 } - for _, namespace := range knownAppService.NamespaceMap["users"] { - // AS namespaces are checked for validity in config - if namespace.RegexpObject.MatchString(userID) { - return true - } - } } + return false } From d46267f9ef591cbf501367e0a89d1d2a0bef67b8 Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Sun, 15 Jan 2023 14:33:47 +0100 Subject: [PATCH 09/26] Simplify GET /login and add test for it --- clientapi/routing/login.go | 23 +++++++---------------- clientapi/routing/login_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 16 deletions(-) diff --git a/clientapi/routing/login.go b/clientapi/routing/login.go index b440948c42..b6e984bf01 100644 --- a/clientapi/routing/login.go +++ b/clientapi/routing/login.go @@ -42,30 +42,21 @@ type flow struct { Type string `json:"type"` } -func passwordLogin() flows { - f := flows{} - f.Flows = append(f.Flows, flow{ - Type: authtypes.LoginTypePassword, - }) - - // TODO: Add config option to disable - f.Flows = append(f.Flows, flow{ - Type: authtypes.LoginTypeApplicationService, - }) - - return f -} - // Login implements GET and POST /login func Login( req *http.Request, userAPI userapi.ClientUserAPI, cfg *config.ClientAPI, ) util.JSONResponse { if req.Method == http.MethodGet { - // TODO: support other forms of login other than password, depending on config options + // TODO: support other forms of login, depending on config options return util.JSONResponse{ Code: http.StatusOK, - JSON: passwordLogin(), + JSON: flows{ + Flows: []flow{ + {Type: authtypes.LoginTypePassword}, + {Type: authtypes.LoginTypeApplicationService}, + }, + }, } } else if req.Method == http.MethodPost { login, cleanup, authErr := auth.LoginFromJSONReader(req, userAPI, userAPI, cfg) diff --git a/clientapi/routing/login_test.go b/clientapi/routing/login_test.go index bff676826d..bb531a1d7f 100644 --- a/clientapi/routing/login_test.go +++ b/clientapi/routing/login_test.go @@ -113,6 +113,39 @@ func TestLogin(t *testing.T) { ctx := context.Background() + t.Run("Supported log-in flows are returned", func(t *testing.T) { + req := test.NewRequest(t, http.MethodGet, "/_matrix/client/v3/login") + rec := httptest.NewRecorder() + base.PublicClientAPIMux.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("failed to get log-in flows: %s", rec.Body.String()) + } + + t.Logf("response: %s", rec.Body.String()) + resp := flows{} + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatal(err) + } + + appServiceFound := false + passwordFound := false + for _, flow := range resp.Flows { + if flow.Type == "m.login.password" { + passwordFound = true + } else if flow.Type == "m.login.application_service" { + appServiceFound = true + } else { + t.Fatalf("got unknown login flow: %s", flow.Type) + } + } + if !appServiceFound { + t.Fatal("m.login.application_service missing from login flows") + } + if !passwordFound { + t.Fatal("m.login.password missing from login flows") + } + }) + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { req := test.NewRequest(t, http.MethodPost, "/_matrix/client/v3/login", test.WithJSONBody(t, map[string]interface{}{ From a5f4c5e466ef3e8c939ac16eab4609287b31dc35 Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Fri, 27 Jan 2023 15:14:17 +0100 Subject: [PATCH 10/26] Fix typo "POSTGERS" in CONTRIBUTING.md --- docs/development/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/development/CONTRIBUTING.md b/docs/development/CONTRIBUTING.md index 71e7516a29..caab1e749f 100644 --- a/docs/development/CONTRIBUTING.md +++ b/docs/development/CONTRIBUTING.md @@ -109,7 +109,7 @@ To configure the connection to a remote Postgres, you can use the following envi ```bash POSTGRES_USER=postgres -POSTGERS_PASSWORD=yourPostgresPassword +POSTGRES_PASSWORD=yourPostgresPassword POSTGRES_HOST=localhost POSTGRES_DB=postgres # the superuser database to use ``` From 4cca740ecf076f51c8ec7bcc0338f99c14d142cb Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Fri, 27 Jan 2023 15:14:57 +0100 Subject: [PATCH 11/26] Replace handcrafted genHTTPRequest with httptest.NewRequest --- clientapi/auth/login_test.go | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/clientapi/auth/login_test.go b/clientapi/auth/login_test.go index b319918512..3aa7c537a4 100644 --- a/clientapi/auth/login_test.go +++ b/clientapi/auth/login_test.go @@ -17,6 +17,7 @@ package auth import ( "context" "net/http" + "net/http/httptest" "reflect" "regexp" "strings" @@ -55,17 +56,6 @@ var cfg = &config.ClientAPI{ }, } -func genHTTPRequest(ctx context.Context, body string, token string) (*http.Request, error) { - req, err := http.NewRequestWithContext(ctx, "POST", "", strings.NewReader(body)) - if err != nil { - return nil, err - } - if token != "" { - req.Header.Add("Authorization", "Bearer "+token) - } - return req, err -} - func TestLoginFromJSONReader(t *testing.T) { ctx := context.Background() @@ -129,9 +119,9 @@ func TestLoginFromJSONReader(t *testing.T) { t.Run(tst.Name, func(t *testing.T) { var userAPI fakeUserInternalAPI - req, err := genHTTPRequest(ctx, tst.Body, tst.Token) - if err != nil { - t.Fatalf("genHTTPRequest failed: %v", err) + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(tst.Body)) + if tst.Token != "" { + req.Header.Add("Authorization", "Bearer "+tst.Token) } login, cleanup, jsonErr := LoginFromJSONReader(req, &userAPI, &userAPI, cfg) @@ -249,9 +239,9 @@ func TestBadLoginFromJSONReader(t *testing.T) { t.Run(tst.Name, func(t *testing.T) { var userAPI fakeUserInternalAPI - req, err := genHTTPRequest(ctx, tst.Body, tst.Token) - if err != nil { - t.Fatalf("genHTTPRequest failed: %v", err) + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(tst.Body)) + if tst.Token != "" { + req.Header.Add("Authorization", "Bearer "+tst.Token) } _, cleanup, errRes := LoginFromJSONReader(req, &userAPI, &userAPI, cfg) From 923f47c3a89603fde5fb69046681233df236ee20 Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Fri, 27 Jan 2023 16:08:17 +0100 Subject: [PATCH 12/26] Improve documentation and naming --- clientapi/auth/login_application_service.go | 2 +- clientapi/routing/register.go | 2 +- internal/validate.go | 39 +++++++++------------ 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/clientapi/auth/login_application_service.go b/clientapi/auth/login_application_service.go index 6baf3e12ed..dd4a9cbb48 100644 --- a/clientapi/auth/login_application_service.go +++ b/clientapi/auth/login_application_service.go @@ -45,7 +45,7 @@ func (t *LoginTypeApplicationService) LoginFromJSON( return nil, nil, err } - _, err := internal.ValidateApplicationService(t.Config, r.Identifier.User, t.Token) + _, err := internal.ValidateApplicationServiceRequest(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 3064c2d640..6dfebead57 100644 --- a/clientapi/routing/register.go +++ b/clientapi/routing/register.go @@ -772,7 +772,7 @@ func handleApplicationServiceRegistration( // Check application service register user request is valid. // The application service's ID is returned if so. - appserviceID, err := internal.ValidateApplicationService( + appserviceID, err := internal.ValidateApplicationServiceRequest( cfg, r.Username, accessToken, ) if err != nil { diff --git a/internal/validate.go b/internal/validate.go index 2526fc1190..7364110160 100644 --- a/internal/validate.go +++ b/internal/validate.go @@ -129,23 +129,25 @@ func userIDIsWithinApplicationServiceNamespace( userID string, appservice *config.ApplicationService, ) bool { - var local, domain, err = gomatrixserverlib.SplitID('@', userID) + var localpart, domain, err = gomatrixserverlib.SplitID('@', userID) if err != nil { // Not a valid userID return false } if !cfg.Matrix.IsLocalServerName(domain) { + // This is a federated userID return false } - if appservice.SenderLocalpart == local { + if localpart == appservice.SenderLocalpart { + // This is the application service bot userID 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 + // Application service namespaces are checked for validity in config if namespace.RegexpObject.MatchString(userID) { return true } @@ -172,26 +174,19 @@ func userIDMatchesMultipleExclusiveNamespaces( 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( +// ValidateApplicationServiceRequest checks if a provided application service +// token corresponds to one that is registered, and, if so, checks if the +// supplied userIDOrLocalpart is within that application service's namespace. +// +// As long as these two requirements are met, the matched application service +// ID will be returned. Otherwise, it will return a JSON response with the +// appropriate error message. +func ValidateApplicationServiceRequest( cfg *config.ClientAPI, - username string, + userIDOrLocalpart string, accessToken string, ) (string, *util.JSONResponse) { - localpart, domain, err := userutil.ParseUsernameParam(username, cfg.Matrix) + localpart, domain, err := userutil.ParseUsernameParam(userIDOrLocalpart, cfg.Matrix) if err != nil { return "", &util.JSONResponse{ Code: http.StatusUnauthorized, @@ -223,7 +218,7 @@ func ValidateApplicationService( 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)), + "Supplied username %s did not match any namespaces for application service ID: %s", userIDOrLocalpart, matchedApplicationService.ID)), } } @@ -232,7 +227,7 @@ func ValidateApplicationService( 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)), + "Supplied username %s matches multiple exclusive application service namespaces. Only 1 match allowed", userIDOrLocalpart)), } } From 25f2b51c3d8af42a0a27ab56e80f54c03b31166f Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Fri, 27 Jan 2023 16:09:28 +0100 Subject: [PATCH 13/26] Move UsernameMatchesExclusiveNamespaces back to register.go --- clientapi/routing/register.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/clientapi/routing/register.go b/clientapi/routing/register.go index 6dfebead57..22f0aa0a69 100644 --- a/clientapi/routing/register.go +++ b/clientapi/routing/register.go @@ -647,6 +647,16 @@ func handleGuestRegistration( } } +// localpartMatchesExclusiveNamespaces will check if a given username matches any +// application service's exclusive users namespace +func localpartMatchesExclusiveNamespaces( + cfg *config.ClientAPI, + localpart string, +) bool { + userID := userutil.MakeUserID(localpart, cfg.Matrix.ServerName) + return cfg.Derived.ExclusiveApplicationServicesUsernameRegexp.MatchString(userID) +} + // handleRegistrationFlow will direct and complete registration flow stages // that the client has requested. // nolint: gocyclo @@ -695,7 +705,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 && - internal.UsernameMatchesExclusiveNamespaces(cfg, r.Username) { + localpartMatchesExclusiveNamespaces(cfg, r.Username) { return util.JSONResponse{ Code: http.StatusBadRequest, JSON: spec.ASExclusive("This username is reserved by an application service."), From 4b98bbe180fe5676b780fa53ace700e401e11279 Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Fri, 27 Jan 2023 16:09:36 +0100 Subject: [PATCH 14/26] Convert TestValidationOfApplicationServices to be table-driven --- internal/validate_test.go | 122 ++++++++++++++++++++++++-------------- 1 file changed, 78 insertions(+), 44 deletions(-) diff --git a/internal/validate_test.go b/internal/validate_test.go index 582e6562a5..78bc531c78 100644 --- a/internal/validate_test.go +++ b/internal/validate_test.go @@ -41,7 +41,7 @@ func Test_validatePassword(t *testing.T) { t.Run(tt.name, func(t *testing.T) { gotErr := ValidatePassword(tt.password) if !reflect.DeepEqual(gotErr, tt.wantError) { - t.Errorf("validatePassword() = %v, wantJSON %v", gotErr, tt.wantError) + t.Errorf("validatePassword() = %v, wantError %v", gotErr, tt.wantError) } if got := PasswordResponse(gotErr); !reflect.DeepEqual(got, tt.wantJSON) { @@ -195,7 +195,7 @@ func TestValidationOfApplicationServices(t *testing.T) { // Create a second fake application service where userIDs ending in // "_overlap" overlap with the first. - regex = "@.*_overlap" + regex = "@_.*_overlap" fakeNamespace = config.ApplicationServiceNamespace{ Exclusive: true, Regex: regex, @@ -221,49 +221,83 @@ func TestValidationOfApplicationServices(t *testing.T) { fakeConfig.Global.ServerName = "localhost" fakeConfig.ClientAPI.Derived.ApplicationServices = []config.ApplicationService{fakeApplicationService, fakeApplicationServiceOverlap} - // Access token is correct, user_id omitted so we are acting as SenderLocalpart - asID, resp := ValidateApplicationService(&fakeConfig.ClientAPI, fakeSenderLocalpart, fakeApplicationService.ASToken) - if resp != nil || asID != fakeApplicationService.ID { - 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 == fakeApplicationService.ID { - 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", fakeApplicationService.ASToken) - if resp != nil || asID != fakeApplicationService.ID { - 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", fakeApplicationService.ASToken) - if resp == nil || asID == fakeApplicationService.ID { - t.Errorf("user_id should not have been valid: @_something_else:localhost") - } - - // Access token is correct, acting as user_id that matches two exclusive namespaces - asID, resp = ValidateApplicationService(&fakeConfig.ClientAPI, "_appservice_overlap", fakeApplicationService.ASToken) - if resp == nil || asID == fakeApplicationService.ID { - t.Errorf("user_id should not have been valid: @_appservice_overlap:localhost") - } - - // Access token is correct, acting as matching user_id that is too long - asID, resp = ValidateApplicationService( - &fakeConfig.ClientAPI, - "_appservice_"+strings.Repeat("a", maxUsernameLength), - fakeApplicationService.ASToken, - ) - if resp == nil || asID == fakeApplicationService.ID { - t.Errorf("user_id exceeding maxUsernameLength should not have been valid") + tests := []struct { + name string + localpart string + asToken string + wantError bool + wantASID string + }{ + // Access token is correct, userID omitted so we are acting as SenderLocalpart + { + name: "correct access token but omitted userID", + localpart: fakeSenderLocalpart, + asToken: fakeApplicationService.ASToken, + wantError: false, + wantASID: fakeApplicationService.ID, + }, + // Access token is incorrect, userID omitted so we are acting as SenderLocalpart + { + name: "incorrect access token but omitted userID", + localpart: fakeSenderLocalpart, + asToken: "xxxx", + wantError: true, + wantASID: "", + }, + // Access token is correct, acting as valid userID + { + name: "correct access token and valid userID", + localpart: "_appservice_bob", + asToken: fakeApplicationService.ASToken, + wantError: false, + wantASID: fakeApplicationService.ID, + }, + // Access token is correct, acting as invalid userID + { + name: "correct access token but invalid userID", + localpart: "_something_else", + asToken: fakeApplicationService.ASToken, + wantError: true, + wantASID: "", + }, + // Access token is correct, acting as userID that matches two exclusive namespaces + { + name: "correct access token but non-exclusive userID", + localpart: "_appservice_overlap", + asToken: fakeApplicationService.ASToken, + wantError: true, + wantASID: "", + }, + // Access token is correct, acting as matching userID that is too long + { + name: "correct access token but too long userID", + localpart: "_appservice_" + strings.Repeat("a", maxUsernameLength), + asToken: fakeApplicationService.ASToken, + wantError: true, + wantASID: "", + }, + // Access token is correct, acting as userID that matches but is invalid + { + name: "correct access token and matching but invalid userID", + localpart: "@_appservice_bob::", + asToken: fakeApplicationService.ASToken, + wantError: true, + wantASID: "", + }, } - // Access token is correct, acting as user_id that matches but is invalid - asID, resp = ValidateApplicationService(&fakeConfig.ClientAPI, "@_appservice_bob::", fakeApplicationService.ASToken) - if resp == nil || asID == fakeApplicationService.ID { - t.Errorf("user_id should not have been valid: @_appservice_bob::") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotASID, gotResp := ValidateApplicationServiceRequest(&fakeConfig.ClientAPI, tt.localpart, tt.asToken) + if tt.wantError && gotResp == nil { + t.Error("expected an error, but succeeded") + } + if !tt.wantError && gotResp != nil { + t.Errorf("expected success, but returned error: %v", *gotResp) + } + if gotASID != tt.wantASID { + t.Errorf("returned '%s', but expected '%s'", gotASID, tt.wantASID) + } + }) } } From fa42d71862670bf704f3e0db50206a5595b32f63 Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Fri, 27 Jan 2023 16:11:47 +0100 Subject: [PATCH 15/26] Update TestValidationOfApplicationServices function name --- internal/validate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/validate_test.go b/internal/validate_test.go index 78bc531c78..4533583e07 100644 --- a/internal/validate_test.go +++ b/internal/validate_test.go @@ -173,7 +173,7 @@ 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) { +func TestValidateApplicationServiceRequest(t *testing.T) { // Create a fake application service regex := "@_appservice_.*" fakeNamespace := config.ApplicationServiceNamespace{ From 1020f604e817855b4663fef9063f338711da14c5 Mon Sep 17 00:00:00 2001 From: KuhnChris Date: Sun, 7 May 2023 20:56:20 +0200 Subject: [PATCH 16/26] linting --- clientapi/auth/login_test.go | 1 - clientapi/routing/sendevent.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/clientapi/auth/login_test.go b/clientapi/auth/login_test.go index 3aa7c537a4..2585dbc3fb 100644 --- a/clientapi/auth/login_test.go +++ b/clientapi/auth/login_test.go @@ -19,7 +19,6 @@ import ( "net/http" "net/http/httptest" "reflect" - "regexp" "strings" "testing" diff --git a/clientapi/routing/sendevent.go b/clientapi/routing/sendevent.go index 41a3793ae2..5b50ad3d43 100644 --- a/clientapi/routing/sendevent.go +++ b/clientapi/routing/sendevent.go @@ -199,7 +199,7 @@ func SendEvent( req.Context(), rsAPI, api.KindNew, []*types.HeaderedEvent{ - &types.HeaderedEvent{PDU: e}, + {PDU: e}, }, device.UserDomain(), domain, From 843a3038fc09650dbab707093136a4dbe83672b0 Mon Sep 17 00:00:00 2001 From: KuhnChris Date: Sun, 7 May 2023 22:00:34 +0200 Subject: [PATCH 17/26] :shrug: --- clientapi/auth/login_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clientapi/auth/login_test.go b/clientapi/auth/login_test.go index 2585dbc3fb..f84e54710e 100644 --- a/clientapi/auth/login_test.go +++ b/clientapi/auth/login_test.go @@ -19,6 +19,7 @@ import ( "net/http" "net/http/httptest" "reflect" + "regexp" "strings" "testing" @@ -32,7 +33,7 @@ import ( var cfg = &config.ClientAPI{ Matrix: &config.Global{ - SigningIdentity: gomatrixserverlib.SigningIdentity{ + SigningIdentity: fclient.SigningIdentity{ ServerName: serverName, }, }, From 814e2b8e0e5e2f70bf1585e1a530230cb38b2524 Mon Sep 17 00:00:00 2001 From: KuhnChris Date: Sun, 7 May 2023 22:20:06 +0200 Subject: [PATCH 18/26] Apparently that cfg structure was there on purpose. --- clientapi/auth/login_test.go | 74 +++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 26 deletions(-) diff --git a/clientapi/auth/login_test.go b/clientapi/auth/login_test.go index f84e54710e..a2c2a719c1 100644 --- a/clientapi/auth/login_test.go +++ b/clientapi/auth/login_test.go @@ -31,31 +31,6 @@ import ( "github.com/matrix-org/util" ) -var cfg = &config.ClientAPI{ - Matrix: &config.Global{ - SigningIdentity: fclient.SigningIdentity{ - ServerName: serverName, - }, - }, - Derived: &config.Derived{ - ApplicationServices: []config.ApplicationService{ - { - ID: "anapplicationservice", - ASToken: "astoken", - NamespaceMap: map[string][]config.ApplicationServiceNamespace{ - "users": { - { - Exclusive: true, - Regex: "@alice:example.com", - RegexpObject: regexp.MustCompile("@alice:example.com"), - }, - }, - }, - }, - }, - }, -} - func TestLoginFromJSONReader(t *testing.T) { ctx := context.Background() @@ -118,6 +93,30 @@ func TestLoginFromJSONReader(t *testing.T) { for _, tst := range tsts { t.Run(tst.Name, func(t *testing.T) { var userAPI fakeUserInternalAPI + cfg := &config.ClientAPI{ + Matrix: &config.Global{ + SigningIdentity: fclient.SigningIdentity{ + ServerName: serverName, + }, + }, + Derived: &config.Derived{ + ApplicationServices: []config.ApplicationService{ + { + ID: "anapplicationservice", + ASToken: "astoken", + NamespaceMap: map[string][]config.ApplicationServiceNamespace{ + "users": { + { + Exclusive: true, + Regex: "@alice:example.com", + RegexpObject: regexp.MustCompile("@alice:example.com"), + }, + }, + }, + }, + }, + }, + } req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(tst.Body)) if tst.Token != "" { @@ -238,7 +237,30 @@ func TestBadLoginFromJSONReader(t *testing.T) { for _, tst := range tsts { t.Run(tst.Name, func(t *testing.T) { var userAPI fakeUserInternalAPI - + cfg := &config.ClientAPI{ + Matrix: &config.Global{ + SigningIdentity: fclient.SigningIdentity{ + ServerName: serverName, + }, + }, + Derived: &config.Derived{ + ApplicationServices: []config.ApplicationService{ + { + ID: "anapplicationservice", + ASToken: "astoken", + NamespaceMap: map[string][]config.ApplicationServiceNamespace{ + "users": { + { + Exclusive: true, + Regex: "@alice:example.com", + RegexpObject: regexp.MustCompile("@alice:example.com"), + }, + }, + }, + }, + }, + }, + } req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(tst.Body)) if tst.Token != "" { req.Header.Add("Authorization", "Bearer "+tst.Token) From bf053595ae813fb619c0292414f005444e69bf90 Mon Sep 17 00:00:00 2001 From: KuhnChris Date: Sun, 18 Jun 2023 21:57:21 +0000 Subject: [PATCH 19/26] Clean up merge from main --- clientapi/auth/login.go | 2 +- clientapi/routing/login.go | 1 - clientapi/routing/login_test.go | 2 +- clientapi/routing/register_test.go | 3 +++ internal/validate.go | 12 ++++++------ internal/validate_test.go | 3 +-- 6 files changed, 12 insertions(+), 11 deletions(-) diff --git a/clientapi/auth/login.go b/clientapi/auth/login.go index 96310f0dfb..58a27e5932 100644 --- a/clientapi/auth/login.go +++ b/clientapi/auth/login.go @@ -74,7 +74,7 @@ func LoginFromJSONReader( if err != nil { err := &util.JSONResponse{ Code: http.StatusForbidden, - JSON: jsonerror.MissingToken(err.Error()), + JSON: spec.MissingToken(err.Error()), } return nil, nil, err } diff --git a/clientapi/routing/login.go b/clientapi/routing/login.go index b6e984bf01..068642f800 100644 --- a/clientapi/routing/login.go +++ b/clientapi/routing/login.go @@ -20,7 +20,6 @@ import ( "github.com/matrix-org/dendrite/clientapi/auth" "github.com/matrix-org/dendrite/clientapi/auth/authtypes" - "github.com/matrix-org/dendrite/clientapi/jsonerror" "github.com/matrix-org/dendrite/clientapi/userutil" "github.com/matrix-org/dendrite/setup/config" userapi "github.com/matrix-org/dendrite/userapi/api" diff --git a/clientapi/routing/login_test.go b/clientapi/routing/login_test.go index bb531a1d7f..9ffe92a837 100644 --- a/clientapi/routing/login_test.go +++ b/clientapi/routing/login_test.go @@ -116,7 +116,7 @@ func TestLogin(t *testing.T) { t.Run("Supported log-in flows are returned", func(t *testing.T) { req := test.NewRequest(t, http.MethodGet, "/_matrix/client/v3/login") rec := httptest.NewRecorder() - base.PublicClientAPIMux.ServeHTTP(rec, req) + routers.Client.ServeHTTP(rec, req) if rec.Code != http.StatusOK { t.Fatalf("failed to get log-in flows: %s", rec.Body.String()) } diff --git a/clientapi/routing/register_test.go b/clientapi/routing/register_test.go index c92f53fa9a..2a88ec3806 100644 --- a/clientapi/routing/register_test.go +++ b/clientapi/routing/register_test.go @@ -22,6 +22,7 @@ import ( "net/http" "net/http/httptest" "reflect" + "regexp" "strings" "testing" "time" @@ -31,6 +32,8 @@ import ( "github.com/matrix-org/dendrite/internal/caching" "github.com/matrix-org/dendrite/internal/sqlutil" "github.com/matrix-org/dendrite/roomserver" + "github.com/matrix-org/dendrite/setup/config" + "github.com/matrix-org/dendrite/setup/jetstream" "github.com/matrix-org/dendrite/test" "github.com/matrix-org/dendrite/test/testrig" "github.com/matrix-org/dendrite/userapi" diff --git a/internal/validate.go b/internal/validate.go index 7364110160..a90a6636f1 100644 --- a/internal/validate.go +++ b/internal/validate.go @@ -20,10 +20,10 @@ import ( "net/http" "regexp" - "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/gomatrixserverlib/spec" "github.com/matrix-org/util" ) @@ -102,7 +102,7 @@ func UsernameResponse(err error) *util.JSONResponse { } // ValidateApplicationServiceUsername returns an error if the username is invalid for an application service -func ValidateApplicationServiceUsername(localpart string, domain gomatrixserverlib.ServerName) error { +func ValidateApplicationServiceUsername(localpart string, domain spec.ServerName) error { userID := userutil.MakeUserID(localpart, domain) return ValidateApplicationServiceUserID(userID) } @@ -190,7 +190,7 @@ func ValidateApplicationServiceRequest( if err != nil { return "", &util.JSONResponse{ Code: http.StatusUnauthorized, - JSON: jsonerror.InvalidUsername(err.Error()), + JSON: spec.InvalidUsername(err.Error()), } } @@ -208,7 +208,7 @@ func ValidateApplicationServiceRequest( if matchedApplicationService == nil { return "", &util.JSONResponse{ Code: http.StatusUnauthorized, - JSON: jsonerror.UnknownToken("Supplied access_token does not match any known application service"), + JSON: spec.UnknownToken("Supplied access_token does not match any known application service"), } } @@ -217,7 +217,7 @@ func ValidateApplicationServiceRequest( // If we didn't find any matches, return M_EXCLUSIVE return "", &util.JSONResponse{ Code: http.StatusBadRequest, - JSON: jsonerror.ASExclusive(fmt.Sprintf( + JSON: spec.ASExclusive(fmt.Sprintf( "Supplied username %s did not match any namespaces for application service ID: %s", userIDOrLocalpart, matchedApplicationService.ID)), } } @@ -226,7 +226,7 @@ func ValidateApplicationServiceRequest( if userIDMatchesMultipleExclusiveNamespaces(cfg, userID) { return "", &util.JSONResponse{ Code: http.StatusBadRequest, - JSON: jsonerror.ASExclusive(fmt.Sprintf( + JSON: spec.ASExclusive(fmt.Sprintf( "Supplied username %s matches multiple exclusive application service namespaces. Only 1 match allowed", userIDOrLocalpart)), } } diff --git a/internal/validate_test.go b/internal/validate_test.go index 4533583e07..a9e6b76498 100644 --- a/internal/validate_test.go +++ b/internal/validate_test.go @@ -7,9 +7,8 @@ import ( "strings" "testing" - "github.com/matrix-org/dendrite/clientapi/jsonerror" "github.com/matrix-org/dendrite/setup/config" - "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/gomatrixserverlib/spec" "github.com/matrix-org/util" ) From 27f28679845ee6e06bcff10326951ecdf6286baf Mon Sep 17 00:00:00 2001 From: KuhnChris Date: Sun, 18 Jun 2023 22:03:33 +0000 Subject: [PATCH 20/26] more cleanups --- clientapi/routing/receipt.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/clientapi/routing/receipt.go b/clientapi/routing/receipt.go index be6542979f..1d7e35562d 100644 --- a/clientapi/routing/receipt.go +++ b/clientapi/routing/receipt.go @@ -23,13 +23,12 @@ import ( "github.com/matrix-org/dendrite/clientapi/producers" "github.com/matrix-org/gomatrixserverlib/spec" - "github.com/matrix-org/dendrite/userapi/api" userapi "github.com/matrix-org/dendrite/userapi/api" "github.com/matrix-org/util" "github.com/sirupsen/logrus" ) -func SetReceipt(req *http.Request, userAPI api.ClientUserAPI, syncProducer *producers.SyncAPIProducer, device *userapi.Device, roomID, receiptType, eventID string) util.JSONResponse { +func SetReceipt(req *http.Request, userAPI userapi.ClientUserAPI, syncProducer *producers.SyncAPIProducer, device *userapi.Device, roomID, receiptType, eventID string) util.JSONResponse { timestamp := spec.AsTimestamp(time.Now()) logrus.WithFields(logrus.Fields{ "roomID": roomID, @@ -54,13 +53,13 @@ func SetReceipt(req *http.Request, userAPI api.ClientUserAPI, syncProducer *prod } } - dataReq := api.InputAccountDataRequest{ + dataReq := userapi.InputAccountDataRequest{ UserID: device.UserID, DataType: "m.fully_read", RoomID: roomID, AccountData: data, } - dataRes := api.InputAccountDataResponse{} + dataRes := userapi.InputAccountDataResponse{} if err := userAPI.InputAccountData(req.Context(), &dataReq, &dataRes); err != nil { util.GetLogger(req.Context()).WithError(err).Error("userAPI.InputAccountData failed") return util.ErrorResponse(err) From 579eb2f99347b955addcdb0c8b24bb156c8b04fa Mon Sep 17 00:00:00 2001 From: KuhnChris Date: Fri, 30 Jun 2023 11:54:50 +0200 Subject: [PATCH 21/26] Make tests go brrrr. --- internal/validate_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/validate_test.go b/internal/validate_test.go index a9e6b76498..6ba831cc65 100644 --- a/internal/validate_test.go +++ b/internal/validate_test.go @@ -215,7 +215,6 @@ func TestValidateApplicationServiceRequest(t *testing.T) { fakeConfig := &config.Dendrite{} fakeConfig.Defaults(config.DefaultOpts{ Generate: true, - Monolithic: true, }) fakeConfig.Global.ServerName = "localhost" fakeConfig.ClientAPI.Derived.ApplicationServices = []config.ApplicationService{fakeApplicationService, fakeApplicationServiceOverlap} From 721782f4f05b8750b354daae2073825f387d17f9 Mon Sep 17 00:00:00 2001 From: KuhnChris Date: Fri, 30 Jun 2023 12:52:23 +0200 Subject: [PATCH 22/26] Remove rouge "," --- internal/validate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/validate_test.go b/internal/validate_test.go index 6ba831cc65..c6d0b57c82 100644 --- a/internal/validate_test.go +++ b/internal/validate_test.go @@ -214,7 +214,7 @@ func TestValidateApplicationServiceRequest(t *testing.T) { // Set up a config fakeConfig := &config.Dendrite{} fakeConfig.Defaults(config.DefaultOpts{ - Generate: true, + Generate: true }) fakeConfig.Global.ServerName = "localhost" fakeConfig.ClientAPI.Derived.ApplicationServices = []config.ApplicationService{fakeApplicationService, fakeApplicationServiceOverlap} From 272f3cc2cce9ea7f8d202fbbec78859b996075e2 Mon Sep 17 00:00:00 2001 From: KuhnChris Date: Fri, 30 Jun 2023 13:24:59 +0200 Subject: [PATCH 23/26] `gofmt -w` fixes it. --- internal/validate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/validate_test.go b/internal/validate_test.go index c6d0b57c82..cd26261337 100644 --- a/internal/validate_test.go +++ b/internal/validate_test.go @@ -214,7 +214,7 @@ func TestValidateApplicationServiceRequest(t *testing.T) { // Set up a config fakeConfig := &config.Dendrite{} fakeConfig.Defaults(config.DefaultOpts{ - Generate: true + Generate: true, }) fakeConfig.Global.ServerName = "localhost" fakeConfig.ClientAPI.Derived.ApplicationServices = []config.ApplicationService{fakeApplicationService, fakeApplicationServiceOverlap} From eb51b1d24edc849cac8e81c1ab44ed497a3d61d2 Mon Sep 17 00:00:00 2001 From: KuhnChris Date: Mon, 3 Jul 2023 14:45:02 +0200 Subject: [PATCH 24/26] Allow loginflows based on config --- clientapi/routing/login.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/clientapi/routing/login.go b/clientapi/routing/login.go index 068642f800..a304ed96df 100644 --- a/clientapi/routing/login.go +++ b/clientapi/routing/login.go @@ -47,14 +47,16 @@ func Login( cfg *config.ClientAPI, ) util.JSONResponse { if req.Method == http.MethodGet { + var loginFlows []flow + loginFlows = []flow{{Type: authtypes.LoginTypePassword}} + if len(cfg.Derived.ApplicationServices) == 0 { + loginFlows = append(loginFlows, flow{Type: authtypes.LoginTypeApplicationService}) + } // TODO: support other forms of login, depending on config options return util.JSONResponse{ Code: http.StatusOK, JSON: flows{ - Flows: []flow{ - {Type: authtypes.LoginTypePassword}, - {Type: authtypes.LoginTypeApplicationService}, - }, + Flows: loginFlows, }, } } else if req.Method == http.MethodPost { From 532cdd1f456115ed253014dc21856f3210f434f8 Mon Sep 17 00:00:00 2001 From: KuhnChris Date: Mon, 3 Jul 2023 16:48:44 +0200 Subject: [PATCH 25/26] Make it more `go` Co-authored-by: Till <2353100+S7evinK@users.noreply.github.com> --- clientapi/routing/login.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clientapi/routing/login.go b/clientapi/routing/login.go index a304ed96df..0f55c88165 100644 --- a/clientapi/routing/login.go +++ b/clientapi/routing/login.go @@ -47,9 +47,8 @@ func Login( cfg *config.ClientAPI, ) util.JSONResponse { if req.Method == http.MethodGet { - var loginFlows []flow - loginFlows = []flow{{Type: authtypes.LoginTypePassword}} - if len(cfg.Derived.ApplicationServices) == 0 { + loginFlows := []flow{{Type: authtypes.LoginTypePassword}} + if len(cfg.Derived.ApplicationServices) > 0 { loginFlows = append(loginFlows, flow{Type: authtypes.LoginTypeApplicationService}) } // TODO: support other forms of login, depending on config options From aa23baa4c0af6b2220a0ffd7891fb56330fc275b Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Fri, 24 Nov 2023 21:15:51 +0100 Subject: [PATCH 26/26] Inject a dummy AS to make the UT happy --- clientapi/routing/login_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/clientapi/routing/login_test.go b/clientapi/routing/login_test.go index e38ba5ba0b..3f89344904 100644 --- a/clientapi/routing/login_test.go +++ b/clientapi/routing/login_test.go @@ -114,6 +114,11 @@ func TestLogin(t *testing.T) { ctx := context.Background() + // Inject a dummy application service, so we have a "m.login.application_service" + // in the login flows + as := &config.ApplicationService{} + cfg.AppServiceAPI.Derived.ApplicationServices = []config.ApplicationService{*as} + t.Run("Supported log-in flows are returned", func(t *testing.T) { req := test.NewRequest(t, http.MethodGet, "/_matrix/client/v3/login") rec := httptest.NewRecorder()