Skip to content

Commit b155f6e

Browse files
committed
fix: preserve existing user roles and incorporate valid OIDC roles during login
1 parent 70ba7df commit b155f6e

File tree

1 file changed

+30
-28
lines changed

1 file changed

+30
-28
lines changed

src/handlers/http/oidc.rs

Lines changed: 30 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -159,46 +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-
180-
// Check if user already exists to preserve their existing roles
178+
181179
let existing_user = Users.get_user(&username);
182-
if !role_exists || group.is_empty() {
183-
group = if let Some(existing_user) = &existing_user {
184-
// Preserve existing user roles instead of assigning default
185-
existing_user.roles.clone()
186-
} else {
187-
// Only assign default role for new users
188-
DEFAULT_ROLE
189-
.lock()
190-
.unwrap()
191-
.clone()
192-
.map(|role| HashSet::from([role]))
193-
.unwrap_or_default()
194-
};
195-
}
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+
};
196200

197-
// User may not exist
198-
// create a new one depending on state of metadata
199-
let user = match (existing_user, group) {
200-
(Some(user), group) => update_user_if_changed(user, group, user_info).await?,
201-
(None, group) => put_user(&username, group, user_info).await?,
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?,
202204
};
203205
let id = Ulid::new();
204206
Users.new_session(&user, SessionKey::SessionId(id));

0 commit comments

Comments
 (0)