Skip to content

Commit

Permalink
Merge pull request feast-dev#29 from dmartinol/feast-rbac
Browse files Browse the repository at this point in the history
replaced aggregated actions with aliases for QUERY and WRITE and ALL
  • Loading branch information
redhatHameed authored Jul 2, 2024
2 parents b9c9bec + 8549818 commit f4c06d2
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 38 deletions.
2 changes: 1 addition & 1 deletion docs/getting-started/concepts/permission.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ The `Permission` class identifies a single permission configured on the feature
- `with_subclasses`: Specify if sub-classes are included in the resource match or not. Defaults to `True`.
- `name_pattern`: A regex to match the resource name. Defaults to `None`, meaning that no name filtering is applied
- `required_tags`: Dictionary of key-value pairs that must match the resource tags. Defaults to `None`, meaning that no tags filtering is applied.
- `actions`: The actions authorized by this permission. Defaults to `AuthzedAction.ALL`.
- `actions`: The actions authorized by this permission. Defaults to `ALL_VALUES`, an alias defined in the `action` module.
- `policy`: The policy to be applied to validate a client request.

Given the above definitions, the feature store can be configured with granular control over each resource, enabling partitioned access by
Expand Down
19 changes: 8 additions & 11 deletions protos/feast/core/Permission.proto
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,14 @@ import "feast/core/Policy.proto";
message Permission {

enum AuthzedAction {
ALL = 0;
CREATE = 1;
READ = 2;
UPDATE = 3;
DELETE = 4;
QUERY = 5;
QUERY_ONLINE = 6;
QUERY_OFFLINE = 7;
WRITE = 8;
WRITE_ONLINE = 9;
WRITE_OFFLINE = 10;
CREATE = 0;
READ = 1;
UPDATE = 2;
DELETE = 3;
QUERY_ONLINE = 4;
QUERY_OFFLINE = 5;
WRITE_ONLINE = 6;
WRITE_OFFLINE = 7;
}

// Name of the permission. Must be unique. Not updated.
Expand Down
5 changes: 4 additions & 1 deletion sdk/python/feast/infra/registry/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,10 @@ def apply_permission(
):
registry = self._prepare_registry_for_changes(project)
for idx, existing_permission_proto in enumerate(registry.permissions):
if existing_permission_proto.name == permission.name and existing_permission_proto.project == project:
if (
existing_permission_proto.name == permission.name
and existing_permission_proto.project == project
):
del registry.permissions[idx]
permission_proto = permission.to_proto()
permission_proto.project = project
Expand Down
18 changes: 15 additions & 3 deletions sdk/python/feast/permissions/action.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,26 @@ class AuthzedAction(enum.Enum):
Identify the type of action being secured by the permissions framework, according to the familiar CRUD and Feast terminology.
"""

ALL = "all" # All actions
CREATE = "create" # Create an instance
READ = "read" # Access the instance state
UPDATE = "update" # Update the instance state
DELETE = "delete" # Deelete an instance
QUERY = "query" # Query both online and offline stores
QUERY_ONLINE = "query_online" # Query the online store only
QUERY_OFFLINE = "query_offline" # Query the offline store only
WRITE = "write" # Query on any store
WRITE_ONLINE = "write_online" # Write to the online store only
WRITE_OFFLINE = "write_offline" # Write to the offline store only


# Alias for all available actions
ALL_ACTIONS = [a for a in AuthzedAction.__members__.values()]

# Alias for all query actions
QUERY = [
AuthzedAction.QUERY_OFFLINE,
AuthzedAction.QUERY_ONLINE,
]
# Alias for all write actions
WRITE = [
AuthzedAction.WRITE_OFFLINE,
AuthzedAction.WRITE_ONLINE,
]
4 changes: 0 additions & 4 deletions sdk/python/feast/permissions/matcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,5 @@ def actions_match_config(
Returns:
bool: `True` if all the given `requested_actions` are defined in the `allowed_actions`.
Whatever the `requested_actions`, it returns `True` if `allowed_actions` includes `AuthzedAction.ALL`
"""
if AuthzedAction.ALL in allowed_actions:
return True

return all(a in allowed_actions for a in requested_actions)
6 changes: 3 additions & 3 deletions sdk/python/feast/permissions/permission.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from typing import TYPE_CHECKING, Any, Dict, Optional, Union

from feast.importer import import_class
from feast.permissions.action import AuthzedAction
from feast.permissions.action import ALL_ACTIONS, AuthzedAction
from feast.permissions.decision import DecisionStrategy
from feast.permissions.matcher import actions_match_config, resource_match_config
from feast.permissions.policy import AllowAll, Policy
Expand Down Expand Up @@ -34,7 +34,7 @@ class Permission(ABC):
name_pattern: A regex to match the resource name. Defaults to None, meaning that no name filtering is applied
required_tags: Dictionary of key-value pairs that must match the resource tags. All these required_tags must
be present in a resource tags with the given value. Defaults to None, meaning that no tags filtering is applied.
actions: The actions authorized by this permission. Defaults to `AuthzedAction.ALL`.
actions: The actions authorized by this permission. Defaults to `ALL_ACTIONS`.
policy: The policy to be applied to validate a client request.
"""

Expand All @@ -54,7 +54,7 @@ def __init__(
with_subclasses: bool = True,
name_pattern: Optional[str] = None,
required_tags: Optional[dict[str, str]] = None,
actions: Union[list[AuthzedAction], AuthzedAction] = AuthzedAction.ALL,
actions: Union[list[AuthzedAction], AuthzedAction] = ALL_ACTIONS,
policy: Policy = AllowAll,
tags: Optional[Dict[str, str]] = None,
):
Expand Down
6 changes: 3 additions & 3 deletions sdk/python/tests/unit/permissions/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def __init__(self, name, tags):
def read_protected(self) -> bool:
return True

@require_permissions(actions=[AuthzedAction.WRITE])
@require_permissions(actions=[AuthzedAction.UPDATE])
def write_protected(self) -> bool:
return True

Expand Down Expand Up @@ -68,7 +68,7 @@ def security_manager() -> SecurityManager:
types=FeatureView,
with_subclasses=True,
policy=RoleBasedPolicy(roles=["writer"]),
actions=[AuthzedAction.WRITE],
actions=[AuthzedAction.UPDATE],
)
)
permissions.append(
Expand All @@ -78,7 +78,7 @@ def security_manager() -> SecurityManager:
with_subclasses=True,
name_pattern="special.*",
policy=RoleBasedPolicy(roles=["admin", "special-reader"]),
actions=[AuthzedAction.READ, AuthzedAction.WRITE],
actions=[AuthzedAction.READ, AuthzedAction.UPDATE],
)
)

Expand Down
10 changes: 5 additions & 5 deletions sdk/python/tests/unit/permissions/test_permission.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
from feast.feature_service import FeatureService
from feast.feature_view import FeatureView
from feast.on_demand_feature_view import OnDemandFeatureView
from feast.permissions.action import ALL_ACTIONS, AuthzedAction
from feast.permissions.permission import (
AuthzedAction,
DecisionStrategy,
Permission,
)
Expand Down Expand Up @@ -40,7 +40,7 @@ def test_defaults():
assertpy.assert_that(p.name_pattern).is_none()
assertpy.assert_that(p.required_tags).is_none()
assertpy.assert_that(type(p.actions)).is_equal_to(list)
assertpy.assert_that(p.actions[0]).is_equal_to(AuthzedAction.ALL)
assertpy.assert_that(p.actions).is_equal_to(ALL_ACTIONS)
assertpy.assert_that(type(p.actions)).is_equal_to(list)
assertpy.assert_that(isinstance(p.policy, Policy)).is_true()
assertpy.assert_that(p.policy).is_equal_to(AllowAll)
Expand All @@ -55,8 +55,8 @@ def test_defaults():
({"types": [FeatureView, FeatureService]}, True),
({"actions": None}, False),
({"actions": []}, False),
({"actions": AuthzedAction.ALL}, True),
({"actions": [AuthzedAction.ALL]}, True),
({"actions": ALL_ACTIONS}, True),
({"actions": ALL_ACTIONS}, True),
({"actions": [AuthzedAction.CREATE, AuthzedAction.DELETE]}, True),
({"policy": None}, False),
({"policy": []}, False),
Expand Down Expand Up @@ -256,7 +256,7 @@ def test_resource_match_with_tags(required_tags, tags, result):

@pytest.mark.parametrize(
("permitted_actions, requested_actions, result"),
[(AuthzedAction.ALL, a, True) for a in AuthzedAction.__members__.values()]
[(ALL_ACTIONS, [a], True) for a in AuthzedAction.__members__.values()]
+ [
(
[AuthzedAction.CREATE, AuthzedAction.DELETE],
Expand Down
14 changes: 7 additions & 7 deletions sdk/python/tests/unit/permissions/test_security_manager.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import assertpy
import pytest

from feast.permissions.action import AuthzedAction
from feast.permissions.action import QUERY, AuthzedAction
from feast.permissions.decision import DecisionStrategy
from feast.permissions.permission import Permission
from feast.permissions.security_manager import assert_permissions, permitted_resources
Expand All @@ -17,14 +17,14 @@ def setup_module():
[
(None, [], False, [False, False]),
("r", [AuthzedAction.READ], False, [True, False]),
("r", [AuthzedAction.WRITE], False, [False, False]),
("r", [AuthzedAction.UPDATE], False, [False, False]),
("w", [AuthzedAction.READ], False, [False, False]),
("w", [AuthzedAction.WRITE], False, [True, False]),
("w", [AuthzedAction.UPDATE], False, [True, False]),
("rw", [AuthzedAction.READ], False, [True, False]),
("rw", [AuthzedAction.WRITE], False, [True, False]),
("rw", [AuthzedAction.READ, AuthzedAction.WRITE], False, [True, False]),
("admin", [AuthzedAction.READ, AuthzedAction.WRITE], True, [True, True]),
("admin", [AuthzedAction.QUERY, AuthzedAction.WRITE], True, [True, True]),
("rw", [AuthzedAction.UPDATE], False, [True, False]),
("rw", [AuthzedAction.READ, AuthzedAction.UPDATE], False, [True, False]),
("admin", [AuthzedAction.READ, AuthzedAction.UPDATE], True, [True, True]),
("admin", QUERY + [AuthzedAction.UPDATE], True, [True, True]),
],
)
def test_access_SecuredFeatureView(
Expand Down

0 comments on commit f4c06d2

Please sign in to comment.