From 70ba7dfc9db5bb35af368573599b13c393138914 Mon Sep 17 00:00:00 2001 From: vkhinvasara Date: Tue, 24 Jun 2025 04:20:27 +0530 Subject: [PATCH 1/2] fix: preserve existing OAuth user roles instead of resetting to default - Modified reply_login function to check for existing users before role assignment - Existing OAuth users now retain their current roles when OIDC provider doesn't return valid groups - Only new users get assigned default roles when no valid groups are provided - Fixes issue where existing OAuth users had their roles reset on every login --- src/handlers/http/oidc.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/handlers/http/oidc.rs b/src/handlers/http/oidc.rs index 286f5f29c..dcfc68db7 100644 --- a/src/handlers/http/oidc.rs +++ b/src/handlers/http/oidc.rs @@ -176,18 +176,27 @@ pub async fn reply_login( } } } + + // Check if user already exists to preserve their existing roles + let existing_user = Users.get_user(&username); if !role_exists || group.is_empty() { - group = DEFAULT_ROLE - .lock() - .unwrap() - .clone() - .map(|role| HashSet::from([role])) - .unwrap_or_default(); + group = if let Some(existing_user) = &existing_user { + // Preserve existing user roles instead of assigning default + existing_user.roles.clone() + } else { + // Only assign default role for new users + DEFAULT_ROLE + .lock() + .unwrap() + .clone() + .map(|role| HashSet::from([role])) + .unwrap_or_default() + }; } // User may not exist // create a new one depending on state of metadata - let user = match (Users.get_user(&username), group) { + let user = match (existing_user, group) { (Some(user), group) => update_user_if_changed(user, group, user_info).await?, (None, group) => put_user(&username, group, user_info).await?, }; From b155f6e128d35b075983bf507000a12fe8285ce7 Mon Sep 17 00:00:00 2001 From: vkhinvasara Date: Tue, 24 Jun 2025 16:03:30 +0530 Subject: [PATCH 2/2] fix: preserve existing user roles and incorporate valid OIDC roles during login --- src/handlers/http/oidc.rs | 58 ++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/src/handlers/http/oidc.rs b/src/handlers/http/oidc.rs index dcfc68db7..1b5db874c 100644 --- a/src/handlers/http/oidc.rs +++ b/src/handlers/http/oidc.rs @@ -159,46 +159,48 @@ pub async fn reply_login( .clone() .expect("OIDC provider did not return a sub which is currently required."); let user_info: user::UserInfo = user_info.into(); - let mut group: HashSet = claims + let group: HashSet = claims .other .remove("groups") .map(serde_json::from_value) .transpose()? .unwrap_or_default(); let metadata = get_metadata().await?; - let mut role_exists = false; + + // Find which OIDC groups match existing roles in Parseable + let mut valid_oidc_roles = HashSet::new(); for role in metadata.roles.iter() { let role_name = role.0; - for group_name in group.iter() { - if group_name.eq(role_name) { - role_exists = true; - break; - } + if group.contains(role_name) { + valid_oidc_roles.insert(role_name.clone()); } } - - // Check if user already exists to preserve their existing roles + let existing_user = Users.get_user(&username); - if !role_exists || group.is_empty() { - group = if let Some(existing_user) = &existing_user { - // Preserve existing user roles instead of assigning default - existing_user.roles.clone() - } else { - // Only assign default role for new users - DEFAULT_ROLE - .lock() - .unwrap() - .clone() - .map(|role| HashSet::from([role])) - .unwrap_or_default() - }; - } + let final_roles = match existing_user { + Some(ref user) => { + // For existing users: keep existing roles + add new valid OIDC roles + let mut roles = user.roles.clone(); + roles.extend(valid_oidc_roles); // Add new matching roles + roles + } + None => { + // For new users: use valid OIDC roles, fallback to default if none + if valid_oidc_roles.is_empty() { + if let Some(default_role) = DEFAULT_ROLE.lock().unwrap().clone() { + HashSet::from([default_role]) + } else { + HashSet::new() + } + } else { + valid_oidc_roles + } + } + }; - // User may not exist - // create a new one depending on state of metadata - let user = match (existing_user, group) { - (Some(user), group) => update_user_if_changed(user, group, user_info).await?, - (None, group) => put_user(&username, group, user_info).await?, + let user = match (existing_user, final_roles) { + (Some(user), roles) => update_user_if_changed(user, roles, user_info).await?, + (None, roles) => put_user(&username, roles, user_info).await?, }; let id = Ulid::new(); Users.new_session(&user, SessionKey::SessionId(id));