Merge pull request 'Update actix-identity to version 0.5.2' (#27) from actix_identity into master

Reviewed-on: #27

Cargo.lock

@@ -82,17 +82,18 @@ dependencies = [
 
 [[package]]
 name = "actix-identity"
-version = "0.4.0"
+version = "0.5.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "171fe3ed055b2dd50c61967911d253d47e76e1d4308acfbf99fc7affe5ec42aa"
+checksum = "1224c9f9593dc27c9077b233ce04adedc1d7febcfc35ee9f53ea3c24df180bec"
 dependencies = [
  "actix-service",
+ "actix-session",
  "actix-utils",
  "actix-web",
- "futures-util",
+ "anyhow",
+ "futures-core",
  "serde",
- "serde_json",
+ "tracing",
- "time",
 ]
 
 [[package]]
@@ -158,6 +159,23 @@ dependencies = [
  "pin-project-lite",
 ]
 
+[[package]]
+name = "actix-session"
+version = "0.7.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "861c2463ccba4af8f272936fcf4999af6305492fc939bf0dfe71db86142ae843"
+dependencies = [
+ "actix-service",
+ "actix-utils",
+ "actix-web",
+ "anyhow",
+ "async-trait",
+ "derive_more",
+ "serde",
+ "serde_json",
+ "tracing",
+]
+
 [[package]]
 name = "actix-utils"
 version = "3.0.0"
@@ -367,6 +385,17 @@ dependencies = [
  "toml",
 ]
 
+[[package]]
+name = "async-trait"
+version = "0.1.56"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "96cf8829f67d2eab0b2dfa42c5d0ef737e0724e4a82b01b3e292456202b19716"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "atty"
 version = "0.2.14"
@@ -423,6 +452,7 @@ version = "0.1.0"
 dependencies = [
  "actix",
  "actix-identity",
+ "actix-session",
  "actix-web",
  "aes-gcm",
  "askama",

Cargo.toml

@@ -7,8 +7,9 @@ edition = "2021"
 
 [dependencies]
 actix = "0.13.0"
-actix-identity = "0.4.0"
+actix-identity = "0.5.2"
 actix-web = "4"
+actix-session = { version = "0.7.0", features = ["cookie-session"] }
 clap = { version = "3.2.12", features = ["derive", "env"] }
 include_dir = "0.7.2"
 log = "0.4.17"

src/constants.rs

@@ -14,10 +14,10 @@ pub const DEFAULT_ADMIN_PASSWORD: &str = "admin";
 pub const APP_NAME: &str = "Basic OIDC";
 
 /// Maximum session duration after inactivity, in seconds
-pub const MAX_INACTIVITY_DURATION: i64 = 60 * 30;
+pub const MAX_INACTIVITY_DURATION: u64 = 60 * 30;
 
 /// Maximum session duration (6 hours)
-pub const MAX_SESSION_DURATION: i64 = 3600 * 6;
+pub const MAX_SESSION_DURATION: u64 = 3600 * 6;
 
 /// Minimum password length
 pub const MIN_PASS_LEN: usize = 4;

src/controllers/login_api.rs

@@ -1,5 +1,5 @@
 use actix_identity::Identity;
-use actix_web::{HttpResponse, Responder, web};
+use actix_web::{HttpRequest, HttpResponse, Responder, web};
 use webauthn_rs::proto::PublicKeyCredential;
 
 use crate::data::session_identity::{SessionIdentity, SessionStatus};
@@ -13,16 +13,17 @@ pub struct AuthWebauthnRequest {
 
 pub async fn auth_webauthn(id: Identity,
                            req: web::Json<AuthWebauthnRequest>,
-                           manager: WebAuthManagerReq) -> impl Responder {
+                           manager: WebAuthManagerReq,
-    if !SessionIdentity(&id).need_2fa_auth() {
+                           http_req: HttpRequest) -> impl Responder {
+    if !SessionIdentity(Some(&id)).need_2fa_auth() {
         return HttpResponse::Unauthorized().json("No 2FA required!");
     }
 
-    let user_id = SessionIdentity(&id).user_id();
+    let user_id = SessionIdentity(Some(&id)).user_id();
 
     match manager.finish_authentication(&user_id, &req.opaque_state, &req.credential) {
         Ok(_) => {
-            SessionIdentity(&id).set_status(SessionStatus::SignedIn);
+            SessionIdentity(Some(&id)).set_status(&http_req, SessionStatus::SignedIn);
             HttpResponse::Ok().body("You are authenticated!")
         }
         Err(e) => {

src/controllers/login_controller.rs

@@ -1,6 +1,6 @@
 use actix::Addr;
 use actix_identity::Identity;
-use actix_web::{HttpResponse, Responder, web};
+use actix_web::{HttpRequest, HttpResponse, Responder, web};
 use askama::Template;
 
 use crate::actors::{bruteforce_actor, users_actor};
@@ -80,7 +80,8 @@ pub async fn login_route(
     bruteforce: web::Data<Addr<BruteForceActor>>,
     query: web::Query<LoginRequestQuery>,
     req: Option<web::Form<LoginRequestBody>>,
-    id: Identity,
+    id: Option<Identity>,
+    http_req: HttpRequest,
 ) -> impl Responder {
     let mut danger = None;
     let mut success = None;
@@ -97,27 +98,29 @@ pub async fn login_route(
 
     // Check if user session must be closed
     if let Some(true) = query.logout {
-        id.forget();
+        if let Some(id) = id {
+            id.logout();
+        }
         success = Some("Goodbye!".to_string());
     }
 
     // Check if user is already authenticated
-    if SessionIdentity(&id).is_authenticated() {
+    else if SessionIdentity(id.as_ref()).is_authenticated() {
         return redirect_user(query.redirect.get());
     }
 
     // Check if the password of the user has to be changed
-    if SessionIdentity(&id).need_new_password() {
+    else if SessionIdentity(id.as_ref()).need_new_password() {
         return redirect_user(&format!("/reset_password?redirect={}", query.redirect.get_encoded()));
     }
 
     // Check if the user has to valide a second factor
-    if SessionIdentity(&id).need_2fa_auth() {
+    else if SessionIdentity(id.as_ref()).need_2fa_auth() {
         return redirect_user(&format!("/2fa_auth?redirect={}", query.redirect.get_encoded()));
     }
 
     // Try to authenticate user
-    if let Some(req) = &req {
+    else if let Some(req) = &req {
         login = req.login.clone();
         let response: LoginResult = users
             .send(users_actor::LoginRequest {
@@ -129,17 +132,16 @@ pub async fn login_route(
 
         match response {
             LoginResult::Success(user) => {
-                SessionIdentity(&id).set_user(&user);
+                let status = if user.need_reset_password {
-
+                    SessionStatus::NeedNewPassword
-                return if user.need_reset_password {
-                    SessionIdentity(&id).set_status(SessionStatus::NeedNewPassword);
-                    redirect_user(&format!("/reset_password?redirect={}", query.redirect.get_encoded()))
                 } else if user.has_two_factor() {
-                    SessionIdentity(&id).set_status(SessionStatus::Need2FA);
+                    SessionStatus::Need2FA
-                    redirect_user(&format!("/2fa_auth?redirect={}", query.redirect.get_encoded()))
                 } else {
-                    redirect_user(query.redirect.get())
+                    SessionStatus::SignedIn
                 };
+
+                SessionIdentity(id.as_ref()).set_user(&http_req, &user, status);
+                return redirect_user(query.redirect.get());
             }
 
             LoginResult::AccountDisabled => {
@@ -189,12 +191,13 @@ pub struct PasswordResetQuery {
 }
 
 /// Reset user password route
-pub async fn reset_password_route(id: Identity, query: web::Query<PasswordResetQuery>,
+pub async fn reset_password_route(id: Option<Identity>, query: web::Query<PasswordResetQuery>,
                                   req: Option<web::Form<ChangePasswordRequestBody>>,
-                                  users: web::Data<Addr<UsersActor>>) -> impl Responder {
+                                  users: web::Data<Addr<UsersActor>>,
+                                  http_req: HttpRequest) -> impl Responder {
     let mut danger = None;
 
-    if !SessionIdentity(&id).need_new_password() {
+    if !SessionIdentity(id.as_ref()).need_new_password() {
         return redirect_user_for_login(query.redirect.get());
     }
 
@@ -205,7 +208,7 @@ pub async fn reset_password_route(id: Identity, query: web::Query<PasswordResetQ
         } else {
             let res: ChangePasswordResult = users
                 .send(users_actor::ChangePasswordRequest {
-                    user_id: SessionIdentity(&id).user_id(),
+                    user_id: SessionIdentity(id.as_ref()).user_id(),
                     new_password: req.password.clone(),
                     temporary: false,
                 })
@@ -215,7 +218,7 @@ pub async fn reset_password_route(id: Identity, query: web::Query<PasswordResetQ
             if !res.0 {
                 danger = Some("Failed to change password!".to_string());
             } else {
-                SessionIdentity(&id).set_status(SessionStatus::SignedIn);
+                SessionIdentity(id.as_ref()).set_status(&http_req, SessionStatus::SignedIn);
                 return redirect_user(query.redirect.get());
             }
         }
@@ -246,17 +249,19 @@ pub struct ChooseSecondFactorQuery {
 }
 
 /// Let the user select the factor to use to authenticate
-pub async fn choose_2fa_method(id: Identity, query: web::Query<ChooseSecondFactorQuery>,
+pub async fn choose_2fa_method(id: Option<Identity>, query: web::Query<ChooseSecondFactorQuery>,
                                users: web::Data<Addr<UsersActor>>) -> impl Responder {
-    if !SessionIdentity(&id).need_2fa_auth() {
+    if !SessionIdentity(id.as_ref()).need_2fa_auth() {
+        log::trace!("User does not require 2fa auth, redirecting");
         return redirect_user_for_login(query.redirect.get());
     }
 
-    let user: User = users.send(users_actor::GetUserRequest(SessionIdentity(&id).user_id()))
+    let user: User = users.send(users_actor::GetUserRequest(SessionIdentity(id.as_ref()).user_id()))
         .await.unwrap().0.expect("Could not find user!");
 
     // Automatically choose factor if there is only one factor
     if user.two_factor.len() == 1 && !query.force_display {
+        log::trace!("User has only one factor, using it by default");
         return redirect_user(&user.two_factor[0].login_url(&query.redirect));
     }
 
@@ -290,16 +295,17 @@ pub struct LoginWithOTPForm {
 
 
 /// Login with OTP
-pub async fn login_with_otp(id: Identity, query: web::Query<LoginWithOTPQuery>,
+pub async fn login_with_otp(id: Option<Identity>, query: web::Query<LoginWithOTPQuery>,
                             form: Option<web::Form<LoginWithOTPForm>>,
-                            users: web::Data<Addr<UsersActor>>) -> impl Responder {
+                            users: web::Data<Addr<UsersActor>>,
+                            http_req: HttpRequest) -> impl Responder {
     let mut danger = None;
 
-    if !SessionIdentity(&id).need_2fa_auth() {
+    if !SessionIdentity(id.as_ref()).need_2fa_auth() {
         return redirect_user_for_login(query.redirect.get());
     }
 
-    let user: User = users.send(users_actor::GetUserRequest(SessionIdentity(&id).user_id()))
+    let user: User = users.send(users_actor::GetUserRequest(SessionIdentity(id.as_ref()).user_id()))
         .await.unwrap().0.expect("Could not find user!");
 
     let factor = match user.find_factor(&query.id) {
@@ -318,7 +324,7 @@ pub async fn login_with_otp(id: Identity, query: web::Query<LoginWithOTPQuery>,
         if !key.check_code(&form.code).unwrap_or(false) {
             danger = Some("Specified code is invalid!".to_string());
         } else {
-            SessionIdentity(&id).set_status(SessionStatus::SignedIn);
+            SessionIdentity(id.as_ref()).set_status(&http_req, SessionStatus::SignedIn);
             return redirect_user(query.redirect.get());
         }
     }
@@ -344,14 +350,14 @@ pub struct LoginWithWebauthnQuery {
 
 
 /// Login with Webauthn
-pub async fn login_with_webauthn(id: Identity, query: web::Query<LoginWithWebauthnQuery>,
+pub async fn login_with_webauthn(id: Option<Identity>, query: web::Query<LoginWithWebauthnQuery>,
                                  manager: WebAuthManagerReq,
                                  users: web::Data<Addr<UsersActor>>) -> impl Responder {
-    if !SessionIdentity(&id).need_2fa_auth() {
+    if !SessionIdentity(id.as_ref()).need_2fa_auth() {
         return redirect_user_for_login(query.redirect.get());
     }
 
-    let user: User = users.send(users_actor::GetUserRequest(SessionIdentity(&id).user_id()))
+    let user: User = users.send(users_actor::GetUserRequest(SessionIdentity(id.as_ref()).user_id()))
         .await.unwrap().0.expect("Could not find user!");
 
     let factor = match user.find_factor(&query.id) {

src/controllers/openid_controller.rs

@@ -146,7 +146,7 @@ pub async fn authorize(user: CurrentUser, id: Identity, query: web::Query<Author
         session_id: SessionID(rand_str(OPEN_ID_SESSION_LEN)),
         client: client.id,
         user: user.uid.clone(),
-        auth_time: SessionIdentity(&id).auth_time(),
+        auth_time: SessionIdentity(Some(&id)).auth_time(),
         redirect_uri,
         authorization_code: rand_str(OPEN_ID_AUTHORIZATION_CODE_LEN),
         authorization_code_expire_at: time() + OPEN_ID_AUTHORIZATION_CODE_TIMEOUT,

src/data/current_user.rs

@@ -38,7 +38,7 @@ impl FromRequest for CurrentUser {
         let user_actor: Addr<UsersActor> = user_actor.as_ref().clone();
         let identity: Identity = Identity::from_request(req, payload).into_inner()
             .expect("Failed to get identity!");
-        let user_id = SessionIdentity(&identity).user_id();
+        let user_id = SessionIdentity(Some(&identity)).user_id();
 
 
         Box::pin(async move {

src/data/session_identity.rs

@@ -1,4 +1,5 @@
 use actix_identity::Identity;
+use actix_web::{HttpMessage, HttpRequest};
 use serde::{Deserialize, Serialize};
 
 use crate::data::user::{User, UserID};
@@ -26,11 +27,16 @@ pub struct SessionIdentityData {
     pub status: SessionStatus,
 }
 
-pub struct SessionIdentity<'a>(pub &'a Identity);
+pub struct SessionIdentity<'a>(pub Option<&'a Identity>);
 
 impl<'a> SessionIdentity<'a> {
     fn get_session_data(&self) -> Option<SessionIdentityData> {
-        Self::deserialize_session_data(self.0.identity())
+        if let Some(id) = self.0 {
+            Self::deserialize_session_data(id.id().ok())
+        }
+        else {
+            None
+        }
     }
 
     pub fn deserialize_session_data(data: Option<String>) -> Option<SessionIdentityData> {
@@ -54,25 +60,29 @@ impl<'a> SessionIdentity<'a> {
         res
     }
 
-    fn set_session_data(&self, data: &SessionIdentityData) {
+    fn set_session_data(&self, req: &HttpRequest, data: &SessionIdentityData) {
         let s = serde_json::to_string(data).expect("Failed to serialize session data!");
 
-        self.0.remember(s);
+        log::debug!("Will set user session data.");
+        if let Err(e) = Identity::login(&req.extensions(), s) {
+            log::error!("Failed to set session data! {}", e);
+        }
+        log::debug!("Did set user session data.");
     }
 
-    pub fn set_user(&self, user: &User) {
+    pub fn set_user(&self, req: &HttpRequest, user: &User, status: SessionStatus) {
-        self.set_session_data(&SessionIdentityData {
+        self.set_session_data(req, &SessionIdentityData {
             id: Some(user.uid.clone()),
             is_admin: user.admin,
             auth_time: time(),
-            status: SessionStatus::SignedIn,
+            status,
         });
     }
 
-    pub fn set_status(&self, status: SessionStatus) {
+    pub fn set_status(&self, req: &HttpRequest, status: SessionStatus) {
         let mut sess = self.get_session_data().unwrap_or_default();
         sess.status = status;
-        self.set_session_data(&sess);
+        self.set_session_data(req, &sess);
     }
 
     pub fn is_authenticated(&self) -> bool {

src/main.rs

@@ -1,10 +1,13 @@
+use core::time::Duration;
 use std::sync::Arc;
 
 use actix::Actor;
-use actix_identity::{CookieIdentityPolicy, IdentityService};
+use actix_identity::config::LogoutBehaviour;
+use actix_identity::IdentityMiddleware;
+use actix_session::SessionMiddleware;
+use actix_session::storage::CookieSessionStore;
 use actix_web::{App, get, HttpResponse, HttpServer, middleware, web};
-use actix_web::cookie::SameSite;
+use actix_web::cookie::{Key, SameSite};
-use actix_web::cookie::time::Duration;
 use actix_web::middleware::Logger;
 use clap::Parser;
 
@@ -35,7 +38,7 @@ async fn main() -> std::io::Result<()> {
 
     // In debug mode only, use dummy token
     if cfg!(debug_assertions) && config.token_key.is_empty() {
-        config.token_key = String::from_utf8_lossy(&[32; 32]).to_string();
+        config.token_key = String::from_utf8_lossy(&[32; 64]).to_string();
     }
 
     if !config.storage_path().exists() {
@@ -81,12 +84,19 @@ async fn main() -> std::io::Result<()> {
             .expect("Failed to load clients list!");
         clients.apply_environment_variables();
 
-        let policy = CookieIdentityPolicy::new(config.token_key.as_bytes())
+        let session_mw =
-            .name(SESSION_COOKIE_NAME)
+            SessionMiddleware::builder(CookieSessionStore::default(),
-            .secure(config.secure_cookie())
+                                       Key::from(config.token_key.as_bytes()))
-            .visit_deadline(Duration::seconds(MAX_INACTIVITY_DURATION))
+                .cookie_name(SESSION_COOKIE_NAME.to_string())
-            .login_deadline(Duration::seconds(MAX_SESSION_DURATION))
+                .cookie_secure(config.secure_cookie())
-            .same_site(SameSite::Lax);
+                .cookie_same_site(SameSite::Lax)
+                .build();
+
+        let identity_middleware = IdentityMiddleware::builder()
+            .logout_behaviour(LogoutBehaviour::PurgeSession)
+            .visit_deadline(Some(Duration::from_secs(MAX_INACTIVITY_DURATION)))
+            .login_deadline(Some(Duration::from_secs(MAX_SESSION_DURATION)))
+            .build();
 
         App::new()
             .app_data(web::Data::new(users_actor.clone()))
@@ -101,7 +111,8 @@ async fn main() -> std::io::Result<()> {
                 .add(("Permissions-Policy", "interest-cohort=()")))
             .wrap(Logger::default())
             .wrap(AuthMiddleware {})
-            .wrap(IdentityService::new(policy))
+            .wrap(identity_middleware)
+            .wrap(session_mw)
 
             // main route
             .route("/", web::get()

src/middlewares/auth_middleware.rs

@@ -4,7 +4,7 @@ use std::future::{Future, ready, Ready};
 use std::pin::Pin;
 use std::rc::Rc;
 
-use actix_identity::RequestIdentity;
+use actix_identity::IdentityExt;
 use actix_web::{
     dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform},
     Error, HttpResponse, web,
@@ -114,7 +114,9 @@ impl<S, B> Service<ServiceRequest> for AuthInnerMiddleware<S>
                 ));
             }
 
-            let session = match SessionIdentity::deserialize_session_data(req.get_identity()) {
+            let id = req.get_identity().ok().map(|r| r.id().unwrap_or_default());
+            let session_data = SessionIdentity::deserialize_session_data(id);
+            let session = match session_data {
                 Some(SessionIdentityData {
                          status: SessionStatus::SignedIn,
                          is_admin: true,
@@ -127,11 +129,17 @@ impl<S, B> Service<ServiceRequest> for AuthInnerMiddleware<S>
                 _ => ConnStatus::SignedOut,
             };
 
+            log::trace!("Connection data: {:#?}", session_data);
+            log::debug!("Connection status: {:?}", session);
+
             // Redirect user to login page
             if !session.is_auth()
                 && (req.path().starts_with(ADMIN_ROUTES)
                 || req.path().starts_with(AUTHENTICATED_ROUTES) || req.path().eq(AUTHORIZE_URI))
             {
+                log::debug!("Redirect unauthenticated user from {} to authorization route.",
+                    req.path());
+
                 let path = req.uri().to_string();
                 return Ok(req
                     .into_response(redirect_user_for_login(path))