Skip to content

fix(auth): add auth scheme hint to token rejected error for alt registries#16794

Merged
arlosi merged 2 commits intorust-lang:masterfrom
enricobolzonello:auth-scheme-hint
Apr 7, 2026
Merged

fix(auth): add auth scheme hint to token rejected error for alt registries#16794
arlosi merged 2 commits intorust-lang:masterfrom
enricobolzonello:auth-scheme-hint

Conversation

@enricobolzonello
Copy link
Copy Markdown
Contributor

What does this PR try to resolve?

Based on the POC PR #15985, expand token rejected error message with authorization scheme.

Addresses issue #15021.

How to test and review this PR?

Run the tests in tests/testsuite/registry_auth.rs:

  • incorrect_token_unrecognized_scheme
  • incorrect_token_bearer_scheme
  • incorrect_token

@rustbot rustbot added A-registry-authentication Area: registry authentication and authorization (authn authz) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 26, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Mar 26, 2026

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @ehuss, @epage, @weihanglo
  • @ehuss, @epage, @weihanglo expanded to ehuss, epage, weihanglo
  • Random selection from ehuss, epage, weihanglo

Copy link
Copy Markdown
Contributor

@arlosi arlosi left a comment

Choose a reason for hiding this comment

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

@enricobolzonello thanks for your work here. I think we're really close to merging this.

Just a minor tweak to the wording. I don't want users to think that schemes other than Basic and Bearer are unsupported by Cargo.

The other direction we could go is to not detect the scheme at all, and just look for whether the token has a space in it. Then we could change the message to say "note: the token does not include an authentication scheme" if there is no space.

What do you think?

Also, could you please rebase this to two commits:

  • First commit adds the tests (which should pass), showing existing behavior
  • Second commit adds the change, and updates the tests expected output so they still pass

View changes since this review

@rustbot rustbot added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 7, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 7, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@arlosi arlosi assigned arlosi and unassigned weihanglo Apr 7, 2026
@enricobolzonello
Copy link
Copy Markdown
Contributor Author

The other direction we could go is to not detect the scheme at all, and just look for whether the token has a space in it. Then we could change the message to say "note: the token does not include an authentication scheme" if there is no space.

What do you think?

I think this is the best approach. Looking closer in the codebase Cargo itself doesn't reject tokens with other schemes so it doesn't make sense to enforce Basic or Bearer.

@arlosi
Copy link
Copy Markdown
Contributor

arlosi commented Apr 7, 2026

Looks good! The tests failed because #16745 changed the registry auth error messages. You'll need to rebase and update test output.

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 7, 2026

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Copy link
Copy Markdown
Contributor

@arlosi arlosi left a comment

Choose a reason for hiding this comment

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

@arlosi arlosi added this pull request to the merge queue Apr 7, 2026
Merged via the queue into rust-lang:master with commit 4fc40e1 Apr 7, 2026
29 checks passed
rust-bors bot pushed a commit to rust-lang/rust that referenced this pull request Apr 8, 2026
Cargo submodule update

11 commits in a357df4c26fc14514e66aae2a269456b5545c7db..101549dddbd2b08e806f50154e3aa4cb3374cc21
2026-04-03 16:47:15 +0000 to 2026-04-08 12:51:20 +0000
- Never include use extra-filename in build scripts (rust-lang/cargo#16855)
- fix(toml): Force script edition warnings on quiet  (rust-lang/cargo#16848)
- GitHub fast path uses `http_async` (rust-lang/cargo#16847)
- feat(manifest): allow git dependency alongside alternate registry (rust-lang/cargo#16810)
- fix(auth): add auth scheme hint to token rejected error for alt registries (rust-lang/cargo#16794)
- Warn on invalid jobserver file descriptors (rust-lang/cargo#16843)
- docs(unstable): List the minimum required MSRV for 'public' field (rust-lang/cargo#16841)
- feat(lints): Emit unused_dependencies lint (rust-lang/cargo#16600)
- fix(tree): clarify error message when `-i` is used without a package name (rust-lang/cargo#16818)
- fix: Typo in target.<cfg>.linker (rust-lang/cargo#16839)
- Send Content-Type header with cargo publish requests (rust-lang/cargo#16832)

r? ghost
@rustbot rustbot added this to the 1.96.0 milestone Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-registry-authentication Area: registry authentication and authorization (authn authz) S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants