Merge branch 'main' into ft/auth_testing

Author: ftheirs

backend/lib/src/data/storage/boxed.rs

@@ -4,7 +4,7 @@ use std::error::Error as StdError;
 
 use async_trait::async_trait;
 
-use super::Storage;
+use super::{Storage, WithExpiry};
 
 /// A boxed storage error that can wrap any storage implementation's error type
 pub type BoxedStorageError = Box<dyn StdError + Send + Sync>;
@@ -21,7 +21,7 @@ pub trait BoxedStorage: Send + Sync {
         expiration_seconds: u64,
     ) -> Result<(), BoxedStorageError>;
 
-    async fn get_nonce(&self, message: &str) -> Result<Option<String>, BoxedStorageError>;
+    async fn get_nonce(&self, message: &str) -> Result<WithExpiry<String>, BoxedStorageError>;
 }
 
 /// Wrapper struct that implements BoxedStorage for any Storage implementation
@@ -63,7 +63,7 @@ where
             .map_err(Self::wrap_err)
     }
 
-    async fn get_nonce(&self, message: &str) -> Result<Option<String>, BoxedStorageError> {
+    async fn get_nonce(&self, message: &str) -> Result<WithExpiry<String>, BoxedStorageError> {
         self.inner.get_nonce(message).await.map_err(Self::wrap_err)
     }
 }

backend/lib/src/data/storage/memory.rs

@@ -24,7 +24,7 @@ use tokio::{
 };
 use tracing::warn;
 
-use super::Storage;
+use super::{Storage, WithExpiry};
 
 /// Errors that can occur during in-memory storage operations
 #[derive(Debug, Error)]
@@ -38,8 +38,17 @@ pub enum InMemoryStorageError {
 struct NonceEntry {
     /// The user address associated with the nonce key
     address: String,
-    /// Timestamp when the nonce will expire from storage
-    expires_at: Instant,
+    /// Timestamp when the nonce was issued
+    issued_at: Instant,
+    /// Duration from `issued_at` when the nonce will expire from storage
+    expiry: Duration,
+}
+
+impl NonceEntry {
+    /// Checks if the entry has expired by the given `at` timestamp
+    fn expired_at(&self, at: Instant) -> bool {
+        at.saturating_duration_since(self.issued_at) >= self.expiry
+    }
 }
 
 /// In-memory storage implementation
@@ -97,7 +106,7 @@ impl InMemoryStorage {
                 let now = Instant::now();
 
                 let mut nonces_guard = nonces.write();
-                nonces_guard.retain(|_, entry| entry.expires_at > now);
+                nonces_guard.retain(|_, entry| !entry.expired_at(now));
             }
         });
 
@@ -138,30 +147,35 @@ impl Storage for InMemoryStorage {
         address: String,
         expiration_seconds: u64,
     ) -> Result<(), Self::Error> {
-        let now = Instant::now();
-        let expires_at = now + Duration::from_secs(expiration_seconds);
+        let issued_at = Instant::now();
+        let expiry = Duration::from_secs(expiration_seconds);
 
         let entry = NonceEntry {
             address,
-            expires_at,
+            issued_at,
+            expiry,
         };
 
         self.nonces.write().insert(message, entry);
         Ok(())
     }
 
-    async fn get_nonce(&self, message: &str) -> Result<Option<String>, Self::Error> {
+    async fn get_nonce(&self, message: &str) -> Result<WithExpiry<String>, Self::Error> {
         let mut nonces = self.nonces.write();
 
         if let Some(entry) = nonces.remove(message) {
             let now = Instant::now();
 
-            if now < entry.expires_at {
-                return Ok(Some(entry.address));
-            }
+            let value = if !entry.expired_at(now) {
+                WithExpiry::Valid(entry.address)
+            } else {
+                WithExpiry::Expired
+            };
+
+            return Ok(value);
         }
 
-        Ok(None)
+        Ok(WithExpiry::NotFound)
     }
 }
 
@@ -192,11 +206,11 @@ mod tests {
 
         // Retrieve nonce
         let retrieved = storage.get_nonce(message).await.unwrap();
-        assert_eq!(retrieved, Some(address.to_string()));
+        assert_eq!(retrieved, WithExpiry::Valid(address.to_string()));
 
         // Verify it can't be retrieved twice
         let retrieved_again = storage.get_nonce(message).await.unwrap();
-        assert_eq!(retrieved_again, None);
+        assert_eq!(retrieved_again, WithExpiry::NotFound);
     }
 
     #[tokio::test]
@@ -214,7 +228,7 @@ mod tests {
 
         // Try to retrieve - should be None since it's expired
         let retrieved = storage.get_nonce(message).await.unwrap();
-        assert_eq!(retrieved, None);
+        assert_eq!(retrieved, WithExpiry::Expired);
     }
 
     #[tokio::test(start_paused = true)]
@@ -230,21 +244,25 @@ mod tests {
             .await
             .unwrap();
 
-        // Should be retrievable immediately
-        let retrieved = storage.get_nonce(message).await.unwrap();
-        assert_eq!(retrieved, Some(address.to_string()));
-
         // Advance time by 2 seconds to expire the nonce
         advance(Duration::from_secs(2)).await;
 
-        // Should return None since it's expired
-        let retrieved_after_expiry = storage.get_nonce(message).await.unwrap();
-        assert_eq!(retrieved_after_expiry, None);
+        // At this point the nonce should be expired, but it should still be in storage
+        {
+            let guard = storage.nonces.read();
+            let expired_entry = guard.get(message).expect("should still be present");
+            assert!(
+                expired_entry.expired_at(Instant::now()),
+                "Nonce should be expired by now"
+            );
+        }
 
         // Advance time to trigger cleanup task (runs every 10 seconds)
         advance(Duration::from_secs(10)).await;
 
-        // Should be gone from storage after cleanup task runs
+        // Should have been cleaned up
+        let retrieved_after_cleanup = storage.get_nonce(message).await.unwrap();
+        assert_eq!(retrieved_after_cleanup, WithExpiry::NotFound);
         assert!(storage.nonces.read().get(message).is_none());
     }
 }

backend/lib/src/data/storage/mod.rs

@@ -10,6 +10,17 @@ pub mod memory;
 pub use boxed::{BoxedStorage, BoxedStorageWrapper};
 pub use memory::InMemoryStorage;
 
+/// Represents the various possibilities of retrieval for an expirable value
+#[derive(Debug, PartialEq, Eq)]
+pub enum WithExpiry<T> {
+    /// The value was found and is still valid
+    Valid(T),
+    /// The value was found but has expired
+    Expired,
+    /// The value was not found
+    NotFound,
+}
+
 /// Storage trait for backend-specific data operations
 #[async_trait]
 pub trait Storage: Send + Sync {
@@ -29,7 +40,5 @@ pub trait Storage: Send + Sync {
     ) -> Result<(), Self::Error>;
 
     /// Retrieve nonce data by message. Will remove the nonce from storage.
-    ///
-    /// Returns None if not found or expired
-    async fn get_nonce(&self, message: &str) -> Result<Option<String>, Self::Error>;
+    async fn get_nonce(&self, message: &str) -> Result<WithExpiry<String>, Self::Error>;
 }

backend/lib/src/services/auth.rs

@@ -23,7 +23,7 @@ use crate::{
         },
         mocks::MOCK_ADDRESS,
     },
-    data::storage::BoxedStorage,
+    data::storage::{BoxedStorage, WithExpiry},
     error::Error,
     models::auth::{JwtClaims, NonceResponse, TokenResponse, User, VerifyResponse},
     services::Services,
@@ -202,8 +202,12 @@ impl AuthService {
             .storage
             .get_nonce(message)
             .await
-            .map_err(|_| Error::Internal)?
-            .ok_or_else(|| Error::Unauthorized("Invalid or expired nonce".to_string()))?;
+            .map_err(|_| Error::Internal)
+            .and_then(|entry| match entry {
+                WithExpiry::Valid(address) => Ok(address),
+                WithExpiry::Expired => Err(Error::Unauthorized("Expired nonce".to_string())),
+                WithExpiry::NotFound => Err(Error::Unauthorized("Invalid nonce".to_string())),
+            })?;
 
         if self.validate_signature {
             let recovered_address = Self::recover_eth_address_from_sig(message, signature)?;
@@ -386,6 +390,8 @@ where
 
 #[cfg(test)]
 mod tests {
+    use std::time::Duration;
+
     use jsonwebtoken::{decode, Algorithm, Validation};
 
     use crate::{
@@ -477,7 +483,7 @@ mod tests {
 
         // Check that message was stored in storage
         let stored_address = storage.get_nonce(&result.message).await.unwrap();
-        assert_eq!(stored_address, Some(MOCK_ADDRESS.to_string()));
+        assert_eq!(stored_address, WithExpiry::Valid(MOCK_ADDRESS.to_string()));
     }
 
     #[test]
@@ -528,11 +534,30 @@ mod tests {
         let result = auth_service.login(message, &sig_str).await;
         assert!(result.is_err(), "Should fail if challenge wasn't called");
         match result {
-            Err(Error::Unauthorized(msg)) => assert!(msg.contains("Invalid or expired nonce")),
+            Err(Error::Unauthorized(msg)) => assert!(msg.contains("Invalid nonce")),
             _ => panic!("Expected unauthorized error for missing nonce"),
         }
     }
 
+    #[tokio::test(start_paused = true)]
+    async fn login_fails_after_expiry() {
+        let (auth_service, _, _) = create_test_auth_service(true);
+        let (address, sk) = eth_wallet();
+
+        let challenge = auth_service.challenge(&address, 1).await.unwrap();
+        let sig_str = sign_message(&sk, &challenge.message);
+
+        // Advance time to expire the nonce
+        tokio::time::advance(Duration::from_secs(AUTH_NONCE_EXPIRATION_SECONDS + 1)).await;
+
+        let result = auth_service.login(&challenge.message, &sig_str).await;
+        assert!(result.is_err(), "Should fail if nonce has expired");
+        match result {
+            Err(Error::Unauthorized(msg)) => assert!(msg.contains("Expired nonce")),
+            _ => panic!("Expected unauthorized error for expired nonce"),
+        }
+    }
+
     #[tokio::test]
     async fn login_rejects_invalid_signature_when_validation_enabled() {
         let (auth_service, _, _) = create_test_auth_service(true);