diff --git a/sdk/python/feast/permissions/enforcer.py b/sdk/python/feast/permissions/enforcer.py index 2cb6608a2d8..4db0241ef5a 100644 --- a/sdk/python/feast/permissions/enforcer.py +++ b/sdk/python/feast/permissions/enforcer.py @@ -43,9 +43,9 @@ def enforce_policy( # If no permissions are defined, deny access to all resources # This is a security measure to prevent unauthorized access logger.warning("No permissions defined - denying access to all resources") - if not filter_only: - raise FeastPermissionError("No permissions defined - access denied") - return [] + raise FeastPermissionError( + "Permissions are not defined - access denied for all resources" + ) _permitted_resources: list[FeastObject] = [] for resource in resources: @@ -71,17 +71,42 @@ def enforce_policy( if evaluator.is_decided(): grant, explanations = evaluator.grant() - if not grant and not filter_only: + if not grant: + if filter_only and p.name_patterns: + continue logger.error(f"Permission denied: {','.join(explanations)}") raise FeastPermissionError(",".join(explanations)) - if grant: - logger.debug( - f"Permission granted for {type(resource).__name__}:{resource.name}" - ) - _permitted_resources.append(resource) + logger.debug( + f"Permission granted for {type(resource).__name__}:{resource.name}" + ) + _permitted_resources.append(resource) break else: - message = f"No permissions defined to manage {actions} on {type(resource)}/{resource.name}." - logger.exception(f"**PERMISSION NOT GRANTED**: {message}") - raise FeastPermissionError(message) + if not filter_only: + message = f"No permissions defined to manage {actions} on {type(resource)}/{resource.name}." + logger.exception(f"**PERMISSION NOT GRANTED**: {message}") + raise FeastPermissionError(message) + else: + # filter_only=True: Check if there are permissions for this resource type + resource_type_permissions = [ + p + for p in permissions + if any(isinstance(resource, t) for t in p.types) # type: ignore + ] + if not resource_type_permissions: + # No permissions exist for this resource type - should raise error + message = f"No permissions defined to manage {actions} on {type(resource)}/{resource.name}." + logger.exception(f"**PERMISSION NOT GRANTED**: {message}") + raise FeastPermissionError(message) + elif not any(p.name_patterns for p in resource_type_permissions): + # Permissions exist for this resource type but no name_patterns - should raise error + message = f"No permissions defined to manage {actions} on {type(resource)}/{resource.name}." + logger.exception(f"**PERMISSION NOT GRANTED**: {message}") + raise FeastPermissionError(message) + else: + # Permissions exist for this resource type with name_patterns - filter out this resource + logger.debug( + f"Filtering out {type(resource).__name__}:{resource.name} - no matching permissions" + ) + continue return _permitted_resources diff --git a/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py b/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py index 71a883e1933..e0f75d1d3d8 100644 --- a/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py +++ b/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py @@ -142,6 +142,11 @@ def _test_get_historical_features(client_fs: FeatureStore): def _test_get_entity(client_fs: FeatureStore, permissions: list[Permission]): + if _is_auth_enabled(client_fs) and len(permissions) == 0: + with pytest.raises(FeastPermissionError): + client_fs.get_entity("driver") + return + if not _is_auth_enabled(client_fs) or _is_permission_enabled( client_fs, permissions, read_entities_perm ): @@ -156,6 +161,18 @@ def _test_get_entity(client_fs: FeatureStore, permissions: list[Permission]): def _test_list_entities(client_fs: FeatureStore, permissions: list[Permission]): + if _is_auth_enabled(client_fs) and len(permissions) == 0: + with pytest.raises(FeastPermissionError): + client_fs.list_entities() + return + + if _is_auth_enabled(client_fs) and _permissions_exist_in_permission_list( + [invalid_list_entities_perm], permissions + ): + with pytest.raises(FeastPermissionError): + client_fs.list_entities() + return + entities = client_fs.list_entities() if not _is_auth_enabled(client_fs) or _is_permission_enabled( @@ -183,6 +200,10 @@ def _test_list_permissions( with pytest.raises(Exception): client_fs.list_permissions() return [] + elif _is_auth_enabled(client_fs) and len(applied_permissions) == 0: + with pytest.raises(FeastPermissionError): + client_fs.list_permissions() + return [] else: permissions = client_fs.list_permissions() @@ -229,6 +250,11 @@ def _is_auth_enabled(client_fs: FeatureStore) -> bool: def _test_get_fv(client_fs: FeatureStore, permissions: list[Permission]): + if _is_auth_enabled(client_fs) and len(permissions) == 0: + with pytest.raises(FeastPermissionError): + client_fs.get_feature_view("driver_hourly_stats") + return + if not _is_auth_enabled(client_fs) or _is_permission_enabled( client_fs, permissions, read_fv_perm ): @@ -249,6 +275,10 @@ def _test_list_fvs(client_fs: FeatureStore, permissions: list[Permission]): with pytest.raises(Exception): client_fs.list_feature_views() return [] + elif _is_auth_enabled(client_fs) and len(permissions) == 0: + with pytest.raises(FeastPermissionError): + client_fs.list_feature_views() + return [] else: fvs = client_fs.list_feature_views() for fv in fvs: diff --git a/sdk/python/tests/unit/permissions/test_security_manager.py b/sdk/python/tests/unit/permissions/test_security_manager.py index ee0ec9e079a..34d8e4962e9 100644 --- a/sdk/python/tests/unit/permissions/test_security_manager.py +++ b/sdk/python/tests/unit/permissions/test_security_manager.py @@ -15,7 +15,7 @@ @pytest.mark.parametrize( "username, requested_actions, allowed, allowed_single, raise_error_in_assert, raise_error_in_permit, intra_communication_flag", [ - (None, [], False, [False, False], [True, True], False, False), + (None, [], False, [False, False], [True, True], True, False), (None, [], True, [True, True], [False, False], False, True), ( "r", @@ -42,7 +42,7 @@ False, [False, False], [True, True], - False, + True, False, ), ("r", [AuthzedAction.UPDATE], True, [True, True], [False, False], False, True), @@ -52,7 +52,7 @@ False, [False, False], [True, True], - False, + True, False, ), ( @@ -116,7 +116,7 @@ False, [False, False], [True, True], - True, + False, False, ), ( @@ -134,7 +134,7 @@ False, [False, True], [True, False], - True, + False, False, ), ( @@ -152,7 +152,7 @@ False, [False, False], [True, True], - True, + False, False, ), (