From 6a69ef8456c7992d3f7e7e52f89bf288ebd1ffcb Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Tue, 16 Nov 2021 19:16:52 +0100 Subject: [PATCH] Fix post-auth redirects & support max_age This also displays some context on login and reauth page about the next step --- crates/core/sqlx-data.json | 580 +++++++++--------- crates/core/src/filters/cookies.rs | 2 +- .../core/src/handlers/oauth2/authorization.rs | 41 +- crates/core/src/handlers/oauth2/token.rs | 11 +- crates/core/src/handlers/views/login.rs | 55 +- crates/core/src/handlers/views/reauth.rs | 49 +- crates/core/src/handlers/views/shared.rs | 25 +- .../src/storage/oauth2/authorization_grant.rs | 18 +- .../src/oauth2/authorization_grant.rs | 53 +- crates/oauth2-types/src/requests.rs | 10 +- crates/templates/src/context.rs | 74 ++- crates/templates/src/lib.rs | 5 +- crates/templates/src/res/login.html | 11 +- crates/templates/src/res/reauth.html | 11 +- 14 files changed, 581 insertions(+), 364 deletions(-) diff --git a/crates/core/sqlx-data.json b/crates/core/sqlx-data.json index f8250a46..28b7648e 100644 --- a/crates/core/sqlx-data.json +++ b/crates/core/sqlx-data.json @@ -54,8 +54,296 @@ ] } }, - "0cc63e00143cf94f63695be24acdcdffd8e8a3da50ea1ddf973a39bc34f861d4": { - "query": "\n SELECT\n og.id AS grant_id,\n og.created_at AS grant_created_at,\n og.cancelled_at AS grant_cancelled_at,\n og.fulfilled_at AS grant_fulfilled_at,\n og.exchanged_at AS grant_exchanged_at,\n og.scope AS grant_scope,\n og.state AS grant_state,\n og.redirect_uri AS grant_redirect_uri,\n og.response_mode AS grant_response_mode,\n og.nonce AS grant_nonce,\n og.max_age AS grant_max_age,\n og.acr_values AS grant_acr_values,\n og.client_id AS client_id,\n og.code AS grant_code,\n og.response_type_code AS grant_response_type_code,\n og.response_type_token AS grant_response_type_token,\n og.response_type_id_token AS grant_response_type_id_token,\n og.code_challenge AS grant_code_challenge,\n og.code_challenge_method AS grant_code_challenge_method,\n os.id AS \"session_id?\",\n us.id AS \"user_session_id?\",\n us.created_at AS \"user_session_created_at?\",\n u.id AS \"user_id?\",\n u.username AS \"user_username?\",\n usa.id AS \"user_session_last_authentication_id?\",\n usa.created_at AS \"user_session_last_authentication_created_at?\"\n FROM\n oauth2_authorization_grants og\n LEFT JOIN oauth2_sessions os\n ON os.id = og.oauth2_session_id\n LEFT JOIN user_sessions us\n ON us.id = os.user_session_id\n LEFT JOIN users u\n ON u.id = us.user_id\n LEFT JOIN user_session_authentications usa\n ON usa.session_id = us.id\n WHERE\n og.id = $1\n ", + "18d98b65c82142c28fb350f596c4439dbb04a55ff5b84586c1cb54601000d00d": { + "query": "\n SELECT\n og.id AS grant_id,\n og.created_at AS grant_created_at,\n og.cancelled_at AS grant_cancelled_at,\n og.fulfilled_at AS grant_fulfilled_at,\n og.exchanged_at AS grant_exchanged_at,\n og.scope AS grant_scope,\n og.state AS grant_state,\n og.redirect_uri AS grant_redirect_uri,\n og.response_mode AS grant_response_mode,\n og.nonce AS grant_nonce,\n og.max_age AS grant_max_age,\n og.acr_values AS grant_acr_values,\n og.client_id AS client_id,\n og.code AS grant_code,\n og.response_type_code AS grant_response_type_code,\n og.response_type_token AS grant_response_type_token,\n og.response_type_id_token AS grant_response_type_id_token,\n og.code_challenge AS grant_code_challenge,\n og.code_challenge_method AS grant_code_challenge_method,\n os.id AS \"session_id?\",\n us.id AS \"user_session_id?\",\n us.created_at AS \"user_session_created_at?\",\n u.id AS \"user_id?\",\n u.username AS \"user_username?\",\n usa.id AS \"user_session_last_authentication_id?\",\n usa.created_at AS \"user_session_last_authentication_created_at?\"\n FROM\n oauth2_authorization_grants og\n LEFT JOIN oauth2_sessions os\n ON os.id = og.oauth2_session_id\n LEFT JOIN user_sessions us\n ON us.id = os.user_session_id\n LEFT JOIN users u\n ON u.id = us.user_id\n LEFT JOIN user_session_authentications usa\n ON usa.session_id = us.id\n WHERE\n og.code = $1\n\n ORDER BY usa.created_at DESC\n LIMIT 1\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "grant_id", + "type_info": "Int8" + }, + { + "ordinal": 1, + "name": "grant_created_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 2, + "name": "grant_cancelled_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 3, + "name": "grant_fulfilled_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 4, + "name": "grant_exchanged_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 5, + "name": "grant_scope", + "type_info": "Text" + }, + { + "ordinal": 6, + "name": "grant_state", + "type_info": "Text" + }, + { + "ordinal": 7, + "name": "grant_redirect_uri", + "type_info": "Text" + }, + { + "ordinal": 8, + "name": "grant_response_mode", + "type_info": "Text" + }, + { + "ordinal": 9, + "name": "grant_nonce", + "type_info": "Text" + }, + { + "ordinal": 10, + "name": "grant_max_age", + "type_info": "Int4" + }, + { + "ordinal": 11, + "name": "grant_acr_values", + "type_info": "Text" + }, + { + "ordinal": 12, + "name": "client_id", + "type_info": "Text" + }, + { + "ordinal": 13, + "name": "grant_code", + "type_info": "Text" + }, + { + "ordinal": 14, + "name": "grant_response_type_code", + "type_info": "Bool" + }, + { + "ordinal": 15, + "name": "grant_response_type_token", + "type_info": "Bool" + }, + { + "ordinal": 16, + "name": "grant_response_type_id_token", + "type_info": "Bool" + }, + { + "ordinal": 17, + "name": "grant_code_challenge", + "type_info": "Text" + }, + { + "ordinal": 18, + "name": "grant_code_challenge_method", + "type_info": "Text" + }, + { + "ordinal": 19, + "name": "session_id?", + "type_info": "Int8" + }, + { + "ordinal": 20, + "name": "user_session_id?", + "type_info": "Int8" + }, + { + "ordinal": 21, + "name": "user_session_created_at?", + "type_info": "Timestamptz" + }, + { + "ordinal": 22, + "name": "user_id?", + "type_info": "Int8" + }, + { + "ordinal": 23, + "name": "user_username?", + "type_info": "Text" + }, + { + "ordinal": 24, + "name": "user_session_last_authentication_id?", + "type_info": "Int8" + }, + { + "ordinal": 25, + "name": "user_session_last_authentication_created_at?", + "type_info": "Timestamptz" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false, + false, + true, + true, + true, + false, + true, + false, + false, + true, + true, + true, + false, + true, + false, + false, + false, + true, + true, + false, + false, + false, + false, + false, + false, + false + ] + } + }, + "2dbccaf2fb557dd36598bf4d00941280535cc523ac3a481903ed825088901bce": { + "query": "\n SELECT\n at.id AS \"access_token_id\",\n at.token AS \"access_token\",\n at.expires_after AS \"access_token_expires_after\",\n at.created_at AS \"access_token_created_at\",\n os.id AS \"session_id!\",\n os.client_id AS \"client_id!\",\n os.scope AS \"scope!\",\n us.id AS \"user_session_id!\",\n us.created_at AS \"user_session_created_at!\",\n u.id AS \"user_id!\",\n u.username AS \"user_username!\",\n usa.id AS \"user_session_last_authentication_id?\",\n usa.created_at AS \"user_session_last_authentication_created_at?\"\n\n FROM oauth2_access_tokens at\n INNER JOIN oauth2_sessions os\n ON os.id = at.oauth2_session_id\n INNER JOIN user_sessions us\n ON us.id = os.user_session_id\n INNER JOIN users u\n ON u.id = us.user_id\n LEFT JOIN user_session_authentications usa\n ON usa.session_id = us.id\n\n WHERE at.token = $1\n AND at.created_at + (at.expires_after * INTERVAL '1 second') >= now()\n AND us.active\n\n ORDER BY usa.created_at DESC\n LIMIT 1\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "access_token_id", + "type_info": "Int8" + }, + { + "ordinal": 1, + "name": "access_token", + "type_info": "Text" + }, + { + "ordinal": 2, + "name": "access_token_expires_after", + "type_info": "Int4" + }, + { + "ordinal": 3, + "name": "access_token_created_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 4, + "name": "session_id!", + "type_info": "Int8" + }, + { + "ordinal": 5, + "name": "client_id!", + "type_info": "Text" + }, + { + "ordinal": 6, + "name": "scope!", + "type_info": "Text" + }, + { + "ordinal": 7, + "name": "user_session_id!", + "type_info": "Int8" + }, + { + "ordinal": 8, + "name": "user_session_created_at!", + "type_info": "Timestamptz" + }, + { + "ordinal": 9, + "name": "user_id!", + "type_info": "Int8" + }, + { + "ordinal": 10, + "name": "user_username!", + "type_info": "Text" + }, + { + "ordinal": 11, + "name": "user_session_last_authentication_id?", + "type_info": "Int8" + }, + { + "ordinal": 12, + "name": "user_session_last_authentication_created_at?", + "type_info": "Timestamptz" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false, + false, + false, + false, + false, + false, + false, + false, + false, + false, + false, + false, + false + ] + } + }, + "307fd9f71e7a94a0a0d9ce523ee9792e127485d0d12480c43f179dd9b75afbab": { + "query": "\n INSERT INTO user_sessions (user_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 + ] + } + }, + "3205a180aaa4661a016dada3a015ffd7a1019cd121e284f11e8120a6664e6288": { + "query": "\n SELECT\n og.id AS grant_id,\n og.created_at AS grant_created_at,\n og.cancelled_at AS grant_cancelled_at,\n og.fulfilled_at AS grant_fulfilled_at,\n og.exchanged_at AS grant_exchanged_at,\n og.scope AS grant_scope,\n og.state AS grant_state,\n og.redirect_uri AS grant_redirect_uri,\n og.response_mode AS grant_response_mode,\n og.nonce AS grant_nonce,\n og.max_age AS grant_max_age,\n og.acr_values AS grant_acr_values,\n og.client_id AS client_id,\n og.code AS grant_code,\n og.response_type_code AS grant_response_type_code,\n og.response_type_token AS grant_response_type_token,\n og.response_type_id_token AS grant_response_type_id_token,\n og.code_challenge AS grant_code_challenge,\n og.code_challenge_method AS grant_code_challenge_method,\n os.id AS \"session_id?\",\n us.id AS \"user_session_id?\",\n us.created_at AS \"user_session_created_at?\",\n u.id AS \"user_id?\",\n u.username AS \"user_username?\",\n usa.id AS \"user_session_last_authentication_id?\",\n usa.created_at AS \"user_session_last_authentication_created_at?\"\n FROM\n oauth2_authorization_grants og\n LEFT JOIN oauth2_sessions os\n ON os.id = og.oauth2_session_id\n LEFT JOIN user_sessions us\n ON us.id = os.user_session_id\n LEFT JOIN users u\n ON u.id = us.user_id\n LEFT JOIN user_session_authentications usa\n ON usa.session_id = us.id\n WHERE\n og.id = $1\n\n ORDER BY usa.created_at DESC\n LIMIT 1\n ", "describe": { "columns": [ { @@ -224,124 +512,6 @@ ] } }, - "2dbccaf2fb557dd36598bf4d00941280535cc523ac3a481903ed825088901bce": { - "query": "\n SELECT\n at.id AS \"access_token_id\",\n at.token AS \"access_token\",\n at.expires_after AS \"access_token_expires_after\",\n at.created_at AS \"access_token_created_at\",\n os.id AS \"session_id!\",\n os.client_id AS \"client_id!\",\n os.scope AS \"scope!\",\n us.id AS \"user_session_id!\",\n us.created_at AS \"user_session_created_at!\",\n u.id AS \"user_id!\",\n u.username AS \"user_username!\",\n usa.id AS \"user_session_last_authentication_id?\",\n usa.created_at AS \"user_session_last_authentication_created_at?\"\n\n FROM oauth2_access_tokens at\n INNER JOIN oauth2_sessions os\n ON os.id = at.oauth2_session_id\n INNER JOIN user_sessions us\n ON us.id = os.user_session_id\n INNER JOIN users u\n ON u.id = us.user_id\n LEFT JOIN user_session_authentications usa\n ON usa.session_id = us.id\n\n WHERE at.token = $1\n AND at.created_at + (at.expires_after * INTERVAL '1 second') >= now()\n AND us.active\n\n ORDER BY usa.created_at DESC\n LIMIT 1\n ", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "access_token_id", - "type_info": "Int8" - }, - { - "ordinal": 1, - "name": "access_token", - "type_info": "Text" - }, - { - "ordinal": 2, - "name": "access_token_expires_after", - "type_info": "Int4" - }, - { - "ordinal": 3, - "name": "access_token_created_at", - "type_info": "Timestamptz" - }, - { - "ordinal": 4, - "name": "session_id!", - "type_info": "Int8" - }, - { - "ordinal": 5, - "name": "client_id!", - "type_info": "Text" - }, - { - "ordinal": 6, - "name": "scope!", - "type_info": "Text" - }, - { - "ordinal": 7, - "name": "user_session_id!", - "type_info": "Int8" - }, - { - "ordinal": 8, - "name": "user_session_created_at!", - "type_info": "Timestamptz" - }, - { - "ordinal": 9, - "name": "user_id!", - "type_info": "Int8" - }, - { - "ordinal": 10, - "name": "user_username!", - "type_info": "Text" - }, - { - "ordinal": 11, - "name": "user_session_last_authentication_id?", - "type_info": "Int8" - }, - { - "ordinal": 12, - "name": "user_session_last_authentication_created_at?", - "type_info": "Timestamptz" - } - ], - "parameters": { - "Left": [ - "Text" - ] - }, - "nullable": [ - false, - false, - false, - false, - false, - false, - false, - false, - false, - false, - false, - false, - false - ] - } - }, - "307fd9f71e7a94a0a0d9ce523ee9792e127485d0d12480c43f179dd9b75afbab": { - "query": "\n INSERT INTO user_sessions (user_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 - ] - } - }, "38641231a3bff71252e8bc0ead3a033c9148762ea64d707642551c01a4c89b84": { "query": "\n INSERT INTO oauth2_authorization_grants\n (client_id, redirect_uri, scope, state, nonce, max_age,\n acr_values, response_mode, code_challenge, code_challenge_method,\n response_type_code, response_type_token, response_type_id_token,\n code)\n VALUES\n ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14)\n RETURNING id, created_at\n ", "describe": { @@ -612,176 +782,6 @@ "nullable": [] } }, - "8dde452a37c8faad20df68eb2b665202e0fb6b4ce805138e5f19d4e7eb0ce802": { - "query": "\n SELECT\n og.id AS grant_id,\n og.created_at AS grant_created_at,\n og.cancelled_at AS grant_cancelled_at,\n og.fulfilled_at AS grant_fulfilled_at,\n og.exchanged_at AS grant_exchanged_at,\n og.scope AS grant_scope,\n og.state AS grant_state,\n og.redirect_uri AS grant_redirect_uri,\n og.response_mode AS grant_response_mode,\n og.nonce AS grant_nonce,\n og.max_age AS grant_max_age,\n og.acr_values AS grant_acr_values,\n og.client_id AS client_id,\n og.code AS grant_code,\n og.response_type_code AS grant_response_type_code,\n og.response_type_token AS grant_response_type_token,\n og.response_type_id_token AS grant_response_type_id_token,\n og.code_challenge AS grant_code_challenge,\n og.code_challenge_method AS grant_code_challenge_method,\n os.id AS \"session_id?\",\n us.id AS \"user_session_id?\",\n us.created_at AS \"user_session_created_at?\",\n u.id AS \"user_id?\",\n u.username AS \"user_username?\",\n usa.id AS \"user_session_last_authentication_id?\",\n usa.created_at AS \"user_session_last_authentication_created_at?\"\n FROM\n oauth2_authorization_grants og\n LEFT JOIN oauth2_sessions os\n ON os.id = og.oauth2_session_id\n LEFT JOIN user_sessions us\n ON us.id = os.user_session_id\n LEFT JOIN users u\n ON u.id = us.user_id\n LEFT JOIN user_session_authentications usa\n ON usa.session_id = us.id\n WHERE\n og.code = $1\n ", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "grant_id", - "type_info": "Int8" - }, - { - "ordinal": 1, - "name": "grant_created_at", - "type_info": "Timestamptz" - }, - { - "ordinal": 2, - "name": "grant_cancelled_at", - "type_info": "Timestamptz" - }, - { - "ordinal": 3, - "name": "grant_fulfilled_at", - "type_info": "Timestamptz" - }, - { - "ordinal": 4, - "name": "grant_exchanged_at", - "type_info": "Timestamptz" - }, - { - "ordinal": 5, - "name": "grant_scope", - "type_info": "Text" - }, - { - "ordinal": 6, - "name": "grant_state", - "type_info": "Text" - }, - { - "ordinal": 7, - "name": "grant_redirect_uri", - "type_info": "Text" - }, - { - "ordinal": 8, - "name": "grant_response_mode", - "type_info": "Text" - }, - { - "ordinal": 9, - "name": "grant_nonce", - "type_info": "Text" - }, - { - "ordinal": 10, - "name": "grant_max_age", - "type_info": "Int4" - }, - { - "ordinal": 11, - "name": "grant_acr_values", - "type_info": "Text" - }, - { - "ordinal": 12, - "name": "client_id", - "type_info": "Text" - }, - { - "ordinal": 13, - "name": "grant_code", - "type_info": "Text" - }, - { - "ordinal": 14, - "name": "grant_response_type_code", - "type_info": "Bool" - }, - { - "ordinal": 15, - "name": "grant_response_type_token", - "type_info": "Bool" - }, - { - "ordinal": 16, - "name": "grant_response_type_id_token", - "type_info": "Bool" - }, - { - "ordinal": 17, - "name": "grant_code_challenge", - "type_info": "Text" - }, - { - "ordinal": 18, - "name": "grant_code_challenge_method", - "type_info": "Text" - }, - { - "ordinal": 19, - "name": "session_id?", - "type_info": "Int8" - }, - { - "ordinal": 20, - "name": "user_session_id?", - "type_info": "Int8" - }, - { - "ordinal": 21, - "name": "user_session_created_at?", - "type_info": "Timestamptz" - }, - { - "ordinal": 22, - "name": "user_id?", - "type_info": "Int8" - }, - { - "ordinal": 23, - "name": "user_username?", - "type_info": "Text" - }, - { - "ordinal": 24, - "name": "user_session_last_authentication_id?", - "type_info": "Int8" - }, - { - "ordinal": 25, - "name": "user_session_last_authentication_created_at?", - "type_info": "Timestamptz" - } - ], - "parameters": { - "Left": [ - "Text" - ] - }, - "nullable": [ - false, - false, - true, - true, - true, - false, - true, - false, - false, - true, - true, - true, - false, - true, - false, - false, - false, - true, - true, - false, - false, - false, - false, - false, - false, - false - ] - } - }, "a09dfe1019110f2ec6eba0d35bafa467ab4b7980dd8b556826f03863f8edb0ab": { "query": "UPDATE user_sessions SET active = FALSE WHERE id = $1", "describe": { diff --git a/crates/core/src/filters/cookies.rs b/crates/core/src/filters/cookies.rs index 70982071..f23d1bb6 100644 --- a/crates/core/src/filters/cookies.rs +++ b/crates/core/src/filters/cookies.rs @@ -186,7 +186,7 @@ impl EncryptedCookieSaver { // TODO: make those options customizable let value = Cookie::build(T::cookie_key(), encrypted) .http_only(true) - .same_site(SameSite::Strict) + .same_site(SameSite::Lax) .finish() .to_string(); diff --git a/crates/core/src/handlers/oauth2/authorization.rs b/crates/core/src/handlers/oauth2/authorization.rs index ab3c6673..b689752e 100644 --- a/crates/core/src/handlers/oauth2/authorization.rs +++ b/crates/core/src/handlers/oauth2/authorization.rs @@ -14,8 +14,7 @@ use std::{ collections::{HashMap, HashSet}, - convert::{TryFrom, TryInto}, - num::NonZeroU32, + convert::TryFrom, }; use chrono::Duration; @@ -40,7 +39,7 @@ use oauth2_types::{ use rand::{distributions::Alphanumeric, thread_rng, Rng}; use serde::{Deserialize, Serialize}; use serde_json::Value; -use sqlx::{PgPool, Postgres, Transaction}; +use sqlx::{PgExecutor, PgPool, Postgres, Transaction}; use url::Url; use warp::{ redirect::see_other, @@ -354,14 +353,6 @@ async fn get( None }; - let max_age: Option = params - .auth - .max_age - .as_ref() - .map(|d| d.num_seconds().try_into().and_then(|d: u32| d.try_into())) - .transpose() - .wrap_error()?; - let grant = new_authorization_grant( &mut txn, client.client_id.clone(), @@ -370,7 +361,7 @@ async fn get( code, params.auth.state, params.auth.nonce, - max_age, + params.auth.max_age, None, response_mode, response_type.contains(&ResponseType::Token), @@ -397,6 +388,14 @@ async fn get( #[derive(Serialize, Deserialize)] pub(crate) struct ContinueAuthorizationGrant { + #[serde( + with = "serde_with::rust::display_fromstr", + bound( + deserialize = "S::AuthorizationGrantData: std::str::FromStr, + ::Err: std::fmt::Display", + serialize = "S::AuthorizationGrantData: std::fmt::Display" + ) + )] data: S::AuthorizationGrantData, } @@ -407,7 +406,7 @@ impl ContinueAuthorizationGrant { pub fn build_uri(&self) -> anyhow::Result where - S::AuthorizationGrantData: Serialize, + S::AuthorizationGrantData: std::fmt::Display, { let qs = serde_urlencoded::to_string(self)?; let path_and_query = PathAndQuery::try_from(format!("/oauth2/authorize/step?{}", qs))?; @@ -420,6 +419,15 @@ impl ContinueAuthorizationGrant { } } +impl ContinueAuthorizationGrant { + pub async fn fetch_authorization_grant( + &self, + executor: impl PgExecutor<'_>, + ) -> anyhow::Result> { + get_grant_by_id(executor, self.data).await + } +} + async fn step( next: ContinueAuthorizationGrant, browser_session: BrowserSession, @@ -427,14 +435,17 @@ async fn step( ) -> Result { // TODO: we should check if the grant here was started by the browser doing that // request using a signed cookie - let grant = get_grant_by_id(&mut txn, next.data).await.wrap_error()?; + let grant = next + .fetch_authorization_grant(&mut txn) + .await + .wrap_error()?; if !matches!(grant.stage, AuthorizationGrantStage::Pending) { return Err(anyhow::anyhow!("authorization grant not pending")).wrap_error(); } let reply = match browser_session.last_authentication { - Some(Authentication { created_at, .. }) if created_at < grant.max_auth_time() => { + Some(Authentication { created_at, .. }) if created_at > grant.max_auth_time() => { let session = derive_session(&mut txn, &grant, browser_session) .await .wrap_error()?; diff --git a/crates/core/src/handlers/oauth2/token.rs b/crates/core/src/handlers/oauth2/token.rs index c779f39b..e84a4bcd 100644 --- a/crates/core/src/handlers/oauth2/token.rs +++ b/crates/core/src/handlers/oauth2/token.rs @@ -13,7 +13,7 @@ // limitations under the License. use anyhow::Context; -use chrono::Duration; +use chrono::{DateTime, Duration, Utc}; use data_encoding::BASE64URL_NOPAD; use headers::{CacheControl, Pragma}; use hyper::{Method, StatusCode}; @@ -29,7 +29,7 @@ use oauth2_types::{ }; use rand::thread_rng; use serde::Serialize; -use serde_with::skip_serializing_none; +use serde_with::{serde_as, skip_serializing_none}; use sha2::{Digest, Sha256}; use sqlx::{pool::PoolConnection, Acquire, PgPool, Postgres}; use tracing::debug; @@ -58,6 +58,7 @@ use crate::{ tokens::{AccessToken, RefreshToken}, }; +#[serde_as] #[skip_serializing_none] #[derive(Serialize, Debug)] struct CustomClaims { @@ -68,6 +69,8 @@ struct CustomClaims { #[serde(rename = "aud")] audiences: Vec, nonce: Option, + #[serde_as(as = "Option")] + auth_time: Option>, at_hash: String, c_hash: String, } @@ -253,6 +256,10 @@ async fn authorization_code_grant( subject: browser_session.user.sub.clone(), audiences: vec![client.client_id.clone()], nonce: authz_grant.nonce.clone(), + auth_time: browser_session + .last_authentication + .as_ref() + .map(|a| a.created_at), at_hash: hash(Sha256::new(), &access_token_str).wrap_error()?, c_hash: hash(Sha256::new(), &grant.code).wrap_error()?, }) diff --git a/crates/core/src/handlers/views/login.rs b/crates/core/src/handlers/views/login.rs index 7aae8e21..ffa25ffc 100644 --- a/crates/core/src/handlers/views/login.rs +++ b/crates/core/src/handlers/views/login.rs @@ -17,7 +17,7 @@ use std::convert::TryFrom; use hyper::http::uri::{Parts, PathAndQuery, Uri}; use mas_data_model::{errors::WrapFormError, BrowserSession, StorageBackend}; use mas_templates::{LoginContext, LoginFormField, TemplateContext, Templates}; -use serde::{Deserialize, Serialize}; +use serde::Deserialize; use sqlx::{pool::PoolConnection, PgPool, Postgres}; use warp::{reply::html, Filter, Rejection, Reply}; @@ -36,27 +36,27 @@ use crate::{ }; #[derive(Deserialize)] -#[serde( - rename_all = "snake_case", - bound = "::AuthorizationGrantData: Deserialize<'de>" -)] +#[serde(bound(deserialize = "S::AuthorizationGrantData: std::str::FromStr, + ::Err: std::fmt::Display"))] pub(crate) struct LoginRequest { - #[serde(flatten, skip_serializing_if = "Option::is_none")] - next: Option>, + #[serde(flatten)] + post_auth_action: Option>, } impl From> for LoginRequest { - fn from(next: PostAuthAction) -> Self { - Self { next: Some(next) } + fn from(post_auth_action: PostAuthAction) -> Self { + Self { + post_auth_action: Some(post_auth_action), + } } } impl LoginRequest { pub fn build_uri(&self) -> anyhow::Result where - S::AuthorizationGrantData: Serialize, + S::AuthorizationGrantData: std::fmt::Display, { - let path_and_query = if let Some(next) = &self.next { + let path_and_query = if let Some(next) = &self.post_auth_action { let qs = serde_urlencoded::to_string(next)?; PathAndQuery::try_from(format!("/login?{}", qs))? } else { @@ -72,10 +72,10 @@ impl LoginRequest { fn redirect(self) -> Result where - S::AuthorizationGrantData: Serialize, + S::AuthorizationGrantData: std::fmt::Display, { let uri = self - .next + .post_auth_action .as_ref() .map(PostAuthAction::build_uri) .transpose() @@ -99,6 +99,7 @@ pub(super) fn filter( ) -> impl Filter + Clone + Send + Sync + 'static { let get = warp::get() .and(with_templates(templates)) + .and(connection(pool)) .and(encrypted_cookie_saver(cookies_config)) .and(updated_csrf_token(cookies_config, csrf_config)) .and(warp::query()) @@ -119,6 +120,7 @@ pub(super) fn filter( async fn get( templates: Templates, + mut conn: PoolConnection, cookie_saver: EncryptedCookieSaver, csrf_token: CsrfToken, query: LoginRequest, @@ -127,7 +129,15 @@ async fn get( if maybe_session.is_some() { Ok(Box::new(query.redirect()?)) } else { - let ctx = LoginContext::default().with_csrf(csrf_token.form_value()); + let ctx = LoginContext::default(); + let ctx = match query.post_auth_action { + Some(next) => { + let next = next.load_context(&mut conn).await.wrap_error()?; + ctx.with_post_action(next) + } + None => ctx, + }; + let ctx = ctx.with_csrf(csrf_token.form_value()); let content = templates.render_login(&ctx)?; let reply = html(content); let reply = cookie_saver.save_encrypted(&csrf_token, reply)?; @@ -158,8 +168,9 @@ async fn post( LoginError::Authentication { .. } => e.on_field(LoginFormField::Password), LoginError::Other(_) => e.on_form(), }; - let ctx = - LoginContext::with_form_error(errored_form).with_csrf(csrf_token.form_value()); + let ctx = LoginContext::default() + .with_form_error(errored_form) + .with_csrf(csrf_token.form_value()); let content = templates.render_login(&ctx)?; let reply = html(content); let reply = cookie_saver.save_encrypted(&csrf_token, reply)?; @@ -167,3 +178,15 @@ async fn post( } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn deserialize_login_request() { + let res: Result, _> = + serde_urlencoded::from_str("next=continue_authorization_grant&data=13"); + res.unwrap().post_auth_action.unwrap(); + } +} diff --git a/crates/core/src/handlers/views/reauth.rs b/crates/core/src/handlers/views/reauth.rs index 562ec4e8..e8dd8d1f 100644 --- a/crates/core/src/handlers/views/reauth.rs +++ b/crates/core/src/handlers/views/reauth.rs @@ -16,9 +16,9 @@ use std::convert::TryFrom; use hyper::http::uri::{Parts, PathAndQuery}; use mas_data_model::{BrowserSession, StorageBackend}; -use mas_templates::{EmptyContext, TemplateContext, Templates}; -use serde::{Deserialize, Serialize}; -use sqlx::{PgPool, Postgres, Transaction}; +use mas_templates::{ReauthContext, TemplateContext, Templates}; +use serde::Deserialize; +use sqlx::{pool::PoolConnection, PgPool, Postgres, Transaction}; use warp::{hyper::Uri, reply::html, Filter, Rejection, Reply}; use super::PostAuthAction; @@ -28,34 +28,34 @@ use crate::{ filters::{ cookies::{encrypted_cookie_saver, EncryptedCookieSaver}, csrf::{protected_form, updated_csrf_token}, - database::transaction, + database::{connection, transaction}, session::session, with_templates, CsrfToken, }, storage::{user::authenticate_session, PostgresqlBackend}, }; #[derive(Deserialize)] -#[serde( - rename_all = "snake_case", - bound = "::AuthorizationGrantData: Deserialize<'de>" -)] +#[serde(bound(deserialize = "S::AuthorizationGrantData: std::str::FromStr, + ::Err: std::fmt::Display"))] pub(crate) struct ReauthRequest { - #[serde(flatten, skip_serializing_if = "Option::is_none")] - next: Option>, + #[serde(flatten)] + post_auth_action: Option>, } impl From> for ReauthRequest { - fn from(next: PostAuthAction) -> Self { - Self { next: Some(next) } + fn from(post_auth_action: PostAuthAction) -> Self { + Self { + post_auth_action: Some(post_auth_action), + } } } impl ReauthRequest { pub fn build_uri(&self) -> anyhow::Result where - S::AuthorizationGrantData: Serialize, + S::AuthorizationGrantData: std::fmt::Display, { - let path_and_query = if let Some(next) = &self.next { + let path_and_query = if let Some(next) = &self.post_auth_action { let qs = serde_urlencoded::to_string(next)?; PathAndQuery::try_from(format!("/reauth?{}", qs))? } else { @@ -71,10 +71,10 @@ impl ReauthRequest { fn redirect(self) -> Result where - S::AuthorizationGrantData: Serialize, + S::AuthorizationGrantData: std::fmt::Display, { let uri = self - .next + .post_auth_action .as_ref() .map(PostAuthAction::build_uri) .transpose() @@ -97,6 +97,7 @@ pub(super) fn filter( ) -> impl Filter + Clone + Send + Sync + 'static { let get = warp::get() .and(with_templates(templates)) + .and(connection(pool)) .and(encrypted_cookie_saver(cookies_config)) .and(updated_csrf_token(cookies_config, csrf_config)) .and(session(pool, cookies_config)) @@ -115,14 +116,21 @@ pub(super) fn filter( async fn get( templates: Templates, + mut conn: PoolConnection, cookie_saver: EncryptedCookieSaver, csrf_token: CsrfToken, session: BrowserSession, - _query: ReauthRequest, + query: ReauthRequest, ) -> Result { - let ctx = EmptyContext - .with_session(session) - .with_csrf(csrf_token.form_value()); + let ctx = ReauthContext::default(); + let ctx = match query.post_auth_action { + Some(next) => { + let next = next.load_context(&mut conn).await.wrap_error()?; + ctx.with_post_action(next) + } + None => ctx, + }; + let ctx = ctx.with_session(session).with_csrf(csrf_token.form_value()); let content = templates.render_reauth(&ctx)?; let reply = html(content); @@ -136,6 +144,7 @@ async fn post( form: ReauthForm, query: ReauthRequest, ) -> Result { + // TODO: recover from errors here authenticate_session(&mut txn, &session, form.password) .await .wrap_error()?; diff --git a/crates/core/src/handlers/views/shared.rs b/crates/core/src/handlers/views/shared.rs index 3f573bfe..be7b3b65 100644 --- a/crates/core/src/handlers/views/shared.rs +++ b/crates/core/src/handlers/views/shared.rs @@ -14,16 +14,20 @@ use hyper::Uri; use mas_data_model::StorageBackend; +use mas_templates::PostAuthContext; use serde::{Deserialize, Serialize}; +use sqlx::PgExecutor; use super::super::oauth2::ContinueAuthorizationGrant; +use crate::storage::PostgresqlBackend; #[derive(Deserialize, Serialize)] #[serde(rename_all = "snake_case", tag = "next")] pub(crate) enum PostAuthAction { #[serde(bound( - deserialize = "S::AuthorizationGrantData: Deserialize<'de>", - serialize = "S::AuthorizationGrantData: Serialize" + deserialize = "S::AuthorizationGrantData: std::str::FromStr, + ::Err: std::fmt::Display", + serialize = "S::AuthorizationGrantData: std::fmt::Display" ))] ContinueAuthorizationGrant(ContinueAuthorizationGrant), } @@ -31,7 +35,7 @@ pub(crate) enum PostAuthAction { impl PostAuthAction { pub fn build_uri(&self) -> anyhow::Result where - S::AuthorizationGrantData: Serialize, + S::AuthorizationGrantData: std::fmt::Display, { match self { PostAuthAction::ContinueAuthorizationGrant(c) => c.build_uri(), @@ -44,3 +48,18 @@ impl From> for PostAuthAction { + pub async fn load_context<'e>( + &self, + executor: impl PgExecutor<'e>, + ) -> anyhow::Result { + match self { + Self::ContinueAuthorizationGrant(c) => { + let grant = c.fetch_authorization_grant(executor).await?; + let grant = grant.into(); + Ok(PostAuthContext::ContinueAuthorizationGrant { grant }) + } + } + } +} diff --git a/crates/core/src/storage/oauth2/authorization_grant.rs b/crates/core/src/storage/oauth2/authorization_grant.rs index 56309957..ff54375a 100644 --- a/crates/core/src/storage/oauth2/authorization_grant.rs +++ b/crates/core/src/storage/oauth2/authorization_grant.rs @@ -121,7 +121,6 @@ struct GrantLookup { grant_redirect_uri: String, grant_response_mode: String, grant_nonce: Option, - #[allow(dead_code)] grant_max_age: Option, grant_acr_values: Option, grant_response_type_code: bool, @@ -274,6 +273,15 @@ impl TryInto> for GrantLookup { .parse() .map_err(|_e| DatabaseInconsistencyError)?; + let max_age = self + .grant_max_age + .map(|m: i32| m.try_into()) + .transpose() + .map_err(|_e| DatabaseInconsistencyError)? + .map(|m: u32| m.try_into()) + .transpose() + .map_err(|_e| DatabaseInconsistencyError)?; + Ok(AuthorizationGrant { data: self.grant_id, stage, @@ -283,7 +291,7 @@ impl TryInto> for GrantLookup { scope, state: self.grant_state, nonce: self.grant_nonce, - max_age: None, // TODO + max_age, // TODO response_mode, redirect_uri, created_at: self.grant_created_at, @@ -340,6 +348,9 @@ pub async fn get_grant_by_id( ON usa.session_id = us.id WHERE og.id = $1 + + ORDER BY usa.created_at DESC + LIMIT 1 "#, id, ) @@ -399,6 +410,9 @@ pub async fn lookup_grant_by_code( ON usa.session_id = us.id WHERE og.code = $1 + + ORDER BY usa.created_at DESC + LIMIT 1 "#, code, ) diff --git a/crates/data-model/src/oauth2/authorization_grant.rs b/crates/data-model/src/oauth2/authorization_grant.rs index f0734975..ddcc9f45 100644 --- a/crates/data-model/src/oauth2/authorization_grant.rs +++ b/crates/data-model/src/oauth2/authorization_grant.rs @@ -21,7 +21,7 @@ use thiserror::Error; use url::Url; use super::{client::Client, session::Session}; -use crate::traits::StorageBackend; +use crate::{traits::StorageBackend, StorageBackendMarker}; #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct Pkce { @@ -53,7 +53,7 @@ pub struct AuthorizationCode { pub struct InvalidTransitionError; #[derive(Debug, Clone, PartialEq, Serialize)] -#[serde(bound = "T: StorageBackend")] +#[serde(bound = "T: StorageBackend", tag = "stage", rename_all = "lowercase")] pub enum AuthorizationGrantStage { Pending, Fulfilled { @@ -117,6 +117,32 @@ impl AuthorizationGrantStage { } } +impl From> for AuthorizationGrantStage<()> { + fn from(s: AuthorizationGrantStage) -> Self { + use AuthorizationGrantStage::*; + match s { + Pending => Pending, + Fulfilled { + session, + fulfilled_at, + } => Fulfilled { + session: session.into(), + fulfilled_at, + }, + Exchanged { + session, + fulfilled_at, + exchanged_at, + } => Exchanged { + session: session.into(), + fulfilled_at, + exchanged_at, + }, + Cancelled { cancelled_at } => Cancelled { cancelled_at }, + } + } +} + #[derive(Debug, Clone, PartialEq, Serialize)] #[serde(bound = "T: StorageBackend")] pub struct AuthorizationGrant { @@ -138,9 +164,30 @@ pub struct AuthorizationGrant { pub created_at: DateTime, } +impl From> for AuthorizationGrant<()> { + fn from(g: AuthorizationGrant) -> Self { + AuthorizationGrant { + data: (), + stage: g.stage.into(), + code: g.code, + client: g.client.into(), + redirect_uri: g.redirect_uri, + scope: g.scope, + state: g.state, + nonce: g.nonce, + max_age: g.max_age, + acr_values: g.acr_values, + response_mode: g.response_mode, + response_type_token: g.response_type_token, + response_type_id_token: g.response_type_id_token, + created_at: g.created_at, + } + } +} + impl AuthorizationGrant { pub fn max_auth_time(&self) -> DateTime { let max_age: Option = self.max_age.map(|x| x.get().into()); - self.created_at + Duration::seconds(max_age.unwrap_or(3600 * 24 * 365)) + self.created_at - Duration::seconds(max_age.unwrap_or(3600 * 24 * 365)) } } diff --git a/crates/oauth2-types/src/requests.rs b/crates/oauth2-types/src/requests.rs index 594ea9cf..770b9675 100644 --- a/crates/oauth2-types/src/requests.rs +++ b/crates/oauth2-types/src/requests.rs @@ -12,15 +12,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{collections::HashSet, hash::Hash}; +use std::{collections::HashSet, hash::Hash, num::NonZeroU32}; use chrono::{DateTime, Duration, Utc}; use language_tags::LanguageTag; use parse_display::{Display, FromStr}; use serde::{Deserialize, Serialize}; use serde_with::{ - rust::StringWithSeparator, serde_as, skip_serializing_none, DurationSeconds, SpaceSeparator, - TimestampSeconds, + rust::StringWithSeparator, serde_as, skip_serializing_none, DisplayFromStr, DurationSeconds, + SpaceSeparator, TimestampSeconds, }; use url::Url; @@ -168,9 +168,9 @@ pub struct AuthorizationRequest { display: Option, - #[serde_as(as = "Option>")] #[serde(default)] - pub max_age: Option, + #[serde_as(as = "Option")] + pub max_age: Option, #[serde_as(as = "Option>")] #[serde(default)] diff --git a/crates/templates/src/context.rs b/crates/templates/src/context.rs index 1d521037..83c0bc7f 100644 --- a/crates/templates/src/context.rs +++ b/crates/templates/src/context.rs @@ -14,7 +14,7 @@ //! Contexts used in templates -use mas_data_model::{errors::ErroredForm, BrowserSession, StorageBackend}; +use mas_data_model::{errors::ErroredForm, AuthorizationGrant, BrowserSession, StorageBackend}; use oauth2_types::errors::OAuth2Error; use serde::{ser::SerializeStruct, Serialize}; use url::Url; @@ -210,10 +210,18 @@ pub enum LoginFormField { Password, } +/// Context used in login and reauth screens, for the post-auth action to do +#[derive(Serialize)] +#[serde(tag = "kind", rename_all = "snake_case")] +pub enum PostAuthContext { + ContinueAuthorizationGrant { grant: AuthorizationGrant<()> }, +} + /// Context used by the `login.html` template #[derive(Serialize)] pub struct LoginContext { form: ErroredForm, + next: Option, } impl TemplateContext for LoginContext { @@ -224,14 +232,23 @@ impl TemplateContext for LoginContext { // TODO: samples with errors vec![LoginContext { form: ErroredForm::default(), + next: None, }] } } impl LoginContext { #[must_use] - pub fn with_form_error(form: ErroredForm) -> Self { - Self { form } + pub fn with_form_error(self, form: ErroredForm) -> Self { + Self { form, ..self } + } + + #[must_use] + pub fn with_post_action(self, next: PostAuthContext) -> Self { + Self { + next: Some(next), + ..self + } } } @@ -239,10 +256,61 @@ impl Default for LoginContext { fn default() -> Self { Self { form: ErroredForm::new(), + next: None, } } } +#[derive(Serialize, Debug, Clone, Copy, Hash, PartialEq, Eq)] +#[serde(rename_all = "kebab-case")] +pub enum ReauthFormField { + Password, +} + +impl TemplateContext for ReauthContext { + fn sample() -> Vec + where + Self: Sized, + { + // TODO: samples with errors + vec![ReauthContext { + form: ErroredForm::default(), + next: None, + }] + } +} + +impl ReauthContext { + #[must_use] + pub fn with_form_error(self, form: ErroredForm) -> Self { + Self { form, ..self } + } + + #[must_use] + pub fn with_post_action(self, next: PostAuthContext) -> Self { + Self { + next: Some(next), + ..self + } + } +} + +impl Default for ReauthContext { + fn default() -> Self { + Self { + form: ErroredForm::new(), + next: None, + } + } +} + +/// Context used by the `reauth.html` template +#[derive(Serialize)] +pub struct ReauthContext { + form: ErroredForm, + next: Option, +} + /// Context used by the `form_post.html` template #[derive(Serialize)] pub struct FormPostContext { diff --git a/crates/templates/src/lib.rs b/crates/templates/src/lib.rs index 3ae30a0c..dbf2cf6f 100644 --- a/crates/templates/src/lib.rs +++ b/crates/templates/src/lib.rs @@ -41,7 +41,8 @@ mod macros; pub use self::context::{ EmptyContext, ErrorContext, FormPostContext, IndexContext, LoginContext, LoginFormField, - TemplateContext, WithCsrf, WithOptionalSession, WithSession, + PostAuthContext, ReauthContext, ReauthFormField, TemplateContext, WithCsrf, + WithOptionalSession, WithSession, }; /// Wrapper around [`tera::Tera`] helping rendering the various templates @@ -202,7 +203,7 @@ register_templates! { pub fn render_index(WithCsrf>) { "index.html" } /// Render the re-authentication form - pub fn render_reauth(WithCsrf>) { "reauth.html" } + pub fn render_reauth(WithCsrf>) { "reauth.html" } /// Render the form used by the form_post response mode pub fn render_form_post(FormPostContext) { "form_post.html" } diff --git a/crates/templates/src/res/login.html b/crates/templates/src/res/login.html index e6ac31b9..d7ce762f 100644 --- a/crates/templates/src/res/login.html +++ b/crates/templates/src/res/login.html @@ -20,7 +20,7 @@ limitations under the License.
-
+
{% if form.has_errors %}
@@ -64,6 +64,15 @@ limitations under the License.
+ + {% if next %} +
+
+

Next action:

+
{{ next | json_encode(pretty=True) | safe }}
+
+
+ {% endif %}
diff --git a/crates/templates/src/res/reauth.html b/crates/templates/src/res/reauth.html index b801393a..62cf6b22 100644 --- a/crates/templates/src/res/reauth.html +++ b/crates/templates/src/res/reauth.html @@ -36,7 +36,16 @@ limitations under the License.
-
{{ current_session | json_encode(pretty=True) | safe }}
+
+

Current session data:

+
{{ current_session | json_encode(pretty=True) | safe }}
+
+ {% if next %} +
+

Next action:

+
{{ next | json_encode(pretty=True) | safe }}
+
+ {% endif %}