Skip to content

Conversation

Eh2406
Copy link
Contributor

@Eh2406 Eh2406 commented Jun 18, 2022

Builds on and is blocked by #10592. This adds initial support for Asymmetric Tokens #10519.

@rust-highfive
Copy link

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 18, 2022
@Eh2406
Copy link
Contributor Author

Eh2406 commented Jun 18, 2022

Big shout out to @brycx for all the work getting pasetors ready for this PR! Literally could not have done this without your help!

@Eh2406 Eh2406 force-pushed the asymmetric_tokens branch 2 times, most recently from ae6834c to 429cfe5 Compare June 20, 2022 15:45
@Eh2406
Copy link
Contributor Author

Eh2406 commented Jun 20, 2022

@joshtriplett how aggressively do we want to tie this to registry_auth? I think we want them tied together when we stabilize it, but we could choose to be more aggressive and only have one Z flag.

@Eh2406 Eh2406 force-pushed the asymmetric_tokens branch from 429cfe5 to 7efacff Compare June 20, 2022 16:28
@joshtriplett
Copy link
Member

@Eh2406 I think we should make them the same feature, since we shouldn't ship authenticated HTTP registries with non-asymmetric tokens.

@joshtriplett joshtriplett added the T-cargo Team: Cargo label Jun 21, 2022
@joshtriplett
Copy link
Member

Capturing an idea from the Cargo team meeting: we could make the asymmetric token a private field (and make sure the enum doesn't implement Display), so that it can't be read out and unintentionally transmitted; signing can use methods.

}
}

pub struct Mutation<'a> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Drive-by comment: this would probably be better as an enum.

@bors
Copy link
Contributor

bors commented Jul 1, 2022

☔ The latest upstream changes (presumably #10802) made this pull request unmergeable. Please resolve the merge conflicts.

@Eh2406
Copy link
Contributor Author

Eh2406 commented Sep 22, 2022

I just noticed that we have a check for setting the token using the command line, this check should probably be extended to the secret key as well.

bail!("registry.token cannot be set through --config for security reasons");

@ehuss
Copy link
Contributor

ehuss commented Nov 19, 2022

With #10592 landed, is there anything needed to be ready for review other than resolving the merge conflicts?

@Eh2406
Copy link
Contributor Author

Eh2406 commented Nov 19, 2022

It needs to be rebased and updated. I remember that there was significant feedback. I'm planning to come back to this PR the end of this week.

Todos:

  • rebase and merge conflicts.
  • update to pasetors 0.6, use the new serde support.
  • Consistently refer to things as asymmetric tokens
  • consistently refer to secret keys and public keys, as that is the recommended terminology for PASETO. (Public vs. Private is easy to accidentally misspeak or type.)
  • add to registry_auth Z flag
  • try to make the asymmetric token a private field (and make sure the enum doesn't implement Display)
  • Mutation should be an enum
  • a check for setting the secret key using the command line
  • cash tokens so that the same token is not regenerated

Copy link
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.

Could you also update unstable.md to describe the feature?

@Eh2406 Eh2406 force-pushed the asymmetric_tokens branch from 862c3c4 to e832894 Compare December 9, 2022 22:53
@ehuss
Copy link
Contributor

ehuss commented Dec 29, 2022

🎉 Thanks! I appreciate that you clearly put in a lot of effort on this.

Can you collect any todo items, and create new issues for them? I think it would help to track what is left, what needs to be decided or resolved, and to organize discussion on those.

@bors r+

@bors
Copy link
Contributor

bors commented Dec 29, 2022

📌 Commit b6adac1 has been approved by ehuss

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 29, 2022
@bors
Copy link
Contributor

bors commented Dec 29, 2022

⌛ Testing commit b6adac1 with merge ef565ef...

bors added a commit that referenced this pull request Dec 29, 2022
Asymmetric tokens

Builds on and is blocked by #10592. This adds initial support for Asymmetric Tokens #10519.
@bors
Copy link
Contributor

bors commented Dec 29, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 29, 2022
@Eh2406
Copy link
Contributor Author

Eh2406 commented Dec 29, 2022

That seams spurious and unrelated.

@Eh2406
Copy link
Contributor Author

Eh2406 commented Dec 29, 2022

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 29, 2022
@bors
Copy link
Contributor

bors commented Dec 29, 2022

⌛ Testing commit b6adac1 with merge 7fb01c6...

@bors
Copy link
Contributor

bors commented Dec 29, 2022

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 7fb01c6 to master...

@bors bors merged commit 7fb01c6 into rust-lang:master Dec 29, 2022
@bors bors mentioned this pull request Dec 29, 2022
16 tasks
weihanglo added a commit to weihanglo/rust that referenced this pull request Jan 4, 2023
8 commits in 2381cbdb4e9b07090f552d34a44a529b6e620e44..8c460b2237a6359a7e3335890db8da049bdd62fc
2022-12-23 12:19:27 +0000 to 2023-01-04 14:30:01 +0000
- test: revive nightly plugin tests to work (rust-lang/cargo#11534)
- Add note to release notes about rejecting multiple registries. (rust-lang/cargo#11531)
- Fix a typo `fresheness` -&gt; `freshness` (rust-lang/cargo#11529)
- Reasons for rebuilding (rust-lang/cargo#11407)
- Asymmetric tokens (rust-lang/cargo#10771)
- Use proper git URL for GitHub repos (rust-lang/cargo#11517)
- Add `registry.default` example (rust-lang/cargo#11516)
- Support vendoring with different revs from same git repo (rust-lang/cargo#10690)

Also update license exceptions and permitted dependencies
for new cargo dependency "pasetors".

A new dependency `getrandom` is added into `rustc-workspace-hacks`,
since it requires feature `js`.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 4, 2023
Update cargo

8 commits in 2381cbdb4e9b07090f552d34a44a529b6e620e44..8c460b2237a6359a7e3335890db8da049bdd62fc
2022-12-23 12:19:27 +0000 to 2023-01-04 14:30:01 +0000
- test: revive nightly plugin tests to work (rust-lang/cargo#11534)
- Add note to release notes about rejecting multiple registries. (rust-lang/cargo#11531)
- Fix a typo `fresheness` -&gt; `freshness` (rust-lang/cargo#11529)
- Reasons for rebuilding (rust-lang/cargo#11407)
- Asymmetric tokens (rust-lang/cargo#10771)
- Use proper git URL for GitHub repos (rust-lang/cargo#11517)
- Add `registry.default` example (rust-lang/cargo#11516)
- Support vendoring with different revs from same git repo (rust-lang/cargo#10690)

Also update license exceptions and permitted dependencies
for new cargo dependency "pasetors".

A new dependency `getrandom` is added into `rustc-workspace-hacks`,
since it requires feature `js`.

r? `@ghost`
@ehuss ehuss added this to the 1.68.0 milestone Jan 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-cargo Team: Cargo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants