diff --git a/src/tests/all.rs b/src/tests/all.rs index 24e5344f151..5fde3436a83 100644 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -61,6 +61,9 @@ macro_rules! bad_resp { }}; } +mod helpers; +mod prelude; + mod badge; mod builders; mod categories; diff --git a/src/tests/builders.rs b/src/tests/builders.rs index 31a51878933..9c7cdcd5cbb 100644 --- a/src/tests/builders.rs +++ b/src/tests/builders.rs @@ -229,7 +229,7 @@ impl<'a> CrateBuilder<'a> { self } - fn build(mut self, connection: &PgConnection) -> CargoResult { + pub fn build(mut self, connection: &PgConnection) -> CargoResult { use diesel::{insert_into, select, update}; let mut krate = self diff --git a/src/tests/helpers.rs b/src/tests/helpers.rs new file mode 100644 index 00000000000..3efa8292802 --- /dev/null +++ b/src/tests/helpers.rs @@ -0,0 +1,3 @@ +pub mod app; +pub mod request; +pub mod response; diff --git a/src/tests/helpers/app.rs b/src/tests/helpers/app.rs new file mode 100644 index 00000000000..b32b448a88c --- /dev/null +++ b/src/tests/helpers/app.rs @@ -0,0 +1,94 @@ +use super::request::RequestBuilder; +use cargo_registry::models::{ApiToken, Email, NewUser, User}; +use cargo_registry::util::CargoResult; +use conduit::Method; +use conduit_middleware::MiddlewareBuilder; +use diesel::prelude::*; +use std::sync::Arc; + +pub struct App { + app: Arc, + middleware: MiddlewareBuilder, + _bomb: crate::record::Bomb, +} + +impl App { + pub fn new() -> Self { + let (bomb, app, middleware) = crate::app(); + Self { + app, + middleware, + _bomb: bomb, + } + } + + /// Obtain the database connection and pass it to the closure + /// + /// Our tests share a database connection with the app server, so it's + /// important that the conenction is dropped before requests are made to + /// ensure it's available for the server to use. The connection will be + /// returned to the server after the given function returns. + pub fn db(&self, f: F) -> CargoResult + where + F: FnOnce(&PgConnection) -> CargoResult, + { + let conn = self.app.diesel_database.get()?; + f(&conn) + } + + /// Create a new user in the database with the given id + pub fn create_user(&self, username: &str) -> CargoResult { + use cargo_registry::schema::emails; + + self.db(|conn| { + let new_user = NewUser { + email: Some("something@example.com"), + ..crate::new_user(username) + }; + let user = new_user.create_or_update(conn)?; + diesel::update(Email::belonging_to(&user)) + .set(emails::verified.eq(true)) + .execute(conn)?; + Ok(user) + }) + } + + /// Sets the database in read only mode. + /// + /// Any attempts to modify the database after calling this function will + /// result in an error. + pub fn set_read_only(&self) -> CargoResult<()> { + self.db(|conn| { + diesel::sql_query("SET TRANSACTION READ ONLY").execute(conn)?; + Ok(()) + }) + } + + /// Create an HTTP `GET` request + pub fn get(&self, path: &str) -> RequestBuilder<'_> { + RequestBuilder::new(&self.middleware, Method::Get, path) + } + + /// Create an HTTP `PUT` request + pub fn put(&self, path: &str) -> RequestBuilder<'_> { + RequestBuilder::new(&self.middleware, Method::Put, path) + } + + /// Create an HTTP `DELETE` request + pub fn delete(&self, path: &str) -> RequestBuilder<'_> { + RequestBuilder::new(&self.middleware, Method::Delete, path) + } + + /// Returns the first API token for the given user, or creates a new one + pub fn token_for(&self, user: &User) -> CargoResult { + self.db(|conn| { + ApiToken::belonging_to(user) + .first(conn) + .optional()? + .map(Ok) + .unwrap_or_else(|| { + ApiToken::insert(conn, user.id, "test_token").map_err(Into::into) + }) + }) + } +} diff --git a/src/tests/helpers/request.rs b/src/tests/helpers/request.rs new file mode 100644 index 00000000000..ad49f60ab19 --- /dev/null +++ b/src/tests/helpers/request.rs @@ -0,0 +1,63 @@ +use super::response::{Response, ResponseError}; +use cargo_registry::middleware::current_user::AuthenticationSource; +use cargo_registry::models::{ApiToken, User}; +use conduit::{Handler, Method, Request}; +use conduit_middleware::MiddlewareBuilder; +use conduit_test::MockRequest; + +pub struct RequestBuilder<'a> { + middleware: &'a MiddlewareBuilder, + request: MockRequest, +} + +impl<'a> RequestBuilder<'a> { + pub(super) fn new(middleware: &'a MiddlewareBuilder, method: Method, path: &str) -> Self { + Self { + middleware, + request: MockRequest::new(method, path), + } + .with_header("User-Agent", "conduit-test") + } + + /// Sends the request signed in as the given user + pub fn signed_in_as(mut self, user: &User) -> Self { + self.clear_auth(); + self.request.mut_extensions().insert(user.clone()); + self.request + .mut_extensions() + .insert(AuthenticationSource::SessionCookie); + self + } + + /// Uses the given token for authentication + pub fn with_token(mut self, token: &ApiToken) -> Self { + self.clear_auth(); + self.with_header("Authorization", &token.token) + } + + pub fn with_header(mut self, name: &str, value: &str) -> Self { + self.request.header(name, value); + self + } + + pub fn with_body>>(mut self, body: T) -> Self { + self.request.with_body(&body.into()); + self + } + + /// Send the request + /// + /// Returns an error if any of the middlewares returned an error, or if + /// the response status was >= 400. + pub fn send(mut self) -> Result { + Response::new(self.middleware.call(&mut self.request)?) + } + + fn clear_auth(&mut self) { + self.request.mut_extensions().remove::(); + self.request + .mut_extensions() + .remove::(); + self.request.header("Authorization", ""); + } +} diff --git a/src/tests/helpers/response.rs b/src/tests/helpers/response.rs new file mode 100644 index 00000000000..a65a02e5fbb --- /dev/null +++ b/src/tests/helpers/response.rs @@ -0,0 +1,93 @@ +use cargo_registry::util::{CargoError, CargoResult}; +use std::error::Error; +use std::fmt; + +pub struct Response { + inner: conduit::Response, + body: String, +} + +impl Response { + pub(super) fn new(mut inner: conduit::Response) -> Result { + use ResponseError::*; + + let mut body = Vec::new(); + inner.body.write_body(&mut body).unwrap(); + + let resp = Response { + inner, + body: String::from_utf8(body).unwrap(), + }; + + match resp.status() { + 400...499 => Err(BadRequest(resp)), + 500...599 => Err(ServerError(resp)), + _ => Ok(resp), + } + } + + pub fn status(&self) -> u32 { + self.inner.status.0 + } + + pub fn text(&self) -> &str { + &self.body + } + + pub fn json<'a, T>(&'a self) -> CargoResult + where + T: serde::Deserialize<'a>, + { + serde_json::from_str(self.text()).map_err(Into::into) + } +} + +pub enum ResponseError { + MiddlewareError(Box), + BadRequest(Response), + ServerError(Response), +} + +impl From> for ResponseError { + fn from(e: Box) -> Self { + ResponseError::MiddlewareError(CargoError::from_std_error(e)) + } +} + +impl fmt::Debug for ResponseError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use ResponseError::*; + match self { + MiddlewareError(e) => write!(f, "MiddlewareError({:?})", e), + BadRequest(e) => write!(f, "BadRequest({:?})", e.text()), + ServerError(e) => write!(f, "ServerError({:?})", e.text()), + } + } +} + +impl fmt::Display for ResponseError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use ResponseError::*; + match self { + MiddlewareError(e) => write!(f, "middleware error: {}", e), + BadRequest(e) => write!(f, "bad request: {}", e.text()), + ServerError(e) => write!(f, "server error: {}", e.text()), + } + } +} + +impl Error for ResponseError {} + +pub trait ResultExt { + fn allow_errors(self) -> CargoResult; +} + +impl ResultExt for Result { + fn allow_errors(self) -> CargoResult { + use ResponseError::*; + self.or_else(|e| match e { + MiddlewareError(e) => Err(e), + BadRequest(r) | ServerError(r) => Ok(r), + }) + } +} diff --git a/src/tests/http-data/krate_only_owners_can_yank b/src/tests/http-data/krate_only_owners_can_yank new file mode 100644 index 00000000000..7eef949eda8 --- /dev/null +++ b/src/tests/http-data/krate_only_owners_can_yank @@ -0,0 +1,73 @@ +[ + { + "request": { + "uri": "http://alexcrichton-test.s3.amazonaws.com/crates/foo_not/foo_not-1.0.0.crate", + "method": "PUT", + "headers": [ + [ + "host", + "alexcrichton-test.s3.amazonaws.com" + ], + [ + "date", + "Fri, 15 Sep 2017 07:53:06 -0700" + ], + [ + "accept", + "*/*" + ], + [ + "user-agent", + "reqwest/0.9.1" + ], + [ + "authorization", + "AWS AKIAICL5IWUZYWWKA7JA:xCp3sUdUdmScjI6ct58zFv6BoGQ=" + ], + [ + "content-length", + "35" + ], + [ + "accept-encoding", + "gzip" + ], + [ + "content-type", + "application/x-tar" + ] + ], + "body": "H4sIAAAAAAAA/+3AAQEAAACCIP+vbkhQwKsBLq+17wAEAAA=" + }, + "response": { + "status": 200, + "headers": [ + [ + "ETag", + "\"f9016ad360cebb4fe2e6e96e5949f022\"" + ], + [ + "x-amz-id-2", + "FCKNKZUo5EeUNwVyhZ9P7ehfXoctqePzXx2RSE1VxoSX9rdfskkyAJUNHAF2AQojRon00LfTLPY=" + ], + [ + "x-amz-request-id", + "3233F8227A852593" + ], + [ + "Server", + "AmazonS3" + ], + [ + "content-length", + "0" + ], + [ + "date", + "Fri, 15 Sep 2017 14:53:07 GMT" + ] + ], + "body": "" + } + } +] diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 4e2b803ce51..022a812baa0 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -1,3 +1,4 @@ +use crate::prelude::*; use crate::{ builders::{CrateBuilder, DependencyBuilder, PublishBuilder, VersionBuilder}, new_category, new_dependency, new_user, CrateMeta, CrateResponse, GoodCrate, OkBool, @@ -75,6 +76,16 @@ impl crate::util::MockTokenUser { } } +fn yank_request<'a>(app: &'a App, krate_name: &str, version: &str) -> RequestBuilder<'a> { + let url = format!("/api/v1/crates/{}/{}/yank", krate_name, version); + app.delete(&url) +} + +fn publish_request(app: &App, publish_builder: PublishBuilder) -> RequestBuilder<'_> { + app.put("/api/v1/crates/new") + .with_body(publish_builder.body()) +} + #[test] fn index() { let (app, anon) = TestApp::init().empty(); @@ -1403,19 +1414,31 @@ fn yank() { } #[test] -fn yank_not_owner() { - let (app, _, _, token) = TestApp::init().with_token(); - let another_user = app.db_new_user("bar"); - let another_user = another_user.as_model(); - app.db(|conn| { - CrateBuilder::new("foo_not", another_user.id).expect_build(conn); - }); +fn only_owners_can_yank() -> CargoResult<()> { + let app = App::new(); + let owner = app.create_user("owner")?; + + let crate_to_publish = PublishBuilder::new("foo_not"); + publish_request(&app, crate_to_publish) + .signed_in_as(&owner) + .send()?; - let json = token.yank("foo_not", "1.0.0").bad_with_status(200); + let user_yank_result = yank_request(&app, "foo_not", "1.0.0") + .signed_in_as(&app.create_user("new_user")?) + .send()? // FIXME: We should return 403 here not 200 + .json::()?; assert_eq!( - json.errors[0].detail, - "crate `foo_not` does not have a version `1.0.0`" + user_yank_result.errors[0].detail, + "must already be an owner to yank or unyank", ); + + let owner_yank_result = yank_request(&app, "foo_not", "1.0.0") + .signed_in_as(&owner) + .send()? + .json::()?; + assert!(owner_yank_result.ok); + + Ok(()) } #[test] diff --git a/src/tests/prelude.rs b/src/tests/prelude.rs new file mode 100644 index 00000000000..cf73dd6a4b2 --- /dev/null +++ b/src/tests/prelude.rs @@ -0,0 +1,5 @@ +pub use crate::helpers::app::*; +pub use crate::helpers::request::RequestBuilder; +pub use crate::helpers::response::ResultExt as _; +pub use crate::util::Bad as ErrorDetails; +pub use cargo_registry::util::CargoResult; diff --git a/src/tests/read_only_mode.rs b/src/tests/read_only_mode.rs index 6f742c6f1b6..f855e7aebbb 100644 --- a/src/tests/read_only_mode.rs +++ b/src/tests/read_only_mode.rs @@ -1,41 +1,52 @@ use crate::builders::CrateBuilder; -use crate::{RequestHelper, TestApp}; +use crate::prelude::*; use diesel::prelude::*; #[test] -fn can_hit_read_only_endpoints_in_read_only_mode() { - let (app, anon) = TestApp::init().empty(); - app.db(set_read_only).unwrap(); - anon.get::<()>("/api/v1/crates").assert_status(200); +fn can_hit_read_only_endpoints_in_read_only_mode() -> CargoResult<()> { + let app = App::new(); + app.set_read_only()?; + app.get("/api/v1/crates").send()?; + Ok(()) } #[test] -fn cannot_hit_endpoint_which_writes_db_in_read_only_mode() { - let (app, _, user, token) = TestApp::init().with_token(); +fn cannot_hit_endpoint_which_writes_db_in_read_only_mode() -> CargoResult<()> { + let app = App::new(); + let user = app.create_user("new_user")?; + let token = app.token_for(&user)?; + app.db(|conn| { - CrateBuilder::new("foo_yank_read_only", user.as_model().id) + CrateBuilder::new("foo_yank_read_only", user.id) .version("1.0.0") - .expect_build(conn); - set_read_only(conn).unwrap(); - }); - token - .delete::<()>("/api/v1/crates/foo_yank_read_only/1.0.0/yank") - .assert_status(503); + .build(conn) + })?; + app.set_read_only()?; + let resp = app + .delete("/api/v1/crates/foo_yank_read_only/1.0.0/yank") + .with_token(&token) + .send() + .allow_errors()?; + assert_eq!(503, resp.status()); + Ok(()) } #[test] -fn can_download_crate_in_read_only_mode() { - let (app, anon, user) = TestApp::with_proxy().with_user(); +fn can_download_crate_in_read_only_mode() -> CargoResult<()> { + let app = App::new(); app.db(|conn| { - CrateBuilder::new("foo_download_read_only", user.as_model().id) + let user = app.create_user("new_user")?; + CrateBuilder::new("foo_download_read_only", user.id) .version("1.0.0") - .expect_build(conn); - set_read_only(conn).unwrap(); - }); + .build(conn) + })?; + app.set_read_only()?; - anon.get::<()>("/api/v1/crates/foo_download_read_only/1.0.0/download") - .assert_status(302); + let resp = app + .get("/api/v1/crates/foo_download_read_only/1.0.0/download") + .send()?; + assert_eq!(302, resp.status()); // We're in read only mode so the download should not have been counted app.db(|conn| { @@ -44,12 +55,8 @@ fn can_download_crate_in_read_only_mode() { let dl_count = version_downloads .select(sum(downloads)) - .get_result::>(conn); - assert_eq!(Ok(None), dl_count); + .get_result::>(conn)?; + assert_eq!(None, dl_count); + Ok(()) }) } - -fn set_read_only(conn: &PgConnection) -> QueryResult<()> { - diesel::sql_query("SET TRANSACTION READ ONLY").execute(conn)?; - Ok(()) -} diff --git a/src/tests/util.rs b/src/tests/util.rs index 623b8474e77..29c3dec27b1 100644 --- a/src/tests/util.rs +++ b/src/tests/util.rs @@ -332,7 +332,7 @@ pub struct Error { pub detail: String, } -#[derive(Deserialize)] +#[derive(Deserialize, Debug)] pub struct Bad { pub errors: Vec, }