Merge pull request #10557 from Turbo87/new-token

models/token: Extract `NewApiToken` struct

Author: Turbo87

src/controllers/token.rs

@@ -7,6 +7,7 @@ use crate::app::AppState;
 use crate::auth::AuthCheck;
 use crate::models::token::{CrateScope, EndpointScope};
 use crate::util::errors::{bad_request, AppResult};
+use crate::util::token::PlainToken;
 use axum::extract::{Path, Query};
 use axum::response::{IntoResponse, Response};
 use axum::Json;
@@ -19,6 +20,7 @@ use diesel::prelude::*;
 use diesel_async::RunQueryDsl;
 use http::request::Parts;
 use http::StatusCode;
+use secrecy::ExposeSecret;
 
 #[derive(Deserialize)]
 pub struct GetParams {
@@ -148,15 +150,16 @@ pub async fn create_api_token(
 
     let recipient = user.email(&mut conn).await?;
 
-    let api_token = ApiToken::insert_with_scopes(
-        &mut conn,
-        user.id,
-        &new.api_token.name,
-        crate_scopes,
-        endpoint_scopes,
-        new.api_token.expired_at,
-    )
-    .await?;
+    let plaintext = PlainToken::generate();
+
+    let new_token = crate::models::token::NewApiToken::builder()
+        .user_id(user.id)
+        .name(&new.api_token.name)
+        .token(plaintext.hashed())
+        .maybe_crate_scopes(crate_scopes)
+        .maybe_endpoint_scopes(endpoint_scopes)
+        .maybe_expired_at(new.api_token.expired_at)
+        .build();
 
     if let Some(recipient) = recipient {
         let email = NewTokenEmail {
@@ -174,7 +177,10 @@ pub async fn create_api_token(
         }
     }
 
-    let api_token = EncodableApiTokenWithToken::from(api_token);
+    let api_token = EncodableApiTokenWithToken {
+        token: new_token.insert(&mut conn).await?,
+        plaintext: plaintext.expose_secret().to_string(),
+    };
 
     Ok(json!({ "api_token": api_token }))
 }

src/models.rs

@@ -14,7 +14,7 @@ pub use self::krate::{Crate, CrateName, NewCrate, RecentCrateDownloads};
 pub use self::owner::{CrateOwner, Owner, OwnerKind};
 pub use self::rights::Rights;
 pub use self::team::{NewTeam, Team};
-pub use self::token::{ApiToken, CreatedApiToken};
+pub use self::token::ApiToken;
 pub use self::user::{NewUser, User};
 pub use self::version::{NewVersion, TopVersions, Version};
 

src/models/token.rs

@@ -1,5 +1,6 @@
 mod scopes;
 
+use bon::Builder;
 use chrono::NaiveDateTime;
 use diesel::dsl::now;
 use diesel::prelude::*;
@@ -12,6 +13,31 @@ use crate::schema::api_tokens;
 use crate::util::rfc3339;
 use crate::util::token::{HashedToken, PlainToken};
 
+#[derive(Debug, Insertable, Builder)]
+#[diesel(table_name = api_tokens, check_for_backend(diesel::pg::Pg))]
+pub struct NewApiToken {
+    pub user_id: i32,
+    #[builder(into)]
+    pub name: String,
+    #[builder(default = PlainToken::generate().hashed())]
+    pub token: HashedToken,
+    /// `None` or a list of crate scope patterns (see RFC #2947)
+    pub crate_scopes: Option<Vec<CrateScope>>,
+    /// A list of endpoint scopes or `None` for the `legacy` endpoint scope (see RFC #2947)
+    pub endpoint_scopes: Option<Vec<EndpointScope>>,
+    pub expired_at: Option<NaiveDateTime>,
+}
+
+impl NewApiToken {
+    pub async fn insert(&self, conn: &mut AsyncPgConnection) -> QueryResult<ApiToken> {
+        diesel::insert_into(api_tokens::table)
+            .values(self)
+            .returning(ApiToken::as_returning())
+            .get_result(conn)
+            .await
+    }
+}
+
 /// The model representing a row in the `api_tokens` database table.
 #[derive(Debug, Identifiable, Queryable, Selectable, Associations, Serialize)]
 #[diesel(belongs_to(User))]
@@ -35,44 +61,6 @@ pub struct ApiToken {
 }
 
 impl ApiToken {
-    /// Generates a new named API token for a user
-    pub async fn insert(
-        conn: &mut AsyncPgConnection,
-        user_id: i32,
-        name: &str,
-    ) -> QueryResult<CreatedApiToken> {
-        Self::insert_with_scopes(conn, user_id, name, None, None, None).await
-    }
-
-    pub async fn insert_with_scopes(
-        conn: &mut AsyncPgConnection,
-        user_id: i32,
-        name: &str,
-        crate_scopes: Option<Vec<CrateScope>>,
-        endpoint_scopes: Option<Vec<EndpointScope>>,
-        expired_at: Option<NaiveDateTime>,
-    ) -> QueryResult<CreatedApiToken> {
-        let token = PlainToken::generate();
-
-        let model: ApiToken = diesel::insert_into(api_tokens::table)
-            .values((
-                api_tokens::user_id.eq(user_id),
-                api_tokens::name.eq(name),
-                api_tokens::token.eq(token.hashed()),
-                api_tokens::crate_scopes.eq(crate_scopes),
-                api_tokens::endpoint_scopes.eq(endpoint_scopes),
-                api_tokens::expired_at.eq(expired_at),
-            ))
-            .returning(ApiToken::as_returning())
-            .get_result(conn)
-            .await?;
-
-        Ok(CreatedApiToken {
-            plaintext: token,
-            model,
-        })
-    }
-
     pub async fn find_by_api_token(
         conn: &mut AsyncPgConnection,
         token: &HashedToken,
@@ -107,12 +95,6 @@ impl ApiToken {
     }
 }
 
-#[derive(Debug)]
-pub struct CreatedApiToken {
-    pub model: ApiToken,
-    pub plaintext: PlainToken,
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

src/tests/routes/me/tokens/create.rs

@@ -1,4 +1,4 @@
-use crate::models::token::{CrateScope, EndpointScope};
+use crate::models::token::{CrateScope, EndpointScope, NewApiToken};
 use crate::models::ApiToken;
 use crate::tests::util::insta::{self, assert_json_snapshot};
 use crate::tests::util::{RequestHelper, TestApp};
@@ -46,7 +46,9 @@ async fn create_token_exceeded_tokens_per_user() {
     let id = user.as_model().id;
 
     for i in 0..1000 {
-        assert_ok!(ApiToken::insert(&mut conn, id, &format!("token {i}")).await);
+        let name = format!("token {i}");
+        let new_token = NewApiToken::builder().name(name).user_id(id).build();
+        assert_ok!(new_token.insert(&mut conn).await);
     }
 
     let response = user.put::<()>("/api/v1/me/tokens", NEW_BAR).await;

src/tests/routes/me/tokens/get.rs

@@ -1,5 +1,4 @@
-use crate::models::token::{CrateScope, EndpointScope};
-use crate::models::ApiToken;
+use crate::models::token::{CrateScope, EndpointScope, NewApiToken};
 use crate::tests::util::{RequestHelper, TestApp};
 use chrono::{Duration, Utc};
 use http::StatusCode;
@@ -31,23 +30,22 @@ async fn show_token_with_scopes() {
     let user_model = user.as_model();
     let id = user_model.id;
 
-    assert_ok!(ApiToken::insert(&mut conn, id, "bar").await);
-    let token = assert_ok!(
-        ApiToken::insert_with_scopes(
-            &mut conn,
-            id,
-            "baz",
-            Some(vec![
-                CrateScope::try_from("serde").unwrap(),
-                CrateScope::try_from("serde-*").unwrap()
-            ]),
-            Some(vec![EndpointScope::PublishUpdate]),
-            Some((Utc::now() - Duration::days(31)).naive_utc()),
-        )
-        .await
-    );
+    let new_token = NewApiToken::builder().name("bar").user_id(id).build();
+    assert_ok!(new_token.insert(&mut conn).await);
 
-    let url = format!("/api/v1/me/tokens/{}", token.model.id);
+    let new_token = NewApiToken::builder()
+        .name("baz")
+        .user_id(id)
+        .crate_scopes(vec![
+            CrateScope::try_from("serde").unwrap(),
+            CrateScope::try_from("serde-*").unwrap(),
+        ])
+        .endpoint_scopes(vec![EndpointScope::PublishUpdate])
+        .expired_at((Utc::now() - Duration::days(31)).naive_utc())
+        .build();
+    let token = assert_ok!(new_token.insert(&mut conn).await);
+
+    let url = format!("/api/v1/me/tokens/{}", token.id);
     let response = user.get::<()>(&url).await;
     assert_eq!(response.status(), StatusCode::OK);
     assert_json_snapshot!(response.json(), {
@@ -70,9 +68,10 @@ async fn show_other_user_token() {
     let user2 = app.db_new_user("baz").await;
     let user2 = user2.as_model();
 
-    let token = assert_ok!(ApiToken::insert(&mut conn, user2.id, "bar").await);
+    let new_token = NewApiToken::builder().name("bar").user_id(user2.id).build();
+    let token = assert_ok!(new_token.insert(&mut conn).await);
 
-    let url = format!("/api/v1/me/tokens/{}", token.model.id);
+    let url = format!("/api/v1/me/tokens/{}", token.id);
     let response = user1.get::<()>(&url).await;
     assert_eq!(response.status(), StatusCode::NOT_FOUND);
 }

src/tests/routes/me/tokens/list.rs

@@ -1,5 +1,4 @@
-use crate::models::token::{CrateScope, EndpointScope};
-use crate::models::ApiToken;
+use crate::models::token::{CrateScope, EndpointScope, NewApiToken};
 use crate::tests::util::insta::{self, assert_json_snapshot};
 use crate::tests::util::{RequestHelper, TestApp};
 use chrono::{Duration, Utc};
@@ -33,32 +32,26 @@ async fn list_tokens() {
     let mut conn = app.db_conn().await;
     let id = user.as_model().id;
 
-    assert_ok!(ApiToken::insert(&mut conn, id, "bar").await);
-    assert_ok!(
-        ApiToken::insert_with_scopes(
-            &mut conn,
-            id,
-            "baz",
-            Some(vec![
-                CrateScope::try_from("serde").unwrap(),
-                CrateScope::try_from("serde-*").unwrap()
-            ]),
-            Some(vec![EndpointScope::PublishUpdate]),
-            None
-        )
-        .await
-    );
-    assert_ok!(
-        ApiToken::insert_with_scopes(
-            &mut conn,
-            id,
-            "qux",
-            None,
-            None,
-            Some((Utc::now() - Duration::days(1)).naive_utc()),
-        )
-        .await
-    );
+    let new_token = NewApiToken::builder().name("bar").user_id(id).build();
+    assert_ok!(new_token.insert(&mut conn).await);
+
+    let new_token = NewApiToken::builder()
+        .name("baz")
+        .user_id(id)
+        .crate_scopes(vec![
+            CrateScope::try_from("serde").unwrap(),
+            CrateScope::try_from("serde-*").unwrap(),
+        ])
+        .endpoint_scopes(vec![EndpointScope::PublishUpdate])
+        .build();
+    assert_ok!(new_token.insert(&mut conn).await);
+
+    let new_token = NewApiToken::builder()
+        .name("qux")
+        .user_id(id)
+        .expired_at((Utc::now() - Duration::days(1)).naive_utc())
+        .build();
+    assert_ok!(new_token.insert(&mut conn).await);
 
     let response = user.get::<()>("/api/v1/me/tokens").await;
     assert_eq!(response.status(), StatusCode::OK);
@@ -80,32 +73,27 @@ async fn list_recently_expired_tokens() {
     let mut conn = app.db_conn().await;
     let id = user.as_model().id;
 
-    assert_ok!(ApiToken::insert(&mut conn, id, "bar").await);
-    assert_ok!(
-        ApiToken::insert_with_scopes(
-            &mut conn,
-            id,
-            "ancient",
-            Some(vec![
-                CrateScope::try_from("serde").unwrap(),
-                CrateScope::try_from("serde-*").unwrap()
-            ]),
-            Some(vec![EndpointScope::PublishUpdate]),
-            Some((Utc::now() - Duration::days(31)).naive_utc()),
-        )
-        .await
-    );
-    assert_ok!(
-        ApiToken::insert_with_scopes(
-            &mut conn,
-            id,
-            "recent",
-            None,
-            None,
-            Some((Utc::now() - Duration::days(1)).naive_utc()),
-        )
-        .await
-    );
+    let new_token = NewApiToken::builder().name("bar").user_id(id).build();
+    assert_ok!(new_token.insert(&mut conn).await);
+
+    let new_token = NewApiToken::builder()
+        .name("ancient")
+        .user_id(id)
+        .crate_scopes(vec![
+            CrateScope::try_from("serde").unwrap(),
+            CrateScope::try_from("serde-*").unwrap(),
+        ])
+        .endpoint_scopes(vec![EndpointScope::PublishUpdate])
+        .expired_at((Utc::now() - Duration::days(31)).naive_utc())
+        .build();
+    assert_ok!(new_token.insert(&mut conn).await);
+
+    let new_token = NewApiToken::builder()
+        .name("recent")
+        .user_id(id)
+        .expired_at((Utc::now() - Duration::days(1)).naive_utc())
+        .build();
+    assert_ok!(new_token.insert(&mut conn).await);
 
     let response = user.get::<()>("/api/v1/me/tokens?expired_days=30").await;
     assert_eq!(response.status(), StatusCode::OK);
@@ -131,8 +119,11 @@ async fn list_tokens_exclude_revoked() {
     let mut conn = app.db_conn().await;
     let id = user.as_model().id;
 
-    let token1 = assert_ok!(ApiToken::insert(&mut conn, id, "bar").await);
-    assert_ok!(ApiToken::insert(&mut conn, id, "baz").await);
+    let new_token = NewApiToken::builder().name("bar").user_id(id).build();
+    let token1 = assert_ok!(new_token.insert(&mut conn).await);
+
+    let new_token = NewApiToken::builder().name("baz").user_id(id).build();
+    assert_ok!(new_token.insert(&mut conn).await);
 
     // List tokens expecting them all to be there.
     let response = user.get::<()>("/api/v1/me/tokens").await;
@@ -143,7 +134,7 @@ async fn list_tokens_exclude_revoked() {
 
     // Revoke the first token.
     let response = user
-        .delete::<()>(&format!("/api/v1/me/tokens/{}", token1.model.id))
+        .delete::<()>(&format!("/api/v1/me/tokens/{}", token1.id))
         .await;
     assert_eq!(response.status(), StatusCode::OK);