Skip to content

Comments

allow to update a private key from file#5294

Merged
rrrooommmaaa merged 3 commits intomasterfrom
issue-5291-import-key-from-file
Jul 14, 2023
Merged

allow to update a private key from file#5294
rrrooommmaaa merged 3 commits intomasterfrom
issue-5291-import-key-from-file

Conversation

@rrrooommmaaa
Copy link
Contributor

@rrrooommmaaa rrrooommmaaa commented Jul 13, 2023

This PR allows to update a key from file

close #5291


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

if (typeof updatedKey === 'undefined') {
await Ui.modal.warning(Lang.setup.keyFormattedWell(this.prvHeaders.begin, String(this.prvHeaders.end)), Ui.testCompatibilityLink);
} else if (updatedKeyEncrypted.identities.length === 0) {
await Ui.modal.error(Lang.setup.prvHasUseridIssue);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This page is now using KeyImportUi for some visual handling, but it's not using KeyImportUi.checkPrv as it is not easy to adapt it here (we can try this in a separate issue)

Also, what should the message be if there is something wrong with UserIds (we currently don't support UserIds that don't contain email address in them)
I added this text
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sosnovsky is there a vision on whether we should support keys with uids without emails?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We currently support identities missing email address for public keys of recipients, but compose module assumes emails[0] matches identities[0] but that's not necessarily true. I suggest to refactor this part. Instead of "synchronized" arrays we can have an array of objects of the type returned by Str.parseEmail, it has { email, name, full } -- everything that's required. full is the original identity

Copy link
Collaborator

Choose a reason for hiding this comment

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

@sosnovsky is there a vision on whether we should support keys with uids without emails?

I think it's quite rare case and we didn't get any requests for such functionality, so we can leave it as is for now. Then we'll probably also need to update our mobile apps to support such keys.

We currently support identities missing email address for public keys of recipients, but compose module assumes emails[0] matches identities[0] but that's not necessarily true. I suggest to refactor this part.

That sounds good 👍

@rrrooommmaaa rrrooommmaaa changed the title allow to import a key from file allow to update a private key from file Jul 13, 2023
Roman Shevchenko added 2 commits July 13, 2023 20:16
@rrrooommmaaa rrrooommmaaa marked this pull request as ready for review July 14, 2023 06:06
@rrrooommmaaa rrrooommmaaa requested a review from sosnovsky as a code owner July 14, 2023 06:06
@rrrooommmaaa rrrooommmaaa enabled auto-merge (squash) July 14, 2023 06:06
Copy link
Collaborator

@sosnovsky sosnovsky left a comment

Choose a reason for hiding this comment

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

Well done 👍

@rrrooommmaaa rrrooommmaaa merged commit c859155 into master Jul 14, 2023
@rrrooommmaaa rrrooommmaaa deleted the issue-5291-import-key-from-file branch July 14, 2023 10:41
Sign up for free to 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.

Add ability to update private key from file

2 participants