Merge pull request #184 from sebadob/fix-proper-cache-eviction-on-use…

…r-delete impl Session::delete_by_user()
sebadob · Nov 24, 2023 · ed76418 · ed76418
2 parents 24af03c + f5e990c
commit ed76418
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 10 deletions.
diff --git a/rauthy-models/src/entity/sessions.rs b/rauthy-models/src/entity/sessions.rs
@@ -93,7 +93,7 @@ impl SessionState {
 // CRUD
 impl Session {
     pub async fn delete(&self, data: &web::Data<AppState>) -> Result<(), ErrorResponse> {
-        sqlx::query!("delete from sessions where id = $1", self.id)
+        sqlx::query!("DELETE FROM sessions WHERE id = $1", self.id)
             .execute(&data.db)
             .await?;
 
@@ -108,11 +108,35 @@ impl Session {
         Ok(())
     }
 
-    // TODO add 'delete_by_user'
+    pub async fn delete_by_user(
+        data: &web::Data<AppState>,
+        user_id: &str,
+    ) -> Result<(), ErrorResponse> {
+        let sessions: Vec<Self> =
+            sqlx::query_as!(Self, "SELECT * FROM sessions WHERE user_id = $1", user_id)
+                .fetch_all(&data.db)
+                .await?;
+
+        // we do not use any fancy 'returning' statement here for compatibility with sqlite
+        sqlx::query!("DELETE FROM sessions WHERE user_id = $1", user_id)
+            .execute(&data.db)
+            .await?;
+
+        for s in sessions {
+            cache_remove(
+                CACHE_NAME_SESSIONS.to_string(),
+                Session::cache_idx(&s.id),
+                &data.caches.ha_cache_config,
+                AckLevel::Quorum,
+            )
+            .await?;
+        }
+
+        Ok(())
+    }
 
     // Returns a session by id
     pub async fn find(data: &web::Data<AppState>, id: String) -> Result<Self, ErrorResponse> {
-        // TODO set remote lookup to true here to be able to switch to in-memory sessions store only?
         let idx = Session::cache_idx(&id);
         let session = cache_get!(
             Session,
@@ -180,7 +204,6 @@ impl Session {
         Ok(())
     }
 
-    // TODO is using old KV store logic - could be optimized
     //github.com/ Invalidates all sessions for the given user_id by setting the expiry to `now()`
     pub async fn invalidate_for_user(
         data: &web::Data<AppState>,

diff --git a/rauthy-models/src/entity/users.rs b/rauthy-models/src/entity/users.rs
@@ -139,10 +139,15 @@ impl User {
 
     // Deletes a user
     pub async fn delete(&self, data: &web::Data<AppState>) -> Result<(), ErrorResponse> {
-        sqlx::query!("delete from users where id = $1", self.id)
+        // Clean up all possibly existing sessions from the cache
+        Session::delete_by_user(data, &self.id).await?;
+
+        // Delete the user itself
+        sqlx::query!("DELETE FROM users WHERE id = $1", self.id)
             .execute(&data.db)
             .await?;
 
+        // Now clean up all caches
         let users = User::find_all(data)
             .await?
             .into_iter()
@@ -176,11 +181,6 @@ impl User {
         )
         .await?;
 
-        // This should not be a problem with a non-existent user, but delete all sessions and
-        // refresh tokens in advance.
-        Session::invalidate_for_user(data, &self.id).await?;
-        RefreshToken::invalidate_for_user(data, &self.id).await?;
-
         Ok(())
     }