Skip to content

Commit ca37efd

Browse files
authored
fix(OIDC): preserve existing OAuth user roles instead of resetting to default (#1356)
- 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
1 parent 51d8fcd commit ca37efd

File tree

1 file changed

+31
-20
lines changed

1 file changed

+31
-20
lines changed

src/handlers/http/oidc.rs

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -159,37 +159,48 @@ pub async fn reply_login(
159159
.clone()
160160
.expect("OIDC provider did not return a sub which is currently required.");
161161
let user_info: user::UserInfo = user_info.into();
162-
let mut group: HashSet<String> = claims
162+
let group: HashSet<String> = claims
163163
.other
164164
.remove("groups")
165165
.map(serde_json::from_value)
166166
.transpose()?
167167
.unwrap_or_default();
168168
let metadata = get_metadata().await?;
169-
let mut role_exists = false;
169+
170+
// Find which OIDC groups match existing roles in Parseable
171+
let mut valid_oidc_roles = HashSet::new();
170172
for role in metadata.roles.iter() {
171173
let role_name = role.0;
172-
for group_name in group.iter() {
173-
if group_name.eq(role_name) {
174-
role_exists = true;
175-
break;
176-
}
174+
if group.contains(role_name) {
175+
valid_oidc_roles.insert(role_name.clone());
177176
}
178177
}
179-
if !role_exists || group.is_empty() {
180-
group = DEFAULT_ROLE
181-
.lock()
182-
.unwrap()
183-
.clone()
184-
.map(|role| HashSet::from([role]))
185-
.unwrap_or_default();
186-
}
187178

188-
// User may not exist
189-
// create a new one depending on state of metadata
190-
let user = match (Users.get_user(&username), group) {
191-
(Some(user), group) => update_user_if_changed(user, group, user_info).await?,
192-
(None, group) => put_user(&username, group, user_info).await?,
179+
let existing_user = Users.get_user(&username);
180+
let final_roles = match existing_user {
181+
Some(ref user) => {
182+
// For existing users: keep existing roles + add new valid OIDC roles
183+
let mut roles = user.roles.clone();
184+
roles.extend(valid_oidc_roles); // Add new matching roles
185+
roles
186+
}
187+
None => {
188+
// For new users: use valid OIDC roles, fallback to default if none
189+
if valid_oidc_roles.is_empty() {
190+
if let Some(default_role) = DEFAULT_ROLE.lock().unwrap().clone() {
191+
HashSet::from([default_role])
192+
} else {
193+
HashSet::new()
194+
}
195+
} else {
196+
valid_oidc_roles
197+
}
198+
}
199+
};
200+
201+
let user = match (existing_user, final_roles) {
202+
(Some(user), roles) => update_user_if_changed(user, roles, user_info).await?,
203+
(None, roles) => put_user(&username, roles, user_info).await?,
193204
};
194205
let id = Ulid::new();
195206
Users.new_session(&user, SessionKey::SessionId(id));

0 commit comments

Comments
 (0)