Refactor how client creation/login is done

go.mod

@@ -6,7 +6,7 @@ require (
 	github.com/chromedp/cdproto v0.0.0-20231025043423-5615e204d422
 	github.com/chromedp/chromedp v0.9.3
 	github.com/docker/go-connections v0.4.0
-	github.com/matrix-org/complement v0.0.0-20240117130049-f24331b29b64
+	github.com/matrix-org/complement v0.0.0-20240126134841-458bfba5f7f3
 	github.com/testcontainers/testcontainers-go v0.26.0
 	github.com/tidwall/gjson v1.16.0
 	golang.org/x/exp v0.0.0-20230905200255-921286631fa9

go.sum

@@ -89,6 +89,8 @@ github.com/matrix-org/complement v0.0.0-20240117124104-5ecb086412c1 h1:YryEz+vrd
 github.com/matrix-org/complement v0.0.0-20240117124104-5ecb086412c1/go.mod h1:GMCwbgMOudedB86u1c5+nfQS1L31sFHZ9/YzTYqWyjU=
 github.com/matrix-org/complement v0.0.0-20240117130049-f24331b29b64 h1:dBvjM8idvlfkgyCeimfSxZTXdE85wdfJfGIEPq4GAu8=
 github.com/matrix-org/complement v0.0.0-20240117130049-f24331b29b64/go.mod h1:GMCwbgMOudedB86u1c5+nfQS1L31sFHZ9/YzTYqWyjU=
+github.com/matrix-org/complement v0.0.0-20240126134841-458bfba5f7f3 h1:M8k58YcnDn/0wSVLYyuei21VnJOd9CitzUuMo0xxIVY=
+github.com/matrix-org/complement v0.0.0-20240126134841-458bfba5f7f3/go.mod h1:GMCwbgMOudedB86u1c5+nfQS1L31sFHZ9/YzTYqWyjU=
 github.com/matrix-org/gomatrix v0.0.0-20220926102614-ceba4d9f7530 h1:kHKxCOLcHH8r4Fzarl4+Y3K5hjothkVW5z7T1dUM11U=
 github.com/matrix-org/gomatrix v0.0.0-20220926102614-ceba4d9f7530/go.mod h1:/gBX06Kw0exX1HrwmoBibFA98yBk/jxKpGVeyQbff+s=
 github.com/matrix-org/gomatrixserverlib v0.0.0-20230921171121-0466775328c7 h1:NhPNNFLHwdDb/upeicBh1GkxX/sFinEp5HF1WBqPtiY=

internal/api/client.go

@@ -67,6 +67,7 @@ type Client interface {
 	// The user for this client
 	UserID() string
 	Type() ClientTypeLang
+	Opts() ClientCreationOpts
 }
 
 type LoggedClient struct {
@@ -184,11 +185,11 @@ type ClientCreationOpts struct {
 	DeviceID string
 }
 
-func FromComplementClient(c *client.CSAPI, password string) ClientCreationOpts {
+func NewClientCreationOpts(c *client.CSAPI) ClientCreationOpts {
 	return ClientCreationOpts{
 		BaseURL:  c.BaseURL,
 		UserID:   c.UserID,
-		Password: password,
+		Password: c.Password,
 		DeviceID: c.DeviceID,
 	}
 }

internal/api/js/js.go

@@ -58,13 +58,15 @@ type JSClient struct {
 	listenerID  atomic.Int32
 	listenersMu *sync.RWMutex
 	userID      string
+	opts        api.ClientCreationOpts
 }
 
 func NewJSClient(t ct.TestLike, opts api.ClientCreationOpts) (api.Client, error) {
 	jsc := &JSClient{
 		listeners:   make(map[int32]func(roomID string, ev api.Event)),
 		userID:      opts.UserID,
 		listenersMu: &sync.RWMutex{},
+		opts:        opts,
 	}
 	portKey := opts.UserID + opts.DeviceID
 	browser, err := chrome.RunHeadless(func(s string) {
@@ -247,6 +249,10 @@ func (c *JSClient) UserID() string {
 	return c.userID
 }
 
+func (c *JSClient) Opts() api.ClientCreationOpts {
+	return c.opts
+}
+
 func (c *JSClient) MustGetEvent(t ct.TestLike, roomID, eventID string) api.Event {
 	t.Helper()
 	// serialised output (if encrypted):

internal/api/rust/rust.go

@@ -43,6 +43,7 @@ type RustClient struct {
 	roomsMu               *sync.RWMutex
 	userID                string
 	persistentStoragePath string
+	opts                  api.ClientCreationOpts
 }
 
 func NewRustClient(t ct.TestLike, opts api.ClientCreationOpts, ssURL string) (api.Client, error) {
@@ -65,6 +66,7 @@ func NewRustClient(t ct.TestLike, opts api.ClientCreationOpts, ssURL string) (ap
 		rooms:     make(map[string]*RustRoomInfo),
 		listeners: make(map[int32]func(roomID string)),
 		roomsMu:   &sync.RWMutex{},
+		opts:      opts,
 	}
 	if opts.PersistentStorage {
 		c.persistentStoragePath = "./rust_storage/" + username
@@ -73,6 +75,10 @@ func NewRustClient(t ct.TestLike, opts api.ClientCreationOpts, ssURL string) (ap
 	return &api.LoggedClient{Client: c}, nil
 }
 
+func (c *RustClient) Opts() api.ClientCreationOpts {
+	return c.opts
+}
+
 func (c *RustClient) Login(t ct.TestLike, opts api.ClientCreationOpts) error {
 	var deviceID *string
 	if opts.DeviceID != "" {

tests/federation_connectivity_test.go

@@ -49,11 +49,11 @@ func TestNewUserCannotGetKeysForOfflineServer(t *testing.T) {
 	ss := deployment.SlidingSyncURL(t)
 	alice := MustCreateClient(t,
 		api.ClientType{HS: "hs1", Lang: api.ClientTypeRust},
-		api.FromComplementClient(csapiAlice, "complement-crypto-password"), ss, WithDoLogin(t))
+		api.NewClientCreationOpts(csapiAlice), ss, WithDoLogin(t))
 	defer alice.Close(t)
 	bob := MustCreateClient(t,
 		api.ClientType{HS: "hs2", Lang: api.ClientTypeJS},
-		api.FromComplementClient(csapiBob, "complement-crypto-password"), ss, WithDoLogin(t))
+		api.NewClientCreationOpts(csapiBob), ss, WithDoLogin(t))
 	defer bob.Close(t)
 	aliceStopSyncing := alice.MustStartSyncing(t)
 	defer aliceStopSyncing()
@@ -77,7 +77,7 @@ func TestNewUserCannotGetKeysForOfflineServer(t *testing.T) {
 	csapiAlice.MustInviteRoom(t, roomID, csapiCharlie.UserID)
 	charlie := MustCreateClient(t,
 		api.ClientType{HS: "hs1", Lang: api.ClientTypeRust},
-		api.FromComplementClient(csapiCharlie, "complement-crypto-password"), ss, WithDoLogin(t))
+		api.NewClientCreationOpts(csapiCharlie), ss, WithDoLogin(t))
 	defer charlie.Close(t)
 	charlieStopSyncing := charlie.MustStartSyncing(t)
 	defer charlieStopSyncing()
@@ -171,15 +171,15 @@ func TestExistingSessionCannotGetKeysForOfflineServer(t *testing.T) {
 	ss := deployment.SlidingSyncURL(t)
 	alice := MustCreateClient(t,
 		api.ClientType{HS: "hs1", Lang: api.ClientTypeRust},
-		api.FromComplementClient(csapiAlice, "complement-crypto-password"), ss, WithDoLogin(t))
+		api.NewClientCreationOpts(csapiAlice), ss, WithDoLogin(t))
 	defer alice.Close(t)
 	bob := MustCreateClient(t,
 		api.ClientType{HS: "hs2", Lang: api.ClientTypeJS},
-		api.FromComplementClient(csapiBob, "complement-crypto-password"), ss, WithDoLogin(t))
+		api.NewClientCreationOpts(csapiBob), ss, WithDoLogin(t))
 	defer bob.Close(t)
 	charlie := MustCreateClient(t,
 		api.ClientType{HS: "hs1", Lang: api.ClientTypeRust},
-		api.FromComplementClient(csapiCharlie, "complement-crypto-password"), ss, WithDoLogin(t))
+		api.NewClientCreationOpts(csapiCharlie), ss, WithDoLogin(t))
 	defer charlie.Close(t)
 	aliceStopSyncing := alice.MustStartSyncing(t)
 	defer aliceStopSyncing()

tests/key_backup_test.go

@@ -18,12 +18,8 @@ func TestCanBackupKeys(t *testing.T) {
 			return
 		}
 		t.Logf("backup creator = %s backup restorer = %s", clientTypeA.Lang, clientTypeB.Lang)
-		deployment := Deploy(t)
-		csapiAlice := deployment.Register(t, clientTypeA.HS, helpers.RegistrationOpts{
-			LocalpartSuffix: "alice",
-			Password:        "complement-crypto-password",
-		})
-		roomID := csapiAlice.MustCreateRoom(t, map[string]interface{}{
+		tc := CreateTestContext(t, clientTypeA)
+		roomID := tc.Alice.MustCreateRoom(t, map[string]interface{}{
 			"name":   t.Name(),
 			"preset": "public_chat", // shared history visibility
 			"invite": []string{},
@@ -41,7 +37,7 @@ func TestCanBackupKeys(t *testing.T) {
 		// SDK testing below
 		// -----------------
 
-		backupCreator := LoginClientFromComplementClient(t, deployment, csapiAlice, clientTypeA)
+		backupCreator := tc.MustLoginClient(t, tc.Alice, clientTypeA)
 		defer backupCreator.Close(t)
 		stopSyncing := backupCreator.MustStartSyncing(t)
 		defer stopSyncing()
@@ -57,11 +53,11 @@ func TestCanBackupKeys(t *testing.T) {
 		t.Logf("recovery key -> %s", recoveryKey)
 
 		// Now login on a new device
-		csapiAlice2 := deployment.Login(t, clientTypeB.HS, csapiAlice, helpers.LoginOpts{
+		csapiAlice2 := tc.Deployment.Login(t, clientTypeB.HS, tc.Alice, helpers.LoginOpts{
 			DeviceID: "BACKUP_RESTORER",
 			Password: "complement-crypto-password",
 		})
-		backupRestorer := LoginClientFromComplementClient(t, deployment, csapiAlice2, clientTypeB)
+		backupRestorer := tc.MustLoginClient(t, csapiAlice2, clientTypeB)
 		defer backupRestorer.Close(t)
 
 		// load the key backup using the recovery key
@@ -86,12 +82,8 @@ func TestBackupWrongRecoveryKeyFails(t *testing.T) {
 			return
 		}
 		t.Logf("backup creator = %s backup restorer = %s", clientTypeA.Lang, clientTypeB.Lang)
-		deployment := Deploy(t)
-		csapiAlice := deployment.Register(t, clientTypeA.HS, helpers.RegistrationOpts{
-			LocalpartSuffix: "alice",
-			Password:        "complement-crypto-password",
-		})
-		roomID := csapiAlice.MustCreateRoom(t, map[string]interface{}{
+		tc := CreateTestContext(t, clientTypeA)
+		roomID := tc.Alice.MustCreateRoom(t, map[string]interface{}{
 			"name":   t.Name(),
 			"preset": "public_chat", // shared history visibility
 			"invite": []string{},
@@ -109,7 +101,7 @@ func TestBackupWrongRecoveryKeyFails(t *testing.T) {
 		// SDK testing below
 		// -----------------
 
-		backupCreator := LoginClientFromComplementClient(t, deployment, csapiAlice, clientTypeA)
+		backupCreator := tc.MustLoginClient(t, tc.Alice, clientTypeA)
 		defer backupCreator.Close(t)
 		stopSyncing := backupCreator.MustStartSyncing(t)
 		defer stopSyncing()
@@ -125,11 +117,11 @@ func TestBackupWrongRecoveryKeyFails(t *testing.T) {
 		t.Logf("recovery key -> %s", recoveryKey)
 
 		// Now login on a new device
-		csapiAlice2 := deployment.Login(t, clientTypeB.HS, csapiAlice, helpers.LoginOpts{
+		csapiAlice2 := tc.Deployment.Login(t, clientTypeB.HS, tc.Alice, helpers.LoginOpts{
 			DeviceID: "BACKUP_RESTORER",
 			Password: "complement-crypto-password",
 		})
-		backupRestorer := LoginClientFromComplementClient(t, deployment, csapiAlice2, clientTypeB)
+		backupRestorer := tc.MustLoginClient(t, csapiAlice2, clientTypeB)
 		defer backupRestorer.Close(t)
 
 		// load the key backup using a valid but wrong recovery key

tests/main_test.go

@@ -141,23 +141,46 @@ func (c *TestContext) CreateNewEncryptedRoom(t *testing.T, creator *client.CSAPI
 	})
 }
 
-func (c *TestContext) MustLoginDevice(t *testing.T, existing *client.CSAPI, clientType api.ClientType, deviceID string) (*client.CSAPI, api.Client) {
-	newClient := c.Deployment.Login(t, clientType.HS, existing, helpers.LoginOpts{
-		DeviceID: deviceID,
-		Password: "complement-crypto-password",
+func (c *TestContext) OptsFromClient(t *testing.T, existing *client.CSAPI, options ...func(*api.ClientCreationOpts)) api.ClientCreationOpts {
+	o := &api.ClientCreationOpts{
+		BaseURL:  existing.BaseURL,
+		UserID:   existing.UserID,
+		DeviceID: existing.DeviceID,
+		Password: existing.Password,
+	}
+	for _, opt := range options {
+		opt(o)
+	}
+	return *o
+}
+
+func WithPersistentStorage() func(*api.ClientCreationOpts) {
+	return func(o *api.ClientCreationOpts) {
+		o.PersistentStorage = true
+	}
+}
+
+func (c *TestContext) MustRegisterNewDevice(t *testing.T, cli *client.CSAPI, hsName, newDeviceID string) *client.CSAPI {
+	return c.Deployment.Login(t, hsName, cli, helpers.LoginOpts{
+		DeviceID: newDeviceID,
+		Password: cli.Password,
 	})
-	return newClient, c.MustLoginClient(t, newClient, clientType)
 }
 
-func (c *TestContext) MustLoginClient(t *testing.T, cli *client.CSAPI, clientType api.ClientType) api.Client {
-	return LoginClientFromComplementClient(t, c.Deployment, cli, clientType)
+func (c *TestContext) MustCreateClient(t *testing.T, cli *client.CSAPI, clientType api.ClientType, options ...func(*api.ClientCreationOpts)) api.Client {
+	t.Helper()
+	cfg := api.NewClientCreationOpts(cli)
+	cfg.BaseURL = c.Deployment.ReverseProxyURLForHS(clientType.HS)
+	for _, opt := range options {
+		opt(&cfg)
+	}
+	client := MustCreateClient(t, clientType, cfg, c.Deployment.SlidingSyncURL(t))
+	return client
 }
 
-func LoginClientFromComplementClient(t *testing.T, dep *deploy.SlidingSyncDeployment, cli *client.CSAPI, clientType api.ClientType) api.Client {
+func (c *TestContext) MustLoginClient(t *testing.T, cli *client.CSAPI, clientType api.ClientType, options ...func(*api.ClientCreationOpts)) api.Client {
 	t.Helper()
-	cfg := api.FromComplementClient(cli, "complement-crypto-password")
-	cfg.BaseURL = dep.ReverseProxyURLForHS(clientType.HS)
-	client := MustCreateClient(t, clientType, cfg, dep.SlidingSyncURL(t))
-	must.NotError(t, "failed to login client", client.Login(t, cfg))
+	client := c.MustCreateClient(t, cli, clientType, options...)
+	must.NotError(t, "failed to login client", client.Login(t, client.Opts()))
 	return client
 }

tests/membership_acls_test.go

@@ -277,7 +277,8 @@ func TestOnNewDeviceBobCanSeeButNotDecryptHistoryInPublicRoom(t *testing.T) {
 		waiter.Wait(t, 5*time.Second)
 
 		// now bob logs in on a new device. He should NOT be able to decrypt this event (though can see it due to history visibility)
-		csapiBob2, bob2 := tc.MustLoginDevice(t, tc.Bob, clientTypeB, "NEW_DEVICE")
+		csapiBob2 := tc.MustRegisterNewDevice(t, tc.Bob, clientTypeB.HS, "NEW_DEVICE")
+		bob2 := tc.MustLoginClient(t, csapiBob2, clientTypeB)
 		bob2StopSyncing := bob2.MustStartSyncing(t)
 		bob2StoppedSyncing := false
 		defer func() {
@@ -353,7 +354,8 @@ func TestChangingDeviceAfterInviteReEncrypts(t *testing.T) {
 		evID := alice.SendMessage(t, roomID, body)
 
 		// now Bob logs in on a different device and accepts the invite. The different device should be able to decrypt the message.
-		_, bob2 := tc.MustLoginDevice(t, tc.Bob, clientTypeB, "NEW_DEVICE")
+		csapiBob2 := tc.MustRegisterNewDevice(t, tc.Bob, clientTypeB.HS, "NEW_DEVICE")
+		bob2 := tc.MustLoginClient(t, csapiBob2, clientTypeB)
 		bob2StopSyncing := bob2.MustStartSyncing(t)
 		defer bob2StopSyncing()
 

tests/one_time_keys_test.go

@@ -95,7 +95,7 @@ func TestFallbackKeyIsUsedIfOneTimeKeysRunOut(t *testing.T) {
 		// =================
 
 		// Upload OTKs and a fallback
-		alice := LoginClientFromComplementClient(t, tc.Deployment, tc.Alice, clientTypeA)
+		alice := tc.MustLoginClient(t, tc.Alice, clientTypeA)
 		defer alice.Close(t)
 		aliceStopSyncing := alice.MustStartSyncing(t)
 		defer aliceStopSyncing()
@@ -106,7 +106,7 @@ func TestFallbackKeyIsUsedIfOneTimeKeysRunOut(t *testing.T) {
 		tc.Alice.MustCreateRoom(t, map[string]interface{}{})
 
 		// also let bob upload OTKs before we block the upload endpoint!
-		bob := LoginClientFromComplementClient(t, tc.Deployment, tc.Bob, clientTypeB)
+		bob := tc.MustLoginClient(t, tc.Bob, clientTypeB)
 		defer bob.Close(t)
 		bobStopSyncing := bob.MustStartSyncing(t)
 		defer bobStopSyncing()
@@ -200,7 +200,7 @@ func TestFailedOneTimeKeyUploadRetries(t *testing.T) {
 				"filter":        "~u .*\\/keys\\/upload.* ~m POST",
 			},
 		}, func() {
-			alice := LoginClientFromComplementClient(t, tc.Deployment, tc.Alice, clientType)
+			alice := tc.MustLoginClient(t, tc.Alice, clientType)
 			defer alice.Close(t)
 			aliceStopSyncing := alice.MustStartSyncing(t)
 			defer aliceStopSyncing()
@@ -248,11 +248,11 @@ func TestFailedKeysClaimRetries(t *testing.T) {
 	ForEachClientType(t, func(t *testing.T, clientType api.ClientType) {
 		tc := CreateTestContext(t, clientType, clientType)
 		// both clients start syncing to upload OTKs
-		alice := LoginClientFromComplementClient(t, tc.Deployment, tc.Alice, clientType)
+		alice := tc.MustLoginClient(t, tc.Alice, clientType)
 		defer alice.Close(t)
 		aliceStopSyncing := alice.MustStartSyncing(t)
 		defer aliceStopSyncing()
-		bob := LoginClientFromComplementClient(t, tc.Deployment, tc.Bob, clientType)
+		bob := tc.MustLoginClient(t, tc.Bob, clientType)
 		defer bob.Close(t)
 		bobStopSyncing := bob.MustStartSyncing(t)
 		defer bobStopSyncing()

tests/room_keys_test.go

@@ -20,7 +20,8 @@ func TestRoomKeyIsCycledOnDeviceLeaving(t *testing.T) {
 		// Alice, Alice2 and Bob are in a room.
 		alice := tc.MustLoginClient(t, tc.Alice, clientTypeA)
 		defer alice.Close(t)
-		csapiAlice2, alice2 := tc.MustLoginDevice(t, tc.Alice, clientTypeA, "OTHER_DEVICE")
+		csapiAlice2 := tc.MustRegisterNewDevice(t, tc.Alice, clientTypeA.HS, "OTHER_DEVICE")
+		alice2 := tc.MustLoginClient(t, csapiAlice2, clientTypeA)
 		defer alice2.Close(t)
 		bob := tc.MustLoginClient(t, tc.Bob, clientTypeB)
 		defer bob.Close(t)

tests/state_synchronisation_test.go

@@ -61,7 +61,7 @@ func testSigkillBeforeKeysUploadResponseRust(t *testing.T, clientType api.Client
 			"filter":       "~u .*\\/keys\\/upload.*",
 		},
 	}, func() {
-		cfg := api.FromComplementClient(tc.Alice, "complement-crypto-password")
+		cfg := api.NewClientCreationOpts(tc.Alice)
 		cfg.BaseURL = tc.Deployment.ReverseProxyURLForHS(clientType.HS)
 		cfg.PersistentStorage = true
 		// run some code in a separate process so we can kill it later
@@ -140,10 +140,7 @@ func testSigkillBeforeKeysUploadResponseJS(t *testing.T, clientType api.ClientTy
 			"filter":       "~u .*\\/keys\\/upload.*",
 		},
 	}, func() {
-		cfg := api.FromComplementClient(tc.Alice, "complement-crypto-password")
-		cfg.BaseURL = tc.Deployment.ReverseProxyURLForHS(clientType.HS)
-		cfg.PersistentStorage = true
-		clientWhichWillBeKilled := MustCreateClient(t, clientType, cfg, tc.Deployment.SlidingSyncURL(t))
+		clientWhichWillBeKilled := tc.MustCreateClient(t, tc.Alice, clientType, WithPersistentStorage())
 		// attempt to login, this should cause OTKs to be uploaded
 		waiter := helpers.NewWaiter()
 		terminateClient = func() {
@@ -154,7 +151,7 @@ func testSigkillBeforeKeysUploadResponseJS(t *testing.T, clientType api.ClientTy
 			waiter.Finish()
 		}
 		go func() {
-			must.NotError(t, "failed to login", clientWhichWillBeKilled.Login(t, cfg))
+			must.NotError(t, "failed to login", clientWhichWillBeKilled.Login(t, clientWhichWillBeKilled.Opts()))
 			// need to start syncing to make JS do /keys/upload
 			// we don't need to stopSyncing because we'll SIGKILL this.
 			clientWhichWillBeKilled.StartSyncing(t)
@@ -163,9 +160,10 @@ func testSigkillBeforeKeysUploadResponseJS(t *testing.T, clientType api.ClientTy
 		waiter.Wait(t, 5*time.Second) // wait for /keys/upload and subsequent SIGKILL
 		t.Logf("terminated browser, making new client")
 		// now make the same client
-		recreatedClient := MustCreateClient(t, clientType, cfg, tc.Deployment.SlidingSyncURL(t))
-		recreatedClient.Login(t, cfg)   // login should work
-		recreatedClient.StartSyncing(t) // ignore errors, we just need to kick it to /keys/upload
+		recreatedClient := tc.MustCreateClient(t, tc.Alice, clientType, WithPersistentStorage())
+		recreatedClient.Login(t, recreatedClient.Opts()) // login should work
+		recreatedClient.StartSyncing(t)                  // ignore errors, we just need to kick it to /keys/upload
+
 		recreatedClient.DeletePersistentStorage(t)
 		recreatedClient.Close(t)
 		// ensure we see the 2nd keys/upload