Skip to content
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

Map decryption errors correctly from rust #3710

Merged
merged 16 commits into from
Sep 13, 2023

Conversation

BillCarsonFr
Copy link
Member

@BillCarsonFr BillCarsonFr commented Sep 6, 2023

The decryptEvent(event: MatrixEvent): Promise<EventDecryptionResult> Promise can be rejected with DecryptionError. This is implicilty part of API as it's used in react sdk (DecryptionFailureTracker for posthog), and it's also needed to query keybackup automatically (done in a follow-up PR).

This PR uses a new functionnality in wasm binding that returns typed error for decryption failures. These error are mapped to the existing DecryptionErrors.
It's not a 1:1 mapping (can we ignore the quality gate because only 75%?)as the crypto backends do not report the same errors, but it maps the important ones.

Based on #3709

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

This change is marked as an internal change (Task), so will not be included in the changelog.

@BillCarsonFr BillCarsonFr added the T-Task Tasks for the team like planning label Sep 6, 2023
@BillCarsonFr BillCarsonFr changed the base branch from develop to valere/element-r/backup/restore/pr2 September 6, 2023 22:41
@BillCarsonFr BillCarsonFr changed the title Valere/element r/backup/restore/pr3 Map decryption errors correctly from rust Sep 6, 2023
Base automatically changed from valere/element-r/backup/restore/pr2 to develop September 13, 2023 09:22
Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good, thanks.

For test coverage, I think a different userId seems worth testing, hopefully this.stopped is easy to test? An event without a roomId seems worth checking too. So maybe that would get us over the threshold?

@BillCarsonFr
Copy link
Member Author

All looks good, thanks.

For test coverage, I think a different userId seems worth testing, hopefully this.stopped is easy to test? An event without a roomId seems worth checking too. So maybe that would get us over the threshold?

I have removed the part that was mapping the rust MismatchedIdentityKeys for now as there is no mapping for it in legacy libolm, and we are thus not using it. It's going to the generic DecryptionError bucket

Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@BillCarsonFr BillCarsonFr added this pull request to the merge queue Sep 13, 2023
Merged via the queue into develop with commit 6d11800 Sep 13, 2023
17 checks passed
@BillCarsonFr BillCarsonFr deleted the valere/element-r/backup/restore/pr3 branch September 13, 2023 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Task Tasks for the team like planning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants