From 2b5fbea6dbc567e177597c0dfbc25f20dc1dee27 Mon Sep 17 00:00:00 2001 From: CicadaCinema <52425971+CicadaCinema@users.noreply.github.com> Date: Wed, 5 Jul 2023 20:58:52 +0000 Subject: [PATCH 1/5] collapse three equivalent case statements into one --- clientapi/routing/register_test.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/clientapi/routing/register_test.go b/clientapi/routing/register_test.go index 2a88ec3806..6ceb9e35e1 100644 --- a/clientapi/routing/register_test.go +++ b/clientapi/routing/register_test.go @@ -541,17 +541,7 @@ func Test_register(t *testing.T) { resp = Register(req, userAPI, &cfg.ClientAPI) switch resp.JSON.(type) { - case spec.InternalServerError: - if !reflect.DeepEqual(tc.wantResponse, resp) { - t.Fatalf("unexpected response: %+v, want: %+v", resp, tc.wantResponse) - } - return - case spec.MatrixError: - if !reflect.DeepEqual(tc.wantResponse, resp) { - t.Fatalf("unexpected response: %+v, want: %+v", resp, tc.wantResponse) - } - return - case util.JSONResponse: + case spec.InternalServerError, spec.MatrixError, util.JSONResponse: if !reflect.DeepEqual(tc.wantResponse, resp) { t.Fatalf("unexpected response: %+v, want: %+v", resp, tc.wantResponse) } From 00e43c60e5b6d73894defddc9e0aa98f5c5ea382 Mon Sep 17 00:00:00 2001 From: CicadaCinema <52425971+CicadaCinema@users.noreply.github.com> Date: Wed, 5 Jul 2023 21:05:22 +0000 Subject: [PATCH 2/5] insert all the logic into a type switch --- clientapi/routing/register_test.go | 43 ++++++++++++++---------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/clientapi/routing/register_test.go b/clientapi/routing/register_test.go index 6ceb9e35e1..85948a588a 100644 --- a/clientapi/routing/register_test.go +++ b/clientapi/routing/register_test.go @@ -540,34 +540,31 @@ func Test_register(t *testing.T) { resp = Register(req, userAPI, &cfg.ClientAPI) - switch resp.JSON.(type) { + switch rr := resp.JSON.(type) { case spec.InternalServerError, spec.MatrixError, util.JSONResponse: if !reflect.DeepEqual(tc.wantResponse, resp) { t.Fatalf("unexpected response: %+v, want: %+v", resp, tc.wantResponse) } return - } - - rr, ok := resp.JSON.(registerResponse) - if !ok { - t.Fatalf("expected a registerresponse, got %T", resp.JSON) - } - - // validate the response - if tc.forceEmpty { - // when not supplying a username, one will be generated. Given this _SHOULD_ be - // the second user, set the username accordingly - reg.Username = "2" - } - wantUserID := strings.ToLower(fmt.Sprintf("@%s:%s", reg.Username, "test")) - if wantUserID != rr.UserID { - t.Fatalf("unexpected userID: %s, want %s", rr.UserID, wantUserID) - } - if rr.DeviceID != *reg.DeviceID { - t.Fatalf("unexpected deviceID: %s, want %s", rr.DeviceID, *reg.DeviceID) - } - if rr.AccessToken == "" { - t.Fatalf("missing accessToken in response") + case registerResponse: + // validate the response + if tc.forceEmpty { + // when not supplying a username, one will be generated. Given this _SHOULD_ be + // the second user, set the username accordingly + reg.Username = "2" + } + wantUserID := strings.ToLower(fmt.Sprintf("@%s:%s", reg.Username, "test")) + if wantUserID != rr.UserID { + t.Fatalf("unexpected userID: %s, want %s", rr.UserID, wantUserID) + } + if rr.DeviceID != *reg.DeviceID { + t.Fatalf("unexpected deviceID: %s, want %s", rr.DeviceID, *reg.DeviceID) + } + if rr.AccessToken == "" { + t.Fatalf("missing accessToken in response") + } + default: + t.Fatalf("expected one of internalservererror, matrixerror, jsonresponse, registerresponse, got %T", resp.JSON) } }) } From 4b78bae72e2389df26f7c0112d61b7fe2ae89721 Mon Sep 17 00:00:00 2001 From: CicadaCinema <52425971+CicadaCinema@users.noreply.github.com> Date: Wed, 5 Jul 2023 21:35:40 +0000 Subject: [PATCH 3/5] do not hard-code an expected username instead, allow it to be set per test case --- clientapi/routing/register_test.go | 68 +++++++++++++++--------------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/clientapi/routing/register_test.go b/clientapi/routing/register_test.go index 85948a588a..ed0d641fc8 100644 --- a/clientapi/routing/register_test.go +++ b/clientapi/routing/register_test.go @@ -298,13 +298,16 @@ func Test_register(t *testing.T) { guestsDisabled bool enableRecaptcha bool captchaBody string - wantResponse util.JSONResponse + // in case of an error, the expected response + wantErrorResponse util.JSONResponse + // in case of success, the expected username assigned + wantUsername string }{ { name: "disallow guests", kind: "guest", guestsDisabled: true, - wantResponse: util.JSONResponse{ + wantErrorResponse: util.JSONResponse{ Code: http.StatusForbidden, JSON: spec.Forbidden(`Guest registration is disabled on "test"`), }, @@ -316,7 +319,7 @@ func Test_register(t *testing.T) { { name: "unknown login type", loginType: "im.not.known", - wantResponse: util.JSONResponse{ + wantErrorResponse: util.JSONResponse{ Code: http.StatusNotImplemented, JSON: spec.Unknown("unknown/unimplemented auth type"), }, @@ -324,16 +327,17 @@ func Test_register(t *testing.T) { { name: "disabled registration", registrationDisabled: true, - wantResponse: util.JSONResponse{ + wantErrorResponse: util.JSONResponse{ Code: http.StatusForbidden, JSON: spec.Forbidden(`Registration is disabled on "test"`), }, }, { - name: "successful registration, numeric ID", - username: "", - password: "someRandomPassword", - forceEmpty: true, + name: "successful registration, numeric ID", + username: "", + password: "someRandomPassword", + forceEmpty: true, + wantUsername: "2", }, { name: "successful registration", @@ -342,7 +346,7 @@ func Test_register(t *testing.T) { { name: "failing registration - user already exists", username: "success", - wantResponse: util.JSONResponse{ + wantErrorResponse: util.JSONResponse{ Code: http.StatusBadRequest, JSON: spec.UserInUse("Desired user ID is already taken."), }, @@ -352,14 +356,14 @@ func Test_register(t *testing.T) { username: "LOWERCASED", // this is going to be lower-cased }, { - name: "invalid username", - username: "#totalyNotValid", - wantResponse: *internal.UsernameResponse(internal.ErrUsernameInvalid), + name: "invalid username", + username: "#totalyNotValid", + wantErrorResponse: *internal.UsernameResponse(internal.ErrUsernameInvalid), }, { name: "numeric username is forbidden", username: "1337", - wantResponse: util.JSONResponse{ + wantErrorResponse: util.JSONResponse{ Code: http.StatusBadRequest, JSON: spec.InvalidUsername("Numeric user IDs are reserved"), }, @@ -367,7 +371,7 @@ func Test_register(t *testing.T) { { name: "disabled recaptcha login", loginType: authtypes.LoginTypeRecaptcha, - wantResponse: util.JSONResponse{ + wantErrorResponse: util.JSONResponse{ Code: http.StatusForbidden, JSON: spec.Unknown(ErrCaptchaDisabled.Error()), }, @@ -376,7 +380,7 @@ func Test_register(t *testing.T) { name: "enabled recaptcha, no response defined", enableRecaptcha: true, loginType: authtypes.LoginTypeRecaptcha, - wantResponse: util.JSONResponse{ + wantErrorResponse: util.JSONResponse{ Code: http.StatusBadRequest, JSON: spec.BadJSON(ErrMissingResponse.Error()), }, @@ -386,7 +390,7 @@ func Test_register(t *testing.T) { enableRecaptcha: true, loginType: authtypes.LoginTypeRecaptcha, captchaBody: `notvalid`, - wantResponse: util.JSONResponse{ + wantErrorResponse: util.JSONResponse{ Code: http.StatusUnauthorized, JSON: spec.BadJSON(ErrInvalidCaptcha.Error()), }, @@ -398,11 +402,11 @@ func Test_register(t *testing.T) { 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: spec.InternalServerError{}}, + name: "captcha invalid from remote", + enableRecaptcha: true, + loginType: authtypes.LoginTypeRecaptcha, + captchaBody: `i should fail for other reasons`, + wantErrorResponse: util.JSONResponse{Code: http.StatusInternalServerError, JSON: spec.InternalServerError{}}, }, } @@ -485,8 +489,8 @@ func Test_register(t *testing.T) { t.Fatalf("unexpected registration flows: %+v, want %+v", r.Flows, cfg.Derived.Registration.Flows) } case spec.MatrixError: - if !reflect.DeepEqual(tc.wantResponse, resp) { - t.Fatalf("(%s), unexpected response: %+v, want: %+v", tc.name, resp, tc.wantResponse) + if !reflect.DeepEqual(tc.wantErrorResponse, resp) { + t.Fatalf("(%s), unexpected response: %+v, want: %+v", tc.name, resp, tc.wantErrorResponse) } return case registerResponse: @@ -542,20 +546,18 @@ func Test_register(t *testing.T) { switch rr := resp.JSON.(type) { case spec.InternalServerError, spec.MatrixError, util.JSONResponse: - if !reflect.DeepEqual(tc.wantResponse, resp) { - t.Fatalf("unexpected response: %+v, want: %+v", resp, tc.wantResponse) + if !reflect.DeepEqual(tc.wantErrorResponse, resp) { + t.Fatalf("unexpected response: %+v, want: %+v", resp, tc.wantErrorResponse) } return case registerResponse: // validate the response - if tc.forceEmpty { - // when not supplying a username, one will be generated. Given this _SHOULD_ be - // the second user, set the username accordingly - reg.Username = "2" - } - wantUserID := strings.ToLower(fmt.Sprintf("@%s:%s", reg.Username, "test")) - if wantUserID != rr.UserID { - t.Fatalf("unexpected userID: %s, want %s", rr.UserID, wantUserID) + if tc.wantUsername != "" { + // if an expected username is provided, validate it + wantUserID := strings.ToLower(fmt.Sprintf("@%s:%s", tc.wantUsername, "test")) + if wantUserID != rr.UserID { + t.Fatalf("unexpected userID: %s, want %s", rr.UserID, wantUserID) + } } if rr.DeviceID != *reg.DeviceID { t.Fatalf("unexpected deviceID: %s, want %s", rr.DeviceID, *reg.DeviceID) From 2aeb0a6663ef16542dee0bc2ceda282ca0523efe Mon Sep 17 00:00:00 2001 From: CicadaCinema <52425971+CicadaCinema@users.noreply.github.com> Date: Wed, 5 Jul 2023 21:43:24 +0000 Subject: [PATCH 4/5] if registration was successful on the first request and an expected username is provided, assert that it is a match --- clientapi/routing/register_test.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/clientapi/routing/register_test.go b/clientapi/routing/register_test.go index ed0d641fc8..de89c9a641 100644 --- a/clientapi/routing/register_test.go +++ b/clientapi/routing/register_test.go @@ -313,8 +313,9 @@ func Test_register(t *testing.T) { }, }, { - name: "allow guests", - kind: "guest", + name: "allow guests", + kind: "guest", + wantUsername: "1", }, { name: "unknown login type", @@ -508,6 +509,13 @@ func Test_register(t *testing.T) { if r.DeviceID == "" { t.Fatalf("missing deviceID in response") } + // if an expected username is provided, assert that it is a match + if tc.wantUsername != "" { + wantUserID := strings.ToLower(fmt.Sprintf("@%s:%s", tc.wantUsername, "test")) + if wantUserID != r.UserID { + t.Fatalf("unexpected userID: %s, want %s", r.UserID, wantUserID) + } + } return default: t.Logf("Got response: %T", resp.JSON) @@ -553,7 +561,7 @@ func Test_register(t *testing.T) { case registerResponse: // validate the response if tc.wantUsername != "" { - // if an expected username is provided, validate it + // if an expected username is provided, assert that it is a match wantUserID := strings.ToLower(fmt.Sprintf("@%s:%s", tc.wantUsername, "test")) if wantUserID != rr.UserID { t.Fatalf("unexpected userID: %s, want %s", rr.UserID, wantUserID) From b1b3e485908b80aa022d6470cc056d68368a9e03 Mon Sep 17 00:00:00 2001 From: CicadaCinema <52425971+CicadaCinema@users.noreply.github.com> Date: Wed, 5 Jul 2023 21:47:10 +0000 Subject: [PATCH 5/5] add another test for registering a user with no username specified --- clientapi/routing/register_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/clientapi/routing/register_test.go b/clientapi/routing/register_test.go index de89c9a641..e730bfa60f 100644 --- a/clientapi/routing/register_test.go +++ b/clientapi/routing/register_test.go @@ -344,6 +344,13 @@ func Test_register(t *testing.T) { name: "successful registration", username: "success", }, + { + name: "successful registration, sequential numeric ID", + username: "", + password: "someRandomPassword", + forceEmpty: true, + wantUsername: "3", + }, { name: "failing registration - user already exists", username: "success",