From 508e4c69ebd9fc4595b9232b3848227bd0de34b1 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Fri, 23 Dec 2022 10:19:03 +0100 Subject: [PATCH] Add test for captcha registration --- clientapi/routing/register.go | 7 +-- clientapi/routing/register_test.go | 91 +++++++++++++++++++++++++++--- internal/validate.go | 2 +- setup/config/config.go | 12 ++-- 4 files changed, 91 insertions(+), 21 deletions(-) diff --git a/clientapi/routing/register.go b/clientapi/routing/register.go index b678256f6..6087bda0c 100644 --- a/clientapi/routing/register.go +++ b/clientapi/routing/register.go @@ -292,7 +292,7 @@ func validateRecaptcha( return ErrMissingResponse } - // Make a POST request to Google's API to check the captcha response + // Make a POST request to the captcha provider API to check the captcha response resp, err := http.PostForm(cfg.RecaptchaSiteVerifyAPI, url.Values{ "secret": {cfg.RecaptchaPrivateKey}, @@ -509,11 +509,6 @@ func Register( if resErr := httputil.UnmarshalJSON(reqBody, &r); resErr != nil { return *resErr } - var l string - var d gomatrixserverlib.ServerName - if l, d, err = cfg.Matrix.SplitLocalID('@', r.Username); err == nil { - r.Username, r.ServerName = l, d - } if req.URL.Query().Get("kind") == "guest" { return handleGuestRegistration(req, r, cfg, userAPI) } diff --git a/clientapi/routing/register_test.go b/clientapi/routing/register_test.go index 9d6dd88c8..b8fd19e90 100644 --- a/clientapi/routing/register_test.go +++ b/clientapi/routing/register_test.go @@ -290,6 +290,8 @@ func Test_register(t *testing.T) { forceEmpty bool registrationDisabled bool guestsDisabled bool + enableRecaptcha bool + captchaBody string wantResponse util.JSONResponse }{ { @@ -340,7 +342,7 @@ func Test_register(t *testing.T) { }, }, { - name: "successful registration", + name: "successful registration uppercase username", username: "LOWERCASED", // this is going to be lower-cased }, { @@ -356,6 +358,46 @@ func Test_register(t *testing.T) { JSON: jsonerror.InvalidUsername("Numeric user IDs are reserved"), }, }, + { + name: "disabled recaptcha login", + loginType: authtypes.LoginTypeRecaptcha, + wantResponse: util.JSONResponse{ + Code: http.StatusForbidden, + JSON: jsonerror.Unknown(ErrCaptchaDisabled.Error()), + }, + }, + { + name: "enabled recaptcha, no response defined", + enableRecaptcha: true, + loginType: authtypes.LoginTypeRecaptcha, + wantResponse: util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON(ErrMissingResponse.Error()), + }, + }, + { + name: "invalid captcha response", + enableRecaptcha: true, + loginType: authtypes.LoginTypeRecaptcha, + captchaBody: `notvalid`, + wantResponse: util.JSONResponse{ + Code: http.StatusUnauthorized, + JSON: jsonerror.BadJSON(ErrInvalidCaptcha.Error()), + }, + }, + { + name: "valid captcha response", + enableRecaptcha: true, + loginType: authtypes.LoginTypeRecaptcha, + captchaBody: `success`, + }, + { + name: "captcha invalid from remote", + enableRecaptcha: true, + loginType: authtypes.LoginTypeRecaptcha, + captchaBody: `i should fail for other reasons`, + wantResponse: util.JSONResponse{Code: http.StatusInternalServerError, JSON: jsonerror.InternalServerError()}, + }, } test.WithAllDatabases(t, func(t *testing.T, dbType test.DBType) { @@ -367,12 +409,37 @@ func Test_register(t *testing.T) { userAPI := userapi.NewInternalAPI(base, &base.Cfg.UserAPI, nil, keyAPI, rsAPI, nil) keyAPI.SetUserAPI(userAPI) - if err := base.Cfg.Derive(); err != nil { - t.Fatalf("failed to derive config: %s", err) - } - for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + if tc.enableRecaptcha { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if err := r.ParseForm(); err != nil { + t.Fatal(err) + } + response := r.Form.Get("response") + + // Respond with valid JSON or no JSON at all to test happy/error cases + switch response { + case "success": + json.NewEncoder(w).Encode(recaptchaResponse{Success: true}) + case "notvalid": + json.NewEncoder(w).Encode(recaptchaResponse{Success: false}) + default: + + } + })) + defer srv.Close() + base.Cfg.ClientAPI.RecaptchaSiteVerifyAPI = srv.URL + } + + if err := base.Cfg.Derive(); err != nil { + t.Fatalf("failed to derive config: %s", err) + } + + base.Cfg.ClientAPI.RecaptchaEnabled = tc.enableRecaptcha + base.Cfg.ClientAPI.RegistrationDisabled = tc.registrationDisabled + base.Cfg.ClientAPI.GuestsDisabled = tc.guestsDisabled + if tc.kind == "" { tc.kind = "user" } @@ -392,10 +459,6 @@ func Test_register(t *testing.T) { } body := &bytes.Buffer{} - - base.Cfg.ClientAPI.RegistrationDisabled = tc.registrationDisabled - base.Cfg.ClientAPI.GuestsDisabled = tc.guestsDisabled - err := json.NewEncoder(body).Encode(reg) if err != nil { t.Fatal(err) @@ -450,6 +513,11 @@ func Test_register(t *testing.T) { Type: authtypes.LoginType(tc.loginType), Session: uia.Session, } + + if tc.captchaBody != "" { + reg.Auth.Response = tc.captchaBody + } + dummy := "dummy" reg.DeviceID = &dummy reg.InitialDisplayName = &dummy @@ -470,6 +538,11 @@ func Test_register(t *testing.T) { t.Fatalf("unexpected response: %+v, want: %+v", resp, tc.wantResponse) } return + case util.JSONResponse: + if !reflect.DeepEqual(tc.wantResponse, resp) { + t.Fatalf("unexpected response: %+v, want: %+v", resp, tc.wantResponse) + } + return } rr, ok := resp.JSON.(registerResponse) diff --git a/internal/validate.go b/internal/validate.go index c8301f834..0461b897e 100644 --- a/internal/validate.go +++ b/internal/validate.go @@ -41,7 +41,7 @@ var ( validUsernameRegex = regexp.MustCompile(`^[0-9a-z_\-=./]+$`) ) -// ValidatePassword returns an error response if the password is invalid +// ValidatePassword returns an error if the password is invalid func ValidatePassword(password string) error { // https://github.com/matrix-org/synapse/blob/v0.20.0/synapse/rest/client/v2_alpha/register.py#L161 if len(password) > maxPasswordLength { diff --git a/setup/config/config.go b/setup/config/config.go index 7e7ed1aa1..6523a2452 100644 --- a/setup/config/config.go +++ b/setup/config/config.go @@ -29,7 +29,7 @@ import ( "github.com/matrix-org/gomatrixserverlib" "github.com/sirupsen/logrus" "golang.org/x/crypto/ed25519" - yaml "gopkg.in/yaml.v2" + "gopkg.in/yaml.v2" jaegerconfig "github.com/uber/jaeger-client-go/config" jaegermetrics "github.com/uber/jaeger-lib/metrics" @@ -314,11 +314,13 @@ func (config *Dendrite) Derive() error { if config.ClientAPI.RecaptchaEnabled { config.Derived.Registration.Params[authtypes.LoginTypeRecaptcha] = map[string]string{"public_key": config.ClientAPI.RecaptchaPublicKey} - config.Derived.Registration.Flows = append(config.Derived.Registration.Flows, - authtypes.Flow{Stages: []authtypes.LoginType{authtypes.LoginTypeRecaptcha}}) + config.Derived.Registration.Flows = []authtypes.Flow{ + {Stages: []authtypes.LoginType{authtypes.LoginTypeRecaptcha}}, + } } else { - config.Derived.Registration.Flows = append(config.Derived.Registration.Flows, - authtypes.Flow{Stages: []authtypes.LoginType{authtypes.LoginTypeDummy}}) + config.Derived.Registration.Flows = []authtypes.Flow{ + {Stages: []authtypes.LoginType{authtypes.LoginTypeDummy}}, + } } // Load application service configuration files