Skip to content

editoast: authz: remove StorageDriver::get_group_id #16705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
leovalais wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

editoast: authz: remove StorageDriver::get_group_id #16705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions editoast/authz/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,6 @@ mod mock_driver {
Ok(self.users.lock().unwrap().get(user_identity).copied())
}

async fn get_group_id(&self, group_name: &GroupName) -> Result<Option<i64>, Self::Error> {
Ok(self.groups.lock().unwrap().get(group_name).copied())
}

async fn get_user_info(&self, user_id: i64) -> Result<Option<UserInfo>, Self::Error> {
let users = self.users.lock().unwrap();
let identities = users
Expand Down
6 changes: 0 additions & 6 deletions editoast/authz/src/regulator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use crate::Authorization;
use crate::Error;
use crate::Role;
use crate::identity::GroupInfo;
use crate::identity::GroupName;
use crate::identity::User as UserSubject;
use crate::identity::UserIdentity;
use crate::identity::UserInfo;
Expand Down Expand Up @@ -39,11 +38,6 @@ pub trait StorageDriver: Clone {
user_identity: &UserIdentity,
) -> impl Future<Output = Result<Option<i64>, Self::Error>> + Send;

fn get_group_id(
&self,
group_name: &GroupName,
) -> impl Future<Output = Result<Option<i64>, Self::Error>> + Send;

fn get_user_info(
&self,
user_id: i64,
Expand Down
13 changes: 0 additions & 13 deletions editoast/editoast_models/src/auth_driver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::sync::Arc;

use authz::StorageDriver;
use authz::identity::GroupInfo;
use authz::identity::GroupName;
use authz::identity::User;
use authz::identity::UserIdentity;
use authz::identity::UserInfo;
Expand Down Expand Up @@ -107,18 +106,6 @@ impl StorageDriver for PgAuthDriver {
Ok(id)
}

#[tracing::instrument(skip_all, fields(%group_name), ret(level = Level::DEBUG), err)]
async fn get_group_id(&self, group_name: &GroupName) -> Result<Option<i64>, Self::Error> {
let conn = self.pool.get().await?;
let id = authn_group::table
.select(authn_group::id)
.filter(authn_group::name.eq(group_name))
.first::<i64>(conn.write().await.deref_mut())
.await
.optional()?;
Ok(id)
}

#[tracing::instrument(skip_all, fields(%user_id), ret(level = Level::DEBUG), err)]
async fn get_user_info(&self, user_id: i64) -> Result<Option<UserInfo>, Self::Error> {
let info = crate::authn::user::User::get_batch_user_identities(
Expand Down
35 changes: 16 additions & 19 deletions editoast/src/client/group.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use clap::Subcommand;

use authz;
use authz::StorageDriver;
use authz::identity::GroupInfo;
use authz::identity::UserInfo;

use database::DbConnectionPoolV2;
Expand Down Expand Up @@ -100,16 +99,14 @@ pub async fn group_info(
openfga_config: OpenfgaConfig,
pool: Arc<DbConnectionPoolV2>,
) -> anyhow::Result<()> {
let regulator = openfga_config.into_regulator(pool).await?;
let regulator = openfga_config.into_regulator(pool.clone()).await?;
let driver = regulator.driver();
let Some(group_id) = driver.get_group_id(&name).await? else {
let Some(editoast_models::Group { id: group_id, name }) =
editoast_models::Group::retrieve(pool.get().await?, name.clone()).await?
else {
tracing::error!(name, "No such group");
return Ok(());
};
let Some(GroupInfo { name }) = driver.get_group_info(group_id).await? else {
tracing::error!(group.id = group_id, "No such group");
return Ok(());
};
let user_ids = regulator.group_members(&authz::Group(group_id)).await?;

println!("id : {group_id}");
Expand Down Expand Up @@ -142,9 +139,10 @@ pub async fn exclude_group(
conn: pool.get().await?,
};

let Some(group_id) = driver.get_group_id(&group_name).await? else {
bail!("No such group: '{group_name}'");
};
let group_id = editoast_models::Group::retrieve(pool.get().await?, group_name.clone())
.await?
.ok_or_else(|| anyhow!("No such group: '{group_name}'"))?
.id;

let mut authz_users = HashSet::new();
for user in &users {
Expand Down Expand Up @@ -183,9 +181,10 @@ pub async fn include_group(
conn: pool.get().await?,
};

let Some(group_id) = driver.get_group_id(&group_name).await? else {
bail!("No such group: '{group_name}'");
};
let group_id = editoast_models::Group::retrieve(pool.get().await?, group_name.clone())
.await?
.ok_or_else(|| anyhow!("No such group: '{group_name}'"))?
.id;

let mut authz_users = HashSet::new();
for user in &users {
Expand Down Expand Up @@ -213,17 +212,15 @@ pub async fn delete_group(
pool: Arc<DbConnectionPoolV2>,
) -> anyhow::Result<()> {
let regulator = openfga_config.into_regulator(pool.clone()).await?;
let driver = regulator.driver();
let mut conn = pool.get().await?;
let system = SystemAuthorizer {
openfga: regulator.openfga(),
conn: conn.clone(),
};
let group_id = if let Some(id) = driver.get_group_id(&name).await? {
id
} else {
anyhow::bail!("group '{name}' could not be deleted (not found)");
};
let group_id = editoast_models::Group::retrieve(pool.get().await?, name.clone())
.await?
.ok_or_else(|| anyhow!("group '{name}' could not be deleted (not found)"))?
.id;
let group = Group(group_id);

// Delete the relationships between the group to be deleted and its members
Expand Down