fix: Use RFC-compliant error responses for OAuth endpoints

src/api/client/oauth/register_client.rs

@@ -22,7 +22,7 @@ pub(crate) async fn register_client_route(
 		.oauth
 		.register_client(&metadata)
 		.await
-		.map_err(|err| (StatusCode::BAD_REQUEST, err.to_owned()).into_response())?;
+		.map_err(|err| (StatusCode::BAD_REQUEST, Json(err)).into_response())?;
 
 	Ok(Json(RegisteredClient { client_id, metadata }).into_response())
 }

src/api/client/oauth/token.rs

@@ -8,7 +8,7 @@ pub(crate) async fn token_route(
 ) -> impl IntoResponse {
 	match services.oauth.issue_token(request).await {
 		| Ok(response) => Ok(Json(response)),
-		| Err(err) => Err((StatusCode::BAD_REQUEST, err.message())),
+		| Err(err) => Err((StatusCode::BAD_REQUEST, Json(err))),
 	}
 }
 
@@ -18,6 +18,6 @@ pub(crate) async fn revoke_token_route(
 ) -> impl IntoResponse {
 	match services.oauth.revoke_token(request.token).await {
 		| Ok(()) => Ok(StatusCode::OK),
-		| Err(err) => Err((StatusCode::BAD_REQUEST, err.message())),
+		| Err(err) => Err((StatusCode::BAD_REQUEST, Json(err))),
 	}
 }

src/service/oauth/grant.rs

@@ -1,4 +1,11 @@
-use std::{collections::BTreeSet, fmt::Debug, hash::Hash, mem::discriminant};
+use std::{
+	borrow::Cow,
+	collections::BTreeSet,
+	error::Error,
+	fmt::{Debug, Display},
+	hash::Hash,
+	mem::discriminant,
+};
 
 use regex::Regex;
 use ruma::OwnedDeviceId;
@@ -63,7 +70,7 @@ impl Hash for Scope {
 	fn hash<H: std::hash::Hasher>(&self, state: &mut H) { discriminant(self).hash(state); }
 }
 
-impl std::fmt::Display for Scope {
+impl Display for Scope {
 	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
 		let urn = match self {
 			| Self::ClientApi => "urn:matrix:client:api:*".to_owned(),
@@ -111,6 +118,46 @@ pub fn to_scopes(&self) -> Result<BTreeSet<Scope>, String> {
 	}
 }
 
+#[derive(Serialize, Debug, Clone)]
+pub struct OAuthError {
+	pub error: ErrorCode,
+	pub error_description: Cow<'static, str>,
+}
+
+impl OAuthError {
+	pub const fn invalid_request(error_description: &'static str) -> Self {
+		Self {
+			error: ErrorCode::InvalidRequest,
+			error_description: Cow::Borrowed(error_description),
+		}
+	}
+
+	pub const fn invalid_grant(error_description: &'static str) -> Self {
+		Self {
+			error: ErrorCode::InvalidGrant,
+			error_description: Cow::Borrowed(error_description),
+		}
+	}
+}
+
+impl Display for OAuthError {
+	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+		write!(f, "OAuth error {:?}: {}", self.error, self.error_description)
+	}
+}
+
+impl Error for OAuthError {}
+
+#[derive(Serialize, Debug, Clone, Copy, PartialEq, Eq)]
+#[serde(rename_all = "snake_case")]
+pub enum ErrorCode {
+	InvalidRequest,
+	AccessDenied,
+	InvalidScope,
+	InvalidGrant,
+	InvalidClientMetadata,
+}
+
 #[derive(Serialize)]
 pub struct AuthorizationCodeResponse {
 	pub state: String,

src/service/oauth/mod.rs

@@ -6,7 +6,7 @@
 
 use base64::Engine;
 use conduwuit::{
-	Err, Result, err, info,
+	Result, info,
 	utils::{self, hash::sha256},
 };
 use database::{Deserialized, Json, Map};
@@ -20,8 +20,8 @@
 	oauth::{
 		client_metadata::{ApplicationType, ClientMetadata, ResponseType},
 		grant::{
-			AuthorizationCodeQuery, AuthorizationCodeResponse, CodeChallengeMethod, ResponseMode,
-			Scope, TokenRequest, TokenResponse, TokenType,
+			AuthorizationCodeQuery, AuthorizationCodeResponse, CodeChallengeMethod, ErrorCode,
+			OAuthError, ResponseMode, Scope, TokenRequest, TokenResponse, TokenType,
 		},
 	},
 	users,
@@ -131,11 +131,11 @@ impl Service {
 
 	fn generate_token() -> String { utils::random_string(Self::RANDOM_TOKEN_LENGTH) }
 
-	pub async fn register_client(
-		&self,
-		metadata: &ClientMetadata,
-	) -> Result<String, &'static str> {
-		metadata.validate()?;
+	pub async fn register_client(&self, metadata: &ClientMetadata) -> Result<String, OAuthError> {
+		metadata.validate().map_err(|error| OAuthError {
+			error: ErrorCode::InvalidClientMetadata,
+			error_description: error.into(),
+		})?;
 
 		let client_id = base64::prelude::BASE64_STANDARD
 			.encode(sha256::hash(serde_json::to_string(metadata).unwrap().as_bytes()));
@@ -263,7 +263,7 @@ pub async fn request_authorization_code(
 		Ok(redirect_uri)
 	}
 
-	pub async fn issue_token(&self, request: TokenRequest) -> Result<TokenResponse> {
+	pub async fn issue_token(&self, request: TokenRequest) -> Result<TokenResponse, OAuthError> {
 		match request {
 			| TokenRequest::AuthorizationCode {
 				code,
@@ -277,17 +277,17 @@ pub async fn issue_token(&self, request: TokenRequest) -> Result<TokenResponse>
 					.remove(&code)
 					.filter(|grant| grant.is_valid_for(&client_id))
 				else {
-					return Err!("Invalid code");
+					return Err(OAuthError::invalid_grant("Invalid authorization code"));
 				};
 
 				if redirect_uri != pending_grant.expected_redirect_uri {
-					return Err!("Unexpected redirect uri");
+					return Err(OAuthError::invalid_grant("Invalid redirect URI"));
 				}
 
 				let expected_code_challenge =
 					base64::prelude::BASE64_URL_SAFE_NO_PAD.encode(sha256::hash(&code_verifier));
 				if expected_code_challenge != pending_grant.code_challenge {
-					return Err!("Invalid code challenge");
+					return Err(OAuthError::invalid_grant("Invalid code challenge"));
 				}
 
 				self.create_session(
@@ -303,7 +303,7 @@ pub async fn issue_token(&self, request: TokenRequest) -> Result<TokenResponse>
 		}
 	}
 
-	pub async fn revoke_token(&self, token: String) -> Result<()> {
+	pub async fn revoke_token(&self, token: String) -> Result<(), OAuthError> {
 		let (user_id, device_id) = if let Ok(refresh_token_info) = self
 			.db
 			.refreshtoken_refreshtokeninfo
@@ -317,7 +317,7 @@ pub async fn revoke_token(&self, token: String) -> Result<()> {
 		{
 			(user_id, device_id)
 		} else {
-			return Err!("Invalid token");
+			return Err(OAuthError::invalid_grant("Invalid access or refersh token"));
 		};
 
 		// This will also call [`Self::remove_session`]
@@ -335,7 +335,7 @@ async fn create_session(
 		requested_scopes: BTreeSet<Scope>,
 		client_name: Option<String>,
 		client_id: String,
-	) -> Result<TokenResponse> {
+	) -> Result<TokenResponse, OAuthError> {
 		let access_token = Self::generate_token();
 		let refresh_token = Self::generate_token();
 
@@ -348,7 +348,7 @@ async fn create_session(
 					None
 				}
 			})
-			.ok_or_else(|| err!("No device ID scope supplied"))?;
+			.ok_or_else(|| OAuthError::invalid_grant("No device ID scope supplied"))?;
 
 		self.services
 			.users
@@ -360,7 +360,10 @@ async fn create_session(
 				client_name,
 				None,
 			)
-			.await?;
+			.await
+			// This can only panic if the authorizing user suffered a spontaneous existence
+			// failure during authentication, which should(?) be impossible(?)
+			.expect("failed to create device");
 
 		self.db.userdeviceid_oauthsessioninfo.put(
 			(&authorizing_user, device_id),
@@ -401,7 +404,7 @@ async fn refresh_session(
 		&self,
 		client_id: String,
 		refresh_token: String,
-	) -> Result<TokenResponse> {
+	) -> Result<TokenResponse, OAuthError> {
 		let Some(refresh_token_info) = self
 			.db
 			.refreshtoken_refreshtokeninfo
@@ -410,7 +413,7 @@ async fn refresh_session(
 			.deserialized::<RefreshTokenInfo>()
 			.ok()
 		else {
-			return Err!("Invalid refresh token");
+			return Err(OAuthError::invalid_grant("Invalid refresh token"));
 		};
 
 		assert_eq!(&client_id, &refresh_token_info.client_id, "refresh token client id mismatch");
@@ -440,7 +443,8 @@ async fn refresh_session(
 				&new_access_token,
 				Some(Self::ACCESS_TOKEN_MAX_AGE),
 			)
-			.await?;
+			.await
+			.expect("should be able to set token");
 
 		self.db.userdeviceid_oauthsessioninfo.put(
 			(&refresh_token_info.user_id, &refresh_token_info.device_id),