Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
72 changes: 38 additions & 34 deletions crates/handlers/src/compat/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,8 @@

use std::sync::{Arc, LazyLock};

use axum::{
Json,
extract::{State, rejection::JsonRejection},
response::IntoResponse,
};
use axum_extra::{extract::WithRejection, typed_header::TypedHeader};
use axum::{Json, extract::State, response::IntoResponse};
use axum_extra::typed_header::TypedHeader;
use chrono::Duration;
use hyper::StatusCode;
use mas_axum_utils::sentry::SentryEventID;
Expand All @@ -34,7 +30,7 @@ use serde_with::{DurationMilliSeconds, serde_as, skip_serializing_none};
use thiserror::Error;
use zeroize::Zeroizing;

use super::MatrixError;
use super::{MatrixError, MatrixJsonBody};
use crate::{
BoundActivityTracker, Limiter, METER, RequesterFingerprint, impl_from_error_for_route,
passwords::PasswordManager, rate_limit::PasswordCheckLimitedError,
Expand Down Expand Up @@ -206,9 +202,6 @@ pub enum RouteError {
#[error("invalid login token")]
InvalidLoginToken,

#[error(transparent)]
InvalidJsonBody(#[from] JsonRejection),

#[error("failed to provision device")]
ProvisionDeviceFailed(#[source] anyhow::Error),
}
Expand All @@ -230,26 +223,6 @@ impl IntoResponse for RouteError {
error: "Too many login attempts",
status: StatusCode::TOO_MANY_REQUESTS,
},
Self::InvalidJsonBody(JsonRejection::MissingJsonContentType(_)) => MatrixError {
errcode: "M_NOT_JSON",
error: "Invalid Content-Type header: expected application/json",
status: StatusCode::BAD_REQUEST,
},
Self::InvalidJsonBody(JsonRejection::JsonSyntaxError(_)) => MatrixError {
errcode: "M_NOT_JSON",
error: "Body is not a valid JSON document",
status: StatusCode::BAD_REQUEST,
},
Self::InvalidJsonBody(JsonRejection::JsonDataError(_)) => MatrixError {
errcode: "M_BAD_JSON",
error: "JSON fields are not valid",
status: StatusCode::BAD_REQUEST,
},
Self::InvalidJsonBody(_) => MatrixError {
errcode: "M_UNKNOWN",
error: "Unknown error while parsing JSON body",
status: StatusCode::BAD_REQUEST,
},
Self::Unsupported => MatrixError {
errcode: "M_UNKNOWN",
error: "Invalid login type",
Expand Down Expand Up @@ -300,7 +273,7 @@ pub(crate) async fn post(
State(limiter): State<Limiter>,
requester: RequesterFingerprint,
user_agent: Option<TypedHeader<headers::UserAgent>>,
WithRejection(Json(input), _): WithRejection<Json<RequestBody>, RouteError>,
MatrixJsonBody(input): MatrixJsonBody<RequestBody>,
) -> Result<impl IntoResponse, RouteError> {
let user_agent = user_agent.map(|ua| UserAgent::parse(ua.as_str().to_owned()));
let login_type = input.credentials.login_type();
Expand Down Expand Up @@ -662,12 +635,12 @@ mod tests {
response.assert_status(StatusCode::BAD_REQUEST);
let body: serde_json::Value = response.json();

insta::assert_json_snapshot!(body, @r###"
insta::assert_json_snapshot!(body, @r#"
{
"errcode": "M_NOT_JSON",
"error": "Invalid Content-Type header: expected application/json"
"error": "Body is not a valid JSON document"
}
"###);
"#);

// Missing keys in body
let request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({}));
Expand Down Expand Up @@ -902,6 +875,37 @@ mod tests {
assert_eq!(body, old_body);
}

/// Test that we can send a login request without a Content-Type header
#[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]
async fn test_no_content_type(pool: PgPool) {
setup();
let state = TestState::from_pool(pool).await.unwrap();

user_with_password(&state, "alice", "password").await;
// Try without a Content-Type header
let mut request = Request::post("/_matrix/client/v3/login").json(serde_json::json!({
"type": "m.login.password",
"identifier": {
"type": "m.id.user",
"user": "alice",
},
"password": "password",
}));
request.headers_mut().remove(hyper::header::CONTENT_TYPE);

let response = state.request(request).await;
response.assert_status(StatusCode::OK);

let body: serde_json::Value = response.json();
insta::assert_json_snapshot!(body, @r###"
{
"access_token": "mct_16tugBE5Ta9LIWoSJaAEHHq2g3fx8S_alcBB4",
"device_id": "ZGpSvYQqlq",
"user_id": "@alice:example.com"
}
"###);
}

/// Test that a user can login with a password using the Matrix
/// compatibility API, using a MXID as identifier
#[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]
Expand Down
125 changes: 122 additions & 3 deletions crates/handlers/src/compat/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,19 @@
// SPDX-License-Identifier: AGPL-3.0-only
// Please see LICENSE in the repository root for full details.

use axum::{Json, response::IntoResponse};
use hyper::StatusCode;
use serde::Serialize;
use axum::{
Json,
body::Bytes,
extract::{
Request,
rejection::{BytesRejection, FailedToBufferBody},
},
response::IntoResponse,
};
use hyper::{StatusCode, header};
use mas_axum_utils::sentry::SentryEventID;
use serde::{Serialize, de::DeserializeOwned};
use thiserror::Error;

pub(crate) mod login;
pub(crate) mod login_sso_complete;
Expand All @@ -27,3 +37,112 @@ impl IntoResponse for MatrixError {
(self.status, Json(self)).into_response()
}
}

#[derive(Debug, Clone, Copy, Default)]
#[must_use]
pub struct MatrixJsonBody<T>(pub T);

#[derive(Debug, Error)]
pub enum MatrixJsonBodyRejection {
#[error("Invalid Content-Type header: expected application/json")]
InvalidContentType,

#[error("Invalid Content-Type header: expected application/json, got {0}")]
ContentTypeNotJson(mime::Mime),

#[error("Failed to read request body")]
BytesRejection(#[from] BytesRejection),

#[error("Body is not a valid JSON document")]
NotJson(#[source] serde_json::Error),

#[error("Request body has invalid parameters")]
BadJson(#[source] serde_json::Error),
}

impl IntoResponse for MatrixJsonBodyRejection {
fn into_response(self) -> axum::response::Response {
let event_id = sentry::capture_error(&self);
let response = match self {
Self::InvalidContentType | Self::ContentTypeNotJson(_) => MatrixError {
errcode: "M_NOT_JSON",
error: "Invalid Content-Type header: expected application/json",
status: StatusCode::BAD_REQUEST,
},

Self::BytesRejection(BytesRejection::FailedToBufferBody(
FailedToBufferBody::LengthLimitError(_),
)) => MatrixError {
errcode: "M_TOO_LARGE",
error: "Request body too large",
status: StatusCode::PAYLOAD_TOO_LARGE,
},

Self::BytesRejection(BytesRejection::FailedToBufferBody(
FailedToBufferBody::UnknownBodyError(_),
)) => MatrixError {
errcode: "M_UNKNOWN",
error: "Failed to read request body",
status: StatusCode::BAD_REQUEST,
},

Self::BytesRejection(_) => MatrixError {
errcode: "M_UNKNOWN",
error: "Unknown error while reading request body",
status: StatusCode::BAD_REQUEST,
},

Self::NotJson(_) => MatrixError {
errcode: "M_NOT_JSON",
error: "Body is not a valid JSON document",
status: StatusCode::BAD_REQUEST,
},

Self::BadJson(_) => MatrixError {
errcode: "M_BAD_JSON",
error: "JSON fields are not valid",
status: StatusCode::BAD_REQUEST,
},
};

(SentryEventID::from(event_id), response).into_response()
}
}

impl<T, S> axum::extract::FromRequest<S> for MatrixJsonBody<T>
where
T: DeserializeOwned,
S: Send + Sync,
{
type Rejection = MatrixJsonBodyRejection;

async fn from_request(req: Request, state: &S) -> Result<Self, Self::Rejection> {
// Matrix spec says it's optional to send a Content-Type header, so we
// only check it if it's present
if let Some(content_type) = req.headers().get(header::CONTENT_TYPE) {
let Ok(content_type) = content_type.to_str() else {
return Err(MatrixJsonBodyRejection::InvalidContentType);
};

let Ok(mime) = content_type.parse::<mime::Mime>() else {
return Err(MatrixJsonBodyRejection::InvalidContentType);
};

let is_json_content_type = mime.type_() == "application"
&& (mime.subtype() == "json" || mime.suffix().is_some_and(|name| name == "json"));

if !is_json_content_type {
return Err(MatrixJsonBodyRejection::ContentTypeNotJson(mime));
}
}

let bytes = Bytes::from_request(req, state).await?;

// We first parse it as a JSON value so that we can distinguish between
// invalid JSON documents and invalid requests
let value: serde_json::Value =
serde_json::from_slice(&bytes).map_err(MatrixJsonBodyRejection::NotJson)?;
let value = serde_json::from_value(value).map_err(MatrixJsonBodyRejection::BadJson)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess these requests are tiny so this is fine. But if they were big, I'd rather avoid this and you could use .is_syntax() on the serde_json Error.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, better do that. I used .is_data() instead

7f4e975

Ok(Self(value))
}
}
Loading