From 5a0f27271e7df1d109ee2e9739a808c1f4a870b5 Mon Sep 17 00:00:00 2001 From: Pierre HUBERT Date: Thu, 28 Mar 2024 18:06:28 +0100 Subject: [PATCH 1/4] Update README --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index 96a08c4..16b201e 100644 --- a/README.md +++ b/README.md @@ -13,10 +13,15 @@ BasicOIDC operates without any database, just with three files : ## Configuration You can configure a list of clients (Relying Parties) in a `clients.yaml` file with the following syntax : ```yaml + # Client ID - id: gitea + # Client name name: Gitea + # Client description description: Git with a cup of tea + # Client secret. Specify this value to use authorization code flow, remove it for implicit authentication flow secret: TOP_SECRET + # The URL where user shall be redirected after authentication redirect_uri: https://mygit.mywebsite.com/ # If you want new accounts to be granted access to this client by default default: true @@ -32,6 +37,7 @@ In order to run BasicOIDC for development, you will need to create a least an em ## Features * [x] `authorization_code` flow +* [x] `implicit` flow * [x] Client authentication using secrets * [x] Bruteforce protection * [x] 2 factors authentication -- 2.45.2 From 719416eea9ee2ceaaf01a7e7fd1aa7d6ff48ef7f Mon Sep 17 00:00:00 2001 From: Pierre HUBERT Date: Thu, 28 Mar 2024 18:27:06 +0100 Subject: [PATCH 2/4] Make state optional --- src/controllers/openid_controller.rs | 23 +++++++++++++++-------- src/data/client.rs | 20 ++++++++++++++++++-- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/controllers/openid_controller.rs b/src/controllers/openid_controller.rs index 025f6d1..4c0d4af 100644 --- a/src/controllers/openid_controller.rs +++ b/src/controllers/openid_controller.rs @@ -97,7 +97,7 @@ pub struct AuthorizeQuery { redirect_uri: String, /// RECOMMENDED. Opaque value used to maintain state between the request and the callback. Typically, Cross-Site Request Forgery (CSRF, XSRF) mitigation is done by cryptographically binding the value of this parameter with a browser cookie. - state: String, + state: Option, /// OPTIONAL. String value used to associate a Client session with an ID Token, and to mitigate replay attacks. The value is passed through unmodified from the Authentication Request to the ID Token. Sufficient entropy MUST be present in the nonce values used to prevent attackers from guessing values. nonce: Option, @@ -118,11 +118,14 @@ fn error_redirect(query: &AuthorizeQuery, error: &str, description: &str) -> Htt .append_header(( "Location", format!( - "{}?error={}?error_description={}&state={}", + "{}?error={}?error_description={}{}", query.redirect_uri, urlencoding::encode(error), urlencoding::encode(description), - urlencoding::encode(&query.state) + match &query.state { + Some(s) => format!("&state={}", urlencoding::encode(s)), + None => "".to_string(), + } ), )) .finish() @@ -167,8 +170,8 @@ pub async fn authorize( ); } - if query.state.is_empty() { - return error_redirect(&query, "invalid_request", "State is empty!"); + if query.state.as_ref().map(String::is_empty).unwrap_or(false) { + return error_redirect(&query, "invalid_request", "State is specified but empty!"); } let code_challenge = match query.0.code_challenge.clone() { @@ -226,9 +229,12 @@ pub async fn authorize( .append_header(( "Location", format!( - "{}?state={}&session_state={}&code={}", + "{}?{}session_state={}&code={}", session.redirect_uri, - urlencoding::encode(&query.0.state), + match &query.0.state { + Some(state) => format!("state={}&", urlencoding::encode(state)), + None => "".to_string(), + }, urlencoding::encode(&session.session_id.0), urlencoding::encode(&session.authorization_code) ), @@ -344,7 +350,8 @@ pub async fn token( .find_by_id(&client_id) .ok_or_else(|| ErrorUnauthorized("Client not found"))?; - if !client.secret.eq(&client_secret) { + // Retrieving token requires the client to have a defined secret + if client.secret != Some(client_secret) { return Ok(error_response( &query, "invalid_request", diff --git a/src/data/client.rs b/src/data/client.rs index 1cdf75d..daf72bd 100644 --- a/src/data/client.rs +++ b/src/data/client.rs @@ -4,6 +4,11 @@ use crate::utils::string_utils::apply_env_vars; #[derive(Clone, Debug, serde::Serialize, serde::Deserialize, Eq, PartialEq)] pub struct ClientID(pub String); +pub enum AuthenticationFlow { + AuthorizationCode, + Implicit, +} + #[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] pub struct Client { /// The ID of the client @@ -16,7 +21,8 @@ pub struct Client { pub description: String, /// The secret used by the client to retrieve authenticated users information - pub secret: String, + /// This value is absent if implicit authentication flow is used + pub secret: Option, /// The URI where the users should be redirected once authenticated pub redirect_uri: String, @@ -42,6 +48,16 @@ impl PartialEq for Client { impl Eq for Client {} +impl Client { + /// Get the client authentication flow + pub fn auth_flow(&self) -> AuthenticationFlow { + match self.secret { + None => AuthenticationFlow::Implicit, + Some(_) => AuthenticationFlow::AuthorizationCode, + } + } +} + pub type ClientManager = EntityManager; impl EntityManager { @@ -66,7 +82,7 @@ impl EntityManager { c.id = ClientID(apply_env_vars(&c.id.0)); c.name = apply_env_vars(&c.name); c.description = apply_env_vars(&c.description); - c.secret = apply_env_vars(&c.secret); + c.secret = c.secret.as_deref().map(apply_env_vars); c.redirect_uri = apply_env_vars(&c.redirect_uri); } } -- 2.45.2 From 24c7072c0c877d8e666c9465e1d69d9f1cee9404 Mon Sep 17 00:00:00 2001 From: Pierre HUBERT Date: Thu, 28 Mar 2024 18:58:51 +0100 Subject: [PATCH 3/4] Refactor code to prepare support of implicit flow --- src/controllers/openid_controller.rs | 108 +++++++++++++++------------ src/data/client.rs | 1 + 2 files changed, 63 insertions(+), 46 deletions(-) diff --git a/src/controllers/openid_controller.rs b/src/controllers/openid_controller.rs index 4c0d4af..edaa063 100644 --- a/src/controllers/openid_controller.rs +++ b/src/controllers/openid_controller.rs @@ -16,7 +16,7 @@ use crate::constants::*; use crate::controllers::base_controller::{build_fatal_error_page, redirect_user}; use crate::data::action_logger::{Action, ActionLogger}; use crate::data::app_config::AppConfig; -use crate::data::client::{ClientID, ClientManager}; +use crate::data::client::{AuthenticationFlow, ClientID, ClientManager}; use crate::data::code_challenge::CodeChallenge; use crate::data::current_user::CurrentUser; use crate::data::id_token::IdToken; @@ -162,14 +162,6 @@ pub async fn authorize( return error_redirect(&query, "invalid_request", "openid scope missing!"); } - if !query.response_type.eq("code") { - return error_redirect( - &query, - "invalid_request", - "Only code response type is supported!", - ); - } - if query.state.as_ref().map(String::is_empty).unwrap_or(false) { return error_redirect(&query, "invalid_request", "State is specified but empty!"); } @@ -201,45 +193,69 @@ pub async fn authorize( ); } - // Save all authentication information in memory - let session = Session { - session_id: SessionID(rand_str(OPEN_ID_SESSION_LEN)), - client: client.id.clone(), - user: user.uid.clone(), - auth_time: SessionIdentity(Some(&id)).auth_time(), - redirect_uri, - authorization_code: rand_str(OPEN_ID_AUTHORIZATION_CODE_LEN), - authorization_code_expire_at: time() + OPEN_ID_AUTHORIZATION_CODE_TIMEOUT, - access_token: None, - access_token_expire_at: time() + OPEN_ID_ACCESS_TOKEN_TIMEOUT, - refresh_token: "".to_string(), - refresh_token_expire_at: 0, - nonce: query.0.nonce, - code_challenge, - }; - sessions - .send(openid_sessions_actor::PushNewSession(session.clone())) - .await - .unwrap(); + // Check that requested authorization flow is supported + if query.response_type != "code" && query.response_type != "id_token" { + return error_redirect(&query, "invalid_request", "Unsupported authorization flow!"); + } - log::trace!("New OpenID session: {:#?}", session); - logger.log(Action::NewOpenIDSession { client: &client }); + match (client.auth_flow(), query.response_type.as_str()) { + (AuthenticationFlow::AuthorizationCode, "code") => { + // Save all authentication information in memory + let session = Session { + session_id: SessionID(rand_str(OPEN_ID_SESSION_LEN)), + client: client.id.clone(), + user: user.uid.clone(), + auth_time: SessionIdentity(Some(&id)).auth_time(), + redirect_uri, + authorization_code: rand_str(OPEN_ID_AUTHORIZATION_CODE_LEN), + authorization_code_expire_at: time() + OPEN_ID_AUTHORIZATION_CODE_TIMEOUT, + access_token: None, + access_token_expire_at: time() + OPEN_ID_ACCESS_TOKEN_TIMEOUT, + refresh_token: "".to_string(), + refresh_token_expire_at: 0, + nonce: query.0.nonce, + code_challenge, + }; + sessions + .send(openid_sessions_actor::PushNewSession(session.clone())) + .await + .unwrap(); - HttpResponse::Found() - .append_header(( - "Location", - format!( - "{}?{}session_state={}&code={}", - session.redirect_uri, - match &query.0.state { - Some(state) => format!("state={}&", urlencoding::encode(state)), - None => "".to_string(), - }, - urlencoding::encode(&session.session_id.0), - urlencoding::encode(&session.authorization_code) - ), - )) - .finish() + log::trace!("New OpenID session: {:#?}", session); + logger.log(Action::NewOpenIDSession { client: &client }); + + HttpResponse::Found() + .append_header(( + "Location", + format!( + "{}?{}session_state={}&code={}", + session.redirect_uri, + match &query.0.state { + Some(state) => format!("state={}&", urlencoding::encode(state)), + None => "".to_string(), + }, + urlencoding::encode(&session.session_id.0), + urlencoding::encode(&session.authorization_code) + ), + )) + .finish() + } + + //(AuthenticationFlow::Implicit, "id_token") => {} + (flow, code) => { + log::warn!( + "For client {:?}, configured with flow {:?}, made request with code {}", + client.id, + flow, + code + ); + error_redirect( + &query, + "invalid_request", + "Requested authentication flow is unsupported / not configured for this client!", + ) + } + } } #[derive(serde::Serialize)] diff --git a/src/data/client.rs b/src/data/client.rs index daf72bd..fdc2538 100644 --- a/src/data/client.rs +++ b/src/data/client.rs @@ -4,6 +4,7 @@ use crate::utils::string_utils::apply_env_vars; #[derive(Clone, Debug, serde::Serialize, serde::Deserialize, Eq, PartialEq)] pub struct ClientID(pub String); +#[derive(Debug, Copy, Clone, Eq, PartialEq)] pub enum AuthenticationFlow { AuthorizationCode, Implicit, -- 2.45.2 From a600d4af71e289dd0543776b54fae268959d0cbe Mon Sep 17 00:00:00 2001 From: Pierre HUBERT Date: Thu, 28 Mar 2024 21:57:20 +0100 Subject: [PATCH 4/4] Start to issue `id_token` --- src/constants.rs | 1 + src/controllers/openid_controller.rs | 81 ++++++++++++++++++++++------ src/data/action_logger.rs | 5 +- src/data/id_token.rs | 2 +- 4 files changed, 71 insertions(+), 18 deletions(-) diff --git a/src/constants.rs b/src/constants.rs index 0b1aec4..beb970d 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -69,6 +69,7 @@ pub const OPEN_ID_AUTHORIZATION_CODE_LEN: usize = 120; pub const OPEN_ID_AUTHORIZATION_CODE_TIMEOUT: u64 = 300; pub const OPEN_ID_ACCESS_TOKEN_LEN: usize = 50; pub const OPEN_ID_ACCESS_TOKEN_TIMEOUT: u64 = 3600; +pub const OPEN_ID_ID_TOKEN_TIMEOUT: u64 = 3600; pub const OPEN_ID_REFRESH_TOKEN_LEN: usize = 120; pub const OPEN_ID_REFRESH_TOKEN_TIMEOUT: u64 = 360000; diff --git a/src/controllers/openid_controller.rs b/src/controllers/openid_controller.rs index edaa063..3e22f00 100644 --- a/src/controllers/openid_controller.rs +++ b/src/controllers/openid_controller.rs @@ -131,6 +131,7 @@ fn error_redirect(query: &AuthorizeQuery, error: &str, description: &str) -> Htt .finish() } +#[allow(clippy::too_many_arguments)] pub async fn authorize( req: HttpRequest, user: CurrentUser, @@ -139,10 +140,13 @@ pub async fn authorize( clients: web::Data>, sessions: web::Data>, logger: ActionLogger, -) -> impl Responder { + jwt_signer: web::Data, +) -> actix_web::Result { let client = match clients.find_by_id(&query.client_id) { None => { - return HttpResponse::BadRequest().body(build_fatal_error_page("Client is invalid!")); + return Ok( + HttpResponse::BadRequest().body(build_fatal_error_page("Client is invalid!")) + ); } Some(c) => c, }; @@ -150,31 +154,42 @@ pub async fn authorize( // Check if 2FA is required if client.enforce_2fa_auth && user.should_request_2fa_for_critical_functions() { let uri = get_2fa_url(&LoginRedirect::from_req(&req), true); - return redirect_user(&uri); + return Ok(redirect_user(&uri)); } + // Validate specified redirect URI let redirect_uri = query.redirect_uri.trim().to_string(); if !redirect_uri.starts_with(&client.redirect_uri) { - return HttpResponse::BadRequest().body(build_fatal_error_page("Redirect URI is invalid!")); + return Ok( + HttpResponse::BadRequest().body(build_fatal_error_page("Redirect URI is invalid!")) + ); } if !query.scope.split(' ').any(|x| x == "openid") { - return error_redirect(&query, "invalid_request", "openid scope missing!"); + return Ok(error_redirect( + &query, + "invalid_request", + "openid scope missing!", + )); } if query.state.as_ref().map(String::is_empty).unwrap_or(false) { - return error_redirect(&query, "invalid_request", "State is specified but empty!"); + return Ok(error_redirect( + &query, + "invalid_request", + "State is specified but empty!", + )); } let code_challenge = match query.0.code_challenge.clone() { Some(chal) => { let meth = query.0.code_challenge_method.as_deref().unwrap_or("plain"); if !meth.eq("S256") && !meth.eq("plain") { - return error_redirect( + return Ok(error_redirect( &query, "invalid_request", "Only S256 and plain code challenge methods are supported!", - ); + )); } Some(CodeChallenge { code_challenge: chal, @@ -186,16 +201,20 @@ pub async fn authorize( // Check if user is authorized to access the application if !user.can_access_app(&client) { - return error_redirect( + return Ok(error_redirect( &query, "invalid_request", "User is not authorized to access this application!", - ); + )); } // Check that requested authorization flow is supported if query.response_type != "code" && query.response_type != "id_token" { - return error_redirect(&query, "invalid_request", "Unsupported authorization flow!"); + return Ok(error_redirect( + &query, + "invalid_request", + "Unsupported authorization flow!", + )); } match (client.auth_flow(), query.response_type.as_str()) { @@ -224,7 +243,7 @@ pub async fn authorize( log::trace!("New OpenID session: {:#?}", session); logger.log(Action::NewOpenIDSession { client: &client }); - HttpResponse::Found() + Ok(HttpResponse::Found() .append_header(( "Location", format!( @@ -238,10 +257,40 @@ pub async fn authorize( urlencoding::encode(&session.authorization_code) ), )) - .finish() + .finish()) + } + + (AuthenticationFlow::Implicit, "id_token") => { + let id_token = IdToken { + issuer: AppConfig::get().website_origin.to_string(), + subject_identifier: user.uid.0.clone(), + audience: client.id.0.to_string(), + expiration_time: time() + OPEN_ID_ID_TOKEN_TIMEOUT, + issued_at: time(), + auth_time: SessionIdentity(Some(&id)).auth_time(), + nonce: query.nonce.clone(), + email: user.email.clone(), + }; + + log::trace!("New OpenID id token: {:#?}", &id_token); + logger.log(Action::NewOpenIDSuccessfulImplicitAuth { client: &client }); + + Ok(HttpResponse::Found() + .append_header(( + "Location", + format!( + "{}?{}token_type=bearer&id_token={}&expires_in={OPEN_ID_ID_TOKEN_TIMEOUT}", + client.redirect_uri, + match &query.0.state { + Some(state) => format!("state={}&", urlencoding::encode(state)), + None => "".to_string(), + }, + jwt_signer.sign_token(id_token.to_jwt_claims())? + ), + )) + .finish()) } - //(AuthenticationFlow::Implicit, "id_token") => {} (flow, code) => { log::warn!( "For client {:?}, configured with flow {:?}, made request with code {}", @@ -249,11 +298,11 @@ pub async fn authorize( flow, code ); - error_redirect( + Ok(error_redirect( &query, "invalid_request", "Requested authentication flow is unsupported / not configured for this client!", - ) + )) } } } diff --git a/src/data/action_logger.rs b/src/data/action_logger.rs index 46e9278..a973185 100644 --- a/src/data/action_logger.rs +++ b/src/data/action_logger.rs @@ -90,6 +90,9 @@ pub enum Action<'a> { NewOpenIDSession { client: &'a Client, }, + NewOpenIDSuccessfulImplicitAuth { + client: &'a Client, + }, ChangedHisPassword, ClearedHisLoginHistory, AddNewFactor(&'a TwoFactor), @@ -199,6 +202,7 @@ impl<'a> Action<'a> { Action::NewOpenIDSession { client } => { format!("opened a new OpenID session with {:?}", client.id) } + Action::NewOpenIDSuccessfulImplicitAuth { client } => format!("finished an implicit flow connection for client {:?}", client.id), Action::ChangedHisPassword => "changed his password".to_string(), Action::ClearedHisLoginHistory => "cleared his login history".to_string(), Action::AddNewFactor(factor) => format!( @@ -206,7 +210,6 @@ impl<'a> Action<'a> { factor.quick_description(), ), Action::Removed2FAFactor { factor_id } => format!("Removed his factor {factor_id:?}"), - } } } diff --git a/src/data/id_token.rs b/src/data/id_token.rs index a0e51ea..dc9c197 100644 --- a/src/data/id_token.rs +++ b/src/data/id_token.rs @@ -1,7 +1,7 @@ use jwt_simple::claims::Audiences; use jwt_simple::prelude::{Duration, JWTClaims}; -#[derive(serde::Serialize)] +#[derive(serde::Serialize, Debug)] pub struct IdToken { /// REQUIRED. Issuer Identifier for the Issuer of the response. The iss value is a case sensitive URL using the https scheme that contains scheme, host, and optionally, port number and path components and no query or fragment components. #[serde(rename = "iss")] -- 2.45.2