diff --git a/server/src/handlers/http/oidc.rs b/server/src/handlers/http/oidc.rs index 55d1d24f6..a2d277cca 100644 --- a/server/src/handlers/http/oidc.rs +++ b/server/src/handlers/http/oidc.rs @@ -36,7 +36,7 @@ use crate::{ option::CONFIG, rbac::{ map::{SessionKey, DEFAULT_ROLE}, - user::{User, UserType}, + user::{self, User, UserType}, Users, }, storage::{self, ObjectStorageError, StorageMetadata}, @@ -138,19 +138,31 @@ pub async fn reply_login( else { return Ok(HttpResponse::Unauthorized().finish()); }; - let username = user_info.sub.unwrap(); - let group: Option> = claims + let username = user_info + .sub + .clone() + .expect("OIDC provider did not return a sub which is currently required."); + let user_info: user::UserInfo = user_info.into(); + + let group: HashSet = claims .other .remove("groups") .map(serde_json::from_value) - .transpose()?; + .transpose()? + .unwrap_or_else(|| { + 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) { - (Some(user), Some(group)) => update_user_if_changed(user, group).await?, - (Some(user), None) => user, - (None, group) => put_user(&username, group).await?, + (Some(user), group) => update_user_if_changed(user, group, user_info).await?, + (None, group) => put_user(&username, group, user_info).await?, }; let id = Ulid::new(); Users.new_session(&user, SessionKey::SessionId(id)); @@ -257,17 +269,10 @@ async fn request_token( // update local cache async fn put_user( username: &str, - group: Option>, + group: HashSet, + user_info: user::UserInfo, ) -> Result { let mut metadata = get_metadata().await?; - let group = group.unwrap_or_else(|| { - DEFAULT_ROLE - .lock() - .unwrap() - .clone() - .map(|role| HashSet::from([role])) - .unwrap_or_default() - }); let user = metadata .users @@ -275,7 +280,7 @@ async fn put_user( .find(|user| user.username() == username) .cloned() .unwrap_or_else(|| { - let user = User::new_oauth(username.to_owned(), group); + let user = User::new_oauth(username.to_owned(), group, user_info); metadata.users.push(user.clone()); user }); @@ -288,14 +293,32 @@ async fn put_user( async fn update_user_if_changed( mut user: User, group: HashSet, + user_info: user::UserInfo, ) -> Result { - // update user if roles have changed - if user.roles == group { + let User { ty, roles } = &mut user; + let UserType::OAuth(oauth_user) = ty else { + unreachable!() + }; + + // update user only if roles or userinfo has changed + if roles == &group && oauth_user.user_info == user_info { return Ok(user); } - let metadata = get_metadata().await?; - user.roles = group; - put_metadata(&metadata).await?; + + oauth_user.user_info = user_info; + *roles = group; + + let mut metadata = get_metadata().await?; + + if let Some(entry) = metadata + .users + .iter_mut() + .find(|x| x.username() == user.username()) + { + entry.clone_from(&user); + put_metadata(&metadata).await?; + } + Users.put_user(user.clone()); Ok(user) } diff --git a/server/src/rbac/map.rs b/server/src/rbac/map.rs index 7afac7090..d76c64b59 100644 --- a/server/src/rbac/map.rs +++ b/server/src/rbac/map.rs @@ -152,10 +152,8 @@ impl Sessions { permissions: Vec, ) { self.remove_expired_session(&user); - self.user_sessions - .entry(user.clone()) - .and_modify(|sessions| sessions.push((key.clone(), expiry))) - .or_default(); + let sessions = self.user_sessions.entry(user.clone()).or_default(); + sessions.push((key.clone(), expiry)); self.active_sessions.insert(key, (user, permissions)); } @@ -220,7 +218,7 @@ impl Sessions { }; (action == required_action || action == Action::All) && ok_stream } - Permission::SelfRole if required_action == Action::GetUserRoles => { + Permission::SelfUser if required_action == Action::GetUserRoles => { context_user.map(|x| x == username).unwrap_or_default() } _ => false, diff --git a/server/src/rbac/role.rs b/server/src/rbac/role.rs index 0b8f70f52..968e58e34 100644 --- a/server/src/rbac/role.rs +++ b/server/src/rbac/role.rs @@ -50,7 +50,7 @@ pub enum Permission { Unit(Action), Stream(Action, String), StreamWithTag(Action, String, Option), - SelfRole, + SelfUser, } // Currently Roles are tied to one stream @@ -77,37 +77,37 @@ impl RoleBuilder { let mut perms = Vec::new(); for action in self.actions { let perm = match action { - Action::Ingest => Permission::Stream(action, self.stream.clone().unwrap()), Action::Query => Permission::StreamWithTag( action, self.stream.clone().unwrap(), self.tag.clone(), ), - Action::CreateStream => Permission::Unit(action), - Action::DeleteStream => Permission::Unit(action), - Action::ListStream => Permission::Unit(action), - Action::GetSchema => Permission::Stream(action, self.stream.clone().unwrap()), - Action::GetStats => Permission::Stream(action, self.stream.clone().unwrap()), - Action::GetRetention => Permission::Stream(action, self.stream.clone().unwrap()), - Action::PutRetention => Permission::Stream(action, self.stream.clone().unwrap()), - Action::PutAlert => Permission::Stream(action, self.stream.clone().unwrap()), - Action::GetAlert => Permission::Stream(action, self.stream.clone().unwrap()), - Action::PutUser => Permission::Unit(action), - Action::ListUser => Permission::Unit(action), - Action::PutUserRoles => Permission::Unit(action), - Action::GetUserRoles => Permission::Unit(action), - Action::DeleteUser => Permission::Unit(action), - Action::GetAbout => Permission::Unit(action), - Action::QueryLLM => Permission::Unit(action), - Action::PutRole => Permission::Unit(action), - Action::GetRole => Permission::Unit(action), - Action::DeleteRole => Permission::Unit(action), - Action::ListRole => Permission::Unit(action), - Action::All => Permission::Stream(action, self.stream.clone().unwrap()), + Action::PutUser + | Action::ListUser + | Action::PutUserRoles + | Action::GetUserRoles + | Action::DeleteUser + | Action::GetAbout + | Action::QueryLLM + | Action::PutRole + | Action::GetRole + | Action::DeleteRole + | Action::ListRole + | Action::CreateStream + | Action::DeleteStream + | Action::ListStream => Permission::Unit(action), + Action::Ingest + | Action::GetSchema + | Action::GetStats + | Action::GetRetention + | Action::PutRetention + | Action::PutAlert + | Action::GetAlert + | Action::All => Permission::Stream(action, self.stream.clone().unwrap()), }; perms.push(perm); } - perms.push(Permission::SelfRole); + perms.push(Permission::SelfUser); perms } } diff --git a/server/src/rbac/user.rs b/server/src/rbac/user.rs index 533976012..0dc188b1d 100644 --- a/server/src/rbac/user.rs +++ b/server/src/rbac/user.rs @@ -57,9 +57,12 @@ impl User { ) } - pub fn new_oauth(username: String, roles: HashSet) -> Self { + pub fn new_oauth(username: String, roles: HashSet, user_info: UserInfo) -> Self { Self { - ty: UserType::OAuth(OAuth { userid: username }), + ty: UserType::OAuth(OAuth { + userid: username, + user_info, + }), roles, } } @@ -69,6 +72,7 @@ impl User { UserType::Native(Basic { ref username, .. }) => username, UserType::OAuth(OAuth { userid: ref username, + .. }) => username, } } @@ -148,5 +152,36 @@ pub fn get_admin_user() -> User { #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub struct OAuth { - userid: String, + pub userid: String, + pub user_info: UserInfo, +} + +#[derive(Debug, Default, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub struct UserInfo { + #[serde(default)] + /// User's full name for display purposes. + pub name: Option, + #[serde(default)] + pub preferred_username: Option, + #[serde(default)] + pub picture: Option, + #[serde(default)] + pub email: Option, + #[serde(default)] + pub gender: Option, + #[serde(default)] + pub updated_at: Option, +} + +impl From for UserInfo { + fn from(user: openid::Userinfo) -> Self { + UserInfo { + name: user.name, + preferred_username: user.preferred_username, + picture: user.picture, + email: user.email, + gender: user.gender, + updated_at: user.updated_at, + } + } }