-
Notifications
You must be signed in to change notification settings - Fork 10
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
Feature/issue 732 compose pub key validation #821
Conversation
@tomholub I currently have only 2 tasks assigned, can you please check which tasks should I take after these ones? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good with a comment + two improvements to do in the future.
Will build and test it now
let selectedRecipients = contextToSend.recipients.filter(\.state.isSelected) | ||
selectedRecipients.forEach(evaluate) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not too sure about this - the validation happens asynchronously as a task. Would that be of any use? By the time it figures out it's wrong, this method may already finish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just for UI, because when recipient is selected then user won't be able to see if it's highlighted with red or orange color. Evaluation result isn't used for determining valid recipients.
FlowCrypt/Functionality/Services/Compose Message Service/ComposeMessageService.swift
Show resolved
Hide resolved
FlowCrypt/Functionality/Services/Compose Message Service/ComposeMessageService.swift
Show resolved
Hide resolved
@sosnovsky I got this crash/assertionFailure but not sure if it's related or not? |
Otherwise works well, and that one was an assertionFailure - not too bad, so I'll merge it. The asynchronous validation can be discussed in another issue. |
This PR adds pub keys validation before sending emails. It also updates encryption to use only valid keys.
close #732
close #744
Tests (delete all except exactly one):
testValidateMessageInputWithExpiredRecipientPubKey
,testValidateMessageInputWithRevokedRecipientPubKey
,testValidateMessageInputWithValidAndInvalidRecipientPubKeys
)To be filled by reviewers
I have reviewed that this PR... (tick whichever items you personally focused on during this review):