editoast: migrate infra grant revoking to authz::v2

editoast/authz/Cargo.toml

@@ -22,6 +22,7 @@ utoipa.workspace = true
 common.workspace = true
 fga = { workspace = true, features = ["test_utilities"] }
 pretty_assertions.workspace = true
+rstest.workspace = true
 stdext.workspace = true
 tokio.workspace = true
 

editoast/authz/src/authorizer.rs

@@ -93,16 +93,6 @@ impl<S: StorageDriver> Authorizer<S> {
             .give_infra_grant(&User(self.user_id), subject, infra, grant)
             .await
     }
-
-    pub async fn revoke_infra_grants(
-        &self,
-        subject: &Subject,
-        infra: &Infra,
-    ) -> Result<Authorization<()>, Error<S::Error>> {
-        self.regulator
-            .revoke_infra_grants(&User(self.user_id), subject, infra)
-            .await
-    }
 }
 
 impl<S: StorageDriver> std::fmt::Debug for Authorizer<S> {

editoast/authz/src/model.rs

@@ -38,22 +38,6 @@ impl Subject {
             Subject::Group(group) => group.0,
         }
     }
-
-    pub(crate) async fn fetch<'a, T, E, Ureq, Greq>(
-        &'a self,
-        client: &fga::Client,
-        u: impl FnOnce(&'a User) -> Ureq,
-        g: impl FnOnce(&'a Group) -> Greq,
-    ) -> Result<T, E>
-    where
-        Ureq: fga::client::Request<Response = T, Error = E>,
-        Greq: fga::client::Request<Response = T, Error = E>,
-    {
-        match self {
-            Subject::User(user) => u(user).fetch(client).await,
-            Subject::Group(group) => g(group).fetch(client).await,
-        }
-    }
 }
 
 #[cfg(test)]

editoast/authz/src/regulator.rs

@@ -275,7 +275,10 @@ impl<S: StorageDriver> Regulator<S> {
         }
 
         // Remove existing grants before adding the new one
-        self.revoke_infra_grants_unchecked(subject, infra).await?;
+        let authorize = v2::special_authorizers::Authorize(&self.openfga);
+        authorize
+            .access_value(v2::infra_revoke_grant(*subject, *infra))
+            .await?;
 
         // Grant the new one
         let mut writes = self.openfga.prepare_writes();
@@ -387,132 +390,6 @@ impl<S: StorageDriver> Regulator<S> {
             })
             .await
     }
-
-    #[tracing::instrument(skip(self), ret(level = Level::DEBUG), err)]
-    pub async fn revoke_infra_grants(
-        &self,
-        issuer: &User,
-        subject: &Subject,
-        infra: &Infra,
-    ) -> Result<Authorization<()>, Error<S::Error>> {
-        // TODO: add a can_revoke privilege in the authorization model
-
-        // Revoking rules:
-        // 1. Only owners (and admins) can fully revoke grants
-        // 2. The last owner of a resource cannot be revoked, even by admins
-        // 3. An owner cannot revoke another owner
-
-        let is_subject_owner = match subject {
-            Subject::User(user) => {
-                self.openfga
-                    .check(Infra::owner().check(user, infra))
-                    .await?
-            }
-            Subject::Group(group) => {
-                self.openfga
-                    .check(Infra::owner().check(Group::member().userset(group), infra))
-                    .await?
-            }
-        };
-
-        if is_subject_owner {
-            let authorize = crate::v2::special_authorizers::Authorize(&self.openfga);
-            let current_owners = authorize
-                .access_value(crate::v2::infra_granted_subjects(*infra, InfraGrant::Owner))
-                .await?;
-            if current_owners.len() == 1 && current_owners.contains(subject) {
-                return Ok(Authorization::Denied {
-                    reason: "cannot remove the last owner from infrastructure",
-                });
-            }
-        }
-
-        if !self.is_admin(issuer).await? {
-            let is_issuer_owner = self
-                .openfga
-                .check(Infra::owner().check(issuer, infra))
-                .await?;
-
-            if !is_issuer_owner {
-                return Ok(Authorization::Denied {
-                    reason: "only owners can revoke grants",
-                });
-            }
-
-            // Rule 3: An owner cannot revoke another owner (only admins can)
-            if is_issuer_owner && is_subject_owner {
-                return Ok(Authorization::Denied {
-                    reason: "owner cannot revoke another owner",
-                });
-            }
-        }
-
-        self.revoke_infra_grants_unchecked(subject, infra).await?;
-        Ok(Authorization::Granted(()))
-    }
-
-    #[tracing::instrument(skip(self), ret(level = Level::DEBUG), err)]
-    pub async fn revoke_infra_grants_unchecked(
-        &self,
-        subject: &Subject,
-        infra: &Infra,
-    ) -> Result<(), Error<S::Error>> {
-        // No need to check if the infra exists. If it doesn't, there won't be any tuples in OpenFGA.
-        // And even if there is, we're about to remove them anyway.
-        // Likewise about both users.
-
-        let mut delete = self.openfga.prepare_deletes();
-
-        if subject
-            .fetch(
-                &self.openfga,
-                |user| Infra::reader().tuple(user, infra),
-                |group| Infra::reader().tuple(Group::member().userset(group), infra),
-            )
-            .await?
-        {
-            match subject {
-                Subject::User(user) => delete.push(&Infra::reader().tuple(user, infra)),
-                Subject::Group(group) => {
-                    delete.push(&Infra::reader().tuple(Group::member().userset(group), infra))
-                }
-            }
-        }
-
-        if subject
-            .fetch(
-                &self.openfga,
-                |user| Infra::writer().tuple(user, infra),
-                |group| Infra::writer().tuple(Group::member().userset(group), infra),
-            )
-            .await?
-        {
-            match subject {
-                Subject::User(user) => delete.push(&Infra::writer().tuple(user, infra)),
-                Subject::Group(group) => {
-                    delete.push(&Infra::writer().tuple(Group::member().userset(group), infra))
-                }
-            }
-        }
-
-        if subject
-            .fetch(
-                &self.openfga,
-                |user| Infra::owner().tuple(user, infra),
-                |group| Infra::owner().tuple(Group::member().userset(group), infra),
-            )
-            .await?
-        {
-            match subject {
-                Subject::User(user) => delete.push(&Infra::owner().tuple(user, infra)),
-                Subject::Group(group) => {
-                    delete.push(&Infra::owner().tuple(Group::member().userset(group), infra))
-                }
-            }
-        }
-        delete.execute().await?;
-        Ok(())
-    }
 }
 
 #[cfg(test)]
@@ -733,98 +610,4 @@ mod tests {
             .assert_infra_grant_eq(bob, infra, Some(InfraGrant::Reader))
             .await;
     }
-
-    // REVOKING TESTS
-
-    #[tokio::test(flavor = "multi_thread")]
-    async fn only_owners_can_revoke() {
-        let regulator = Regulator::new(crate::authz_client!(), MockAuthDriver::default());
-        let infra = Infra(1);
-        let alice = regulator.alice().await;
-        let bob = regulator.bob().await;
-
-        regulator
-            .set_infra_grant(alice, infra, InfraGrant::Writer)
-            .await;
-        regulator
-            .set_infra_grant(bob, infra, InfraGrant::Reader)
-            .await;
-
-        regulator
-            .revoke_infra_grants(&alice, &bob.into(), &infra)
-            .await
-            .expect("revoke operation should complete")
-            .expect_denied("only owners can revoke grants");
-
-        regulator
-            .assert_infra_grant_eq(bob, infra, Some(InfraGrant::Reader))
-            .await;
-    }
-
-    #[tokio::test(flavor = "multi_thread")]
-    async fn admins_can_revoke() {
-        let regulator = Regulator::new(crate::authz_client!(), MockAuthDriver::default());
-        let infra = Infra(1);
-        let alice = regulator.alice().await;
-        let walter = regulator.walter().await;
-
-        regulator
-            .set_infra_grant(alice, infra, InfraGrant::Reader)
-            .await;
-
-        regulator
-            .revoke_infra_grants(&walter, &alice.into(), &infra)
-            .await
-            .expect("revoke operation should complete")
-            .expect_allowed("admin can revoke grants");
-
-        regulator.assert_infra_grant_eq(alice, infra, None).await;
-    }
-
-    #[tokio::test(flavor = "multi_thread")]
-    async fn last_owner_cannot_be_revoked_by_admin() {
-        let regulator = Regulator::new(crate::authz_client!(), MockAuthDriver::default());
-        let infra = Infra(1);
-        let alice = regulator.alice().await;
-        let walter = regulator.walter().await;
-
-        regulator
-            .set_infra_grant(alice, infra, InfraGrant::Owner)
-            .await;
-
-        regulator
-            .revoke_infra_grants(&walter, &alice.into(), &infra)
-            .await
-            .expect("revoke operation should complete")
-            .expect_denied("last owner cannot be revoked");
-
-        regulator
-            .assert_infra_grant_eq(alice, infra, Some(InfraGrant::Owner))
-            .await;
-    }
-
-    #[tokio::test(flavor = "multi_thread")]
-    async fn owner_cannot_revoke_another_owner() {
-        let regulator = Regulator::new(crate::authz_client!(), MockAuthDriver::default());
-        let infra = Infra(1);
-        let alice = regulator.alice().await;
-        let bob = regulator.bob().await;
-
-        regulator
-            .set_infra_grant(alice, infra, InfraGrant::Owner)
-            .await;
-        regulator
-            .set_infra_grant(bob, infra, InfraGrant::Owner)
-            .await;
-
-        regulator
-            .revoke_infra_grants(&alice, &bob.into(), &infra)
-            .await
-            .expect("revoke operation should complete")
-            .expect_denied("owner cannot revoke another owner");
-
-        regulator
-            .assert_infra_grant_eq(bob, infra, Some(InfraGrant::Owner))
-            .await;
-    }
 }

editoast/authz/src/v2.rs

@@ -15,6 +15,7 @@ use futures::future::BoxFuture;
 
 use crate::Group;
 use crate::Infra;
+use crate::InfraGrant;
 use crate::InfraPrivilege;
 use crate::Role;
 use crate::Subject;
@@ -59,6 +60,10 @@ pub enum Check {
     HasRole(Actor, Role),
     /// The actor needs an infra privilege to perform the operation
     HasInfraPrivilege(Actor, InfraPrivilege, Infra),
+    /// The subject must not have the specified effective infra grant
+    SubjectEffectiveInfraGrantIsNot(InfraGrant, Subject, Infra),
+    /// The subject must not be the last direct owner of the infra
+    IsntLastInfraOwner(Subject, Infra),
     /// The subject must exist in PostgreSQL
     SubjectExists(Subject),
     /// The infra must exist in PostgreSQL
@@ -317,3 +322,22 @@ pub mod special_authorizers {
         }
     }
 }
+
+impl<L, R, Rejection, Error> Authorizer for itertools::Either<L, R>
+where
+    L: Authorizer<Rejection = Rejection, Error = Error>,
+    R: Authorizer<Rejection = Rejection, Error = Error>,
+{
+    type Rejection = Rejection;
+    type Error = Error;
+
+    async fn authorize<'a, T>(
+        &'a self,
+        data: Protected<T>,
+    ) -> Result<Access<'a, T, Self::Rejection>, Self::Error> {
+        match self {
+            itertools::Either::Left(l) => l.authorize(data).await,
+            itertools::Either::Right(r) => r.authorize(data).await,
+        }
+    }
+}