-
Notifications
You must be signed in to change notification settings - Fork 65
Auth: Make requirement to have username in repo URL optional? Or more doc examples #750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
I just reread #729 and realized you wrote a similar thing there too,
The admin config is also a good point. It won't be a problem with PPM since we'll be using |
Thanks for the thorough testing! We need a way to opt in into authenticated repos. On some platforms searching for passwords in the system keyring may require user interaction, so we can't just start looking for every configured repository. Having a username in the repo URL seems like a good way to opt in. But you are right about the admin config. One way to work around this is to allow an empty username in the URL. E.g.
Good idea.
We can't really do that because we can't search for credentials if there is no
We cannot do that, either. Well, we can make the username itself optional, but we need to require for the user to put at least a |
I see, that makes more sense now. On Linux, I think I was using the file backend so there was no login prompt. In that case.. what about making the username optional only for Or alternatively, I was just looking at pip and uv, and both do require opt-in to keyring: But it looks like they use a CLI flag, environment variable, or pip config setting instead. That could be another option besides the empty username. If there aren't any clear options, then I think also fine to put off for an initial release. Like I mentioned earlier, we won't have this problem with PPM since the username will always be the same. And for Workbench servers, it'll probably be the admin setting a global repo URL with credentials for all users, where the user doesn't actually have to configure anything. |
Well, because you already had your credentials set up in a keyring and the keyring was not locked. Otherwise there is definitely user interaction. (pak does not work well with the file backend btw.)
Maybe, but I'd prefer to keep this simple, at least for the default behavior. If we have default sources of credentials and optional ones, that complicates things.
A CLI flag is not great, at least not by itself. we'd definitely need another way that you can set globally. We don't have config settings, so I guess we are left with an env var. So how about we have an env var, and if that is set that's also an opt-in to try to authenticate all repos. In this case I am not sure what we should do if only some repos are authenticated. I don't think that there is a real chance of sending credentials to the wrong hosts, at least not in practice, so that's ok. I think we could give warnings for failed credential lookups when the user (or admin) has globally opted into authenticated repos. |
So in the repo auth docs, the first line does say that the username is required to be in the repo URL:
But I had still missed this and spent some time trying to figure out why the
options(repos =
method wasn't working for an authenticated repo.Other auth methods like
curl
with a.netrc
don't require you to put the username in the repo URL, so I believe other users may hit this gotcha as well if quickly skimming through the docs.So a few suggestions I can think of:
repos
option to the docs. More users will be familiar with this method, and some tools like the RStudio IDE have repo settings where you just type in a repo URL.Add hints to the error messaging if you omitted a username, and some netrc/keyring credentials happen to match the URL
Make the username requirement optional. With
curl
and.netrc
, usernames are optional, and curl appears to select the first.netrc
entry from quick testing:And it finds the right entry if the username is present:
This would just be more convenient, especially since I doubt most users are going to have multiple logins configured for the same repo anyway. And also less user configuration required when converting an unauthenticated repo to authenticated.
The text was updated successfully, but these errors were encountered: