-
-
Notifications
You must be signed in to change notification settings - Fork 590
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
crypto: configure key sharing strategy based on DeviceIsolationMode #4425
Conversation
522b369
to
2be0544
Compare
Will need to be updated in light of #4428 |
2be0544
to
4050919
Compare
4050919
to
5e43195
Compare
fix eslint import error cryptoMode was renamed to deviceIsolationMode post rebase fix: Device Isolation mode name changes
5e43195
to
87f5304
Compare
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me in general. A couple of nits
src/rust-crypto/RoomEncryptor.ts
Outdated
public async prepareForEncryption(globalBlacklistUnverifiedDevices: boolean): Promise<void> { | ||
public async prepareForEncryption( | ||
globalBlacklistUnverifiedDevices: boolean, | ||
deviceIsolationMode: DeviceIsolationMode = new AllDevicesIsolationMode(false), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
given that this is only called in one place (except in tests), we should not provide a default value.
Likewise below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM otherwise |
Fixes element-hq/element-web#28038
based on the work done on #4407
Checklist
public
/exported
symbols have accurate TSDoc documentation.