Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 20 additions & 9 deletions crates/handlers/src/graphql/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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,
)
Expand All @@ -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
Expand All @@ -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(
Expand All @@ -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,
)
Expand All @@ -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
Expand All @@ -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 {
Expand Down
9 changes: 9 additions & 0 deletions crates/handlers/src/graphql/mutations/browser_session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
}
}
30 changes: 30 additions & 0 deletions crates/handlers/src/graphql/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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>;
Expand All @@ -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;
}

Expand All @@ -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
}
Loading