letro/letro-authentication-service
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Merge branch 'quenting/upstream-oauth/better-conflict-options' into quenting/upstream-oauth/skip-interactive

Browse Source
This commit is contained in:
Quentin Gliech
2025-11-28 18:08:09 +01:00
parent 7c16212c4c 450b1fa24c
commit a56482cf60
7 changed files with 571 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
crates/cli/src/sync.rs
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
}

17
crates/config/src/sections/upstream_oauth2.rs
View File

@@ -140,13 +140,13 @@ impl ConfigurationSection for UpstreamOAuth2Config {
if matches!(
provider.claims_imports.localpart.on_conflict,
OnConflict::Add
OnConflict::Add | OnConflict::Replace | OnConflict::Set
) && !matches!(
provider.claims_imports.localpart.action,
ImportAction::Force | ImportAction::Require
) {
return Err(annotate(figment::Error::custom(
"The field `action` must be either `force` or `require` when `on_conflict` is set to `add`",
"The field `action` must be either `force` or `require` when `on_conflict` is set to `add`, `replace` or `set`",
)).with_path("claims_imports.localpart").into());
}
}
@@ -226,13 +226,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 {

13
crates/data-model/src/upstream_oauth2/provider.rs
View File

@@ -418,11 +418,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,
}

515
crates/handlers/src/upstream_oauth2/link.rs
View File

@@ -31,8 +31,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::{
@@ -579,6 +581,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
@@ -1666,4 +1754,427 @@ mod tests {
Ok((link, session))
}
#[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]
async fn test_link_existing_account_replace_conflict(pool: PgPool) {
let existing_username = "john";
let subject = "subject";
let old_subject = "old_subject";
setup();
let state = TestState::from_pool(pool).await.unwrap();
let mut rng = state.rng();
let cookies = CookieHelper::new();
let claims_imports = UpstreamOAuthProviderClaimsImports {
localpart: UpstreamOAuthProviderLocalpartPreference {
action: mas_data_model::UpstreamOAuthProviderImportAction::Require,
template: None,
// This will replace any existing links for this provider and user
on_conflict: mas_data_model::UpstreamOAuthProviderOnConflict::Replace,
},
email: UpstreamOAuthProviderImportPreference {
action: mas_data_model::UpstreamOAuthProviderImportAction::Require,
template: None,
},
..UpstreamOAuthProviderClaimsImports::default()
};
let id_token_claims = serde_json::json!({
"preferred_username": existing_username,
"email": "any@example.com",
"email_verified": true,
});
let id_token = sign_token(&mut rng, &state.key_store, id_token_claims.clone()).unwrap();
// Provision a provider and a link
let mut repo = state.repository().await.unwrap();
let provider = repo
.upstream_oauth_provider()
.add(
&mut rng,
&state.clock,
UpstreamOAuthProviderParams {
issuer: Some("https://example.com/".to_owned()),
human_name: Some("Example Ltd.".to_owned()),
brand_name: None,
scope: Scope::from_iter([OPENID]),
token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None,
token_endpoint_signing_alg: None,
id_token_signed_response_alg: JsonWebSignatureAlg::Rs256,
client_id: "client".to_owned(),
encrypted_client_secret: None,
claims_imports,
authorization_endpoint_override: None,
token_endpoint_override: None,
userinfo_endpoint_override: None,
fetch_userinfo: false,
userinfo_signed_response_alg: None,
jwks_uri_override: None,
discovery_mode: mas_data_model::UpstreamOAuthProviderDiscoveryMode::Oidc,
pkce_mode: mas_data_model::UpstreamOAuthProviderPkceMode::Auto,
response_mode: None,
additional_authorization_parameters: Vec::new(),
forward_login_hint: false,
on_backchannel_logout:
mas_data_model::UpstreamOAuthProviderOnBackchannelLogout::DoNothing,
ui_order: 0,
},
)
.await
.unwrap();
// Create an existing user
let user = repo
.user()
.add(&mut rng, &state.clock, existing_username.to_owned())
.await
.unwrap();
// Create an existing link for this user and provider with a different subject
let old_link = repo
.upstream_oauth_link()
.add(
&mut rng,
&state.clock,
&provider,
old_subject.to_owned(),
None,
)
.await
.unwrap();
repo.upstream_oauth_link()
.associate_to_user(&old_link, &user)
.await
.unwrap();
// Provision upstream authorization session to setup cookies
let (link, session) = add_linked_upstream_session(
&mut rng,
&state.clock,
&mut repo,
&provider,
subject,
&id_token.into_string(),
id_token_claims,
)
.await
.unwrap();
repo.save().await.unwrap();
let cookie_jar = state.cookie_jar();
let upstream_sessions = UpstreamSessionsCookie::default()
.add(session.id, provider.id, "state".to_owned(), None)
.add_link_to_session(session.id, link.id)
.unwrap();
let cookie_jar = upstream_sessions.save(cookie_jar, &state.clock);
cookies.import(cookie_jar);
let request = Request::get(&*mas_router::UpstreamOAuth2Link::new(link.id).path()).empty();
let request = cookies.with_cookies(request);
let response = state.request(request).await;
cookies.save_cookies(&response);
response.assert_status(StatusCode::SEE_OTHER);
// Check that the new link is associated with the existing user
let mut repo = state.repository().await.unwrap();
let new_link = repo
.upstream_oauth_link()
.find_by_subject(&provider, subject)
.await
.unwrap()
.expect("new link exists");
assert_eq!(new_link.user_id, Some(user.id));
// Check that the old link was removed
let old_link_result = repo
.upstream_oauth_link()
.find_by_subject(&provider, old_subject)
.await
.unwrap();
assert!(
old_link_result.is_none(),
"Old link should have been removed"
);
}
#[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]
async fn test_link_existing_account_set_conflict_success(pool: PgPool) {
let existing_username = "john";
let subject = "subject";
setup();
let state = TestState::from_pool(pool).await.unwrap();
let mut rng = state.rng();
let cookies = CookieHelper::new();
let claims_imports = UpstreamOAuthProviderClaimsImports {
localpart: UpstreamOAuthProviderLocalpartPreference {
action: mas_data_model::UpstreamOAuthProviderImportAction::Require,
template: None,
// This will only link if there are no existing links for this provider and user
on_conflict: mas_data_model::UpstreamOAuthProviderOnConflict::Set,
},
email: UpstreamOAuthProviderImportPreference {
action: mas_data_model::UpstreamOAuthProviderImportAction::Require,
template: None,
},
..UpstreamOAuthProviderClaimsImports::default()
};
let id_token_claims = serde_json::json!({
"preferred_username": existing_username,
"email": "any@example.com",
"email_verified": true,
});
let id_token = sign_token(&mut rng, &state.key_store, id_token_claims.clone()).unwrap();
// Provision a provider and a link
let mut repo = state.repository().await.unwrap();
let provider = repo
.upstream_oauth_provider()
.add(
&mut rng,
&state.clock,
UpstreamOAuthProviderParams {
issuer: Some("https://example.com/".to_owned()),
human_name: Some("Example Ltd.".to_owned()),
brand_name: None,
scope: Scope::from_iter([OPENID]),
token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None,
token_endpoint_signing_alg: None,
id_token_signed_response_alg: JsonWebSignatureAlg::Rs256,
client_id: "client".to_owned(),
encrypted_client_secret: None,
claims_imports,
authorization_endpoint_override: None,
token_endpoint_override: None,
userinfo_endpoint_override: None,
fetch_userinfo: false,
userinfo_signed_response_alg: None,
jwks_uri_override: None,
discovery_mode: mas_data_model::UpstreamOAuthProviderDiscoveryMode::Oidc,
pkce_mode: mas_data_model::UpstreamOAuthProviderPkceMode::Auto,
response_mode: None,
additional_authorization_parameters: Vec::new(),
forward_login_hint: false,
on_backchannel_logout:
mas_data_model::UpstreamOAuthProviderOnBackchannelLogout::DoNothing,
ui_order: 0,
},
)
.await
.unwrap();
// Create an existing user (with no existing links for this provider)
let user = repo
.user()
.add(&mut rng, &state.clock, existing_username.to_owned())
.await
.unwrap();
// Provision upstream authorization session to setup cookies
let (link, session) = add_linked_upstream_session(
&mut rng,
&state.clock,
&mut repo,
&provider,
subject,
&id_token.into_string(),
id_token_claims,
)
.await
.unwrap();
repo.save().await.unwrap();
let cookie_jar = state.cookie_jar();
let upstream_sessions = UpstreamSessionsCookie::default()
.add(session.id, provider.id, "state".to_owned(), None)
.add_link_to_session(session.id, link.id)
.unwrap();
let cookie_jar = upstream_sessions.save(cookie_jar, &state.clock);
cookies.import(cookie_jar);
let request = Request::get(&*mas_router::UpstreamOAuth2Link::new(link.id).path()).empty();
let request = cookies.with_cookies(request);
let response = state.request(request).await;
cookies.save_cookies(&response);
response.assert_status(StatusCode::SEE_OTHER);
// Check that the new link is associated with the existing user
let mut repo = state.repository().await.unwrap();
let new_link = repo
.upstream_oauth_link()
.find_by_subject(&provider, subject)
.await
.unwrap()
.expect("new link exists");
assert_eq!(new_link.user_id, Some(user.id));
}
#[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]
async fn test_link_existing_account_set_conflict_failure(pool: PgPool) {
let existing_username = "john";
let subject = "subject";
let old_subject = "old_subject";
setup();
let state = TestState::from_pool(pool).await.unwrap();
let mut rng = state.rng();
let cookies = CookieHelper::new();
let claims_imports = UpstreamOAuthProviderClaimsImports {
localpart: UpstreamOAuthProviderLocalpartPreference {
action: mas_data_model::UpstreamOAuthProviderImportAction::Require,
template: None,
// This will only link if there are no existing links for this provider and user
on_conflict: mas_data_model::UpstreamOAuthProviderOnConflict::Set,
},
email: UpstreamOAuthProviderImportPreference {
action: mas_data_model::UpstreamOAuthProviderImportAction::Require,
template: None,
},
..UpstreamOAuthProviderClaimsImports::default()
};
let id_token_claims = serde_json::json!({
"preferred_username": existing_username,
"email": "any@example.com",
"email_verified": true,
});
let id_token = sign_token(&mut rng, &state.key_store, id_token_claims.clone()).unwrap();
// Provision a provider and a link
let mut repo = state.repository().await.unwrap();
let provider = repo
.upstream_oauth_provider()
.add(
&mut rng,
&state.clock,
UpstreamOAuthProviderParams {
issuer: Some("https://example.com/".to_owned()),
human_name: Some("Example Ltd.".to_owned()),
brand_name: None,
scope: Scope::from_iter([OPENID]),
token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None,
token_endpoint_signing_alg: None,
id_token_signed_response_alg: JsonWebSignatureAlg::Rs256,
client_id: "client".to_owned(),
encrypted_client_secret: None,
claims_imports,
authorization_endpoint_override: None,
token_endpoint_override: None,
userinfo_endpoint_override: None,
fetch_userinfo: false,
userinfo_signed_response_alg: None,
jwks_uri_override: None,
discovery_mode: mas_data_model::UpstreamOAuthProviderDiscoveryMode::Oidc,
pkce_mode: mas_data_model::UpstreamOAuthProviderPkceMode::Auto,
response_mode: None,
additional_authorization_parameters: Vec::new(),
forward_login_hint: false,
on_backchannel_logout:
mas_data_model::UpstreamOAuthProviderOnBackchannelLogout::DoNothing,
ui_order: 0,
},
)
.await
.unwrap();
// Create an existing user
let user = repo
.user()
.add(&mut rng, &state.clock, existing_username.to_owned())
.await
.unwrap();
// Create an existing link for this user and provider with a different subject
let old_link = repo
.upstream_oauth_link()
.add(
&mut rng,
&state.clock,
&provider,
old_subject.to_owned(),
None,
)
.await
.unwrap();
repo.upstream_oauth_link()
.associate_to_user(&old_link, &user)
.await
.unwrap();
// Provision upstream authorization session to setup cookies
let (link, session) = add_linked_upstream_session(
&mut rng,
&state.clock,
&mut repo,
&provider,
subject,
&id_token.into_string(),
id_token_claims,
)
.await
.unwrap();
repo.save().await.unwrap();
let cookie_jar = state.cookie_jar();
let upstream_sessions = UpstreamSessionsCookie::default()
.add(session.id, provider.id, "state".to_owned(), None)
.add_link_to_session(session.id, link.id)
.unwrap();
let cookie_jar = upstream_sessions.save(cookie_jar, &state.clock);
cookies.import(cookie_jar);
let request = Request::get(&*mas_router::UpstreamOAuth2Link::new(link.id).path()).empty();
let request = cookies.with_cookies(request);
let response = state.request(request).await;
cookies.save_cookies(&response);
// Should return an error page because the user already has a link for this
// provider
response.assert_status(StatusCode::OK);
response.assert_header_value(CONTENT_TYPE, "text/html; charset=utf-8");
// Verify the error message is displayed
assert!(response.body().contains("User exists"));
assert!(response.body().contains("replacing upstream account links"));
// Check that the new link was NOT associated with the existing user
let mut repo = state.repository().await.unwrap();
let new_link = repo
.upstream_oauth_link()
.find_by_subject(&provider, subject)
.await
.unwrap()
.expect("new link exists");
// The new link should still not be associated with the user
assert_eq!(new_link.user_id, None);
// Check that the old link is still there
let old_link_result = repo
.upstream_oauth_link()
.find_by_subject(&provider, old_subject)
.await
.unwrap();
assert!(old_link_result.is_some(), "Old link should still exist");
assert_eq!(old_link_result.unwrap().user_id, Some(user.id));
}
}

14
docs/config.schema.json
View File

@@ -2576,14 +2576,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"
}
]
},

4
docs/reference/configuration.md
View File

@@ -788,8 +788,10 @@ upstream_oauth2:
# How to handle when localpart already exists.
# Possible values are (default: fail):
# - `add` : Adds the upstream account link to the existing user, regardless of whether there is an existing link or not.
# - `fail` : Fails the upstream OAuth 2.0 login.
# - `add` : Adds the upstream account link to the existing user, regardless of whether there is an existing link or not.
# - `replace` : Replace any existing upstream OAuth 2.0 identity link for this provider on the matching user.
# - `set` : Adds the upstream account link *only* if there is no existing link for this provider on the matching user.
#on_conflict: fail
# The display name is the user's display name.

25
docs/setup/sso.md
View File

@@ -69,20 +69,25 @@ The template has the following variables available:
## Allow linking existing user accounts
The authentication service supports linking external provider identities to existing local user accounts.
The authentication service supports linking external provider identities to existing local user accounts if the `localpart` matches.
To enable this behavior, the following option must be explicitly set in the provider configuration:
If the `localpart` given by the upstream provider matches an existing user and the `claims_imports.localpart.action` is set to `force` or `require`, by default the service will refuse to link to that existing account.
This behaviour is controlled by the `claims_imports.localpart.on_conflict` option, which can be set to:
* `fail` *(default)*: fails the upstream OAuth 2.0 login
* `add`: automatically adds the upstream account to the existing user, regardless of whether the existing user already has another upstream account or not
* `set`: automatically adds the upstream account to the existing user only if there are no other upstream accounts for that provider linked to the user
* `replace`: automatically replaces any upstream account for that provider linked to the user
```yaml
claims_imports:
localpart:
on_conflict: add
upstream_oauth2:
providers:
- id: …
claims_imports:
localpart:
action: force
on_conflict: set
```
`on_conflict` configuration is specific to `localpart` claim_imports, it can be either:
* `add` : when a user authenticates with the provider for the first time, the system checks whether a local user already exists with a `localpart` matching the attribute mapping `localpart` , _by default `{{ user.preferred_username }}`_. If a match is found, the external identity is linked to the existing local account.
* `fail` *(default)* : fails the sso login.
To enable this option, the `localpart` mapping must be set to either `force` or `require`.
> ⚠️ **Security Notice**
> Enabling this option can introduce a risk of account takeover.