Skip to content

Signed-only messages not recognized as signed - #1088

Merged
tomholub merged 9 commits into
masterfrom
feature/issue-1020
Nov 29, 2021
Merged

Signed-only messages not recognized as signed#1088
tomholub merged 9 commits into
masterfrom
feature/issue-1020

Conversation

@ivan-ushakov

@ivan-ushakovivan-ushakov commented Nov 24, 2021

Copy link
Copy Markdown
Contributor

close#1020


Tests(delete all except exactly one):

  • Tests added or updated

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@ivan-ushakov
ivan-ushakov marked this pull request as draft November 24, 2021 20:16
@ivan-ushakovivan-ushakov changed the title Feature/issue 1020Signed-only messages not recognized as signedNov 24, 2021
@ivan-ushakov

Copy link
Copy Markdown
ContributorAuthor

Still fighting with email crafting for UI tests

Comment threadappium/tests/screenobjects/email.screen.ts
Comment threadappium/tests/screenobjects/inbox.screen.ts Outdated
Comment threadappium/tests/screenobjects/inbox.screen.ts Outdated
@ivan-ushakov

Copy link
Copy Markdown
ContributorAuthor

@tomholub
Do you know how to create this two?

  1. signed-only message where the pubkey is not available
  2. signed-only message that was tempered during transit

For the first one I can not use mime-email-plain-signed.txt because it has text with special symbol. Also I want to follow my convention for the subjects and text.

@ivan-ushakov
ivan-ushakov marked this pull request as ready for review November 26, 2021 18:31
@ivan-ushakov

ivan-ushakov commented Nov 26, 2021

Copy link
Copy Markdown
ContributorAuthor
  1. signed-only message where the pubkey is not available

For this one I used my email generator and created signed message with generated key pair (not published anywhere).

  1. signed-only message that was tempered during transit

For this one I just sent signed email with browser extension, downloaded original message in MIME and changed message text from Signed only message that was tempered during transit to Signed only message that was Tempered during transit and imported with Thunderbird.

@ivan-ushakov

Copy link
Copy Markdown
ContributorAuthor

UI tests failing because I added new messages and now you need to scroll down to have them visible. I agree about using Search functionality in UI tests for this.

@tomholub

Copy link
Copy Markdown
Collaborator

UI tests failing because I added new messages and now you need to scroll down to have them visible. I agree about using Search functionality in UI tests for this.

in progress here #1103

@ivan-ushakov

Copy link
Copy Markdown
ContributorAuthor

I moved UI test to search approach to save some time.

@tomholub

Copy link
Copy Markdown
Collaborator

@ivan-ushakov master tests are fixed now

@tomholubtomholub left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is excellent! With one problem below.

I'm not sure if the problem is in the supplied message or in the app code.

Comment on lines +58 to +63
// signed only message where the pubkey is not available
await MailFolderScreen.clickSearchButton();
await SearchScreen.searchAndClickEmailBySubject('Signed only message where the pubkey is not available');

await EmailScreen.checkEncryptionBadge('decrypt error');
await EmailScreen.clickBackButton();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This one is not right - signed only message that is missing pubkey should have encryption badge of not encrypted and a signature badge that says it cannot be verified due to missing pubkey.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Maybe email was crafter with error. Let's create separate issue

@tomholubtomholub left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'll merge it either way, as it is an improvement and adds a great test.

But that case needs to be debugged.

@tomholub
tomholub merged commit d452d99 into masterNov 29, 2021
@tomholub
tomholub deleted the feature/issue-1020 branch November 29, 2021 18:14
Sign up for freeto 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.

signed-only messages not recognized as signed + ui tests

2 participants

@ivan-ushakov@tomholub