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

Upgrade verification if necessary #1751

Merged
merged 1 commit into from
Mar 28, 2023
Merged

Conversation

Anderas
Copy link
Contributor

@Anderas Anderas commented Mar 27, 2023

Set needsVerificationUpgrade property when migrating from legacy to rust crypto, which will be used to display altered UX for "verify your session" modal. This is because sessions considered as verified in the legacy crypto should not always be considered as verified in rust crypto, as the latter imposes stricter rules on session verification. In case the rust does consider the session as verified the property is reset.

Note that this is not meant to be 100% reliable way to detect verification state change. For instance if legacy crypto was already unverified, migrating to rust would still display the "upgrade verification" dialoge. This is acceptable, given that the "upgrade verification" prompt will only be shown until either successful verification or ignoring of the alert.

Additionally keep the existing realm store around after migration instead of deleting it. It will be removed in a future PR.

@Anderas Anderas requested review from a team, flescio and BillCarsonFr and removed request for a team March 27, 2023 13:58
@codecov
Copy link

codecov bot commented Mar 27, 2023

Codecov Report

❗ No coverage uploaded for pull request base (hotfix/0.26.4@91dbe20). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@               Coverage Diff                @@
##             hotfix/0.26.4    #1751   +/-   ##
================================================
  Coverage                 ?   37.80%           
================================================
  Files                    ?      615           
  Lines                    ?    97060           
  Branches                 ?    41929           
================================================
  Hits                     ?    36696           
  Misses                   ?    59314           
  Partials                 ?     1050           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@BillCarsonFr BillCarsonFr left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@pixlwave pixlwave left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@flescio flescio left a comment

Choose a reason for hiding this comment

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

LGTM!

@Anderas Anderas merged commit 88c5f92 into hotfix/0.26.4 Mar 28, 2023
@Anderas Anderas deleted the andy/upgrade_security branch March 28, 2023 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants