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

Replace event verification logic with new code in js-sdk #11528

Merged
merged 15 commits into from
Sep 18, 2023

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Sep 4, 2023

Part of element-hq/element-web#25321.

Rip out the existing verification magic, and replace it with the new API introduced in matrix-org/matrix-js-sdk#3693.

Requires matrix-org/matrix-js-sdk#3693.

Based on #11364


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

@richvdh richvdh added the T-Task Refactoring, enabling or disabling functionality, other engineering tasks label Sep 4, 2023
Base automatically changed from rav/element-r/shields/tests to develop September 5, 2023 12:08
@richvdh richvdh force-pushed the rav/element-r/shields/new_api branch from a93b8dc to a3863fb Compare September 5, 2023 14:06
break;

case EventShieldReason.MISMATCHED_SENDER_KEY:
shieldReasonMessage = _t("Encrypted by an unverified session");
Copy link
Member Author

Choose a reason for hiding this comment

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

this doesn't seem like the right error, but it preserves existing functionality (this situation replicates the code path at https://github.com/matrix-org/matrix-react-sdk/pull/11528/files#diff-9ff5535291b7c2e655ccd3d02c4f632b6f2dc9c30072514641e31c2a8751c57eL593)

@richvdh richvdh force-pushed the rav/element-r/shields/new_api branch from 980a5c3 to e5fb21d Compare September 8, 2023 02:41
@richvdh richvdh changed the base branch from develop to florianduros/element-r/cross-user-verification September 8, 2023 02:42
This is required because a transition from "valid event" to "unencrypted event"
no longer triggers a state change, so the component does not render
itself. Previously, this would be a transition from `verified:
E2EState.Normal` to `verified: null`.
Copy link
Contributor

@weeman1337 weeman1337 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 to me 👍 Some quick tests on the deployment were all fine!

// If the verification state changed, the height might have changed
if (prevState.verified !== this.state.verified && this.props.onHeightChanged) {
// If the shield state changed, the height might have changed.
// XXX: does the shield *actually* cause a change in height? Not sure.
Copy link
Contributor

Choose a reason for hiding this comment

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

I did some tests with different layouts. As far as I can see the shield doesn't change the height.

Base automatically changed from florianduros/element-r/cross-user-verification to develop September 18, 2023 16:34
@richvdh richvdh added this pull request to the merge queue Sep 18, 2023
Merged via the queue into develop with commit cf2340b Sep 18, 2023
76 checks passed
@richvdh richvdh deleted the rav/element-r/shields/new_api branch September 18, 2023 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants