Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SSO Support #4881

Merged
merged 21 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
e0b20cd
Added OAUTH2 OIDC support
privacyguard Jun 30, 2024
6b2ee3d
Fixes and improvements based on review feedback
privacyguard Jul 6, 2024
347815f
use derive_new::new instead of TypedBuilder
privacyguard Jul 6, 2024
5cf4839
merge migrations into a single file
privacyguard Jul 8, 2024
6c6015e
fixes based on review feedback
privacyguard Jul 10, 2024
0d151a9
remove unnecessary hostname_ui config
privacyguard Jul 12, 2024
f23eafc
improvement based on review feedback
privacyguard Jul 12, 2024
d97d27a
improvements based on review feedback
privacyguard Jul 14, 2024
337598f
delete user oauth accounts at account deletion
privacyguard Jul 14, 2024
4d09b74
fixes and improvements based on review feedback
privacyguard Jul 15, 2024
331eff9
removed auto_approve_application
privacyguard Jul 15, 2024
438516d
support registration application with sso
privacyguard Jul 16, 2024
a5a3ec9
improvements based on review feedback
privacyguard Jul 22, 2024
2d6ee77
making the TokenResponse an internal struct as it should be
privacyguard Jul 22, 2024
8edfe1d
remove duplicate struct
privacyguard Jul 23, 2024
cd2d6c6
prevent oauth linking to unverified accounts
privacyguard Jul 24, 2024
d5dbe3f
switched to manually entered username and removed the oauth name claim
privacyguard Jul 24, 2024
454402b
fix cargo fmt
privacyguard Jul 24, 2024
8e3038d
fix compile error
privacyguard Jul 24, 2024
7698794
improvements based on review feedback
privacyguard Aug 4, 2024
cfa0174
fixes and improvements based on review feedback
privacyguard Sep 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 7 additions & 5 deletions crates/api/src/local_user/change_password.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@ pub async fn change_password(
}

// Check the old password
let valid: bool = verify(
&data.old_password,
&local_user_view.local_user.password_encrypted,
)
.unwrap_or(false);
let valid: bool = if let Some(password_encrypted) = &local_user_view.local_user.password_encrypted
{
verify(&data.old_password, password_encrypted).unwrap_or(false)
} else {
data.old_password.is_empty()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be false right? If there's no password encrypted, that means its an oauth-type account, and you shouldn't be able to change the password.

I think you could use map then: let valid: bool = password_encrypted.map(|password_encrypted| verify...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every OIDC account is linked to a local_user. If you sign up with OIDC with a new email a local_user gets created. You can set a password to an account created using OIDC and start using the email and password to login if you want.

This is needed when you want to delete your account because a password is required for account deletion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds confusing. Wouldnt it be better to allow account deletion directly with oauth?

Though it makes sense that users can switch from oauth to password login and vice versa.

};

if !valid {
Err(LemmyErrorType::IncorrectLogin)?
}
Expand Down
45 changes: 8 additions & 37 deletions crates/api/src/local_user/login.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{check_totp_2fa_valid, local_user::check_email_verified};
use crate::check_totp_2fa_valid;
use actix_web::{
web::{Data, Json},
HttpRequest,
Expand All @@ -8,12 +8,7 @@ use lemmy_api_common::{
claims::Claims,
context::LemmyContext,
person::{Login, LoginResponse},
utils::check_user_valid,
};
use lemmy_db_schema::{
source::{local_site::LocalSite, registration_application::RegistrationApplication},
utils::DbPool,
RegistrationMode,
utils::{check_email_verified, check_registration_application, check_user_valid},
};
use lemmy_db_views::structs::{LocalUserView, SiteView};
use lemmy_utils::error::{LemmyErrorType, LemmyResult};
Expand All @@ -34,11 +29,12 @@ pub async fn login(
.ok_or(LemmyErrorType::IncorrectLogin)?;

// Verify the password
let valid: bool = verify(
&data.password,
&local_user_view.local_user.password_encrypted,
)
.unwrap_or(false);
let valid: bool = local_user_view
.local_user
.password_encrypted
.as_ref()
.and_then(|password_encrypted| verify(&data.password, password_encrypted).ok())
.unwrap_or(false);
if !valid {
Err(LemmyErrorType::IncorrectLogin)?
}
Expand All @@ -65,28 +61,3 @@ pub async fn login(
registration_created: false,
}))
}

async fn check_registration_application(
local_user_view: &LocalUserView,
local_site: &LocalSite,
pool: &mut DbPool<'_>,
) -> LemmyResult<()> {
if (local_site.registration_mode == RegistrationMode::RequireApplication
|| local_site.registration_mode == RegistrationMode::Closed)
&& !local_user_view.local_user.accepted_application
&& !local_user_view.local_user.admin
{
// Fetch the registration application. If no admin id is present its still pending. Otherwise it
// was processed (either accepted or denied).
let local_user_id = local_user_view.local_user.id;
let registration = RegistrationApplication::find_by_local_user_id(pool, local_user_id)
.await?
.ok_or(LemmyErrorType::CouldntFindRegistrationApplication)?;
if registration.admin_id.is_some() {
Err(LemmyErrorType::RegistrationDenied(registration.deny_reason))?
} else {
Err(LemmyErrorType::RegistrationApplicationIsPending)?
}
}
Ok(())
}
15 changes: 0 additions & 15 deletions crates/api/src/local_user/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
use lemmy_db_views::structs::{LocalUserView, SiteView};
use lemmy_utils::{error::LemmyResult, LemmyErrorType};

pub mod add_admin;
pub mod ban_person;
pub mod block;
Expand All @@ -20,15 +17,3 @@ pub mod save_settings;
pub mod update_totp;
pub mod validate_auth;
pub mod verify_email;

/// Check if the user's email is verified if email verification is turned on
/// However, skip checking verification if the user is an admin
fn check_email_verified(local_user_view: &LocalUserView, site_view: &SiteView) -> LemmyResult<()> {
if !local_user_view.local_user.admin
&& site_view.local_site.require_email_verification
&& !local_user_view.local_user.email_verified
{
Err(LemmyErrorType::EmailNotVerified)?
}
Ok(())
}
3 changes: 1 addition & 2 deletions crates/api/src/local_user/reset_password.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use crate::local_user::check_email_verified;
use actix_web::web::{Data, Json};
use lemmy_api_common::{
context::LemmyContext,
person::PasswordReset,
utils::send_password_reset_email,
utils::{check_email_verified, send_password_reset_email},
SuccessResponse,
};
use lemmy_db_views::structs::{LocalUserView, SiteView};
Expand Down
4 changes: 4 additions & 0 deletions crates/api/src/site/leave_admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use lemmy_db_schema::{
local_site_url_blocklist::LocalSiteUrlBlocklist,
local_user::{LocalUser, LocalUserUpdateForm},
moderator::{ModAdd, ModAddForm},
oauth_provider::OAuthProvider,
tagline::Tagline,
},
traits::Crud,
Expand Down Expand Up @@ -63,6 +64,7 @@ pub async fn leave_admin(
let taglines = Tagline::get_all(&mut context.pool(), site_view.local_site.id).await?;
let custom_emojis =
CustomEmojiView::get_all(&mut context.pool(), site_view.local_site.id).await?;
let oauth_providers = OAuthProvider::get_all_public(&mut context.pool()).await?;
let blocked_urls = LocalSiteUrlBlocklist::get_all(&mut context.pool()).await?;

Ok(Json(GetSiteResponse {
Expand All @@ -74,6 +76,8 @@ pub async fn leave_admin(
discussion_languages,
taglines,
custom_emojis,
oauth_providers: Some(oauth_providers),
admin_oauth_providers: None,
blocked_urls,
}))
}
1 change: 1 addition & 0 deletions crates/api_common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub mod community;
#[cfg(feature = "full")]
pub mod context;
pub mod custom_emoji;
pub mod oauth_provider;
pub mod person;
pub mod post;
pub mod private_message;
Expand Down
69 changes: 69 additions & 0 deletions crates/api_common/src/oauth_provider.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
use lemmy_db_schema::newtypes::OAuthProviderId;
use serde::{Deserialize, Serialize};
use serde_with::skip_serializing_none;
#[cfg(feature = "full")]
use ts_rs::TS;
use url::Url;

#[derive(Debug, Serialize, Deserialize, Clone)]
#[cfg_attr(feature = "full", derive(TS))]
#[cfg_attr(feature = "full", ts(export))]
/// Create an external auth method.
pub struct CreateOAuthProvider {
pub display_name: String,
pub issuer: String,
pub authorization_endpoint: String,
pub token_endpoint: String,
pub userinfo_endpoint: String,
pub id_claim: String,
pub client_id: String,
pub client_secret: String,
pub scopes: String,
pub auto_verify_email: bool,
pub account_linking_enabled: bool,
pub enabled: bool,
}

#[derive(Debug, Serialize, Deserialize, Clone)]
#[cfg_attr(feature = "full", derive(TS))]
#[cfg_attr(feature = "full", ts(export))]
/// Edit an external auth method.
pub struct EditOAuthProvider {
pub id: OAuthProviderId,
pub display_name: Option<String>,
pub authorization_endpoint: Option<String>,
pub token_endpoint: Option<String>,
pub userinfo_endpoint: Option<String>,
pub id_claim: Option<String>,
pub client_secret: Option<String>,
pub scopes: Option<String>,
pub auto_verify_email: Option<bool>,
pub account_linking_enabled: Option<bool>,
pub enabled: Option<bool>,
}

#[derive(Debug, Serialize, Deserialize, Clone, Default)]
#[cfg_attr(feature = "full", derive(TS))]
#[cfg_attr(feature = "full", ts(export))]
/// Delete an external auth method.
pub struct DeleteOAuthProvider {
pub id: OAuthProviderId,
}

#[skip_serializing_none]
#[derive(Debug, Serialize, Deserialize, Clone)]
#[cfg_attr(feature = "full", derive(TS))]
#[cfg_attr(feature = "full", ts(export))]
/// Logging in with an OAuth 2.0 authorization
pub struct AuthenticateWithOauth {
pub code: String,
#[cfg_attr(feature = "full", ts(type = "string"))]
pub oauth_provider_id: OAuthProviderId,
#[cfg_attr(feature = "full", ts(type = "string"))]
pub redirect_uri: Url,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whats this for, is it required for oauth?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used in the oauth token issuance API call:

("redirect_uri", redirect_uri.as_str()),

pub show_nsfw: Option<bool>,
/// Username is mandatory at registration time
pub username: Option<String>,
/// An answer is mandatory if require application is enabled on the server
pub answer: Option<String>,
}
1 change: 1 addition & 0 deletions crates/api_common/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub fn client_builder(settings: &Settings) -> ClientBuilder {
.user_agent(user_agent.clone())
.timeout(REQWEST_TIMEOUT)
.connect_timeout(REQWEST_TIMEOUT)
.use_rustls_tls()
Nutomic marked this conversation as resolved.
Show resolved Hide resolved
}

/// Fetches metadata for the given link and optionally generates thumbnail.
Expand Down
7 changes: 7 additions & 0 deletions crates/api_common/src/site.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use lemmy_db_schema::{
instance::Instance,
language::Language,
local_site_url_blocklist::LocalSiteUrlBlocklist,
oauth_provider::{OAuthProvider, PublicOAuthProvider},
person::Person,
tagline::Tagline,
},
Expand Down Expand Up @@ -200,6 +201,7 @@ pub struct CreateSite {
pub blocked_instances: Option<Vec<String>>,
pub taglines: Option<Vec<String>>,
pub registration_mode: Option<RegistrationMode>,
pub oauth_registration: Option<bool>,
pub content_warning: Option<String>,
pub default_post_listing_mode: Option<PostListingMode>,
}
Expand Down Expand Up @@ -282,6 +284,8 @@ pub struct EditSite {
/// A list of taglines shown at the top of the front page.
pub taglines: Option<Vec<String>>,
pub registration_mode: Option<RegistrationMode>,
/// Whether or not external auth methods can auto-register users.
pub oauth_registration: Option<bool>,
/// Whether to email admins for new reports.
pub reports_email_admins: Option<bool>,
/// If present, nsfw content is visible by default. Should be displayed by frontends/clients
Expand Down Expand Up @@ -316,6 +320,9 @@ pub struct GetSiteResponse {
pub taglines: Vec<Tagline>,
/// A list of custom emojis your site supports.
pub custom_emojis: Vec<CustomEmojiView>,
/// A list of external auth methods your site supports.
pub oauth_providers: Option<Vec<PublicOAuthProvider>>,
pub admin_oauth_providers: Option<Vec<OAuthProvider>>,
pub blocked_urls: Vec<LocalSiteUrlBlocklist>,
}

Expand Down
50 changes: 49 additions & 1 deletion crates/api_common/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,21 @@ use lemmy_db_schema::{
local_site::LocalSite,
local_site_rate_limit::LocalSiteRateLimit,
local_site_url_blocklist::LocalSiteUrlBlocklist,
oauth_account::OAuthAccount,
password_reset_request::PasswordResetRequest,
person::{Person, PersonUpdateForm},
person_block::PersonBlock,
post::{Post, PostRead},
registration_application::RegistrationApplication,
site::Site,
},
traits::Crud,
utils::DbPool,
RegistrationMode,
};
use lemmy_db_views::{
comment_view::CommentQuery,
structs::{LocalImageView, LocalUserView},
structs::{LocalImageView, LocalUserView, SiteView},
};
use lemmy_db_views_actor::structs::{
CommunityModeratorView,
Expand Down Expand Up @@ -192,6 +195,46 @@ pub fn check_user_valid(person: &Person) -> LemmyResult<()> {
}
}

/// Check if the user's email is verified if email verification is turned on
/// However, skip checking verification if the user is an admin
pub fn check_email_verified(
local_user_view: &LocalUserView,
site_view: &SiteView,
) -> LemmyResult<()> {
if !local_user_view.local_user.admin
&& site_view.local_site.require_email_verification
&& !local_user_view.local_user.email_verified
{
Err(LemmyErrorType::EmailNotVerified)?
}
Ok(())
}

pub async fn check_registration_application(
local_user_view: &LocalUserView,
local_site: &LocalSite,
pool: &mut DbPool<'_>,
) -> LemmyResult<()> {
if (local_site.registration_mode == RegistrationMode::RequireApplication
|| local_site.registration_mode == RegistrationMode::Closed)
&& !local_user_view.local_user.accepted_application
&& !local_user_view.local_user.admin
{
// Fetch the registration application. If no admin id is present its still pending. Otherwise it
// was processed (either accepted or denied).
let local_user_id = local_user_view.local_user.id;
let registration = RegistrationApplication::find_by_local_user_id(pool, local_user_id)
.await?
.ok_or(LemmyErrorType::CouldntFindRegistrationApplication)?;
if registration.admin_id.is_some() {
Err(LemmyErrorType::RegistrationDenied(registration.deny_reason))?
} else {
Err(LemmyErrorType::RegistrationApplicationIsPending)?
}
}
Ok(())
}

/// Checks that a normal user action (eg posting or voting) is allowed in a given community.
///
/// In particular it checks that neither the user nor community are banned or deleted, and that
Expand Down Expand Up @@ -852,6 +895,11 @@ pub async fn purge_user_account(person_id: PersonId, context: &LemmyContext) ->
// Leave communities they mod
CommunityModerator::leave_all_communities(pool, person_id).await?;

// Delete the oauth accounts linked to the local user
if let Ok(Some(local_user)) = LocalUserView::read_person(pool, person_id).await {
OAuthAccount::delete_user_accounts(pool, local_user.local_user.id).await?;
}

Person::delete_account(pool, person_id).await?;

Ok(())
Expand Down
3 changes: 3 additions & 0 deletions crates/api_crud/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ moka.workspace = true
anyhow.workspace = true
webmention = "0.6.0"
accept-language = "3.1.0"
serde_json = { workspace = true }
serde = { workspace = true }
serde_with = { workspace = true }

[package.metadata.cargo-machete]
ignored = ["futures"]
1 change: 1 addition & 0 deletions crates/api_crud/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
pub mod comment;
pub mod community;
pub mod custom_emoji;
pub mod oauth_provider;
pub mod post;
pub mod private_message;
pub mod site;
Expand Down
Loading