From 81b06cc9111de49ba1470092cd0cb750236ded6a Mon Sep 17 00:00:00 2001 From: avdb13 Date: Fri, 15 Nov 2024 00:17:17 +0000 Subject: [PATCH] feat: add PKCE --- crates/api_common/src/oauth_provider.rs | 5 +++ crates/api_common/src/utils.rs | 13 +++++++ crates/api_crud/src/oauth_provider/create.rs | 1 + crates/api_crud/src/oauth_provider/update.rs | 1 + crates/api_crud/src/user/create.rs | 38 ++++++++++++++----- crates/db_schema/src/schema.rs | 1 + crates/db_schema/src/source/oauth_provider.rs | 5 +++ crates/utils/src/error.rs | 1 + .../2024-11-12-080606_oauth_pkce/down.sql | 3 ++ .../2024-11-12-080606_oauth_pkce/up.sql | 3 ++ 10 files changed, 61 insertions(+), 10 deletions(-) create mode 100644 migrations/2024-11-12-080606_oauth_pkce/down.sql create mode 100644 migrations/2024-11-12-080606_oauth_pkce/up.sql diff --git a/crates/api_common/src/oauth_provider.rs b/crates/api_common/src/oauth_provider.rs index 36fef3b18..21605af66 100644 --- a/crates/api_common/src/oauth_provider.rs +++ b/crates/api_common/src/oauth_provider.rs @@ -25,6 +25,8 @@ pub struct CreateOAuthProvider { #[cfg_attr(feature = "full", ts(optional))] pub account_linking_enabled: Option, #[cfg_attr(feature = "full", ts(optional))] + pub use_pkce: Option, + #[cfg_attr(feature = "full", ts(optional))] pub enabled: Option, } @@ -54,6 +56,8 @@ pub struct EditOAuthProvider { #[cfg_attr(feature = "full", ts(optional))] pub account_linking_enabled: Option, #[cfg_attr(feature = "full", ts(optional))] + pub use_pkce: Option, + #[cfg_attr(feature = "full", ts(optional))] pub enabled: Option, } @@ -72,6 +76,7 @@ pub struct DeleteOAuthProvider { /// Logging in with an OAuth 2.0 authorization pub struct AuthenticateWithOauth { pub code: String, + pub pkce_code_verifier: Option, pub oauth_provider_id: OAuthProviderId, pub redirect_uri: Url, #[cfg_attr(feature = "full", ts(optional))] diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 09cdac28c..997c137b4 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -1162,6 +1162,19 @@ fn build_proxied_image_url( )) } +pub fn check_code_verifier(code_verifier: &str) -> LemmyResult<&str> { + static VALID_CODE_VERIFIER_REGEX: LazyLock = + LazyLock::new(|| Regex::new(r"^[a-zA-Z0-9\-._~]{43,128}$").expect("compile regex")); + + let check = VALID_CODE_VERIFIER_REGEX.is_match(code_verifier); + + if !check { + Err(LemmyErrorType::InvalidCodeVerifier.into()) + } else { + Ok(code_verifier) + } +} + #[cfg(test)] mod tests { diff --git a/crates/api_crud/src/oauth_provider/create.rs b/crates/api_crud/src/oauth_provider/create.rs index fe44ae56e..c1e30066a 100644 --- a/crates/api_crud/src/oauth_provider/create.rs +++ b/crates/api_crud/src/oauth_provider/create.rs @@ -35,6 +35,7 @@ pub async fn create_oauth_provider( scopes: data.scopes.to_string(), auto_verify_email: data.auto_verify_email, account_linking_enabled: data.account_linking_enabled, + use_pkce: data.use_pkce, enabled: data.enabled, }; let oauth_provider = OAuthProvider::create(&mut context.pool(), &oauth_provider_form).await?; diff --git a/crates/api_crud/src/oauth_provider/update.rs b/crates/api_crud/src/oauth_provider/update.rs index b4734bf36..081527ecb 100644 --- a/crates/api_crud/src/oauth_provider/update.rs +++ b/crates/api_crud/src/oauth_provider/update.rs @@ -32,6 +32,7 @@ pub async fn update_oauth_provider( auto_verify_email: data.auto_verify_email, account_linking_enabled: data.account_linking_enabled, enabled: data.enabled, + use_pkce: data.use_pkce, updated: Some(Some(naive_now())), }; diff --git a/crates/api_crud/src/user/create.rs b/crates/api_crud/src/user/create.rs index ed560e3d6..d20c6abea 100644 --- a/crates/api_crud/src/user/create.rs +++ b/crates/api_crud/src/user/create.rs @@ -6,6 +6,7 @@ use lemmy_api_common::{ oauth_provider::AuthenticateWithOauth, person::{LoginResponse, Register}, utils::{ + check_code_verifier, check_email_verified, check_registration_application, check_user_valid, @@ -228,9 +229,20 @@ pub async fn authenticate_with_oauth( return Err(LemmyErrorType::OauthAuthorizationInvalid)?; } - let token_response = - oauth_request_access_token(&context, &oauth_provider, &data.code, redirect_uri.as_str()) - .await?; + let pkce_code_verifier = data + .pkce_code_verifier + .as_deref() + .map(check_code_verifier) + .transpose()?; + + let token_response = oauth_request_access_token( + &context, + &oauth_provider, + &data.code, + pkce_code_verifier, + redirect_uri.as_str(), + ) + .await?; let user_info = oidc_get_user_info( &context, @@ -509,20 +521,26 @@ async fn oauth_request_access_token( context: &Data, oauth_provider: &OAuthProvider, code: &str, + pkce_code_verifier: Option<&str>, redirect_uri: &str, ) -> LemmyResult { + let mut form = vec![ + ("grant_type", "authorization_code"), + ("code", code), + ("redirect_uri", redirect_uri), + ("client_id", &oauth_provider.client_id), + ("client_secret", &oauth_provider.client_secret), + ]; + if let Some(code_verifier) = pkce_code_verifier { + form.push(("code_verifier", code_verifier)); + } + // Request an Access Token from the OAUTH provider let response = context .client() .post(oauth_provider.token_endpoint.as_str()) .header("Accept", "application/json") - .form(&[ - ("grant_type", "authorization_code"), - ("code", code), - ("redirect_uri", redirect_uri), - ("client_id", &oauth_provider.client_id), - ("client_secret", &oauth_provider.client_secret), - ]) + .form(&*form) .send() .await; diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs index 9e80c4693..a9c825f92 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -631,6 +631,7 @@ diesel::table! { scopes -> Text, auto_verify_email -> Bool, account_linking_enabled -> Bool, + use_pkce -> Bool, enabled -> Bool, published -> Timestamptz, updated -> Nullable, diff --git a/crates/db_schema/src/source/oauth_provider.rs b/crates/db_schema/src/source/oauth_provider.rs index a70405a5e..333e76432 100644 --- a/crates/db_schema/src/source/oauth_provider.rs +++ b/crates/db_schema/src/source/oauth_provider.rs @@ -57,6 +57,8 @@ pub struct OAuthProvider { pub auto_verify_email: bool, /// Allows linking an OAUTH account to an existing user account by matching emails pub account_linking_enabled: bool, + /// switch to enable or disable PKCE + pub use_pkce: bool, /// switch to enable or disable an oauth provider pub enabled: bool, pub published: DateTime, @@ -83,6 +85,7 @@ impl Serialize for PublicOAuthProvider { state.serialize_field("authorization_endpoint", &self.0.authorization_endpoint)?; state.serialize_field("client_id", &self.0.client_id)?; state.serialize_field("scopes", &self.0.scopes)?; + state.serialize_field("use_pkce", &self.0.use_pkce)?; state.end() } } @@ -102,6 +105,7 @@ pub struct OAuthProviderInsertForm { pub scopes: String, pub auto_verify_email: Option, pub account_linking_enabled: Option, + pub use_pkce: Option, pub enabled: Option, } @@ -118,6 +122,7 @@ pub struct OAuthProviderUpdateForm { pub scopes: Option, pub auto_verify_email: Option, pub account_linking_enabled: Option, + pub use_pkce: Option, pub enabled: Option, pub updated: Option>>, } diff --git a/crates/utils/src/error.rs b/crates/utils/src/error.rs index d52df2f72..9d8d75a97 100644 --- a/crates/utils/src/error.rs +++ b/crates/utils/src/error.rs @@ -76,6 +76,7 @@ pub enum LemmyErrorType { MissingAnEmail, RateLimitError, InvalidName, + InvalidCodeVerifier, InvalidDisplayName, InvalidMatrixId, InvalidPostTitle, diff --git a/migrations/2024-11-12-080606_oauth_pkce/down.sql b/migrations/2024-11-12-080606_oauth_pkce/down.sql new file mode 100644 index 000000000..50c09050a --- /dev/null +++ b/migrations/2024-11-12-080606_oauth_pkce/down.sql @@ -0,0 +1,3 @@ +ALTER TABLE oauth_provider + DROP COLUMN use_pkce; + diff --git a/migrations/2024-11-12-080606_oauth_pkce/up.sql b/migrations/2024-11-12-080606_oauth_pkce/up.sql new file mode 100644 index 000000000..b03d74f7f --- /dev/null +++ b/migrations/2024-11-12-080606_oauth_pkce/up.sql @@ -0,0 +1,3 @@ +ALTER TABLE oauth_provider + ADD COLUMN use_pkce boolean DEFAULT FALSE NOT NULL; +