Skip to content

Commit 1524f1c

Browse files
jyejarejyejare
authored
fix: Denial by default to all resources when no permissions set (#5663)
* Denial by default to all resources when no permissions set Signed-off-by: jyejare <jyejare@redhat.com> filter only for named patterns No matching permissions are handled * Tests are updated to match the new behavior Signed-off-by: jyejare <jyejare@redhat.com> --------- Signed-off-by: jyejare <jyejare@redhat.com>
1 parent de37f66 commit 1524f1c

File tree

3 files changed

+73
-18
lines changed

3 files changed

+73
-18
lines changed

‎sdk/python/feast/permissions/enforcer.py‎

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ def enforce_policy(
4343
# If no permissions are defined, deny access to all resources
4444
# This is a security measure to prevent unauthorized access
4545
logger.warning("No permissions defined - denying access to all resources")
46-
if not filter_only:
47-
raise FeastPermissionError("No permissions defined - access denied")
48-
return []
46+
raise FeastPermissionError(
47+
"Permissions are not defined - access denied for all resources"
48+
)
4949

5050
_permitted_resources: list[FeastObject] = []
5151
for resource in resources:
@@ -71,17 +71,42 @@ def enforce_policy(
7171

7272
if evaluator.is_decided():
7373
grant, explanations = evaluator.grant()
74-
if not grant and not filter_only:
74+
if not grant:
75+
if filter_only and p.name_patterns:
76+
continue
7577
logger.error(f"Permission denied: {','.join(explanations)}")
7678
raise FeastPermissionError(",".join(explanations))
77-
if grant:
78-
logger.debug(
79-
f"Permission granted for {type(resource).__name__}:{resource.name}"
80-
)
81-
_permitted_resources.append(resource)
79+
logger.debug(
80+
f"Permission granted for {type(resource).__name__}:{resource.name}"
81+
)
82+
_permitted_resources.append(resource)
8283
break
8384
else:
84-
message = f"No permissions defined to manage {actions} on {type(resource)}/{resource.name}."
85-
logger.exception(f"**PERMISSION NOT GRANTED**: {message}")
86-
raise FeastPermissionError(message)
85+
if not filter_only:
86+
message = f"No permissions defined to manage {actions} on {type(resource)}/{resource.name}."
87+
logger.exception(f"**PERMISSION NOT GRANTED**: {message}")
88+
raise FeastPermissionError(message)
89+
else:
90+
# filter_only=True: Check if there are permissions for this resource type
91+
resource_type_permissions = [
92+
p
93+
for p in permissions
94+
if any(isinstance(resource, t) for t in p.types) # type: ignore
95+
]
96+
if not resource_type_permissions:
97+
# No permissions exist for this resource type - should raise error
98+
message = f"No permissions defined to manage {actions} on {type(resource)}/{resource.name}."
99+
logger.exception(f"**PERMISSION NOT GRANTED**: {message}")
100+
raise FeastPermissionError(message)
101+
elif not any(p.name_patterns for p in resource_type_permissions):
102+
# Permissions exist for this resource type but no name_patterns - should raise error
103+
message = f"No permissions defined to manage {actions} on {type(resource)}/{resource.name}."
104+
logger.exception(f"**PERMISSION NOT GRANTED**: {message}")
105+
raise FeastPermissionError(message)
106+
else:
107+
# Permissions exist for this resource type with name_patterns - filter out this resource
108+
logger.debug(
109+
f"Filtering out {type(resource).__name__}:{resource.name} - no matching permissions"
110+
)
111+
continue
87112
return _permitted_resources

‎sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,11 @@ def _test_get_historical_features(client_fs: FeatureStore):
142142

143143

144144
def _test_get_entity(client_fs: FeatureStore, permissions: list[Permission]):
145+
if _is_auth_enabled(client_fs) and len(permissions) == 0:
146+
with pytest.raises(FeastPermissionError):
147+
client_fs.get_entity("driver")
148+
return
149+
145150
if not _is_auth_enabled(client_fs) or _is_permission_enabled(
146151
client_fs, permissions, read_entities_perm
147152
):
@@ -156,6 +161,18 @@ def _test_get_entity(client_fs: FeatureStore, permissions: list[Permission]):
156161

157162

158163
def _test_list_entities(client_fs: FeatureStore, permissions: list[Permission]):
164+
if _is_auth_enabled(client_fs) and len(permissions) == 0:
165+
with pytest.raises(FeastPermissionError):
166+
client_fs.list_entities()
167+
return
168+
169+
if _is_auth_enabled(client_fs) and _permissions_exist_in_permission_list(
170+
[invalid_list_entities_perm], permissions
171+
):
172+
with pytest.raises(FeastPermissionError):
173+
client_fs.list_entities()
174+
return
175+
159176
entities = client_fs.list_entities()
160177

161178
if not _is_auth_enabled(client_fs) or _is_permission_enabled(
@@ -183,6 +200,10 @@ def _test_list_permissions(
183200
with pytest.raises(Exception):
184201
client_fs.list_permissions()
185202
return []
203+
elif _is_auth_enabled(client_fs) and len(applied_permissions) == 0:
204+
with pytest.raises(FeastPermissionError):
205+
client_fs.list_permissions()
206+
return []
186207
else:
187208
permissions = client_fs.list_permissions()
188209

@@ -229,6 +250,11 @@ def _is_auth_enabled(client_fs: FeatureStore) -> bool:
229250

230251

231252
def _test_get_fv(client_fs: FeatureStore, permissions: list[Permission]):
253+
if _is_auth_enabled(client_fs) and len(permissions) == 0:
254+
with pytest.raises(FeastPermissionError):
255+
client_fs.get_feature_view("driver_hourly_stats")
256+
return
257+
232258
if not _is_auth_enabled(client_fs) or _is_permission_enabled(
233259
client_fs, permissions, read_fv_perm
234260
):
@@ -249,6 +275,10 @@ def _test_list_fvs(client_fs: FeatureStore, permissions: list[Permission]):
249275
with pytest.raises(Exception):
250276
client_fs.list_feature_views()
251277
return []
278+
elif _is_auth_enabled(client_fs) and len(permissions) == 0:
279+
with pytest.raises(FeastPermissionError):
280+
client_fs.list_feature_views()
281+
return []
252282
else:
253283
fvs = client_fs.list_feature_views()
254284
for fv in fvs:

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
@pytest.mark.parametrize(
1616
"username, requested_actions, allowed, allowed_single, raise_error_in_assert, raise_error_in_permit, intra_communication_flag",
1717
[
18-
(None, [], False, [False, False], [True, True], False, False),
18+
(None, [], False, [False, False], [True, True], True, False),
1919
(None, [], True, [True, True], [False, False], False, True),
2020
(
2121
"r",
@@ -42,7 +42,7 @@
4242
False,
4343
[False, False],
4444
[True, True],
45-
False,
45+
True,
4646
False,
4747
),
4848
("r", [AuthzedAction.UPDATE], True, [True, True], [False, False], False, True),
@@ -52,7 +52,7 @@
5252
False,
5353
[False, False],
5454
[True, True],
55-
False,
55+
True,
5656
False,
5757
),
5858
(
@@ -116,7 +116,7 @@
116116
False,
117117
[False, False],
118118
[True, True],
119-
True,
119+
False,
120120
False,
121121
),
122122
(
@@ -134,7 +134,7 @@
134134
False,
135135
[False, True],
136136
[True, False],
137-
True,
137+
False,
138138
False,
139139
),
140140
(
@@ -152,7 +152,7 @@
152152
False,
153153
[False, False],
154154
[True, True],
155-
True,
155+
False,
156156
False,
157157
),
158158
(

0 commit comments

Comments
 (0)