GNU bug report logs -
#52476
Fwd: [ELPA/oauth2] Request for patches review
Previous Next
To reply to this bug, email your comments to 52476 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
avityazev <at> posteo.org, julien <at> danjou.info, bug-gnu-emacs <at> gnu.org:
bug#52476; Package
emacs.
(Tue, 14 Dec 2021 02:40:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Stefan Kangas <stefan <at> marxist.se>:
New bug report received and forwarded. Copy sent to
avityazev <at> posteo.org, julien <at> danjou.info, bug-gnu-emacs <at> gnu.org.
(Tue, 14 Dec 2021 02:40:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
I have seen no followups to this within two weeks, so I'm forwarding
this to the bug tracker so that we don't lose track of it.
-------------------- Start of forwarded message --------------------
From: Aleksandr Vityazev <avityazev <at> posteo.org>
To: emacs-devel <at> gnu.org, julien <at> danjou.info
Subject: [ELPA/oauth2] Request for patches review
Date: Sun, 28 Nov 2021 12:38:58 +0000
[Message part 2 (text/plain, attachment)]
[0001-oauth2-Update-version-to-0.17.patch (text/x-patch, attachment)]
[0002-oauth2-oauth2-token-Fix-formatting.patch (text/x-patch, attachment)]
[0004-oauth2-Remove-oauth2-url-http-handle-authentication-.patch (text/x-patch, attachment)]
[0005-oauth2-oauth2-extra-headers-Update-doc-string.patch (text/x-patch, attachment)]
[0006-oauth2-Use-rx-with-string-match-p.patch (text/x-patch, attachment)]
[0007-oauth2-oauth2-epoch-time-New-function.patch (text/x-patch, attachment)]
[0008-oauth2-oauth2-created-at-New-function.patch (text/x-patch, attachment)]
[0009-oauth2-oauth2-auth-and-store-Remove-comment.patch (text/x-patch, attachment)]
[0010-oauth2-oauth2-created-at-Use-if-let.patch (text/x-patch, attachment)]
[0011-oauth2-oauth2-authz-bearer-header-Change-arg-name.patch (text/x-patch, attachment)]
[Message part 13 (text/plain, attachment)]
Added tag(s) patch.
Request was from
Stefan Kangas <stefankangas <at> gmail.com>
to
control <at> debbugs.gnu.org.
(Thu, 18 Aug 2022 11:12:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#52476; Package
emacs.
(Thu, 08 Sep 2022 14:20:02 GMT)
Full text and
rfc822 format available.
Message #10 received at 52476 <at> debbugs.gnu.org (full text, mbox):
Stefan Kangas <stefan <at> marxist.se> writes:
> I have seen no followups to this within two weeks, so I'm forwarding
> this to the bug tracker so that we don't lose track of it.
This was half a year ago -- Julien, do you have any comments here?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#52476; Package
emacs.
(Thu, 08 Sep 2022 15:52:01 GMT)
Full text and
rfc822 format available.
Message #13 received at 52476 <at> debbugs.gnu.org (full text, mbox):
No comment sir.
--
Julien Danjou
On Thu, Sep 8, 2022, at 16:19, Lars Ingebrigtsen wrote:
> Stefan Kangas <stefan <at> marxist.se> writes:
>
>> I have seen no followups to this within two weeks, so I'm forwarding
>> this to the bug tracker so that we don't lose track of it.
>
> This was half a year ago -- Julien, do you have any comments here?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#52476; Package
emacs.
(Fri, 09 Sep 2022 17:07:02 GMT)
Full text and
rfc822 format available.
Message #16 received at 52476 <at> debbugs.gnu.org (full text, mbox):
"Julien Danjou" <julien <at> danjou.info> writes:
> No comment sir.
I'm not familiar with the oath2 code at all (and don't use the library).
Could somebody that uses this take a look at this patch series?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#52476; Package
emacs.
(Sun, 23 Feb 2025 01:40:02 GMT)
Full text and
rfc822 format available.
Message #19 received at 52476 <at> debbugs.gnu.org (full text, mbox):
Stefan Kangas <stefan <at> marxist.se> writes:
> I have seen no followups to this within two weeks, so I'm forwarding
> this to the bug tracker so that we don't lose track of it.
>
> -------------------- Start of forwarded message --------------------
> From: Aleksandr Vityazev <avityazev <at> posteo.org>
> To: emacs-devel <at> gnu.org, julien <at> danjou.info
> Subject: [ELPA/oauth2] Request for patches review
> Date: Sun, 28 Nov 2021 12:38:58 +0000
Xiyue, since you have looked into oauth2 recently, perhaps you would be
willing to take a look at this? I'm not sure how relevant any of this
is with your recent changes.
You can find the patches here:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=52476
Severity set to 'wishlist' from 'normal'
Request was from
Stefan Kangas <stefankangas <at> gmail.com>
to
control <at> debbugs.gnu.org.
(Sun, 23 Feb 2025 01:40:03 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#52476; Package
emacs.
(Sun, 23 Feb 2025 22:07:01 GMT)
Full text and
rfc822 format available.
Message #24 received at 52476 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi Stefan,
Stefan Kangas <stefankangas <at> gmail.com> writes:
> Stefan Kangas <stefan <at> marxist.se> writes:
>
>> I have seen no followups to this within two weeks, so I'm forwarding
>> this to the bug tracker so that we don't lose track of it.
>>
>> -------------------- Start of forwarded message --------------------
>> From: Aleksandr Vityazev <avityazev <at> posteo.org>
>> To: emacs-devel <at> gnu.org, julien <at> danjou.info
>> Subject: [ELPA/oauth2] Request for patches review
>> Date: Sun, 28 Nov 2021 12:38:58 +0000
>
> Xiyue, since you have looked into oauth2 recently, perhaps you would be
> willing to take a look at this? I'm not sure how relevant any of this
> is with your recent changes.
>
> You can find the patches here:
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=52476
Thanks for letting me know of this patch set - I wasn't when working on
the last changes on oauth2. I'll try to take a look on each patch and
try to summarize which parts are still good additions. I'll do one
patch per mail so that it doesn't become too long. This will take a
while, so please allow me some time.
--
Regards,
Xiyue Deng
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#52476; Package
emacs.
(Sun, 23 Feb 2025 23:17:01 GMT)
Full text and
rfc822 format available.
Message #27 received at 52476 <at> debbugs.gnu.org (full text, mbox):
Xiyue Deng <manphiz <at> gmail.com> writes:
> Hi Stefan,
>
> Stefan Kangas <stefankangas <at> gmail.com> writes:
>
>> Xiyue, since you have looked into oauth2 recently, perhaps you would be
>> willing to take a look at this? I'm not sure how relevant any of this
>> is with your recent changes.
>>
>> You can find the patches here:
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=52476
>
> Thanks for letting me know of this patch set - I wasn't when working on
> the last changes on oauth2. I'll try to take a look on each patch and
> try to summarize which parts are still good additions. I'll do one
> patch per mail so that it doesn't become too long. This will take a
> while, so please allow me some time.
Thanks! There's no rush, so please take your time.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#52476; Package
emacs.
(Mon, 05 May 2025 00:11:01 GMT)
Full text and
rfc822 format available.
Message #30 received at 52476 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Stefan Kangas <stefankangas <at> gmail.com> writes:
> Xiyue Deng <manphiz <at> gmail.com> writes:
>
>> Hi Stefan,
>>
>> Stefan Kangas <stefankangas <at> gmail.com> writes:
>>
>>> Xiyue, since you have looked into oauth2 recently, perhaps you would be
>>> willing to take a look at this? I'm not sure how relevant any of this
>>> is with your recent changes.
>>>
>>> You can find the patches here:
>>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=52476
>>
>> Thanks for letting me know of this patch set - I wasn't when working on
>> the last changes on oauth2. I'll try to take a look on each patch and
>> try to summarize which parts are still good additions. I'll do one
>> patch per mail so that it doesn't become too long. This will take a
>> while, so please allow me some time.
>
> Thanks! There's no rush, so please take your time.
Sorry for the unexpected long delay. It turns out what I proposed was a
bit too time consuming and it's easier to just give an overview with all
the patches and propose what I think may be beneficial to be submitted.
In general, the propose patch set has a few types of changes:
* Small clean ups
* Doc improvements
* Change of interface and implementation behavior
I think the first 2 can be incorporated, though for the last one I'd
refrain from adding at this time as they could break clients (like my
plugin :P)
I'll prepare a patch set with other improvements under a different bug.
Please see below for more detailed comments. Please do correct me if
any of my assumptions is wrong.
> diff --git a/oauth2.el b/oauth2.el
> index 87e0c39c5c..ae0d7c3174 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 <at> posteo.org>
> ;; Copyright (C) 2011-2021 Free Software Foundation, Inc
>
> ;; Author: Julien Danjou <julien <at> danjou.info>
> ;; 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
>
Changing the copyright holder and homepage should not be necessary.
> ;; 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))
> +
I don't find any actual use of 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)
> +
This moves the custom definition to the beginning alongside other
defvar/defcustom, which I think is good.
> (defvar url-http-data)
> (defvar url-http-method)
> (defvar url-http-extra-headers)
> @@ -62,25 +72,29 @@
> (setcar msg (concat "[oauth2] " (car msg)))
> (apply #'message msg)))
>
> -(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."
> - (let ((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"))
> - (if scope (concat "&scope=" (url-hexify-string scope)) "")
> - (if state (concat "&state=" (url-hexify-string state)) "")
> - ;; The following two parameters are required for Gmail
> - ;; OAuth2 to generate the refresh token
> - "&access_type=offline"
> - "&prompt=consent")))
> - (browse-url url)
> - (read-string (concat "Follow the instruction on your default browser, or "
> - "visit:\n" url
> - "\nEnter the code your browser displayed: "))))
> +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 (rx "?") auth-url) "&" "?")
> + "client_id=" (url-hexify-string client-id)
> + "&response_type=code"
> + "&redirect_uri=" (url-hexify-string redirect-uri)
> + (if scope (concat "&scope=" (url-hexify-string scope)) "")
> + (if state (concat "&state=" (url-hexify-string state)) "")
> + ;; The following two parameters are required for Gmail
> + ;; OAuth2 to generate the refresh token
> + "&access_type=offline"
> + "&prompt=consent"))
> + (read-string "Enter the code your browser displayed: "))
>
This part uses cl-defun to give `redirect-uri' a default value at the
declaration. I'm not sure whether this could change the interface. The
documentation improvements are good though.
> (defun oauth2-request-access-parse ()
> "Parse the result of an OAuth request."
> @@ -104,182 +118,213 @@ 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)
> + (expires-in "" :read-only t)
> + (created-at "" :read-only t)
> + (access-response "" :read-only t))
This makes the aouth2-token readonly. Though it could be a good thing,
it would require some changes to the implementation and hence some
public interfaces, so I'd suggest against it for now.
> +
> +(defun oauth2--epoch-time ()
> + "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."
> + (if-let ((it (cdr (assoc 'created_at response))))
> + it
> + (prog1 (oauth2--epoch-time)
> + (cl-pushnew (cons 'created_at (oauth2--epoch-time)) response))))
> +
This is part of the implementation to add support for reusing cached
access token during the time it's valid. I am also thinking about
implementing this, but in a slightly different way.
> +(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
> (url-encode-url
> (concat
> "client_id=" client-id
> - (when client-secret
> + (when client-secret
> (concat "&client_secret=" client-secret))
> "&code=" code
> - "&redirect_uri=" (or redirect-uri "urn:ietf:wg:oauth:2.0:oob")
> + "&redirect_uri=" (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)))
>
Similar to the other `cl-defun' change. It also make use of a new
interface added, so probably better not to change much here. The doc
improvements are good though.
> ;;;###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))
> + :expires-in (cdr (assoc 'expires_in response))
> + :created-at (oauth2--created-at 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)))
>
This part is because of oauth2-token is now readonly. I tend to
consider this as not strictly necessary.
> ;;;###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))
> +(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.
> +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."
>
> -(defun oauth2-compute-id (auth-url token-url scope client-id)
> - "Compute an unique id based on URLs.
> + (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))
> + :expires-in (cdr (assoc 'expires_in response))
> + :created-at (oauth2--created-at response)
> + :token-url token-url
> + :access-response response)))
> +
> +(defun oauth2-compute-id (auth-url token-url scope)
> + "Compute an unique id based on AUTH-URL, TOKEN-URL and SCOPE.
> This allows to store the token in an unique way."
> (secure-hash 'sha512 (concat auth-url token-url scope client-id)))
>
Similarly due to oauth2-token being readonly. Not strictly necessary.
> ;;;###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 client-id))
> - (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)))
> + (expires (cdr (assoc 'expires_in (plist-get plist :access-response))))
> + (created (cdr (assoc 'created_at (plist-get plist :access-response))))
> + (current-time (oauth2--epoch-time))
> + (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))))
> + (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
> + ,(oauth2-token-access-token token)
> + :refresh-token
> + ,(oauth2-token-refresh-token token)
> + :access-response
> + ,(oauth2-token-access-response token)))
> + (plstore-save plstore)
> + token)))
>
Ditto.
> (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) "&" "?")
> + (if (string-match-p (rx "?") url) "&" "?")
> "access_token=" (oauth2-token-access-token token)))
>
Slight improvement to doc and regexp. Not strictly required.
> -(defvar oauth--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-authz-bearer-header (access-token)
> + "Return `Authoriztions: Bearer' header with ACCESS-TOKEN."
> + (cons "Authorization" (format "Bearer %s" access-token)))
>
Slight improvements. Not really necessary.
> -(defun oauth2-extra-headers (extra-headers)
> - "Return EXTRA-HEADERS with `Authorization: Bearer' added."
> - (cons (oauth2-authz-bearer-header (oauth2-token-access-token (car oauth--token-data)))
> +(defun oauth2-extra-headers (token extra-headers)
> + "Return EXTRA-HEADERS with `Authorization: Bearer' header with TOKEN added."
> + (cons (oauth2-authz-bearer-header (oauth2-token-access-token token))
> extra-headers))
>
This changes the interface, so probably not good. Could probably
implement under a new name.
> -
> -;; 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)
> - (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.
> - (when (boundp 'success) (setq success t)) ;For URL library in Emacs<24.4.
> - t))) ;For URL library in Emacs≥24.4.
> -(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 ((oauth--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)))
>
This seems to change the implementation based on the tweaked function
above. So also not necessary.
> ;;;###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.
> - (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)))
Ditto.
>
> (provide 'oauth2)
>
--
Regards,
Xiyue Deng
[signature.asc (application/pgp-signature, inline)]
This bug report was last modified 41 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.