~pkal/public-inbox

1

[PATCH 2/3] Move autocrypt-accounts insertions to dedicated, interactive function

Details
Message ID
<20201229002924.10117-3-swflint@flintfam.org>
DKIM signature
missing
Download raw message
Patch: +17 -2
This allows the use of pre-existing keys, and makes sure that saving
of this information is done the same way, at all times.
---
 autocrypt.el | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/autocrypt.el b/autocrypt.el
index 5fd6b6f..a3f215f 100644
--- a/autocrypt.el
+++ b/autocrypt.el
@@ -510,11 +510,26 @@ Will handle and remove \"Do-(Discourage-)Autocrypt\" if found."
    (let ((res (epg-context-result-for ctx 'generate-key)))
      (unless res
        (error "Could not determine fingerprint"))
      (push (list email (cdr (assq 'fingerprint (car res))) 'none) autocrypt-accounts)
      (autocrypt-save-data))
      (autocrypt-insert-account email (cdr (assq 'fingerprint (car res))) 'none))
    (message "Successfully generated key for %s, and added to key chain."
             email)))

(defun autocrypt-insert-account (email fingerprint preference)
  "Store information about a GPG key.
EMAIL, FINGERPRINT are the mail address and fingerprint
associated with the key.
PREFERENCE is one of 'mutual, 'no-preference, 'none, or nil."
  (interactive
   (list
    (read-string "Email: " user-mail-address)
    (read-string "Key Fingerprint: ")
    (let ((value (completing-read "Preference: " '("mutual" "no-preference" "none") #'identity t "mutual" nil nil)))
      (if (string-empty-p value)
          nil
        (intern value)))))
  (push (list email fingerprint preference) autocrypt-accounts)
  (autocrypt-save-data))


;;; MINOR MODES

-- 
2.29.2
Details
Message ID
<87mtxwvpba.fsf@posteo.net>
In-Reply-To
<20201229002924.10117-3-swflint@flintfam.org> (view parent)
DKIM signature
missing
Download raw message
"Samuel W. Flint" <swflint@flintfam.org> writes:

> This allows the use of pre-existing keys, and makes sure that saving
> of this information is done the same way, at all times.
> ---
>  autocrypt.el | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/autocrypt.el b/autocrypt.el
> index 5fd6b6f..a3f215f 100644
> --- a/autocrypt.el
> +++ b/autocrypt.el
> @@ -510,11 +510,26 @@ Will handle and remove \"Do-(Discourage-)Autocrypt\" if found."
>      (let ((res (epg-context-result-for ctx 'generate-key)))
>        (unless res
>          (error "Could not determine fingerprint"))
> -      (push (list email (cdr (assq 'fingerprint (car res))) 'none) autocrypt-accounts)
> -      (autocrypt-save-data))
> +      (autocrypt-insert-account email (cdr (assq 'fingerprint (car res))) 'none))
>      (message "Successfully generated key for %s, and added to key chain."
>               email)))
>  
> +(defun autocrypt-insert-account (email fingerprint preference)
> +  "Store information about a GPG key.

The thing about this is that the autocrypt spec doesn't recommend using
your own keys. And for that matter, they shouldn't just be any key, but
specifically Ed25519 plus Cv25519. So I'm uncertain if adding this
command would be the right thing to do.

> +EMAIL, FINGERPRINT are the mail address and fingerprint
> +associated with the key.
> +PREFERENCE is one of 'mutual, 'no-preference, 'none, or nil."
> +  (interactive
> +   (list
> +    (read-string "Email: " user-mail-address)
> +    (read-string "Key Fingerprint: ")

But setting that aside, just reading any string here, especially without
validating if this is a valid fingerprint, will just lead to issues
later on. This should either be replaced by a completing-read with
accounts we have private keys for (minus the ones that are already in
autocrypt-accounts).

> +    (let ((value (completing-read "Preference: " '("mutual" "no-preference" "none") #'identity t "mutual" nil nil)))
> +      (if (string-empty-p value)
> +          nil
> +        (intern value)))))

The use of completing read here is also weird. Why is identity the
predicate? Why is "mutual" the initial input (instead of the default)? I
personally think it would be better to replace this by a series of
yes-or-no-p questions, so something like:

  Do you have an encryption preference?
  - No  => no-preference
  - Yes -> Do you prefer mutual encryption?
           - Yes => mutual
           - No  => none

The questions should be rephrased and re-orders, because it makes no
sense to people who aren't familiar with autocrypt. Maybe read-answer
would also be worth considering? There was another function in this
class, but I can't remember it's name right now.

> +  (push (list email fingerprint preference) autocrypt-accounts)
> +  (autocrypt-save-data))
> +
>  
>  ;;; MINOR MODES

-- 
	Philip K.
Reply to thread Export thread (mbox)