Add uv auth commands (login, logout, and token)#15539
Conversation
zanieb
temporarily deployed
to
uv-test-registries
GitHub Actions
Inactive
zanieb
temporarily deployed
to
uv-test-registries
GitHub Actions
Inactive
zanieb
temporarily deployed
to
uv-test-registries
GitHub Actions
Inactive
zanieb
changed the title
uv auth commands (login, logout, and show)uv auth commands (login, logout, and token)
| KeyringProviderType::Native | ||
| ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Do we need to enable users to specify a keyring provider type on a per-URL basis? Like, would it ever be the case that a user wants to use (e.g.) the AWS keyring plugin via subprocess, but then store credentials for some URLs via the native plugin? (This is, in a vague way, coming from me being confused that --keyring-provider is respected here at all, since it only ever works for native.)
There was a problem hiding this comment.
(This is, in a vague way, coming from me being confused that --keyring-provider is respected here at all, since it only ever works for native.)
I think we actually can support the subprocess keyring here — I just didn't want to add support for that yet.
Do we need to enable users to specify a keyring provider type on a per-URL basis? Like, would it ever be the case that a user wants to use (e.g.) the AWS keyring plugin via subprocess, but then store credentials for some URLs via the native plugin?
This is probably a realistic use-case, yeah. I think we'd need to add a concept like keyring-provider-service which maps a service URL to a keyring provider.
| // FIXME: It would be preferable to accept the value of --password or --token | ||
| // from stdin, perhaps checking here for `-` as an indicator to read stdin. We | ||
| // could then warn if the password is provided as a plaintext argument. | ||
| let password = if let Some(password) = password { |
There was a problem hiding this comment.
I am also wondering if we should not accept credentials over plaintext on the CLI, hmm.
There was a problem hiding this comment.
As in, not accept it via a CLI flag and require stdin?
There was a problem hiding this comment.
I don't think it's quite right to outright ban this, e.g., Git allows credentials interactively and we already allow passwords in plaintext in URLs.
| } | ||
| }; | ||
|
|
||
| let credentials = Credentials::basic(Some(username), Some(password)); |
There was a problem hiding this comment.
Again a vague thing where I'm wondering if we should be using Credentials::Bearer for tokens, rather than basic.
There was a problem hiding this comment.
Ah that's complicated. Our current semantics for tokens is that it's a shortcut for __token__:<token> because that's a common pattern for allowing tokens over HTTP basic authentication. This matches, e.g., the publish API we have. I think we want to allow Bearer authentication too, but I think it'd be another dimension of options?
| .as_ref() | ||
| .map(|username| format!("{username}@{url}")) | ||
| .unwrap_or_else(|| url.to_string()); | ||
| let username = username.unwrap_or_else(|| String::from("__token__")); |
There was a problem hiding this comment.
If you wanted to get fancy, you could make this Cow... Like username.map(Cow::Owned).unwrap_or(Cow::Borrowed("__token__")).
(You def don't need to do this, just thought you mind find it interesting.)
There was a problem hiding this comment.
👍 I thought about using Cow but figured it wasn't worth the time right now.
zanieb
temporarily deployed
to
uv-test-registries
GitHub Actions
Inactive
Picks up the work from - #14559 - #14896 There are some high-level changes from those pull requests 1. We do not stash seen credentials in the keyring automatically 2. We use `auth login` and `auth logout` (for future consistency) 3. We add a `token` command for showing the credential that will be used As well as many smaller changes to API, messaging, testing, etc. --------- Co-authored-by: John Mumm <jtfmumm@gmail.com>
Picks up the work from - #14559 - #14896 There are some high-level changes from those pull requests 1. We do not stash seen credentials in the keyring automatically 2. We use `auth login` and `auth logout` (for future consistency) 3. We add a `token` command for showing the credential that will be used As well as many smaller changes to API, messaging, testing, etc. --------- Co-authored-by: John Mumm <jtfmumm@gmail.com>
Picks up the work from - #14559 - #14896 There are some high-level changes from those pull requests 1. We do not stash seen credentials in the keyring automatically 2. We use `auth login` and `auth logout` (for future consistency) 3. We add a `token` command for showing the credential that will be used As well as many smaller changes to API, messaging, testing, etc. --------- Co-authored-by: John Mumm <jtfmumm@gmail.com>
Picks up the work from - #14559 - #14896 There are some high-level changes from those pull requests 1. We do not stash seen credentials in the keyring automatically 2. We use `auth login` and `auth logout` (for future consistency) 3. We add a `token` command for showing the credential that will be used As well as many smaller changes to API, messaging, testing, etc. --------- Co-authored-by: John Mumm <jtfmumm@gmail.com>
3 participants
Picks up the work from
--keyring-provider native#14559There are some high-level changes from those pull requests
auth loginandauth logout(for future consistency)tokencommand for showing the credential that will be usedAs well as many smaller changes to API, messaging, testing, etc.