Skip to content

issue 4369 choose valid key for sending - #4372

Merged
tomholub merged 62 commits into
masterfrom
issue-4369-choose-right-key
Apr 11, 2022
Merged

issue 4369 choose valid key for sending#4372
tomholub merged 62 commits into
masterfrom
issue-4369-choose-right-key

Conversation

@tomholub

@tomholubtomholub commented Apr 5, 2022

Copy link
Copy Markdown
Collaborator

This PR ensures that the extension always works with a valid key when available, even if first received key is revoked.

close#4369 - choose valid key
issue #4376 - KeyIdentity.type renamed to KeyIdentity.family

  • 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

@tomholub

Copy link
Copy Markdown
CollaboratorAuthor

also todo - need to ensure there is no more primary especially primaryKi. Always treat as array.

@lgtm-com

This comment was marked as resolved.

@lgtm-com

This comment was marked as resolved.

@lgtm-com

This comment was marked as resolved.

@tomholub
tomholub marked this pull request as ready for review April 7, 2022 08:48
@tomholub

Copy link
Copy Markdown
CollaboratorAuthor

@rrrooommmaaa do you think you could review this today?

If yes, I'll wait with the release for the review. If not, I'd do a release and tell the customer to only test on evaluation environment for now.

@rrrooommmaaa

Copy link
Copy Markdown
Contributor

@rrrooommmaaa do you think you could review this today?

Sure, I'll take a look

@tomholub

Copy link
Copy Markdown
CollaboratorAuthor

(for me - need to look at revoked@key-manager-autoimport-no-prv-create-no-attester-submit.flowcrypt.test - cannot draft or send msg test and some of the other ones)

@github-actions

This comment was marked as off-topic.

@tomholub

This comment was marked as off-topic.

Comment threadtest/source/tests/compose.ts Outdated
@tomholub
tomholub enabled auto-merge (squash) April 7, 2022 20:38
@tomholub

Copy link
Copy Markdown
CollaboratorAuthor

@rrrooommmaaa we're green. Tomorrow please have another look. Thanks!

@tomholubtomholub left a comment

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

.

@rrrooommmaaarrrooommmaaa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I outlined my concerns and ideas

Comment threadextension/chrome/elements/compose-modules/compose-storage-module.ts Outdated
Comment threadextension/js/common/core/crypto/smime/smime-key.ts
Comment threadextension/js/common/assert.ts Outdated
Comment threadextension/chrome/settings/setup.ts
Comment threadextension/chrome/settings/modules/security.ts Outdated
Comment threadextension/chrome/settings/modules/keyserver.ts Outdated

@tomholubtomholub left a comment

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

re-review. looks good, will file one more issue and merge this

return new Attachment({ name: `flowcrypt-backup-${this.view.acctEmail.replace(/[^A-Za-z0-9]+/g, '')}.asc`, type: 'application/pgp-keys', data: Buf.fromUtfStr(armoredKey) });
private asBackupFile = (prv: KeyInfoWithIdentity) => {
return new Attachment({
name: `flowcrypt-backup-${this.view.acctEmail.replace(/[^A-Za-z0-9]+/g, '')}-${prv.id}.asc`,

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

I updated backups to use one file per key, and include fingerprint in each filename.

Comment threadtest/source/tests/compose.ts
@tomholub
tomholub disabled auto-merge April 11, 2022 10:06
@tomholub
tomholub merged commit cd824fc into masterApr 11, 2022
@tomholub
tomholub deleted the issue-4369-choose-right-key branch April 11, 2022 10:06
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.

browser extension drafts issue when first (assumed primary) key is revoked

3 participants

@tomholub@rrrooommmaaa@martgil