From b3587c677ce059990a0b8cb570fa313f040c7747 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Tue, 12 Oct 2021 19:03:01 +0200 Subject: [PATCH] WIP: Refactor higher-level data-model to its own crate --- Cargo.lock | 11 + crates/core/Cargo.toml | 1 + crates/core/sqlx-data.json | 204 ++++++-------- crates/core/src/filters/session.rs | 32 ++- .../core/src/handlers/oauth2/authorization.rs | 14 +- crates/core/src/handlers/views/index.rs | 7 +- crates/core/src/handlers/views/login.rs | 7 +- crates/core/src/handlers/views/logout.rs | 17 +- crates/core/src/handlers/views/reauth.rs | 19 +- crates/core/src/handlers/views/register.rs | 7 +- crates/core/src/storage/mod.rs | 22 +- crates/core/src/storage/oauth2/session.rs | 22 +- crates/core/src/storage/user.rs | 252 ++++++++---------- crates/core/src/templates/context.rs | 18 +- crates/core/src/templates/res/base.html | 2 +- crates/data-model/Cargo.toml | 13 + crates/data-model/src/lib.rs | 134 ++++++++++ 17 files changed, 456 insertions(+), 326 deletions(-) create mode 100644 crates/data-model/Cargo.toml create mode 100644 crates/data-model/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 33aaed3e..d715ad2e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1390,6 +1390,7 @@ dependencies = [ "jwt-compact", "k256", "mas-config", + "mas-data-model", "mime", "oauth2-types", "password-hash", @@ -1413,6 +1414,16 @@ dependencies = [ "warp", ] +[[package]] +name = "mas-data-model" +version = "0.1.0" +dependencies = [ + "chrono", + "oauth2-types", + "serde", + "thiserror", +] + [[package]] name = "matchers" version = "0.0.1" diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index 89904dcd..605c3bfc 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -66,6 +66,7 @@ cookie = "0.15.1" oauth2-types = { path = "../oauth2-types", features = ["sqlx_type"] } mas-config = { path = "../config" } +mas-data-model = { path = "../data-model" } [dependencies.jwt-compact] # Waiting on the next release because of the bump of the `rsa` dependency diff --git a/crates/core/sqlx-data.json b/crates/core/sqlx-data.json index 06f41ccb..fa7fa539 100644 --- a/crates/core/sqlx-data.json +++ b/crates/core/sqlx-data.json @@ -93,8 +93,8 @@ ] } }, - "35bedaa6fdf7ac91d54b458b4637f2182c2f82be3e2f80cd2db934ee279a7f2a": { - "query": "\n SELECT id, username\n FROM users\n WHERE id = $1\n ", + "307fd9f71e7a94a0a0d9ce523ee9792e127485d0d12480c43f179dd9b75afbab": { + "query": "\n INSERT INTO user_sessions (user_id)\n VALUES ($1)\n RETURNING id, created_at\n ", "describe": { "columns": [ { @@ -104,8 +104,8 @@ }, { "ordinal": 1, - "name": "username", - "type_info": "Text" + "name": "created_at", + "type_info": "Timestamptz" } ], "parameters": { @@ -169,56 +169,6 @@ ] } }, - "4f925a277d73df779360f81e0cf5d7983b50ebe744f461559dd561b7e36c20d4": { - "query": "\n SELECT\n s.id,\n u.id as user_id,\n u.username,\n s.active,\n s.created_at,\n a.created_at as \"last_authd_at?\"\n FROM user_sessions s\n INNER JOIN users u \n ON s.user_id = u.id\n LEFT JOIN user_session_authentications a\n ON a.session_id = s.id\n WHERE s.id = $1 AND s.active\n ORDER BY a.created_at DESC\n LIMIT 1\n ", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "id", - "type_info": "Int8" - }, - { - "ordinal": 1, - "name": "user_id", - "type_info": "Int8" - }, - { - "ordinal": 2, - "name": "username", - "type_info": "Text" - }, - { - "ordinal": 3, - "name": "active", - "type_info": "Bool" - }, - { - "ordinal": 4, - "name": "created_at", - "type_info": "Timestamptz" - }, - { - "ordinal": 5, - "name": "last_authd_at?", - "type_info": "Timestamptz" - } - ], - "parameters": { - "Left": [ - "Int8" - ] - }, - "nullable": [ - false, - false, - false, - false, - false, - false - ] - } - }, "562b0d4dcf857e99c20e9288e9c8bd46232290715c0d2459b0398a1c746cf65d": { "query": "\n SELECT\n rt.id,\n rt.oauth2_session_id,\n rt.oauth2_access_token_id,\n os.client_id AS \"client_id!\",\n os.scope AS \"scope!\"\n FROM oauth2_refresh_tokens rt\n INNER JOIN oauth2_sessions os\n ON os.id = rt.oauth2_session_id\n WHERE rt.token = $1 AND rt.next_token_id IS NULL\n ", "describe": { @@ -273,56 +223,6 @@ "nullable": [] } }, - "62986972431bfc4649e3d8c8c7648f9049c4197773e53496422ad8b8aa15b459": { - "query": "\n SELECT\n s.id,\n u.id as user_id,\n u.username,\n s.active,\n s.created_at,\n a.created_at as \"last_authd_at?\"\n FROM user_sessions s\n INNER JOIN users u \n ON s.user_id = u.id\n LEFT JOIN user_session_authentications a\n ON a.session_id = s.id\n WHERE s.id = $1\n ORDER BY a.created_at DESC\n LIMIT 1\n ", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "id", - "type_info": "Int8" - }, - { - "ordinal": 1, - "name": "user_id", - "type_info": "Int8" - }, - { - "ordinal": 2, - "name": "username", - "type_info": "Text" - }, - { - "ordinal": 3, - "name": "active", - "type_info": "Bool" - }, - { - "ordinal": 4, - "name": "created_at", - "type_info": "Timestamptz" - }, - { - "ordinal": 5, - "name": "last_authd_at?", - "type_info": "Timestamptz" - } - ], - "parameters": { - "Left": [ - "Int8" - ] - }, - "nullable": [ - false, - false, - false, - false, - false, - false - ] - } - }, "73f2d928f7bf88af79a3685bd6346652b4e4454b0ce75e38343840c9765e3f27": { "query": "\n INSERT INTO oauth2_refresh_tokens\n (oauth2_session_id, oauth2_access_token_id, token)\n VALUES\n ($1, $2, $3)\n RETURNING\n id, oauth2_session_id, oauth2_access_token_id, token, next_token_id, \n created_at, updated_at\n ", "describe": { @@ -381,6 +281,56 @@ ] } }, + "7fa7d001583e98f5e4626751fa2c8743e7b83a240d1a51de50a7dba95c4e8a6b": { + "query": "\n SELECT\n s.id,\n u.id as user_id,\n u.username,\n s.created_at,\n a.id as \"last_authentication_id?\",\n a.created_at as \"last_authd_at?\"\n FROM user_sessions s\n INNER JOIN users u \n ON s.user_id = u.id\n LEFT JOIN user_session_authentications a\n ON a.session_id = s.id\n WHERE s.id = $1 AND s.active\n ORDER BY a.created_at DESC\n LIMIT 1\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Int8" + }, + { + "ordinal": 1, + "name": "user_id", + "type_info": "Int8" + }, + { + "ordinal": 2, + "name": "username", + "type_info": "Text" + }, + { + "ordinal": 3, + "name": "created_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 4, + "name": "last_authentication_id?", + "type_info": "Int8" + }, + { + "ordinal": 5, + "name": "last_authd_at?", + "type_info": "Timestamptz" + } + ], + "parameters": { + "Left": [ + "Int8" + ] + }, + "nullable": [ + false, + false, + false, + false, + false, + false + ] + } + }, "88ac8783bd5881c42eafd9cf87a16fe6031f3153fd6a8618e689694584aeb2de": { "query": "\n DELETE FROM oauth2_access_tokens\n WHERE id = $1\n ", "describe": { @@ -393,26 +343,6 @@ "nullable": [] } }, - "9ba45ab114b656105cc46b0c10fb05769860fcdc05eaf54d6225640fb914dab9": { - "query": "\n INSERT INTO user_session_authentications (session_id)\n VALUES ($1)\n RETURNING created_at\n ", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "created_at", - "type_info": "Timestamptz" - } - ], - "parameters": { - "Left": [ - "Int8" - ] - }, - "nullable": [ - false - ] - } - }, "a09dfe1019110f2ec6eba0d35bafa467ab4b7980dd8b556826f03863f8edb0ab": { "query": "UPDATE user_sessions SET active = FALSE WHERE id = $1", "describe": { @@ -611,6 +541,32 @@ ] } }, + "d9d27eb4a0c11818a636d407438c4bc567a39396e7e236b3e776504417988eab": { + "query": "\n INSERT INTO user_session_authentications (session_id)\n VALUES ($1)\n RETURNING id, created_at\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Int8" + }, + { + "ordinal": 1, + "name": "created_at", + "type_info": "Timestamptz" + } + ], + "parameters": { + "Left": [ + "Int8" + ] + }, + "nullable": [ + false, + false + ] + } + }, "eaddc1e33715ad31b4195fda72dbe870f179dd8da53a88d0543b72a278ed1d3d": { "query": "\n DELETE FROM oauth2_codes\n WHERE id = $1\n ", "describe": { diff --git a/crates/core/src/filters/session.rs b/crates/core/src/filters/session.rs index 8489e84f..49e87dac 100644 --- a/crates/core/src/filters/session.rs +++ b/crates/core/src/filters/session.rs @@ -14,6 +14,7 @@ //! Load user sessions from the database +use mas_data_model::BrowserSession; use serde::{Deserialize, Serialize}; use sqlx::{pool::PoolConnection, Executor, PgPool, Postgres}; use thiserror::Error; @@ -30,7 +31,7 @@ use super::{ }; use crate::{ config::CookiesConfig, - storage::{lookup_active_session, user::ActiveSessionLookupError, SessionInfo}, + storage::{lookup_active_session, user::ActiveSessionLookupError, PostgresqlBackend}, }; /// The session is missing or failed to load @@ -58,19 +59,19 @@ pub struct SessionCookie { } impl SessionCookie { - /// Forge the cookie from a [`SessionInfo`] + /// Forge the cookie from a [`BrowserSession`] #[must_use] - pub fn from_session_info(info: &SessionInfo) -> Self { + pub fn from_session(session: &BrowserSession) -> Self { Self { - current: info.key(), + current: session.data, } } - /// Load the [`SessionInfo`] from database - pub async fn load_session_info( + /// Load the [`BrowserSession`] from database + pub async fn load_session( &self, executor: impl Executor<'_, Database = Postgres>, - ) -> Result { + ) -> Result, ActiveSessionLookupError> { let res = lookup_active_session(executor, self.current).await?; Ok(res) } @@ -87,8 +88,11 @@ impl EncryptableCookieValue for SessionCookie { pub fn optional_session( pool: &PgPool, cookies_config: &CookiesConfig, -) -> impl Filter,), Error = Rejection> + Clone + Send + Sync + 'static -{ +) -> impl Filter>,), Error = Rejection> + + Clone + + Send + + Sync + + 'static { session(pool, cookies_config) .map(Some) .recover(none_on_error::<_, SessionLoadError>) @@ -106,7 +110,11 @@ pub fn optional_session( pub fn session( pool: &PgPool, cookies_config: &CookiesConfig, -) -> impl Filter + Clone + Send + Sync + 'static { +) -> impl Filter,), Error = Rejection> + + Clone + + Send + + Sync + + 'static { encrypted(cookies_config) .and(connection(pool)) .and_then(load_session) @@ -117,8 +125,8 @@ pub fn session( async fn load_session( session: SessionCookie, mut conn: PoolConnection, -) -> Result { - let session_info = session.load_session_info(&mut conn).await?; +) -> Result, Rejection> { + let session_info = session.load_session(&mut conn).await?; Ok(session_info) } diff --git a/crates/core/src/handlers/oauth2/authorization.rs b/crates/core/src/handlers/oauth2/authorization.rs index b5b73a1b..312ae589 100644 --- a/crates/core/src/handlers/oauth2/authorization.rs +++ b/crates/core/src/handlers/oauth2/authorization.rs @@ -24,6 +24,7 @@ use hyper::{ StatusCode, }; use itertools::Itertools; +use mas_data_model::BrowserSession; use oauth2_types::{ errors::{ErrorResponse, InvalidRequest, OAuth2Error}, pkce, @@ -59,7 +60,7 @@ use crate::{ refresh_token::add_refresh_token, session::{get_session_by_id, start_session}, }, - SessionInfo, + PostgresqlBackend, }, templates::{FormPostContext, Templates}, tokens::{AccessToken, RefreshToken}, @@ -297,7 +298,7 @@ async fn actually_reply( async fn get( clients: Vec, params: Params, - maybe_session: Option, + maybe_session: Option>, mut txn: Transaction<'_, Postgres>, ) -> Result { // First, find out what client it is @@ -307,7 +308,7 @@ async fn get( .ok_or_else(|| anyhow::anyhow!("could not find client")) .wrap_error()?; - let maybe_session_id = maybe_session.as_ref().map(SessionInfo::key); + let maybe_session_id = maybe_session.as_ref().map(|s| s.data); let scope: String = { let it = params.auth.scope.iter().map(ToString::to_string); @@ -394,7 +395,7 @@ impl StepRequest { async fn step( oauth2_session_id: i64, - user_session: SessionInfo, + user_session: BrowserSession, mut txn: Transaction<'_, Postgres>, ) -> Result { let mut oauth2_session = get_session_by_id(&mut txn, oauth2_session_id) @@ -411,8 +412,9 @@ async fn step( let redirect_uri = oauth2_session.redirect_uri().wrap_error()?; // Check if the active session is valid - let reply = if user_session.active - && user_session.last_authd_at >= oauth2_session.max_auth_time() + // TODO: this is ugly & should check if the session is active + let reply = if user_session.last_authentication.map(|x| x.created_at) + >= oauth2_session.max_auth_time() { // Yep! Let's complete the auth now let mut params = AuthorizationResponse::default(); diff --git a/crates/core/src/handlers/views/index.rs b/crates/core/src/handlers/views/index.rs index 946eabc7..33e67093 100644 --- a/crates/core/src/handlers/views/index.rs +++ b/crates/core/src/handlers/views/index.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use mas_data_model::BrowserSession; use sqlx::PgPool; use url::Url; use warp::{reply::html, Filter, Rejection, Reply}; @@ -24,7 +25,7 @@ use crate::{ session::optional_session, with_templates, CsrfToken, }, - storage::SessionInfo, + storage::PostgresqlBackend, templates::{IndexContext, TemplateContext, Templates}, }; @@ -51,10 +52,10 @@ async fn get( templates: Templates, cookie_saver: EncryptedCookieSaver, csrf_token: CsrfToken, - session: Option, + maybe_session: Option>, ) -> Result { let ctx = IndexContext::new(discovery_url) - .maybe_with_session(session) + .maybe_with_session(maybe_session) .with_csrf(&csrf_token); let content = templates.render_index(&ctx)?; diff --git a/crates/core/src/handlers/views/login.rs b/crates/core/src/handlers/views/login.rs index 701851b8..7f667bb9 100644 --- a/crates/core/src/handlers/views/login.rs +++ b/crates/core/src/handlers/views/login.rs @@ -15,6 +15,7 @@ use std::convert::TryFrom; use hyper::http::uri::{Parts, PathAndQuery, Uri}; +use mas_data_model::BrowserSession; use serde::{Deserialize, Serialize}; use sqlx::{pool::PoolConnection, PgPool, Postgres}; use warp::{reply::html, Filter, Rejection, Reply}; @@ -29,7 +30,7 @@ use crate::{ session::{optional_session, SessionCookie}, with_templates, CsrfToken, }, - storage::{login, SessionInfo}, + storage::{login, PostgresqlBackend}, templates::{LoginContext, LoginFormField, TemplateContext, Templates}, }; @@ -108,7 +109,7 @@ async fn get( cookie_saver: EncryptedCookieSaver, csrf_token: CsrfToken, query: LoginRequest, - maybe_session: Option, + maybe_session: Option>, ) -> Result, Rejection> { if maybe_session.is_some() { Ok(Box::new(query.redirect()?)) @@ -133,7 +134,7 @@ async fn post( // TODO: recover match login(&mut conn, &form.username, form.password).await { Ok(session_info) => { - let session_cookie = SessionCookie::from_session_info(&session_info); + let session_cookie = SessionCookie::from_session(&session_info); let reply = query.redirect()?; let reply = cookie_saver.save_encrypted(&session_cookie, reply)?; Ok(Box::new(reply)) diff --git a/crates/core/src/handlers/views/logout.rs b/crates/core/src/handlers/views/logout.rs index e5baf058..6dc06760 100644 --- a/crates/core/src/handlers/views/logout.rs +++ b/crates/core/src/handlers/views/logout.rs @@ -12,14 +12,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -use sqlx::{pool::PoolConnection, PgPool, Postgres}; +use mas_data_model::BrowserSession; +use sqlx::{PgPool, Postgres, Transaction}; use warp::{hyper::Uri, Filter, Rejection, Reply}; use crate::{ config::CookiesConfig, errors::WrapError, - filters::{csrf::protected_form, database::connection, session::session}, - storage::SessionInfo, + filters::{csrf::protected_form, database::transaction, session::session}, + storage::{user::end_session, PostgresqlBackend}, }; pub(super) fn filter( @@ -29,16 +30,18 @@ pub(super) fn filter( warp::path!("logout") .and(warp::post()) .and(session(pool, cookies_config)) - .and(connection(pool)) + .and(transaction(pool)) .and(protected_form(cookies_config)) .and_then(post) } async fn post( - session: SessionInfo, - mut conn: PoolConnection, + session: BrowserSession, + mut txn: Transaction<'_, Postgres>, _form: (), ) -> Result { - session.end(&mut conn).await.wrap_error()?; + end_session(&mut txn, &session).await.wrap_error()?; + txn.commit().await.wrap_error()?; + Ok(warp::redirect(Uri::from_static("/login"))) } diff --git a/crates/core/src/handlers/views/reauth.rs b/crates/core/src/handlers/views/reauth.rs index 669b984d..b8bc2f3c 100644 --- a/crates/core/src/handlers/views/reauth.rs +++ b/crates/core/src/handlers/views/reauth.rs @@ -12,8 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +use mas_data_model::BrowserSession; use serde::Deserialize; -use sqlx::{pool::PoolConnection, PgPool, Postgres}; +use sqlx::{PgPool, Postgres, Transaction}; use warp::{hyper::Uri, reply::html, Filter, Rejection, Reply}; use crate::{ @@ -22,11 +23,11 @@ use crate::{ filters::{ cookies::{encrypted_cookie_saver, EncryptedCookieSaver}, csrf::{protected_form, updated_csrf_token}, - database::connection, + database::transaction, session::session, with_templates, CsrfToken, }, - storage::SessionInfo, + storage::{user::authenticate_session, PostgresqlBackend}, templates::{EmptyContext, TemplateContext, Templates}, }; @@ -50,7 +51,7 @@ pub(super) fn filter( let post = warp::post() .and(session(pool, cookies_config)) - .and(connection(pool)) + .and(transaction(pool)) .and(protected_form(cookies_config)) .and_then(post); @@ -61,7 +62,7 @@ async fn get( templates: Templates, cookie_saver: EncryptedCookieSaver, csrf_token: CsrfToken, - session: SessionInfo, + session: BrowserSession, ) -> Result { let ctx = EmptyContext.with_session(session).with_csrf(&csrf_token); @@ -72,14 +73,14 @@ async fn get( } async fn post( - session: SessionInfo, - mut conn: PoolConnection, + session: BrowserSession, + mut txn: Transaction<'_, Postgres>, form: ReauthForm, ) -> Result { - let _session = session - .reauth(&mut conn, form.password) + authenticate_session(&mut txn, &session, form.password) .await .wrap_error()?; + txn.commit().await.wrap_error()?; Ok(warp::redirect(Uri::from_static("/"))) } diff --git a/crates/core/src/handlers/views/register.rs b/crates/core/src/handlers/views/register.rs index 19c6a6be..99af9953 100644 --- a/crates/core/src/handlers/views/register.rs +++ b/crates/core/src/handlers/views/register.rs @@ -16,6 +16,7 @@ use std::convert::TryFrom; use argon2::Argon2; use hyper::http::uri::{Parts, PathAndQuery, Uri}; +use mas_data_model::BrowserSession; use serde::{Deserialize, Serialize}; use sqlx::{pool::PoolConnection, PgPool, Postgres}; use warp::{reply::html, Filter, Rejection, Reply}; @@ -30,7 +31,7 @@ use crate::{ session::{optional_session, SessionCookie}, with_templates, CsrfToken, }, - storage::{register_user, user::start_session, SessionInfo}, + storage::{register_user, user::start_session, PostgresqlBackend}, templates::{EmptyContext, TemplateContext, Templates}, }; @@ -110,7 +111,7 @@ async fn get( cookie_saver: EncryptedCookieSaver, csrf_token: CsrfToken, query: RegisterRequest, - maybe_session: Option, + maybe_session: Option>, ) -> Result, Rejection> { if maybe_session.is_some() { Ok(Box::new(query.redirect()?)) @@ -140,7 +141,7 @@ async fn post( let session_info = start_session(&mut conn, user).await.wrap_error()?; - let session_cookie = SessionCookie::from_session_info(&session_info); + let session_cookie = SessionCookie::from_session(&session_info); let reply = query.redirect()?; let reply = cookie_saver.save_encrypted(&session_cookie, reply)?; Ok(reply) diff --git a/crates/core/src/storage/mod.rs b/crates/core/src/storage/mod.rs index 90952ded..14869cc5 100644 --- a/crates/core/src/storage/mod.rs +++ b/crates/core/src/storage/mod.rs @@ -16,12 +16,32 @@ #![allow(clippy::used_underscore_binding)] // This is needed by sqlx macros +use mas_data_model::StorageBackend; +use serde::Serialize; use sqlx::migrate::Migrator; +use thiserror::Error; + +#[derive(Debug, Error)] +#[error("databse query returned an inconsistent state")] +pub struct DatabaseInconsistencyError; + +#[derive(Serialize, Debug, Clone, PartialEq)] +pub struct PostgresqlBackend; + +impl StorageBackend for PostgresqlBackend { + type AccessTokenData = i64; + type AuthenticationData = i64; + type AuthorizationCodeData = i64; + type BrowserSessionData = i64; + type ClientData = (); + type SessionData = i64; + type UserData = i64; +} pub mod oauth2; pub mod user; -pub use self::user::{login, lookup_active_session, register_user, SessionInfo, User}; +pub use self::user::{login, lookup_active_session, register_user}; /// Embedded migrations, allowing them to run on startup pub static MIGRATOR: Migrator = sqlx::migrate!(); diff --git a/crates/core/src/storage/oauth2/session.rs b/crates/core/src/storage/oauth2/session.rs index 8974d6c3..7af4c700 100644 --- a/crates/core/src/storage/oauth2/session.rs +++ b/crates/core/src/storage/oauth2/session.rs @@ -17,6 +17,7 @@ use std::{collections::HashSet, convert::TryFrom, str::FromStr, string::ToString use anyhow::Context; use chrono::{DateTime, Duration, Utc}; use itertools::Itertools; +use mas_data_model::BrowserSession; use oauth2_types::{ pkce, requests::{ResponseMode, ResponseType}, @@ -25,10 +26,8 @@ use serde::Serialize; use sqlx::{Executor, FromRow, Postgres}; use url::Url; -use super::{ - super::{user::lookup_session, SessionInfo}, - authorization_code::{add_code, OAuth2Code}, -}; +use super::authorization_code::{add_code, OAuth2Code}; +use crate::storage::{lookup_active_session, PostgresqlBackend}; #[derive(FromRow, Serialize)] pub struct OAuth2Session { @@ -60,10 +59,11 @@ impl OAuth2Session { pub async fn fetch_session( &self, executor: impl Executor<'_, Database = Postgres>, - ) -> anyhow::Result> { + ) -> anyhow::Result>> { match self.user_session_id { Some(id) => { - let info = lookup_session(executor, id).await?; + // TODO: and if the session is inactive? + let info = lookup_active_session(executor, id).await?; Ok(Some(info)) } None => Ok(None), @@ -80,19 +80,19 @@ impl OAuth2Session { pub async fn match_or_set_session( &mut self, executor: impl Executor<'_, Database = Postgres>, - session: SessionInfo, - ) -> anyhow::Result { + session: BrowserSession, + ) -> anyhow::Result> { match self.user_session_id { - Some(id) if id == session.key() => Ok(session), + Some(id) if id == session.data => Ok(session), Some(id) => Err(anyhow::anyhow!( "session mismatch, expected {}, got {}", id, - session.key() + session.data )), None => { sqlx::query!( "UPDATE oauth2_sessions SET user_session_id = $1 WHERE id = $2", - session.key(), + session.data, self.id, ) .execute(executor) diff --git a/crates/core/src/storage/user.rs b/crates/core/src/storage/user.rs index 35fae3a4..2aa6cdcc 100644 --- a/crates/core/src/storage/user.rs +++ b/crates/core/src/storage/user.rs @@ -12,76 +12,29 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::borrow::BorrowMut; +use std::{borrow::BorrowMut, convert::TryInto}; use anyhow::Context; use argon2::Argon2; use chrono::{DateTime, Utc}; +use mas_data_model::{Authentication, BrowserSession, User}; use password_hash::{PasswordHash, PasswordHasher, SaltString}; use rand::rngs::OsRng; -use serde::Serialize; use sqlx::{Acquire, Executor, FromRow, Postgres, Transaction}; use thiserror::Error; use tokio::task; use tracing::{info_span, Instrument}; use warp::reject::Reject; +use super::{DatabaseInconsistencyError, PostgresqlBackend}; use crate::errors::HtmlError; -#[derive(Serialize, Debug, Clone, FromRow)] -pub struct User { +#[derive(Debug, Clone, FromRow)] +struct UserLookup { pub id: i64, pub username: String, } -#[derive(Serialize, Debug, Clone, FromRow)] -pub struct SessionInfo { - id: i64, - user_id: i64, - username: String, - pub active: bool, - created_at: DateTime, - pub last_authd_at: Option>, -} - -impl SessionInfo { - #[must_use] - pub fn key(&self) -> i64 { - self.id - } - - pub async fn reauth( - mut self, - conn: impl Acquire<'_, Database = Postgres>, - password: String, - ) -> anyhow::Result { - let mut txn = conn.begin().await?; - self.last_authd_at = Some(authenticate_session(&mut txn, self.id, password).await?); - txn.commit().await?; - Ok(self) - } - - pub async fn end( - mut self, - executor: impl Executor<'_, Database = Postgres>, - ) -> anyhow::Result { - end_session(executor, self.id).await?; - self.active = false; - Ok(self) - } - - pub(crate) fn samples() -> Vec { - vec![SessionInfo { - id: 1, - user_id: 2, - username: "john".to_string(), - active: true, - created_at: Utc::now(), - last_authd_at: Some(Utc::now()), - }] - } -} - #[derive(Debug, Error)] pub enum LoginError { #[error("could not find user {username:?}")] @@ -116,7 +69,7 @@ pub async fn login( conn: impl Acquire<'_, Database = Postgres>, username: &str, password: String, -) -> Result { +) -> Result, LoginError> { let mut txn = conn.begin().await.context("could not start transaction")?; let user = lookup_user_by_username(&mut txn, username) .await @@ -132,8 +85,8 @@ pub async fn login( })?; let mut session = start_session(&mut txn, user).await?; - session.last_authd_at = Some( - authenticate_session(&mut txn, session.id, password) + session.last_authentication = Some( + authenticate_session(&mut txn, &session, password) .await .map_err(|source| { if matches!(source, AuthenticationError::Password { .. }) { @@ -152,30 +105,73 @@ pub async fn login( #[derive(Debug, Error)] #[error("could not fetch session")] -pub struct ActiveSessionLookupError(#[from] sqlx::Error); +pub enum ActiveSessionLookupError { + Fetch(#[from] sqlx::Error), + Conversion(#[from] DatabaseInconsistencyError), +} impl Reject for ActiveSessionLookupError {} impl ActiveSessionLookupError { #[must_use] pub fn not_found(&self) -> bool { - matches!(self.0, sqlx::Error::RowNotFound) + matches!( + self, + ActiveSessionLookupError::Fetch(sqlx::Error::RowNotFound) + ) + } +} + +struct SessionLookup { + id: i64, + user_id: i64, + username: String, + created_at: DateTime, + last_authentication_id: Option, + last_authd_at: Option>, +} + +impl TryInto> for SessionLookup { + type Error = DatabaseInconsistencyError; + + fn try_into(self) -> Result, Self::Error> { + let user = User { + data: self.user_id, + username: self.username, + sub: format!("fake-sub-{}", self.user_id), + }; + + let last_authentication = match (self.last_authentication_id, self.last_authd_at) { + (Some(id), Some(created_at)) => Some(Authentication { + data: id, + created_at, + }), + (None, None) => None, + _ => return Err(DatabaseInconsistencyError), + }; + + Ok(BrowserSession { + data: self.id, + user, + created_at: self.created_at, + last_authentication, + }) } } pub async fn lookup_active_session( executor: impl Executor<'_, Database = Postgres>, id: i64, -) -> Result { +) -> Result, ActiveSessionLookupError> { let res = sqlx::query_as!( - SessionInfo, + SessionLookup, r#" SELECT s.id, u.id as user_id, u.username, - s.active, s.created_at, + a.id as "last_authentication_id?", a.created_at as "last_authd_at?" FROM user_sessions s INNER JOIN users u @@ -189,65 +185,43 @@ pub async fn lookup_active_session( id, ) .fetch_one(executor) - .await?; + .await? + .try_into()?; Ok(res) } -pub async fn lookup_session( - executor: impl Executor<'_, Database = Postgres>, +#[derive(FromRow)] +struct SessionStartResult { id: i64, -) -> anyhow::Result { - sqlx::query_as!( - SessionInfo, - r#" - SELECT - s.id, - u.id as user_id, - u.username, - s.active, - s.created_at, - a.created_at as "last_authd_at?" - FROM user_sessions s - INNER JOIN users u - ON s.user_id = u.id - LEFT JOIN user_session_authentications a - ON a.session_id = s.id - WHERE s.id = $1 - ORDER BY a.created_at DESC - LIMIT 1 - "#, - id, - ) - .fetch_one(executor) - .await - .context("could not fetch session") + created_at: DateTime, } pub async fn start_session( executor: impl Executor<'_, Database = Postgres>, - user: User, -) -> anyhow::Result { - let (id, created_at): (i64, DateTime) = sqlx::query_as( + user: User, +) -> anyhow::Result> { + let res = sqlx::query_as!( + SessionStartResult, r#" INSERT INTO user_sessions (user_id) VALUES ($1) RETURNING id, created_at "#, + user.data, ) - .bind(user.id) .fetch_one(executor) .await .context("could not create session")?; - Ok(SessionInfo { - id, - user_id: user.id, - username: user.username, - active: true, - created_at, - last_authd_at: None, - }) + let session = BrowserSession { + data: res.id, + user, + created_at: res.created_at, + last_authentication: None, + }; + + Ok(session) } #[derive(Debug, Error)] @@ -265,11 +239,17 @@ pub enum AuthenticationError { Internal(#[from] tokio::task::JoinError), } +#[derive(FromRow)] +struct AuthenticationInsertionResult { + id: i64, + created_at: DateTime, +} + pub async fn authenticate_session( txn: &mut Transaction<'_, Postgres>, - session_id: i64, + session: &BrowserSession, password: String, -) -> Result, AuthenticationError> { +) -> Result, AuthenticationError> { // First, fetch the hashed password from the user associated with that session let hashed_password: String = sqlx::query_scalar!( r#" @@ -279,7 +259,7 @@ pub async fn authenticate_session( ON u.id = s.user_id WHERE s.id = $1 "#, - session_id, + session.data, ) .fetch_one(txn.borrow_mut()) .await @@ -297,19 +277,23 @@ pub async fn authenticate_session( .await??; // That went well, let's insert the auth info - let created_at: DateTime = sqlx::query_scalar!( + let res = sqlx::query_as!( + AuthenticationInsertionResult, r#" INSERT INTO user_session_authentications (session_id) VALUES ($1) - RETURNING created_at + RETURNING id, created_at "#, - session_id, + session.data, ) .fetch_one(txn.borrow_mut()) .await .map_err(AuthenticationError::Save)?; - Ok(created_at) + Ok(Authentication { + data: res.id, + created_at: res.created_at, + }) } pub async fn register_user( @@ -317,7 +301,7 @@ pub async fn register_user( phf: impl PasswordHasher, username: &str, password: &str, -) -> anyhow::Result { +) -> anyhow::Result> { let salt = SaltString::generate(&mut OsRng); let hashed_password = PasswordHash::generate(phf, password, salt.as_str())?; @@ -336,20 +320,24 @@ pub async fn register_user( .context("could not insert user")?; Ok(User { - id, + data: id, username: username.to_string(), + sub: format!("fake-sub-{}", id), }) } pub async fn end_session( executor: impl Executor<'_, Database = Postgres>, - id: i64, + session: &BrowserSession, ) -> anyhow::Result<()> { - let res = sqlx::query!("UPDATE user_sessions SET active = FALSE WHERE id = $1", id) - .execute(executor) - .instrument(info_span!("End session")) - .await - .context("could not end session")?; + let res = sqlx::query!( + "UPDATE user_sessions SET active = FALSE WHERE id = $1", + session.data, + ) + .execute(executor) + .instrument(info_span!("End session")) + .await + .context("could not end session")?; match res.rows_affected() { 1 => Ok(()), @@ -358,32 +346,12 @@ pub async fn end_session( } } -#[allow(dead_code)] -pub async fn lookup_user_by_id( - executor: impl Executor<'_, Database = Postgres>, - id: i64, -) -> anyhow::Result { - sqlx::query_as!( - User, - r#" - SELECT id, username - FROM users - WHERE id = $1 - "#, - id - ) - .fetch_one(executor) - .instrument(info_span!("Fetch user")) - .await - .context("could not fetch user") -} - pub async fn lookup_user_by_username( executor: impl Executor<'_, Database = Postgres>, username: &str, -) -> Result { - sqlx::query_as!( - User, +) -> Result, sqlx::Error> { + let res = sqlx::query_as!( + UserLookup, r#" SELECT id, username FROM users @@ -393,5 +361,11 @@ pub async fn lookup_user_by_username( ) .fetch_one(executor) .instrument(info_span!("Fetch user")) - .await + .await?; + + Ok(User { + data: res.id, + username: res.username, + sub: format!("fake-sub-{}", res.id), + }) } diff --git a/crates/core/src/templates/context.rs b/crates/core/src/templates/context.rs index a8050e54..c62bf961 100644 --- a/crates/core/src/templates/context.rs +++ b/crates/core/src/templates/context.rs @@ -14,16 +14,17 @@ //! Contexts used in templates +use mas_data_model::BrowserSession; use oauth2_types::errors::OAuth2Error; use serde::{ser::SerializeStruct, Serialize}; use url::Url; -use crate::{errors::ErroredForm, filters::CsrfToken, storage::SessionInfo}; +use crate::{errors::ErroredForm, filters::CsrfToken, storage::PostgresqlBackend}; /// Helper trait to construct context wrappers pub trait TemplateContext { /// Attach a user session to the template context - fn with_session(self, current_session: SessionInfo) -> WithSession + fn with_session(self, current_session: BrowserSession) -> WithSession where Self: Sized, { @@ -34,7 +35,10 @@ pub trait TemplateContext { } /// Attach an optional user session to the template context - fn maybe_with_session(self, current_session: Option) -> WithOptionalSession + fn maybe_with_session( + self, + current_session: Option>, + ) -> WithOptionalSession where Self: Sized, { @@ -91,7 +95,7 @@ impl TemplateContext for WithCsrf { /// Context with a user session in it #[derive(Serialize)] pub struct WithSession { - current_session: SessionInfo, + current_session: BrowserSession, #[serde(flatten)] inner: T, @@ -102,7 +106,7 @@ impl TemplateContext for WithSession { where Self: Sized, { - SessionInfo::samples() + BrowserSession::::samples() .into_iter() .flat_map(|session| { T::sample().into_iter().map(move |inner| WithSession { @@ -117,7 +121,7 @@ impl TemplateContext for WithSession { /// Context with an optional user session in it #[derive(Serialize)] pub struct WithOptionalSession { - current_session: Option, + current_session: Option>, #[serde(flatten)] inner: T, @@ -128,7 +132,7 @@ impl TemplateContext for WithOptionalSession { where Self: Sized, { - SessionInfo::samples() + BrowserSession::::samples() .into_iter() .map(Some) // Wrap all samples in an Option .chain(std::iter::once(None)) // Add the "None" option diff --git a/crates/core/src/templates/res/base.html b/crates/core/src/templates/res/base.html index bfa8c780..951a3f0b 100644 --- a/crates/core/src/templates/res/base.html +++ b/crates/core/src/templates/res/base.html @@ -34,7 +34,7 @@ limitations under the License.
{% if current_session %}
- Howdy {{ current_session.username }}! + Howdy {{ current_session.user.username }}!
diff --git a/crates/data-model/Cargo.toml b/crates/data-model/Cargo.toml new file mode 100644 index 00000000..75a2f866 --- /dev/null +++ b/crates/data-model/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "mas-data-model" +version = "0.1.0" +authors = ["Quentin Gliech "] +edition = "2018" +license = "Apache-2.0" + +[dependencies] +chrono = "0.4.19" +thiserror = "1.0.29" +serde = "1.0.130" + +oauth2-types = { path = "../oauth2-types" } diff --git a/crates/data-model/src/lib.rs b/crates/data-model/src/lib.rs new file mode 100644 index 00000000..09ec1b14 --- /dev/null +++ b/crates/data-model/src/lib.rs @@ -0,0 +1,134 @@ +// Copyright 2021 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use chrono::{DateTime, Duration, Utc}; +use oauth2_types::{pkce::CodeChallengeMethod, scope::Scope}; +use serde::Serialize; + +pub trait StorageBackend { + type UserData: Clone + std::fmt::Debug + PartialEq; + type AuthenticationData: Clone + std::fmt::Debug + PartialEq; + type BrowserSessionData: Clone + std::fmt::Debug + PartialEq; + type ClientData: Clone + std::fmt::Debug + PartialEq; + type SessionData: Clone + std::fmt::Debug + PartialEq; + type AuthorizationCodeData: Clone + std::fmt::Debug + PartialEq; + type AccessTokenData: Clone + std::fmt::Debug + PartialEq; +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +pub struct User { + #[serde(skip_serializing)] + pub data: T::UserData, + pub username: String, + pub sub: String, +} + +impl User +where + T::UserData: Default, +{ + pub fn samples() -> Vec { + vec![User { + data: Default::default(), + username: "john".to_string(), + sub: "123-456".to_string(), + }] + } +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +pub struct Authentication { + #[serde(skip_serializing)] + pub data: T::AuthenticationData, + pub created_at: DateTime, +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +pub struct BrowserSession { + #[serde(skip_serializing)] + pub data: T::BrowserSessionData, + pub user: User, + pub created_at: DateTime, + pub last_authentication: Option>, +} + +impl BrowserSession +where + T::BrowserSessionData: Default, + T::UserData: Default, +{ + pub fn samples() -> Vec { + User::::samples() + .into_iter() + .map(|user| BrowserSession { + data: Default::default(), + user, + created_at: Utc::now(), + last_authentication: None, + }) + .collect() + } +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +pub struct Client { + #[serde(skip_serializing)] + pub data: T::ClientData, + pub client_id: String, +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +pub struct Session { + #[serde(skip_serializing)] + pub data: T::SessionData, + pub browser_session: Option>, + pub client: Client, + pub scope: Scope, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct Pkce { + challenge_method: CodeChallengeMethod, + challenge: String, +} + +impl Pkce { + pub fn new(challenge_method: CodeChallengeMethod, challenge: String) -> Self { + Pkce { + challenge_method, + challenge, + } + } + + pub fn verify(&self, verifier: &str) -> bool { + self.challenge_method.verify(&self.challenge, verifier) + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct AuthorizationCode { + #[serde(skip_serializing)] + pub data: T::AuthorizationCodeData, + pub code: String, + pub pkce: Pkce, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AccessToken { + pub data: T::AccessTokenData, + pub jti: String, + pub token: String, + pub expires_after: Duration, + pub created_at: DateTime, +}