From 2ad8eeaea8306c39f5528b0a0ac1093b0b61a17a Mon Sep 17 00:00:00 2001
From: Nikhil Sinha <nikhil.sinha@cloudsurfex.com>
Date: Sat, 6 Apr 2024 00:17:20 +0530
Subject: [PATCH] fix for - 1. default role not assigned to the oauth user if
 group does not exist 2. name used instead of id

fixes #638, #868
---
 server/src/handlers/http/oidc.rs | 33 +++++++++++++++++++++-----------
 server/src/rbac/user.rs          |  2 +-
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/server/src/handlers/http/oidc.rs b/server/src/handlers/http/oidc.rs
index 92c711c58..bcc749225 100644
--- a/server/src/handlers/http/oidc.rs
+++ b/server/src/handlers/http/oidc.rs
@@ -139,24 +139,35 @@ pub async fn reply_login(
         return Ok(HttpResponse::Unauthorized().finish());
     };
     let username = user_info
-        .sub
+        .name
         .clone()
         .expect("OIDC provider did not return a sub which is currently required.");
     let user_info: user::UserInfo = user_info.into();
-
-    let group: HashSet<String> = claims
+    let mut group: HashSet<String> = claims
         .other
         .remove("groups")
         .map(serde_json::from_value)
         .transpose()?
-        .unwrap_or_else(|| {
-            DEFAULT_ROLE
-                .lock()
-                .unwrap()
-                .clone()
-                .map(|role| HashSet::from([role]))
-                .unwrap_or_default()
-        });
+        .unwrap_or_default();
+    let metadata = get_metadata().await?;
+    let mut role_exists = false;
+    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 !role_exists || group.is_empty() {
+        group = 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
diff --git a/server/src/rbac/user.rs b/server/src/rbac/user.rs
index 6780c0fe9..c037a8381 100644
--- a/server/src/rbac/user.rs
+++ b/server/src/rbac/user.rs
@@ -60,7 +60,7 @@ impl User {
     pub fn new_oauth(username: String, roles: HashSet<String>, user_info: UserInfo) -> Self {
         Self {
             ty: UserType::OAuth(OAuth {
-                userid: username,
+                userid: user_info.name.clone().unwrap_or(username),
                 user_info,
             }),
             roles,