Skip to content

Commit 6da9f67

Browse files
committed
Cleanup revoked tokens instead of expired ones
If we continue deleting expired tokens, we might not record whether the token was used or not, and not know what to do in case of a double-refresh. Revoked tokens are safe to delete. This also reduces the frequency of the cleanup job to once an hour.
1 parent 4054cf2 commit 6da9f67

File tree

5 files changed

+13
-13
lines changed

5 files changed

+13
-13
lines changed
Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/storage-pg/src/oauth2/access_token.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -250,20 +250,20 @@ impl OAuth2AccessTokenRepository for PgOAuth2AccessTokenRepository<'_> {
250250
}
251251

252252
#[tracing::instrument(
253-
name = "db.oauth2_access_token.cleanup_expired",
253+
name = "db.oauth2_access_token.cleanup_revoked",
254254
skip_all,
255255
fields(
256256
db.query.text,
257257
),
258258
err,
259259
)]
260-
async fn cleanup_expired(&mut self, clock: &dyn Clock) -> Result<usize, Self::Error> {
261-
// Cleanup token which expired more than 15 minutes ago
262-
let threshold = clock.now() - Duration::microseconds(15 * 60 * 1000 * 1000);
260+
async fn cleanup_revoked(&mut self, clock: &dyn Clock) -> Result<usize, Self::Error> {
261+
// Cleanup token that were revoked more than an hour ago
262+
let threshold = clock.now() - Duration::microseconds(60 * 60 * 1000 * 1000);
263263
let res = sqlx::query!(
264264
r#"
265265
DELETE FROM oauth2_access_tokens
266-
WHERE expires_at < $1
266+
WHERE revoked_at < $1
267267
"#,
268268
threshold,
269269
)

crates/storage/src/oauth2/access_token.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ pub trait OAuth2AccessTokenRepository: Send + Sync {
107107
access_token: AccessToken,
108108
) -> Result<AccessToken, Self::Error>;
109109

110-
/// Cleanup expired access tokens
110+
/// Cleanup revoked access tokens
111111
///
112112
/// Returns the number of access tokens that were cleaned up
113113
///
@@ -118,7 +118,7 @@ pub trait OAuth2AccessTokenRepository: Send + Sync {
118118
/// # Errors
119119
///
120120
/// Returns [`Self::Error`] if the underlying repository fails
121-
async fn cleanup_expired(&mut self, clock: &dyn Clock) -> Result<usize, Self::Error>;
121+
async fn cleanup_revoked(&mut self, clock: &dyn Clock) -> Result<usize, Self::Error>;
122122
}
123123

124124
repository_impl!(OAuth2AccessTokenRepository:
@@ -150,5 +150,5 @@ repository_impl!(OAuth2AccessTokenRepository:
150150
access_token: AccessToken,
151151
) -> Result<AccessToken, Self::Error>;
152152

153-
async fn cleanup_expired(&mut self, clock: &dyn Clock) -> Result<usize, Self::Error>;
153+
async fn cleanup_revoked(&mut self, clock: &dyn Clock) -> Result<usize, Self::Error>;
154154
);

crates/tasks/src/database.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,15 @@ impl RunnableJob for CleanupExpiredTokensJob {
2424

2525
let count = repo
2626
.oauth2_access_token()
27-
.cleanup_expired(&clock)
27+
.cleanup_revoked(&clock)
2828
.await
2929
.map_err(JobError::retry)?;
3030
repo.save().await.map_err(JobError::retry)?;
3131

3232
if count == 0 {
3333
debug!("no token to clean up");
3434
} else {
35-
info!(count, "cleaned up expired tokens");
35+
info!(count, "cleaned up revoked tokens");
3636
}
3737

3838
Ok(())

crates/tasks/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ pub async fn init(
121121
.register_handler::<mas_storage::queue::VerifyEmailJob>()
122122
.add_schedule(
123123
"cleanup-expired-tokens",
124-
"*/15 * * * * *".parse()?,
124+
"0 0 * * * *".parse()?,
125125
mas_storage::queue::CleanupExpiredTokensJob,
126126
);
127127

0 commit comments

Comments
 (0)