Skip to content
This repository has been archived by the owner on Nov 25, 2024. It is now read-only.

Commit

Permalink
Refactor registration tests, remove hard-coded username validation (#…
Browse files Browse the repository at this point in the history 
…3138)

### Pull Request Checklist

* [x] I have added Go unit tests or [Complement integration
tests](https://github.com/matrix-org/complement) for this PR _or_ I have
justified why this PR doesn't need tests
* [x] I have already signed off privately

This PR is in preparation for #3137 and removes the hard-coded username
validation (previously only dependent on `forceEmpty`).

---------

Co-authored-by: kegsay <[email protected]>
  • Loading branch information
@CicadaCinema @kegsay
CicadaCinema and kegsay authored Nov 22, 2023
1 parent 210123b commit f25cce2
Showing 1 changed file with 63 additions and 59 deletions.
122 changes: 63 additions & 59 deletions clientapi/routing/register_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,51 +298,63 @@ 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"`),
},
},
{
name: "allow guests",
kind: "guest",
name: "allow guests",
kind: "guest",
wantUsername: "1",
},
{
name: "unknown login type",
loginType: "im.not.known",
wantResponse: util.JSONResponse{
wantErrorResponse: util.JSONResponse{
Code: http.StatusNotImplemented,
JSON: spec.Unknown("unknown/unimplemented auth type"),
},
},
{
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",
username: "success",
},
{
name: "successful registration, sequential numeric ID",
username: "",
password: "someRandomPassword",
forceEmpty: true,
wantUsername: "3",
},
{
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."),
},
Expand All @@ -352,22 +364,22 @@ 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"),
},
},
{
name: "disabled recaptcha login",
loginType: authtypes.LoginTypeRecaptcha,
wantResponse: util.JSONResponse{
wantErrorResponse: util.JSONResponse{
Code: http.StatusForbidden,
JSON: spec.Unknown(ErrCaptchaDisabled.Error()),
},
Expand All @@ -376,7 +388,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()),
},
Expand All @@ -386,7 +398,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()),
},
Expand All @@ -398,11 +410,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{}},
},
}

Expand Down Expand Up @@ -486,8 +498,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:
Expand All @@ -505,6 +517,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)
Expand Down Expand Up @@ -541,44 +560,29 @@ 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)
switch rr := resp.JSON.(type) {
case spec.InternalServerError, spec.MatrixError, util.JSONResponse:
if !reflect.DeepEqual(tc.wantErrorResponse, resp) {
t.Fatalf("unexpected response: %+v, want: %+v", resp, tc.wantErrorResponse)
}
return
case spec.MatrixError:
if !reflect.DeepEqual(tc.wantResponse, resp) {
t.Fatalf("unexpected response: %+v, want: %+v", resp, tc.wantResponse)
case registerResponse:
// validate the response
if tc.wantUsername != "" {
// 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)
}
}
return
case util.JSONResponse:
if !reflect.DeepEqual(tc.wantResponse, resp) {
t.Fatalf("unexpected response: %+v, want: %+v", resp, tc.wantResponse)
if rr.DeviceID != *reg.DeviceID {
t.Fatalf("unexpected deviceID: %s, want %s", rr.DeviceID, *reg.DeviceID)
}
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")
if rr.AccessToken == "" {
t.Fatalf("missing accessToken in response")
}
default:
t.Fatalf("expected one of internalservererror, matrixerror, jsonresponse, registerresponse, got %T", resp.JSON)
}
})
}
Expand Down

0 comments on commit f25cce2

Please sign in to comment.