Skip to content

Commit

Permalink
Added update user last_activity_at (#1013)
Browse files Browse the repository at this point in the history
Co-authored-by: Ellen <lionqueen94@gmail.com>
  • Loading branch information
cikzh and Lionqueen94 authored Feb 18, 2025
1 parent 0477efe commit a3d10d7
Show file tree
Hide file tree
Showing 6 changed files with 180 additions and 8 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions backend/src/authentication/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,7 @@ pub async fn development_login(
Ok((updated_jar, Json(LoginResponse::from(&user))))
}

#[derive(Serialize, ToSchema)]
#[cfg_attr(test, derive(Deserialize))]
#[derive(Serialize, Deserialize, ToSchema)]
pub struct UserListResponse {
pub users: Vec<User>,
}
Expand Down
3 changes: 3 additions & 0 deletions backend/src/authentication/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ use utoipa::ToSchema;

pub use self::api::*;

#[cfg(test)]
pub use self::session::Sessions;

mod api;
pub mod error;
mod password;
Expand Down
81 changes: 77 additions & 4 deletions backend/src/authentication/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use axum::{
use axum_extra::extract::CookieJar;
use chrono::{DateTime, Utc};
use serde::{Deserialize, Serialize};
use sqlx::{query_as, Error, FromRow, SqlitePool};
use sqlx::{query, query_as, Error, FromRow, SqlitePool};
use utoipa::ToSchema;

use crate::{APIError, AppState};
Expand All @@ -18,6 +18,8 @@ use super::{
SESSION_COOKIE_NAME,
};

const MIN_UPDATE_LAST_ACTIVITY_AT_SECS: i64 = 60; // 1 minute

/// User object, corresponds to a row in the users table
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, Hash, FromRow, ToSchema)]
pub struct User {
Expand Down Expand Up @@ -49,6 +51,33 @@ impl User {
&self.username
}

pub fn last_activity_at(&self) -> Option<DateTime<Utc>> {
self.last_activity_at
}

/// Updates the `last_activity_at` field, but first checks if it has been
/// longer than `MIN_UPDATE_LAST_ACTIVITY_AT_SECS`, to prevent excessive
/// database writes.
pub async fn update_last_activity_at(&self, users: &Users) -> Result<(), sqlx::Error> {
if self.should_update_last_activity_at() {
users.update_last_activity_at(self.id()).await?;
}

Ok(())
}

fn should_update_last_activity_at(&self) -> bool {
if let Some(last_activity_at) = self.last_activity_at {
chrono::Utc::now()
.signed_duration_since(last_activity_at)
.num_seconds()
> MIN_UPDATE_LAST_ACTIVITY_AT_SECS
} else {
// Also update when no timestamp is set yet
true
}
}

#[cfg(test)]
pub fn fullname(&self) -> Option<&str> {
self.fullname.as_deref()
Expand Down Expand Up @@ -77,7 +106,9 @@ where
return Err(AuthenticationError::NoSessionCookie.into());
};

Ok(users.get_by_session_key(session_cookie.value()).await?)
let user = users.get_by_session_key(session_cookie.value()).await?;
user.update_last_activity_at(&users).await?;
Ok(user)
}
}

Expand All @@ -102,7 +133,10 @@ where
};

match users.get_by_session_key(session_cookie.value()).await {
Ok(user) => Ok(Some(user)),
Ok(user) => {
user.update_last_activity_at(&users).await?;
Ok(Some(user))
}
Err(AuthenticationError::UserNotFound)
| Err(AuthenticationError::SessionKeyNotFound) => Ok(None),
Err(e) => Err(e.into()),
Expand Down Expand Up @@ -336,6 +370,16 @@ impl Users {
.await?;
Ok(users)
}

pub async fn update_last_activity_at(&self, user_id: u32) -> Result<(), Error> {
query!(
r#"UPDATE users SET last_activity_at = CURRENT_TIMESTAMP WHERE id = ?"#,
user_id,
)
.fetch_all(&self.0)
.await?;
Ok(())
}
}

impl FromRef<AppState> for Users {
Expand All @@ -351,7 +395,10 @@ mod tests {
use test_log::test;

use crate::authentication::{
error::AuthenticationError, role::Role, session::Sessions, user::Users,
error::AuthenticationError,
role::Role,
session::Sessions,
user::{User, Users},
};

#[test(sqlx::test)]
Expand Down Expand Up @@ -544,4 +591,30 @@ mod tests {
assert_eq!(serialized_user["username"], "test_user".to_string());
assert!(serialized_user.get("password_hash").is_none());
}

#[test]
fn test_should_update_last_activity_at() {
let mut user = User {
id: 2,
username: "user1".to_string(),
fullname: Some("Full Name".to_string()),
role: Role::Typist,
needs_password_change: false,
password_hash: "h4sh".to_string(),
last_activity_at: None,
updated_at: chrono::Utc::now(),
created_at: chrono::Utc::now(),
};

// Should update when no timestamp is net
assert!(user.should_update_last_activity_at());

// Should not update when trying to update too soon
user.last_activity_at = Some(chrono::Utc::now());
assert!(!user.should_update_last_activity_at());

// Should update when `last_activity_at` was 2 minutes ago
user.last_activity_at = Some(chrono::Utc::now() - chrono::Duration::minutes(2));
assert!(user.should_update_last_activity_at());
}
}
28 changes: 26 additions & 2 deletions backend/tests/shared/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#![cfg(test)]

use axum::http::StatusCode;
use reqwest::Client;
use axum::http::{HeaderValue, StatusCode};
use hyper::header::CONTENT_TYPE;
use reqwest::{Body, Client};
use serde_json::json;
use std::net::SocketAddr;

use abacus::data_entry::{
Expand Down Expand Up @@ -187,3 +189,25 @@ pub async fn create_result_with_non_example_data_entry(
.await;
check_data_entry_status_is_definitive(addr, polling_station_id, election_id).await;
}

/// Calls the login endpoint and returns the session cookie
pub async fn login(addr: &SocketAddr) -> Option<HeaderValue> {
let url = format!("http://{addr}/api/user/login");

let response = reqwest::Client::new()
.post(&url)
.header(CONTENT_TYPE, "application/json")
.body(Body::from(
json!({
"username": "user",
"password": "password",
})
.to_string(),
))
.send()
.await
.unwrap();

assert_eq!(response.status(), StatusCode::OK);
response.headers().get("set-cookie").cloned()
}
61 changes: 61 additions & 0 deletions backend/tests/user_integration_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
#![cfg(test)]

use abacus::authentication::UserListResponse;
use hyper::{header::CONTENT_TYPE, StatusCode};
use reqwest::Body;
use serde_json::json;
use sqlx::SqlitePool;
use test_log::test;
use utils::serve_api;

pub mod shared;
pub mod utils;
#[test(sqlx::test(fixtures(path = "../fixtures", scripts("election_2", "users"))))]
async fn test_user_last_activity_at_updating(pool: SqlitePool) {
// Assert the user has no last activity timestamp yet
let addr = serve_api(pool).await;
let url = format!("http://{addr}/api/user");
let response = reqwest::Client::new().get(&url).send().await.unwrap();
assert_eq!(response.status(), StatusCode::OK);
let body: UserListResponse = response.json().await.unwrap();
let user = body.users.first().unwrap();
assert!(user.last_activity_at().is_none());

// Login, so we can call the whoami endpoint
let url = format!("http://{addr}/api/user/login");
let response = reqwest::Client::new()
.post(&url)
.header(CONTENT_TYPE, "application/json")
.body(Body::from(
json!({
"username": "user",
"password": "password",
})
.to_string(),
))
.send()
.await
.unwrap();

assert_eq!(response.status(), StatusCode::OK);

let cookie = shared::login(&addr).await.unwrap();

// Call an endpoint using the `FromRequestParts` for `User`
let url = format!("http://{addr}/api/user/whoami");
let response = reqwest::Client::new()
.get(&url)
.header("cookie", &cookie)
.send()
.await
.unwrap();
assert_eq!(response.status(), StatusCode::OK);

// Test that a timestamp is present
let url = format!("http://{addr}/api/user");
let response = reqwest::Client::new().get(&url).send().await.unwrap();
assert_eq!(response.status(), StatusCode::OK);
let body: UserListResponse = response.json().await.unwrap();
let user = body.users.first().unwrap();
assert!(user.last_activity_at().is_some());
}

0 comments on commit a3d10d7

Please sign in to comment.