-
Notifications
You must be signed in to change notification settings - Fork 53
Wkd.lookupEmail returns several keys #3561
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
Changes from all commits
1e16d4a
bacf762
577457b
2a97507
e306014
38d3c54
08c37f2
bc934ea
79896fc
481cedd
570d1c5
444ab4a
5843824
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -232,6 +232,7 @@ export class OpenPGPKey { | |
| curve: (algoInfo as any).curve as string | undefined, | ||
| algorithmId: opgp.enums.publicKey[algoInfo.algorithm] | ||
| }, | ||
| revoked: keyWithoutWeakPackets.revocationSignatures.length > 0 | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this is the flag I was talking about. It's easily calculated when creating the
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ah. My intuition was to update the schema to add this flag to storage. Then filter based on that when pulling keys out of storage. What you say is that you pull all keys for that user, then parse them and choose the appropriate key to use based on the parsed information. Your approach is simpler, and therefore better.
I tend to do this in two ways - once in high level code and once more - a last resort hard stop - in low level code:
All of the above places would have an if to handle this situation and not update key that's already revoked. Often this will result in a ui modal or toast message to the user if it was user-initiated action. In situations when this was an automatic action (like compose window lookups after user enters recipient), the UI interaction could be skipped and the update would be skipped silently (still in high level code).
If the error ever throws, it will get reported to the backend, and I'll get to know about it. Then we'll have the stack trace to know which case we missed to handle in high level code. I don't know if I'm over doing it. We could just silently skip updating in low level code and don't tell the user. But that could lead to some behavior that may appear buggy to the user.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Actually, to make a unified solution for both OpenPGP and S/MIME revocation (we do want to support CRL later on, don't we?), the easiest would be to add a flag, or keep a table of revoked keys' fingerprints.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll let you choose an approach that you deem appropriate. I'd be ok to prevent updating revoked keys in a followup PR - the situation now is already better then before (it used to not be even able to import revoked keys, I think). |
||
| } as Key); | ||
| (key as any)[internal] = keyWithoutWeakPackets; | ||
| (key as any).rawKey = opgpKey; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.