From d6da876cabe0180acd0ebca173d973c8d3450d99 Mon Sep 17 00:00:00 2001
From: pennae <github@quasiparticle.net>
Date: Wed, 13 Jul 2022 18:09:19 +0200
Subject: keep oauth tokens around a bit after expiry

firefox wants to delete profile access tokens after they're expired and
logs errors if it can't do that. since this happens every hour we can
end up with a bunch of error logs very quickly, so we better let it do
what it wants.
---
 ...20713142453_token_deletion_grace_period.down.sql |  2 ++
 ...0220713142453_token_deletion_grace_period.up.sql | 13 +++++++++++++
 src/api/auth/oauth.rs                               | 21 +++++++++------------
 src/db/mod.rs                                       |  7 +++----
 4 files changed, 27 insertions(+), 16 deletions(-)
 create mode 100644 migrations/20220713142453_token_deletion_grace_period.down.sql
 create mode 100644 migrations/20220713142453_token_deletion_grace_period.up.sql

diff --git a/migrations/20220713142453_token_deletion_grace_period.down.sql b/migrations/20220713142453_token_deletion_grace_period.down.sql
new file mode 100644
index 0000000..afbac32
--- /dev/null
+++ b/migrations/20220713142453_token_deletion_grace_period.down.sql
@@ -0,0 +1,2 @@
+drop procedure prune_expired_tokens;
+alter procedure prune_expired_tokens_1() rename to prune_expired_tokens;
diff --git a/migrations/20220713142453_token_deletion_grace_period.up.sql b/migrations/20220713142453_token_deletion_grace_period.up.sql
new file mode 100644
index 0000000..6228602
--- /dev/null
+++ b/migrations/20220713142453_token_deletion_grace_period.up.sql
@@ -0,0 +1,13 @@
+alter procedure prune_expired_tokens() rename to prune_expired_tokens_1;
+
+create procedure prune_expired_tokens()
+language sql
+begin atomic
+       delete from key_fetch where expires_at <= now();
+       -- give oauth tokens a grace period, otherwise firefox will log an error
+       -- once per hour trying to destroy a token that has already been timed out.
+       delete from oauth_token where expires_at + '1 day'::interval <= now();
+       delete from oauth_authorization where expires_at <= now();
+       delete from device_commands where expires <= now();
+       delete from invite_codes where expires_at <= now();
+end;
diff --git a/src/api/auth/oauth.rs b/src/api/auth/oauth.rs
index b0ed8ee..39fe6ae 100644
--- a/src/api/auth/oauth.rs
+++ b/src/api/auth/oauth.rs
@@ -7,6 +7,7 @@ use serde_json::Value;
 use sha2::Digest;
 use subtle::ConstantTimeEq;
 
+use crate::api::{EMPTY, Empty};
 use crate::api::auth::WithVerifiedFxaLogin;
 use crate::db::DbConn;
 use crate::types::oauth::{Scope, ScopeSet};
@@ -187,25 +188,21 @@ pub(crate) async fn scoped_key_data(
 #[derive(Debug, Deserialize)]
 #[serde(deny_unknown_fields)]
 pub(crate) struct OauthDestroy {
+    #[allow(dead_code)]
     client_id: String,
     token: OauthToken,
 }
 
 #[post("/oauth/destroy", data = "<data>")]
-pub(crate) async fn destroy(db: &DbConn, data: Json<OauthDestroy>) -> auth::Result<()> {
+pub(crate) async fn destroy(db: &DbConn, data: Json<OauthDestroy>) -> auth::Result<Empty> {
     // MISSING api spec allows an optional basic auth header, but what for?
     // TODO fxa also checks the authorization header if present, but firefox doesn't send it
-    let client_id = if let Ok(t) = db.get_refresh_token(&data.token.hash()).await {
-        t.client_id
-    } else if let Ok(t) = db.get_access_token(&data.token.hash()).await {
-        t.client_id
-    } else {
-        return Err(auth::Error::InvalidParameter);
-    };
-    // fxa does constant-time checks for client_id, do that here too.
-    if client_id.as_bytes().ct_eq(data.client_id.as_bytes()).into() {
-        db.delete_oauth_token(&data.token.hash()).await?;
-        Ok(Json(()))
+    // NOTE fxa does a constant-time check for whether the provided client_id matches that
+    // of the token being destroyed. since we only support a public list we can skip that,
+    // which also lets us more easily deal with firefox's habit of destroying tokens that are
+    // already expired
+    if db.delete_oauth_token(&data.token.hash()).await? {
+        Ok(EMPTY)
     } else {
         Err(auth::Error::InvalidParameter)
     }
diff --git a/src/db/mod.rs b/src/db/mod.rs
index 040507f..fa2b62f 100644
--- a/src/db/mod.rs
+++ b/src/db/mod.rs
@@ -559,11 +559,10 @@ impl DbConn {
         Ok(())
     }
 
-    pub(crate) async fn delete_oauth_token(&self, id: &OauthTokenID) -> sqlx::Result<()> {
-        query!(r#"delete from oauth_token where id = $1"#, id as _)
+    pub(crate) async fn delete_oauth_token(&self, id: &OauthTokenID) -> sqlx::Result<bool> {
+        Ok(query!(r#"delete from oauth_token where id = $1"#, id as _)
             .execute(&mut self.get().await?.tx)
-            .await?;
-        Ok(())
+            .await?.rows_affected() > 0)
     }
 
     pub(crate) async fn delete_refresh_token(&self, id: &OauthTokenID) -> sqlx::Result<()> {
-- 
cgit v1.2.3