Skip to content
This repository has been archived by the owner on Sep 10, 2024. It is now read-only.

Commit

Permalink
Disallow OAuth 2.0 use of the GraphQL API by default
Browse files Browse the repository at this point in the history
  • Loading branch information
sandhose committed Aug 7, 2024
1 parent eb4072f commit 1bdad26
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 14 deletions.
10 changes: 7 additions & 3 deletions crates/cli/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,13 @@ pub fn build_router(
mas_config::HttpResource::Human => {
router.merge(mas_handlers::human_router::<AppState>(templates.clone()))
}
mas_config::HttpResource::GraphQL { playground } => {
router.merge(mas_handlers::graphql_router::<AppState>(*playground))
}
mas_config::HttpResource::GraphQL {
playground,
undocumented_oauth2_access,
} => router.merge(mas_handlers::graphql_router::<AppState>(
*playground,
*undocumented_oauth2_access,
)),
mas_config::HttpResource::Assets { path } => {
let static_service = ServeDir::new(path)
.append_index_html_on_directories(false)
Expand Down
11 changes: 9 additions & 2 deletions crates/config/src/sections/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,12 @@ pub enum Resource {
/// GraphQL endpoint
GraphQL {
/// Enabled the GraphQL playground
#[serde(default)]
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
playground: bool,

/// Allow access for OAuth 2.0 clients (undocumented)
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
undocumented_oauth2_access: bool,
},

/// OAuth-related APIs
Expand Down Expand Up @@ -379,7 +383,10 @@ impl Default for HttpConfig {
Resource::Human,
Resource::OAuth,
Resource::Compat,
Resource::GraphQL { playground: true },
Resource::GraphQL {
playground: false,
undocumented_oauth2_access: false,
},
Resource::Assets {
path: http_listener_assets_path_default(),
},
Expand Down
41 changes: 38 additions & 3 deletions crates/handlers/src/graphql/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use axum::{
extract::{RawQuery, State as AxumState},
http::StatusCode,
response::{Html, IntoResponse, Response},
Json,
Extension, Json,
};
use axum_extra::typed_header::TypedHeader;
use chrono::{DateTime, Utc};
Expand Down Expand Up @@ -65,6 +65,13 @@ use crate::{impl_from_error_for_route, passwords::PasswordManager, BoundActivity
#[cfg(test)]
mod tests;

/// Extra parameters we get from the listener configuration, because they are
/// per-listener options. We pass them through request extensions.
#[derive(Debug, Clone)]
pub struct ExtraRouterParameters {
pub undocumented_oauth2_access: bool,
}

struct GraphQLState {
pool: PgPool,
homeserver_connection: Arc<dyn HomeserverConnection<Error = anyhow::Error>>,
Expand Down Expand Up @@ -217,13 +224,19 @@ impl IntoResponse for RouteError {
}

async fn get_requester(
undocumented_oauth2_access: bool,
clock: &impl Clock,
activity_tracker: &BoundActivityTracker,
mut repo: BoxRepository,
session_info: SessionInfo,
token: Option<&str>,
) -> Result<Requester, RouteError> {
let requester = if let Some(token) = token {
// If we haven't enabled undocumented_oauth2_access on the listener, we bail out
if !undocumented_oauth2_access {
return Err(RouteError::InvalidToken);
}

let token = repo
.oauth2_access_token()
.find_by_token(token)
Expand Down Expand Up @@ -281,6 +294,9 @@ async fn get_requester(

pub async fn post(
AxumState(schema): AxumState<Schema>,
Extension(ExtraRouterParameters {
undocumented_oauth2_access,
}): Extension<ExtraRouterParameters>,
clock: BoxClock,
repo: BoxRepository,
activity_tracker: BoundActivityTracker,
Expand All @@ -294,7 +310,15 @@ pub async fn post(
.as_ref()
.map(|TypedHeader(Authorization(bearer))| bearer.token());
let (session_info, _cookie_jar) = cookie_jar.session_info();
let requester = get_requester(&clock, &activity_tracker, repo, session_info, token).await?;
let requester = get_requester(
undocumented_oauth2_access,
&clock,
&activity_tracker,
repo,
session_info,
token,
)
.await?;

let content_type = content_type.map(|TypedHeader(h)| h.to_string());

Expand Down Expand Up @@ -323,6 +347,9 @@ pub async fn post(

pub async fn get(
AxumState(schema): AxumState<Schema>,
Extension(ExtraRouterParameters {
undocumented_oauth2_access,
}): Extension<ExtraRouterParameters>,
clock: BoxClock,
repo: BoxRepository,
activity_tracker: BoundActivityTracker,
Expand All @@ -334,7 +361,15 @@ pub async fn get(
.as_ref()
.map(|TypedHeader(Authorization(bearer))| bearer.token());
let (session_info, _cookie_jar) = cookie_jar.session_info();
let requester = get_requester(&clock, &activity_tracker, repo, session_info, token).await?;
let requester = get_requester(
undocumented_oauth2_access,
&clock,
&activity_tracker,
repo,
session_info,
token,
)
.await?;

let request =
async_graphql::http::parse_query_string(&query.unwrap_or_default())?.data(requester);
Expand Down
10 changes: 8 additions & 2 deletions crates/handlers/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ use axum::{
http::Method,
response::{Html, IntoResponse},
routing::{get, post},
Router,
Extension, Router,
};
use graphql::ExtraRouterParameters;
use headers::HeaderName;
use hyper::{
header::{
Expand Down Expand Up @@ -108,7 +109,7 @@ where
Router::new().route(mas_router::Healthcheck::route(), get(self::health::get))
}

pub fn graphql_router<S>(playground: bool) -> Router<S>
pub fn graphql_router<S>(playground: bool, undocumented_oauth2_access: bool) -> Router<S>
where
S: Clone + Send + Sync + 'static,
graphql::Schema: FromRef<S>,
Expand All @@ -123,6 +124,11 @@ where
mas_router::GraphQL::route(),
get(self::graphql::get).post(self::graphql::post),
)
// Pass the undocumented_oauth2_access parameter through the request extension, as it is
// per-listener
.layer(Extension(ExtraRouterParameters {
undocumented_oauth2_access,
}))
.layer(
CorsLayer::new()
.allow_origin(Any)
Expand Down
4 changes: 3 additions & 1 deletion crates/handlers/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,9 @@ impl TestState {
.merge(crate::api_router())
.merge(crate::compat_router())
.merge(crate::human_router(self.templates.clone()))
.merge(crate::graphql_router(false))
// We enable undocumented_oauth2_access for the tests, as it is easier to query the API
// with it
.merge(crate::graphql_router(false, true))
.merge(crate::admin_api_router().1)
.with_state(self.clone())
.into_service();
Expand Down
8 changes: 5 additions & 3 deletions docs/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@
"name": "compat"
},
{
"name": "graphql",
"playground": true
"name": "graphql"
},
{
"name": "assets"
Expand Down Expand Up @@ -742,7 +741,10 @@
},
"playground": {
"description": "Enabled the GraphQL playground",
"default": false,
"type": "boolean"
},
"undocumented_oauth2_access": {
"description": "Allow access for OAuth 2.0 clients (undocumented)",
"type": "boolean"
}
}
Expand Down

0 comments on commit 1bdad26

Please sign in to comment.