diff --git a/src/tests/account_lock.rs b/src/tests/account_lock.rs index 3bbe56d4b9..7782b5acc8 100644 --- a/src/tests/account_lock.rs +++ b/src/tests/account_lock.rs @@ -26,12 +26,17 @@ fn account_locked_indefinitely() { let (app, _anon, user) = TestApp::init().with_user(); lock_account(&app, user.as_model().id, None); - user.get::<()>(URL) - .bad_with_status(StatusCode::FORBIDDEN) - .assert_error(&format!( - "This account is indefinitely locked. Reason: {}", - LOCK_REASON - )); + let response = user.get::<()>(URL); + response.assert_status(StatusCode::FORBIDDEN); + + let error_message = format!( + "This account is indefinitely locked. Reason: {}", + LOCK_REASON + ); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": error_message }] }) + ); } #[test] @@ -42,12 +47,17 @@ fn account_locked_with_future_expiry() { lock_account(&app, user.as_model().id, Some(until)); let until = until.format("%Y-%m-%d at %H:%M:%S UTC"); - user.get::<()>(URL) - .bad_with_status(StatusCode::FORBIDDEN) - .assert_error(&format!( - "This account is locked until {}. Reason: {}", - until, LOCK_REASON, - )); + let response = user.get::<()>(URL); + response.assert_status(StatusCode::FORBIDDEN); + + let error_message = format!( + "This account is locked until {}. Reason: {}", + until, LOCK_REASON, + ); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": error_message }] }) + ); } #[test] diff --git a/src/tests/all.rs b/src/tests/all.rs index e19759eebd..eca833ff45 100644 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -11,7 +11,7 @@ extern crate serde; #[macro_use] extern crate serde_json; -use crate::util::{Bad, RequestHelper, TestApp}; +use crate::util::{RequestHelper, TestApp}; use cargo_registry::{ models::{Crate, CrateOwner, Dependency, NewCategory, NewTeam, NewUser, Team, User, Version}, schema::crate_owners, @@ -182,14 +182,6 @@ fn req(method: conduit::Method, path: &str) -> MockRequest { request } -fn bad_resp(r: &mut AppResponse) -> Option { - let bad = json::(r); - if bad.errors.is_empty() { - return None; - } - Some(bad) -} - fn json(r: &mut AppResponse) -> T where for<'de> T: serde::Deserialize<'de>, diff --git a/src/tests/krate/dependencies.rs b/src/tests/krate/dependencies.rs index 4a741721fc..020981bb20 100644 --- a/src/tests/krate/dependencies.rs +++ b/src/tests/krate/dependencies.rs @@ -26,6 +26,10 @@ fn dependencies() { .good(); assert_eq!(deps.dependencies[0].crate_id, "bar_deps"); - anon.get::<()>("/api/v1/crates/foo_deps/1.0.2/dependencies") - .bad_with_status(StatusCode::OK); + let response = anon.get::<()>("/api/v1/crates/foo_deps/1.0.2/dependencies"); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "crate `foo_deps` does not have a version `1.0.2`" }] }) + ); } diff --git a/src/tests/krate/publish.rs b/src/tests/krate/publish.rs index 5dd3d79db2..40900b2238 100644 --- a/src/tests/krate/publish.rs +++ b/src/tests/krate/publish.rs @@ -86,15 +86,15 @@ fn invalid_names() { let bad_name = |name: &str, error_message: &str| { let crate_to_publish = PublishBuilder::new(name).version("1.0.0"); - let json = token - .enqueue_publish(crate_to_publish) - .bad_with_status(StatusCode::OK); - - assert!( - json.errors[0].detail.contains(error_message,), - "{:?}", - json.errors - ); + let response = token.enqueue_publish(crate_to_publish); + response.assert_status(StatusCode::OK); + + let json = response.json(); + let json = json.as_object().unwrap(); + let errors = json.get("errors").unwrap().as_array().unwrap(); + let first_error = errors.first().unwrap().as_object().unwrap(); + let detail = first_error.get("detail").unwrap().as_str().unwrap(); + assert!(detail.contains(error_message), "{:?}", detail); }; let error_message = "expected a valid crate name"; @@ -251,9 +251,13 @@ fn reject_new_krate_with_non_exact_dependency() { let crate_to_publish = PublishBuilder::new("new_dep") .version("1.0.0") .dependency(dependency); - token - .enqueue_publish(crate_to_publish) - .bad_with_status(StatusCode::OK); + + let response = token.enqueue_publish(crate_to_publish); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "no known crate named `foo_dep`" }] }) + ); } #[test] @@ -696,19 +700,28 @@ fn bad_keywords() { let (_, _, _, token) = TestApp::init().with_token(); let crate_to_publish = PublishBuilder::new("foo_bad_key").keyword("super-long-keyword-name-oh-no"); - token - .enqueue_publish(crate_to_publish) - .bad_with_status(StatusCode::OK); + let response = token.enqueue_publish(crate_to_publish); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "invalid upload request: invalid length 29, expected a keyword with less than 20 characters at line 1 column 221" }] }) + ); let crate_to_publish = PublishBuilder::new("foo_bad_key").keyword("?@?%"); - token - .enqueue_publish(crate_to_publish) - .bad_with_status(StatusCode::OK); + let response = token.enqueue_publish(crate_to_publish); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "invalid upload request: invalid value: string \"?@?%\", expected a valid keyword specifier at line 1 column 196" }] }) + ); let crate_to_publish = PublishBuilder::new("foo_bad_key").keyword("áccênts"); - token - .enqueue_publish(crate_to_publish) - .bad_with_status(StatusCode::OK); + let response = token.enqueue_publish(crate_to_publish); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "invalid upload request: invalid value: string \"áccênts\", expected a valid keyword specifier at line 1 column 201" }] }) + ); } #[test] diff --git a/src/tests/krate/search.rs b/src/tests/krate/search.rs index ca13d77fdb..ec7e36ce20 100644 --- a/src/tests/krate/search.rs +++ b/src/tests/krate/search.rs @@ -676,19 +676,19 @@ fn pagination_parameters_only_accept_integers() { CrateBuilder::new("pagination_links_3", user.id).expect_build(conn); }); - let invalid_per_page_json = anon - .get_with_query::<()>("/api/v1/crates", "page=1&per_page=100%22%EF%BC%8Cexception") - .bad_with_status(StatusCode::BAD_REQUEST); + let response = + anon.get_with_query::<()>("/api/v1/crates", "page=1&per_page=100%22%EF%BC%8Cexception"); + response.assert_status(StatusCode::BAD_REQUEST); assert_eq!( - invalid_per_page_json.errors[0].detail, - "invalid digit found in string" + response.json(), + json!({ "errors": [{ "detail": "invalid digit found in string" }] }) ); - let invalid_page_json = anon - .get_with_query::<()>("/api/v1/crates", "page=100%22%EF%BC%8Cexception&per_page=1") - .bad_with_status(StatusCode::BAD_REQUEST); + let response = + anon.get_with_query::<()>("/api/v1/crates", "page=100%22%EF%BC%8Cexception&per_page=1"); + response.assert_status(StatusCode::BAD_REQUEST); assert_eq!( - invalid_page_json.errors[0].detail, - "invalid digit found in string" + response.json(), + json!({ "errors": [{ "detail": "invalid digit found in string" }] }) ); } diff --git a/src/tests/krate/yanking.rs b/src/tests/krate/yanking.rs index ce1e7d1dea..a2e74f73d5 100644 --- a/src/tests/krate/yanking.rs +++ b/src/tests/krate/yanking.rs @@ -109,12 +109,11 @@ fn yank_by_a_non_owner_fails() { .expect_build(conn); }); - let json = token - .yank("foo_not", "1.0.0") - .bad_with_status(StatusCode::OK); + let response = token.yank("foo_not", "1.0.0"); + response.assert_status(StatusCode::OK); assert_eq!( - json.errors[0].detail, - "must already be an owner to yank or unyank" + response.json(), + json!({ "errors": [{ "detail": "must already be an owner to yank or unyank" }] }) ); } diff --git a/src/tests/owners.rs b/src/tests/owners.rs index 6bee4468ca..3d67d5bef4 100644 --- a/src/tests/owners.rs +++ b/src/tests/owners.rs @@ -135,28 +135,30 @@ fn owners_can_remove_self() { .db(|conn| CrateBuilder::new("owners_selfremove", user.as_model().id).expect_build(conn)); // Deleting yourself when you're the only owner isn't allowed. - let json = token - .remove_named_owner("owners_selfremove", username) - .bad_with_status(StatusCode::OK); - assert!(json.errors[0] - .detail - .contains("cannot remove all individual owners of a crate")); + let response = token.remove_named_owner("owners_selfremove", username); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "cannot remove all individual owners of a crate. Team member don't have permission to modify owners, so at least one individual owner is required." }] }) + ); create_and_add_owner(&app, &token, "secondowner", &krate); // Deleting yourself when there are other owners is allowed. - let json = token - .remove_named_owner("owners_selfremove", username) - .good(); - assert!(json.ok); + let response = token.remove_named_owner("owners_selfremove", username); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "msg": "owners successfully removed", "ok": true }) + ); // After you delete yourself, you no longer have permisions to manage the crate. - let json = token - .remove_named_owner("owners_selfremove", username) - .bad_with_status(StatusCode::OK); - assert!(json.errors[0] - .detail - .contains("only owners have permission to modify owners",)); + let response = token.remove_named_owner("owners_selfremove", username); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "only owners have permission to modify owners" }] }) + ); } // Verify consistency when adidng or removing multiple owners in a single request. @@ -172,35 +174,46 @@ fn modify_multiple_owners() { let user3 = create_and_add_owner(&app, &token, "user3", &krate); // Deleting all owners is not allowed. - let json = token - .remove_named_owners("owners_multiple", &[username, "user2", "user3"]) - .bad_with_status(StatusCode::OK); - assert!(&json.errors[0] - .detail - .contains("cannot remove all individual owners of a crate")); + let response = token.remove_named_owners("owners_multiple", &[username, "user2", "user3"]); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "cannot remove all individual owners of a crate. Team member don't have permission to modify owners, so at least one individual owner is required." }] }) + ); assert_eq!(app.db(|conn| krate.owners(&conn).unwrap()).len(), 3); // Deleting two owners at once is allowed. - let json = token - .remove_named_owners("owners_multiple", &["user2", "user3"]) - .good(); - assert!(json.ok); + let response = token.remove_named_owners("owners_multiple", &["user2", "user3"]); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "msg": "owners successfully removed", "ok": true }) + ); assert_eq!(app.db(|conn| krate.owners(&conn).unwrap()).len(), 1); // Adding multiple users fails if one of them already is an owner. - let json = token - .add_named_owners("owners_multiple", &["user2", username]) - .bad_with_status(StatusCode::OK); - assert!(&json.errors[0].detail.contains("is already an owner")); + let response = token.add_named_owners("owners_multiple", &["user2", username]); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "`foo` is already an owner" }] }) + ); assert_eq!(app.db(|conn| krate.owners(&conn).unwrap()).len(), 1); // Adding multiple users at once succeeds. - let json = token - .add_named_owners("owners_multiple", &["user2", "user3"]) - .good(); - assert!(json.ok); + let response = token.add_named_owners("owners_multiple", &["user2", "user3"]); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ + "msg": "user user2 has been invited to be an owner of crate owners_multiple,user user3 has been invited to be an owner of crate owners_multiple", + "ok": true, + }) + ); + user2.accept_ownership_invitation(&krate.name, krate.id); user3.accept_ownership_invitation(&krate.name, krate.id); + assert_eq!(app.db(|conn| krate.owners(&conn).unwrap()).len(), 3); } diff --git a/src/tests/team.rs b/src/tests/team.rs index 35477f8b33..e8d23bfddc 100644 --- a/src/tests/team.rs +++ b/src/tests/team.rs @@ -49,14 +49,11 @@ fn not_github() { CrateBuilder::new("foo_not_github", user.as_model().id).expect_build(conn); }); - let json = token - .add_named_owner("foo_not_github", "dropbox:foo:foo") - .bad_with_status(StatusCode::OK); - - assert!( - json.errors[0].detail.contains("unknown organization"), - "{:?}", - json.errors + let response = token.add_named_owner("foo_not_github", "dropbox:foo:foo"); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "unknown organization handler, only 'github:org:team' is supported" }] }) ); } @@ -68,16 +65,11 @@ fn weird_name() { CrateBuilder::new("foo_weird_name", user.as_model().id).expect_build(conn); }); - let json = token - .add_named_owner("foo_weird_name", "github:foo/../bar:wut") - .bad_with_status(StatusCode::OK); - - assert!( - json.errors[0] - .detail - .contains("organization cannot contain"), - "{:?}", - json.errors + let response = token.add_named_owner("foo_weird_name", "github:foo/../bar:wut"); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "organization cannot contain special characters like /" }] }) ); } @@ -90,14 +82,11 @@ fn one_colon() { CrateBuilder::new("foo_one_colon", user.as_model().id).expect_build(conn); }); - let json = token - .add_named_owner("foo_one_colon", "github:foo") - .bad_with_status(StatusCode::OK); - - assert!( - json.errors[0].detail.contains("missing github team"), - "{:?}", - json.errors + let response = token.add_named_owner("foo_one_colon", "github:foo"); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "missing github team argument; format is github:org:team" }] }) ); } @@ -109,19 +98,14 @@ fn nonexistent_team() { CrateBuilder::new("foo_nonexistent", user.as_model().id).expect_build(conn); }); - let json = token - .add_named_owner( - "foo_nonexistent", - "github:crates-test-org:this-does-not-exist", - ) - .bad_with_status(StatusCode::OK); - - assert!( - json.errors[0] - .detail - .contains("could not find the github team crates-test-org/this-does-not-exist"), - "{:?}", - json.errors + let response = token.add_named_owner( + "foo_nonexistent", + "github:crates-test-org:this-does-not-exist", + ); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "could not find the github team crates-test-org/this-does-not-exist" }] }) ); } @@ -164,19 +148,14 @@ fn add_team_as_non_member() { CrateBuilder::new("foo_team_non_member", user.as_model().id).expect_build(conn); }); - let json = token - .add_named_owner( - "foo_team_non_member", - "github:crates-test-org:just-for-crates-2", - ) - .bad_with_status(StatusCode::OK); - - assert!( - json.errors[0] - .detail - .contains("only members of a team can add it as an owner"), - "{:?}", - json.errors + let response = token.add_named_owner( + "foo_team_non_member", + "github:crates-test-org:just-for-crates-2", + ); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "only members of a team can add it as an owner" }] }) ); } @@ -197,12 +176,12 @@ fn remove_team_as_named_owner() { // Removing the individual owner is not allowed, since team members don't // have permission to manage ownership - let json = token_on_both_teams - .remove_named_owner("foo_remove_team", username) - .bad_with_status(StatusCode::OK); - assert!(json.errors[0] - .detail - .contains("cannot remove all individual owners of a crate")); + let response = token_on_both_teams.remove_named_owner("foo_remove_team", username); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "cannot remove all individual owners of a crate. Team member don't have permission to modify owners, so at least one individual owner is required." }] }) + ); token_on_both_teams .remove_named_owner("foo_remove_team", "github:crates-test-org:core") @@ -210,16 +189,11 @@ fn remove_team_as_named_owner() { let user_on_one_team = app.db_new_user(mock_user_on_only_one_team().gh_login); let crate_to_publish = PublishBuilder::new("foo_remove_team").version("2.0.0"); - let json = user_on_one_team - .enqueue_publish(crate_to_publish) - .bad_with_status(StatusCode::OK); - - assert!( - json.errors[0] - .detail - .contains("this crate exists but you don't seem to be an owner.",), - "{:?}", - json.errors + let response = user_on_one_team.enqueue_publish(crate_to_publish); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "this crate exists but you don't seem to be an owner. If you believe this is a mistake, perhaps you need to accept an invitation to be an owner before publishing." }] }) ); } @@ -241,16 +215,12 @@ fn remove_team_as_team_owner() { let user_on_one_team = app.db_new_user(mock_user_on_only_one_team().gh_login); let token_on_one_team = user_on_one_team.db_new_token("arbitrary token name"); - let json = token_on_one_team - .remove_named_owner("foo_remove_team_owner", "github:crates-test-org:core") - .bad_with_status(StatusCode::OK); - - assert!( - json.errors[0] - .detail - .contains("team members don't have permission to modify owners",), - "{:?}", - json.errors + let response = token_on_one_team + .remove_named_owner("foo_remove_team_owner", "github:crates-test-org:core"); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "team members don't have permission to modify owners" }] }) ); } @@ -272,16 +242,11 @@ fn publish_not_owned() { let user_on_one_team = app.db_new_user(mock_user_on_only_one_team().gh_login); let crate_to_publish = PublishBuilder::new("foo_not_owned").version("2.0.0"); - let json = user_on_one_team - .enqueue_publish(crate_to_publish) - .bad_with_status(StatusCode::OK); - - assert!( - json.errors[0] - .detail - .contains("this crate exists but you don't seem to be an owner.",), - "{:?}", - json.errors + let response = user_on_one_team.enqueue_publish(crate_to_publish); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "this crate exists but you don't seem to be an owner. If you believe this is a mistake, perhaps you need to accept an invitation to be an owner before publishing." }] }) ); } @@ -324,16 +289,11 @@ fn add_owners_as_team_owner() { let user_on_one_team = app.db_new_user(mock_user_on_only_one_team().gh_login); let token_on_one_team = user_on_one_team.db_new_token("arbitrary token name"); - let json = token_on_one_team - .add_named_owner("foo_add_owner", "arbitrary_username") - .bad_with_status(StatusCode::OK); - - assert!( - json.errors[0] - .detail - .contains("team members don't have permission to modify owners",), - "{:?}", - json.errors + let response = token_on_one_team.add_named_owner("foo_add_owner", "arbitrary_username"); + response.assert_status(StatusCode::OK); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "team members don't have permission to modify owners" }] }) ); } diff --git a/src/tests/token.rs b/src/tests/token.rs index 63bfa5536f..3b88ee6d90 100644 --- a/src/tests/token.rs +++ b/src/tests/token.rs @@ -6,6 +6,7 @@ use crate::{ use cargo_registry::{ models::ApiToken, schema::api_tokens, + util::errors::TOKEN_FORMAT_ERROR, views::{EncodableApiTokenWithToken, EncodableMe}, }; use std::collections::HashSet; @@ -28,14 +29,6 @@ struct NewResponse { #[derive(Deserialize)] struct RevokedResponse {} -macro_rules! assert_contains { - ($e:expr, $f:expr) => { - if !$e.contains($f) { - panic!(format!("expected '{}' to contain '{}'", $e, $f)); - } - }; -} - // Default values used by many tests static URL: &str = "/api/v1/me/tokens"; static NEW_BAR: &[u8] = br#"{ "api_token": { "name": "bar" } }"#; @@ -118,33 +111,36 @@ fn create_token_logged_out() { fn create_token_invalid_request() { let (_, _, user) = TestApp::init().with_user(); let invalid = br#"{ "name": "" }"#; - let json = user - .put::<()>(URL, invalid) - .bad_with_status(StatusCode::BAD_REQUEST); - - assert_contains!(json.errors[0].detail, "invalid new token request"); + let response = user.put::<()>(URL, invalid); + response.assert_status(StatusCode::BAD_REQUEST); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "invalid new token request: Error(\"missing field `api_token`\", line: 1, column: 14)" }] }) + ); } #[test] fn create_token_no_name() { let (_, _, user) = TestApp::init().with_user(); let empty_name = br#"{ "api_token": { "name": "" } }"#; - let json = user - .put::<()>(URL, empty_name) - .bad_with_status(StatusCode::BAD_REQUEST); - - assert_eq!(json.errors[0].detail, "name must have a value"); + let response = user.put::<()>(URL, empty_name); + response.assert_status(StatusCode::BAD_REQUEST); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "name must have a value" }] }) + ); } #[test] fn create_token_long_body() { let (_, _, user) = TestApp::init().with_user(); let too_big = &[5; 5192]; // Send a request with a 5kB body of 5's - let json = user - .put::<()>(URL, too_big) - .bad_with_status(StatusCode::BAD_REQUEST); - - assert_contains!(json.errors[0].detail, "max content length"); + let response = user.put::<()>(URL, too_big); + response.assert_status(StatusCode::BAD_REQUEST); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "max content length is: 2000" }] }) + ); } #[test] @@ -156,11 +152,12 @@ fn create_token_exceeded_tokens_per_user() { assert_ok!(ApiToken::insert(conn, id, &format!("token {}", i))); } }); - let json = user - .put::<()>(URL, NEW_BAR) - .bad_with_status(StatusCode::BAD_REQUEST); - - assert_contains!(json.errors[0].detail, "maximum tokens per user"); + let response = user.put::<()>(URL, NEW_BAR); + response.assert_status(StatusCode::BAD_REQUEST); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "maximum tokens per user is: 500" }] }) + ); } #[test] @@ -202,16 +199,14 @@ fn create_token_multiple_users_have_different_values() { #[test] fn cannot_create_token_with_token() { let (_, _, _, token) = TestApp::init().with_token(); - let json = token - .put::<()>( - "/api/v1/me/tokens", - br#"{ "api_token": { "name": "baz" } }"#, - ) - .bad_with_status(StatusCode::BAD_REQUEST); - - assert_contains!( - json.errors[0].detail, - "cannot use an API token to create a new API token" + let response = token.put::<()>( + "/api/v1/me/tokens", + br#"{ "api_token": { "name": "baz" } }"#, + ); + response.assert_status(StatusCode::BAD_REQUEST); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "cannot use an API token to create a new API token" }] }) ); } @@ -313,9 +308,10 @@ fn old_tokens_give_specific_error_message() { let mut request = anon.get_request(url); request.header(header::AUTHORIZATION, "oldtoken"); - let json = anon - .run::<()>(request) - .bad_with_status(StatusCode::UNAUTHORIZED); - - assert_contains!(json.errors[0].detail, "revoked"); + let response = anon.run::<()>(request); + response.assert_status(StatusCode::UNAUTHORIZED); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": TOKEN_FORMAT_ERROR }] }) + ); } diff --git a/src/tests/user.rs b/src/tests/user.rs index db280868dc..2684f873c8 100644 --- a/src/tests/user.rs +++ b/src/tests/user.rs @@ -113,10 +113,12 @@ fn auth_gives_a_token() { #[test] fn access_token_needs_data() { let (_, anon) = TestApp::init().empty(); - let json = anon - .get::<()>("/api/private/session/authorize") - .bad_with_status(StatusCode::BAD_REQUEST); - assert!(json.errors[0].detail.contains("invalid state")); + let response = anon.get::<()>("/api/private/session/authorize"); + response.assert_status(StatusCode::BAD_REQUEST); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "invalid state parameter" }] }) + ); } #[test] @@ -295,8 +297,12 @@ fn following() { assert_eq!(r.versions.len(), 0); assert_eq!(r.meta.more, false); - user.get_with_query::<()>("/api/v1/me/updates", "page=0") - .bad_with_status(StatusCode::BAD_REQUEST); + let response = user.get_with_query::<()>("/api/v1/me/updates", "page=0"); + response.assert_status(StatusCode::BAD_REQUEST); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "page indexing starts from 1, page 0 is invalid" }] }) + ); } #[test] @@ -487,23 +493,18 @@ fn test_empty_email_not_added() { let (_app, _anon, user) = TestApp::init().with_user(); let model = user.as_model(); - let json = user - .update_email_more_control(model.id, Some("")) - .bad_with_status(StatusCode::BAD_REQUEST); - assert!( - json.errors[0].detail.contains("empty email rejected"), - "{:?}", - json.errors + let response = user.update_email_more_control(model.id, Some("")); + response.assert_status(StatusCode::BAD_REQUEST); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "empty email rejected" }] }) ); - let json = user - .update_email_more_control(model.id, None) - .bad_with_status(StatusCode::BAD_REQUEST); - - assert!( - json.errors[0].detail.contains("empty email rejected"), - "{:?}", - json.errors + let response = user.update_email_more_control(model.id, None); + response.assert_status(StatusCode::BAD_REQUEST); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "empty email rejected" }] }) ); } @@ -519,25 +520,25 @@ fn test_other_users_cannot_change_my_email() { let another_user = app.db_new_user("not_me"); let another_user_model = another_user.as_model(); - let json = user - .update_email_more_control( - another_user_model.id, - Some("pineapple@pineapples.pineapple"), - ) - .bad_with_status(StatusCode::BAD_REQUEST); - assert!( - json.errors[0] - .detail - .contains("current user does not match requested user",), - "{:?}", - json.errors + let response = user.update_email_more_control( + another_user_model.id, + Some("pineapple@pineapples.pineapple"), + ); + response.assert_status(StatusCode::BAD_REQUEST); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "current user does not match requested user" }] }) ); - anon.update_email_more_control( + let response = anon.update_email_more_control( another_user_model.id, Some("pineapple@pineapples.pineapple"), - ) - .bad_with_status(StatusCode::FORBIDDEN); + ); + response.assert_status(StatusCode::FORBIDDEN); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "must be logged in to perform that action" }] }) + ); } /* Given a new user, test that their email can be added diff --git a/src/tests/util.rs b/src/tests/util.rs index 2f3a55b5eb..1433020649 100644 --- a/src/tests/util.rs +++ b/src/tests/util.rs @@ -554,22 +554,6 @@ pub struct Error { pub detail: String, } -#[derive(Deserialize)] -pub struct Bad { - pub errors: Vec, -} - -impl Bad { - pub fn assert_error(&self, expected: &str) { - assert_eq!( - 1, - self.errors.len(), - "the number of errors in this response is not 1" - ); - assert_eq!(expected, self.errors[0].detail); - } -} - /// A type providing helper methods for working with responses #[must_use] pub struct Response { @@ -602,18 +586,6 @@ where crate::json(&mut self.response) } - /// Assert the response status code and deserialze into a list of errors - /// - /// Cargo endpoints return a status 200 on error instead of 400. - #[track_caller] - pub fn bad_with_status(&mut self, expected: StatusCode) -> Bad { - assert_eq!(self.response.status(), expected); - match crate::bad_resp(&mut self.response) { - None => panic!("ok response: {:?}", self.response.status()), - Some(b) => b, - } - } - #[track_caller] pub fn assert_status(&self, status: StatusCode) -> &Self { assert_eq!(status, self.response.status()); diff --git a/src/util/errors.rs b/src/util/errors.rs index 64834fe606..03f5f9c0c3 100644 --- a/src/util/errors.rs +++ b/src/util/errors.rs @@ -24,6 +24,7 @@ use crate::util::AppResponse; mod json; +pub use json::TOKEN_FORMAT_ERROR; pub(crate) use json::{InsecurelyGeneratedTokenRevoked, NotFound, ReadOnlyMode, TooManyRequests}; /// Returns an error with status 200 and the provided description as JSON diff --git a/src/util/errors/json.rs b/src/util/errors/json.rs index f12eb5cc6b..3f375250b4 100644 --- a/src/util/errors/json.rs +++ b/src/util/errors/json.rs @@ -181,19 +181,20 @@ impl AppError for InsecurelyGeneratedTokenRevoked { } } +pub const TOKEN_FORMAT_ERROR: &str = + "The given API token does not match the format used by crates.io. \ + \ + Tokens generated before 2020-07-14 were generated with an insecure \ + random number generator, and have been revoked. You can generate a \ + new token at https://crates.io/me. \ + \ + For more information please see \ + https://blog.rust-lang.org/2020/07/14/crates-io-security-advisory.html. \ + We apologize for any inconvenience."; + impl fmt::Display for InsecurelyGeneratedTokenRevoked { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str( - "The given API token does not match the format used by crates.io. \ - \ - Tokens generated before 2020-07-14 were generated with an insecure \ - random number generator, and have been revoked. You can generate a \ - new token at https://crates.io/me. \ - \ - For more information please see \ - https://blog.rust-lang.org/2020/07/14/crates-io-security-advisory.html. \ - We apologize for any inconvenience.", - )?; + f.write_str(TOKEN_FORMAT_ERROR)?; Result::Ok(()) } }