Revoke OAuth session on code reuse

crates/handlers/src/oauth2/token.rs

@@ -30,6 +30,7 @@ use mas_storage::{
     oauth2::{
         access_token::{add_access_token, revoke_access_token},
         authorization_grant::{exchange_grant, lookup_grant_by_code},
+        end_oauth_session,
         refresh_token::{add_refresh_token, lookup_active_refresh_token, replace_refresh_token},
     },
     DatabaseInconsistencyError,
@@ -184,6 +185,9 @@ async fn authorization_code_grant(
         .await
         .wrap_error()?;
 
+    // TODO: that's not a timestamp from the DB. Let's assume they are in sync
+    let now = Utc::now();
+
     let session = match authz_grant.stage {
         AuthorizationGrantStage::Cancelled { cancelled_at } => {
             debug!(%cancelled_at, "Authorization grant was cancelled");
@@ -192,12 +196,17 @@ async fn authorization_code_grant(
         AuthorizationGrantStage::Exchanged {
             exchanged_at,
             fulfilled_at,
-            session: _,
+            session,
         } => {
-            // TODO: we should invalidate the existing session if a code is used twice after
-            // some period of time. See the `oidcc-codereuse-30seconds` test from the
-            // conformance suite
             debug!(%exchanged_at, %fulfilled_at, "Authorization code was already exchanged");
+
+            // Ending the session if the token was already exchanged more than 20s ago
+            if now - exchanged_at > Duration::seconds(20) {
+                debug!("Ending potentially compromised session");
+                end_oauth_session(&mut txn, session).await.wrap_error()?;
+                txn.commit().await.wrap_error()?;
+            }
+
             return error(InvalidGrant);
         }
         AuthorizationGrantStage::Pending => {
@@ -206,10 +215,13 @@ async fn authorization_code_grant(
         }
         AuthorizationGrantStage::Fulfilled {
             ref session,
-            fulfilled_at: _,
+            fulfilled_at,
         } => {
-            // TODO: we should check that the session was not fullfilled too long ago
+            if now - fulfilled_at > Duration::minutes(10) {
-            // (30s to 1min?). The main problem is getting a timestamp from the database
+                debug!("Code exchange took more than 10 minutes");
+                return error(InvalidGrant);
+            }
+
             session
         }
     };

crates/storage/migrations/20220114112012_oauth_session_end.down.sql

@@ -0,0 +1,16 @@
+-- Copyright 2021 The Matrix.org Foundation C.I.C.
+--
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+ALTER TABLE oauth2_sessions
+  DROP COLUMN "ended_at";

crates/storage/migrations/20220114112012_oauth_session_end.up.sql

@@ -0,0 +1,16 @@
+-- Copyright 2021 The Matrix.org Foundation C.I.C.
+--
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+ALTER TABLE oauth2_sessions
+  ADD COLUMN "ended_at" TIMESTAMP WITH TIME ZONE DEFAULT NULL;

crates/storage/src/oauth2/access_token.rs

@@ -125,6 +125,7 @@ pub async fn lookup_active_access_token(
             WHERE at.token = $1
               AND at.created_at + (at.expires_after * INTERVAL '1 second') >= now()
               AND us.active
+              AND os.ended_at IS NULL
 
             ORDER BY usa.created_at DESC
             LIMIT 1

crates/storage/src/oauth2/mod.rs

@@ -12,6 +12,31 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use mas_data_model::Session;
+use sqlx::PgExecutor;
+
+use crate::PostgresqlBackend;
+
 pub mod access_token;
 pub mod authorization_grant;
 pub mod refresh_token;
+
+pub async fn end_oauth_session(
+    executor: impl PgExecutor<'_>,
+    session: Session<PostgresqlBackend>,
+) -> anyhow::Result<()> {
+    let res = sqlx::query!(
+        r#"
+            UPDATE oauth2_sessions
+            SET ended_at = NOW()
+            WHERE id = $1
+        "#,
+        session.data,
+    )
+    .execute(executor)
+    .await?;
+
+    anyhow::ensure!(res.rows_affected() == 1);
+
+    Ok(())
+}

crates/storage/src/oauth2/refresh_token.rs

@@ -112,6 +112,7 @@ pub async fn lookup_active_refresh_token(
             WHERE rt.token = $1
               AND rt.next_token_id IS NULL
               AND us.active
+              AND os.ended_at IS NULL
 
             ORDER BY usa.created_at DESC
             LIMIT 1