diff --git a/migrations/2019-12-14-005407_create_crate_owner_actions/down.sql b/migrations/2019-12-14-005407_create_crate_owner_actions/down.sql new file mode 100644 index 00000000000..38af4929577 --- /dev/null +++ b/migrations/2019-12-14-005407_create_crate_owner_actions/down.sql @@ -0,0 +1 @@ +DROP TABLE crate_owner_actions; diff --git a/migrations/2019-12-14-005407_create_crate_owner_actions/up.sql b/migrations/2019-12-14-005407_create_crate_owner_actions/up.sql new file mode 100644 index 00000000000..6359ec20408 --- /dev/null +++ b/migrations/2019-12-14-005407_create_crate_owner_actions/up.sql @@ -0,0 +1,8 @@ +CREATE TABLE crate_owner_actions ( + id SERIAL PRIMARY KEY, + crate_id INTEGER NOT NULL REFERENCES crates(id) ON DELETE CASCADE, + user_id INTEGER NOT NULL REFERENCES users(id), + api_token_id INTEGER REFERENCES api_tokens(id), + action INTEGER NOT NULL, + time TIMESTAMP NOT NULL DEFAULT now() +); diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index 2817ebf9c54..5ee6c5b43ef 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -7,8 +7,8 @@ use crate::controllers::frontend_prelude::*; use crate::models::{ - Category, Crate, CrateCategory, CrateKeyword, CrateVersions, Keyword, RecentCrateDownloads, - User, Version, VersionOwnerAction, + Category, Crate, CrateCategory, CrateKeyword, CrateOwnerAction, CrateVersions, Keyword, + RecentCrateDownloads, User, Version, VersionOwnerAction, }; use crate::schema::*; use crate::views::{ @@ -106,6 +106,7 @@ pub fn show(req: &mut dyn Request) -> AppResult { let conn = req.db_conn()?; let krate = Crate::by_name(name).first::(&*conn)?; + let krate_owner_actions = CrateOwnerAction::by_crate(&*conn, &krate)?; let mut versions_and_publishers = krate .all_versions() .left_outer_join(users::table) @@ -162,6 +163,7 @@ pub fn show(req: &mut dyn Request) -> AppResult { Some(badges), false, recent_downloads, + krate_owner_actions, ), versions: versions_publishers_and_audit_actions .into_iter() diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs index 1ddf032433d..9ccc1c32efa 100644 --- a/src/controllers/krate/owners.rs +++ b/src/controllers/krate/owners.rs @@ -3,7 +3,7 @@ use serde_json; use crate::controllers::prelude::*; -use crate::models::{Crate, Owner, Rights, Team, User}; +use crate::models::{insert_crate_owner_action, Crate, CrateAction, Owner, Rights, Team, User}; use crate::views::EncodableOwner; /// Handles the `GET /crates/:crate_id/owners` route. @@ -95,6 +95,7 @@ fn modify_owners(req: &mut dyn Request, add: bool) -> AppResult { let user = req.user()?; let crate_name = &req.params()["crate_id"]; let conn = req.db_conn()?; + let authentication_source = req.authentication_source()?; conn.transaction(|| { let krate = Crate::by_name(crate_name).first::(&*conn)?; @@ -124,6 +125,15 @@ fn modify_owners(req: &mut dyn Request, add: bool) -> AppResult { let msg = krate.owner_add(app, &conn, user, login)?; msgs.push(msg); } + + insert_crate_owner_action( + &conn, + krate.id, + user.id, + authentication_source.api_token_id(), + CrateAction::InviteUser, + )?; + msgs.join(",") } else { for login in &logins { @@ -136,6 +146,15 @@ fn modify_owners(req: &mut dyn Request, add: bool) -> AppResult { at least one individual owner is required.", )); } + + insert_crate_owner_action( + &conn, + krate.id, + user.id, + authentication_source.api_token_id(), + CrateAction::RemoveUser, + )?; + "owners successfully removed".to_owned() }; diff --git a/src/models.rs b/src/models.rs index 9fea1f7874d..69b4c837f76 100644 --- a/src/models.rs +++ b/src/models.rs @@ -8,6 +8,7 @@ pub use self::email::{Email, NewEmail}; pub use self::follow::Follow; pub use self::keyword::{CrateKeyword, Keyword}; pub use self::krate::{Crate, CrateVersions, NewCrate, RecentCrateDownloads}; +pub use self::krate_owner_actions::{insert_crate_owner_action, CrateAction, CrateOwnerAction}; pub use self::owner::{CrateOwner, Owner, OwnerKind}; pub use self::rights::Rights; pub use self::team::{NewTeam, Team}; @@ -27,6 +28,7 @@ mod email; mod follow; mod keyword; pub mod krate; +mod krate_owner_actions; mod owner; mod rights; mod team; diff --git a/src/models/krate.rs b/src/models/krate.rs index 46d035a4dae..bee6cd16397 100644 --- a/src/models/krate.rs +++ b/src/models/krate.rs @@ -10,11 +10,11 @@ use crate::app::App; use crate::email; use crate::models::version::TopVersions; use crate::models::{ - Badge, Category, CrateOwner, CrateOwnerInvitation, Keyword, NewCrateOwnerInvitation, Owner, - OwnerKind, ReverseDependency, User, Version, + Badge, Category, CrateOwner, CrateOwnerAction, CrateOwnerInvitation, Keyword, + NewCrateOwnerInvitation, Owner, OwnerKind, ReverseDependency, User, Version, }; use crate::util::{cargo_err, AppResult}; -use crate::views::{EncodableCrate, EncodableCrateLinks}; +use crate::views::{EncodableAuditAction, EncodableCrate, EncodableCrateLinks}; use crate::models::helpers::with_count::*; use crate::publish_rate_limit::PublishRateLimit; @@ -294,6 +294,7 @@ impl Crate { badges, exact_match, recent_downloads, + vec![], ) } @@ -307,6 +308,7 @@ impl Crate { badges: Option>, exact_match: bool, recent_downloads: Option, + audit_actions: Vec<(CrateOwnerAction, User)>, ) -> EncodableCrate { let Crate { name, @@ -327,6 +329,14 @@ impl Crate { let category_ids = categories.map(|cats| cats.iter().map(|cat| cat.slug.clone()).collect()); let badges = badges.map(|bs| bs.into_iter().map(Badge::encodable).collect()); let documentation = Crate::remove_blocked_documentation_urls(documentation); + let audit_actions = audit_actions + .into_iter() + .map(|(audit_action, user)| EncodableAuditAction { + action: audit_action.action.into(), + user: User::encodable_public(user), + time: audit_action.time, + }) + .collect(); EncodableCrate { id: name.clone(), @@ -354,6 +364,7 @@ impl Crate { owner_user: Some(format!("/api/v1/crates/{}/owner_user", name)), reverse_dependencies: format!("/api/v1/crates/{}/reverse_dependencies", name), }, + audit_actions, } } diff --git a/src/models/krate_owner_actions.rs b/src/models/krate_owner_actions.rs new file mode 100644 index 00000000000..0c404fbd73f --- /dev/null +++ b/src/models/krate_owner_actions.rs @@ -0,0 +1,101 @@ +use chrono::NaiveDateTime; +use diesel::prelude::*; +use diesel::{ + deserialize::{self, FromSql}, + pg::Pg, + serialize::{self, Output, ToSql}, + sql_types::Integer, +}; +use std::io::Write; + +use crate::models::{ApiToken, Crate, User}; +use crate::schema::*; + +#[derive(Debug, Clone, Copy, PartialEq, FromSqlRow, AsExpression)] +#[repr(i32)] +#[sql_type = "Integer"] +pub enum CrateAction { + InviteUser = 0, + RemoveUser = 1, +} + +impl Into<&'static str> for CrateAction { + fn into(self) -> &'static str { + match self { + CrateAction::InviteUser => "invite_user", + CrateAction::RemoveUser => "remove_user", + } + } +} + +impl Into for CrateAction { + fn into(self) -> String { + let string: &'static str = self.into(); + + string.into() + } +} + +impl FromSql for CrateAction { + fn from_sql(bytes: Option<&[u8]>) -> deserialize::Result { + match >::from_sql(bytes)? { + 0 => Ok(CrateAction::InviteUser), + 1 => Ok(CrateAction::RemoveUser), + n => Err(format!("unknown crate action: {}", n).into()), + } + } +} + +impl ToSql for CrateAction { + fn to_sql(&self, out: &mut Output<'_, W, Pg>) -> serialize::Result { + ToSql::::to_sql(&(*self as i32), out) + } +} + +#[derive(Debug, Clone, Copy, Queryable, Identifiable, Associations)] +#[belongs_to(Crate)] +#[belongs_to(User)] +#[belongs_to(ApiToken)] +pub struct CrateOwnerAction { + pub id: i32, + pub crate_id: i32, + pub user_id: i32, + pub api_token_id: Option, + pub action: CrateAction, + pub time: NaiveDateTime, +} + +impl CrateOwnerAction { + pub fn all(conn: &PgConnection) -> QueryResult> { + crate_owner_actions::table.load(conn) + } + + pub fn by_crate(conn: &PgConnection, krate: &Crate) -> QueryResult> { + use crate_owner_actions::dsl::crate_id; + + crate_owner_actions::table + .filter(crate_id.eq(krate.id)) + .inner_join(users::table) + .order(crate_owner_actions::dsl::id) + .load(conn) + } +} + +pub fn insert_crate_owner_action( + conn: &PgConnection, + crate_id_: i32, + user_id_: i32, + api_token_id_: Option, + action_: CrateAction, +) -> QueryResult { + use crate_owner_actions::dsl::{action, api_token_id, crate_id, user_id}; + + diesel::insert_into(crate_owner_actions::table) + .values(( + crate_id.eq(crate_id_), + user_id.eq(user_id_), + api_token_id.eq(api_token_id_), + action.eq(action_), + )) + .get_result(conn) +} diff --git a/src/schema.patch b/src/schema.patch index 19d60a0ec6d..125f0580b17 100644 --- a/src/schema.patch +++ b/src/schema.patch @@ -10,7 +10,8 @@ index df884e4..18e08cd 100644 use diesel_full_text_search::{TsVector as Tsvector}; /// Representation of the `api_tokens` table. -@@ -125,14 +125,8 @@ table! { +@@ -169,16 +171,10 @@ + /// /// Its SQL type is `Timestamp`. /// /// (Automatically generated by Diesel.) @@ -25,8 +26,8 @@ index df884e4..18e08cd 100644 } table! { -@@ -608,11 +610,29 @@ table! { - /// (Automatically generated by Diesel.) + use diesel::sql_types::*; +@@ -711,10 +707,28 @@ rendered_at -> Timestamp, } } @@ -55,10 +56,10 @@ index df884e4..18e08cd 100644 /// Representation of the `reserved_crate_names` table. /// -@@ -881,23 +901,25 @@ table! { - - joinable!(api_tokens -> users (user_id)); - joinable!(badges -> crates (crate_id)); +@@ -1045,11 +1059,12 @@ + joinable!(crate_owner_actions -> api_tokens (api_token_id)); + joinable!(crate_owner_actions -> crates (crate_id)); + joinable!(crate_owner_actions -> users (user_id)); joinable!(crate_owner_invitations -> crates (crate_id)); joinable!(crate_owners -> crates (crate_id)); -joinable!(crate_owners -> users (created_by)); @@ -69,8 +70,7 @@ index df884e4..18e08cd 100644 joinable!(crates_keywords -> crates (crate_id)); joinable!(crates_keywords -> keywords (keyword_id)); joinable!(dependencies -> crates (crate_id)); - joinable!(dependencies -> versions (version_id)); - joinable!(emails -> users (user_id)); +@@ -1058,10 +1073,11 @@ joinable!(follows -> crates (crate_id)); joinable!(follows -> users (user_id)); joinable!(publish_limit_buckets -> users (user_id)); @@ -80,11 +80,9 @@ index df884e4..18e08cd 100644 joinable!(version_authors -> users (user_id)); joinable!(version_authors -> versions (version_id)); joinable!(version_downloads -> versions (version_id)); - joinable!(version_owner_actions -> api_tokens (owner_token_id)); - -@@ -913,13 +935,14 @@ allow_tables_to_appear_in_same_query!( - emails, - follows, + joinable!(version_owner_actions -> api_tokens (api_token_id)); + joinable!(version_owner_actions -> users (user_id)); +@@ -1087,10 +1103,11 @@ keywords, metadata, publish_limit_buckets, @@ -96,4 +94,3 @@ index df884e4..18e08cd 100644 users, version_authors, version_downloads, - versions, diff --git a/src/schema.rs b/src/schema.rs index 9f916069cac..b47f0ccdb1c 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -176,6 +176,53 @@ table! { } } +table! { + use diesel::sql_types::*; + use diesel_full_text_search::{TsVector as Tsvector}; + + /// Representation of the `crate_owner_actions` table. + /// + /// (Automatically generated by Diesel.) + crate_owner_actions (id) { + /// The `id` column of the `crate_owner_actions` table. + /// + /// Its SQL type is `Int4`. + /// + /// (Automatically generated by Diesel.) + id -> Int4, + /// The `crate_id` column of the `crate_owner_actions` table. + /// + /// Its SQL type is `Int4`. + /// + /// (Automatically generated by Diesel.) + crate_id -> Int4, + /// The `user_id` column of the `crate_owner_actions` table. + /// + /// Its SQL type is `Int4`. + /// + /// (Automatically generated by Diesel.) + user_id -> Int4, + /// The `api_token_id` column of the `crate_owner_actions` table. + /// + /// Its SQL type is `Nullable`. + /// + /// (Automatically generated by Diesel.) + api_token_id -> Nullable, + /// The `action` column of the `crate_owner_actions` table. + /// + /// Its SQL type is `Int4`. + /// + /// (Automatically generated by Diesel.) + action -> Int4, + /// The `time` column of the `crate_owner_actions` table. + /// + /// Its SQL type is `Timestamp`. + /// + /// (Automatically generated by Diesel.) + time -> Timestamp, + } +} + table! { use diesel::sql_types::*; use diesel_full_text_search::{TsVector as Tsvector}; @@ -1009,6 +1056,9 @@ table! { joinable!(api_tokens -> users (user_id)); joinable!(badges -> crates (crate_id)); +joinable!(crate_owner_actions -> api_tokens (api_token_id)); +joinable!(crate_owner_actions -> crates (crate_id)); +joinable!(crate_owner_actions -> users (user_id)); joinable!(crate_owner_invitations -> crates (crate_id)); joinable!(crate_owners -> crates (crate_id)); joinable!(crate_owners -> teams (owner_id)); @@ -1041,6 +1091,7 @@ allow_tables_to_appear_in_same_query!( background_jobs, badges, categories, + crate_owner_actions, crate_owner_invitations, crate_owners, crates, diff --git a/src/tasks/dump_db/dump-db.toml b/src/tasks/dump_db/dump-db.toml index bf552c6ad57..7c5b5ff5076 100644 --- a/src/tasks/dump_db/dump-db.toml +++ b/src/tasks/dump_db/dump-db.toml @@ -217,3 +217,11 @@ published_by = "public" [versions_published_by.columns] version_id = "private" email = "private" + +[crate_owner_actions.columns] +id = "private" +crate_id = "private" +user_id = "private" +api_token_id = "private" +action = "private" +time = "private" diff --git a/src/tests/krate.rs b/src/tests/krate.rs index f85912dc487..be56db4c256 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -2262,6 +2262,80 @@ fn test_cargo_invite_owners() { ) } +#[test] +fn test_invite_user_audit_action() { + let (app, anon, owner) = TestApp::init().with_user(); + + let new_user = app.db_new_user("cilantro"); + app.db(|conn| { + CrateBuilder::new("guacamole", owner.as_model().id).expect_build(conn); + }); + + #[derive(Serialize)] + struct OwnerReq { + owners: Option>, + } + #[derive(Deserialize, Debug)] + struct OwnerResp { + // server must include `ok: true` to support old cargo clients + ok: bool, + msg: String, + } + + let body = serde_json::to_string(&OwnerReq { + owners: Some(vec![new_user.as_model().gh_login.clone()]), + }); + owner + .put::("/api/v1/crates/guacamole/owners", body.unwrap().as_bytes()) + .good(); + + let json = anon.show_crate("guacamole"); + + assert_eq!(json.krate.audit_actions.len(), 1); + let action = &json.krate.audit_actions[0]; + assert_eq!(action.action, "invite_user"); + assert_eq!(action.user.id, owner.as_model().id); +} + +#[test] +fn test_remove_user_audit_action() { + let (app, anon, owner) = TestApp::init().with_user(); + + let new_user = app.db_new_user("cilantro"); + app.db(|conn| { + CrateBuilder::new("guacamole", owner.as_model().id).expect_build(conn); + }); + + #[derive(Serialize)] + struct OwnerReq { + owners: Option>, + } + #[derive(Deserialize, Debug)] + struct OwnerResp { + // server must include `ok: true` to support old cargo clients + ok: bool, + msg: String, + } + + let body = serde_json::to_string(&OwnerReq { + owners: Some(vec![new_user.as_model().gh_login.clone()]), + }) + .unwrap(); + owner + .put::("/api/v1/crates/guacamole/owners", body.as_bytes()) + .good(); + owner + .delete_with_body::("/api/v1/crates/guacamole/owners", body.as_bytes()) + .good(); + + let json = anon.show_crate("guacamole"); + + assert_eq!(json.krate.audit_actions.len(), 2); + let action = &json.krate.audit_actions[1]; + assert_eq!(action.action, "remove_user"); + assert_eq!(action.user.id, owner.as_model().id); +} + #[test] fn new_krate_tarball_with_hard_links() { let (_, _, _, token) = TestApp::init().with_token(); diff --git a/src/views.rs b/src/views.rs index e5b86a8f98b..bd06b0703db 100644 --- a/src/views.rs +++ b/src/views.rs @@ -104,6 +104,7 @@ pub struct EncodableCrate { pub repository: Option, pub links: EncodableCrateLinks, pub exact_match: bool, + pub audit_actions: Vec, } #[derive(Serialize, Deserialize, Debug)] @@ -378,6 +379,7 @@ mod tests { reverse_dependencies: "".to_string(), }, exact_match: false, + audit_actions: vec![], }; let json = serde_json::to_string(&crt).unwrap(); assert!(json