Check timings when validating an SSO login

- exchanging a token twice should not work
 - exchanging a token more than 30s after its fullfillment should not
   work
 - exchanging a pending token should not work
 - fullfilling a login more than 30min after its creation should not
   work
 - also have better errors in some cases

crates/handlers/src/compat/login.rs

@@ -13,14 +13,15 @@
 // limitations under the License.
 
 use axum::{response::IntoResponse, Extension, Json};
-use chrono::Duration;
+use chrono::{Duration, Utc};
 use hyper::StatusCode;
 use mas_config::MatrixConfig;
-use mas_data_model::{CompatSession, Device, TokenType};
+use mas_data_model::{CompatSession, CompatSsoLoginState, Device, TokenType};
 use mas_storage::{
     compat::{
         add_compat_access_token, add_compat_refresh_token, compat_login,
         get_compat_sso_login_by_token, mark_compat_sso_login_as_exchanged,
+        CompatSsoLoginLookupError,
     },
     PostgresqlBackend,
 };
@@ -132,6 +133,12 @@ pub enum RouteError {
 
     #[error("login failed")]
     LoginFailed,
+
+    #[error("login took too long")]
+    LoginTookTooLong,
+
+    #[error("invalid login token")]
+    InvalidLoginToken,
 }
 
 impl From<sqlx::Error> for RouteError {
@@ -140,6 +147,16 @@ impl From<sqlx::Error> for RouteError {
     }
 }
 
+impl From<CompatSsoLoginLookupError> for RouteError {
+    fn from(e: CompatSsoLoginLookupError) -> Self {
+        if e.not_found() {
+            Self::InvalidLoginToken
+        } else {
+            Self::Internal(Box::new(e))
+        }
+    }
+}
+
 impl IntoResponse for RouteError {
     fn into_response(self) -> axum::response::Response {
         match self {
@@ -158,6 +175,16 @@ impl IntoResponse for RouteError {
                 error: "Invalid username/password",
                 status: StatusCode::FORBIDDEN,
             },
+            Self::LoginTookTooLong => MatrixError {
+                errcode: "M_UNAUTHORIZED",
+                error: "Login token expired",
+                status: StatusCode::FORBIDDEN,
+            },
+            Self::InvalidLoginToken => MatrixError {
+                errcode: "M_UNAUTHORIZED",
+                error: "Invalid login token",
+                status: StatusCode::FORBIDDEN,
+            },
         }
         .into_response()
     }
@@ -222,10 +249,38 @@ async fn token_login(
     token: &str,
 ) -> Result<CompatSession<PostgresqlBackend>, RouteError> {
     let login = get_compat_sso_login_by_token(&mut *txn, token).await?;
+
+    let now = Utc::now();
+    match login.state {
+        CompatSsoLoginState::Pending => {
+            tracing::error!(
+                login.data,
+                "Exchanged a token for a login that was not fullfilled yet"
+            );
+            return Err(RouteError::InvalidLoginToken);
+        }
+        CompatSsoLoginState::Fullfilled { fullfilled_at, .. } => {
+            if now > fullfilled_at + Duration::seconds(30) {
+                return Err(RouteError::LoginTookTooLong);
+            }
+        }
+        CompatSsoLoginState::Exchanged { exchanged_at, .. } => {
+            if now > exchanged_at + Duration::seconds(30) {
+                // TODO: log that session out
+                tracing::error!(
+                    login.data,
+                    "Login token exchanged a second time more than 30s after"
+                );
+            }
+
+            return Err(RouteError::InvalidLoginToken);
+        }
+    }
+
     let login = mark_compat_sso_login_as_exchanged(&mut *txn, login).await?;
 
     match login.state {
-        mas_data_model::CompatSsoLoginState::Exchanged { session, .. } => Ok(session),
+        CompatSsoLoginState::Exchanged { session, .. } => Ok(session),
         _ => unreachable!(),
     }
 }

crates/handlers/src/compat/login_sso_complete.rs

@@ -21,6 +21,7 @@ use axum::{
     Extension,
 };
 use axum_extra::extract::PrivateCookieJar;
+use chrono::{Duration, Utc};
 use mas_axum_utils::{
     csrf::{CsrfExt, ProtectedForm},
     FancyError, SessionInfoExt,
@@ -29,7 +30,7 @@ use mas_config::Encrypter;
 use mas_data_model::Device;
 use mas_router::Route;
 use mas_storage::compat::{fullfill_compat_sso_login, get_compat_sso_login_by_id};
-use mas_templates::{CompatSsoContext, TemplateContext, Templates};
+use mas_templates::{CompatSsoContext, ErrorContext, TemplateContext, Templates};
 use rand::thread_rng;
 use serde::Serialize;
 use sqlx::PgPool;
@@ -66,6 +67,16 @@ pub async fn get(
 
     let login = get_compat_sso_login_by_id(&mut conn, id).await?;
 
+    // Bail out if that login session is more than 30min old
+    if Utc::now() > login.created_at + Duration::minutes(30) {
+        let ctx = ErrorContext::new()
+            .with_code("compat_sso_login_expired")
+            .with_description("This login session expired.".to_string());
+
+        let content = templates.render_error(&ctx).await?;
+        return Ok((cookie_jar, Html(content)).into_response());
+    }
+
     let ctx = CompatSsoContext::new(login)
         .with_session(session)
         .with_csrf(csrf_token.form_value());
@@ -77,6 +88,7 @@ pub async fn get(
 
 pub async fn post(
     Extension(pool): Extension<PgPool>,
+    Extension(templates): Extension<Templates>,
     cookie_jar: PrivateCookieJar<Encrypter>,
     Path(id): Path<i64>,
     Form(form): Form<ProtectedForm<()>>,
@@ -98,6 +110,16 @@ pub async fn post(
 
     let login = get_compat_sso_login_by_id(&mut txn, id).await?;
 
+    // Bail out if that login session is more than 30min old
+    if Utc::now() > login.created_at + Duration::minutes(30) {
+        let ctx = ErrorContext::new()
+            .with_code("compat_sso_login_expired")
+            .with_description("This login session expired.".to_string());
+
+        let content = templates.render_error(&ctx).await?;
+        return Ok((cookie_jar, Html(content)).into_response());
+    }
+
     let redirect_uri = {
         let mut redirect_uri = login.redirect_uri.clone();
         let existing_params = redirect_uri

crates/storage/src/compat.rs

@@ -29,7 +29,7 @@ use crate::{
     user::lookup_user_by_username, DatabaseInconsistencyError, IdAndCreationTime, PostgresqlBackend,
 };
 
-pub struct CompatAccessTokenLookup {
+struct CompatAccessTokenLookup {
     compat_access_token_id: i64,
     compat_access_token: String,
     compat_access_token_created_at: DateTime<Utc>,
@@ -654,12 +654,26 @@ impl TryFrom<CompatSsoLoginLookup> for CompatSsoLogin<PostgresqlBackend> {
     }
 }
 
+#[derive(Debug, Error)]
+#[error("failed to lookup compat SSO login")]
+pub enum CompatSsoLoginLookupError {
+    Database(#[from] sqlx::Error),
+    Inconsistency(#[from] DatabaseInconsistencyError),
+}
+
+impl CompatSsoLoginLookupError {
+    #[must_use]
+    pub fn not_found(&self) -> bool {
+        matches!(self, Self::Database(sqlx::Error::RowNotFound))
+    }
+}
+
 #[allow(clippy::too_many_lines)]
 #[tracing::instrument(skip(executor), err)]
 pub async fn get_compat_sso_login_by_id(
     executor: impl PgExecutor<'_>,
     id: i64,
-) -> anyhow::Result<CompatSsoLogin<PostgresqlBackend>> {
+) -> Result<CompatSsoLogin<PostgresqlBackend>, CompatSsoLoginLookupError> {
     let res = sqlx::query_as!(
         CompatSsoLoginLookup,
         r#"
@@ -693,8 +707,7 @@ pub async fn get_compat_sso_login_by_id(
     )
     .fetch_one(executor)
     .instrument(tracing::info_span!("Lookup compat SSO login"))
-    .await
+    .await?;
-    .context("could not lookup compat SSO login")?;
 
     Ok(res.try_into()?)
 }
@@ -704,7 +717,7 @@ pub async fn get_compat_sso_login_by_id(
 pub async fn get_compat_sso_login_by_token(
     executor: impl PgExecutor<'_>,
     token: &str,
-) -> anyhow::Result<CompatSsoLogin<PostgresqlBackend>> {
+) -> Result<CompatSsoLogin<PostgresqlBackend>, CompatSsoLoginLookupError> {
     let res = sqlx::query_as!(
         CompatSsoLoginLookup,
         r#"
@@ -738,8 +751,7 @@ pub async fn get_compat_sso_login_by_token(
     )
     .fetch_one(executor)
     .instrument(tracing::info_span!("Lookup compat SSO login"))
-    .await
+    .await?;
-    .context("could not lookup compat SSO login")?;
 
     Ok(res.try_into()?)
 }
@@ -750,8 +762,12 @@ pub async fn fullfill_compat_sso_login(
     mut login: CompatSsoLogin<PostgresqlBackend>,
     device: Device,
 ) -> anyhow::Result<CompatSsoLogin<PostgresqlBackend>> {
-    // TODO: check if login is in pending state
+    if !matches!(login.state, CompatSsoLoginState::Pending) {
+        bail!("sso login in wrong state");
+    };
+
     let mut txn = conn.begin().await.context("could not start transaction")?;
+
     let res = sqlx::query_as!(
         IdAndCreationTime,
         r#"