From ee8a74d48e21661b22352f9f1bf134efd562652e Mon Sep 17 00:00:00 2001
From: Florian Amsallem <florian.amsallem@gmail.com>
Date: Thu, 7 Nov 2024 11:38:24 +0100
Subject: [PATCH] editoast: fix grant and revoke role to group using CLI

Signed-off-by: Florian Amsallem <florian.amsallem@gmail.com>
---
 editoast/editoast_authz/src/authorizer.rs | 15 +++++++
 editoast/src/client/roles.rs              | 51 +++++++++++++++++------
 editoast/src/models/auth.rs               | 25 ++++++++---
 3 files changed, 73 insertions(+), 18 deletions(-)

diff --git a/editoast/editoast_authz/src/authorizer.rs b/editoast/editoast_authz/src/authorizer.rs
index 7a3df9d76ec..93de37ba827 100644
--- a/editoast/editoast_authz/src/authorizer.rs
+++ b/editoast/editoast_authz/src/authorizer.rs
@@ -7,6 +7,7 @@ use crate::roles::BuiltinRoleSet;
 
 pub type UserIdentity = String;
 pub type UserName = String;
+pub type GroupName = String;
 
 #[derive(Debug, Clone)]
 pub struct UserInfo {
@@ -14,6 +15,11 @@ pub struct UserInfo {
     pub name: UserName,
 }
 
+#[derive(Debug, Clone)]
+pub struct GroupInfo {
+    pub name: GroupName,
+}
+
 #[derive(Clone)]
 pub struct Authorizer<S: StorageDriver> {
     user: UserInfo,
@@ -37,6 +43,11 @@ pub trait StorageDriver: Clone {
         user_id: i64,
     ) -> impl Future<Output = Result<Option<UserInfo>, Self::Error>> + Send;
 
+    fn get_group_info(
+        &self,
+        group_id: i64,
+    ) -> impl Future<Output = Result<Option<GroupInfo>, Self::Error>> + Send;
+
     fn ensure_user(&self, user: &UserInfo)
         -> impl Future<Output = Result<i64, Self::Error>> + Send;
 
@@ -339,5 +350,9 @@ mod tests {
                 });
             Ok(user_info)
         }
+
+        async fn get_group_info(&self, _group_id: i64) -> Result<Option<GroupInfo>, Self::Error> {
+            Ok(None)
+        }
     }
 }
diff --git a/editoast/src/client/roles.rs b/editoast/src/client/roles.rs
index f9b67f98e13..fbe465c3d69 100644
--- a/editoast/src/client/roles.rs
+++ b/editoast/src/client/roles.rs
@@ -3,7 +3,7 @@ use std::{collections::HashSet, fmt::Display, sync::Arc};
 use anyhow::{anyhow, bail};
 use clap::{Args, Subcommand};
 use editoast_authz::{
-    authorizer::{StorageDriver, UserInfo},
+    authorizer::{GroupInfo, StorageDriver, UserInfo},
     roles::BuiltinRoleSet,
     BuiltinRole,
 };
@@ -55,16 +55,41 @@ pub fn list_roles() {
 #[derive(Debug)]
 struct Subject {
     id: i64,
-    info: UserInfo,
+    info: SubjectInfo,
+}
+impl Subject {
+    /// Create a new subject representing a user
+    pub fn new_user(id: i64, info: UserInfo) -> Self {
+        Self {
+            id,
+            info: SubjectInfo::User(info),
+        }
+    }
+
+    /// Create a new subject representing a group
+    pub fn new_group(id: i64, info: GroupInfo) -> Self {
+        Self {
+            id,
+            info: SubjectInfo::Group(info),
+        }
+    }
+}
+
+#[derive(Debug)]
+enum SubjectInfo {
+    User(UserInfo),
+    Group(GroupInfo),
 }
 
 impl Display for Subject {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let Self {
-            id,
-            info: UserInfo { identity, name },
-        } = self;
-        write!(f, "{identity}#{id} ({name})")
+        let Self { id, info } = self;
+        match info {
+            SubjectInfo::User(UserInfo { name, identity }) => {
+                write!(f, "User {}#{} ({})", identity, id, name)
+            }
+            SubjectInfo::Group(info) => write!(f, "Group #{} ({})", id, info.name),
+        }
     }
 }
 
@@ -78,13 +103,15 @@ async fn parse_and_fetch_subject(
         let uid = driver.get_user_id(subject).await?;
         uid.ok_or_else(|| anyhow!("No subject with identity '{subject}' found"))?
     };
-    if let Some(info) = driver.get_user_info(id).await? {
-        let subject = Subject { id, info };
-        info!("Subject {subject}");
-        Ok(subject)
+    let subject = if let Some(info) = driver.get_user_info(id).await? {
+        Subject::new_user(id, info)
+    } else if let Some(info) = driver.get_group_info(id).await? {
+        Subject::new_group(id, info)
     } else {
         bail!("No subject found with ID {id}");
-    }
+    };
+    info!("{subject}");
+    Ok(subject)
 }
 
 pub async fn list_subject_roles(
diff --git a/editoast/src/models/auth.rs b/editoast/src/models/auth.rs
index 9dfb4d59961..cf2c19a7898 100644
--- a/editoast/src/models/auth.rs
+++ b/editoast/src/models/auth.rs
@@ -4,7 +4,7 @@ use std::{collections::HashSet, sync::Arc};
 use diesel::{dsl, prelude::*};
 use diesel_async::{scoped_futures::ScopedFutureExt as _, RunQueryDsl};
 use editoast_authz::{
-    authorizer::{StorageDriver, UserIdentity, UserInfo},
+    authorizer::{GroupInfo, StorageDriver, UserIdentity, UserInfo},
     roles::BuiltinRoleSet,
 };
 use editoast_models::DbConnectionPoolV2;
@@ -60,13 +60,26 @@ impl<B: BuiltinRoleSet + Send + Sync> StorageDriver for PgAuthDriver<B> {
             .filter(authn_user::id.eq(user_id))
             .first::<(String, Option<String>)>(conn.write().await.deref_mut())
             .await
-            .optional()
-            .map(|res| {
-                res.map(|(identity, name)| UserInfo {
+            .optional()?
+            .map(|(identity, name)| {
+                UserInfo {
                     identity,
                     name: name.unwrap_or_default(), // FIXME: make the column non-nullable
-                })
-            })?;
+                }
+            });
+        Ok(info)
+    }
+
+    #[tracing::instrument(skip_all, fields(%group_id), ret(level = Level::DEBUG), err)]
+    async fn get_group_info(&self, group_id: i64) -> Result<Option<GroupInfo>, Self::Error> {
+        let conn = self.pool.get().await?;
+        let info = authn_group::table
+            .select(authn_group::name)
+            .filter(authn_group::id.eq(group_id))
+            .first::<String>(conn.write().await.deref_mut())
+            .await
+            .optional()?
+            .map(|name| GroupInfo { name });
         Ok(info)
     }