merge the huge authentication MR mess (reject requests with authentication when not used)

and (fix: allow invalid auth when no auth is required)

Signed-off-by: strawberry <strawberry@puppygock.gay>

src/api/ruma_wrapper/axum.rs

@@ -15,13 +15,20 @@ use bytes::{Buf, BufMut, Bytes, BytesMut};
 use http::{Request, StatusCode};
 use ruma::{
 	api::{client::error::ErrorKind, AuthScheme, IncomingRequest, OutgoingResponse},
-	CanonicalJsonValue, OwnedDeviceId, OwnedServerName, UserId,
+	CanonicalJsonValue, OwnedDeviceId, OwnedServerName, OwnedUserId, UserId,
 };
 use serde::Deserialize;
 use tracing::{debug, error, trace, warn};
 
 use super::{Ruma, RumaResponse};
-use crate::{services, Error, Result};
+use crate::{service::appservice::RegistrationInfo, services, Error, Result};
+
+enum Token {
+	Appservice(Box<RegistrationInfo>),
+	User((OwnedUserId, OwnedDeviceId)),
+	Invalid,
+	None,
+}
 
 #[derive(Deserialize)]
 struct QueryParams {
@@ -66,7 +73,7 @@ where
 		let query_params: QueryParams = match serde_html_form::from_str(query) {
 			Ok(params) => params,
 			Err(e) => {
-				error!(%query, "Failed to deserialize query parameters: {}", e);
+				error!(%query, "Failed to deserialize query parameters: {e}");
 				return Err(Error::BadRequest(ErrorKind::Unknown, "Failed to read query parameters"));
 			},
 		};
@@ -76,31 +83,49 @@ where
 			None => query_params.access_token.as_deref(),
 		};
 
-		let mut json_body = serde_json::from_slice::<CanonicalJsonValue>(&body).ok();
+		let token = if let Some(token) = token {
-
+			if let Some(reg_info) = services().appservice.find_from_token(token).await {
-		let appservice_registration = if let Some(token) = token {
+				Token::Appservice(Box::new(reg_info))
-			services().appservice.find_from_token(token).await
+			} else if let Some((user_id, device_id)) = services().users.find_from_token(token)? {
+				Token::User((user_id, OwnedDeviceId::from(device_id)))
 			} else {
-			None
+				Token::Invalid
+			}
+		} else {
+			Token::None
 		};
 
-		let (sender_user, sender_device, sender_servername, from_appservice) =
+		let mut json_body = serde_json::from_slice::<CanonicalJsonValue>(&body).ok();
-			if let Some(info) = appservice_registration {
+
-				match metadata.authentication {
+		let (sender_user, sender_device, sender_servername, from_appservice) = match (metadata.authentication, token) {
-					AuthScheme::AccessToken => {
+			(AuthScheme::None, Token::Invalid) => (None, None, None, false),
-						let user_id = query_params.user_id.map_or_else(
+			(_, Token::Invalid) => {
+				return Err(Error::BadRequest(
+					ErrorKind::UnknownToken {
+						soft_logout: false,
+					},
+					"Unknown access token.",
+				))
+			},
+			(
+				AuthScheme::AccessToken
+				| AuthScheme::AppserviceToken
+				| AuthScheme::AccessTokenOptional
+				| AuthScheme::None,
+				Token::Appservice(info),
+			) => {
+				let user_id = query_params
+					.user_id
+					.map_or_else(
 						|| {
 							UserId::parse_with_server_name(
 								info.registration.sender_localpart.as_str(),
 								services().globals.server_name(),
 							)
-								.unwrap()
 						},
-							|s| UserId::parse(s).unwrap(),
+						UserId::parse,
-						);
+					)
-
+					.map_err(|_| Error::BadRequest(ErrorKind::InvalidUsername, "Username is invalid."))?;
-						debug!("User ID: {:?}", user_id);
-
 				if !services().users.exists(&user_id)? {
 					return Err(Error::BadRequest(ErrorKind::forbidden(), "User does not exist."));
 				}
@@ -108,74 +133,14 @@ where
 				// TODO: Check if appservice is allowed to be that user
 				(Some(user_id), None, None, true)
 			},
-					AuthScheme::AccessTokenOptional | AuthScheme::AppserviceToken => {
+			(AuthScheme::AccessToken, Token::None) => {
-						let user_id = query_params.user_id.map_or_else(
-							|| {
-								UserId::parse_with_server_name(
-									info.registration.sender_localpart.as_str(),
-									services().globals.server_name(),
-								)
-								.unwrap()
-							},
-							|s| UserId::parse(s).unwrap(),
-						);
-
-						debug!("User ID: {:?}", user_id);
-
-						if !services().users.exists(&user_id)? {
-							(None, None, None, true)
-						} else {
-							// TODO: Check if appservice is allowed to be that user
-							(Some(user_id), None, None, true)
-						}
-					},
-					AuthScheme::ServerSignatures | AuthScheme::None => (None, None, None, true),
-				}
-			} else {
-				match metadata.authentication {
-					AuthScheme::AccessToken => {
-						let Some(token) = token else {
 				return Err(Error::BadRequest(ErrorKind::MissingToken, "Missing access token."));
-						};
-
-						match services().users.find_from_token(token)? {
-							None => {
-								return Err(Error::BadRequest(
-									ErrorKind::UnknownToken {
-										soft_logout: false,
 			},
-									"Unknown access token.",
+			(
-								))
+				AuthScheme::AccessToken | AuthScheme::AccessTokenOptional | AuthScheme::None,
-							},
+				Token::User((user_id, device_id)),
-							Some((user_id, device_id)) => {
+			) => (Some(user_id), Some(device_id), None, false),
-								(Some(user_id), Some(OwnedDeviceId::from(device_id)), None, false)
+			(AuthScheme::ServerSignatures, Token::None) => {
-							},
-						}
-					},
-					AuthScheme::AccessTokenOptional => {
-						let token = token.unwrap_or("");
-
-						if token.is_empty() {
-							(None, None, None, false)
-						} else {
-							match services().users.find_from_token(token)? {
-								None => {
-									return Err(Error::BadRequest(
-										ErrorKind::UnknownToken {
-											soft_logout: false,
-										},
-										"Unknown access token.",
-									))
-								},
-								Some((user_id, device_id)) => {
-									(Some(user_id), Some(OwnedDeviceId::from(device_id)), None, false)
-								},
-							}
-						}
-					},
-					// treat non-appservice registrations as None authentication
-					AuthScheme::AppserviceToken => (None, None, None, false),
-					AuthScheme::ServerSignatures => {
 				if !services().globals.allow_federation() {
 					return Err(Error::bad_config("Federation is disabled."));
 				}
@@ -184,7 +149,7 @@ where
 					.extract::<TypedHeader<Authorization<XMatrix>>>()
 					.await
 					.map_err(|e| {
-								warn!("Missing or invalid Authorization header: {}", e);
+						warn!("Missing or invalid Authorization header: {e}");
 
 						let msg = match e.reason() {
 							TypedHeaderRejectionReason::Missing => "Missing Authorization header.",
@@ -232,28 +197,22 @@ where
 					.fetch_signing_keys_for_server(&x_matrix.origin, vec![x_matrix.key.clone()])
 					.await;
 
-						let keys = match keys_result {
+				let keys = keys_result.map_err(|e| {
-							Ok(b) => b,
+					warn!("Failed to fetch signing keys: {e}");
-							Err(e) => {
+					Error::BadRequest(ErrorKind::forbidden(), "Failed to fetch signing keys.")
-								warn!("Failed to fetch signing keys: {}", e);
+				})?;
-								return Err(Error::BadRequest(ErrorKind::forbidden(), "Failed to fetch signing keys."));
-							},
-						};
 
 				let pub_key_map = BTreeMap::from_iter([(x_matrix.origin.as_str().to_owned(), keys)]);
 
 				match ruma::signatures::verify_json(&pub_key_map, &request_map) {
 					Ok(()) => (None, None, Some(x_matrix.origin), false),
 					Err(e) => {
-								warn!(
+						warn!("Failed to verify json request from {}: {e}\n{request_map:?}", x_matrix.origin);
-									"Failed to verify json request from {}: {}\n{:?}",
-									x_matrix.origin, e, request_map
-								);
 
 						if parts.uri.to_string().contains('@') {
 							warn!(
-										"Request uri contained '@' character. Make sure your reverse proxy gives \
+								"Request uri contained '@' character. Make sure your reverse proxy gives Conduit the \
-										 Conduit the raw uri (apache: use nocanon)"
+								 raw uri (apache: use nocanon)"
 							);
 						}
 
@@ -264,39 +223,21 @@ where
 					},
 				}
 			},
-					AuthScheme::None => match parts.uri.path() {
+			(AuthScheme::None | AuthScheme::AppserviceToken | AuthScheme::AccessTokenOptional, Token::None) => {
-						// TOOD: can we do a better check here?
-						// allow_public_room_directory_without_auth
-						"/_matrix/client/v3/publicRooms" | "/_matrix/client/r0/publicRooms" => {
-							if !services()
-								.globals
-								.config
-								.allow_public_room_directory_without_auth
-							{
-								let Some(token) = token else {
-									return Err(Error::BadRequest(ErrorKind::MissingToken, "Missing access token."));
-								};
-
-								match services().users.find_from_token(token)? {
-									None => {
-										return Err(Error::BadRequest(
-											ErrorKind::UnknownToken {
-												soft_logout: false,
-											},
-											"Unknown access token.",
-										))
-									},
-									Some((user_id, device_id)) => {
-										(Some(user_id), Some(OwnedDeviceId::from(device_id)), None, false)
-									},
-								}
-							} else {
 				(None, None, None, false)
-							}
 			},
-						_ => (None, None, None, false),
+			(AuthScheme::ServerSignatures, Token::Appservice(_) | Token::User(_)) => {
+				return Err(Error::BadRequest(
+					ErrorKind::Unauthorized,
+					"Only server signatures should be used on this endpoint.",
+				));
+			},
+			(AuthScheme::AppserviceToken, Token::User(_)) => {
+				return Err(Error::BadRequest(
+					ErrorKind::Unauthorized,
+					"Only appservice access tokens should be used on this endpoint.",
+				));
 			},
-				}
 		};
 
 		let mut http_request = Request::builder().uri(parts.uri).method(parts.method);
@@ -398,7 +339,7 @@ impl Credentials for XMatrix {
 				"key" => key = Some(value.to_owned()),
 				"sig" => sig = Some(value.to_owned()),
 				"destination" => destination = Some(value.to_owned()),
-				_ => debug!("Unexpected field `{}` in X-Matrix Authorization header", name),
+				_ => debug!("Unexpected field `{name}` in X-Matrix Authorization header"),
 			}
 		}