Adding audit trail actions into the publish, yank and unyank transactions.

markcatley · markcatley · commit 6a0c3e6abc3d · 2019-10-08T12:18:46.000+13:00
diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs
@@ -7,7 +7,11 @@ use swirl::Job;
 use crate::controllers::prelude::*;
 use crate::git;
 use crate::models::dependency;
-use crate::models::{Badge, Category, Keyword, NewCrate, NewVersion, Rights, User};
+use crate::models::{
+    Badge, Category, Keyword, NewCrate, NewVersion, NewVersionOwnerAction, Rights, User,
+    VersionAction,
+};
+
 use crate::render;
 use crate::util::{read_fill, read_le_u32};
 use crate::util::{CargoError, ChainError, Maximums};
@@ -143,6 +147,14 @@ pub fn publish(req: &mut dyn Request) -> CargoResult<Response> {
         )?
         .save(&conn, &new_crate.authors, &verified_email_address)?;
 
+        NewVersionOwnerAction::new(
+            version.id,
+            user.id,
+            req.authentication_source()?.api_token_id(),
+            VersionAction::Publish,
+        )
+        .save(&conn)?;
+
         // Link this new version to all dependencies
         let git_deps = dependency::add_dependencies(&conn, &new_crate.deps, version.id)?;
 
diff --git a/src/controllers/version/yank.rs b/src/controllers/version/yank.rs
@@ -5,7 +5,7 @@ use swirl::Job;
 use super::version_and_crate;
 use crate::controllers::prelude::*;
 use crate::git;
-use crate::models::Rights;
+use crate::models::{NewVersionOwnerAction, Rights, VersionAction};
 use crate::util::CargoError;
 
 /// Handles the `DELETE /crates/:crate_id/:version/yank` route.
@@ -35,6 +35,14 @@ fn modify_yank(req: &mut dyn Request, yanked: bool) -> CargoResult<Response> {
     if user.rights(req.app(), &owners)? < Rights::Publish {
         return Err(human("must already be an owner to yank or unyank"));
     }
+    let action = if yanked {
+        VersionAction::Yank
+    } else {
+        VersionAction::Unyank
+    };
+    let api_token_id = req.authentication_source()?.api_token_id();
+
+    NewVersionOwnerAction::new(version.id, user.id, api_token_id, action).save(&conn)?;
 
     git::yank(krate.name, version, yanked)
         .enqueue(&conn)
diff --git a/src/middleware/current_user.rs b/src/middleware/current_user.rs
@@ -6,16 +6,17 @@ use diesel::prelude::*;
 use crate::db::RequestTransaction;
 use crate::util::errors::{std_error, CargoResult, ChainError, Unauthorized};
 
+use crate::models::ApiToken;
 use crate::models::User;
 use crate::schema::users;
 
 #[derive(Debug, Clone, Copy)]
 pub struct CurrentUser;
 
-#[derive(Debug, Clone, Eq, PartialEq)]
+#[derive(Debug, Copy, Clone, Eq, PartialEq)]
 pub enum AuthenticationSource {
     SessionCookie,
-    ApiToken { auth_header: String },
+    ApiToken { api_token_id: i32 },
 }
 
 impl Middleware for CurrentUser {
@@ -43,10 +44,17 @@ impl Middleware for CurrentUser {
             // Otherwise, look for an `Authorization` header on the request
             // and try to find a user in the database with a matching API token
             let user_auth = if let Some(headers) = req.headers().find("Authorization") {
-                let auth_header = headers[0].to_string();
-
-                User::find_by_api_token(&conn, &auth_header)
-                    .map(|user| (AuthenticationSource::ApiToken { auth_header }, user))
+                ApiToken::find_by_api_token(&conn, headers[0])
+                    .and_then(|api_token| {
+                        User::find(&conn, api_token.user_id).map(|user| {
+                            (
+                                AuthenticationSource::ApiToken {
+                                    api_token_id: api_token.id,
+                                },
+                                user,
+                            )
+                        })
+                    })
                     .optional()
                     .map_err(|e| Box::new(e) as Box<dyn Error + Send>)?
             } else {
@@ -85,3 +93,12 @@ impl<'a> RequestUser for dyn Request + 'a {
             .chain_error(|| Unauthorized)
     }
 }
+
+impl AuthenticationSource {
+    pub fn api_token_id(self) -> Option<i32> {
+        match self {
+            AuthenticationSource::SessionCookie => None,
+            AuthenticationSource::ApiToken { api_token_id } => Some(api_token_id),
+        }
+    }
+}
diff --git a/src/models.rs b/src/models.rs
@@ -1,4 +1,4 @@
-pub use self::action::{VersionAction, VersionOwnerAction};
+pub use self::action::{NewVersionOwnerAction, VersionAction, VersionOwnerAction};
 pub use self::badge::{Badge, CrateBadge, MaintenanceStatus};
 pub use self::category::{Category, CrateCategory, NewCategory};
 pub use self::crate_owner_invitation::{CrateOwnerInvitation, NewCrateOwnerInvitation};
diff --git a/src/models/action.rs b/src/models/action.rs
@@ -1,26 +1,102 @@
 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, User, Version};
 use crate::schema::*;
 
-#[derive(Debug, Clone, Copy)]
-#[repr(u32)]
+#[derive(Debug, Clone, Copy, PartialEq, FromSqlRow, AsExpression)]
+#[repr(i32)]
+#[sql_type = "Integer"]
 pub enum VersionAction {
     Publish = 0,
     Yank = 1,
     Unyank = 2,
 }
 
+impl FromSql<Integer, Pg> for VersionAction {
+    fn from_sql(bytes: Option<&[u8]>) -> deserialize::Result<Self> {
+        match <i32 as FromSql<Integer, Pg>>::from_sql(bytes)? {
+            0 => Ok(VersionAction::Publish),
+            1 => Ok(VersionAction::Yank),
+            2 => Ok(VersionAction::Unyank),
+            n => Err(format!("unknown version action: {}", n).into()),
+        }
+    }
+}
+
+impl ToSql<Integer, Pg> for VersionAction {
+    fn to_sql<W: Write>(&self, out: &mut Output<'_, W, Pg>) -> serialize::Result {
+        ToSql::<Integer, Pg>::to_sql(&(*self as i32), out)
+    }
+}
+
 #[derive(Debug, Clone, Copy, Queryable, Identifiable, Associations)]
 #[belongs_to(Version)]
 #[belongs_to(User, foreign_key = "owner_id")]
 #[belongs_to(ApiToken, foreign_key = "owner_token_id")]
 #[table_name = "version_owner_actions"]
 pub struct VersionOwnerAction {
     pub id: i32,
+    pub version_id: Option<i32>,
+    pub owner_id: Option<i32>,
+    pub owner_token_id: Option<i32>,
+    pub action: VersionAction,
+    pub time: NaiveDateTime,
+}
+
+impl VersionOwnerAction {
+    pub fn all(conn: &PgConnection) -> QueryResult<Vec<VersionOwnerAction>> {
+        version_owner_actions::table.load(conn)
+    }
+
+    pub fn by_version_id_and_action(
+        conn: &PgConnection,
+        _version_id: i32,
+        _action: VersionAction,
+    ) -> QueryResult<Vec<VersionOwnerAction>> {
+        use version_owner_actions::dsl::{action, version_id};
+
+        version_owner_actions::table
+            .filter(version_id.eq(_version_id))
+            .filter(action.eq(_action))
+            .load(conn)
+    }
+}
+
+#[derive(Copy, Clone, Debug, Insertable)]
+#[table_name = "version_owner_actions"]
+pub struct NewVersionOwnerAction {
     pub version_id: i32,
     pub owner_id: i32,
-    pub owner_token_id: i32,
+    pub owner_token_id: Option<i32>,
     pub action: VersionAction,
-    pub time: NaiveDateTime,
+}
+
+impl NewVersionOwnerAction {
+    pub fn new(
+        version_id: i32,
+        owner_id: i32,
+        owner_token_id: Option<i32>,
+        action: VersionAction,
+    ) -> Self {
+        Self {
+            version_id,
+            owner_id,
+            owner_token_id,
+            action,
+        }
+    }
+
+    pub fn save(&self, conn: &PgConnection) -> QueryResult<VersionOwnerAction> {
+        diesel::insert_into(version_owner_actions::table)
+            .values(self)
+            .get_result(conn)
+    }
 }
diff --git a/src/models/token.rs b/src/models/token.rs
@@ -46,6 +46,24 @@ impl ApiToken {
             last_used_at: self.last_used_at,
         }
     }
+
+    pub fn find_by_api_token(conn: &PgConnection, token_: &str) -> QueryResult<ApiToken> {
+        use crate::schema::api_tokens::dsl::{api_tokens, last_used_at, revoked, token};
+        use diesel::{dsl::now, update};
+
+        let tokens = api_tokens
+            .filter(token.eq(token_))
+            .filter(revoked.eq(false));
+
+        // If the database is in read only mode, we can't update last_used_at.
+        // Try updating in a new transaction, if that fails, fall back to reading
+        conn.transaction(|| {
+            update(tokens)
+                .set(last_used_at.eq(now.nullable()))
+                .get_result::<ApiToken>(conn)
+        })
+        .or_else(|_| tokens.first(conn))
+    }
 }
 
 #[cfg(test)]
diff --git a/src/models/user.rs b/src/models/user.rs
@@ -1,11 +1,10 @@
-use diesel::dsl::now;
 use diesel::prelude::*;
 use std::borrow::Cow;
 
 use crate::app::App;
 use crate::util::CargoResult;
 
-use crate::models::{Crate, CrateOwner, Email, NewEmail, Owner, OwnerKind, Rights};
+use crate::models::{ApiToken, Crate, CrateOwner, Email, NewEmail, Owner, OwnerKind, Rights};
 use crate::schema::{crate_owners, emails, users};
 use crate::views::{EncodablePrivateUser, EncodablePublicUser};
 
@@ -105,27 +104,14 @@ impl<'a> NewUser<'a> {
 }
 
 impl User {
-    /// Queries the database for a user with a certain `api_token` value.
-    pub fn find_by_api_token(conn: &PgConnection, token_: &str) -> QueryResult<User> {
-        use crate::schema::api_tokens::dsl::{api_tokens, last_used_at, revoked, token, user_id};
-        use diesel::update;
-
-        let tokens = api_tokens
-            .filter(token.eq(token_))
-            .filter(revoked.eq(false));
-
-        // If the database is in read only mode, we can't update last_used_at.
-        // Try updating in a new transaction, if that fails, fall back to reading
-        let user_id_ = conn
-            .transaction(|| {
-                update(tokens)
-                    .set(last_used_at.eq(now.nullable()))
-                    .returning(user_id)
-                    .get_result::<i32>(conn)
-            })
-            .or_else(|_| tokens.select(user_id).first(conn))?;
-
-        users::table.find(user_id_).first(conn)
+    pub fn find(conn: &PgConnection, id: i32) -> QueryResult<User> {
+        users::table.find(id).first(conn)
+    }
+
+    pub fn find_by_api_token(conn: &PgConnection, token: &str) -> QueryResult<User> {
+        let api_token = ApiToken::find_by_api_token(conn, token)?;
+
+        Self::find(conn, api_token.user_id)
     }
 
     pub fn owning(krate: &Crate, conn: &PgConnection) -> CargoResult<Vec<Owner>> {
diff --git a/src/tests/http-data/krate_publish_records_an_audit_action b/src/tests/http-data/krate_publish_records_an_audit_action
@@ -0,0 +1,73 @@
+[
+  {
+    "request": {
+      "uri": "http://alexcrichton-test.s3.amazonaws.com/crates/fyk/fyk-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/http-data/krate_unyank_records_an_audit_action b/src/tests/http-data/krate_unyank_records_an_audit_action
diff --git a/src/tests/http-data/krate_yank_records_an_audit_action b/src/tests/http-data/krate_yank_records_an_audit_action
diff --git a/src/tests/krate.rs b/src/tests/krate.rs

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-pub use self::action::{VersionAction, VersionOwnerAction};`
	`1`	`+pub use self::action::{NewVersionOwnerAction, VersionAction, VersionOwnerAction};`
`2`	`2`	`pub use self::badge::{Badge, CrateBadge, MaintenanceStatus};`
`3`	`3`	`pub use self::category::{Category, CrateCategory, NewCategory};`
`4`	`4`	`pub use self::crate_owner_invitation::{CrateOwnerInvitation, NewCrateOwnerInvitation};`