Skip to content
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

Implement RFC 3139: alternative registry authentication support #10592

Merged
merged 1 commit into from
Nov 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@
- Added `-Zcheck-cfg=output` to support build-scripts declaring their
supported set of `cfg` values with `cargo:rustc-check-cfg`.
[#10539](https://github.com/rust-lang/cargo/pull/10539)
- `-Z http-registry` now uses https://index.crates.io/ when accessing crates-io.
- `-Z sparse-registry` now uses https://index.crates.io/ when accessing crates-io.
[#10725](https://github.com/rust-lang/cargo/pull/10725)
- Fixed formatting of `.workspace` key in `cargo add` for workspace inheritance.
[#10705](https://github.com/rust-lang/cargo/pull/10705)
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ git2-curl = "0.16.0"
glob = "0.3.0"
hex = "0.4"
home = "0.5"
http-auth = { version = "0.1.6", default-features = false }
humantime = "2.0.0"
indexmap = "1"
ignore = "0.4.7"
Expand Down
60 changes: 49 additions & 11 deletions crates/cargo-test-support/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use std::fs::{self, File};
use std::io::{BufRead, BufReader, Read, Write};
use std::net::{SocketAddr, TcpListener, TcpStream};
use std::path::PathBuf;
use std::thread;
use std::thread::{self, JoinHandle};
use tar::{Builder, Header};
use url::Url;

Expand Down Expand Up @@ -61,6 +61,8 @@ pub struct RegistryBuilder {
alternative: Option<String>,
/// If set, the authorization token for the registry.
token: Option<String>,
/// If set, the registry requires authorization for all operations.
auth_required: bool,
/// If set, serves the index over http.
http_index: bool,
/// If set, serves the API over http.
Expand All @@ -76,7 +78,7 @@ pub struct RegistryBuilder {
}

pub struct TestRegistry {
_server: Option<HttpServerHandle>,
server: Option<HttpServerHandle>,
index_url: Url,
path: PathBuf,
api_url: Url,
Expand All @@ -98,6 +100,17 @@ impl TestRegistry {
.as_deref()
.expect("registry was not configured with a token")
}

/// Shutdown the server thread and wait for it to stop.
/// `Drop` automatically stops the server, but this additionally
/// waits for the thread to stop.
pub fn join(self) {
if let Some(mut server) = self.server {
server.stop();
let handle = server.handle.take().unwrap();
handle.join().unwrap();
}
}
}

impl RegistryBuilder {
Expand All @@ -106,6 +119,7 @@ impl RegistryBuilder {
RegistryBuilder {
alternative: None,
token: None,
auth_required: false,
http_api: false,
http_index: false,
api: true,
Expand Down Expand Up @@ -160,6 +174,14 @@ impl RegistryBuilder {
self
}

/// Sets this registry to require the authentication token for
/// all operations.
#[must_use]
pub fn auth_required(mut self) -> Self {
self.auth_required = true;
self
}

/// Operate the index over http
#[must_use]
pub fn http_index(mut self) -> Self {
Expand Down Expand Up @@ -207,6 +229,7 @@ impl RegistryBuilder {
registry_path.clone(),
dl_path,
token.clone(),
self.auth_required,
self.custom_responders,
);
let index_url = if self.http_index {
Expand All @@ -226,7 +249,7 @@ impl RegistryBuilder {
let registry = TestRegistry {
api_url,
index_url,
_server: server,
server,
dl_url,
path: registry_path,
token,
Expand Down Expand Up @@ -293,6 +316,11 @@ impl RegistryBuilder {
}
}

let auth = if self.auth_required {
r#","auth-required":true"#
} else {
""
};
let api = if self.api {
format!(r#","api":"{}""#, registry.api_url)
} else {
Expand All @@ -302,7 +330,7 @@ impl RegistryBuilder {
repo(&registry.path)
.file(
"config.json",
&format!(r#"{{"dl":"{}"{api}}}"#, registry.dl_url),
&format!(r#"{{"dl":"{}"{api}{auth}}}"#, registry.dl_url),
)
.build();
fs::create_dir_all(api_path.join("api/v1/crates")).unwrap();
Expand Down Expand Up @@ -442,6 +470,7 @@ pub fn alt_init() -> TestRegistry {

pub struct HttpServerHandle {
addr: SocketAddr,
handle: Option<JoinHandle<()>>,
}

impl HttpServerHandle {
Expand All @@ -456,10 +485,8 @@ impl HttpServerHandle {
pub fn dl_url(&self) -> Url {
Url::parse(&format!("http://{}/dl", self.addr.to_string())).unwrap()
}
}

impl Drop for HttpServerHandle {
fn drop(&mut self) {
fn stop(&self) {
if let Ok(mut stream) = TcpStream::connect(self.addr) {
// shutdown the server
let _ = stream.write_all(b"stop");
Expand All @@ -468,6 +495,12 @@ impl Drop for HttpServerHandle {
}
}

impl Drop for HttpServerHandle {
fn drop(&mut self) {
self.stop();
}
}

/// Request to the test http server
#[derive(Clone)]
pub struct Request {
Expand Down Expand Up @@ -504,6 +537,7 @@ pub struct HttpServer {
registry_path: PathBuf,
dl_path: PathBuf,
token: Option<String>,
auth_required: bool,
custom_responders: HashMap<&'static str, Box<dyn Send + Fn(&Request, &HttpServer) -> Response>>,
}

Expand All @@ -512,6 +546,7 @@ impl HttpServer {
registry_path: PathBuf,
dl_path: PathBuf,
token: Option<String>,
auth_required: bool,
api_responders: HashMap<
&'static str,
Box<dyn Send + Fn(&Request, &HttpServer) -> Response>,
Expand All @@ -524,10 +559,11 @@ impl HttpServer {
registry_path,
dl_path,
token,
auth_required,
custom_responders: api_responders,
};
thread::spawn(move || server.start());
HttpServerHandle { addr }
let handle = Some(thread::spawn(move || server.start()));
HttpServerHandle { addr, handle }
}

fn start(&self) {
Expand Down Expand Up @@ -615,7 +651,7 @@ impl HttpServer {
/// Route the request
fn route(&self, req: &Request) -> Response {
let authorized = |mutatation: bool| {
if mutatation {
if mutatation || self.auth_required {
self.token == req.authorization
} else {
assert!(req.authorization.is_none(), "unexpected token");
Expand Down Expand Up @@ -676,7 +712,9 @@ impl HttpServer {
pub fn unauthorized(&self, _req: &Request) -> Response {
Response {
code: 401,
headers: vec![],
headers: vec![
r#"WWW-Authenticate: Cargo login_url="https://test-registry-login/me""#.to_string(),
],
body: b"Unauthorized message from server.".to_vec(),
}
}
Expand Down
12 changes: 10 additions & 2 deletions crates/crates-io/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ pub struct Registry {
token: Option<String>,
/// Curl handle for issuing requests.
handle: Easy,
/// Whether to include the authorization token with all requests.
auth_required: bool,
}

#[derive(PartialEq, Clone, Copy)]
Expand Down Expand Up @@ -199,11 +201,17 @@ impl Registry {
/// handle.useragent("my_crawler (example.com/info)");
/// let mut reg = Registry::new_handle(String::from("https://crates.io"), None, handle);
/// ```
pub fn new_handle(host: String, token: Option<String>, handle: Easy) -> Registry {
pub fn new_handle(
host: String,
token: Option<String>,
handle: Easy,
auth_required: bool,
) -> Registry {
Registry {
host,
token,
handle,
auth_required,
}
}

Expand Down Expand Up @@ -377,7 +385,7 @@ impl Registry {
headers.append("Accept: application/json")?;
headers.append("Content-Type: application/json")?;

if authorized == Auth::Authorized {
if self.auth_required || authorized == Auth::Authorized {
let token = match self.token.as_ref() {
Some(s) => s,
None => bail!("no upload token found, please run `cargo login`"),
Expand Down
4 changes: 2 additions & 2 deletions crates/credential/cargo-credential-1password/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
[package]
name = "cargo-credential-1password"
version = "0.1.0"
version = "0.2.0"
edition = "2021"
license = "MIT OR Apache-2.0"
repository = "https://github.com/rust-lang/cargo"
description = "A Cargo credential process that stores tokens in a 1password vault."

[dependencies]
cargo-credential = { version = "0.1.0", path = "../cargo-credential" }
cargo-credential = { version = "0.2.0", path = "../cargo-credential" }
serde = { version = "1.0.117", features = ["derive"] }
serde_json = "1.0.59"
54 changes: 30 additions & 24 deletions crates/credential/cargo-credential-1password/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ struct ListItem {

#[derive(Deserialize)]
struct Overview {
title: String,
url: String,
}

impl OnePasswordKeychain {
Expand Down Expand Up @@ -175,11 +175,7 @@ impl OnePasswordKeychain {
Ok(buffer)
}

fn search(
&self,
session: &Option<String>,
registry_name: &str,
) -> Result<Option<String>, Error> {
fn search(&self, session: &Option<String>, index_url: &str) -> Result<Option<String>, Error> {
let cmd = self.make_cmd(
session,
&[
Expand All @@ -196,15 +192,15 @@ impl OnePasswordKeychain {
.map_err(|e| format!("failed to deserialize JSON from 1password list: {}", e))?;
let mut matches = items
.into_iter()
.filter(|item| item.overview.title == registry_name);
.filter(|item| item.overview.url == index_url);
match matches.next() {
Some(login) => {
// Should this maybe just sort on `updatedAt` and return the newest one?
if matches.next().is_some() {
return Err(format!(
"too many 1password logins match registry name {}, \
"too many 1password logins match registry `{}`, \
consider deleting the excess entries",
registry_name
index_url
)
.into());
}
Expand All @@ -214,7 +210,13 @@ impl OnePasswordKeychain {
}
}

fn modify(&self, session: &Option<String>, uuid: &str, token: &str) -> Result<(), Error> {
fn modify(
&self,
session: &Option<String>,
uuid: &str,
token: &str,
_name: Option<&str>,
) -> Result<(), Error> {
let cmd = self.make_cmd(
session,
&["edit", "item", uuid, &format!("password={}", token)],
Expand All @@ -226,20 +228,24 @@ impl OnePasswordKeychain {
fn create(
&self,
session: &Option<String>,
registry_name: &str,
api_url: &str,
index_url: &str,
token: &str,
name: Option<&str>,
) -> Result<(), Error> {
let title = match name {
Some(name) => format!("Cargo registry token for {}", name),
None => "Cargo registry token".to_string(),
};
let cmd = self.make_cmd(
session,
&[
"create",
"item",
"Login",
&format!("password={}", token),
&format!("url={}", api_url),
&format!("url={}", index_url),
"--title",
registry_name,
&title,
"--tags",
CARGO_TAG,
],
Expand Down Expand Up @@ -276,36 +282,36 @@ impl Credential for OnePasswordKeychain {
env!("CARGO_PKG_NAME")
}

fn get(&self, registry_name: &str, _api_url: &str) -> Result<String, Error> {
fn get(&self, index_url: &str) -> Result<String, Error> {
let session = self.signin()?;
if let Some(uuid) = self.search(&session, registry_name)? {
if let Some(uuid) = self.search(&session, index_url)? {
self.get_token(&session, &uuid)
} else {
return Err(format!(
"no 1password entry found for registry `{}`, try `cargo login` to add a token",
registry_name
index_url
)
.into());
}
}

fn store(&self, registry_name: &str, api_url: &str, token: &str) -> Result<(), Error> {
fn store(&self, index_url: &str, token: &str, name: Option<&str>) -> Result<(), Error> {
let session = self.signin()?;
// Check if an item already exists.
if let Some(uuid) = self.search(&session, registry_name)? {
self.modify(&session, &uuid, token)
if let Some(uuid) = self.search(&session, index_url)? {
self.modify(&session, &uuid, token, name)
} else {
self.create(&session, registry_name, api_url, token)
self.create(&session, index_url, token, name)
}
}

fn erase(&self, registry_name: &str, _api_url: &str) -> Result<(), Error> {
fn erase(&self, index_url: &str) -> Result<(), Error> {
let session = self.signin()?;
// Check if an item already exists.
if let Some(uuid) = self.search(&session, registry_name)? {
if let Some(uuid) = self.search(&session, index_url)? {
self.delete(&session, &uuid)?;
} else {
eprintln!("not currently logged in to `{}`", registry_name);
eprintln!("not currently logged in to `{}`", index_url);
}
Ok(())
}
Expand Down
Loading