From 65d2e4de9e7963c52bc344084cb8aadb90f07e1f Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Fri, 27 Sep 2024 16:47:51 +0100 Subject: [PATCH] Add ClientCreationOpts.ExtraOpts and move rust specific options to it This avoids polluting the shared client API with client-specific options. --- internal/api/client.go | 31 ++++++-- internal/api/rust/rust.go | 22 +++-- tests/rust/notification_test.go | 137 ++++++++++++++++++++------------ 3 files changed, 127 insertions(+), 63 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index 9efbb63..36797e6 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -216,10 +216,6 @@ func (c *LoggedClient) logPrefix() string { return fmt.Sprintf("[%s](%s)", c.UserID(), c.Type()) } -// magic value for EnableCrossProcessRefreshLockProcessName which configures the FFI client -// according to iOS NSE. -const ProcessNameNSE string = "NSE" - // ClientCreationOpts are options to use when creating crypto clients. // // This contains a mixture of generic options which can be used across any client, and specific @@ -241,13 +237,27 @@ type ClientCreationOpts struct { // this flag and always use persistence. PersistentStorage bool - // Rust only. If set, enables the cross process refresh lock on the FFI client with the process name provided. - EnableCrossProcessRefreshLockProcessName string + // A map containing any client-specific creation options, for use for client-specific tests. + // Any options in this map MUST BE SERIALISABLE as they may be sent over RPC boundaries. + ExtraOpts map[string]any + // Rust only. If set with EnableCrossProcessRefreshLockProcessName=ProcessNameNSE, the client will be seeded // with a logged in session. AccessToken string } +// GetExtraOption is a safe way to get an extra option from ExtraOpts, with a default value if the key does not exist. +func (o *ClientCreationOpts) GetExtraOption(key string, defaultValue any) any { + if o.ExtraOpts == nil { + return defaultValue + } + val, ok := o.ExtraOpts[key] + if !ok { + return defaultValue + } + return val +} + func NewClientCreationOpts(c *client.CSAPI) ClientCreationOpts { return ClientCreationOpts{ BaseURL: c.BaseURL, @@ -268,8 +278,13 @@ func (o *ClientCreationOpts) Combine(other *ClientCreationOpts) { if other.DeviceID != "" { o.DeviceID = other.DeviceID } - if other.EnableCrossProcessRefreshLockProcessName != "" { - o.EnableCrossProcessRefreshLockProcessName = other.EnableCrossProcessRefreshLockProcessName + if other.ExtraOpts != nil { + if o.ExtraOpts == nil { + o.ExtraOpts = make(map[string]any) + } + for k, v := range other.ExtraOpts { + o.ExtraOpts[k] = v + } } if other.Password != "" { o.Password = other.Password diff --git a/internal/api/rust/rust.go b/internal/api/rust/rust.go index 13228fe..4b90757 100644 --- a/internal/api/rust/rust.go +++ b/internal/api/rust/rust.go @@ -41,6 +41,14 @@ func SetupLogs(prefix string) { var zero uint32 +const ( + OptionEnableCrossProcessRefreshLockProcessName = "EnableCrossProcessRefreshLockProcessName" +) + +// magic value for EnableCrossProcessRefreshLockProcessName which configures the FFI client +// according to iOS NSE. +const ProcessNameNSE string = "NSE" + type RustRoomInfo struct { stream *matrix_sdk_ffi.TaskHandle room *matrix_sdk_ffi.Room @@ -71,10 +79,11 @@ func NewRustClient(t ct.TestLike, opts api.ClientCreationOpts) (api.Client, erro SlidingSyncVersionBuilder(slidingSyncVersion). AutoEnableCrossSigning(true) var clientSessionDelegate matrix_sdk_ffi.ClientSessionDelegate - if opts.EnableCrossProcessRefreshLockProcessName != "" { - t.Logf("enabling cross process refresh lock with proc name=%s", opts.EnableCrossProcessRefreshLockProcessName) + xprocessName := opts.GetExtraOption(OptionEnableCrossProcessRefreshLockProcessName, "").(string) + if xprocessName != "" { + t.Logf("enabling cross process refresh lock with proc name=%s", xprocessName) clientSessionDelegate = NewMemoryClientSessionDelegate() - ab = ab.EnableCrossProcessRefreshLock(opts.EnableCrossProcessRefreshLockProcessName, clientSessionDelegate) + ab = ab.EnableCrossProcessRefreshLock(xprocessName, clientSessionDelegate) } // @alice:hs1, FOOBAR => alice_hs1_FOOBAR username := strings.Replace(opts.UserID[1:], ":", "_", -1) + "_" + opts.DeviceID @@ -104,7 +113,7 @@ func NewRustClient(t ct.TestLike, opts api.ClientCreationOpts) (api.Client, erro if err := client.RestoreSession(session); err != nil { return nil, fmt.Errorf("RestoreSession: %s", err) } - if opts.EnableCrossProcessRefreshLockProcessName == api.ProcessNameNSE { + if xprocessName == ProcessNameNSE { clientSessionDelegate.SaveSessionInKeychain(session) t.Logf("configure NSE client with logged in user: %+v", session) // We purposefully don't SetDelegate as it appears to be unnecessary. @@ -333,8 +342,9 @@ func (c *RustClient) StartSyncing(t ct.TestLike) (stopSyncing func(), err error) // > thread '' panicked at 'there is no reactor running, must be called from the context of a Tokio 1.x runtime' // where the stack trace doesn't hit any test code, but does start at a `free_` function. sb := c.FFIClient.SyncService() - if c.opts.EnableCrossProcessRefreshLockProcessName != "" { - sb2 := sb.WithCrossProcessLock(&c.opts.EnableCrossProcessRefreshLockProcessName) + xprocessName := c.opts.GetExtraOption(OptionEnableCrossProcessRefreshLockProcessName, "").(string) + if xprocessName != "" { + sb2 := sb.WithCrossProcessLock(&xprocessName) sb.Destroy() sb = sb2 } diff --git a/tests/rust/notification_test.go b/tests/rust/notification_test.go index 0b78db7..f791322 100644 --- a/tests/rust/notification_test.go +++ b/tests/rust/notification_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/matrix-org/complement-crypto/internal/api" + "github.com/matrix-org/complement-crypto/internal/api/rust" "github.com/matrix-org/complement-crypto/internal/cc" "github.com/matrix-org/complement-crypto/internal/deploy/callback" "github.com/matrix-org/complement-crypto/internal/deploy/mitm" @@ -60,8 +61,10 @@ func testNSEReceive(t *testing.T, numMsgsBefore, numMsgsAfter int) { alice := tc.MustLoginClient(t, &cc.ClientCreationRequest{ User: tc.Alice, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - EnableCrossProcessRefreshLockProcessName: "main", + PersistentStorage: true, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: "main", + }, }, }) alice.Logf(t, "syncing and sending dummy message to ensure e2ee keys are uploaded") @@ -83,9 +86,11 @@ func testNSEReceive(t *testing.T, numMsgsBefore, numMsgsAfter int) { User: tc.Alice, Multiprocess: true, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - EnableCrossProcessRefreshLockProcessName: api.ProcessNameNSE, - AccessToken: accessToken, + PersistentStorage: true, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: rust.ProcessNameNSE, + }, + AccessToken: accessToken, }, }) // this should login already as we provided an access token defer client.Close(t) @@ -103,8 +108,10 @@ func TestNSEReceiveForNonPreKeyMessage(t *testing.T) { alice := tc.MustLoginClient(t, &cc.ClientCreationRequest{ User: tc.Alice, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - EnableCrossProcessRefreshLockProcessName: "main", + PersistentStorage: true, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: "main", + }, }, }) stopSyncing := alice.MustStartSyncing(t) @@ -128,9 +135,11 @@ func TestNSEReceiveForNonPreKeyMessage(t *testing.T) { client := tc.MustCreateClient(t, &cc.ClientCreationRequest{ User: tc.Alice, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - EnableCrossProcessRefreshLockProcessName: api.ProcessNameNSE, - AccessToken: accessToken, + PersistentStorage: true, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: rust.ProcessNameNSE, + }, + AccessToken: accessToken, }, }) // this should login already as we provided an access token defer client.Close(t) @@ -153,8 +162,10 @@ func TestMultiprocessNSE(t *testing.T) { alice := tc.MustLoginClient(t, &cc.ClientCreationRequest{ User: tc.Alice, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - EnableCrossProcessRefreshLockProcessName: "main", + PersistentStorage: true, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: "main", + }, }, }) stopSyncing := alice.MustStartSyncing(t) @@ -188,9 +199,11 @@ func TestMultiprocessNSE(t *testing.T) { alice = tc.MustCreateClient(t, &cc.ClientCreationRequest{ User: tc.Alice, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - EnableCrossProcessRefreshLockProcessName: "main", - AccessToken: accessToken, + PersistentStorage: true, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: "main", + }, + AccessToken: accessToken, }, }) // this should login already as we provided an access token stopSyncing = alice.MustStartSyncing(t) @@ -206,9 +219,11 @@ func TestMultiprocessNSE(t *testing.T) { User: tc.Alice, Multiprocess: true, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - AccessToken: accessToken, - EnableCrossProcessRefreshLockProcessName: api.ProcessNameNSE, + PersistentStorage: true, + AccessToken: accessToken, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: "main", + }, }, }) // this should login already as we provided an access token @@ -244,9 +259,11 @@ func TestMultiprocessNSE(t *testing.T) { User: tc.Alice, Multiprocess: true, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - EnableCrossProcessRefreshLockProcessName: api.ProcessNameNSE, - AccessToken: accessToken, + PersistentStorage: true, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: rust.ProcessNameNSE, + }, + AccessToken: accessToken, }, }) } // else we reuse the same NSE process for bob's message @@ -281,8 +298,10 @@ func TestMultiprocessNSE(t *testing.T) { alice2 := tc.MustLoginClient(t, &cc.ClientCreationRequest{ User: newDevice, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - EnableCrossProcessRefreshLockProcessName: "main", + PersistentStorage: true, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: "main", + }, }, }) alice2.MustLoadBackup(t, recoveryKey) @@ -303,8 +322,10 @@ func TestMultiprocessNSEBackupKeyMacError(t *testing.T) { alice := tc.MustLoginClient(t, &cc.ClientCreationRequest{ User: tc.Alice, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - EnableCrossProcessRefreshLockProcessName: "main", + PersistentStorage: true, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: "main", + }, }, }) stopSyncing := alice.MustStartSyncing(t) @@ -334,9 +355,11 @@ func TestMultiprocessNSEBackupKeyMacError(t *testing.T) { alice = tc.MustCreateClient(t, &cc.ClientCreationRequest{ User: tc.Alice, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - AccessToken: accessToken, - EnableCrossProcessRefreshLockProcessName: "main", + PersistentStorage: true, + AccessToken: accessToken, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: "main", + }, }, }) // this should login already as we provided an access token stopSyncing = alice.MustStartSyncing(t) @@ -352,9 +375,11 @@ func TestMultiprocessNSEBackupKeyMacError(t *testing.T) { User: tc.Alice, Multiprocess: true, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - EnableCrossProcessRefreshLockProcessName: api.ProcessNameNSE, - AccessToken: accessToken, + PersistentStorage: true, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: rust.ProcessNameNSE, + }, + AccessToken: accessToken, }, }) // this should login already as we provided an access token @@ -392,8 +417,10 @@ func TestMultiprocessNSEBackupKeyMacError(t *testing.T) { alice2 := tc.MustLoginClient(t, &cc.ClientCreationRequest{ User: newDevice, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - EnableCrossProcessRefreshLockProcessName: "main", + PersistentStorage: true, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: "main", + }, }, }) alice2.MustLoadBackup(t, recoveryKey) @@ -414,8 +441,10 @@ func TestMultiprocessNSEOlmSessionWedge(t *testing.T) { alice := tc.MustLoginClient(t, &cc.ClientCreationRequest{ User: tc.Alice, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - EnableCrossProcessRefreshLockProcessName: "main", + PersistentStorage: true, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: "main", + }, }, }) stopSyncing := alice.MustStartSyncing(t) @@ -447,9 +476,11 @@ func TestMultiprocessNSEOlmSessionWedge(t *testing.T) { alice = tc.MustCreateClient(t, &cc.ClientCreationRequest{ User: tc.Alice, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - EnableCrossProcessRefreshLockProcessName: "main", - AccessToken: accessToken, + PersistentStorage: true, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: "main", + }, + AccessToken: accessToken, }, }) // this should login already as we provided an access token stopSyncing = alice.MustStartSyncing(t) @@ -469,9 +500,11 @@ func TestMultiprocessNSEOlmSessionWedge(t *testing.T) { User: tc.Alice, Multiprocess: true, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - AccessToken: accessToken, - EnableCrossProcessRefreshLockProcessName: api.ProcessNameNSE, + PersistentStorage: true, + AccessToken: accessToken, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: rust.ProcessNameNSE, + }, }, }) // this should login already as we provided an access token @@ -604,8 +637,10 @@ func TestMultiprocessInitialE2EESyncDoesntDropDeviceListUpdates(t *testing.T) { bob := tc.MustLoginClient(t, &cc.ClientCreationRequest{ User: tc.Bob, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - EnableCrossProcessRefreshLockProcessName: "main", + PersistentStorage: true, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: "main", + }, }, }) stopSyncing := bob.MustStartSyncing(t) @@ -636,9 +671,11 @@ func TestMultiprocessInitialE2EESyncDoesntDropDeviceListUpdates(t *testing.T) { bob = tc.MustCreateClient(t, &cc.ClientCreationRequest{ User: tc.Bob, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - EnableCrossProcessRefreshLockProcessName: "main", - AccessToken: accessToken, + PersistentStorage: true, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: "main", + }, + AccessToken: accessToken, }, }) // this should login already as we provided an access token stopSyncing = bob.MustStartSyncing(t) @@ -647,9 +684,11 @@ func TestMultiprocessInitialE2EESyncDoesntDropDeviceListUpdates(t *testing.T) { User: tc.Bob, Multiprocess: true, Opts: api.ClientCreationOpts{ - PersistentStorage: true, - AccessToken: accessToken, - EnableCrossProcessRefreshLockProcessName: api.ProcessNameNSE, + PersistentStorage: true, + AccessToken: accessToken, + ExtraOpts: map[string]any{ + rust.OptionEnableCrossProcessRefreshLockProcessName: rust.ProcessNameNSE, + }, }, }) // this should login already as we provided an access token