Merge pull request #16 from korpling/feature/check-code-coverage

Check code coverage

.github/workflows/code_coverage.yml

@@ -1,6 +1,10 @@
 name: Code Coverage
 
-on: [pull_request, push]
+on:
+  push:
+    branches:
+      - main
+  pull_request:
 
 jobs:
   coverage:

.github/workflows/rust.yml

@@ -1,6 +1,5 @@
 on:
   push:
-  merge_group:
 
 name: Rust
 

.gitignore

@@ -2,3 +2,4 @@
 *.db
 *.db-shm
 *.db-wal
+/lcov.info

README.md

@@ -1,3 +1,6 @@
+[![codecov](https://codecov.io/gh/korpling/annis-web/graph/badge.svg?token=FX7LX6OA37)](https://codecov.io/gh/korpling/annis-web)
+
+
 # ANNIS frontend experiments
 
 ANNIS is an open source, versatile web browser-based search and visualization
@@ -42,11 +45,11 @@ We recommend installing the following Cargo subcommands for developing annis-web
   third party license file
 - [cargo-watch](https://crates.io/crates/cargo-watch) allows automatic re-compilation
 - [cargo-llvm-cov](https://crates.io/crates/cargo-llvm-cov) for determining the code coverage
-- [cargo-insta](https://crates.io/crates/cargo-insta) allows to review the test snapshot files.
+- [cargo-insta](https://crates.io/crates/cargo-insta) allows reviewing the test snapshot files.
 
 ### Running the web server
 
-When developing, you can run a webserver that is automatically re-compiled when
+When developing, you can run a web server that is automatically re-compiled when
 any of the source files changes.
 
 ```bash

about.toml

@@ -14,6 +14,8 @@ accepted = [
   "Unicode-DFS-2016",
 ]
 
+no-clearly-defined = true
+
 workarounds = [
   "ring",
 ]

src/auth.rs

@@ -20,35 +20,17 @@ pub struct LoginInfo {
     oauth_token: AnnisTokenResponse,
 
     /// Unix time stamp when the session attached to this login information expires.
-    pub user_session_expiry: Option<i64>,
+    user_session_expiry: Option<i64>,
 
     /// An authentificated HTTP client
     client: reqwest::Client,
 }
 
-fn parse_unverified_username(token: &str) -> Result<Option<String>> {
-    let splitted: Vec<_> = token.splitn(3, '.').collect();
-
-    if let Some(raw_claims) = splitted.get(1) {
-        let claims_json = base64::engine::general_purpose::URL_SAFE_NO_PAD.decode(raw_claims)?;
-        let claims: serde_json::Value =
-            serde_json::from_str(&String::from_utf8_lossy(&claims_json))?;
-        let mut user_name = None;
-        if let Some(claims) = claims.as_object() {
-            if let Some(preferred_username) = claims.get("preferred_username") {
-                user_name = preferred_username.as_str();
-            } else if let Some(sub) = claims.get("sub") {
-                user_name = sub.as_str();
-            }
-        }
-        Ok(user_name.map(str::to_string))
-    } else {
-        Err(AppError::JwtMissingPayload)
-    }
-}
-
 impl LoginInfo {
-    pub fn new(oauth_token: AnnisTokenResponse, user_session_expiry: Option<i64>) -> Result<Self> {
+    pub fn from_token(
+        oauth_token: AnnisTokenResponse,
+        user_session_expiry: Option<i64>,
+    ) -> Result<Self> {
         let mut default_headers = reqwest::header::HeaderMap::new();
 
         let value =
@@ -64,6 +46,14 @@ impl LoginInfo {
         Ok(result)
     }
 
+    pub fn expires_unix(&self) -> Option<i64> {
+        self.user_session_expiry
+    }
+
+    pub fn set_expiration_unix(&mut self, exp: Option<i64>) {
+        self.user_session_expiry = exp;
+    }
+
     pub fn renew_token(&mut self, oauth_token: AnnisTokenResponse) -> Result<()> {
         self.oauth_token = oauth_token;
         // Also recreate the HTTP client, because it needs to use the new bearer token as default header
@@ -96,6 +86,27 @@ impl LoginInfo {
     }
 }
 
+fn parse_unverified_username(token: &str) -> Result<Option<String>> {
+    let splitted: Vec<_> = token.splitn(3, '.').collect();
+
+    if let Some(raw_claims) = splitted.get(1) {
+        let claims_json = base64::engine::general_purpose::URL_SAFE_NO_PAD.decode(raw_claims)?;
+        let claims: serde_json::Value =
+            serde_json::from_str(&String::from_utf8_lossy(&claims_json))?;
+        let mut user_name = None;
+        if let Some(claims) = claims.as_object() {
+            if let Some(preferred_username) = claims.get("preferred_username") {
+                user_name = preferred_username.as_str();
+            } else if let Some(sub) = claims.get("sub") {
+                user_name = sub.as_str();
+            }
+        }
+        Ok(user_name.map(str::to_string))
+    } else {
+        Err(AppError::JwtMissingPayload)
+    }
+}
+
 async fn refresh_token_action(
     refresh_instant: Instant,
     refresh_token: RefreshToken,
@@ -222,7 +233,7 @@ mod tests {
 
         app_state.login_info.insert(
             session_id.to_string(),
-            LoginInfo::new(token.clone(), None).unwrap(),
+            LoginInfo::from_token(token.clone(), None).unwrap(),
         );
 
         let token_request_time = Instant::now();

src/errors.rs

@@ -4,6 +4,7 @@ use axum::{
     http::{self, header::InvalidHeaderValue, StatusCode},
     response::{Html, IntoResponse},
 };
+use chrono::OutOfRangeError;
 use minijinja::context;
 use oauth2::{basic::BasicErrorResponseType, StandardErrorResponse};
 use reqwest::Url;
@@ -141,6 +142,8 @@ pub enum AppError {
         code: http::StatusCode,
         message: String,
     },
+    #[error(transparent)]
+    ChronoOutOfRangeError(#[from] OutOfRangeError),
 }
 
 impl From<(http::StatusCode, &'static str)> for AppError {

src/lib.rs

@@ -15,10 +15,11 @@ use axum::{
     routing::get,
     BoxError, Router,
 };
+use chrono::Duration;
 use config::CliConfig;
 use include_dir::{include_dir, Dir};
 use state::GlobalAppState;
-use std::{sync::Arc, time::Duration};
+use std::sync::Arc;
 use tower::ServiceBuilder;
 use tower_sessions::{
     cookie::SameSite, sqlx::SqlitePool, MokaStore, SessionManagerLayer, SessionStore, SqliteStore,
@@ -48,7 +49,7 @@ async fn static_file(Path(path): Path<String>) -> Result<impl IntoResponse> {
     Ok(response)
 }
 
-pub async fn app(config: &CliConfig) -> Result<Router> {
+pub async fn app(config: &CliConfig, cleanup_interval: Duration) -> Result<Router> {
     let global_state = GlobalAppState::new(config)?;
     let global_state = Arc::new(global_state);
 
@@ -61,20 +62,21 @@ pub async fn app(config: &CliConfig) -> Result<Router> {
         tokio::task::spawn(
             store
                 .clone()
-                .continuously_delete_expired(Duration::from_secs(60 * 60)),
+                .continuously_delete_expired(cleanup_interval.to_std()?),
         );
 
-        app_with_state(global_state, store).await
+        app_with_state(global_state, store, cleanup_interval).await
     } else {
         // Fallback to a a store based on a cache
         let store = MokaStore::new(Some(1_000));
-        app_with_state(global_state, store).await
+        app_with_state(global_state, store, cleanup_interval).await
     }
 }
 
 async fn app_with_state<S: SessionStore>(
     global_state: Arc<GlobalAppState>,
     session_store: S,
+    cleanup_interval: Duration,
 ) -> Result<Router> {
     let routes = Router::new()
         .route("/", get(|| async { Redirect::temporary("corpora") }))
@@ -90,10 +92,11 @@ async fn app_with_state<S: SessionStore>(
             StatusCode::BAD_REQUEST
         }))
         .layer(SessionManagerLayer::new(session_store).with_same_site(SameSite::Lax));
+    let cleanup_interval = cleanup_interval.to_std()?;
 
     tokio::task::spawn(async move {
         loop {
-            tokio::time::sleep(Duration::from_secs(60)).await;
+            tokio::time::sleep(cleanup_interval).await;
             global_state.cleanup().await;
         }
     });

src/main.rs

@@ -1,4 +1,5 @@
 use annis_web::{app, config::CliConfig};
+use chrono::Duration;
 use clap::Parser;
 use std::{net::SocketAddr, str::FromStr};
 use tracing::{error, info};
@@ -13,7 +14,7 @@ async fn main() {
     let cli = CliConfig::parse();
 
     let addr = SocketAddr::from(([127, 0, 0, 1], cli.port));
-    match app(&cli).await {
+    match app(&cli, Duration::hours(1)).await {
         Ok(router) => {
             info!("Starting server with address http://{addr}", addr = addr);
             let server = axum::Server::bind(&addr).serve(router.into_make_service());

src/state.rs

@@ -1,3 +1,5 @@
+use crate::auth::LoginInfo;
+use crate::{config::CliConfig, errors::AppError, Result, TEMPLATES_DIR};
 use axum::{async_trait, extract::FromRequestParts, http::request::Parts};
 use chrono::Utc;
 use dashmap::DashMap;
@@ -10,8 +12,6 @@ use time::OffsetDateTime;
 use tokio::{sync::mpsc::Receiver, task::JoinHandle};
 use url::Url;
 
-use crate::{auth::LoginInfo, config::CliConfig, errors::AppError, Result, TEMPLATES_DIR};
-
 #[derive(Serialize, Deserialize, Debug, Clone, Default)]
 pub struct Session {
     selected_corpora: BTreeSet<String>,
@@ -187,16 +187,17 @@ impl GlobalAppState {
             self.login_info
                 .alter(&session.id().to_string(), |_, mut l| {
                     if let (Some(old_expiry), Some(new_expiry)) =
-                        (l.user_session_expiry, session.expiration_time())
+                        (l.expires_unix(), session.expiration_time())
                     {
                         // Check if the new expiration date is actually longer before replacing it
                         if old_expiry < new_expiry.unix_timestamp() {
-                            l.user_session_expiry = Some(new_expiry.unix_timestamp());
+                            l.set_expiration_unix(Some(new_expiry.unix_timestamp()));
                         }
                     } else {
                         // Use the new expiration date
-                        l.user_session_expiry =
-                            session.expiration_time().map(|t| t.unix_timestamp());
+                        l.set_expiration_unix(
+                            session.expiration_time().map(|t| t.unix_timestamp()),
+                        );
                     }
                     l
                 });
@@ -214,11 +215,100 @@ impl GlobalAppState {
     /// Cleans up ressources coupled to sessions that are expired or non-existing.
     pub async fn cleanup(&self) {
         self.login_info.retain(|_session_id, login_info| {
-            if let Some(expiry) = login_info.user_session_expiry {
+            if let Some(expiry) = login_info.expires_unix() {
                 Utc::now().timestamp() < expiry
             } else {
                 true
             }
         });
     }
 }
+
+#[cfg(test)]
+mod tests {
+
+    use crate::config::CliConfig;
+
+    use super::*;
+
+    use oauth2::{basic::BasicTokenType, AccessToken, StandardTokenResponse};
+
+    #[test]
+    fn client_access_time_updated_existing() {
+        let config = CliConfig::default();
+        let state = GlobalAppState::new(&config).unwrap();
+
+        // Create a session that should be updated when accessed
+        let now = OffsetDateTime::now_utc();
+
+        // The user session will only expire in 1 day
+        let session_expiration = now.checked_add(time::Duration::days(1)).unwrap();
+        let raw_session = tower_sessions::Session::new(Some(session_expiration));
+        let session_id = raw_session.id().to_string();
+
+        let mut session = Session::default();
+        session.session_id = session_id.clone();
+        session.session = raw_session;
+
+        let access_token = AccessToken::new("ABC".into());
+        let token_response = StandardTokenResponse::new(
+            access_token,
+            BasicTokenType::Bearer,
+            oauth2::EmptyExtraTokenFields {},
+        );
+        // Simulate an old access to the login info, which would trigger a cleanup
+        let expired_login_info =
+            LoginInfo::from_token(token_response, Some(now.unix_timestamp() - 1)).unwrap();
+
+        state
+            .login_info
+            .insert(session.session_id.clone(), expired_login_info.clone());
+
+        let session_arg = SessionArg::Session(session.clone());
+        state.create_client(&session_arg).unwrap();
+        // The login info expiration time must be updated to match the session
+        assert_eq!(
+            Some(session_expiration.unix_timestamp()),
+            state.login_info.get(&session_id).unwrap().expires_unix()
+        );
+    }
+
+    #[test]
+    fn client_access_time_updated_set_from_session() {
+        let config = CliConfig::default();
+        let state = GlobalAppState::new(&config).unwrap();
+
+        // Create a session that should be updated when accessed
+        let now = OffsetDateTime::now_utc();
+
+        // The user session will only expire in 1 day
+        let session_expiration = now.checked_add(time::Duration::days(1)).unwrap();
+        let raw_session = tower_sessions::Session::new(Some(session_expiration));
+        let session_id = raw_session.id().to_string();
+
+        let mut session = Session::default();
+        session.session_id = session_id.clone();
+        session.session = raw_session;
+
+        let access_token = AccessToken::new("ABC".into());
+        let token_response = StandardTokenResponse::new(
+            access_token,
+            BasicTokenType::Bearer,
+            oauth2::EmptyExtraTokenFields {},
+        );
+        // Simulate an old access to the login info, which does not have a expiration date
+        let expired_login_info = LoginInfo::from_token(token_response, None).unwrap();
+
+        state
+            .login_info
+            .insert(session.session_id.clone(), expired_login_info.clone());
+
+        let session_arg = SessionArg::Session(session.clone());
+        state.create_client(&session_arg).unwrap();
+        // The login info expiration time must be updated to match the session
+        assert_eq!(
+            Some(session_expiration.unix_timestamp()),
+            state.login_info.get(&session_id).unwrap().expires_unix()
+        );
+    }
+}