From b774f9e5295341a68171d94765320114d0b5b407 Mon Sep 17 00:00:00 2001 From: Holger Dürer Date: Wed, 28 Feb 2018 20:48:24 +0000 Subject: Keep track of to which instance secrets in plstore belong. While testing out issue 149 (https://github.com/jdenen/mastodon.el/issues/149) I had problems due to stale client information being cached. With this change we store various pieces of information (the client information in the plstore and the auth tokens) in alists keyed by the instance url (and the plstore key contains the instance url as well to allow us to store data per instance). --- fixture/client.plstore | 3 +- lisp/mastodon-auth.el | 24 +++++++++------ lisp/mastodon-client.el | 30 ++++++++++-------- lisp/mastodon-http.el | 11 ++++--- test/mastodon-auth-tests.el | 15 +++++---- test/mastodon-client-tests.el | 72 ++++++++++++++++++++++++++++++------------- 6 files changed, 99 insertions(+), 56 deletions(-) diff --git a/fixture/client.plstore b/fixture/client.plstore index 3514ed9..e050018 100644 --- a/fixture/client.plstore +++ b/fixture/client.plstore @@ -1,2 +1,3 @@ ;;; public entries -*- mode: plstore -*- -(("mastodon" :client_id "id" :client_secret "secret")) +(("mastodon-http://other.example" :client_id "id1" :client_secret "secret1") + ("mastodon-http://mastodon.example" :client_id "id2" :client_secret "secret2")) diff --git a/lisp/mastodon-auth.el b/lisp/mastodon-auth.el index 83d7d04..b2399d2 100644 --- a/lisp/mastodon-auth.el +++ b/lisp/mastodon-auth.el @@ -40,8 +40,8 @@ :prefix "mastodon-auth-" :group 'mastodon) -(defvar mastodon-auth--token nil - "User access token.") +(defvar mastodon-auth--token-alist nil + "Alist of User access tokens keyed by instance url.") (defun mastodon-auth--generate-token () "Make POST to generate auth token." @@ -53,7 +53,8 @@ ("username" . ,(read-string "Email: ")) ("password" . ,(read-passwd "Password: ")) ("scope" . "read write follow")) - nil)) + nil + :unauthenticated)) (defun mastodon-auth--get-token () "Make auth token request and return JSON response." @@ -67,13 +68,16 @@ (json-read-from-string json-string)))) (defun mastodon-auth--access-token () - "Return `mastodon-auth--token'. - -Generate token and set `mastodon-auth--token' if nil." - (or mastodon-auth--token - (let* ((json (mastodon-auth--get-token)) - (token (plist-get json :access_token))) - (setq mastodon-auth--token token)))) + "Return the access token to use with the current `mastodon-instance-url'. + +Generate token and set if none known yet." + (let ((token + (cdr (assoc mastodon-instance-url mastodon-auth--token-alist)))) + (unless token + (let ((json (mastodon-auth--get-token))) + (setq token (plist-get json :access_token)) + (push (cons mastodon-instance-url token) mastodon-auth--token-alist))) + token)) (provide 'mastodon-auth) ;;; mastodon-auth.el ends here diff --git a/lisp/mastodon-client.el b/lisp/mastodon-client.el index b97197e..f8beb81 100644 --- a/lisp/mastodon-client.el +++ b/lisp/mastodon-client.el @@ -30,6 +30,7 @@ ;;; Code: (require 'plstore) +(defvar mastodon-instance-url) (autoload 'mastodon-http--api "mastodon-http") (autoload 'mastodon-http--post "mastodon-http") @@ -39,8 +40,8 @@ :group 'mastodon :type 'file) -(defvar mastodon-client--client-details nil - "Client id and secret.") +(defvar mastodon-client--client-details-alist nil + "An alist of Client id and secrets keyed by the instance url.") (defun mastodon-client--register () "POST client to Mastodon." @@ -50,7 +51,8 @@ ("redirect_uris" . "urn:ietf:wg:oauth:2.0:oob") ("scopes" . "read write follow") ("website" . "https://github.com/jdenen/mastodon.el")) - nil)) + nil + :unauthenticated)) (defun mastodon-client--fetch () "Return JSON from `mastodon-client--register' call." @@ -72,8 +74,8 @@ Make `mastodon-client--fetch' call to determine client values." (let ((plstore (plstore-open (mastodon-client--token-file))) - (client (mastodon-client--fetch))) - (plstore-put plstore "mastodon" client nil) + (client (mastodon-client--fetch))) + (plstore-put plstore (concat "mastodon-" mastodon-instance-url) client nil) (plstore-save plstore) (plstore-close plstore) client)) @@ -81,19 +83,23 @@ Make `mastodon-client--fetch' call to determine client values." (defun mastodon-client--read () "Retrieve client_id and client_secret from `mastodon-client--token-file'." (let* ((plstore (plstore-open (mastodon-client--token-file))) - (mastodon (plstore-get plstore "mastodon"))) - (when mastodon - (delete "mastodon" mastodon)))) + (mastodon (plstore-get plstore (concat "mastodon-" mastodon-instance-url)))) + (cdr mastodon))) (defun mastodon-client () - "Return variable `mastodon-client--client-details' plist. + "Return variable client secrets to use for the current `mastodon-instance-url'.. Read plist from `mastodon-client--token-file' if variable is nil. Fetch and store plist if `mastodon-client--read' returns nil." - (or mastodon-client--client-details - (setq mastodon-client--client-details + (let ((client-details (cdr (assoc mastodon-instance-url mastodon-client--client-details-alist)))) + (unless client-details + (setq client-details (or (mastodon-client--read) - (mastodon-client--store))))) + (mastodon-client--store))) + (push (cons mastodon-instance-url client-details) + mastodon-client--client-details-alist)) + client-details)) (provide 'mastodon-client) ;;; mastodon-client.el ends here + diff --git a/lisp/mastodon-http.el b/lisp/mastodon-http.el index f519e20..905a853 100644 --- a/lisp/mastodon-http.el +++ b/lisp/mastodon-http.el @@ -31,7 +31,6 @@ (require 'json) (defvar mastodon-instance-url) -(defvar mastodon-auth--token) (autoload 'mastodon-auth--access-token "mastodon-auth") (defvar mastodon-http--api-version "v1") @@ -68,10 +67,10 @@ Open RESPONSE buffer if unsuccessful." (funcall success) (switch-to-buffer response)))) -(defun mastodon-http--post (url args headers) +(defun mastodon-http--post (url args headers &optional unauthenticed-p) "POST synchronously to URL with ARGS and HEADERS. -Authorization header is included by default." +Authorization header is included by default unless UNAUTHENTICED-P is non-nil." (let ((url-request-method "POST") (url-request-data (when args @@ -82,8 +81,10 @@ Authorization header is included by default." args "&"))) (url-request-extra-headers - `(("Authorization" . ,(concat "Bearer " mastodon-auth--token)) - ,headers))) + (append + (unless unauthenticed-p + `(("Authorization" . (concat "Bearer " (mastodon-auth--access-token))))) + headers))) (with-temp-buffer (url-retrieve-synchronously url)))) diff --git a/test/mastodon-auth-tests.el b/test/mastodon-auth-tests.el index 70c63d8..719a56c 100644 --- a/test/mastodon-auth-tests.el +++ b/test/mastodon-auth-tests.el @@ -14,7 +14,8 @@ ("username" . "foo@bar.com") ("password" . "password") ("scope" . "read write follow")) - nil)) + nil + :unauthenticated)) (mastodon-auth--generate-token)))) (ert-deftest get-token () @@ -26,15 +27,17 @@ (current-buffer))) (should (equal (mastodon-auth--get-token) '(:access_token "abcdefg")))))) -(ert-deftest access-token-1 () - "Should return `mastodon-auth--token' if non-nil." - (let ((mastodon-auth--token "foobar")) +(ert-deftest access-token-found () + "Should return value in `mastodon-auth--token-alist' if found." + (let ((mastodon-instance-url "https://instance.url") + (mastodon-auth--token-alist '(("https://instance.url" . "foobar")) )) (should (string= (mastodon-auth--access-token) "foobar")))) (ert-deftest access-token-2 () "Should set and return `mastodon-auth--token' if nil." - (let ((mastodon-auth--token nil)) + (let ((mastodon-instance-url "https://instance.url") + (mastodon-auth--token nil)) (with-mock (mock (mastodon-auth--get-token) => '(:access_token "foobaz")) (should (string= (mastodon-auth--access-token) "foobaz")) - (should (string= mastodon-auth--token "foobaz"))))) + (should (equal mastodon-auth--token-alist '(("https://instance.url" . "foobaz"))))))) diff --git a/test/mastodon-client-tests.el b/test/mastodon-client-tests.el index c339efa..dfe175b 100644 --- a/test/mastodon-client-tests.el +++ b/test/mastodon-client-tests.el @@ -9,7 +9,8 @@ ("redirect_uris" . "urn:ietf:wg:oauth:2.0:oob") ("scopes" . "read write follow") ("website" . "https://github.com/jdenen/mastodon.el")) - nil)) + nil + :unauthenticated)) (mastodon-client--register))) (ert-deftest fetch () @@ -23,53 +24,80 @@ (ert-deftest store-1 () "Should return the client plist." - (let ((plist '(:client_id "id" :client_secret "secret"))) + (let ((mastodon-instance-url "http://mastodon.example") + (plist '(:client_id "id" :client_secret "secret"))) (with-mock (mock (mastodon-client--token-file) => "stubfile.plstore") (mock (mastodon-client--fetch) => '(:client_id "id" :client_secret "secret")) (let* ((plstore (plstore-open "stubfile.plstore")) - (client (delete "mastodon" (plstore-get plstore "mastodon")))) - (should (equal (mastodon-client--store) plist)) - )))) + (client (cdr (plstore-get plstore "mastodon-http://mastodon.example")))) + (should (equal (mastodon-client--store) plist)))))) (ert-deftest store-2 () "Should store client in `mastodon-client--token-file'." - (let* ((plstore (plstore-open "stubfile.plstore")) - (client (delete "mastodon" (plstore-get plstore "mastodon")))) + (let* ((mastodon-instance-url "http://mastodon.example") + (plstore (plstore-open "stubfile.plstore")) + (client (cdr (plstore-get plstore "mastodon-http://mastodon.example")))) (plstore-close plstore) (should (string= (plist-get client :client_id) "id")) (should (string= (plist-get client :client_secret) "secret")))) -(ert-deftest read-1 () +(ert-deftest read-finds-match () "Should return mastodon client from `mastodon-token-file' if it exists." - (with-mock - (mock (mastodon-client--token-file) => "fixture/client.plstore") - (should (equal (mastodon-client--read) '(:client_id "id" :client_secret "secret"))))) + (let ((mastodon-instance-url "http://mastodon.example")) + (with-mock + (mock (mastodon-client--token-file) => "fixture/client.plstore") + (should (equal (mastodon-client--read) + '(:client_id "id2" :client_secret "secret2")))))) + +(ert-deftest read-finds-no-match () + "Should return mastodon client from `mastodon-token-file' if it exists." + (let ((mastodon-instance-url "http://mastodon.social")) + (with-mock + (mock (mastodon-client--token-file) => "fixture/client.plstore") + (should (equal (mastodon-client--read) nil))))) -(ert-deftest read-2 () +(ert-deftest read-empty-store () "Should return nil if mastodon client is not present in the plstore." (with-mock (mock (mastodon-client--token-file) => "fixture/empty.plstore") (should (equal (mastodon-client--read) nil)))) -(ert-deftest client-1 () - "Should return `mastondon-client' if non-nil." - (let ((mastodon-client--client-details t)) - (should (eq (mastodon-client) t)))) +(ert-deftest client-set-and-matching () + "Should return `mastondon-client' if `mastodon-client--client-details-alist' is non-nil and instance url is included." + (let ((mastodon-instance-url "http://mastodon.example") + (mastodon-client--client-details-alist '(("https://other.example" . :no-match) + ("http://mastodon.example" . :matches)))) + (should (eq (mastodon-client) :matches)))) + +(ert-deftest client-set-but-not-matching () + "Should read from `mastodon-token-file' if wrong data is cached." + (let ((mastodon-instance-url "http://mastodon.example") + (mastodon-client--client-details-alist '(("http://other.example" :wrong)))) + (with-mock + (mock (mastodon-client--read) => '(:client_id "foo" :client_secret "bar")) + (should (equal (mastodon-client) '(:client_id "foo" :client_secret "bar"))) + (should (equal mastodon-client--client-details-alist + '(("http://mastodon.example" :client_id "foo" :client_secret "bar") + ("http://other.example" :wrong))))))) -(ert-deftest client-2 () +(ert-deftest client-unset () "Should read from `mastodon-token-file' if available." - (let ((mastodon-client--client-details nil)) + (let ((mastodon-instance-url "http://mastodon.example") + (mastodon-client--client-details-alist nil)) (with-mock (mock (mastodon-client--read) => '(:client_id "foo" :client_secret "bar")) (should (equal (mastodon-client) '(:client_id "foo" :client_secret "bar"))) - (should (equal mastodon-client--client-details '(:client_id "foo" :client_secret "bar")))))) + (should (equal mastodon-client--client-details-alist + '(("http://mastodon.example" :client_id "foo" :client_secret "bar"))))))) -(ert-deftest client-3 () +(ert-deftest client-unset-and-not-in-storage () "Should store client data in plstore if it can't be read." - (let ((mastodon-client--client-details nil)) + (let ((mastodon-instance-url "http://mastodon.example") + (mastodon-client--client-details-alist nil)) (with-mock (mock (mastodon-client--read)) (mock (mastodon-client--store) => '(:client_id "foo" :client_secret "baz")) (should (equal (mastodon-client) '(:client_id "foo" :client_secret "baz"))) - (should (equal mastodon-client--client-details '(:client_id "foo" :client_secret "baz")))))) + (should (equal mastodon-client--client-details-alist + '(("http://mastodon.example" :client_id "foo" :client_secret "baz"))))))) -- cgit v1.2.3