Skip to content

Add cli for setting and unsetting credentials to system keyring#14896

Closed
jtfmumm wants to merge 15 commits intojtfm/keyring-explorationfrom
jtfm/credentials-cli
Closed

Add cli for setting and unsetting credentials to system keyring#14896
jtfmumm wants to merge 15 commits intojtfm/keyring-explorationfrom
jtfm/credentials-cli

Conversation

@jtfmumm
Copy link
Contributor

@jtfmumm jtfmumm commented Jul 25, 2025 •
edited
Loading

This is a work in progress that depends on #14559. There are still open design questions.

There are a couple of FIXMEs in this draft PR. Some are for more doc details, but a few are about adding support for accepting passwords over stdin and providing a security warning if the user passes a plaintext password to a --password option.

Also left to do:

  • Automatically remove a trailing /simple from a service before persisting credentials.
  • Possibly put automatic storage of credentials behind a --preview flag, unless we think it's enough that you must explicitly configure a native keyring provider (we've discussed making it the default, but that's not the behavior here)
  • Determine how best to automate testing
  • Add documentation

@jtfmumm jtfmumm added enhancement New feature or improvement to existing functionality do-not-merge Pull request is not ready to merge labels Jul 25, 2025
@jtfmumm jtfmumm force-pushed the jtfm/credentials-cli branch from 9011c3b to 5f28e9e Compare July 25, 2025 14:10
jtfmumm
jtfmumm commented Jul 25, 2025
View reviewed changes
crates/uv-auth/src/keyring.rs
url: &DisplaySafeUrl,
username: &str,
) -> Result<(), uv_keyring::Error> {
if let KeyringProviderBackend::Native = &self.backend {
Copy link
Contributor Author

@jtfmumm jtfmumm Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could warn here if this is not set.

jtfmumm
jtfmumm commented Jul 25, 2025
View reviewed changes
crates/uv-cli/src/lib.rs

#[derive(Args)]
pub struct AuthUnsetArgs {
/// The authentication service to configure
Copy link
Contributor Author

@jtfmumm jtfmumm Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be fleshed out more once we've decided how we're presenting the service

jtfmumm
jtfmumm commented Jul 25, 2025
View reviewed changes
crates/uv-cli/src/lib.rs

#[derive(Args)]
pub struct AuthSetArgs {
/// The authentication service to configure
Copy link
Contributor Author

@jtfmumm jtfmumm Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be fleshed out more once we've decided how we're presenting the service

jtfmumm
jtfmumm commented Jul 25, 2025
View reviewed changes
crates/uv/src/commands/auth/mod.rs
@@ -0,0 +1 @@
pub(crate) mod configure;
Copy link
Contributor Author

@jtfmumm jtfmumm Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I thought this would need to be split out, but I'll probably update this to use just commands/auth.rs

@jtfmumm jtfmumm temporarily deployed to uv-test-registries July 25, 2025 14:14 — with GitHub Actions Inactive
@jtfmumm jtfmumm force-pushed the jtfm/credentials-cli branch from 5f28e9e to 3be17ea Compare July 25, 2025 14:38
@jtfmumm jtfmumm temporarily deployed to uv-test-registries July 25, 2025 14:40 — with GitHub Actions Inactive
@jtfmumm jtfmumm force-pushed the jtfm/credentials-cli branch from 3be17ea to 5280823 Compare July 25, 2025 15:05
@jtfmumm jtfmumm temporarily deployed to uv-test-registries July 25, 2025 15:07 — with GitHub Actions Inactive
@jtfmumm jtfmumm force-pushed the jtfm/keyring-exploration branch 2 times, most recently from f8756ea to 166e9e1 Compare August 6, 2025 13:22
@jtfmumm jtfmumm force-pushed the jtfm/keyring-exploration branch from 166e9e1 to 496c7a2 Compare August 7, 2025 14:27
@jtfmumm jtfmumm force-pushed the jtfm/keyring-exploration branch from d1ab06b to a2df725 Compare August 8, 2025 09:17
@jtfmumm jtfmumm force-pushed the jtfm/credentials-cli branch from 5280823 to 63be74d Compare August 11, 2025 10:34
@jtfmumm jtfmumm temporarily deployed to uv-test-registries August 11, 2025 10:38 — with GitHub Actions Inactive
@jtfmumm jtfmumm force-pushed the jtfm/keyring-exploration branch from a2df725 to a898fe4 Compare August 14, 2025 10:31
@jtfmumm jtfmumm force-pushed the jtfm/keyring-exploration branch 2 times, most recently from 80154a2 to ec49f65 Compare August 22, 2025 15:45
@zanieb zanieb mentioned this pull request Aug 27, 2025
Merged
zanieb added a commit that referenced this pull request Aug 28, 2025
@zanieb @jtfmumm
Add uv auth commands (login, logout, and token) (#15539)
41893a6 
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>
zanieb added a commit that referenced this pull request Aug 29, 2025
@zanieb @jtfmumm
Add uv auth commands (login, logout, and token) (#15539)
bbe9a2c 
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>
zanieb added a commit that referenced this pull request Aug 30, 2025
@zanieb @jtfmumm
Add uv auth commands (login, logout, and token) (#15539)
f272bbe 
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>
zanieb added a commit that referenced this pull request Aug 31, 2025
@zanieb @jtfmumm
Add uv auth commands (login, logout, and token) (#15539)
26eace7 
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>
@jtfmumm
Copy link
Contributor Author

jtfmumm commented Sep 2, 2025

Closed in favor of #15539

@jtfmumm jtfmumm closed this Sep 2, 2025
zanieb added a commit that referenced this pull request Sep 2, 2025
@zanieb @jtfmumm
Add uv auth commands (login, logout, and token) (#15539)
460ea6e 
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

do-not-merge Pull request is not ready to merge enhancement New feature or improvement to existing functionality

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jtfmumm