summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorpennae <github@quasiparticle.net>2022-07-13 18:09:19 +0200
committerpennae <github@quasiparticle.net>2022-07-13 18:09:19 +0200
commitd6da876cabe0180acd0ebca173d973c8d3450d99 (patch)
treed0a76d4c9a2e83d918e87258865f11ae2c4b49af
parentd62d1be05ca16a4836ad66440fda477f4ed6817a (diff)
downloadminor-skulk-d6da876cabe0180acd0ebca173d973c8d3450d99.tar.gz
minor-skulk-d6da876cabe0180acd0ebca173d973c8d3450d99.tar.xz
minor-skulk-d6da876cabe0180acd0ebca173d973c8d3450d99.zip
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.
-rw-r--r--migrations/20220713142453_token_deletion_grace_period.down.sql2
-rw-r--r--migrations/20220713142453_token_deletion_grace_period.up.sql13
-rw-r--r--src/api/auth/oauth.rs21
-rw-r--r--src/db/mod.rs7
4 files changed, 27 insertions, 16 deletions
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<()> {