Skip to content

Commit

Permalink
Replace Localpart with LocalpartSuffix (#674)
Browse files Browse the repository at this point in the history
* Replace Localpart with LocalpartSuffix

This means clients can't register specific localparts, which is
good for dirty runs.

* Fix bad refactor
  • Loading branch information
kegsay authored Oct 13, 2023
1 parent d92f9ba commit dd10fb0
Show file tree
Hide file tree
Showing 23 changed files with 178 additions and 189 deletions.
8 changes: 4 additions & 4 deletions helpers/clientopts.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package helpers

type RegistrationOpts struct {
Localpart string // default '' (don't care)
DeviceID string // default '' (generate new)
Password string // default 'complement_meets_min_password_requirement'
IsAdmin bool // default false
LocalpartSuffix string // default '' (don't care)
DeviceID string // default '' (generate new)
Password string // default 'complement_meets_min_password_requirement'
IsAdmin bool // default false
}

type LoginOpts struct {
Expand Down
5 changes: 1 addition & 4 deletions internal/docker/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,7 @@ func (d *Deployment) Register(t *testing.T, hsName string, opts helpers.Registra
if password == "" {
password = "complement_meets_min_password_req"
}
localpart := opts.Localpart
if localpart == "" {
localpart = fmt.Sprintf("user-%v", d.localpartCounter.Add(1))
}
localpart := fmt.Sprintf("user-%v-%v", d.localpartCounter.Add(1), opts.LocalpartSuffix)
var userID, accessToken, deviceID string
if opts.IsAdmin {
userID, accessToken, deviceID = client.RegisterSharedSecret(t, localpart, password, opts.IsAdmin)
Expand Down
3 changes: 1 addition & 2 deletions tests/csapi/account_change_password_pushers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ func TestChangePasswordPushers(t *testing.T) {
password1 := "superuser"
password2 := "my_new_password"
passwordClient := deployment.Register(t, "hs1", helpers.RegistrationOpts{
Localpart: "test_change_password_pusher_user",
Password: password1,
Password: password1,
})

// sytest: Pushers created with a different access token are deleted on password change
Expand Down
11 changes: 5 additions & 6 deletions tests/csapi/account_change_password_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package csapi_tests

import (
"io/ioutil"
"io"
"testing"

"github.com/matrix-org/complement"
Expand All @@ -20,11 +20,10 @@ func TestChangePassword(t *testing.T) {
password1 := "superuser"
password2 := "my_new_password"
passwordClient := deployment.Register(t, "hs1", helpers.RegistrationOpts{
Localpart: "test_change_password_user",
Password: password1,
Password: password1,
})
unauthedClient := deployment.Client(t, "hs1", "")
_, sessionTest := createSession(t, deployment, "test_change_password_user", "superuser")
_, sessionTest := createSession(t, deployment, passwordClient.UserID, "superuser")
// sytest: After changing password, can't log in with old password
t.Run("After changing password, can't log in with old password", func(t *testing.T) {

Expand Down Expand Up @@ -82,7 +81,7 @@ func TestChangePassword(t *testing.T) {

// sytest: After changing password, different sessions can optionally be kept
t.Run("After changing password, different sessions can optionally be kept", func(t *testing.T) {
_, sessionOptional := createSession(t, deployment, "test_change_password_user", password2)
_, sessionOptional := createSession(t, deployment, passwordClient.UserID, password2)
reqBody := client.WithJSONBody(t, map[string]interface{}{
"auth": map[string]interface{}{
"type": "m.login.password",
Expand Down Expand Up @@ -135,7 +134,7 @@ func createSession(t *testing.T, deployment complement.Deployment, userID, passw
"password": password,
})
res := authedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "login"}, reqBody)
body, err := ioutil.ReadAll(res.Body)
body, err := io.ReadAll(res.Body)
if err != nil {
t.Fatalf("unable to read response body: %v", err)
}
Expand Down
3 changes: 1 addition & 2 deletions tests/csapi/account_deactivate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ func TestDeactivateAccount(t *testing.T) {
defer deployment.Destroy(t)
password := "superuser"
authedClient := deployment.Register(t, "hs1", helpers.RegistrationOpts{
Localpart: "test_deactivate_user",
Password: password,
Password: password,
})
unauthedClient := deployment.Client(t, "hs1", "")

Expand Down
8 changes: 2 additions & 6 deletions tests/csapi/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ func TestCanRegisterAdmin(t *testing.T) {
deployment := complement.Deploy(t, b.BlueprintAlice)
defer deployment.Destroy(t)
deployment.Register(t, "hs1", helpers.RegistrationOpts{
Localpart: "admin",
Password: "adminpassword",
IsAdmin: true,
IsAdmin: true,
})
}

Expand All @@ -32,9 +30,7 @@ func TestServerNotices(t *testing.T) {
deployment := complement.Deploy(t, b.BlueprintAlice)
defer deployment.Destroy(t)
admin := deployment.Register(t, "hs1", helpers.RegistrationOpts{
Localpart: "admin",
Password: "adminpassword",
IsAdmin: true,
IsAdmin: true,
})
alice := deployment.Client(t, "hs1", "@alice:hs1")

Expand Down
7 changes: 3 additions & 4 deletions tests/csapi/apidoc_device_management_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ func TestDeviceManagement(t *testing.T) {
defer deployment.Destroy(t)
unauthedClient := deployment.Client(t, "hs1", "")
authedClient := deployment.Register(t, "hs1", helpers.RegistrationOpts{
Localpart: "test_device_management_user",
Password: "superuser",
Password: "superuser",
})

// sytest: GET /device/{deviceId}
Expand Down Expand Up @@ -199,8 +198,8 @@ func TestDeviceManagement(t *testing.T) {
// sytest: DELETE /device/{deviceId} requires UI auth user to match device owner
t.Run("DELETE /device/{deviceId} requires UI auth user to match device owner", func(t *testing.T) {
bob := deployment.Register(t, "hs1", helpers.RegistrationOpts{
Localpart: "bob",
Password: "bobspassword",
LocalpartSuffix: "bob",
Password: "bobspassword",
})

newDeviceID, session2 := createSession(t, deployment, authedClient.UserID, "superuser")
Expand Down
75 changes: 40 additions & 35 deletions tests/csapi/apidoc_login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package csapi_tests
import (
"encoding/json"
"fmt"
"strings"
"testing"

"github.com/tidwall/gjson"
Expand All @@ -13,15 +14,15 @@ import (
"github.com/matrix-org/complement/helpers"
"github.com/matrix-org/complement/match"
"github.com/matrix-org/complement/must"
"github.com/matrix-org/gomatrixserverlib"
)

func TestLogin(t *testing.T) {
deployment := complement.Deploy(t, b.BlueprintAlice)
defer deployment.Destroy(t)
unauthedClient := deployment.Client(t, "hs1", "")
deployment.Register(t, "hs1", helpers.RegistrationOpts{
Localpart: "test_login_user",
Password: "superuser",
testClient := deployment.Register(t, "hs1", helpers.RegistrationOpts{
Password: "superuser",
})
t.Run("parallel", func(t *testing.T) {
// sytest: GET /login yields a set of flows
Expand All @@ -48,14 +49,14 @@ func TestLogin(t *testing.T) {
// sytest: POST /login can log in as a user
t.Run("POST /login can login as user", func(t *testing.T) {
t.Parallel()
res := unauthedClient.MustDo(t, "POST", []string{"_matrix", "client", "v3", "login"}, client.WithRawBody(json.RawMessage(`{
res := unauthedClient.MustDo(t, "POST", []string{"_matrix", "client", "v3", "login"}, client.WithJSONBody(t, map[string]interface{}{
"type": "m.login.password",
"identifier": {
"identifier": map[string]interface{}{
"type": "m.id.user",
"user": "@test_login_user:hs1"
"user": testClient.UserID,
},
"password": "superuser"
}`)))
"password": "superuser",
}))

must.MatchResponse(t, res, match.HTTPResponse{
JSON: []match.JSON{
Expand All @@ -67,15 +68,15 @@ func TestLogin(t *testing.T) {
t.Run("POST /login returns the same device_id as that in the request", func(t *testing.T) {
t.Parallel()
deviceID := "test_device_id"
res := unauthedClient.MustDo(t, "POST", []string{"_matrix", "client", "v3", "login"}, client.WithRawBody(json.RawMessage(`{
res := unauthedClient.MustDo(t, "POST", []string{"_matrix", "client", "v3", "login"}, client.WithJSONBody(t, map[string]interface{}{
"type": "m.login.password",
"identifier": {
"identifier": map[string]interface{}{
"type": "m.id.user",
"user": "@test_login_user:hs1"
"user": testClient.UserID,
},
"password": "superuser",
"device_id": "`+deviceID+`"
}`)))
"password": "superuser",
"device_id": deviceID,
}))

must.MatchResponse(t, res, match.HTTPResponse{
JSON: []match.JSON{
Expand All @@ -87,15 +88,16 @@ func TestLogin(t *testing.T) {
// sytest: POST /login can log in as a user with just the local part of the id
t.Run("POST /login can log in as a user with just the local part of the id", func(t *testing.T) {
t.Parallel()

res := unauthedClient.MustDo(t, "POST", []string{"_matrix", "client", "v3", "login"}, client.WithRawBody(json.RawMessage(`{
localpart, _, err := gomatrixserverlib.SplitID('@', testClient.UserID)
must.NotError(t, "failed to get localpart from user ID", err)
res := unauthedClient.MustDo(t, "POST", []string{"_matrix", "client", "v3", "login"}, client.WithJSONBody(t, map[string]interface{}{
"type": "m.login.password",
"identifier": {
"identifier": map[string]interface{}{
"type": "m.id.user",
"user": "test_login_user"
"user": localpart,
},
"password": "superuser"
}`)))
"password": "superuser",
}))

must.MatchResponse(t, res, match.HTTPResponse{
JSON: []match.JSON{
Expand All @@ -106,29 +108,29 @@ func TestLogin(t *testing.T) {
// sytest: POST /login as non-existing user is rejected
t.Run("POST /login as non-existing user is rejected", func(t *testing.T) {
t.Parallel()
res := unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "login"}, client.WithRawBody(json.RawMessage(`{
res := unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "login"}, client.WithJSONBody(t, map[string]interface{}{
"type": "m.login.password",
"identifier": {
"identifier": map[string]interface{}{
"type": "m.id.user",
"user": "i-dont-exist"
"user": "i-dont-exist",
},
"password": "superuser"
}`)))
"password": "superuser",
}))
must.MatchResponse(t, res, match.HTTPResponse{
StatusCode: 403,
})
})
// sytest: POST /login wrong password is rejected
t.Run("POST /login wrong password is rejected", func(t *testing.T) {
t.Parallel()
res := unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "login"}, client.WithRawBody(json.RawMessage(`{
res := unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "login"}, client.WithJSONBody(t, map[string]interface{}{
"type": "m.login.password",
"identifier": {
"identifier": map[string]interface{}{
"type": "m.id.user",
"user": "@test_login_user:hs1"
"user": testClient.UserID,
},
"password": "wrong_password"
}`)))
"password": "wrong_password",
}))
must.MatchResponse(t, res, match.HTTPResponse{
StatusCode: 403,
JSON: []match.JSON{
Expand All @@ -141,14 +143,17 @@ func TestLogin(t *testing.T) {
t.Run("Login with uppercase username works and GET /whoami afterwards also", func(t *testing.T) {
t.Parallel()
// login should be possible with uppercase username
res := unauthedClient.MustDo(t, "POST", []string{"_matrix", "client", "v3", "login"}, client.WithRawBody(json.RawMessage(`{
localpart, domain, err := gomatrixserverlib.SplitID('@', testClient.UserID)
must.NotError(t, "failed to get localpart from user ID", err)

res := unauthedClient.MustDo(t, "POST", []string{"_matrix", "client", "v3", "login"}, client.WithJSONBody(t, map[string]interface{}{
"type": "m.login.password",
"identifier": {
"identifier": map[string]interface{}{
"type": "m.id.user",
"user": "@Test_login_user:hs1"
"user": fmt.Sprintf("@%s:%s", strings.ToUpper(localpart), domain),
},
"password": "superuser"
}`)))
"password": "superuser",
}))
// extract access_token
js := must.ParseJSON(t, res.Body)
defer res.Body.Close()
Expand Down
3 changes: 1 addition & 2 deletions tests/csapi/apidoc_logout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ func TestLogout(t *testing.T) {

password := "superuser"
verifyClientUser := deployment.Register(t, "hs1", helpers.RegistrationOpts{
Localpart: "testuser",
Password: password,
Password: password,
})

// sytest: Can logout current device
Expand Down
11 changes: 7 additions & 4 deletions tests/csapi/apidoc_register_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/matrix-org/complement/helpers"
"github.com/matrix-org/complement/match"
"github.com/matrix-org/complement/must"
"github.com/matrix-org/gomatrixserverlib"
)

// TODO:
Expand Down Expand Up @@ -183,8 +184,8 @@ func TestRegistration(t *testing.T) {
localpart := fmt.Sprintf("chrtestuser%s", string(testChars[x]))
t.Run(string(testChars[x]), func(t *testing.T) {
deployment.Register(t, "hs1", helpers.RegistrationOpts{
Localpart: localpart,
Password: "sUp3rs3kr1t",
LocalpartSuffix: localpart,
Password: "sUp3rs3kr1t",
})
})
}
Expand Down Expand Up @@ -279,9 +280,11 @@ func TestRegistration(t *testing.T) {
t.Parallel()
testUserName := "username_not_available"
// Don't need the return value here, just need a user to be registered to test against
deployment.Register(t, "hs1", helpers.RegistrationOpts{Localpart: testUserName})
inUseClient := deployment.Register(t, "hs1", helpers.RegistrationOpts{LocalpartSuffix: testUserName})
localpart, _, err := gomatrixserverlib.SplitID('@', inUseClient.UserID)
must.NotError(t, "failed to get localpart from user ID", err)
res := unauthedClient.Do(t, "GET", []string{"_matrix", "client", "v3", "register", "available"}, client.WithQueries(url.Values{
"username": []string{testUserName},
"username": []string{localpart},
}))
must.MatchResponse(t, res, match.HTTPResponse{
StatusCode: 400,
Expand Down
Loading

0 comments on commit dd10fb0

Please sign in to comment.