From 465217908e5ee6450c41d902ea886849a872c101 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Thu, 29 Nov 2018 12:44:48 -0500 Subject: [PATCH 1/2] Share structs between controller and tests Changed from .contains() to .first().unwrap() == in tests because contains is weird https://github.com/rust-lang/rust/issues/42671 --- src/controllers/krate/publish.rs | 17 +++-------------- src/models/badge.rs | 8 ++++---- src/models/category.rs | 7 ++++--- src/tests/all.rs | 13 +------------ src/tests/badge.rs | 26 ++++++++++++++++---------- src/views/mod.rs | 13 +++++++++++++ 6 files changed, 41 insertions(+), 43 deletions(-) diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index a6f7648da9c..afe38c8b415 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -14,7 +14,7 @@ use util::{read_fill, read_le_u32}; use controllers::prelude::*; use models::dependency; use models::{Badge, Category, Keyword, NewCrate, NewVersion, Rights, User}; -use views::{EncodableCrate, EncodableCrateUpload}; +use views::{EncodableCrateUpload, GoodCrate, PublishWarnings}; /// Handles the `PUT /crates/new` route. /// Used by `cargo publish` to publish a new crate or to publish a new version of an @@ -196,23 +196,12 @@ pub fn publish(req: &mut dyn Request) -> CargoResult { crate_bomb.path = None; readme_bomb.path = None; - #[derive(Serialize)] - struct Warnings<'a> { - invalid_categories: Vec<&'a str>, - invalid_badges: Vec<&'a str>, - } - let warnings = Warnings { + let warnings = PublishWarnings { invalid_categories: ignored_invalid_categories, invalid_badges: ignored_invalid_badges, }; - #[derive(Serialize)] - struct R<'a> { - #[serde(rename = "crate")] - krate: EncodableCrate, - warnings: Warnings<'a>, - } - Ok(req.json(&R { + Ok(req.json(&GoodCrate { krate: krate.minimal_encodable(&max_version, None, false, None), warnings, })) diff --git a/src/models/badge.rs b/src/models/badge.rs index d622f0fc091..a19b66cdff0 100644 --- a/src/models/badge.rs +++ b/src/models/badge.rs @@ -104,11 +104,11 @@ impl Badge { serde_json::from_value(serde_json::to_value(self).unwrap()).unwrap() } - pub fn update_crate<'a>( + pub fn update_crate( conn: &PgConnection, krate: &Crate, - badges: Option<&'a HashMap>>, - ) -> QueryResult> { + badges: Option<&HashMap>>, + ) -> QueryResult> { use diesel::{delete, insert_into}; let mut invalid_badges = vec![]; @@ -126,7 +126,7 @@ impl Badge { badges::attributes.eq(attributes_json), )); } else { - invalid_badges.push(&**k); + invalid_badges.push(k.to_string()); } } } diff --git a/src/models/category.rs b/src/models/category.rs index 229d054219a..f69814c940f 100644 --- a/src/models/category.rs +++ b/src/models/category.rs @@ -101,17 +101,18 @@ impl Category { } } - pub fn update_crate<'a>( + pub fn update_crate( conn: &PgConnection, krate: &Crate, - slugs: &[&'a str], - ) -> QueryResult> { + slugs: &[&str], + ) -> QueryResult> { conn.transaction(|| { let categories = Category::by_slugs_case_sensitive(slugs).load::(conn)?; let invalid_categories = slugs .iter() .cloned() .filter(|s| !categories.iter().any(|c| c.slug == *s)) + .map(|s| s.to_string()) .collect(); let crate_categories = categories .iter() diff --git a/src/tests/all.rs b/src/tests/all.rs index 588eccdf693..48a98dc70c5 100644 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -45,7 +45,7 @@ use models::{Crate, CrateOwner, Dependency, Team, User, Version}; use models::{NewCategory, NewTeam, NewUser}; use schema::*; use views::krate_publish as u; -use views::{EncodableCrate, EncodableKeyword, EncodableOwner, EncodableVersion}; +use views::{EncodableCrate, EncodableKeyword, EncodableOwner, EncodableVersion, GoodCrate}; macro_rules! t { ($e:expr) => { @@ -93,22 +93,11 @@ mod user; mod util; mod version; -#[derive(Deserialize, Debug)] -pub struct GoodCrate { - #[serde(rename = "crate")] - krate: EncodableCrate, - warnings: Warnings, -} #[derive(Deserialize)] pub struct CrateList { crates: Vec, meta: CrateMeta, } -#[derive(Deserialize, Debug)] -struct Warnings { - invalid_categories: Vec, - invalid_badges: Vec, -} #[derive(Deserialize)] struct CrateMeta { total: i32, diff --git a/src/tests/badge.rs b/src/tests/badge.rs index 06414bfb50b..992cbe05d39 100644 --- a/src/tests/badge.rs +++ b/src/tests/badge.rs @@ -367,7 +367,7 @@ fn travis_ci_required_keys() { let invalid_badges = Badge::update_crate(&conn, &krate, Some(&badges)).unwrap(); assert_eq!(invalid_badges.len(), 1); - assert!(invalid_badges.contains(&"travis-ci")); + assert_eq!(invalid_badges.first().unwrap(), "travis-ci"); assert_eq!(krate.badges(&conn).unwrap(), vec![]); } @@ -384,7 +384,7 @@ fn gitlab_required_keys() { let invalid_badges = Badge::update_crate(&conn, &krate, Some(&badges)).unwrap(); assert_eq!(invalid_badges.len(), 1); - assert!(invalid_badges.contains(&"gitlab")); + assert_eq!(invalid_badges.first().unwrap(), "gitlab"); assert_eq!(krate.badges(&conn).unwrap(), vec![]); } @@ -406,7 +406,10 @@ fn isitmaintained_issue_resolution_required_keys() { let invalid_badges = Badge::update_crate(&conn, &krate, Some(&badges)).unwrap(); assert_eq!(invalid_badges.len(), 1); - assert!(invalid_badges.contains(&"isitmaintained_issue_resolution")); + assert_eq!( + invalid_badges.first().unwrap(), + "isitmaintained_issue_resolution" + ); assert_eq!(krate.badges(&conn).unwrap(), vec![]); } @@ -428,7 +431,10 @@ fn isitmaintained_open_issues_required_keys() { let invalid_badges = Badge::update_crate(&conn, &krate, Some(&badges)).unwrap(); assert_eq!(invalid_badges.len(), 1); - assert!(invalid_badges.contains(&"isitmaintained_open_issues")); + assert_eq!( + invalid_badges.first().unwrap(), + "isitmaintained_open_issues" + ); assert_eq!(krate.badges(&conn).unwrap(), vec![]); } @@ -445,7 +451,7 @@ fn codecov_required_keys() { let invalid_badges = Badge::update_crate(&conn, &krate, Some(&badges)).unwrap(); assert_eq!(invalid_badges.len(), 1); - assert!(invalid_badges.contains(&"codecov")); + assert_eq!(invalid_badges.first().unwrap(), "codecov"); assert_eq!(krate.badges(&conn).unwrap(), vec![]); } @@ -462,7 +468,7 @@ fn coveralls_required_keys() { let invalid_badges = Badge::update_crate(&conn, &krate, Some(&badges)).unwrap(); assert_eq!(invalid_badges.len(), 1); - assert!(invalid_badges.contains(&"coveralls")); + assert_eq!(invalid_badges.first().unwrap(), "coveralls"); assert_eq!(krate.badges(&conn).unwrap(), vec![]); } @@ -479,7 +485,7 @@ fn circle_ci_required_keys() { let invalid_badges = Badge::update_crate(&conn, &krate, Some(&badges)).unwrap(); assert_eq!(invalid_badges.len(), 1); - assert!(invalid_badges.contains(&"circle-ci")); + assert_eq!(invalid_badges.first().unwrap(), "circle-ci"); assert_eq!(krate.badges(&conn).unwrap(), vec![]); } @@ -499,7 +505,7 @@ fn maintenance_required_keys() { let invalid_badges = Badge::update_crate(&conn, &krate, Some(&badges)).unwrap(); assert_eq!(invalid_badges.len(), 1); - assert!(invalid_badges.contains(&"maintenance")); + assert_eq!(invalid_badges.first().unwrap(), "maintenance"); assert_eq!(krate.badges(&conn).unwrap(), vec![]); } @@ -521,7 +527,7 @@ fn maintenance_invalid_values() { let invalid_badges = Badge::update_crate(&conn, &krate, Some(&badges)).unwrap(); assert_eq!(invalid_badges.len(), 1); - assert!(invalid_badges.contains(&"maintenance")); + assert_eq!(invalid_badges.first().unwrap(), "maintenance"); assert_eq!(krate.badges(&conn).unwrap(), vec![]); } @@ -542,6 +548,6 @@ fn unknown_badge() { let invalid_badges = Badge::update_crate(&conn, &krate, Some(&badges)).unwrap(); assert_eq!(invalid_badges.len(), 1); - assert!(invalid_badges.contains(&"not-a-badge")); + assert_eq!(invalid_badges.first().unwrap(), "not-a-badge"); assert_eq!(krate.badges(&conn).unwrap(), vec![]); } diff --git a/src/views/mod.rs b/src/views/mod.rs index 5ce2e3da790..26e7eda580b 100644 --- a/src/views/mod.rs +++ b/src/views/mod.rs @@ -210,6 +210,19 @@ pub struct EncodableVersionLinks { pub authors: String, } +#[derive(Serialize, Deserialize, Debug)] +pub struct GoodCrate { + #[serde(rename = "crate")] + pub krate: EncodableCrate, + pub warnings: PublishWarnings, +} + +#[derive(Serialize, Deserialize, Debug)] +pub struct PublishWarnings { + pub invalid_categories: Vec, + pub invalid_badges: Vec, +} + // TODO: Prefix many of these with `Encodable` then clean up the reexports pub mod krate_publish; pub use self::krate_publish::CrateDependency as EncodableCrateDependency; From ab89c5abc524e0d9399c7a56d29b866e12c98c14 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Thu, 29 Nov 2018 13:36:17 -0500 Subject: [PATCH 2/2] Warn when a crate publisher doesn't have a verified email address See: https://github.com/rust-lang/crates-io-cargo-teams/issues/8 --- src/controllers/krate/publish.rs | 11 +++ src/models/user.rs | 10 +++ ...rate_new_krate_with_unverified_email_warns | 73 +++++++++++++++++++ ..._new_krate_with_verified_email_doesnt_warn | 73 +++++++++++++++++++ .../krate_new_krate_without_any_email_warns | 73 +++++++++++++++++++ src/tests/krate.rs | 66 ++++++++++++++++- src/views/mod.rs | 1 + 7 files changed, 306 insertions(+), 1 deletion(-) create mode 100644 src/tests/http-data/krate_new_krate_with_unverified_email_warns create mode 100644 src/tests/http-data/krate_new_krate_with_verified_email_doesnt_warn create mode 100644 src/tests/http-data/krate_new_krate_without_any_email_warns diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index afe38c8b415..3957c5df281 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -64,6 +64,16 @@ pub fn publish(req: &mut dyn Request) -> CargoResult { let categories: Vec<_> = categories.iter().map(|k| &***k).collect(); let conn = req.db_conn()?; + + let mut other_warnings = vec![]; + if !user.has_verified_email(&conn)? { + other_warnings.push(String::from( + "You do not currently have a verified email address associated with your crates.io \ + account. Starting 2019-02-28, a verified email will be required to publish crates. \ + Visit https://crates.io/me to set and verify your email address.", + )); + } + // Create a transaction on the database, if there are no errors, // commit the transactions to record a new or updated crate. conn.transaction(|| { @@ -199,6 +209,7 @@ pub fn publish(req: &mut dyn Request) -> CargoResult { let warnings = PublishWarnings { invalid_categories: ignored_invalid_categories, invalid_badges: ignored_invalid_badges, + other: other_warnings, }; Ok(req.json(&GoodCrate { diff --git a/src/models/user.rs b/src/models/user.rs index 81286431d05..32bdea2dba0 100644 --- a/src/models/user.rs +++ b/src/models/user.rs @@ -154,6 +154,16 @@ impl User { Ok(best) } + pub fn has_verified_email(&self, conn: &PgConnection) -> CargoResult { + use diesel::dsl::exists; + let email_exists = diesel::select(exists( + emails::table + .filter(emails::user_id.eq(self.id)) + .filter(emails::verified.eq(true)), + )).get_result(&*conn)?; + Ok(email_exists) + } + /// Converts this `User` model into an `EncodablePrivateUser` for JSON serialization. pub fn encodable_private( self, diff --git a/src/tests/http-data/krate_new_krate_with_unverified_email_warns b/src/tests/http-data/krate_new_krate_with_unverified_email_warns new file mode 100644 index 00000000000..f3d39f24850 --- /dev/null +++ b/src/tests/http-data/krate_new_krate_with_unverified_email_warns @@ -0,0 +1,73 @@ +[ + { + "request": { + "uri": "http://alexcrichton-test.s3.amazonaws.com/crates/foo_unverified_email/foo_unverified_email-1.0.0.crate", + "method": "PUT", + "headers": [ + [ + "accept", + "*/*" + ], + [ + "content-length", + "35" + ], + [ + "host", + "alexcrichton-test.s3.amazonaws.com" + ], + [ + "accept-encoding", + "gzip" + ], + [ + "user-agent", + "reqwest/0.9.1" + ], + [ + "content-type", + "application/x-tar" + ], + [ + "authorization", + "AWS AKIAICL5IWUZYWWKA7JA:uDc39eNdF6CcwB+q+JwKsoDLQc4=" + ], + [ + "date", + "Fri, 15 Sep 2017 07:53:06 -0700" + ] + ], + "body": "H4sIAAAAAAAA/+3AAQEAAACCIP+vbkhQwKsBLq+17wAEAAA=" + }, + "response": { + "status": 200, + "headers": [ + [ + "x-amz-request-id", + "26589A5E52F8395C" + ], + [ + "ETag", + "\"f9016ad360cebb4fe2e6e96e5949f022\"" + ], + [ + "date", + "Fri, 15 Sep 2017 14:53:07 GMT" + ], + [ + "content-length", + "0" + ], + [ + "x-amz-id-2", + "JdIvnNTw53aqXjBIqBLNuN4kxf/w1XWX+xuIiGBDYy7yzOSDuAMtBSrTW4ZWetcCIdqCUHuQ51A=" + ], + [ + "Server", + "AmazonS3" + ] + ], + "body": "" + } + } +] \ No newline at end of file diff --git a/src/tests/http-data/krate_new_krate_with_verified_email_doesnt_warn b/src/tests/http-data/krate_new_krate_with_verified_email_doesnt_warn new file mode 100644 index 00000000000..47eeb232c6e --- /dev/null +++ b/src/tests/http-data/krate_new_krate_with_verified_email_doesnt_warn @@ -0,0 +1,73 @@ +[ + { + "request": { + "uri": "http://alexcrichton-test.s3.amazonaws.com/crates/foo_verified_email/foo_verified_email-1.0.0.crate", + "method": "PUT", + "headers": [ + [ + "accept", + "*/*" + ], + [ + "content-length", + "35" + ], + [ + "host", + "alexcrichton-test.s3.amazonaws.com" + ], + [ + "accept-encoding", + "gzip" + ], + [ + "user-agent", + "reqwest/0.9.1" + ], + [ + "content-type", + "application/x-tar" + ], + [ + "authorization", + "AWS AKIAICL5IWUZYWWKA7JA:uDc39eNdF6CcwB+q+JwKsoDLQc4=" + ], + [ + "date", + "Fri, 15 Sep 2017 07:53:06 -0700" + ] + ], + "body": "H4sIAAAAAAAA/+3AAQEAAACCIP+vbkhQwKsBLq+17wAEAAA=" + }, + "response": { + "status": 200, + "headers": [ + [ + "x-amz-request-id", + "26589A5E52F8395C" + ], + [ + "ETag", + "\"f9016ad360cebb4fe2e6e96e5949f022\"" + ], + [ + "date", + "Fri, 15 Sep 2017 14:53:07 GMT" + ], + [ + "content-length", + "0" + ], + [ + "x-amz-id-2", + "JdIvnNTw53aqXjBIqBLNuN4kxf/w1XWX+xuIiGBDYy7yzOSDuAMtBSrTW4ZWetcCIdqCUHuQ51A=" + ], + [ + "Server", + "AmazonS3" + ] + ], + "body": "" + } + } +] \ No newline at end of file diff --git a/src/tests/http-data/krate_new_krate_without_any_email_warns b/src/tests/http-data/krate_new_krate_without_any_email_warns new file mode 100644 index 00000000000..320600c9030 --- /dev/null +++ b/src/tests/http-data/krate_new_krate_without_any_email_warns @@ -0,0 +1,73 @@ +[ + { + "request": { + "uri": "http://alexcrichton-test.s3.amazonaws.com/crates/foo_no_email/foo_no_email-1.0.0.crate", + "method": "PUT", + "headers": [ + [ + "accept", + "*/*" + ], + [ + "content-length", + "35" + ], + [ + "host", + "alexcrichton-test.s3.amazonaws.com" + ], + [ + "accept-encoding", + "gzip" + ], + [ + "user-agent", + "reqwest/0.9.1" + ], + [ + "content-type", + "application/x-tar" + ], + [ + "authorization", + "AWS AKIAICL5IWUZYWWKA7JA:uDc39eNdF6CcwB+q+JwKsoDLQc4=" + ], + [ + "date", + "Fri, 15 Sep 2017 07:53:06 -0700" + ] + ], + "body": "H4sIAAAAAAAA/+3AAQEAAACCIP+vbkhQwKsBLq+17wAEAAA=" + }, + "response": { + "status": 200, + "headers": [ + [ + "x-amz-request-id", + "26589A5E52F8395C" + ], + [ + "ETag", + "\"f9016ad360cebb4fe2e6e96e5949f022\"" + ], + [ + "date", + "Fri, 15 Sep 2017 14:53:07 GMT" + ], + [ + "content-length", + "0" + ], + [ + "x-amz-id-2", + "JdIvnNTw53aqXjBIqBLNuN4kxf/w1XWX+xuIiGBDYy7yzOSDuAMtBSrTW4ZWetcCIdqCUHuQ51A=" + ], + [ + "Server", + "AmazonS3" + ] + ], + "body": "" + } + } +] \ No newline at end of file diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 7e9096df7df..b76950306ac 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -22,7 +22,7 @@ use cargo_registry::models::krate::MAX_NAME_LENGTH; use builders::{CrateBuilder, DependencyBuilder, PublishBuilder, VersionBuilder}; use models::{Category, Crate}; -use schema::{api_tokens, crates, metadata, versions}; +use schema::{api_tokens, crates, emails, metadata, versions}; use views::{ EncodableCategory, EncodableCrate, EncodableDependency, EncodableKeyword, EncodableVersion, EncodableVersionDownload, @@ -1020,6 +1020,70 @@ fn new_krate_with_readme() { assert_eq!(json.krate.max_version, "1.0.0"); } +// This warning will soon become a hard error. +// See https://github.com/rust-lang/crates-io-cargo-teams/issues/8 +#[test] +fn new_krate_without_any_email_warns() { + let (_, _, _, token) = TestApp::with_proxy().with_token(); + + let crate_to_publish = PublishBuilder::new("foo_no_email"); + + let json = token.publish(crate_to_publish).good(); + assert_eq!(json.warnings.other.len(), 1); + assert_eq!(json.warnings.other[0], "You do not currently have a verified email address \ + associated with your crates.io account. Starting 2019-02-28, a verified email will be required \ + to publish crates. Visit https://crates.io/me to set and verify your email address."); +} + +// This warning will soon become a hard error. +// See https://github.com/rust-lang/crates-io-cargo-teams/issues/8 +#[test] +fn new_krate_with_unverified_email_warns() { + let (app, _, user, token) = TestApp::with_proxy().with_token(); + let user = user.as_model(); + + app.db(|conn| { + insert_into(emails::table) + .values(( + emails::user_id.eq(user.id), + emails::email.eq("something@example.com"), + )).execute(conn) + .unwrap(); + }); + + let crate_to_publish = PublishBuilder::new("foo_unverified_email"); + + let json = token.publish(crate_to_publish).good(); + assert_eq!(json.warnings.other.len(), 1); + assert_eq!(json.warnings.other[0], "You do not currently have a verified email address \ + associated with your crates.io account. Starting 2019-02-28, a verified email will be required \ + to publish crates. Visit https://crates.io/me to set and verify your email address."); +} + +#[test] +fn new_krate_with_verified_email_doesnt_warn() { + let (app, _, user, token) = TestApp::with_proxy().with_token(); + let user = user.as_model(); + + // TODO: Move this to TestApp setup for user so we don't have to do this for every test + // that publishes a crate; then edit the test for the user without a verified email to + // remove the verified email + app.db(|conn| { + insert_into(emails::table) + .values(( + emails::user_id.eq(user.id), + emails::email.eq("something@example.com"), + emails::verified.eq(true), + )).execute(conn) + .unwrap(); + }); + + let crate_to_publish = PublishBuilder::new("foo_verified_email"); + + let json = token.publish(crate_to_publish).good(); + assert_eq!(json.warnings.other.len(), 0); +} + #[test] fn summary_doesnt_die() { let (_, anon) = TestApp::init().empty(); diff --git a/src/views/mod.rs b/src/views/mod.rs index 26e7eda580b..0ee1f4e5d07 100644 --- a/src/views/mod.rs +++ b/src/views/mod.rs @@ -221,6 +221,7 @@ pub struct GoodCrate { pub struct PublishWarnings { pub invalid_categories: Vec, pub invalid_badges: Vec, + pub other: Vec, } // TODO: Prefix many of these with `Encodable` then clean up the reexports