fix: Validating permission to update an existing request on both the new and the old instance

sdk/python/feast/permissions/security_manager.py

@@ -87,20 +87,19 @@ def assert_permissions(
 
 def assert_permissions_to_update(
     resource: FeastObject,
-    actions: Union[AuthzedAction, List[AuthzedAction]],
     getter: Callable[[str, str, bool], FeastObject],
     project: str,
     allow_cache: bool = True,
 ) -> FeastObject:
     """
-    Verify if the current user is authorized to execute the requested actions on the given resource and also checks the authorization on
-    the existing resource, if any.
+    Verify if the current user is authorized to create or update the given resource.
+    If the resource already exists, the user must be granted permission to execute DESCRIBE and UPDATE actions.
+    If the resource does not exist, the user must be granted permission to execute the CREATE action.
 
     If no permissions are defined, the result is to deny the execution.
 
     Args:
         resource: The resources for which we need to enforce authorized permission.
-        actions: The requested actions to be authorized.
         getter: The getter function used to retrieve the existing resource instance by name.
         The signature must be `get_permission(self, name: str, project: str, allow_cache: bool)`
         project: The project nane used in the getter function.
@@ -111,16 +110,17 @@ def assert_permissions_to_update(
     Raises:
         PermissionError: If the current user is not authorized to execute all the requested actions on the given resource or on the existing one.
     """
-    resource_to_update = assert_permissions(resource=resource, actions=actions)
+    actions = [AuthzedAction.DESCRIBE, AuthzedAction.UPDATE]
     try:
         existing_resource = getter(
-            name=resource_to_update.name,
+            name=resource.name,
             project=project,
             allow_cache=allow_cache,
         )  # type: ignore[call-arg]
         assert_permissions(resource=existing_resource, actions=actions)
     except FeastObjectNotFoundException:
-        pass
+        actions = [AuthzedAction.CREATE]
+    resource_to_update = assert_permissions(resource=resource, actions=actions)
     return resource_to_update
 
 

sdk/python/feast/registry_server.py

@@ -16,7 +16,7 @@
 from feast.infra.infra_object import Infra
 from feast.infra.registry.base_registry import BaseRegistry
 from feast.on_demand_feature_view import OnDemandFeatureView
-from feast.permissions.action import CRUD, AuthzedAction
+from feast.permissions.action import AuthzedAction
 from feast.permissions.permission import Permission
 from feast.permissions.security_manager import (
     assert_permissions,
@@ -45,7 +45,6 @@ def ApplyEntity(self, request: RegistryServer_pb2.ApplyEntityRequest, context):
             Entity,
             assert_permissions_to_update(
                 resource=Entity.from_proto(request.entity),
-                actions=CRUD,
                 getter=self.proxied_registry.get_entity,
                 project=request.project,
             ),
@@ -106,7 +105,6 @@ def ApplyDataSource(
             DataSource,
             assert_permissions_to_update(
                 resource=DataSource.from_proto(request.data_source),
-                actions=CRUD,
                 getter=self.proxied_registry.get_data_source,
                 project=request.project,
             ),
@@ -192,7 +190,6 @@ def ApplyFeatureView(
 
         assert_permissions_to_update(
             resource=feature_view,
-            actions=CRUD,
             # Will replace with the new get_any_feature_view method later
             getter=self.proxied_registry.get_feature_view,
             project=request.project,
@@ -318,20 +315,16 @@ def ListOnDemandFeatureViews(
     def ApplyFeatureService(
         self, request: RegistryServer_pb2.ApplyFeatureServiceRequest, context
     ):
-        feature_service = assert_permissions_to_update(
-            resource=FeatureService.from_proto(request.feature_service),
-            actions=CRUD,
-            getter=self.proxied_registry.get_feature_service,
-            project=request.project,
+        feature_service = cast(
+            FeatureService,
+            assert_permissions_to_update(
+                resource=FeatureService.from_proto(request.feature_service),
+                getter=self.proxied_registry.get_feature_service,
+                project=request.project,
+            ),
         )
         self.proxied_registry.apply_feature_service(
-            feature_service=cast(
-                FeatureService,
-                assert_permissions(
-                    resource=feature_service,
-                    actions=CRUD,
-                ),
-            ),
+            feature_service=feature_service,
             project=request.project,
             commit=request.commit,
         )
@@ -394,7 +387,6 @@ def ApplySavedDataset(
             SavedDataset,
             assert_permissions_to_update(
                 resource=SavedDataset.from_proto(request.saved_dataset),
-                actions=CRUD,
                 getter=self.proxied_registry.get_saved_dataset,
                 project=request.project,
             ),
@@ -461,7 +453,6 @@ def ApplyValidationReference(
             ValidationReference,
             assert_permissions_to_update(
                 resource=ValidationReference.from_proto(request.validation_reference),
-                actions=CRUD,
                 getter=self.proxied_registry.get_validation_reference,
                 project=request.project,
             ),
@@ -574,7 +565,6 @@ def ApplyPermission(
             Permission,
             assert_permissions_to_update(
                 resource=Permission.from_proto(request.permission),
-                actions=CRUD,
                 getter=self.proxied_registry.get_permission,
                 project=request.project,
             ),

sdk/python/tests/unit/permissions/conftest.py

@@ -3,6 +3,7 @@
 import pytest
 
 from feast import FeatureView
+from feast.entity import Entity
 from feast.infra.registry.base_registry import BaseRegistry
 from feast.permissions.decorator import require_permissions
 from feast.permissions.permission import AuthzedAction, Permission
@@ -48,7 +49,10 @@ def users() -> list[User]:
     users.append(User("r", ["reader"]))
     users.append(User("w", ["writer"]))
     users.append(User("rw", ["reader", "writer"]))
-    users.append(User("admin", ["reader", "writer", "admin"]))
+    users.append(User("special", ["reader", "writer", "special-reader"]))
+    users.append(User("updater", ["updater"]))
+    users.append(User("creator", ["creator"]))
+    users.append(User("admin", ["updater", "creator"]))
     return dict([(u.username, u) for u in users])
 
 
@@ -76,10 +80,26 @@ def security_manager() -> SecurityManager:
             name="special",
             types=FeatureView,
             name_pattern="special.*",
-            policy=RoleBasedPolicy(roles=["admin", "special-reader"]),
+            policy=RoleBasedPolicy(roles=["special-reader"]),
             actions=[AuthzedAction.DESCRIBE, AuthzedAction.UPDATE],
         )
     )
+    permissions.append(
+        Permission(
+            name="entity_updater",
+            types=Entity,
+            policy=RoleBasedPolicy(roles=["updater"]),
+            actions=[AuthzedAction.DESCRIBE, AuthzedAction.UPDATE],
+        )
+    )
+    permissions.append(
+        Permission(
+            name="entity_creator",
+            types=Entity,
+            policy=RoleBasedPolicy(roles=["creator"]),
+            actions=[AuthzedAction.CREATE],
+        )
+    )
 
     registry = Mock(spec=BaseRegistry)
     registry.list_permissions = Mock(return_value=permissions)

sdk/python/tests/unit/permissions/test_security_manager.py

@@ -1,8 +1,14 @@
 import assertpy
 import pytest
 
+from feast.entity import Entity
+from feast.errors import FeastObjectNotFoundException
 from feast.permissions.action import READ, AuthzedAction
-from feast.permissions.security_manager import assert_permissions, permitted_resources
+from feast.permissions.security_manager import (
+    assert_permissions,
+    assert_permissions_to_update,
+    permitted_resources,
+)
 
 
 @pytest.mark.parametrize(
@@ -24,15 +30,15 @@
             True,
         ),
         (
-            "admin",
+            "special",
             [AuthzedAction.DESCRIBE, AuthzedAction.UPDATE],
             False,
             [False, True],
             [True, False],
             True,
         ),
         (
-            "admin",
+            "special",
             READ + [AuthzedAction.UPDATE],
             False,
             [False, False],
@@ -81,3 +87,83 @@ def test_access_SecuredFeatureView(
         else:
             result = assert_permissions(resource=r, actions=requested_actions)
             assertpy.assert_that(result).is_none()
+
+
+@pytest.mark.parametrize(
+    "username, allowed",
+    [
+        (None, False),
+        ("r", False),
+        ("w", False),
+        ("rw", False),
+        ("special", False),
+        ("updater", False),
+        ("creator", True),
+        ("admin", True),
+    ],
+)
+def test_create_entity(
+    security_manager,
+    users,
+    username,
+    allowed,
+):
+    sm = security_manager
+    entity = Entity(
+        name="",
+    )
+
+    user = users.get(username)
+    sm.set_current_user(user)
+
+    def getter(name: str, project: str, allow_cache: bool):
+        raise FeastObjectNotFoundException()
+
+    if allowed:
+        result = assert_permissions_to_update(
+            resource=entity, getter=getter, project=""
+        )
+        assertpy.assert_that(result).is_equal_to(entity)
+    else:
+        with pytest.raises(PermissionError):
+            assert_permissions_to_update(resource=entity, getter=getter, project="")
+
+
+@pytest.mark.parametrize(
+    "username, allowed",
+    [
+        (None, False),
+        ("r", False),
+        ("w", False),
+        ("rw", False),
+        ("special", False),
+        ("updater", True),
+        ("creator", False),
+        ("admin", True),
+    ],
+)
+def test_update_entity(
+    security_manager,
+    users,
+    username,
+    allowed,
+):
+    sm = security_manager
+    entity = Entity(
+        name="",
+    )
+
+    user = users.get(username)
+    sm.set_current_user(user)
+
+    def getter(name: str, project: str, allow_cache: bool):
+        return entity
+
+    if allowed:
+        result = assert_permissions_to_update(
+            resource=entity, getter=getter, project=""
+        )
+        assertpy.assert_that(result).is_equal_to(entity)
+    else:
+        with pytest.raises(PermissionError):
+            assert_permissions_to_update(resource=entity, getter=getter, project="")