diff options
| author | Owen Jacobson <owen@grimoire.ca> | 2024-09-11 21:52:57 -0400 |
|---|---|---|
| committer | Owen Jacobson <owen@grimoire.ca> | 2024-09-11 21:54:57 -0400 |
| commit | b16742b0e782bc795fa748d46c3eb6438fb19adc (patch) | |
| tree | 4e31515c57cc896fcdca953c73d62828ac303d1e /src | |
| parent | 4563f221bf61123b15f9608bb14e8f46db05e4f6 (diff) | |
Expire tokens based on when they were last used, not based on when they were issued.
This lets us shorten the expiry interval - by quite a bit. Tokens in regular use will now live indefinitely, while tokens that go unused for _one week_ will be invalidated and deleted. This will reduce the number of "dead" tokens (still valid, but _de facto_ no longer in use) stored in the table, and limit the exposure period if a token is leaked and then not used immediately.
It's also much less likely to produce surprise logouts three months after installation. You'll either stay logged in, or have to log in again much, much sooner, making it feel a lot more regular and less surprising.
Diffstat (limited to 'src')
| -rw-r--r-- | src/login/extract/login.rs | 6 | ||||
| -rw-r--r-- | src/login/repo/tokens.rs | 40 |
2 files changed, 35 insertions, 11 deletions
diff --git a/src/login/extract/login.rs b/src/login/extract/login.rs index b756fa6..405aea8 100644 --- a/src/login/extract/login.rs +++ b/src/login/extract/login.rs @@ -27,14 +27,14 @@ impl FromRequestParts<SqlitePool> for Login { // // let Ok(identity_token) = IdentityToken::from_request_parts(parts, state).await; let identity_token = IdentityToken::from_request_parts(parts, state).await?; - let requested_at = RequestedAt::from_request_parts(parts, state).await?; + let RequestedAt(requested_at) = RequestedAt::from_request_parts(parts, state).await?; let token = identity_token.token().ok_or(LoginError::Forbidden)?; let db = State::<SqlitePool>::from_request_parts(parts, state).await?; let mut tx = db.begin().await?; - tx.tokens().expire(requested_at.timestamp()).await?; - let login = tx.tokens().validate(token).await?; + tx.tokens().expire(requested_at).await?; + let login = tx.tokens().validate(token, requested_at).await?; tx.commit().await?; login.ok_or(LoginError::Forbidden) diff --git a/src/login/repo/tokens.rs b/src/login/repo/tokens.rs index 3ec3d63..39505f1 100644 --- a/src/login/repo/tokens.rs +++ b/src/login/repo/tokens.rs @@ -21,7 +21,7 @@ pub struct Tokens<'t>(&'t mut SqliteConnection); impl<'c> Tokens<'c> { /// Issue a new token for an existing login. The issued_at timestamp will - /// be used to control expiry. + /// be used to control expiry, until the token is actually used. pub async fn issue( &mut self, login: &LoginId, @@ -32,8 +32,8 @@ impl<'c> Tokens<'c> { let secret = sqlx::query_scalar!( r#" insert - into token (secret, login, issued_at) - values ($1, $2, $3) + into token (secret, login, issued_at, last_used_at) + values ($1, $2, $3, $3) returning secret as "secret!" "#, secret, @@ -63,14 +63,17 @@ impl<'c> Tokens<'c> { Ok(()) } + /// Expire and delete all tokens that haven't been used within the expiry + /// interval (right now, 7 days) prior to `expire_at`. Tokens that are in + /// use within that period will be retained. pub async fn expire(&mut self, expire_at: DateTime) -> Result<(), BoxedError> { - // Somewhat arbitrarily, expire after 90 days. - let expired_issue_at = expire_at - TimeDelta::days(90); + // Somewhat arbitrarily, expire after 7 days. + let expired_issue_at = expire_at - TimeDelta::days(7); sqlx::query!( r#" delete from token - where issued_at < $1 + where last_used_at < $1 "#, expired_issue_at, ) @@ -81,8 +84,29 @@ impl<'c> Tokens<'c> { } /// Validate a token by its secret, retrieving the associated Login record. - /// Will return [None] if the token is not valid. - pub async fn validate(&mut self, secret: &str) -> Result<Option<Login>, BoxedError> { + /// Will return [None] if the token is not valid. The token's last-used + /// timestamp will be set to `used_at`. + pub async fn validate( + &mut self, + secret: &str, + used_at: DateTime, + ) -> Result<Option<Login>, BoxedError> { + // I would use `update … returning` to do this in one query, but + // sqlite3, as of this writing, does not allow an update's `returning` + // clause to reference columns from tables joined into the update. Two + // queries is fine, but it feels untidy. + sqlx::query!( + r#" + update token + set last_used_at = $1 + where secret = $2 + "#, + used_at, + secret, + ) + .execute(&mut *self.0) + .await?; + let login = sqlx::query_as!( Login, r#" |
