Skip to content

Move SQL queries out of API authentication middleware #2857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
praseodym merged 10 commits into from
Feb 5, 2026
Merged

Move SQL queries out of API authentication middleware #2857

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9c4e775
Move authentication session module as session_repo module to reposito…
oliver3 Feb 4, 2026
7c79d66
Move session extractor and get_cookie() back to middleware
oliver3 Feb 4, 2026
ecfe834
Remove api AuthenticationError from session_repo, split-off save func…
oliver3 Feb 5, 2026
fedb66f
Separate session create and save
oliver3 Feb 5, 2026
1e40798
Move session create and SESSION_LIFE_TIME to middleware
oliver3 Feb 5, 2026
18ee3e6
Move RequestSessionData as SessionIdentifier to session_repo and conv…
oliver3 Feb 5, 2026
d343135
Add session_repo to Sigrid components
oliver3 Feb 5, 2026
06bf1bc
Revert unnecessary cookie clone
oliver3 Feb 5, 2026
8b59322
Use pub(crate) when possible
oliver3 Feb 5, 2026
b3f0bf2
Remove empty util file
oliver3 Feb 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions backend/src/api/audit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,11 @@ mod tests {
infra::audit_log::{
AuditEvent, AuditLogListResponse, AuditLogUser, AuditService, UserLoggedInDetails,
},
repository::user_repo::{self, User, UserId},
repository::{
session_repo,
session_repo::Session,
user_repo::{self, User, UserId},
},
};

const TEST_USER_AGENT: &str = "TestAgent/1.0";
Expand Down Expand Up @@ -193,15 +197,13 @@ mod tests {
};

let mut conn = pool.acquire().await.unwrap();
let session = crate::api::middleware::authentication::session::create(
&mut conn,
let session = Session::create(
UserId::from(1),
TEST_USER_AGENT,
TEST_IP_ADDRESS,
TimeDelta::seconds(60 * 30),
)
.await
.unwrap();
);
session_repo::save(&mut conn, &session).await.unwrap();

let app = Router::new()
.route("/api/log", get(audit_log_list))
Expand Down Expand Up @@ -263,15 +265,13 @@ mod tests {
};

let mut conn = pool.acquire().await.unwrap();
let session = crate::api::middleware::authentication::session::create(
&mut conn,
let session = Session::create(
UserId::from(1),
TEST_USER_AGENT,
TEST_IP_ADDRESS,
TimeDelta::seconds(60 * 30),
)
.await
.unwrap();
);
session_repo::save(&mut conn, &session).await.unwrap();

let app = Router::new()
.route("/api/log-users", get(audit_log_list_users))
Expand Down
21 changes: 13 additions & 8 deletions backend/src/api/authentication.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,19 @@ use crate::{
APIError, AppState, ErrorResponse, SqlitePoolExt,
api::middleware::authentication::{
IncompleteUser, SECURE_COOKIES, SESSION_COOKIE_NAME, SESSION_LIFE_TIME,
error::AuthenticationError, session,
error::AuthenticationError,
},
domain::role::Role,
error::ErrorReference,
infra::audit_log::{
AuditEvent, AuditService, UserDetails, UserLoggedInDetails, UserLoggedOutDetails,
UserLoginFailedDetails,
},
repository::user_repo::{self, User, UserId},
repository::{
session_repo,
session_repo::Session,
user_repo::{self, User, UserId},
},
};

impl From<AuthenticationError> for APIError {
Expand Down Expand Up @@ -141,10 +145,10 @@ async fn login(
let mut tx = pool.begin_immediate().await?;

// Remove expired sessions, we do this after a login to prevent the necessity of periodical cleanup jobs
session::delete_expired_sessions(&mut tx).await?;
session_repo::delete_expired_sessions(&mut tx).await?;

// Remove possible old session for this user
session::delete_user_session(&mut tx, user.id()).await?;
session_repo::delete_user_session(&mut tx, user.id()).await?;

// Get the client IP address if available
let ip = audit_service
Expand All @@ -153,10 +157,11 @@ async fn login(
.to_string();

// Create a new session and cookie
let session = session::create(&mut tx, user.id(), &user_agent, &ip, SESSION_LIFE_TIME).await?;
let session = Session::create(user.id(), &user_agent, &ip, SESSION_LIFE_TIME);
session_repo::save(&mut tx, &session).await?;

// Log the login event
let logged_in_users_count = session::count(&mut tx).await?;
let logged_in_users_count = session_repo::count(&mut tx).await?;
audit_service
.with_user(user.clone())
.log(
Expand Down Expand Up @@ -395,7 +400,7 @@ async fn logout(
// Log audit event when a valid session exists
let mut tx = pool.begin_immediate().await?;

if let Some(session) = session::get_by_key(&mut tx, session_key)
if let Some(session) = session_repo::get_by_key(&mut tx, session_key)
.await
.ok()
.flatten()
Expand All @@ -413,7 +418,7 @@ async fn logout(
}

// Remove session from the database
session::delete(&mut tx, session_key).await?;
session_repo::delete(&mut tx, session_key).await?;

// Commit the transaction
tx.commit().await?;
Expand Down
71 changes: 28 additions & 43 deletions backend/src/api/middleware/authentication/middleware.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,25 @@ use chrono::Utc;
use sqlx::SqlitePool;
use tracing::{debug, error, info};

use super::{SESSION_MIN_LIFE_TIME, request_data::RequestSessionData, session::Session};
use super::{SESSION_LIFE_TIME, SESSION_MIN_LIFE_TIME, session::get_expires_at};
use crate::{
SqlitePoolExt,
api::authentication::set_default_cookie_properties,
infra::audit_log::{AuditEvent, AuditService},
repository::{user_repo, user_repo::User},
repository::{
session_repo,
session_repo::{Session, SessionIdentifier},
user_repo,
user_repo::User,
},
};

/// Inject user and session
pub async fn inject_user(
State(pool): State<SqlitePool>,
mut request: Request<Body>,
) -> Request<Body> {
let Some(request_data) = request.extract_parts::<RequestSessionData>().await.ok() else {
let Some(session_id) = request.extract_parts::<SessionIdentifier>().await.ok() else {
return request;
};

Expand All @@ -31,8 +36,7 @@ pub async fn inject_user(
return request;
};

let Ok(Some(session)) = super::session::get_by_request_data(&mut conn, &request_data).await
else {
let Ok(Some(session)) = session_repo::get_by_identifier(&mut conn, &session_id).await else {
return request;
};

Expand Down Expand Up @@ -78,7 +82,13 @@ pub async fn extend_session(
// extend lifetime of session and set new cookie if the session is still valid and will soon expire
if (expires - now) < SESSION_MIN_LIFE_TIME && expires > now && !do_not_extend {
match pool.begin_immediate().await {
Ok(mut tx) => match super::session::extend_session(&mut tx, &session).await {
Ok(mut tx) => match session_repo::extend_session(
&mut tx,
&session,
get_expires_at(SESSION_LIFE_TIME),
)
.await
{
Ok(session) => {
let _ = audit_service
.with_user(user.clone())
Expand Down Expand Up @@ -131,11 +141,9 @@ mod test {

use super::*;
use crate::{
api::middleware::authentication::{
DO_NOT_EXTEND_SESSION_HEADER, SESSION_LIFE_TIME, session,
},
api::middleware::authentication::{DO_NOT_EXTEND_SESSION_HEADER, SESSION_LIFE_TIME},
domain::role::Role,
repository::user_repo::UserId,
repository::{session_repo, user_repo::UserId},
};

const TEST_USER_AGENT: &str = "TestAgent/1.0";
Expand All @@ -152,15 +160,13 @@ mod test {

let user = User::test_user(Role::Administrator, UserId::from(1));
let mut conn = pool.acquire().await.unwrap();
let session = session::create(
&mut conn,
let session = Session::create(
user.id(),
TEST_USER_AGENT,
TEST_IP_ADDRESS,
SESSION_LIFE_TIME,
)
.await
.unwrap();
);
session_repo::save(&mut conn, &session).await.unwrap();

let cookie = session.get_cookie();

Expand Down Expand Up @@ -216,15 +222,8 @@ mod test {

let life_time = SESSION_MIN_LIFE_TIME + TimeDelta::seconds(30); // min life time + 30 seconds
let mut conn = pool.acquire().await.unwrap();
let session = session::create(
&mut conn,
user.id(),
TEST_USER_AGENT,
TEST_IP_ADDRESS,
life_time,
)
.await
.unwrap();
let session = Session::create(user.id(), TEST_USER_AGENT, TEST_IP_ADDRESS, life_time);
session_repo::save(&mut conn, &session).await.unwrap();

let updated_response = extend_session(
State(pool.clone()),
Expand All @@ -250,15 +249,8 @@ mod test {
);

let life_time = SESSION_MIN_LIFE_TIME - TimeDelta::seconds(30); // min life time - 30 seconds
let session = session::create(
&mut conn,
user.id(),
TEST_USER_AGENT,
TEST_IP_ADDRESS,
life_time,
)
.await
.unwrap();
let session = Session::create(user.id(), TEST_USER_AGENT, TEST_IP_ADDRESS, life_time);
session_repo::save(&mut conn, &session).await.unwrap();

let updated_response = extend_session(
State(pool.clone()),
Expand All @@ -273,7 +265,7 @@ mod test {
let cookie = updated_response.headers().get(SET_COOKIE).unwrap();
let cookie = Cookie::from_str(cookie.to_str().unwrap()).unwrap();

let new_session = session::get_by_key(&mut conn, cookie.value())
let new_session = session_repo::get_by_key(&mut conn, cookie.value())
.await
.unwrap()
.unwrap();
Expand Down Expand Up @@ -301,15 +293,8 @@ mod test {

let life_time = SESSION_MIN_LIFE_TIME - TimeDelta::seconds(30); // min life time - 30 seconds
let mut conn = pool.acquire().await.unwrap();
let session = session::create(
&mut conn,
user.id(),
TEST_USER_AGENT,
TEST_IP_ADDRESS,
life_time,
)
.await
.unwrap();
let session = Session::create(user.id(), TEST_USER_AGENT, TEST_IP_ADDRESS, life_time);
session_repo::save(&mut conn, &session).await.unwrap();

let mut headers = HeaderMap::new();
headers.insert(
Expand Down
42 changes: 18 additions & 24 deletions backend/src/api/middleware/authentication/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ pub use role::{Admin, AdminOrCoordinator, Coordinator, IncompleteUser, Typist};
pub mod error;
mod middleware;
pub mod password;
pub mod request_data;
mod role;
pub mod session;
mod util;
mod session;
pub mod session_identifier;

/// Session lifetime, for both cookie and database
/// Also change the translation string "users.session_expired" in the frontend if this value is changed
Expand Down Expand Up @@ -56,7 +55,10 @@ mod tests {
domain::role::Role,
error::ErrorReference,
infra::audit_log::{AuditEvent, LogFilter, UserLoginFailedDetails},
repository::user_repo::{self, User, UserId},
repository::{
session_repo::{self, Session},
user_repo::{self, User, UserId},
},
};

const TEST_USER_AGENT: &str = "TestAgent/1.0";
Expand Down Expand Up @@ -494,15 +496,13 @@ mod tests {
async fn test_list(pool: SqlitePool) {
let app = create_app(pool.clone());
let mut conn = pool.acquire().await.unwrap();
let session = session::create(
&mut conn,
let session = Session::create(
UserId::from(1),
TEST_USER_AGENT,
TEST_IP_ADDRESS,
SESSION_LIFE_TIME,
)
.await
.unwrap();
);
session_repo::save(&mut conn, &session).await.unwrap();
let mut cookie = session.get_cookie();
set_default_cookie_properties(&mut cookie);
let response = app
Expand Down Expand Up @@ -531,15 +531,13 @@ mod tests {

// with a normal long-valid session the user should not get a new cookie
let mut conn = pool.acquire().await.unwrap();
let session = session::create(
&mut conn,
let session = Session::create(
UserId::from(1),
TEST_USER_AGENT,
TEST_IP_ADDRESS,
SESSION_LIFE_TIME,
)
.await
.unwrap();
);
session_repo::save(&mut conn, &session).await.unwrap();
let mut cookie = session.get_cookie();
set_default_cookie_properties(&mut cookie);

Expand All @@ -561,15 +559,13 @@ mod tests {
assert_eq!(response.headers().get("set-cookie"), None);

// with a session that is about to expire the user should get a new cookie, and the session lifetime should be extended
let session: session::Session = session::create(
&mut conn,
let session = Session::create(
UserId::from(1),
TEST_USER_AGENT,
TEST_IP_ADDRESS,
SESSION_MIN_LIFE_TIME / 2,
)
.await
.unwrap();
);
session_repo::save(&mut conn, &session).await.unwrap();
let mut cookie = session.get_cookie();
set_default_cookie_properties(&mut cookie);

Expand Down Expand Up @@ -670,15 +666,13 @@ mod tests {
let app = create_app(pool.clone());
// user id 5 is a typist
let mut conn = pool.acquire().await.unwrap();
let session = session::create(
&mut conn,
let session = Session::create(
UserId::from(5),
TEST_USER_AGENT,
TEST_IP_ADDRESS,
SESSION_LIFE_TIME,
)
.await
.unwrap();
);
session_repo::save(&mut conn, &session).await.unwrap();
let mut cookie = session.get_cookie();
set_default_cookie_properties(&mut cookie);
let response = app
Expand Down
Loading
Loading