seanfarley / emacs-bitwarden

Emacs Bitwarden command wrapper.
GNU General Public License v3.0
50 stars 13 forks source link

Consider completing-read interface? #12

Open jsilve24 opened 2 years ago

jsilve24 commented 2 years ago

First off thanks again for putting this package together.

I wrote a completing-read interface for quick access to usernames and passwords that I think may be of more broad interest. In comparison to the widget interface I find the completing-read interface is faster (feels 2-3 times faster but that's just a guess), feels more natural given my heavy use of minibuffer based completions elsewhere in emacs, and allows copying usernames not just passwords.

The main interface is throught the functions bitwarden-kill-password and bitwarden-kill-username. Since they will often be called one after the other, this code stores both the password and username corresponding to a users selection and does not reprompt if within bitwarden-time-to-store. After that time the code automatically unbinds these variables so the passwords are not floating around in free text. A future improvement will also be to clear out the kill ring of these items when unbinding these variables. I just haven't gotten to this yet.

If this is something you think would be of broad interest I can make it a pull request. (this is just an exerpt of the relevant code from my config).

(defvar bitwarden-time-to-store "5 min"
  "Length of time to store last selected username and password before deleting. String should be recognized by the command run-at-time.")

;;;###autoload
(defun bitwarden--parse-hash-to-plist (e)
  "Pass emacs-bitwarden hash-table format to a plist."
  (let* ((login (gethash "login" e)))
    (message (gethash "name" e))
    (if login
    (list (gethash "name" e)
          (list
           :username (gethash "username" login)
           :password (gethash "password" login)))
      nil)))

;;;###autoload
(defun bitwarden-list-completing-read ()
  "A completing read interface built on top of emacs-bitwarden."
  (interactive)
  (let* ((vault (bitwarden-search))
     (vault (remq nil (mapcar 'bitwarden--parse-hash-to-plist vault)))
     (select (completing-read "Select:" vault))
     (select (car (cdr  (assoc select vault)))))
    (if (and  (boundp 'bitwarden--unstore-timer)
          (timerp bitwarden--unstore-timer))
    (progn 
      (cancel-timer bitwarden--unstore-timer)
      (makunbound 'bitwarden--unstore-timer)))
    (setq bitwarden--username (plist-get select :username)
      bitwarden--password (plist-get select :password)
      bitwarden--unstore-timer (run-at-time
                    bitwarden-time-to-store nil
                    (lambda ()
                      (makunbound 'bitwarden--username)
                      (makunbound 'bitwarden--password)))))
  (message  "Username and Passwored temporarily stored."))

;;;###autoload
(defun bitwarden-kill-password (&optional arg)
  "Yank password from bitwarden--username.

With prefix argument, repeat completin-read selection even if there was a recent selection (e.g., the variable bitwarden--password is bound)."
  (interactive "P")
  (if (or  (not (boundp 'bitwarden--password))
       arg)
      (bitwarden-list-completing-read))
  (kill-new bitwarden--password))

;;;###autoload
(defun bitwarden-kill-username (&optional arg)
  "Yank password from bitwarden--username.

With prefix argument, repeat completin-read selection even if there was a recent selection (e.g., the variable bitwarden--username is bound)."
  (interactive "P")
  (if (or  (not (boundp 'bitwarden--username))
       arg)
      (bitwarden-list-completing-read))
  (kill-new bitwarden--username))
seanfarley commented 2 years ago

Thanks for the code! I'll try to take a look this weekend :-D

emin63 commented 1 year ago

I wrote a completing-read interface for quick access to usernames and passwords that I think may be of more broad interest.

I found this useful and patched it into my own system. Thanks! I definitely think it would be valuable in the overall bitwarden.el package. Thanks also to @seanfarley for creating this in the first place.

One question: what is the expected use case? I added a snippet like below to my init

(setq my-bitwarden-keymap (make-keymap 'my-bitwarden-keymap))
(define-key my-bitwarden-keymap "n" 'bitwarden-kill-username)
(define-key my-bitwarden-keymap "p" 'bitwarden-kill-password)
(define-key my-bitwarden-keymap "u" 'bitwarden-unlock)
(define-key my-bitwarden-keymap "l" 'bitwarden-lock)
(define-key my-mode-map "v" my-bitwarden-keymap) ;; my-mode-map is bound to C-o

and then when I need something I do something like the following:

  1. C-o v u to call bitwarden-unlock
  2. C-u C-o v n to call bitwarden-kill-username with a prefix argument so it lets me search for the name.
  3. C-y to yank the killed username
  4. C-o v n to call bitwarden-kill-password which I think gets the password associated with killed username
  5. C-y to yank the killed password

Is that expected usage or is there a better way?

jsilve24 commented 1 year ago

Way simpler, here is my config:

(use-package bitwarden
  :straight (bitwarden :type git :host github :repo "jsilve24/emacs-bitwarden")
  :config
  (setq bitwarden-user "xxx@xxx.com")
  (setq bitwarden-api-secret-key
    (plist-get (car (auth-source-search :host "bitwarden.xxx.key"))
           :secret))
  (setq bitwarden-api-client-id
    (plist-get (car (auth-source-search :host "bitwarden.xxx.id"))
           :secret))
  (setq bitwarden-automatic-unlock
    (let* ((matches (auth-source-search :user "xxx@xxx.com"
                        :host "bitwarden.xxx.user"
                        :require '(:secret)
                        :max 1))
           (entry (nth 0 matches)))
      (plist-get entry :secret)))
  ;; (bitwarden-auth-source-enable)
  (bitwarden-login)
  (bitwarden-unlock))

(with-eval-after-load 'exwm
  (exwm-input-set-key (kbd  "s-u") #'bitwarden-kill-username)
  (exwm-input-set-key (kbd  "s-p") #'bitwarden-kill-password))

So its really just a few key-strokes. My guess is that you are using the C-u prefix because you just called it for some other reason. You only need the C-u prefix if its within the default timer interval (5 min).