Improved persistent session API.

Author: adsnaider

src/controllers/user/session.rs

@@ -13,7 +13,6 @@ use crate::models::persistent_session::SessionCookie;
 use crate::models::{NewUser, PersistentSession, User};
 use crate::schema::users;
 use crate::util::errors::ReadOnlyMode;
-use crate::util::token::SecureToken;
 use crate::Env;
 
 /// Handles the `GET /api/private/session/begin` route.
@@ -104,10 +103,7 @@ pub fn authorize(req: &mut dyn RequestExt) -> EndpointResult {
     let user = save_user_to_database(&ghuser, token.secret(), &req.app().emails, &*req.db_conn()?)?;
 
     // Setup a persistent session for the newly logged in user.
-    let token = SecureToken::generate(crate::util::token::SecureTokenKind::Session);
-    let session = PersistentSession::create(user.id, &token).insert(&*req.db_conn()?)?;
-
-    let cookie = SessionCookie::new(session.id, token.plaintext().to_string());
+    let (_session, cookie) = PersistentSession::create(user.id).insert(&*req.db_conn()?)?;
 
     // Setup persistent session cookie.
     let secure = req.app().config.env() == Env::Production;

src/models/persistent_session.rs

@@ -6,6 +6,7 @@ use std::str::FromStr;
 use thiserror::Error;
 
 use crate::schema::persistent_sessions;
+use crate::util::token::NewSecureToken;
 use crate::util::token::SecureToken;
 use crate::util::token::SecureTokenKind;
 
@@ -31,12 +32,10 @@ pub struct PersistentSession {
 }
 
 impl PersistentSession {
-    /// Creates a `NewPersistentSession` that can be inserted into the database.
-    pub fn create<'a>(user_id: i32, token: &'a SecureToken) -> NewPersistentSession<'a> {
-        NewPersistentSession {
-            user_id,
-            hashed_token: token,
-        }
+    /// Creates a `NewPersistentSession` for the `user_id` and the token associated with it.
+    pub fn create<'a>(user_id: i32) -> NewPersistentSession {
+        let token = SecureToken::generate(SecureTokenKind::Session);
+        NewPersistentSession { user_id, token }
     }
 
     /// Finds the session with the ID.
@@ -81,19 +80,32 @@ impl PersistentSession {
 }
 
 /// A new, insertable persistent session.
-#[derive(Clone, Debug, PartialEq, Eq, Insertable)]
-#[table_name = "persistent_sessions"]
-pub struct NewPersistentSession<'a> {
+pub struct NewPersistentSession {
     user_id: i32,
-    hashed_token: &'a SecureToken,
+    token: NewSecureToken,
 }
 
-impl NewPersistentSession<'_> {
+impl NewPersistentSession {
     /// Inserts the session into the database.
-    pub fn insert(self, conn: &PgConnection) -> Result<PersistentSession, diesel::result::Error> {
-        diesel::insert_into(persistent_sessions::table)
-            .values(self)
-            .get_result(conn)
+    ///
+    /// # Returns
+    ///
+    /// The
+    pub fn insert(
+        self,
+        conn: &PgConnection,
+    ) -> Result<(PersistentSession, SessionCookie), diesel::result::Error> {
+        let session: PersistentSession = diesel::insert_into(persistent_sessions::table)
+            .values((
+                persistent_sessions::user_id.eq(&self.user_id),
+                persistent_sessions::hashed_token.eq(&*self.token),
+            ))
+            .get_result(conn)?;
+        let id = session.id;
+        Ok((
+            session,
+            SessionCookie::new(id, self.token.plaintext().to_string()),
+        ))
     }
 }
 

src/tests/authentication.rs

@@ -3,8 +3,6 @@ use crate::TestApp;
 
 use crate::util::encode_session_header;
 use cargo_registry::models::persistent_session::SessionCookie;
-use cargo_registry::util::token::SecureToken;
-use cargo_registry::util::token::SecureTokenKind;
 use conduit::{header, Body, Method, StatusCode};
 
 static URL: &str = "/api/v1/me/updates";
@@ -44,11 +42,9 @@ fn persistent_session_revoked_after_logout() {
 fn incorrect_session_is_forbidden() {
     let (_, anon) = TestApp::init().empty();
 
-    let token = SecureToken::generate(SecureTokenKind::Session);
+    let token = "scio:1234:abcdfdfdsafdsfd".to_string();
     // Create a cookie that isn't in the database.
-    let cookie = SessionCookie::new(123, token.plaintext().to_string())
-        .build(false)
-        .to_string();
+    let cookie = SessionCookie::new(123, token).build(false).to_string();
     let mut request = anon.request_builder(Method::GET, URL);
     request.header(header::COOKIE, &cookie);
     let response: Response<Body> = anon.run(request);

src/tests/util.rs

@@ -26,8 +26,6 @@ use crate::{
 use cargo_registry::models::persistent_session::SessionCookie;
 use cargo_registry::models::PersistentSession;
 use cargo_registry::models::{ApiToken, CreatedApiToken, User};
-use cargo_registry::util::token::SecureToken;
-use cargo_registry::util::token::SecureTokenKind;
 
 use conduit::{BoxError, Handler, Method};
 use conduit_cookie::SessionMiddleware;
@@ -276,17 +274,15 @@ impl MockCookieUser {
     }
 
     pub fn with_session(&self) -> MockSessionUser {
-        let token = SecureToken::generate(SecureTokenKind::Session);
-
-        let session = self.app.db(|conn| {
-            PersistentSession::create(self.user.id, &token)
+        let (_session, session_cookie) = self.app.db(|conn| {
+            PersistentSession::create(self.user.id)
                 .insert(conn)
                 .unwrap()
         });
 
         MockSessionUser {
             app: self.app.clone(),
-            session_cookie: SessionCookie::new(session.id, token.plaintext().to_string()),
+            session_cookie,
         }
     }
 }

src/util.rs

@@ -12,7 +12,7 @@ mod io_util;
 mod request_helpers;
 mod request_proxy;
 pub mod rfc3339;
-pub mod token;
+pub(crate) mod token;
 
 pub type AppResponse = Response<conduit::Body>;
 pub type EndpointResult = Result<AppResponse, Box<dyn errors::AppError>>;

src/util/token.rs

@@ -12,7 +12,7 @@ pub struct SecureToken {
 }
 
 impl SecureToken {
-    pub fn generate(kind: SecureTokenKind) -> NewSecureToken {
+    pub(crate) fn generate(kind: SecureTokenKind) -> NewSecureToken {
         let plaintext = format!(
             "{}{}",
             kind.prefix(),
@@ -26,7 +26,7 @@ impl SecureToken {
         }
     }
 
-    pub fn parse(kind: SecureTokenKind, plaintext: &str) -> Option<Self> {
+    pub(crate) fn parse(kind: SecureTokenKind, plaintext: &str) -> Option<Self> {
         // This will both reject tokens without a prefix and tokens of the wrong kind.
         if SecureTokenKind::from_token(plaintext) != Some(kind) {
             return None;
@@ -60,18 +60,18 @@ impl FromSql<Bytea, Pg> for SecureToken {
     }
 }
 
-pub struct NewSecureToken {
+pub(crate) struct NewSecureToken {
     plaintext: String,
     inner: SecureToken,
 }
 
 impl NewSecureToken {
-    pub fn plaintext(&self) -> &str {
+    pub(crate) fn plaintext(&self) -> &str {
         &self.plaintext
     }
 
     #[cfg(test)]
-    pub fn into_inner(self) -> SecureToken {
+    pub(crate) fn into_inner(self) -> SecureToken {
         self.inner
     }
 }
@@ -120,7 +120,7 @@ secure_token_kind! {
     /// NEVER CHANGE THE PREFIX OF EXISTING TOKEN TYPES!!! Doing so will implicitly revoke all the
     /// tokens of that kind, distrupting production users.
     #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
-    pub enum SecureTokenKind {
+    pub(crate) enum SecureTokenKind {
         Api => "cio", // Crates.IO
         Session => "scio", // Session tokens.
     }