From 77b04ef1d45b44de1dd31f821420ecc7fe2487c7 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Tue, 1 Apr 2025 16:11:54 +0200 Subject: [PATCH] Clear the session cookie on logout from the GraphQL API --- crates/handlers/src/graphql/mod.rs | 29 ++++++++++++------ .../src/graphql/mutations/browser_session.rs | 9 ++++++ crates/handlers/src/graphql/state.rs | 30 +++++++++++++++++++ 3 files changed, 59 insertions(+), 9 deletions(-) diff --git a/crates/handlers/src/graphql/mod.rs b/crates/handlers/src/graphql/mod.rs index abf8d7c4b..4c854ed21 100644 --- a/crates/handlers/src/graphql/mod.rs +++ b/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, token: Option<&str>, ) -> Result { @@ -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 --git a/crates/handlers/src/graphql/mutations/browser_session.rs b/crates/handlers/src/graphql/mutations/browser_session.rs index 8e43e5699..688e17dcb 100644 --- a/crates/handlers/src/graphql/mutations/browser_session.rs +++ b/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 --git a/crates/handlers/src/graphql/state.rs b/crates/handlers/src/graphql/state.rs index 83d50b86a..737f43340 100644 --- a/crates/handlers/src/graphql/state.rs +++ b/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; @@ -30,6 +33,8 @@ pub type BoxState = Box; 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 +}