Forge uses APIHOST including full url for auth-source-search bugging passwords-store #720
Labels
enhancement
New feature or request
Comments
|
I am considering @@ -764,9 +764,14 @@ (defun ghub--basic-auth (host username)
(defun ghub--token (host username package &optional nocreate forge)
(let* ((user (ghub--ident username package))
+ (host* (and (string-match "/" host)
+ (substring host 0 (match-beginning 0))))
(token
(or (car (ghub--auth-source-get (list :secret)
:host host :user user))
+ (and host*
+ (car (ghub--auth-source-get (list :secret)
+ :host host :user user)))
(progn
;; Auth-Source caches the information that there is no
;; value, but in our case that is a situation that needs
@@ -775,6 +780,8 @@ (defun ghub--token (host username package &optional nocreate forge)
;; The (:max 1) is needed and has to be placed at the
;; end for Emacs releases before 26.1. #24 #64 #72
(auth-source-forget (list :host host :user user :max 1))
+ (when host*
+ (auth-source-forget (list :host host* :user user :max 1)))
(and (not nocreate)
(error "\
Required %s token (\"%s\" for \"%s\") does not exist.but to many suggestions are coming in right now and I won't be able to deal with this any time soon. |
Take your time! What I did to workaround/fix the issue was this: diff --git a/lisp/ghub.el b/lisp/ghub.el
index 4451526..d98949d 100644
--- a/lisp/ghub.el
+++ b/lisp/ghub.el
@@ -775,7 +775,7 @@ and call `auth-source-forget+'."
(let* ((user (ghub--ident username package))
(token
(or (car (ghub--auth-source-get (list :secret)
- :host host :user user))
+ :host (url-host (url-generic-parse-url (concat "//" host))) :user user))
(progn
;; Auth-Source caches the information that there is no
;; value, but in our case that is a situation that needsI was going to post this earlier but forgot. |
Thaodan
added a commit
to Thaodan/ghub
that referenced
this issue
Nov 3, 2024
Signed-off-by: Björn Bidar <[email protected]>
Thaodan
added a commit
to Thaodan/emacs.d
that referenced
this issue
Nov 3, 2024
Signed-off-by: Björn Bidar <[email protected]>
tarsius
added a commit
to magit/ghub
that referenced
this issue
Jan 13, 2025
I was unable to reproduce this issue (both with Just in case I misunderstood, you are saying that if some host (e.g., ;;; Test utility
(defun pass-entry-p (host user)
"If an entry is found for USER on HOST, return USER, else nil."
(plist-get (car (auth-source-pass-search :host host :user user)) :user))
;;; Commands for copy-pasting into terminal
pass add [email protected]
pass add example.com/[email protected]
pass add [email protected]@example.com
pass rm [email protected]
pass rm example.com/[email protected]
pass rm [email protected]@example.com
;;; Test using email as user, no extra
;; $ pass ls
;; Password Store
;; └── [email protected]
(setq auth-source-pass-extra-query-keywords nil)
(pass-entry-p "gmail.com" "me") => "me"
;; $ pass ls
;; Password Store
;; ├── example.com This service uses
;; │ └── [email protected] email addresses as users
;; └── [email protected]
(setq auth-source-pass-extra-query-keywords nil)
(pass-entry-p "gmail.com" "me") => "me"
(pass-entry-p "example.com" "[email protected]") => "[email protected]"
;; $ pass ls
;; Password Store
;; ├── [email protected]@example.com
;; └── [email protected]
(setq auth-source-pass-extra-query-keywords nil)
(pass-entry-p "gmail.com" "me") => "me"
(pass-entry-p "example.com" "[email protected]") => "[email protected]"
;;; Test using email as user, with extra
;; $ pass ls
;; Password Store
;; └── [email protected]
(setq auth-source-pass-extra-query-keywords t)
(pass-entry-p "gmail.com" "me") => "me"
;; $ pass ls
;; Password Store
;; ├── example.com This service uses
;; │ └── [email protected] email addresses as users
;; └── [email protected]
(setq auth-source-pass-extra-query-keywords t)
(pass-entry-p "gmail.com" "me") => "me"
(pass-entry-p "example.com" "[email protected]") => "[email protected]"
;; $ pass ls
;; Password Store
;; ├── [email protected]@example.com
;; └── [email protected]
(setq auth-source-pass-extra-query-keywords t)
(pass-entry-p "gmail.com" "me") => "me"
(pass-entry-p "example.com" "[email protected]") => "[email protected]"I can however reproduce the issue you are having with Ghub includes the path as part of the host for Gitlab hosts. This is problematic, but worked until people started using alternative What Ghub sees as "host with a path", in At least in the most simple of tests, $ pass ls
Password Store
└── [email protected]
└── api
└── v4
$ pass show [email protected]/api/v4
abc
;; $ pass ls
;; Password Store
;; └── [email protected]
;; └── api
;; └── v4
;; $ pass show [email protected]/api/v4
;; abc
(setq auth-source-pass-extra-query-keywords nil)
(list
(auth-source-pass-search :host "gitlab.com")
(auth-source-pass-search :host "gitlab.com" :user "me")
(auth-source-pass-search :host "gitlab.com" :user "api/v4")
(auth-source-pass-search :host "gitlab.com/api/v4")
(auth-source-pass-search :host "gitlab.com/api/v4" :user "me")
(auth-source-pass-search :host "gitlab.com/api/v4" :user "api/v4")
) => (nil nil nil nil nil nil) |
|
Jonas Bernoulli ***@***.***> writes:
> - `auth-source-pass-extra-query-keywords` set to `t`
> - This option can be enabled to deal with usernames that contain
> ***@***.***` for other hosts to not break retrieving secrets for
> user `foo` on `bar.host`.
>
I was unable to reproduce this issue (both with `nil` and `t` as the value of this variable. Below I include my experiments. I would be interested in learning about the failure case, so if you can still reproduce this, please provide reproduction steps, similar to what I have tried here.
Just in case I misunderstood, you are saying that if some host (e.g., `example.com`) uses email addresses (e.g., ***@***.***`) as "user names", and we add entries for both to the password store, then `auth-source-pass` can no longer retrieve the password for the email address (a.k.a. user `me` at host `gmail.com`), right?
Yes kinda but this behavior is not a forge or ghub issue but one inside
auth-source. Just the :host parameter was used wrongly. Which meant that
now that with `auth-source-pass-extra-query-keywords` = `t` the :host
specifier passed to auth-source was taken into account with
auth-source-search in the pass backend.
So before the patch to the forge token for me in
Work/Gitlab/<instance-host>/<token-name> e.g.
Work/Gitla/gitlab.com/Thaodan^Forge
Setting auth-source-pass-extra-query-keywords to nil broke retrieving
secrets where the username is inside the password but that does not
affect Forge only setting it to t broke Github with forge.
I can however reproduce the issue you are having with `ghub` and `auth-source-pass`.
Ghub includes the *path* as part of the *host* for Gitlab hosts. This
is problematic, but worked until people started using alternative
`auth-source` back-ends. I would not do it if I were to start today,
but changing it now would be hard and risk regressions. For now I am
limiting changes to kludges, which come with a smaller risk of
breaking existing working setups.
An option would be to first to look with the proper hostname and then if
there is no result to use the full path.
What Ghub sees as "host with a path", in `pass` becomes an entry that specifies a user using the prefix notation, as well as using the postfix notation, and the two users are not the same.
I don't know of these notations sorry.
At least in the most simple of tests, `pass` itself can actually deal with this:
``` shell
$ pass ls
Password Store
└── ***@***.***
└── api
└── v4
$ pass show ***@***.***/api/v4
abc
```
pass show does open a specific password with name i.e. the relative
filename relative to the pass folder.
`auth-source-pass-source` however returns `nil`.
``` emacs-lisp
;; $ pass ls
;; Password Store
;; └── ***@***.***
;; └── api
;; └── v4
;; $ pass show ***@***.***/api/v4
;; abc
(setq auth-source-pass-extra-query-keywords nil)
(list
(auth-source-pass-search :host "gitlab.com")
(auth-source-pass-search :host "gitlab.com" :user "me")
(auth-source-pass-search :host "gitlab.com" :user "api/v4")
(auth-source-pass-search :host "gitlab.com/api/v4")
(auth-source-pass-search :host "gitlab.com/api/v4" :user "me")
(auth-source-pass-search :host "gitlab.com/api/v4" :user "api/v4")
) => (nil nil nil nil nil nil)
```
This example doesn't show the exact issue it is that if the :host is
taken into account by the pass backend that there's no host matching
gitlab.com/api/v4 but only github.com the hostname does not include any
parts of the path.
***@***.*** would not be the username realistically, in this example
there is no host if ***@***.*** is the username.
|
If it is possible to demonstrate that in, say, five commands, then please do.
That's what we do now. If by "proper hostname" you mean
I.e., if we use something that in our interpretation means |
|
Jonas Bernoulli <[email protected]> writes:
>Setting auth-source-pass-extra-query-keywords to nil broke retrieving
> secrets where the username is inside the password but that does not
> affect Forge only setting it to t broke Github with forge.
If it is possible to demonstrate that in, say, five commands, then
please do.
;; No match
(let ((auth-source-pass-extra-query-keywords t))
(auth-source-forget-all-cached)
(auth-source-search :host "gitlab.com/api/v3"
:user "user^forge"))
;; Match
(let ((auth-source-pass-extra-query-keywords nil))
(auth-source-forget-all-cached)
(auth-source-search :host "gitlab.com/api/v3"
:user "user^forge"))
;; No match
(let ((auth-source-pass-extra-query-keywords t))
(auth-source-forget-all-cached)
(auth-source-search :host "gitlab.com/api/v3"
:user "user^forge"
:secret))
;; Match
(let ((auth-source-pass-extra-query-keywords nil))
(auth-source-forget-all-cached)
(auth-source-search :host "gitlab.com/api/v3"
:user "user^forge"
:secret))
;; Match
(let ((auth-source-pass-extra-query-keywords t))
(auth-source-forget-all-cached)
(auth-source-search :host "gitlab.com"
:user "user^forge"
:secret))
;; Match
;; For this case there might be match for the file
;; user^[email protected] in the root if github.com/user^forge
;; would be in a subfolder which would match after the file outside
;; of a folder if auth-source-pass-extra-query-keywords isn't t.
(let ((auth-source-pass-extra-query-keywords nil))
(auth-source-forget-all-cached)
(auth-source-search :host "gitlab.com"
:user "user^forge"
:secret))
In all these examples the password file is in github.com/user^forge.
> An option would be to first to look with the proper hostname and
> then if there is no result to use the full path.
That's what we do now. If by "proper hostname" you mean
`<actual-host>/<path>`, i.e, "what forge traditionally needed". I
wouldn't use the word "proper" for that, given that the issue we are
dealing with, is exactly caused/triggered by using something that
isn't actually just a hostname, in a field intended hold just an
actual hostname.
By "proper hostname" I mean the hostname without the path, proper as
seen by auth-source. That would mean hostname plus path would be invalid
for :host as that slot only refers to the hostname in the general sense
i.e. machine name plus optional.
:host is always taken into account if
auth-source-pass-extra-query-keywords is t than there a :host attribute
to match against just as with other auth-source backends.
Without the variable being t just :user is considered which will match
as the user is supposed to be user^forge.
>> What Ghub sees as "host with a path", in `pass` becomes an entry
> that specifies a user using the prefix notation, as well as using
> the postfix notation, and the two users are not the same.
>
> I don't know of these notations sorry.
- User as a prefix: ***@***.***`.
- User as a suffix: `host/user`. (I called it "postfix" before.)
I.e., if we use something that in our interpretation means
***@***.***/path`, then `pass` sees ***@***.***/another-user?!?`.
Auth-source-pass (and other applications) can guess the right secret
from the attributes of the password.
Without auth-source-pass-extra-keywords the auth-source pass backend
tries the first best possible match which is:
- hostname.tld i.e. example.com
- hostname.tld/user i.e. example.com/user
- [email protected] i.e. [email protected]
Then the list above repeats but with the port
taken int account too.
At the very bottom it's the same list again but subfolders take into
account i.e. a/example.com/user.
This behavior is unlike other pass clients such as browser addons which
always match against host just if auth-source-pass-extra-keywords is t.
The path after the hostname is never taken int account. It is always,
hostname or user. As pass can carry any arbitrary key value pair the
port key which auth-sourch does support can also exist.
With auth-source-pass-extra-keywords is t the :host keyword is checked
for a match before returning a match.
This ensures that cases where the username is the user email
address can match but a path containing the :host and :user matches.
If the variable is nil or not does not matter for ghub as long as the
:host keyword contains a valid host to match against.
I tried to explain everything as best as I could, if there's anything
else check the manual please.
|
|
Thanks! I don't have the time to study this now, but will make sure to not let it go to waste. Ps: When you post by email, email addresses get obfuscated. Could you please post the parts that ended up looking like this again?: |
|
I edited my comment t remove the obfuscation.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Preconditions
auth-source-pass-extra-query-keywordsset tot[email protected]for other hosts to not break retrieving secrets for userfooon
bar.host.password-storeinsideauth-sources.Environment
Same for All dependencies
Expected
Secret is found.
Observed
Forge uses the full url that includes not just the host for Gitlab based Forges. Forge should not use the full url to retrieve passwords I think.
Backtrace
The text was updated successfully, but these errors were encountered: