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.

Sign up for GitHub

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

Jump to bottom

Mark as verified if our own device matches keys #1348

Merged
merged 4 commits into from
Jan 17, 2023
Merged

Mark as verified if our own device matches keys #1348

merged 4 commits into from
Jan 17, 2023

Conversation

Anderas
Copy link
Contributor

@Anderas Anderas commented Jan 13, 2023

During migration we only convert private keys but not devices / identities. When we eventually query our own user and recieve current device, we will not consider it verified.

To solve this we mark current device as verified if just inserting it and we have master key stored locally.

@Anderas Anderas requested a review from poljar January 13, 2023 08:30
@Anderas Anderas marked this pull request as ready for review January 13, 2023 11:07
@poljar
Copy link
Contributor

poljar commented Jan 13, 2023

There seems to be some weirdness going on here with the CI, likely because the PR started out as a draft. Let's try to unwedge this.

@poljar poljar closed this Jan 13, 2023
@poljar poljar reopened this Jan 13, 2023
@Anderas Anderas force-pushed the andy/migration_trust branch from cd0c47a to 4bc4a95 Compare January 13, 2023 12:01
@Anderas Anderas force-pushed the andy/migration_trust branch from 4bc4a95 to 9bb62bf Compare January 13, 2023 12:02
@Anderas
Copy link
Contributor Author

Anderas commented Jan 13, 2023

It was actually running a new test only present on main against my PR, not really sure why. After rebasing it works.

poljar
poljar requested changes Jan 13, 2023
View reviewed changes
Copy link
Contributor

@poljar poljar 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, let's please add the mentioned comment or some variation of it and we're good to go.

crates/matrix-sdk-crypto/src/olm/signing/mod.rs Outdated Show resolved Hide resolved
crates/matrix-sdk-crypto/src/identities/manager.rs Show resolved Hide resolved
@Anderas Anderas requested a review from poljar January 13, 2023 14:10
poljar
poljar approved these changes Jan 13, 2023
View reviewed changes
Copy link
Contributor

@poljar poljar 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, let's see if @dkasak want's to take a look at this as well.

@poljar poljar requested a review from dkasak January 13, 2023 14:30
@Anderas Anderas mentioned this pull request Jan 16, 2023
Merged
dkasak
dkasak approved these changes Jan 17, 2023
View reviewed changes
Copy link
Member

@dkasak dkasak left a comment

Choose a reason for hiding this comment

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

Looks generally good to me, apart from a minor suggestion.

crates/matrix-sdk-crypto/src/identities/manager.rs Outdated Show resolved Hide resolved
@poljar poljar enabled auto-merge (squash) January 17, 2023 18:32
@poljar poljar merged commit 0af38d9 into main Jan 17, 2023
@poljar poljar deleted the andy/migration_trust branch January 17, 2023 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@poljar poljar poljar approved these changes

@dkasak dkasak dkasak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Anderas @poljar @dkasak