Add more options to deal with localpart conflicts on upstream OAuth 2.0 logins

This commit is contained in:
Quentin Gliech
2025-11-28 10:53:28 +01:00
parent e90f11b8f8
commit f97f56ed11
5 changed files with 128 additions and 10 deletions

View File

@@ -45,6 +45,12 @@ fn map_import_on_conflict(
mas_config::UpstreamOAuth2OnConflict::Add => {
mas_data_model::UpstreamOAuthProviderOnConflict::Add
}
mas_config::UpstreamOAuth2OnConflict::Replace => {
mas_data_model::UpstreamOAuthProviderOnConflict::Replace
}
mas_config::UpstreamOAuth2OnConflict::Set => {
mas_data_model::UpstreamOAuthProviderOnConflict::Set
}
mas_config::UpstreamOAuth2OnConflict::Fail => {
mas_data_model::UpstreamOAuthProviderOnConflict::Fail
}

View File

@@ -206,13 +206,20 @@ impl ImportAction {
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default, JsonSchema)]
#[serde(rename_all = "lowercase")]
pub enum OnConflict {
/// Fails the sso login on conflict
/// Fails the upstream OAuth 2.0 login on conflict
#[default]
Fail,
/// Adds the oauth identity link, regardless of whether there is an existing
/// link or not
/// Adds the upstream OAuth 2.0 identity link, regardless of whether there
/// is an existing link or not
Add,
/// Replace any existing upstream OAuth 2.0 identity link
Replace,
/// Adds the upstream OAuth 2.0 identity link *only* if there is no existing
/// link for this provider on the matching user
Set,
}
impl OnConflict {

View File

@@ -415,11 +415,18 @@ impl ImportAction {
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)]
#[serde(rename_all = "lowercase")]
pub enum OnConflict {
/// Fails the upstream OAuth 2.0 login
/// Fails the upstream OAuth 2.0 login on conflict
#[default]
Fail,
/// Adds the upstream account link, regardless of whether there is an
/// existing link or not
/// Adds the upstream OAuth 2.0 identity link, regardless of whether there
/// is an existing link or not
Add,
/// Replace any existing upstream OAuth 2.0 identity link
Replace,
/// Adds the upstream OAuth 2.0 identity link *only* if there is no existing
/// link for this provider on the matching user
Set,
}

View File

@@ -25,8 +25,10 @@ use mas_matrix::HomeserverConnection;
use mas_policy::Policy;
use mas_router::UrlBuilder;
use mas_storage::{
BoxRepository, RepositoryAccess,
upstream_oauth2::{UpstreamOAuthLinkRepository, UpstreamOAuthSessionRepository},
BoxRepository, Pagination, RepositoryAccess,
upstream_oauth2::{
UpstreamOAuthLinkFilter, UpstreamOAuthLinkRepository, UpstreamOAuthSessionRepository,
},
user::{BrowserSessionRepository, UserEmailRepository, UserRepository},
};
use mas_templates::{
@@ -585,6 +587,92 @@ pub(crate) async fn get(
.associate_to_user(&link, &existing_user)
.await?;
}
// We matched anexisting user and the conflict resolution is to replace any
// link on the existing user with this one
UpstreamOAuthProviderOnConflict::Replace => {
// Find existing links for this provider and user
let filter = UpstreamOAuthLinkFilter::new()
.for_provider(&provider)
.for_user(&existing_user);
let mut cursor = Pagination::first(100);
let mut removed = 0;
loop {
let page = repo.upstream_oauth_link().list(filter, cursor).await?;
for edge in page.edges {
// Remove any existing links for this provider and user
repo.upstream_oauth_link().remove(&clock, edge.node).await?;
cursor = cursor.after(edge.cursor);
removed += 1;
}
if !page.has_next_page {
break;
}
}
if removed > 0 {
tracing::warn!(
user.id = %existing_user.id,
upstream_oauth_provider.id = %provider.id,
upstream_oauth_link.id = %link.id,
upstream_oauth_link.subject = link.subject,
"Upstream account mapped localpart {localpart:?} matched an existing user, replaced {removed} links"
);
} else {
tracing::info!(
user.id = %existing_user.id,
upstream_oauth_provider.id = %provider.id,
upstream_oauth_link.id = %link.id,
upstream_oauth_link.subject = link.subject,
"Upstream account mapped localpart {localpart:?} matched an existing user, linking"
);
}
// Add link to the user
repo.upstream_oauth_link()
.associate_to_user(&link, &existing_user)
.await?;
}
// We matched an existing user and the conflict resolution is link to the
// existing user *only if* there is no existing link on that user
UpstreamOAuthProviderOnConflict::Set => {
// Find existing links for this provider and user
let filter = UpstreamOAuthLinkFilter::new()
.for_provider(&provider)
.for_user(&existing_user);
let count = repo.upstream_oauth_link().count(filter).await?;
if count > 0 {
tracing::warn!(
upstream_oauth_provider.id = %provider.id,
upstream_oauth_link.id = %link.id,
user.id = %existing_user.id,
"Upstream provider returned a localpart {localpart:?} which is already used by another user. That user already has a ({count}) link to this provider, which isn't allowed by the conflict resolution"
);
// TODO: translate
let ctx = ErrorContext::new()
.with_code("User exists")
.with_description(format!(
r"Upstream account provider returned {localpart:?} as username,
which is not linked to another existing upstream account.
Your homeserver does not allow replacing upstream account links automatically."
))
.with_language(&locale);
return Ok((
cookie_jar,
Html(templates.render_error(&ctx)?).into_response(),
));
}
// Add link to the user
repo.upstream_oauth_link()
.associate_to_user(&link, &existing_user)
.await?;
}
}
// Now that we've resolved the conflict, log in that existing user

View File

@@ -2572,14 +2572,24 @@
"description": "How to handle an existing localpart claim",
"oneOf": [
{
"description": "Fails the sso login on conflict",
"description": "Fails the upstream OAuth 2.0 login on conflict",
"type": "string",
"const": "fail"
},
{
"description": "Adds the oauth identity link, regardless of whether there is an existing\n link or not",
"description": "Adds the upstream OAuth 2.0 identity link, regardless of whether there\n is an existing link or not",
"type": "string",
"const": "add"
},
{
"description": "Replace any existing upstream OAuth 2.0 identity link",
"type": "string",
"const": "replace"
},
{
"description": "Adds the upstream OAuth 2.0 identity link *only* if there is no existing\n link for this provider on the matching user",
"type": "string",
"const": "set"
}
]
},