diff --git a/actix-cors/src/all_or_some.rs b/actix-cors/src/all_or_some.rs index d8cec53a1..999f7e9b4 100644 --- a/actix-cors/src/all_or_some.rs +++ b/actix-cors/src/all_or_some.rs @@ -1,5 +1,5 @@ /// An enum signifying that some of type `T` is allowed, or `All` (anything is allowed). -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum AllOrSome { /// Everything is allowed. Usually equivalent to the `*` value. All, diff --git a/actix-limitation/src/lib.rs b/actix-limitation/src/lib.rs index a3cb9363a..43c02c1d9 100644 --- a/actix-limitation/src/lib.rs +++ b/actix-limitation/src/lib.rs @@ -73,7 +73,7 @@ pub const DEFAULT_COOKIE_NAME: &str = "sid"; pub const DEFAULT_SESSION_KEY: &str = "rate-api-id"; /// Rate limiter. -#[derive(Clone, Debug)] +#[derive(Debug, Clone)] pub struct Limiter { client: Client, limit: usize, diff --git a/actix-session/CHANGES.md b/actix-session/CHANGES.md index 93cbecc73..d76f86a7c 100644 --- a/actix-session/CHANGES.md +++ b/actix-session/CHANGES.md @@ -1,8 +1,19 @@ # Changes ## Unreleased - 2022-xx-xx +- Added `TtlExtensionPolicy` enum to support different strategies for extending the TTL attached to the session state. `TtlExtensionPolicy::OnEveryRequest` now allows for long-lived sessions that do not expire if the user remains active. [#233] +- `SessionLength` is now called `SessionLifecycle`. [#233] +- `SessionLength::Predetermined` is now called `SessionLifecycle::PersistentSession`. [#233] +- The fields for Both `SessionLength` variants have been extracted into separate types (`PersistentSession` and `BrowserSession`). All fields are now private, manipulated via methods, to allow adding more configuration parameters in the future in a non-breaking fashion. [#233] +- `SessionLength::Predetermined::max_session_length` is now called `PersistentSession::session_ttl`. [#233] +- `SessionLength::BrowserSession::state_ttl` is now called `BrowserSession::session_state_ttl`. [#233] +- `SessionMiddlewareBuilder::max_session_length` is now called `SessionMiddlewareBuilder::session_lifecycle`. [#233] +- The `SessionStore` trait requires the implementation of a new method, `SessionStore::update_ttl`. [#233] +- All types used to configure `SessionMiddleware` have been moved to the `config` sub-module [#233] - Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. +[#233]: https://github.com/actix/actix-extras/pull/233 + ## 0.6.2 - 2022-03-25 - Implement `SessionExt` for `GuardContext`. [#234] diff --git a/actix-session/Cargo.toml b/actix-session/Cargo.toml index dd9c3e0d2..d5f1f1da3 100644 --- a/actix-session/Cargo.toml +++ b/actix-session/Cargo.toml @@ -38,7 +38,6 @@ derive_more = "0.99.5" rand = { version = "0.8", optional = true } serde = { version = "1" } serde_json = { version = "1" } -time = "0.3" tracing = { version = "0.1.30", default-features = false, features = ["log"] } # redis-actor-session diff --git a/actix-session/src/config.rs b/actix-session/src/config.rs new file mode 100644 index 000000000..6cd96764d --- /dev/null +++ b/actix-session/src/config.rs @@ -0,0 +1,369 @@ +//! Configuration options to tune the behaviour of [`SessionMiddleware`]. + +use actix_web::cookie::{time::Duration, Key, SameSite}; + +use crate::{storage::SessionStore, SessionMiddleware}; + +/// Determines what type of session cookie should be used and how its lifecycle should be managed. +/// +/// Used by [`SessionMiddlewareBuilder::session_lifecycle`]. +#[derive(Debug, Clone)] +#[non_exhaustive] +pub enum SessionLifecycle { + /// The session cookie will expire when the current browser session ends. + /// + /// When does a browser session end? It depends on the browser! Chrome, for example, will often + /// continue running in the background when the browser is closed—session cookies are not + /// deleted and they will still be available when the browser is opened again. + /// Check the documentation of the browsers you are targeting for up-to-date information. + BrowserSession(BrowserSession), + + /// The session cookie will be a [persistent cookie]. + /// + /// Persistent cookies have a pre-determined lifetime, specified via the `Max-Age` or `Expires` + /// attribute. They do not disappear when the current browser session ends. + /// + /// [persistent cookie]: https://www.whitehatsec.com/glossary/content/persistent-session-cookie + PersistentSession(PersistentSession), +} + +impl From for SessionLifecycle { + fn from(session: BrowserSession) -> Self { + Self::BrowserSession(session) + } +} + +impl From for SessionLifecycle { + fn from(session: PersistentSession) -> Self { + Self::PersistentSession(session) + } +} + +/// A [session lifecycle](SessionLifecycle) strategy where the session cookie expires when the +/// browser's current session ends. +/// +/// When does a browser session end? It depends on the browser. Chrome, for example, will often +/// continue running in the background when the browser is closed—session cookies are not deleted +/// and they will still be available when the browser is opened again. Check the documentation of +/// the browsers you are targeting for up-to-date information. +#[derive(Debug, Clone)] +pub struct BrowserSession { + state_ttl: Duration, + state_ttl_extension_policy: TtlExtensionPolicy, +} + +impl BrowserSession { + /// Sets a time-to-live (TTL) when storing the session state in the storage backend. + /// + /// We do not want to store session states indefinitely, otherwise we will inevitably run out of + /// storage by holding on to the state of countless abandoned or expired sessions! + /// + /// We are dealing with the lifecycle of two uncorrelated object here: the session cookie + /// and the session state. It is not a big issue if the session state outlives the cookie— + /// we are wasting some space in the backend storage, but it will be cleaned up eventually. + /// What happens, instead, if the cookie outlives the session state? A new session starts— + /// e.g. if sessions are being used for authentication, the user is de-facto logged out. + /// + /// It is not possible to predict with certainty how long a browser session is going to + /// last—you need to provide a reasonable upper bound. You do so via `state_ttl`—it dictates + /// what TTL should be used for session state when the lifecycle of the session cookie is + /// tied to the browser session length. [`SessionMiddleware`] will default to 1 day if + /// `state_ttl` is left unspecified. + /// + /// You can mitigate the risk of the session cookie outliving the session state by + /// specifying a more aggressive state TTL extension policy - check out + /// [`BrowserSession::state_ttl_extension_policy`] for more details. + pub fn state_ttl(mut self, ttl: Duration) -> Self { + self.state_ttl = ttl; + self + } + + /// Determine under what circumstances the TTL of your session state should be extended. + /// + /// Defaults to [`TtlExtensionPolicy::OnStateChanges`] if left unspecified. + /// + /// See [`TtlExtensionPolicy`] for more details. + pub fn state_ttl_extension_policy(mut self, ttl_extension_policy: TtlExtensionPolicy) -> Self { + self.state_ttl_extension_policy = ttl_extension_policy; + self + } +} + +impl Default for BrowserSession { + fn default() -> Self { + Self { + state_ttl: default_ttl(), + state_ttl_extension_policy: default_ttl_extension_policy(), + } + } +} + +/// A [session lifecycle](SessionLifecycle) strategy where the session cookie will be [persistent]. +/// +/// Persistent cookies have a pre-determined expiration, specified via the `Max-Age` or `Expires` +/// attribute. They do not disappear when the current browser session ends. +/// +/// [persistent]: https://www.whitehatsec.com/glossary/content/persistent-session-cookie +#[derive(Debug, Clone)] +pub struct PersistentSession { + session_ttl: Duration, + ttl_extension_policy: TtlExtensionPolicy, +} + +impl PersistentSession { + /// Specifies how long the session cookie should live. + /// + /// Defaults to 1 day if left unspecified. + /// + /// The session TTL is also used as the TTL for the session state in the storage backend. + /// + /// A persistent session can live more than the specified TTL if the TTL is extended. + /// See [`session_ttl_extension_policy`](Self::session_ttl_extension_policy) for more details. + pub fn session_ttl(mut self, session_ttl: Duration) -> Self { + self.session_ttl = session_ttl; + self + } + + /// Determines under what circumstances the TTL of your session should be extended. + /// See [`TtlExtensionPolicy`] for more details. + /// + /// Defaults to [`TtlExtensionPolicy::OnStateChanges`] if left unspecified. + pub fn session_ttl_extension_policy( + mut self, + ttl_extension_policy: TtlExtensionPolicy, + ) -> Self { + self.ttl_extension_policy = ttl_extension_policy; + self + } +} + +impl Default for PersistentSession { + fn default() -> Self { + Self { + session_ttl: default_ttl(), + ttl_extension_policy: default_ttl_extension_policy(), + } + } +} + +/// Configuration for which events should trigger an extension of the time-to-live for your session. +/// +/// If you are using a [`BrowserSession`], `TtlExtensionPolicy` controls how often the TTL of +/// the session state should be refreshed. The browser is in control of the lifecycle of the +/// session cookie. +/// +/// If you are using a [`PersistentSession`], `TtlExtensionPolicy` controls both the expiration +/// of the session cookie and the TTL of the session state. +#[derive(Debug, Clone)] +#[non_exhaustive] +pub enum TtlExtensionPolicy { + /// The TTL is refreshed every time the server receives a request associated with a session. + /// + /// # Performance impact + /// Refreshing the TTL on every request is not free. + /// It implies a refresh of the TTL on the session state. This translates into a request over + /// the network if you are using a remote system as storage backend (e.g. Redis). + /// This impacts both the total load on your storage backend (i.e. number of + /// queries it has to handle) and the latency of the requests served by your server. + OnEveryRequest, + + /// The TTL is refreshed every time the session state changes or the session key is renewed. + OnStateChanges, +} + +/// Determines how to secure the content of the session cookie. +/// +/// Used by [`SessionMiddlewareBuilder::cookie_content_security`]. +#[derive(Debug, Clone, Copy)] +pub enum CookieContentSecurity { + /// The cookie content is encrypted when using `CookieContentSecurity::Private`. + /// + /// Encryption guarantees confidentiality and integrity: the client cannot tamper with the + /// cookie content nor decode it, as long as the encryption key remains confidential. + Private, + + /// The cookie content is signed when using `CookieContentSecurity::Signed`. + /// + /// Signing guarantees integrity, but it doesn't ensure confidentiality: the client cannot + /// tamper with the cookie content, but they can read it. + Signed, +} + +pub(crate) const fn default_ttl() -> Duration { + Duration::days(1) +} + +pub(crate) const fn default_ttl_extension_policy() -> TtlExtensionPolicy { + TtlExtensionPolicy::OnStateChanges +} + +/// A fluent builder to construct a [`SessionMiddleware`] instance with custom configuration +/// parameters. +#[must_use] +pub struct SessionMiddlewareBuilder { + storage_backend: Store, + configuration: Configuration, +} + +impl SessionMiddlewareBuilder { + pub(crate) fn new(store: Store, configuration: Configuration) -> Self { + Self { + storage_backend: store, + configuration, + } + } + + /// Set the name of the cookie used to store the session ID. + /// + /// Defaults to `id`. + pub fn cookie_name(mut self, name: String) -> Self { + self.configuration.cookie.name = name; + self + } + + /// Set the `Secure` attribute for the cookie used to store the session ID. + /// + /// If the cookie is set as secure, it will only be transmitted when the connection is secure + /// (using `https`). + /// + /// Default is `true`. + pub fn cookie_secure(mut self, secure: bool) -> Self { + self.configuration.cookie.secure = secure; + self + } + + /// Determines what type of session cookie should be used and how its lifecycle should be managed. + /// Check out [`SessionLifecycle`]'s documentation for more details on the available options. + /// + /// Default is [`SessionLifecycle::BrowserSession`]. + pub fn session_lifecycle>(mut self, session_lifecycle: S) -> Self { + match session_lifecycle.into() { + SessionLifecycle::BrowserSession(BrowserSession { + state_ttl, + state_ttl_extension_policy, + }) => { + self.configuration.cookie.max_age = None; + self.configuration.session.state_ttl = state_ttl; + self.configuration.ttl_extension_policy = state_ttl_extension_policy; + } + SessionLifecycle::PersistentSession(PersistentSession { + session_ttl, + ttl_extension_policy, + }) => { + self.configuration.cookie.max_age = Some(session_ttl); + self.configuration.session.state_ttl = session_ttl; + self.configuration.ttl_extension_policy = ttl_extension_policy; + } + } + + self + } + + /// Set the `SameSite` attribute for the cookie used to store the session ID. + /// + /// By default, the attribute is set to `Lax`. + pub fn cookie_same_site(mut self, same_site: SameSite) -> Self { + self.configuration.cookie.same_site = same_site; + self + } + + /// Set the `Path` attribute for the cookie used to store the session ID. + /// + /// By default, the attribute is set to `/`. + pub fn cookie_path(mut self, path: String) -> Self { + self.configuration.cookie.path = path; + self + } + + /// Set the `Domain` attribute for the cookie used to store the session ID. + /// + /// Use `None` to leave the attribute unspecified. If unspecified, the attribute defaults + /// to the same host that set the cookie, excluding subdomains. + /// + /// By default, the attribute is left unspecified. + pub fn cookie_domain(mut self, domain: Option) -> Self { + self.configuration.cookie.domain = domain; + self + } + + /// Choose how the session cookie content should be secured. + /// + /// - [`CookieContentSecurity::Private`] selects encrypted cookie content. + /// - [`CookieContentSecurity::Signed`] selects signed cookie content. + /// + /// # Default + /// By default, the cookie content is encrypted. Encrypted was chosen instead of signed as + /// default because it reduces the chances of sensitive information being exposed in the session + /// key by accident, regardless of [`SessionStore`] implementation you chose to use. + /// + /// For example, if you are using cookie-based storage, you definitely want the cookie content + /// to be encrypted—the whole session state is embedded in the cookie! If you are using + /// Redis-based storage, signed is more than enough - the cookie content is just a unique + /// tamper-proof session key. + pub fn cookie_content_security(mut self, content_security: CookieContentSecurity) -> Self { + self.configuration.cookie.content_security = content_security; + self + } + + /// Set the `HttpOnly` attribute for the cookie used to store the session ID. + /// + /// If the cookie is set as `HttpOnly`, it will not be visible to any JavaScript snippets + /// running in the browser. + /// + /// Default is `true`. + pub fn cookie_http_only(mut self, http_only: bool) -> Self { + self.configuration.cookie.http_only = http_only; + self + } + + /// Finalise the builder and return a [`SessionMiddleware`] instance. + #[must_use] + pub fn build(self) -> SessionMiddleware { + SessionMiddleware::from_parts(self.storage_backend, self.configuration) + } +} + +#[derive(Clone)] +pub(crate) struct Configuration { + pub(crate) cookie: CookieConfiguration, + pub(crate) session: SessionConfiguration, + pub(crate) ttl_extension_policy: TtlExtensionPolicy, +} + +#[derive(Clone)] +pub(crate) struct SessionConfiguration { + pub(crate) state_ttl: Duration, +} + +#[derive(Clone)] +pub(crate) struct CookieConfiguration { + pub(crate) secure: bool, + pub(crate) http_only: bool, + pub(crate) name: String, + pub(crate) same_site: SameSite, + pub(crate) path: String, + pub(crate) domain: Option, + pub(crate) max_age: Option, + pub(crate) content_security: CookieContentSecurity, + pub(crate) key: Key, +} + +pub(crate) fn default_configuration(key: Key) -> Configuration { + Configuration { + cookie: CookieConfiguration { + secure: true, + http_only: true, + name: "id".into(), + same_site: SameSite::Lax, + path: "/".into(), + domain: None, + max_age: None, + content_security: CookieContentSecurity::Private, + key, + }, + session: SessionConfiguration { + state_ttl: default_ttl(), + }, + ttl_extension_policy: default_ttl_extension_policy(), + } +} diff --git a/actix-session/src/lib.rs b/actix-session/src/lib.rs index 67d5c92a5..fcc51744b 100644 --- a/actix-session/src/lib.rs +++ b/actix-session/src/lib.rs @@ -139,32 +139,25 @@ #![doc(html_favicon_url = "https://actix.rs/favicon.ico")] #![cfg_attr(docsrs, feature(doc_cfg))] +pub mod config; mod middleware; mod session; mod session_ext; pub mod storage; -pub use self::middleware::{ - CookieContentSecurity, SessionLength, SessionMiddleware, SessionMiddlewareBuilder, -}; +pub use self::middleware::SessionMiddleware; pub use self::session::{Session, SessionStatus}; pub use self::session_ext::SessionExt; #[cfg(test)] pub mod test_helpers { use actix_web::cookie::Key; - use rand::{distributions::Alphanumeric, thread_rng, Rng}; - use crate::{storage::SessionStore, CookieContentSecurity}; + use crate::{config::CookieContentSecurity, storage::SessionStore}; /// Generate a random cookie signing/encryption key. pub fn key() -> Key { - let signing_key: String = thread_rng() - .sample_iter(&Alphanumeric) - .take(64) - .map(char::from) - .collect(); - Key::from(signing_key.as_bytes()) + Key::generate() } /// A ready-to-go acceptance test suite to verify that sessions behave as expected @@ -187,6 +180,11 @@ pub mod test_helpers { acceptance_tests::basic_workflow(store_builder.clone(), *policy).await; acceptance_tests::expiration_is_refreshed_on_changes(store_builder.clone(), *policy) .await; + acceptance_tests::expiration_is_always_refreshed_if_configured_to_refresh_on_every_request( + store_builder.clone(), + *policy, + ) + .await; acceptance_tests::complex_workflow( store_builder.clone(), is_invalidation_supported, @@ -199,18 +197,18 @@ pub mod test_helpers { mod acceptance_tests { use actix_web::{ - dev::Service, + cookie::time, + dev::{Service, ServiceResponse}, guard, middleware, test, web::{self, get, post, resource, Bytes}, App, HttpResponse, Result, }; use serde::{Deserialize, Serialize}; use serde_json::json; - use time::Duration; + use crate::config::{CookieContentSecurity, PersistentSession, TtlExtensionPolicy}; use crate::{ - middleware::SessionLength, storage::SessionStore, test_helpers::key, - CookieContentSecurity, Session, SessionExt, SessionMiddleware, + storage::SessionStore, test_helpers::key, Session, SessionExt, SessionMiddleware, }; pub(super) async fn basic_workflow( @@ -228,9 +226,10 @@ pub mod test_helpers { .cookie_name("actix-test".into()) .cookie_domain(Some("localhost".into())) .cookie_content_security(policy) - .session_length(SessionLength::Predetermined { - max_session_length: Some(time::Duration::seconds(100)), - }) + .session_lifecycle( + PersistentSession::default() + .session_ttl(time::Duration::seconds(100)), + ) .build(), ) .service(web::resource("/").to(|ses: Session| async move { @@ -246,12 +245,7 @@ pub mod test_helpers { let request = test::TestRequest::get().to_request(); let response = app.call(request).await.unwrap(); - let cookie = response - .response() - .cookies() - .find(|c| c.name() == "actix-test") - .unwrap() - .clone(); + let cookie = response.get_cookie("actix-test").unwrap().clone(); assert_eq!(cookie.path().unwrap(), "/test/"); let request = test::TestRequest::with_uri("/test/") @@ -261,6 +255,55 @@ pub mod test_helpers { assert_eq!(body, Bytes::from_static(b"counter: 100")); } + pub(super) async fn expiration_is_always_refreshed_if_configured_to_refresh_on_every_request< + F, + Store, + >( + store_builder: F, + policy: CookieContentSecurity, + ) where + Store: SessionStore + 'static, + F: Fn() -> Store + Clone + Send + 'static, + { + let session_ttl = time::Duration::seconds(60); + let app = test::init_service( + App::new() + .wrap( + SessionMiddleware::builder(store_builder(), key()) + .cookie_content_security(policy) + .session_lifecycle( + PersistentSession::default() + .session_ttl(session_ttl) + .session_ttl_extension_policy( + TtlExtensionPolicy::OnEveryRequest, + ), + ) + .build(), + ) + .service(web::resource("/").to(|ses: Session| async move { + let _ = ses.insert("counter", 100); + "test" + })) + .service(web::resource("/test/").to(|| async move { "no-changes-in-session" })), + ) + .await; + + // Create session + let request = test::TestRequest::get().to_request(); + let response = app.call(request).await.unwrap(); + let cookie_1 = response.get_cookie("id").expect("Cookie is set"); + assert_eq!(cookie_1.max_age(), Some(session_ttl)); + + // Fire a request that doesn't touch the session state, check + // that the session cookie is present and its expiry is set to the maximum we configured. + let request = test::TestRequest::with_uri("/test/") + .cookie(cookie_1) + .to_request(); + let response = app.call(request).await.unwrap(); + let cookie_2 = response.get_cookie("id").expect("Cookie is set"); + assert_eq!(cookie_2.max_age(), Some(session_ttl)); + } + pub(super) async fn expiration_is_refreshed_on_changes( store_builder: F, policy: CookieContentSecurity, @@ -268,14 +311,15 @@ pub mod test_helpers { Store: SessionStore + 'static, F: Fn() -> Store + Clone + Send + 'static, { + let session_ttl = time::Duration::seconds(60); let app = test::init_service( App::new() .wrap( SessionMiddleware::builder(store_builder(), key()) .cookie_content_security(policy) - .session_length(SessionLength::Predetermined { - max_session_length: Some(time::Duration::seconds(60)), - }) + .session_lifecycle( + PersistentSession::default().session_ttl(session_ttl), + ) .build(), ) .service(web::resource("/").to(|ses: Session| async move { @@ -288,25 +332,19 @@ pub mod test_helpers { let request = test::TestRequest::get().to_request(); let response = app.call(request).await.unwrap(); - let cookie_1 = response - .response() - .cookies() - .find(|c| c.name() == "id") - .expect("Cookie is set"); - assert_eq!(cookie_1.max_age(), Some(Duration::seconds(60))); + let cookie_1 = response.get_cookie("id").expect("Cookie is set"); + assert_eq!(cookie_1.max_age(), Some(session_ttl)); - let request = test::TestRequest::with_uri("/test/").to_request(); + let request = test::TestRequest::with_uri("/test/") + .cookie(cookie_1.clone()) + .to_request(); let response = app.call(request).await.unwrap(); assert!(response.response().cookies().next().is_none()); - let request = test::TestRequest::get().to_request(); + let request = test::TestRequest::get().cookie(cookie_1).to_request(); let response = app.call(request).await.unwrap(); - let cookie_2 = response - .response() - .cookies() - .find(|c| c.name() == "id") - .expect("Cookie is set"); - assert_eq!(cookie_2.max_age(), Some(Duration::seconds(60))); + let cookie_2 = response.get_cookie("id").expect("Cookie is set"); + assert_eq!(cookie_2.max_age(), Some(session_ttl)); } pub(super) async fn guard(store_builder: F, policy: CookieContentSecurity) @@ -320,9 +358,9 @@ pub mod test_helpers { SessionMiddleware::builder(store_builder(), key()) .cookie_name("test-session".into()) .cookie_content_security(policy) - .session_length(SessionLength::Predetermined { - max_session_length: Some(time::Duration::days(7)), - }) + .session_lifecycle( + PersistentSession::default().session_ttl(time::Duration::days(7)), + ) .build(), ) .wrap(middleware::Logger::default()) @@ -402,15 +440,16 @@ pub mod test_helpers { Store: SessionStore + 'static, F: Fn() -> Store + Clone + Send + 'static, { + let session_ttl = time::Duration::days(7); let srv = actix_test::start(move || { App::new() .wrap( SessionMiddleware::builder(store_builder(), key()) .cookie_name("test-session".into()) .cookie_content_security(policy) - .session_length(SessionLength::Predetermined { - max_session_length: Some(time::Duration::days(7)), - }) + .session_lifecycle( + PersistentSession::default().session_ttl(session_ttl), + ) .build(), ) .wrap(middleware::Logger::default()) @@ -456,7 +495,7 @@ pub mod test_helpers { .into_iter() .find(|c| c.name() == "test-session") .unwrap(); - assert_eq!(cookie_1.max_age(), Some(Duration::days(7))); + assert_eq!(cookie_1.max_age(), Some(session_ttl)); // Step 3: GET index, including session cookie #1 in request // - set-cookie will *not* be in response @@ -494,7 +533,7 @@ pub mod test_helpers { .into_iter() .find(|c| c.name() == "test-session") .unwrap(); - assert_eq!(cookie_2.max_age(), Some(Duration::days(7))); + assert_eq!(cookie_2.max_age(), cookie_1.max_age()); // Step 5: POST to login, including session cookie #2 in request // - set-cookie actix-session will be in response (session cookie #3) @@ -675,5 +714,18 @@ pub mod test_helpers { Ok(HttpResponse::Ok().body(body)) } + + trait ServiceResponseExt { + fn get_cookie(&self, cookie_name: &str) -> Option>; + } + + impl ServiceResponseExt for ServiceResponse { + fn get_cookie(&self, cookie_name: &str) -> Option> { + self.response() + .cookies() + .into_iter() + .find(|c| c.name() == cookie_name) + } + } } } diff --git a/actix-session/src/middleware.rs b/actix-session/src/middleware.rs index ce27cf4b5..09389ec26 100644 --- a/actix-session/src/middleware.rs +++ b/actix-session/src/middleware.rs @@ -3,15 +3,18 @@ use std::{collections::HashMap, convert::TryInto, fmt, future::Future, pin::Pin, use actix_utils::future::{ready, Ready}; use actix_web::{ body::MessageBody, - cookie::{Cookie, CookieJar, Key, SameSite}, + cookie::{Cookie, CookieJar, Key}, dev::{forward_ready, ResponseHead, Service, ServiceRequest, ServiceResponse, Transform}, http::header::{HeaderValue, SET_COOKIE}, HttpResponse, }; use anyhow::Context; -use time::Duration; use crate::{ + config::{ + self, Configuration, CookieConfiguration, CookieContentSecurity, SessionMiddlewareBuilder, + TtlExtensionPolicy, + }, storage::{LoadError, SessionKey, SessionStore}, Session, SessionStatus, }; @@ -66,8 +69,9 @@ use crate::{ /// If you want to customise use [`builder`](Self::builder) instead of [`new`](Self::new): /// /// ```no_run -/// use actix_web::{cookie::Key, web, App, HttpServer, HttpResponse, Error}; -/// use actix_session::{Session, SessionMiddleware, storage::RedisActorSessionStore, SessionLength}; +/// use actix_web::{App, cookie::{Key, time}, Error, HttpResponse, HttpServer, web}; +/// use actix_session::{Session, SessionMiddleware, storage::RedisActorSessionStore}; +/// use actix_session::config::PersistentSession; /// /// // The secret key would usually be read from a configuration file/environment variables. /// fn get_secret_key() -> Key { @@ -87,9 +91,10 @@ use crate::{ /// RedisActorSessionStore::new(redis_connection_string), /// secret_key.clone() /// ) -/// .session_length(SessionLength::Predetermined { -/// max_session_length: Some(time::Duration::days(5)), -/// }) +/// .session_lifecycle( +/// PersistentSession::default() +/// .session_ttl(time::Duration::days(5)) +/// ) /// .build(), /// ) /// .default_service(web::to(|| HttpResponse::Ok()))) @@ -114,117 +119,6 @@ pub struct SessionMiddleware { configuration: Rc, } -#[derive(Clone)] -struct Configuration { - cookie: CookieConfiguration, - session: SessionConfiguration, -} - -#[derive(Clone)] -struct SessionConfiguration { - state_ttl: Duration, -} - -#[derive(Clone)] -struct CookieConfiguration { - secure: bool, - http_only: bool, - name: String, - same_site: SameSite, - path: String, - domain: Option, - max_age: Option, - content_security: CookieContentSecurity, - key: Key, -} - -/// Describes how long a session should last. -/// -/// Used by [`SessionMiddlewareBuilder::session_length`]. -#[derive(Clone, Debug)] -pub enum SessionLength { - /// The session cookie will expire when the current browser session ends. - /// - /// When does a browser session end? It depends on the browser! Chrome, for example, will often - /// continue running in the background when the browser is closed—session cookies are not - /// deleted and they will still be available when the browser is opened again. Check the - /// documentation of the browsers you are targeting for up-to-date information. - BrowserSession { - /// We must provide a time-to-live (TTL) when storing the session state in the storage - /// backend—we do not want to store session states indefinitely, otherwise we will - /// inevitably run out of storage by holding on to the state of countless abandoned or - /// expired sessions! - /// - /// We are dealing with the lifecycle of two uncorrelated object here: the session cookie - /// and the session state. It is not a big issue if the session state outlives the cookie— - /// we are wasting some space in the backend storage, but it will be cleaned up eventually. - /// What happens, instead, if the cookie outlives the session state? A new session starts— - /// e.g. if sessions are being used for authentication, the user is de-facto logged out. - /// - /// It is not possible to predict with certainty how long a browser session is going to - /// last—you need to provide a reasonable upper bound. You do so via `state_ttl`—it dictates - /// what TTL should be used for session state when the lifecycle of the session cookie is - /// tied to the browser session length. [`SessionMiddleware`] will default to 1 day if - /// `state_ttl` is left unspecified. - state_ttl: Option, - }, - - /// The session cookie will be a [persistent cookie]. - /// - /// Persistent cookies have a pre-determined lifetime, specified via the `Max-Age` or `Expires` - /// attribute. They do not disappear when the current browser session ends. - /// - /// [persistent cookie]: https://www.whitehatsec.com/glossary/content/persistent-session-cookie - Predetermined { - /// Set `max_session_length` to specify how long the session cookie should live. - /// [`SessionMiddleware`] will default to 1 day if `max_session_length` is set to `None`. - /// - /// `max_session_length` is also used as the TTL for the session state in the - /// storage backend. - max_session_length: Option, - }, -} - -/// Used by [`SessionMiddlewareBuilder::cookie_content_security`] to determine how to secure -/// the content of the session cookie. -#[derive(Debug, Clone, Copy)] -pub enum CookieContentSecurity { - /// The cookie content is encrypted when using `CookieContentSecurity::Private`. - /// - /// Encryption guarantees confidentiality and integrity: the client cannot tamper with the - /// cookie content nor decode it, as long as the encryption key remains confidential. - Private, - - /// The cookie content is signed when using `CookieContentSecurity::Signed`. - /// - /// Signing guarantees integrity, but it doesn't ensure confidentiality: the client cannot - /// tamper with the cookie content, but they can read it. - Signed, -} - -fn default_configuration(key: Key) -> Configuration { - Configuration { - cookie: CookieConfiguration { - secure: true, - http_only: true, - name: "id".into(), - same_site: SameSite::Lax, - path: "/".into(), - domain: None, - max_age: None, - content_security: CookieContentSecurity::Private, - key, - }, - session: SessionConfiguration { - state_ttl: default_ttl(), - }, - } -} - -fn default_ttl() -> Duration { - Duration::days(1) -} - impl SessionMiddleware { /// Use [`SessionMiddleware::new`] to initialize the session framework using the default /// parameters. @@ -234,10 +128,7 @@ impl SessionMiddleware { /// [`SessionStore]); /// - a secret key, to sign or encrypt the content of client-side session cookie. pub fn new(store: Store, key: Key) -> Self { - Self { - storage_backend: Rc::new(store), - configuration: Rc::new(default_configuration(key)), - } + Self::builder(store, key).build() } /// A fluent API to configure [`SessionMiddleware`]. @@ -247,124 +138,13 @@ impl SessionMiddleware { /// [`SessionStore]); /// - a secret key, to sign or encrypt the content of client-side session cookie. pub fn builder(store: Store, key: Key) -> SessionMiddlewareBuilder { - SessionMiddlewareBuilder { + SessionMiddlewareBuilder::new(store, config::default_configuration(key)) + } + + pub(crate) fn from_parts(store: Store, configuration: Configuration) -> Self { + Self { storage_backend: Rc::new(store), - configuration: default_configuration(key), - } - } -} - -/// A fluent builder to construct a [`SessionMiddleware`] instance with custom configuration -/// parameters. -#[must_use] -pub struct SessionMiddlewareBuilder { - storage_backend: Rc, - configuration: Configuration, -} - -impl SessionMiddlewareBuilder { - /// Set the name of the cookie used to store the session ID. - /// - /// Defaults to `id`. - pub fn cookie_name(mut self, name: String) -> Self { - self.configuration.cookie.name = name; - self - } - - /// Set the `Secure` attribute for the cookie used to store the session ID. - /// - /// If the cookie is set as secure, it will only be transmitted when the connection is secure - /// (using `https`). - /// - /// Default is `true`. - pub fn cookie_secure(mut self, secure: bool) -> Self { - self.configuration.cookie.secure = secure; - self - } - - /// Determine how long a session should last - check out [`SessionLength`]'s documentation for - /// more details on the available options. - /// - /// Default is [`SessionLength::BrowserSession`]. - pub fn session_length(mut self, session_length: SessionLength) -> Self { - match session_length { - SessionLength::BrowserSession { state_ttl } => { - self.configuration.cookie.max_age = None; - self.configuration.session.state_ttl = state_ttl.unwrap_or_else(default_ttl); - } - SessionLength::Predetermined { max_session_length } => { - let ttl = max_session_length.unwrap_or_else(default_ttl); - self.configuration.cookie.max_age = Some(ttl); - self.configuration.session.state_ttl = ttl; - } - } - - self - } - - /// Set the `SameSite` attribute for the cookie used to store the session ID. - /// - /// By default, the attribute is set to `Lax`. - pub fn cookie_same_site(mut self, same_site: SameSite) -> Self { - self.configuration.cookie.same_site = same_site; - self - } - - /// Set the `Path` attribute for the cookie used to store the session ID. - /// - /// By default, the attribute is set to `/`. - pub fn cookie_path(mut self, path: String) -> Self { - self.configuration.cookie.path = path; - self - } - - /// Set the `Domain` attribute for the cookie used to store the session ID. - /// - /// Use `None` to leave the attribute unspecified. If unspecified, the attribute defaults - /// to the same host that set the cookie, excluding subdomains. - /// - /// By default, the attribute is left unspecified. - pub fn cookie_domain(mut self, domain: Option) -> Self { - self.configuration.cookie.domain = domain; - self - } - - /// Choose how the session cookie content should be secured. - /// - /// - [`CookieContentSecurity::Private`] selects encrypted cookie content. - /// - [`CookieContentSecurity::Signed`] selects signed cookie content. - /// - /// # Default - /// By default, the cookie content is encrypted. Encrypted was chosen instead of signed as - /// default because it reduces the chances of sensitive information being exposed in the session - /// key by accident, regardless of [`SessionStore`] implementation you chose to use. - /// - /// For example, if you are using cookie-based storage, you definitely want the cookie content - /// to be encrypted—the whole session state is embedded in the cookie! If you are using - /// Redis-based storage, signed is more than enough - the cookie content is just a unique - /// tamper-proof session key. - pub fn cookie_content_security(mut self, content_security: CookieContentSecurity) -> Self { - self.configuration.cookie.content_security = content_security; - self - } - - /// Set the `HttpOnly` attribute for the cookie used to store the session ID. - /// - /// If the cookie is set as `HttpOnly`, it will not be visible to any JavaScript snippets - /// running in the browser. - /// - /// Default is `true`. - pub fn cookie_http_only(mut self, http_only: bool) -> Self { - self.configuration.cookie.http_only = http_only; - self - } - - /// Finalise the builder and return a [`SessionMiddleware`] instance. - #[must_use] - pub fn build(self) -> SessionMiddleware { - SessionMiddleware { - storage_backend: self.storage_backend, - configuration: Rc::new(self.configuration), + configuration: Rc::new(configuration), } } } @@ -509,16 +289,39 @@ where } SessionStatus::Unchanged => { - // Nothing to do; we avoid the unnecessary call to the storage. + if matches!( + configuration.ttl_extension_policy, + TtlExtensionPolicy::OnEveryRequest + ) { + storage_backend + .update_ttl(&session_key, &configuration.session.state_ttl) + .await + .map_err(e500)?; + + if configuration.cookie.max_age.is_some() { + set_session_cookie( + res.response_mut().head_mut(), + session_key, + &configuration.cookie, + ) + .map_err(e500)?; + } + } } - } + }; } - }; + } + Ok(res) }) } } +/// Examines the session cookie attached to the incoming request, if there is one, and tries +/// to extract the session key. +/// +/// It returns `None` if there is no session cookie or if the session cookie is considered invalid +/// (e.g., when failing a signature check). fn extract_session_key(req: &ServiceRequest, config: &CookieConfiguration) -> Option { let cookies = req.cookies().ok()?; let session_cookie = cookies diff --git a/actix-session/src/storage/cookie.rs b/actix-session/src/storage/cookie.rs index 34bdceae4..10cc05bc6 100644 --- a/actix-session/src/storage/cookie.rs +++ b/actix-session/src/storage/cookie.rs @@ -1,6 +1,7 @@ use std::convert::TryInto; -use time::Duration; +use actix_web::cookie::time::Duration; +use anyhow::Error; use super::SessionKey; use crate::storage::{ @@ -34,9 +35,9 @@ use crate::storage::{ /// ``` /// /// # Limitations -/// Cookies are subject to size limits - we require session keys to be shorter than 4096 bytes. This -/// translates into a limit on the maximum size of the session state when using cookies as storage -/// backend. +/// Cookies are subject to size limits so we require session keys to be shorter than 4096 bytes. +/// This translates into a limit on the maximum size of the session state when using cookies as +/// storage backend. /// /// The session cookie can always be inspected by end users via the developer tools exposed by their /// browsers. We strongly recommend setting the policy to [`CookieContentSecurity::Private`] when @@ -45,7 +46,7 @@ use crate::storage::{ /// There is no way to invalidate a session before its natural expiry when using cookies as the /// storage backend. /// -/// [`CookieContentSecurity::Private`]: crate::CookieContentSecurity::Private +/// [`CookieContentSecurity::Private`]: crate::config::CookieContentSecurity::Private #[cfg_attr(docsrs, doc(cfg(feature = "cookie-session")))] #[derive(Default)] #[non_exhaustive] @@ -89,6 +90,10 @@ impl SessionStore for CookieSessionStore { }) } + async fn update_ttl(&self, _session_key: &SessionKey, _ttl: &Duration) -> Result<(), Error> { + Ok(()) + } + async fn delete(&self, _session_key: &SessionKey) -> Result<(), anyhow::Error> { Ok(()) } diff --git a/actix-session/src/storage/interface.rs b/actix-session/src/storage/interface.rs index 64b10338f..2b52c59ed 100644 --- a/actix-session/src/storage/interface.rs +++ b/actix-session/src/storage/interface.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; +use actix_web::cookie::time::Duration; use derive_more::Display; -use time::Duration; use super::SessionKey; @@ -36,6 +36,13 @@ pub trait SessionStore { ttl: &Duration, ) -> Result; + /// Updates the TTL of the session state associated to a pre-existing session key. + async fn update_ttl( + &self, + session_key: &SessionKey, + ttl: &Duration, + ) -> Result<(), anyhow::Error>; + /// Deletes a session from the store. async fn delete(&self, session_key: &SessionKey) -> Result<(), anyhow::Error>; } diff --git a/actix-session/src/storage/redis_actor.rs b/actix-session/src/storage/redis_actor.rs index f226dec34..744f01156 100644 --- a/actix-session/src/storage/redis_actor.rs +++ b/actix-session/src/storage/redis_actor.rs @@ -1,6 +1,7 @@ use actix::Addr; use actix_redis::{resp_array, Command, RedisActor, RespValue}; -use time::{self, Duration}; +use actix_web::cookie::time::Duration; +use anyhow::Error; use super::SessionKey; use crate::storage::{ @@ -238,6 +239,24 @@ impl SessionStore for RedisActorSessionStore { } } + async fn update_ttl(&self, session_key: &SessionKey, ttl: &Duration) -> Result<(), Error> { + let cache_key = (self.configuration.cache_keygen)(session_key.as_ref()); + + let cmd = Command(resp_array![ + "EXPIRE", + cache_key, + ttl.whole_seconds().to_string() + ]); + + match self.addr.send(cmd).await? { + Ok(RespValue::Integer(_)) => Ok(()), + val => Err(anyhow::anyhow!( + "Failed to update the session state TTL: {:?}", + val + )), + } + } + async fn delete(&self, session_key: &SessionKey) -> Result<(), anyhow::Error> { let cache_key = (self.configuration.cache_keygen)(session_key.as_ref()); @@ -258,9 +277,11 @@ impl SessionStore for RedisActorSessionStore { } #[cfg(test)] -mod test { +mod tests { use std::collections::HashMap; + use actix_web::cookie::time::Duration; + use super::*; use crate::test_helpers::acceptance_test_suite; @@ -286,7 +307,7 @@ mod test { let session_key = generate_session_key(); let initial_session_key = session_key.as_ref().to_owned(); let updated_session_key = store - .update(session_key, HashMap::new(), &time::Duration::seconds(1)) + .update(session_key, HashMap::new(), &Duration::seconds(1)) .await .unwrap(); assert_ne!(initial_session_key, updated_session_key.as_ref()); diff --git a/actix-session/src/storage/redis_rs.rs b/actix-session/src/storage/redis_rs.rs index cba2b356b..04a780279 100644 --- a/actix-session/src/storage/redis_rs.rs +++ b/actix-session/src/storage/redis_rs.rs @@ -1,7 +1,8 @@ -use std::sync::Arc; +use std::{convert::TryInto, sync::Arc}; -use redis::{aio::ConnectionManager, Cmd, FromRedisValue, RedisResult, Value}; -use time::{self, Duration}; +use actix_web::cookie::time::Duration; +use anyhow::{Context, Error}; +use redis::{aio::ConnectionManager, AsyncCommands, Cmd, FromRedisValue, RedisResult, Value}; use super::SessionKey; use crate::storage::{ @@ -28,6 +29,7 @@ use crate::storage::{ /// let secret_key = get_secret_key(); /// let redis_connection_string = "redis://127.0.0.1:6379"; /// let store = RedisSessionStore::new(redis_connection_string).await.unwrap(); +/// /// HttpServer::new(move || /// App::new() /// .wrap(SessionMiddleware::new( @@ -221,6 +223,21 @@ impl SessionStore for RedisSessionStore { } } + async fn update_ttl(&self, session_key: &SessionKey, ttl: &Duration) -> Result<(), Error> { + let cache_key = (self.configuration.cache_keygen)(session_key.as_ref()); + + self.client + .clone() + .expire( + &cache_key, + ttl.whole_seconds().try_into().context( + "Failed to convert the state TTL into the number of whole seconds remaining", + )?, + ) + .await?; + Ok(()) + } + async fn delete(&self, session_key: &SessionKey) -> Result<(), anyhow::Error> { let cache_key = (self.configuration.cache_keygen)(session_key.as_ref()); self.execute_command(redis::cmd("DEL").arg(&[&cache_key])) @@ -272,9 +289,10 @@ impl RedisSessionStore { } #[cfg(test)] -mod test { +mod tests { use std::collections::HashMap; + use actix_web::cookie::time; use redis::AsyncCommands; use super::*; diff --git a/actix-session/src/storage/session_key.rs b/actix-session/src/storage/session_key.rs index d82e18284..ad5c47a1d 100644 --- a/actix-session/src/storage/session_key.rs +++ b/actix-session/src/storage/session_key.rs @@ -2,16 +2,15 @@ use std::convert::TryFrom; use derive_more::{Display, From}; -/// A session key, the string stored in a client-side cookie to associate a user -/// with its session state on the backend. +/// A session key, the string stored in a client-side cookie to associate a user with its session +/// state on the backend. /// -/// ## Validation -/// -/// Session keys are stored as cookies, therefore they cannot be arbitrary long. -/// We require session keys to be smaller than 4064 bytes. +/// # Validation +/// Session keys are stored as cookies, therefore they cannot be arbitrary long. Session keys are +/// required to be smaller than 4064 bytes. /// /// ```rust -/// use std::convert::TryInto; +/// # use std::convert::TryInto; /// use actix_session::storage::SessionKey; /// /// let key: String = std::iter::repeat('a').take(4065).collect(); @@ -24,15 +23,15 @@ pub struct SessionKey(String); impl TryFrom for SessionKey { type Error = InvalidSessionKeyError; - fn try_from(v: String) -> Result { - if v.len() > 4064 { + fn try_from(val: String) -> Result { + if val.len() > 4064 { return Err(anyhow::anyhow!( "The session key is bigger than 4064 bytes, the upper limit on cookie content." ) .into()); } - Ok(SessionKey(v)) + Ok(SessionKey(val)) } } @@ -43,8 +42,8 @@ impl AsRef for SessionKey { } impl From for String { - fn from(k: SessionKey) -> Self { - k.0 + fn from(key: SessionKey) -> Self { + key.0 } } diff --git a/actix-session/tests/opaque_errors.rs b/actix-session/tests/opaque_errors.rs index 90e0f5c95..378a34752 100644 --- a/actix-session/tests/opaque_errors.rs +++ b/actix-session/tests/opaque_errors.rs @@ -71,6 +71,10 @@ impl SessionStore for MockStore { todo!() } + async fn update_ttl(&self, _session_key: &SessionKey, _ttl: &Duration) -> Result<(), Error> { + todo!() + } + async fn delete(&self, _session_key: &SessionKey) -> Result<(), Error> { todo!() }