unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [ELPA/oauth2] Request for patches review
@ 2021-11-28 12:38 Aleksandr Vityazev
  0 siblings, 0 replies; only message in thread
From: Aleksandr Vityazev @ 2021-11-28 12:38 UTC (permalink / raw)
  To: emacs-devel, julien

[-- Attachment #1: Type: text/plain, Size: 577 bytes --]

Hello,

In a previous post [1], I mentioned that Elpa's oauth2 does not work
correctly for me, so I decided to make changes. Some changes probably
cannot be made to Elpa, since my version of oauth2 requires Emacs
version 27.1, but at least it will be possible to update the doc string
for functions. Therefore, I ask you to check the changes so that I
correct them where necessary. Here is the repo with the new version [2].

[1] https://lists.gnu.org/archive/html/emacs-devel/2021-11/msg01275.html
[2] https://git.sr.ht/~akagi/oauth2/

-- 
Best regargds, 
Aleksandr Vityazev


[-- Attachment #2: 0001-oauth2-Update-version-to-0.17.patch --]
[-- Type: text/x-patch, Size: 17721 bytes --]

From e52ba22450bec9b77afdd943a03d79b2f89870da Mon Sep 17 00:00:00 2001
Message-Id: <e52ba22450bec9b77afdd943a03d79b2f89870da.1638101267.git.avityazev@posteo.org>
In-Reply-To: <cover.1638101267.git.avityazev@posteo.org>
References: <cover.1638101267.git.avityazev@posteo.org>
From: Aleksandr Vityazev <avityazev@posteo.org>
Date: Thu, 25 Nov 2021 10:39:08 +0300
Subject: [PATCH 01/11] * oauth2: Update version to 0.17

* oauth2: Update version to 0.17, require Emacs version 27.1.
(oauth2-request-authorization, oauth2-request-access): use cl-defun,
update doc string.
(oauth2-token): make read-only.
(oauth2-request-access): update accordingly to changes
(oauth2-auth, oauth2-compute-id, oauth2-url-append-access-token): Update doc string.
(oauth2-auth-and-store): change order of args and update doc string.
(oauth2--url-advice): Rename.
---
 oauth2.el | 244 ++++++++++++++++++++++++++++++++----------------------
 1 file changed, 145 insertions(+), 99 deletions(-)

diff --git a/oauth2.el b/oauth2.el
index 0099e06..1444890 100644
--- a/oauth2.el
+++ b/oauth2.el
@@ -1,11 +1,13 @@
 ;;; oauth2.el --- OAuth 2.0 Authorization Protocol  -*- lexical-binding:t -*-
 
+;; Copyright © 2021 Aleksandr Vityazev <avityazev@posteo.org>
 ;; Copyright (C) 2011-2021 Free Software Foundation, Inc
 
 ;; Author: Julien Danjou <julien@danjou.info>
-;; Version: 0.16
+;; Version: 0.17
 ;; Keywords: comm
-;; Package-Requires: ((cl-lib "0.5") (nadvice "0.3"))
+;; Package-Requires: ((emacs "27.1"))
+;; Homepage: https://git.sr.ht/~akagi/oauth2
 
 ;; This file is part of GNU Emacs.
 
@@ -36,11 +38,19 @@
 
 ;;; Code:
 
-(eval-when-compile (require 'cl-lib))
+(eval-when-compile
+  (require 'cl-lib)
+  (require 'subr-x))
+
 (require 'plstore)
 (require 'json)
 (require 'url-http)
 
+(defcustom oauth2-token-file (concat user-emacs-directory "oauth2.plstore")
+  "File path where store OAuth tokens."
+  :type 'file
+  :group 'oauth2)
+
 (defvar url-http-data)
 (defvar url-http-method)
 (defvar url-http-extra-headers)
@@ -53,15 +63,22 @@
   :link '(url-link :tag "Savannah" "http://git.savannah.gnu.org/cgit/emacs/elpa.git/tree/?h=externals/oauth2")
   :link '(url-link :tag "ELPA" "https://elpa.gnu.org/packages/oauth2.html"))
 
-(defun oauth2-request-authorization (auth-url client-id &optional scope state redirect-uri)
+(cl-defun oauth2-request-authorization (auth-url client-id &optional scope state
+                                           (redirect-uri "urn:ietf:wg:oauth:2.0:oob"))
   "Request OAuth authorization at AUTH-URL by launching `browse-url'.
 CLIENT-ID is the client id provided by the provider.
-It returns the code provided by the service."
+The REDIRECT-URI is the registered redirect_uri for your CLIENT-ID,
+the default for the desktop applications is \"urn:ietf:wg:oauth:2.0:oob\".
+SCOPE identify the resources that your application could access
+on the user's behalf.  STATE it is a string that your application
+uses to maintain state between the request and redirect response.
+
+Return the code provided by the service."
   (browse-url (concat auth-url
                       (if (string-match-p "\?" auth-url) "&" "?")
                       "client_id=" (url-hexify-string client-id)
                       "&response_type=code"
-                      "&redirect_uri=" (url-hexify-string (or redirect-uri "urn:ietf:wg:oauth:2.0:oob"))
+                      "&redirect_uri=" (url-hexify-string redirect-uri)
                       (if scope (concat "&scope=" (url-hexify-string scope)) "")
                       (if state (concat "&state=" (url-hexify-string state)) "")))
   (read-string "Enter the code your browser displayed: "))
@@ -84,21 +101,27 @@ It returns the code provided by the service."
         data))))
 
 (cl-defstruct oauth2-token
-  plstore
-  plstore-id
-  client-id
-  client-secret
-  access-token
-  refresh-token
-  token-url
-  access-response)
-
-(defun oauth2-request-access (token-url client-id client-secret code &optional redirect-uri)
+  (plstore "" :read-only t)
+  (plstore-id "" :read-only t)
+  (client-id "" :read-only t)
+  (client-secret "" :read-only t)
+  (access-token "" :read-only t)
+  (refresh-token "" :read-only t)
+  (token-url "" :read-only t)
+  (access-response "" :read-only t))
+
+(cl-defun oauth2-request-access (token-url client-id client-secret code &optional
+                                     (redirect-uri "urn:ietf:wg:oauth:2.0:oob"))
   "Request OAuth access at TOKEN-URL.
+CLIENT-ID is the client id provided by the provider.
+CLIENT-SECRET is the client secret associated with your CLIENT-ID.
+The REDIRECT-URI is the registered redirect_uri for your CLIENT-ID,
+the default for the desktop applications is \"urn:ietf:wg:oauth:2.0:oob\".
 The CODE should be obtained with `oauth2-request-authorization'.
-Return an `oauth2-token' structure."
+
+Return an response from requested service."
   (when code
-    (let ((result
+    (let ((response
            (oauth2-make-access-request
             token-url
             (concat
@@ -106,102 +129,125 @@ Return an `oauth2-token' structure."
 	     (when client-secret
                (concat  "&client_secret=" client-secret))
              "&code=" code
-             "&redirect_uri=" (url-hexify-string (or redirect-uri "urn:ietf:wg:oauth:2.0:oob"))
+             "&redirect_uri=" (url-hexify-string redirect-uri)
              "&grant_type=authorization_code"))))
-      (make-oauth2-token :client-id client-id
-                         :client-secret client-secret
-                         :access-token (cdr (assoc 'access_token result))
-                         :refresh-token (cdr (assoc 'refresh_token result))
-                         :token-url token-url
-                         :access-response result))))
+      response)))
 
 ;;;###autoload
 (defun oauth2-refresh-access (token)
   "Refresh OAuth access TOKEN.
 TOKEN should be obtained with `oauth2-request-access'."
-  (setf (oauth2-token-access-token token)
-        (cdr (assoc 'access_token
-                    (oauth2-make-access-request
-                     (oauth2-token-token-url token)
-                     (concat "client_id=" (oauth2-token-client-id token)
-			     (when (oauth2-token-client-secret token)
-                               (concat "&client_secret=" (oauth2-token-client-secret token)))
-                             "&refresh_token=" (oauth2-token-refresh-token token)
-                             "&grant_type=refresh_token")))))
-  ;; If the token has a plstore, update it
-  (let ((plstore (oauth2-token-plstore token)))
-    (when plstore
-      (plstore-put plstore (oauth2-token-plstore-id token)
-                   nil `(:access-token
-                         ,(oauth2-token-access-token token)
-                         :refresh-token
-                         ,(oauth2-token-refresh-token token)
-                         :access-response
-                         ,(oauth2-token-access-response token)
-                         ))
-      (plstore-save plstore)))
-  token)
+  (let ((plstore (oauth2-token-plstore token))
+        (plstore-id (oauth2-token-plstore-id token))
+        (client-id (oauth2-token-client-id token))
+        (client-secret (oauth2-token-client-secret token))
+        (token-url (oauth2-token-token-url token)))
+    (let* ((response (oauth2-make-access-request
+                      (oauth2-token-token-url token)
+                      (concat "client_id=" client-id
+			      (when client-secret
+                                (concat "&client_secret=" client-secret))
+                              "&refresh_token=" (oauth2-token-refresh-token token)
+                              "&grant_type=refresh_token")))
+           (new-token (make-oauth2-token :plstore plstore
+                                         :plstore-id plstore-id
+                                         :client-id client-id
+                                         :client-secret client-secret
+                                         :access-token (cdr (assoc 'access_token response))
+                                         :refresh-token (cdr (assoc 'refresh_token response))
+                                         :token-url token-url
+                                         :access-response response)))
+      (when plstore
+        (plstore-put plstore plstore-id
+                     nil `(:access-token
+                           ,(oauth2-token-access-token new-token)
+                           :refresh-token
+                           ,(oauth2-token-refresh-token new-token)
+                           :access-response
+                           ,(oauth2-token-access-response new-token)))
+        (plstore-save plstore))
+      new-token)))
 
 ;;;###autoload
 (defun oauth2-auth (auth-url token-url client-id client-secret &optional scope state redirect-uri)
-  "Authenticate application via OAuth2."
-  (oauth2-request-access
-   token-url
-   client-id
-   client-secret
-   (oauth2-request-authorization
-    auth-url client-id scope state redirect-uri)
-   redirect-uri))
+  "Authenticate application via OAuth2 at AUTH-URL and TOKEN-URL.
+CLIENT-ID is the client id provided by the provider.
+CLIENT-SECRET is the client secret associated with your CLIENT-ID.
+The REDIRECT-URI is the registered redirect_uri for your CLIENT-ID.
+SCOPE identify the resources that your application could access
+on the user's behalf.  STATE it is a string that your application
+uses to maintain state between the request and redirect response.
 
-(defcustom oauth2-token-file (concat user-emacs-directory "oauth2.plstore")
-  "File path where store OAuth tokens."
-  :group 'oauth2
-  :type 'file)
+Return an `oauth2-token' structure."
+
+  (let ((plstore (plstore-open oauth2-token-file))
+        (id (oauth2-compute-id auth-url token-url scope))
+        (response (oauth2-request-access
+                   token-url
+                   client-id
+                   client-secret
+                   (oauth2-request-authorization
+                    auth-url client-id scope state redirect-uri)
+                   redirect-uri)))
+    (make-oauth2-token :plstore plstore
+                       :plstore-id id
+                       :client-id client-id
+                       :client-secret client-secret
+                       :access-token (cdr (assoc 'access_token response))
+                       :refresh-token (cdr (assoc 'refresh_token response))
+                       :token-url token-url
+                       :access-response response)))
 
 (defun oauth2-compute-id (auth-url token-url scope)
-  "Compute an unique id based on URLs.
+  "Compute an unique id based on AUTH-URL, TOKEN-URL and SCOPE.
 This allows to store the token in an unique way."
   (secure-hash 'md5 (concat auth-url token-url scope)))
 
 ;;;###autoload
-(defun oauth2-auth-and-store (auth-url token-url scope client-id client-secret &optional redirect-uri state)
-  "Request access to a resource and store it using `plstore'."
-  ;; We store a MD5 sum of all URL
-  (let* ((plstore (plstore-open oauth2-token-file))
-         (id (oauth2-compute-id auth-url token-url scope))
-         (plist (cdr (plstore-get plstore id))))
-    ;; Check if we found something matching this access
-    (if plist
-        ;; We did, return the token object
-        (make-oauth2-token :plstore plstore
-                           :plstore-id id
-                           :client-id client-id
-                           :client-secret client-secret
-                           :access-token (plist-get plist :access-token)
-                           :refresh-token (plist-get plist :refresh-token)
-                           :token-url token-url
-                           :access-response (plist-get plist :access-response))
-      (let ((token (oauth2-auth auth-url token-url
-                                client-id client-secret scope state redirect-uri)))
-        ;; Set the plstore
-        (setf (oauth2-token-plstore token) plstore)
-        (setf (oauth2-token-plstore-id token) id)
-        (plstore-put plstore id nil `(:access-token
-                                      ,(oauth2-token-access-token token)
-                                      :refresh-token
-                                      ,(oauth2-token-refresh-token token)
-                                      :access-response
-                                      ,(oauth2-token-access-response token)))
-        (plstore-save plstore)
-        token))))
+(defun oauth2-auth-and-store (auth-url token-url client-id client-secret scope &optional state redirect-uri)
+  "Request access to a resource and store it using `plstore'.
+If the token has not yet been saved in plsote, then authenticate
+the application via OAuth2 using the AUTH-URL, TOKEN-URL and after that
+save the token in the `oauth2-token-file'.
+CLIENT-ID is the client id provided by the provider.
+CLIENT-SECRET is the client secret associated with your CLIENT-ID.
+The REDIRECT-URI is the registered redirect_uri for your CLIENT-ID.
+SCOPE identify the resources that your application could access
+on the user's behalf.  STATE it is a string that your application
+uses to maintain state between the request and redirect response.
+
+Return an `oauth2-token' structure."
+  (if-let ((plstore (plstore-open oauth2-token-file))
+           (id (oauth2-compute-id auth-url token-url scope))
+           (plist (cdr (plstore-get plstore id))))
+      ;; Check if we found something matching this access
+      ;; We did, return the token object
+      (make-oauth2-token :plstore plstore
+                         :plstore-id id
+                         :client-id client-id
+                         :client-secret client-secret
+                         :access-token (plist-get plist :access-token)
+                         :refresh-token (plist-get plist :refresh-token)
+                         :token-url token-url
+                         :access-response (plist-get plist :access-response))
+    (let ((token (oauth2-auth auth-url token-url
+                        client-id client-secret scope state redirect-uri)))
+      (plstore-put plstore id nil `(:access-token
+                                    ,(oauth2-token-access-token token)
+                                    :refresh-token
+                                    ,(oauth2-token-refresh-token token)
+                                    :access-response
+                                    ,(oauth2-token-access-response token)))
+      (plstore-save plstore)
+      token)))
 
 (defun oauth2-url-append-access-token (token url)
-  "Append access token to URL."
+  "Append access TOKEN to URL."
   (concat url
           (if (string-match-p "\?" url) "&" "?")
           "access_token=" (oauth2-token-access-token token)))
 
-(defvar oauth--url-advice nil)
+(defvar oauth2--url-advice nil)
 (defvar oauth--token-data)
 
 (defun oauth2-authz-bearer-header (token)
@@ -216,7 +262,7 @@ This allows to store the token in an unique way."
 
 ;; FIXME: We should change URL so that this can be done without an advice.
 (defun oauth2--url-http-handle-authentication-hack (orig-fun &rest args)
-  (if (not oauth--url-advice)
+  (if (not oauth2--url-advice)
       (apply orig-fun args)
     (let ((url-request-method url-http-method)
           (url-request-data url-http-data)
@@ -227,8 +273,8 @@ This allows to store the token in an unique way."
                              url-callback-function
                              url-callback-arguments)
       ;; This is to make `url' think it's done.
-      (when (boundp 'success) (setq success t)) ;For URL library in Emacs<24.4.
-      t)))                                      ;For URL library in Emacs≥24.4.
+      t)))
+
 (advice-add 'url-http-handle-authentication :around
             #'oauth2--url-http-handle-authentication-hack)
 
@@ -237,7 +283,7 @@ This allows to store the token in an unique way."
   "Retrieve an URL synchronously using TOKEN to access it.
 TOKEN can be obtained with `oauth2-auth'."
   (let* ((oauth--token-data (cons token url)))
-    (let ((oauth--url-advice t)         ;Activate our advice.
+    (let ((oauth2--url-advice t)         ;Activate our advice.
           (url-request-method request-method)
           (url-request-data request-data)
           (url-request-extra-headers
@@ -246,14 +292,14 @@ TOKEN can be obtained with `oauth2-auth'."
 
 ;;;###autoload
 (defun oauth2-url-retrieve (token url callback &optional
-                                  cbargs
-                                  request-method request-data request-extra-headers)
+                            cbargs
+                            request-method request-data request-extra-headers)
   "Retrieve an URL asynchronously using TOKEN to access it.
 TOKEN can be obtained with `oauth2-auth'.  CALLBACK gets called with CBARGS
 when finished.  See `url-retrieve'."
   ;; TODO add support for SILENT and INHIBIT-COOKIES.  How to handle this in `url-http-handle-authentication'.
   (let* ((oauth--token-data (cons token url)))
-    (let ((oauth--url-advice t)         ;Activate our advice.
+    (let ((oauth2--url-advice t)         ;Activate our advice.
           (url-request-method request-method)
           (url-request-data request-data)
           (url-request-extra-headers
-- 
2.34.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-oauth2-oauth2-token-Fix-formatting.patch --]
[-- Type: text/x-patch, Size: 1342 bytes --]

From 46f26e03c8b4f42c407077263c0f1e9371e07d4d Mon Sep 17 00:00:00 2001
Message-Id: <46f26e03c8b4f42c407077263c0f1e9371e07d4d.1638101267.git.avityazev@posteo.org>
In-Reply-To: <cover.1638101267.git.avityazev@posteo.org>
References: <cover.1638101267.git.avityazev@posteo.org>
From: Aleksandr Vityazev <avityazev@posteo.org>
Date: Thu, 25 Nov 2021 11:07:13 +0300
Subject: [PATCH 02/11] * oauth2: (oauth2-token): Fix formatting.

---
 oauth2.el | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/oauth2.el b/oauth2.el
index 1444890..fff89af 100644
--- a/oauth2.el
+++ b/oauth2.el
@@ -101,13 +101,13 @@ Return the code provided by the service."
         data))))
 
 (cl-defstruct oauth2-token
-  (plstore "" :read-only t)
-  (plstore-id "" :read-only t)
-  (client-id "" :read-only t)
-  (client-secret "" :read-only t)
-  (access-token "" :read-only t)
-  (refresh-token "" :read-only t)
-  (token-url "" :read-only t)
+  (plstore         "" :read-only t)
+  (plstore-id      "" :read-only t)
+  (client-id       "" :read-only t)
+  (client-secret   "" :read-only t)
+  (access-token    "" :read-only t)
+  (refresh-token   "" :read-only t)
+  (token-url       "" :read-only t)
   (access-response "" :read-only t))
 
 (cl-defun oauth2-request-access (token-url client-id client-secret code &optional
-- 
2.34.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0004-oauth2-Remove-oauth2-url-http-handle-authentication-.patch --]
[-- Type: text/x-patch, Size: 8892 bytes --]

From fb0ded3ba9e8b1d3c2cb10942119874d1e8d0251 Mon Sep 17 00:00:00 2001
Message-Id: <fb0ded3ba9e8b1d3c2cb10942119874d1e8d0251.1638101267.git.avityazev@posteo.org>
In-Reply-To: <cover.1638101267.git.avityazev@posteo.org>
References: <cover.1638101267.git.avityazev@posteo.org>
From: Aleksandr Vityazev <avityazev@posteo.org>
Date: Thu, 25 Nov 2021 15:33:57 +0300
Subject: [PATCH 04/11] * oauth2: Remove
 oauth2--url-http-handle-authentication-hack

(oauth2-auth-and-store): Check the token expiration date.
(oauth2-token): Add new slots.
---
 oauth2.el | 91 +++++++++++++++++++++++++------------------------------
 1 file changed, 41 insertions(+), 50 deletions(-)

diff --git a/oauth2.el b/oauth2.el
index fff89af..31da145 100644
--- a/oauth2.el
+++ b/oauth2.el
@@ -108,6 +108,8 @@ Return the code provided by the service."
   (access-token    "" :read-only t)
   (refresh-token   "" :read-only t)
   (token-url       "" :read-only t)
+  (expires-in      "" :read-only t)
+  (created-at      "" :read-only t)
   (access-response "" :read-only t))
 
 (cl-defun oauth2-request-access (token-url client-id client-secret code &optional
@@ -155,6 +157,8 @@ TOKEN should be obtained with `oauth2-request-access'."
                                          :client-secret client-secret
                                          :access-token (cdr (assoc 'access_token response))
                                          :refresh-token (cdr (assoc 'refresh_token response))
+                                         :expires-in (cdr (assoc 'expires_in response))
+                                         :created-at (cdr (assoc 'created_at response))
                                          :token-url token-url
                                          :access-response response)))
       (when plstore
@@ -169,7 +173,8 @@ TOKEN should be obtained with `oauth2-request-access'."
       new-token)))
 
 ;;;###autoload
-(defun oauth2-auth (auth-url token-url client-id client-secret &optional scope state redirect-uri)
+(defun oauth2-auth
+    (auth-url token-url client-id client-secret &optional scope state redirect-uri)
   "Authenticate application via OAuth2 at AUTH-URL and TOKEN-URL.
 CLIENT-ID is the client id provided by the provider.
 CLIENT-SECRET is the client secret associated with your CLIENT-ID.
@@ -195,6 +200,8 @@ Return an `oauth2-token' structure."
                        :client-secret client-secret
                        :access-token (cdr (assoc 'access_token response))
                        :refresh-token (cdr (assoc 'refresh_token response))
+                       :expires-in (cdr (assoc 'expires_in response))
+                       :created-at (cdr (assoc 'created_at response))
                        :token-url token-url
                        :access-response response)))
 
@@ -204,7 +211,8 @@ This allows to store the token in an unique way."
   (secure-hash 'md5 (concat auth-url token-url scope)))
 
 ;;;###autoload
-(defun oauth2-auth-and-store (auth-url token-url client-id client-secret scope &optional state redirect-uri)
+(defun oauth2-auth-and-store
+    (auth-url token-url client-id client-secret scope &optional state redirect-uri)
   "Request access to a resource and store it using `plstore'.
 If the token has not yet been saved in plsote, then authenticate
 the application via OAuth2 using the AUTH-URL, TOKEN-URL and after that
@@ -219,17 +227,25 @@ uses to maintain state between the request and redirect response.
 Return an `oauth2-token' structure."
   (if-let ((plstore (plstore-open oauth2-token-file))
            (id (oauth2-compute-id auth-url token-url scope))
-           (plist (cdr (plstore-get plstore id))))
+           (plist (cdr (plstore-get plstore id)))
+           (expires (cdr (assoc 'expires_in (plist-get plist :access-response))))
+           (created (cdr (assoc 'created_at (plist-get plist :access-response))))
+           (current-time (time-convert nil 'integer))
+           (token (make-oauth2-token :plstore plstore
+                                     :plstore-id id
+                                     :client-id client-id
+                                     :client-secret client-secret
+                                     :access-token (plist-get plist :access-token)
+                                     :refresh-token (plist-get plist :refresh-token)
+                                     :expires-in expires
+                                     :created-at created
+                                     :token-url token-url
+                                     :access-response (plist-get plist :access-response))))
       ;; Check if we found something matching this access
       ;; We did, return the token object
-      (make-oauth2-token :plstore plstore
-                         :plstore-id id
-                         :client-id client-id
-                         :client-secret client-secret
-                         :access-token (plist-get plist :access-token)
-                         :refresh-token (plist-get plist :refresh-token)
-                         :token-url token-url
-                         :access-response (plist-get plist :access-response))
+      (if (< current-time (+ expires created))
+          token
+        (oauth2-refresh-access token))
     (let ((token (oauth2-auth auth-url token-url
                         client-id client-secret scope state redirect-uri)))
       (plstore-put plstore id nil `(:access-token
@@ -247,48 +263,25 @@ Return an `oauth2-token' structure."
           (if (string-match-p "\?" url) "&" "?")
           "access_token=" (oauth2-token-access-token token)))
 
-(defvar oauth2--url-advice nil)
-(defvar oauth--token-data)
-
 (defun oauth2-authz-bearer-header (token)
   "Return `Authoriztions: Bearer' header with TOKEN."
   (cons "Authorization" (format "Bearer %s" token)))
 
-(defun oauth2-extra-headers (extra-headers)
+(defun oauth2-extra-headers (token extra-headers)
   "Return EXTRA-HEADERS with `Authorization: Bearer' added."
-  (cons (oauth2-authz-bearer-header (oauth2-token-access-token (car oauth--token-data)))
+  (cons (oauth2-authz-bearer-header (oauth2-token-access-token token))
         extra-headers))
 
-
-;; FIXME: We should change URL so that this can be done without an advice.
-(defun oauth2--url-http-handle-authentication-hack (orig-fun &rest args)
-  (if (not oauth2--url-advice)
-      (apply orig-fun args)
-    (let ((url-request-method url-http-method)
-          (url-request-data url-http-data)
-          (url-request-extra-headers
-           (oauth2-extra-headers url-http-extra-headers)))
-      (oauth2-refresh-access (car oauth--token-data))
-      (url-retrieve-internal (cdr oauth--token-data)
-                             url-callback-function
-                             url-callback-arguments)
-      ;; This is to make `url' think it's done.
-      t)))
-
-(advice-add 'url-http-handle-authentication :around
-            #'oauth2--url-http-handle-authentication-hack)
-
 ;;;###autoload
-(defun oauth2-url-retrieve-synchronously (token url &optional request-method request-data request-extra-headers)
+(defun oauth2-url-retrieve-synchronously
+    (token url &optional request-method request-data request-extra-headers)
   "Retrieve an URL synchronously using TOKEN to access it.
 TOKEN can be obtained with `oauth2-auth'."
-  (let* ((oauth--token-data (cons token url)))
-    (let ((oauth2--url-advice t)         ;Activate our advice.
-          (url-request-method request-method)
-          (url-request-data request-data)
-          (url-request-extra-headers
-           (oauth2-extra-headers request-extra-headers)))
-      (url-retrieve-synchronously url))))
+  (let ((url-request-method request-method)
+        (url-request-data request-data)
+        (url-request-extra-headers
+         (oauth2-extra-headers token request-extra-headers)))
+    (url-retrieve-synchronously url)))
 
 ;;;###autoload
 (defun oauth2-url-retrieve (token url callback &optional
@@ -298,13 +291,11 @@ TOKEN can be obtained with `oauth2-auth'."
 TOKEN can be obtained with `oauth2-auth'.  CALLBACK gets called with CBARGS
 when finished.  See `url-retrieve'."
   ;; TODO add support for SILENT and INHIBIT-COOKIES.  How to handle this in `url-http-handle-authentication'.
-  (let* ((oauth--token-data (cons token url)))
-    (let ((oauth2--url-advice t)         ;Activate our advice.
-          (url-request-method request-method)
-          (url-request-data request-data)
-          (url-request-extra-headers
-           (oauth2-extra-headers request-extra-headers)))
-      (url-retrieve url callback cbargs))))
+  (let ((url-request-method request-method)
+        (url-request-data request-data)
+        (url-request-extra-headers
+         (oauth2-extra-headers token request-extra-headers)))
+    (url-retrieve url callback cbargs)))
 
 (provide 'oauth2)
 
-- 
2.34.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0005-oauth2-oauth2-extra-headers-Update-doc-string.patch --]
[-- Type: text/x-patch, Size: 1021 bytes --]

From 2224924ea465450c0a9156d101a8552df48e6178 Mon Sep 17 00:00:00 2001
Message-Id: <2224924ea465450c0a9156d101a8552df48e6178.1638101267.git.avityazev@posteo.org>
In-Reply-To: <cover.1638101267.git.avityazev@posteo.org>
References: <cover.1638101267.git.avityazev@posteo.org>
From: Aleksandr Vityazev <avityazev@posteo.org>
Date: Fri, 26 Nov 2021 16:09:02 +0300
Subject: [PATCH 05/11] * oauth2 (oauth2-extra-headers): Update doc string.

---
 oauth2.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/oauth2.el b/oauth2.el
index 31da145..f77ae6a 100644
--- a/oauth2.el
+++ b/oauth2.el
@@ -268,7 +268,7 @@ Return an `oauth2-token' structure."
   (cons "Authorization" (format "Bearer %s" token)))
 
 (defun oauth2-extra-headers (token extra-headers)
-  "Return EXTRA-HEADERS with `Authorization: Bearer' added."
+  "Return EXTRA-HEADERS with `Authorization: Bearer' header with TOKEN added."
   (cons (oauth2-authz-bearer-header (oauth2-token-access-token token))
         extra-headers))
 
-- 
2.34.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: 0006-oauth2-Use-rx-with-string-match-p.patch --]
[-- Type: text/x-patch, Size: 1457 bytes --]

From a0affb78c3536c02189f291549553bcb3ae7d76c Mon Sep 17 00:00:00 2001
Message-Id: <a0affb78c3536c02189f291549553bcb3ae7d76c.1638101267.git.avityazev@posteo.org>
In-Reply-To: <cover.1638101267.git.avityazev@posteo.org>
References: <cover.1638101267.git.avityazev@posteo.org>
From: Aleksandr Vityazev <avityazev@posteo.org>
Date: Fri, 26 Nov 2021 16:14:43 +0300
Subject: [PATCH 06/11] * oauth2: Use rx with string-match-p.

---
 oauth2.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/oauth2.el b/oauth2.el
index f77ae6a..3dab614 100644
--- a/oauth2.el
+++ b/oauth2.el
@@ -75,7 +75,7 @@ uses to maintain state between the request and redirect response.
 
 Return the code provided by the service."
   (browse-url (concat auth-url
-                      (if (string-match-p "\?" auth-url) "&" "?")
+                      (if (string-match-p (rx "?") auth-url) "&" "?")
                       "client_id=" (url-hexify-string client-id)
                       "&response_type=code"
                       "&redirect_uri=" (url-hexify-string redirect-uri)
@@ -260,7 +260,7 @@ Return an `oauth2-token' structure."
 (defun oauth2-url-append-access-token (token url)
   "Append access TOKEN to URL."
   (concat url
-          (if (string-match-p "\?" url) "&" "?")
+          (if (string-match-p (rx "?") url) "&" "?")
           "access_token=" (oauth2-token-access-token token)))
 
 (defun oauth2-authz-bearer-header (token)
-- 
2.34.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #7: 0007-oauth2-oauth2-epoch-time-New-function.patch --]
[-- Type: text/x-patch, Size: 3014 bytes --]

From 1272151cb165831170834a2c34cc646c09a0b4c6 Mon Sep 17 00:00:00 2001
Message-Id: <1272151cb165831170834a2c34cc646c09a0b4c6.1638101267.git.avityazev@posteo.org>
In-Reply-To: <cover.1638101267.git.avityazev@posteo.org>
References: <cover.1638101267.git.avityazev@posteo.org>
From: Aleksandr Vityazev <avityazev@posteo.org>
Date: Sat, 27 Nov 2021 15:42:47 +0300
Subject: [PATCH 07/11] * oauth2 (oauth2--epoch-time): New function.

---
 oauth2.el | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/oauth2.el b/oauth2.el
index 3dab614..4bb886e 100644
--- a/oauth2.el
+++ b/oauth2.el
@@ -112,6 +112,10 @@ Return the code provided by the service."
   (created-at      "" :read-only t)
   (access-response "" :read-only t))
 
+(defun oauth2--epoch-time ()
+  "Return seconds since the Epoch."
+  (time-convert (current-time) 'integer))
+
 (cl-defun oauth2-request-access (token-url client-id client-secret code &optional
                                      (redirect-uri "urn:ietf:wg:oauth:2.0:oob"))
   "Request OAuth access at TOKEN-URL.
@@ -158,7 +162,8 @@ TOKEN should be obtained with `oauth2-request-access'."
                                          :access-token (cdr (assoc 'access_token response))
                                          :refresh-token (cdr (assoc 'refresh_token response))
                                          :expires-in (cdr (assoc 'expires_in response))
-                                         :created-at (cdr (assoc 'created_at response))
+                                         :created-at (let ((it (cdr (assoc 'created_at response))))
+                                                       (if it it (oauth2--epoch-time)))
                                          :token-url token-url
                                          :access-response response)))
       (when plstore
@@ -201,7 +206,8 @@ Return an `oauth2-token' structure."
                        :access-token (cdr (assoc 'access_token response))
                        :refresh-token (cdr (assoc 'refresh_token response))
                        :expires-in (cdr (assoc 'expires_in response))
-                       :created-at (cdr (assoc 'created_at response))
+                       :created-at (let ((it (cdr (assoc 'created_at response))))
+                                     (if it it (oauth2--epoch-time)))
                        :token-url token-url
                        :access-response response)))
 
@@ -230,7 +236,7 @@ Return an `oauth2-token' structure."
            (plist (cdr (plstore-get plstore id)))
            (expires (cdr (assoc 'expires_in (plist-get plist :access-response))))
            (created (cdr (assoc 'created_at (plist-get plist :access-response))))
-           (current-time (time-convert nil 'integer))
+           (current-time (oauth2--epoch-time))
            (token (make-oauth2-token :plstore plstore
                                      :plstore-id id
                                      :client-id client-id
-- 
2.34.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #8: 0008-oauth2-oauth2-created-at-New-function.patch --]
[-- Type: text/x-patch, Size: 2703 bytes --]

From 111c0d7c6b928c632d12870de0b531bb6b32e06f Mon Sep 17 00:00:00 2001
Message-Id: <111c0d7c6b928c632d12870de0b531bb6b32e06f.1638101267.git.avityazev@posteo.org>
In-Reply-To: <cover.1638101267.git.avityazev@posteo.org>
References: <cover.1638101267.git.avityazev@posteo.org>
From: Aleksandr Vityazev <avityazev@posteo.org>
Date: Sun, 28 Nov 2021 02:19:08 +0300
Subject: [PATCH 08/11] * oauth2 (oauth2--created-at): New function.

---
 oauth2.el | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/oauth2.el b/oauth2.el
index 4bb886e..7ca6452 100644
--- a/oauth2.el
+++ b/oauth2.el
@@ -116,6 +116,14 @@ Return the code provided by the service."
   "Return seconds since the Epoch."
   (time-convert (current-time) 'integer))
 
+(defun oauth2--created-at (response)
+  "Return epoch time from RESPONSE.
+If there no created_at key in RESPONSE, add
+\(cons 'created_at . epoch-time\) to RESPONSE."
+  (let ((it (cdr (assoc 'created_at response))))
+    (if it it (prog1 (oauth2--epoch-time)
+                (cl-pushnew (cons 'created_at (oauth2--epoch-time)) response)))))
+
 (cl-defun oauth2-request-access (token-url client-id client-secret code &optional
                                      (redirect-uri "urn:ietf:wg:oauth:2.0:oob"))
   "Request OAuth access at TOKEN-URL.
@@ -162,8 +170,7 @@ TOKEN should be obtained with `oauth2-request-access'."
                                          :access-token (cdr (assoc 'access_token response))
                                          :refresh-token (cdr (assoc 'refresh_token response))
                                          :expires-in (cdr (assoc 'expires_in response))
-                                         :created-at (let ((it (cdr (assoc 'created_at response))))
-                                                       (if it it (oauth2--epoch-time)))
+                                         :created-at (oauth2--created-at response)
                                          :token-url token-url
                                          :access-response response)))
       (when plstore
@@ -206,8 +213,7 @@ Return an `oauth2-token' structure."
                        :access-token (cdr (assoc 'access_token response))
                        :refresh-token (cdr (assoc 'refresh_token response))
                        :expires-in (cdr (assoc 'expires_in response))
-                       :created-at (let ((it (cdr (assoc 'created_at response))))
-                                     (if it it (oauth2--epoch-time)))
+                       :created-at (oauth2--created-at response)
                        :token-url token-url
                        :access-response response)))
 
-- 
2.34.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #9: 0009-oauth2-oauth2-auth-and-store-Remove-comment.patch --]
[-- Type: text/x-patch, Size: 1072 bytes --]

From 4b71a470af257faf31d27987b6d4bffb6ce67e00 Mon Sep 17 00:00:00 2001
Message-Id: <4b71a470af257faf31d27987b6d4bffb6ce67e00.1638101267.git.avityazev@posteo.org>
In-Reply-To: <cover.1638101267.git.avityazev@posteo.org>
References: <cover.1638101267.git.avityazev@posteo.org>
From: Aleksandr Vityazev <avityazev@posteo.org>
Date: Sun, 28 Nov 2021 14:38:35 +0300
Subject: [PATCH 09/11] * oauth2 (oauth2-auth-and-store): Remove comment.

---
 oauth2.el | 2 --
 1 file changed, 2 deletions(-)

diff --git a/oauth2.el b/oauth2.el
index 7ca6452..9906e8a 100644
--- a/oauth2.el
+++ b/oauth2.el
@@ -253,8 +253,6 @@ Return an `oauth2-token' structure."
                                      :created-at created
                                      :token-url token-url
                                      :access-response (plist-get plist :access-response))))
-      ;; Check if we found something matching this access
-      ;; We did, return the token object
       (if (< current-time (+ expires created))
           token
         (oauth2-refresh-access token))
-- 
2.34.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #10: 0010-oauth2-oauth2-created-at-Use-if-let.patch --]
[-- Type: text/x-patch, Size: 1319 bytes --]

From 09e8df6d652d87d04b75d0bbea8f5e7494df991c Mon Sep 17 00:00:00 2001
Message-Id: <09e8df6d652d87d04b75d0bbea8f5e7494df991c.1638101267.git.avityazev@posteo.org>
In-Reply-To: <cover.1638101267.git.avityazev@posteo.org>
References: <cover.1638101267.git.avityazev@posteo.org>
From: Aleksandr Vityazev <avityazev@posteo.org>
Date: Sun, 28 Nov 2021 14:46:18 +0300
Subject: [PATCH 10/11] * oauth2 (oauth2--created-at): Use if-let.

---
 oauth2.el | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/oauth2.el b/oauth2.el
index 9906e8a..49a12bc 100644
--- a/oauth2.el
+++ b/oauth2.el
@@ -120,9 +120,10 @@ Return the code provided by the service."
   "Return epoch time from RESPONSE.
 If there no created_at key in RESPONSE, add
 \(cons 'created_at . epoch-time\) to RESPONSE."
-  (let ((it (cdr (assoc 'created_at response))))
-    (if it it (prog1 (oauth2--epoch-time)
-                (cl-pushnew (cons 'created_at (oauth2--epoch-time)) response)))))
+  (if-let ((it (cdr (assoc 'created_at response))))
+      it
+    (prog1 (oauth2--epoch-time)
+      (cl-pushnew (cons 'created_at (oauth2--epoch-time)) response))))
 
 (cl-defun oauth2-request-access (token-url client-id client-secret code &optional
                                      (redirect-uri "urn:ietf:wg:oauth:2.0:oob"))
-- 
2.34.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #11: 0011-oauth2-oauth2-authz-bearer-header-Change-arg-name.patch --]
[-- Type: text/x-patch, Size: 1260 bytes --]

From 9c6c00c0abde2a17f0833e074f20114636a3341f Mon Sep 17 00:00:00 2001
Message-Id: <9c6c00c0abde2a17f0833e074f20114636a3341f.1638101267.git.avityazev@posteo.org>
In-Reply-To: <cover.1638101267.git.avityazev@posteo.org>
References: <cover.1638101267.git.avityazev@posteo.org>
From: Aleksandr Vityazev <avityazev@posteo.org>
Date: Sun, 28 Nov 2021 15:07:20 +0300
Subject: [PATCH 11/11] * oauth2 (oauth2-authz-bearer-header): Change arg name.

---
 oauth2.el | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/oauth2.el b/oauth2.el
index 49a12bc..b8227a4 100644
--- a/oauth2.el
+++ b/oauth2.el
@@ -274,9 +274,9 @@ Return an `oauth2-token' structure."
           (if (string-match-p (rx "?") url) "&" "?")
           "access_token=" (oauth2-token-access-token token)))
 
-(defun oauth2-authz-bearer-header (token)
-  "Return `Authoriztions: Bearer' header with TOKEN."
-  (cons "Authorization" (format "Bearer %s" token)))
+(defun oauth2-authz-bearer-header (access-token)
+  "Return `Authoriztions: Bearer' header with ACCESS-TOKEN."
+  (cons "Authorization" (format "Bearer %s" access-token)))
 
 (defun oauth2-extra-headers (token extra-headers)
   "Return EXTRA-HEADERS with `Authorization: Bearer' header with TOKEN added."
-- 
2.34.0


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2021-11-28 12:38 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-28 12:38 [ELPA/oauth2] Request for patches review Aleksandr Vityazev

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).