crypto: Replace cryptoMode with DeviceIsolationMode concept #4429
Conversation
src/crypto-api/index.ts
Outdated
| */ | ||
| Transition, | ||
| public errorOnVerifiedUserProblems: boolean; |
There was a problem hiding this comment.
This option is not used yet in this PR. It is used for encrypting. Added here to showcase that IsolationMode is not just an enum but need more information
BillCarsonFr
force-pushed
the
valere/rename_crypto_mode
branch
from
bf42611
to
ab23354
Compare
src/rust-crypto/rust-crypto.ts
Outdated
| case CryptoMode.Transition: | ||
|
|
||
| switch (isolationMode.kind) { | ||
| case "OnlySignedIsolation": | ||
| trustRequirement = RustSdkCryptoJs.TrustRequirement.CrossSignedOrLegacy; |
There was a problem hiding this comment.
CrossSignedOrLegacy is the mode that we want. CrossSigned only would be for a more extreme mode that we don't have yet.
This comment was marked as resolved.
This comment was marked as resolved.
src/crypto-api/index.ts
Outdated
| * Events are decrypted only if they come from a cross-signed device other events will result in a decryption | ||
| * failure. (To access the failure reason, see {@link MatrixEvent.decryptionFailureReason}.) | ||
| */ | ||
| export class OnlySignedIsolation { |
There was a problem hiding this comment.
WDYT about calling this OnlySignedDeviceIsolationMode ? I know it's a bit of a long name but I feel it gives a better indication of what it does.
(And... NoIsolationDeviceIsolationMode ?)
There was a problem hiding this comment.
I find the name very long and strange (NoIsolationIsolationMode).
What about namespaces? First time I see that, but would allow to do new DeviceIsolationMode.OnlySigned() so it's a clear indication that these are DeviceIsolationMode without the need of bigger name, also easy to discover?
export namespace DeviceIsolationMode {
export class None {..}
export class OnlySigned {..}
}
There was a problem hiding this comment.
Per my monologue in #element-dev:matrix.org, I'm not enthusiastic about namespaces, because they seem to be deprecated (and we have a lint rule that enforces that we don't use them).
There was a problem hiding this comment.
Honestly, I don't find NoIsolationIsolationMode that bad, but I agree it's hardly elegant
There was a problem hiding this comment.
OnlySignedDevicesIsolationMode and AllDevicesIsolationMode ?
There was a problem hiding this comment.
Renamed as per andy suggestion, also renamed the DeviceIsolationModeKind enum to still match classes names 20f5882
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Fixes #4428
Checklist
public/exportedsymbols have accurate TSDoc documentation.