From b1497604556279acaf666245ebd3bbc19ca51737 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Sun, 25 Jul 2021 14:40:42 +0200 Subject: [PATCH] properly save session with multiple auth This will help knowing when the user last authed, support MFA & other login types, support acr_values & max_time, etc. --- .../20210722072901_user_sessions.down.sql | 17 ++ .../20210722072901_user_sessions.up.sql | 35 +++ .../src/handlers/mod.rs | 14 +- .../src/handlers/views/login.rs | 5 +- .../src/handlers/views/logout.rs | 2 +- .../src/handlers/views/mod.rs | 1 + .../src/handlers/views/reauth.rs | 54 ++++ .../src/middlewares/errors.rs | 2 +- .../src/storage/client.rs | 21 +- .../src/storage/user.rs | 264 ++++++++++++++---- .../src/templates.rs | 8 +- .../templates/base.html | 4 +- .../templates/login.html | 4 +- .../templates/reauth.html | 45 +++ 14 files changed, 389 insertions(+), 87 deletions(-) create mode 100644 matrix-authentication-service/migrations/20210722072901_user_sessions.down.sql create mode 100644 matrix-authentication-service/migrations/20210722072901_user_sessions.up.sql create mode 100644 matrix-authentication-service/src/handlers/views/reauth.rs create mode 100644 matrix-authentication-service/templates/reauth.html diff --git a/matrix-authentication-service/migrations/20210722072901_user_sessions.down.sql b/matrix-authentication-service/migrations/20210722072901_user_sessions.down.sql new file mode 100644 index 000000000..3a33ff4a1 --- /dev/null +++ b/matrix-authentication-service/migrations/20210722072901_user_sessions.down.sql @@ -0,0 +1,17 @@ +-- 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. + +DROP TRIGGER set_timestamp ON user_sessions; +DROP TABLE user_session_authentications; +DROP TABLE user_sessions; diff --git a/matrix-authentication-service/migrations/20210722072901_user_sessions.up.sql b/matrix-authentication-service/migrations/20210722072901_user_sessions.up.sql new file mode 100644 index 000000000..b5d71ff23 --- /dev/null +++ b/matrix-authentication-service/migrations/20210722072901_user_sessions.up.sql @@ -0,0 +1,35 @@ +-- 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. + +-- A logged in session +CREATE TABLE user_sessions ( + "id" SERIAL PRIMARY KEY, + "user_id" INT NOT NULL REFERENCES users (id) ON DELETE CASCADE, + "active" BOOLEAN NOT NULL DEFAULT TRUE, + + "created_at" TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), + "updated_at" TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now() +); + +CREATE TRIGGER set_timestamp + BEFORE UPDATE ON user_sessions + FOR EACH ROW + EXECUTE PROCEDURE trigger_set_timestamp(); + +-- An authentication within a session +CREATE TABLE user_session_authentications ( + "id" SERIAL PRIMARY KEY, + "session_id" INT NOT NULL REFERENCES user_sessions (id) ON DELETE CASCADE, + "created_at" TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now() +); diff --git a/matrix-authentication-service/src/handlers/mod.rs b/matrix-authentication-service/src/handlers/mod.rs index fb52ec5a1..e9d9e4a99 100644 --- a/matrix-authentication-service/src/handlers/mod.rs +++ b/matrix-authentication-service/src/handlers/mod.rs @@ -20,6 +20,7 @@ use tide::{ security::{CorsMiddleware, Origin}, Middleware, Redirect, Server, }; +use tracing::error; use url::Url; use crate::{ @@ -67,8 +68,8 @@ async fn redirect_uri_from_params( None }; - let redirect_uri = client.resolve_redirect_uri(redirect_uri)?; - Ok(redirect_uri) + let redirect_uri = client.resolve_redirect_uri(&redirect_uri)?; + Ok(redirect_uri.clone()) } #[async_trait] @@ -83,6 +84,7 @@ impl Middleware for BrowserErrorHandler { let redirect_uri = redirect_uri_from_params(params, storage).await; let mut response = next.run(request).await; if let Some(err) = response.take_error() { + error!("{}", err); if let Ok(mut redirect_uri) = redirect_uri { redirect_uri .query_pairs_mut() @@ -128,11 +130,19 @@ pub fn install(app: &mut Server) { views.with(state.session_middleware()); views.with(state.csrf_middleware()); views.with(crate::middlewares::errors); + views.at("/").get(self::views::index::get); + views .at("/login") .get(self::views::login::get) .post(self::views::login::post); + + views + .at("/reauth") + .get(self::views::reauth::get) + .post(self::views::reauth::post); + views.at("/logout").post(self::views::logout::post); views diff --git a/matrix-authentication-service/src/handlers/views/login.rs b/matrix-authentication-service/src/handlers/views/login.rs index f2cdc31cb..79fce0d2b 100644 --- a/matrix-authentication-service/src/handlers/views/login.rs +++ b/matrix-authentication-service/src/handlers/views/login.rs @@ -27,6 +27,7 @@ pub async fn get(req: Request) -> tide::Result { let state = req.state(); let ctx = common_context(&req).await?; + // TODO: check if there is an existing session let content = state.templates().render("login.html", &ctx)?; let body = Response::builder(200) .body(content) @@ -40,13 +41,13 @@ pub async fn post(mut req: Request) -> tide::Result { let form = form.verify_csrf(&req)?; let state = req.state(); - let user = state + let session_info = state .storage() .login(&form.username, &form.password) .await?; let session = req.session_mut(); - session.insert("current_user", user.key())?; + session.insert("current_session", session_info.key())?; Ok(Redirect::new("/").into()) } diff --git a/matrix-authentication-service/src/handlers/views/logout.rs b/matrix-authentication-service/src/handlers/views/logout.rs index ad1fbf518..5c2180d9e 100644 --- a/matrix-authentication-service/src/handlers/views/logout.rs +++ b/matrix-authentication-service/src/handlers/views/logout.rs @@ -21,7 +21,7 @@ pub async fn post(mut req: Request) -> tide::Result { form.verify_csrf(&req)?; let session = req.session_mut(); - session.remove("current_user"); + session.remove("current_session"); Ok(Redirect::new("/").into()) } diff --git a/matrix-authentication-service/src/handlers/views/mod.rs b/matrix-authentication-service/src/handlers/views/mod.rs index 6696aae0a..2631a0dd7 100644 --- a/matrix-authentication-service/src/handlers/views/mod.rs +++ b/matrix-authentication-service/src/handlers/views/mod.rs @@ -15,3 +15,4 @@ pub(super) mod index; pub(super) mod login; pub(super) mod logout; +pub(super) mod reauth; diff --git a/matrix-authentication-service/src/handlers/views/reauth.rs b/matrix-authentication-service/src/handlers/views/reauth.rs new file mode 100644 index 000000000..32b0ba1b6 --- /dev/null +++ b/matrix-authentication-service/src/handlers/views/reauth.rs @@ -0,0 +1,54 @@ +// 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 serde::Deserialize; +use tide::{Redirect, Request, Response}; + +use crate::{csrf::CsrfForm, state::State, templates::common_context}; + +#[derive(Deserialize)] +struct ReauthForm { + password: String, +} + +pub async fn get(req: Request) -> tide::Result { + let state = req.state(); + let ctx = common_context(&req).await?; + + // TODO: check if there is an existing session + let content = state.templates().render("reauth.html", &ctx)?; + let body = Response::builder(200) + .body(content) + .content_type("text/html") + .into(); + Ok(body) +} + +pub async fn post(mut req: Request) -> tide::Result { + let form: CsrfForm = req.body_form().await?; + let form = form.verify_csrf(&req)?; + let state = req.state(); + let session = req.session(); + + let session_id = session + .get("current_session") + .ok_or_else(|| anyhow::anyhow!("could not find existing session"))?; + + let _session = state + .storage() + .lookup_and_reauth_session(session_id, &form.password) + .await?; + + Ok(Redirect::new("/").into()) +} diff --git a/matrix-authentication-service/src/middlewares/errors.rs b/matrix-authentication-service/src/middlewares/errors.rs index 47666403a..728f700c2 100644 --- a/matrix-authentication-service/src/middlewares/errors.rs +++ b/matrix-authentication-service/src/middlewares/errors.rs @@ -153,7 +153,7 @@ pub fn middleware<'a>( // way with a backtrace if we have one let details = response.take_error().map(|err| { format!( - "{}{}", + "{:?}{}", err, err.backtrace() .map(|bt| format!("\nBacktrace:\n{}", bt.to_string())) diff --git a/matrix-authentication-service/src/storage/client.rs b/matrix-authentication-service/src/storage/client.rs index 5cbc66e4c..986ceeae4 100644 --- a/matrix-authentication-service/src/storage/client.rs +++ b/matrix-authentication-service/src/storage/client.rs @@ -14,14 +14,15 @@ use std::collections::HashSet; +use serde::Serialize; use thiserror::Error; use url::Url; use crate::config::OAuth2ClientConfig; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Serialize)] pub struct Client { - client_id: String, + pub client_id: String, redirect_uris: Option>, } @@ -34,17 +35,15 @@ pub struct ClientLookupError; pub struct InvalidRedirectUriError; impl Client { - pub fn resolve_redirect_uri( - &self, - suggested_uri: Option, - ) -> Result { + pub fn resolve_redirect_uri<'a>( + &'a self, + suggested_uri: &'a Option, + ) -> Result<&'a Url, InvalidRedirectUriError> { match (suggested_uri, &self.redirect_uris) { (None, None) => Err(InvalidRedirectUriError), - (None, Some(redirect_uris)) => redirect_uris - .iter() - .next() - .cloned() - .ok_or(InvalidRedirectUriError), + (None, Some(redirect_uris)) => { + redirect_uris.iter().next().ok_or(InvalidRedirectUriError) + } (Some(suggested_uri), None) => Ok(suggested_uri), (Some(suggested_uri), Some(redirect_uris)) => { if redirect_uris.contains(&suggested_uri) { diff --git a/matrix-authentication-service/src/storage/user.rs b/matrix-authentication-service/src/storage/user.rs index ee7b5928f..11fd3737b 100644 --- a/matrix-authentication-service/src/storage/user.rs +++ b/matrix-authentication-service/src/storage/user.rs @@ -12,91 +12,231 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::borrow::BorrowMut; + use anyhow::Context; use argon2::Argon2; -use password_hash::{PasswordHash, SaltString}; +use chrono::{DateTime, Utc}; +use password_hash::{PasswordHash, PasswordHasher, SaltString}; use rand::rngs::OsRng; use serde::Serialize; -use sqlx::{FromRow, PgPool}; +use sqlx::{Executor, FromRow, PgPool, Postgres, Transaction}; use tracing::{info_span, Instrument}; #[derive(Serialize, Debug, Clone, FromRow)] pub struct User { - id: i32, - username: String, + pub id: i32, + pub username: String, } -impl User { +#[derive(Serialize, Debug, Clone, FromRow)] +pub struct SessionInfo { + id: i32, + user_id: i32, + username: String, + active: bool, + created_at: DateTime, + last_authd_at: Option>, +} + +impl SessionInfo { pub fn key(&self) -> i32 { self.id } } impl super::Storage { - pub async fn login(&self, username: &str, password: &str) -> anyhow::Result { - let mut conn = self.pool.acquire().await?; - - let (id, username, hashed_password): (i32, String, String) = sqlx::query_as( - r#" - SELECT id, username, hashed_password - FROM users - WHERE username = $1 - "#, - ) - .bind(&username) - .fetch_one(&mut conn) - .instrument(info_span!("Fetch user")) - .await - .context("could not find user")?; - - let context = Argon2::default(); - let hasher = PasswordHash::new(&hashed_password)?; - hasher.verify_password(&[&context], &password)?; - - Ok(User { id, username }) + pub async fn login(&self, username: &str, password: &str) -> anyhow::Result { + let mut txn = self.pool.begin().await?; + let user = lookup_user_by_username(&mut txn, username).await?; + let mut session = start_session(&mut txn, user).await?; + session.last_authd_at = Some(authenticate_session(&mut txn, session.id, password).await?); + txn.commit().await?; + Ok(session) } pub async fn register_user(&self, username: &str, password: &str) -> anyhow::Result { - let context = Argon2::default(); - let salt = SaltString::generate(&mut OsRng); - let hashed_password = PasswordHash::generate(context, password, salt.as_str())?; - let mut conn = self.pool.acquire().await?; - - let result: (i32,) = sqlx::query_as( - r#" - INSERT INTO users (username, hashed_password) - VALUES ($1, $2) - RETURNING id - "#, - ) - .bind(&username) - .bind(&hashed_password.to_string()) - .fetch_one(&mut conn) - .instrument(info_span!("Register user")) - .await - .context("could not insert user")?; - - Ok(User { - id: result.0, - username: username.to_string(), - }) + let hasher = Argon2::default(); + register_user(&mut conn, hasher, username, password).await } - pub async fn lookup_user(&self, id: i32) -> anyhow::Result { + pub async fn lookup_session(&self, id: i32) -> anyhow::Result { let mut conn = self.pool.acquire().await?; + lookup_session(&mut conn, id).await + } - sqlx::query_as( - r#" - SELECT id, username - FROM users - WHERE id = $1 - "#, - ) - .bind(&id) - .fetch_one(&mut conn) - .instrument(info_span!("Fetch user")) - .await - .context("could not fetch user") + pub async fn lookup_and_reauth_session( + &self, + session_id: i32, + password: &str, + ) -> anyhow::Result { + let mut txn = self.pool.begin().await?; + let mut session = lookup_session(&mut txn, session_id).await?; + session.last_authd_at = Some(authenticate_session(&mut txn, session.id, password).await?); + txn.commit().await?; + Ok(session) } } + +pub async fn lookup_session( + executor: impl Executor<'_, Database = Postgres>, + id: i32, +) -> anyhow::Result { + sqlx::query_as( + 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 + "#, + ) + .bind(id) + .fetch_one(executor) + .await + .context("could not fetch session") +} + +pub async fn start_session( + executor: impl Executor<'_, Database = Postgres>, + user: User, +) -> anyhow::Result { + let (id, created_at): (i32, DateTime) = sqlx::query_as( + r#" + INSERT INTO user_sessions (user_id) + VALUES ($1) + RETURNING id, created_at + "#, + ) + .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, + }) +} + +pub async fn authenticate_session( + txn: &mut Transaction<'_, Postgres>, + session_id: i32, + password: &str, +) -> anyhow::Result> { + // First, fetch the hashed password from the user associated with that session + let hashed_password: String = sqlx::query_scalar( + r#" + SELECT u.hashed_password + FROM user_sessions s + INNER JOIN users u + ON u.id = s.user_id + WHERE s.id = $1 + "#, + ) + .bind(session_id) + .fetch_one(txn.borrow_mut()) + .await + .context("could not fetch user password hash")?; + + // TODO: pass verifiers list as parameter + let context = Argon2::default(); + let hasher = PasswordHash::new(&hashed_password)?; + hasher.verify_password(&[&context], &password)?; + + // That went well, let's insert the auth info + let created_at: DateTime = sqlx::query_scalar( + r#" + INSERT INTO user_session_authentications (session_id) + VALUES ($1) + RETURNING created_at + "#, + ) + .bind(session_id) + .fetch_one(txn.borrow_mut()) + .await + .context("could not save session auth")?; + + Ok(created_at) +} + +pub async fn register_user( + executor: impl Executor<'_, Database = Postgres>, + phf: impl PasswordHasher, + username: &str, + password: &str, +) -> anyhow::Result { + let salt = SaltString::generate(&mut OsRng); + let hashed_password = PasswordHash::generate(phf, password, salt.as_str())?; + + let id: i32 = sqlx::query_scalar( + r#" + INSERT INTO users (username, hashed_password) + VALUES ($1, $2) + RETURNING id + "#, + ) + .bind(&username) + .bind(&hashed_password.to_string()) + .fetch_one(executor) + .instrument(info_span!("Register user")) + .await + .context("could not insert user")?; + + Ok(User { + id, + username: username.to_string(), + }) +} + +#[allow(dead_code)] +pub async fn lookup_user_by_id( + executor: impl Executor<'_, Database = Postgres>, + id: i32, +) -> anyhow::Result { + sqlx::query_as( + r#" + SELECT id, username + FROM users + WHERE id = $1 + "#, + ) + .bind(&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, +) -> anyhow::Result { + sqlx::query_as( + r#" + SELECT id, username + FROM users + WHERE username = $1 + "#, + ) + .bind(&username) + .fetch_one(executor) + .instrument(info_span!("Fetch user")) + .await + .context("could not fetch user") +} diff --git a/matrix-authentication-service/src/templates.rs b/matrix-authentication-service/src/templates.rs index 0e475e097..ce604f204 100644 --- a/matrix-authentication-service/src/templates.rs +++ b/matrix-authentication-service/src/templates.rs @@ -31,10 +31,10 @@ pub async fn common_context(req: &Request) -> Result = session.get("current_user"); - if let Some(user) = user { - let user = state.storage().lookup_user(user).await?; - ctx.insert("current_user", &user); + let session_id: Option<_> = session.get("current_session"); + if let Some(session_id) = session_id { + let user = state.storage().lookup_session(session_id).await?; + ctx.insert("current_session", &user); } let token: Option<&CsrfToken> = req.ext(); diff --git a/matrix-authentication-service/templates/base.html b/matrix-authentication-service/templates/base.html index 0cd3d5a79..e2651a155 100644 --- a/matrix-authentication-service/templates/base.html +++ b/matrix-authentication-service/templates/base.html @@ -32,9 +32,9 @@ limitations under the License.
- {% if current_user %} + {% if current_session %}
- Howdy {{ current_user.username }}! + Howdy {{ current_session.username }}!
diff --git a/matrix-authentication-service/templates/login.html b/matrix-authentication-service/templates/login.html index 2d6da4a5a..9d6ffd431 100644 --- a/matrix-authentication-service/templates/login.html +++ b/matrix-authentication-service/templates/login.html @@ -40,8 +40,8 @@ limitations under the License.
-
- + +
diff --git a/matrix-authentication-service/templates/reauth.html b/matrix-authentication-service/templates/reauth.html new file mode 100644 index 000000000..b801393af --- /dev/null +++ b/matrix-authentication-service/templates/reauth.html @@ -0,0 +1,45 @@ +{# +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. +#} + +{% extends "base.html" %} + +{% block content %} +
+
+
+
+
+ +
+ +
+ +
+
+ +
+ +
+
+
+
+ 
{{ current_session | json_encode(pretty=True) | safe }}
+
+
+
+
+{% endblock content %} +