Clear the session cookie on logout from the GraphQL API (#4328)

Author: sandhose

Diff for: crates/handlers/src/graphql/mod.rs

@@ -38,6 +38,7 @@ use opentelemetry_semantic_conventions::trace::{GRAPHQL_DOCUMENT, GRAPHQL_OPERAT
 use rand::{SeedableRng, thread_rng};
 use rand_chacha::ChaChaRng;
 use sqlx::PgPool;
+use state::has_session_ended;
 use tracing::{Instrument, info_span};
 use ulid::Ulid;
 
@@ -237,7 +238,7 @@ async fn get_requester(
     clock: &impl Clock,
     activity_tracker: &BoundActivityTracker,
     mut repo: BoxRepository,
-    session_info: SessionInfo,
+    session_info: &SessionInfo,
     user_agent: Option<String>,
     token: Option<&str>,
 ) -> Result<Requester, RouteError> {
@@ -328,13 +329,13 @@ pub async fn post(
         .as_ref()
         .map(|TypedHeader(Authorization(bearer))| bearer.token());
     let user_agent = user_agent.map(|TypedHeader(h)| h.to_string());
-    let (session_info, _cookie_jar) = cookie_jar.session_info();
+    let (session_info, mut cookie_jar) = cookie_jar.session_info();
     let requester = get_requester(
         undocumented_oauth2_access,
         &clock,
         &activity_tracker,
         repo,
-        session_info,
+        &session_info,
         user_agent,
         token,
     )
@@ -352,7 +353,12 @@ pub async fn post(
     .data(requester); // XXX: this should probably return another error response?
 
     let span = span_for_graphql_request(&request);
-    let response = schema.execute(request).instrument(span).await;
+    let mut response = schema.execute(request).instrument(span).await;
+
+    if has_session_ended(&mut response) {
+        let session_info = session_info.mark_session_ended();
+        cookie_jar = cookie_jar.update_session_info(&session_info);
+    }
 
     let cache_control = response
         .cache_control
@@ -362,7 +368,7 @@ pub async fn post(
 
     let headers = response.http_headers.clone();
 
-    Ok((headers, cache_control, Json(response)))
+    Ok((headers, cache_control, cookie_jar, Json(response)))
 }
 
 pub async fn get(
@@ -382,13 +388,13 @@ pub async fn get(
         .as_ref()
         .map(|TypedHeader(Authorization(bearer))| bearer.token());
     let user_agent = user_agent.map(|TypedHeader(h)| h.to_string());
-    let (session_info, _cookie_jar) = cookie_jar.session_info();
+    let (session_info, mut cookie_jar) = cookie_jar.session_info();
     let requester = get_requester(
         undocumented_oauth2_access,
         &clock,
         &activity_tracker,
         repo,
-        session_info,
+        &session_info,
         user_agent,
         token,
     )
@@ -398,7 +404,12 @@ pub async fn get(
         async_graphql::http::parse_query_string(&query.unwrap_or_default())?.data(requester);
 
     let span = span_for_graphql_request(&request);
-    let response = schema.execute(request).instrument(span).await;
+    let mut response = schema.execute(request).instrument(span).await;
+
+    if has_session_ended(&mut response) {
+        let session_info = session_info.mark_session_ended();
+        cookie_jar = cookie_jar.update_session_info(&session_info);
+    }
 
     let cache_control = response
         .cache_control
@@ -408,7 +419,7 @@ pub async fn get(
 
     let headers = response.http_headers.clone();
 
-    Ok((headers, cache_control, Json(response)))
+    Ok((headers, cache_control, cookie_jar, Json(response)))
 }
 
 pub async fn playground() -> impl IntoResponse {

Diff for: crates/handlers/src/graphql/mutations/browser_session.rs

@@ -88,6 +88,15 @@ impl BrowserSessionMutations {
 
         repo.save().await?;
 
+        // If we are ending the *current* session, we need to clear the session cookie
+        // as well
+        if requester
+            .browser_session()
+            .is_some_and(|s| s.id == session.id)
+        {
+            ctx.mark_session_ended();
+        }
+
         Ok(EndBrowserSessionPayload::Ended(Box::new(session)))
     }
 }

Diff for: crates/handlers/src/graphql/state.rs

@@ -4,6 +4,7 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 // Please see LICENSE in the repository root for full details.
 
+use async_graphql::{Response, ServerError};
 use mas_data_model::SiteConfig;
 use mas_matrix::HomeserverConnection;
 use mas_policy::Policy;
@@ -12,6 +13,8 @@ use mas_storage::{BoxClock, BoxRepository, BoxRng, RepositoryError};
 
 use crate::{Limiter, graphql::Requester, passwords::PasswordManager};
 
+const CLEAR_SESSION_SENTINEL: &str = "__CLEAR_SESSION__";
+
 #[async_trait::async_trait]
 pub trait State {
     async fn repository(&self) -> Result<BoxRepository, RepositoryError>;
@@ -30,6 +33,8 @@ pub type BoxState = Box<dyn State + Send + Sync + 'static>;
 pub trait ContextExt {
     fn state(&self) -> &BoxState;
 
+    fn mark_session_ended(&self);
+
     fn requester(&self) -> &Requester;
 }
 
@@ -38,7 +43,32 @@ impl ContextExt for async_graphql::Context<'_> {
         self.data_unchecked()
     }
 
+    fn mark_session_ended(&self) {
+        // Add a sentinel to the error context, so that we can know that we need to
+        // clear the session
+        // XXX: this is a bit of a hack, but the only sane way to get infos from within
+        // a mutation up to the HTTP handler
+        self.add_error(ServerError::new(CLEAR_SESSION_SENTINEL, None));
+    }
+
     fn requester(&self) -> &Requester {
         self.data_unchecked()
     }
 }
+
+/// Returns true if the response contains a sentinel error indicating that the
+/// current cookie session has ended, and the session cookie should be cleared.
+///
+/// Also removes the sentinel error from the response.
+pub fn has_session_ended(response: &mut Response) -> bool {
+    let errors = std::mem::take(&mut response.errors);
+    let mut must_clear_session = false;
+    for error in errors {
+        if error.message == CLEAR_SESSION_SENTINEL {
+            must_clear_session = true;
+        } else {
+            response.errors.push(error);
+        }
+    }
+    must_clear_session
+}