Skip to content

Commit af81369

Browse files
tmihalactmihalac
committed
Intra server communication
- Code changes following review Signed-off-by: Theodor Mihalache <tmihalac@redhat.com>
1 parent 1304300 commit af81369

File tree

5 files changed

+60
-41
lines changed

5 files changed

+60
-41
lines changed

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

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -103,14 +103,11 @@ def assert_permissions(
103103
Raises:
104104
PermissionError: If the current user is not authorized to execute the requested actions on the given resources.
105105
"""
106-
intra_communication_base64 = os.getenv("INTRA_COMMUNICATION_BASE64")
106+
107107
sm = get_security_manager()
108-
if sm is None or (
109-
sm.current_user is not None
110-
and sm.current_user.username == intra_communication_base64
111-
):
108+
if not is_auth_necessary(sm):
112109
return resource
113-
return sm.assert_permissions(
110+
return sm.assert_permissions( # type: ignore[union-attr]
114111
resources=[resource], actions=actions, filter_only=False
115112
)[0]
116113

@@ -131,14 +128,10 @@ def permitted_resources(
131128
list[FeastObject]]: A filtered list of the permitted resources, possibly empty.
132129
"""
133130

134-
intra_communication_base64 = os.getenv("INTRA_COMMUNICATION_BASE64")
135131
sm = get_security_manager()
136-
if sm is None or (
137-
sm.current_user is not None
138-
and sm.current_user.username == intra_communication_base64
139-
):
132+
if not is_auth_necessary(sm):
140133
return resources
141-
return sm.assert_permissions(resources=resources, actions=actions, filter_only=True)
134+
return sm.assert_permissions(resources=resources, actions=actions, filter_only=True) # type: ignore[union-attr]
142135

143136

144137
"""
@@ -171,3 +164,13 @@ def no_security_manager():
171164

172165
global _sm
173166
_sm = None
167+
168+
169+
def is_auth_necessary(sm: Optional[SecurityManager]) -> bool:
170+
intra_communication_base64 = os.getenv("INTRA_COMMUNICATION_BASE64")
171+
172+
return (
173+
sm is not None
174+
and sm.current_user is not None
175+
and sm.current_user.username != intra_communication_base64
176+
)

‎sdk/python/tests/unit/permissions/auth/test_token_parser.py‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,16 @@ def test_oidc_token_validation_failure(mock_oauth2, oidc_config):
6464

6565

6666
@mock.patch.dict(os.environ, {"INTRA_COMMUNICATION_BASE64": "test1234"})
67-
@pytest.mark.parametrize("intra_communication_val", ("test1234", "my-name"))
68-
def test_oidc_inter_server_comm(intra_communication_val, oidc_config, monkeypatch):
67+
@pytest.mark.parametrize(
68+
"intra_communication_val, is_intra_server",
69+
[
70+
("test1234", True),
71+
("my-name", False),
72+
],
73+
)
74+
def test_oidc_inter_server_comm(
75+
intra_communication_val, is_intra_server, oidc_config, monkeypatch
76+
):
6977
async def mock_oath2(self, request):
7078
return "OK"
7179

@@ -84,7 +92,7 @@ async def mock_oath2(self, request):
8492
"preferred_username": f"{intra_communication_val}",
8593
}
8694

87-
if intra_communication_val != "test1234":
95+
if not is_intra_server:
8896
user_data["resource_access"] = {_CLIENT_ID: {"roles": ["reader", "writer"]}}
8997

9098
monkeypatch.setattr(
@@ -98,7 +106,7 @@ async def mock_oath2(self, request):
98106
token_parser.user_details_from_access_token(access_token=access_token)
99107
)
100108

101-
if intra_communication_val == "test1234":
109+
if is_intra_server:
102110
assertpy.assert_that(user).is_not_none()
103111
assertpy.assert_that(user.username).is_equal_to(intra_communication_val)
104112
assertpy.assert_that(user.roles).is_equal_to([])
@@ -175,16 +183,23 @@ def test_k8s_token_validation_failure(mock_jwt, mock_config):
175183

176184

177185
@mock.patch.dict(os.environ, {"INTRA_COMMUNICATION_BASE64": "test1234"})
178-
@pytest.mark.parametrize("intra_communication_val", ("test1234", "my-name"))
186+
@pytest.mark.parametrize(
187+
"intra_communication_val, is_intra_server",
188+
[
189+
("test1234", True),
190+
("my-name", False),
191+
],
192+
)
179193
def test_k8s_inter_server_comm(
180194
intra_communication_val,
195+
is_intra_server,
181196
oidc_config,
182197
request,
183198
rolebindings,
184199
clusterrolebindings,
185200
monkeypatch,
186201
):
187-
if intra_communication_val == "test1234":
202+
if is_intra_server:
188203
subject = f":::{intra_communication_val}"
189204
else:
190205
sa_name = request.getfixturevalue("sa_name")
@@ -225,7 +240,7 @@ def test_k8s_inter_server_comm(
225240
token_parser.user_details_from_access_token(access_token=access_token)
226241
)
227242

228-
if intra_communication_val == "test1234":
243+
if is_intra_server:
229244
assertpy.assert_that(user).is_not_none()
230245
assertpy.assert_that(user.username).is_equal_to(intra_communication_val)
231246
assertpy.assert_that(user.roles).is_equal_to([])

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ def users() -> list[User]:
4949
users.append(User("w", ["writer"]))
5050
users.append(User("rw", ["reader", "writer"]))
5151
users.append(User("admin", ["reader", "writer", "admin"]))
52-
users.append(User("test1234", []))
5352
return dict([(u.username, u) for u in users])
5453

5554

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
@pytest.mark.parametrize(
66
"username, can_read, can_write",
77
[
8-
(None, False, False),
8+
(None, True, True),
99
("r", True, False),
1010
("w", False, True),
1111
("rw", True, True),

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

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@
33

44
from feast.permissions.action import READ, AuthzedAction
55
from feast.permissions.security_manager import assert_permissions, permitted_resources
6+
from feast.permissions.user import User
67

78

89
@pytest.mark.parametrize(
910
"username, requested_actions, allowed, allowed_single, raise_error_in_assert, raise_error_in_permit, intra_communication_flag",
1011
[
11-
(None, [], False, [False, False], [True, True], False, False),
12-
(None, [], False, [False, False], [True, True], False, True),
12+
(None, [], True, [True, True], [False, False], False, False),
13+
(None, [], True, [True, True], [False, False], False, True),
1314
(
1415
"r",
1516
[AuthzedAction.DESCRIBE],
@@ -28,7 +29,7 @@
2829
False,
2930
True,
3031
),
31-
("test1234", [], True, [True, True], [False, False], False, True),
32+
("server_intra_com_val", [], True, [True, True], [False, False], False, True),
3233
(
3334
"r",
3435
[AuthzedAction.UPDATE],
@@ -38,7 +39,7 @@
3839
False,
3940
False,
4041
),
41-
("r", [AuthzedAction.UPDATE], False, [False, False], [True, True], False, True),
42+
("r", [AuthzedAction.UPDATE], True, [True, True], [False, False], False, True),
4243
(
4344
"w",
4445
[AuthzedAction.DESCRIBE],
@@ -51,8 +52,8 @@
5152
(
5253
"w",
5354
[AuthzedAction.DESCRIBE],
54-
False,
55-
[False, False],
55+
True,
56+
[True, True],
5657
[True, True],
5758
False,
5859
True,
@@ -115,10 +116,10 @@
115116
(
116117
"rw",
117118
[AuthzedAction.DESCRIBE, AuthzedAction.UPDATE],
118-
False,
119-
[False, False],
120-
[True, True],
121119
True,
120+
[True, True],
121+
[False, False],
122+
False,
122123
True,
123124
),
124125
(
@@ -133,10 +134,10 @@
133134
(
134135
"admin",
135136
[AuthzedAction.DESCRIBE, AuthzedAction.UPDATE],
136-
False,
137-
[False, True],
138-
[True, False],
139137
True,
138+
[True, True],
139+
[False, False],
140+
False,
140141
True,
141142
),
142143
(
@@ -151,10 +152,10 @@
151152
(
152153
"admin",
153154
READ + [AuthzedAction.UPDATE],
154-
False,
155-
[False, False],
156-
[True, True],
157155
True,
156+
[True, True],
157+
[False, False],
158+
False,
158159
True,
159160
),
160161
],
@@ -172,17 +173,18 @@ def test_access_SecuredFeatureView(
172173
intra_communication_flag,
173174
monkeypatch,
174175
):
176+
sm = security_manager
177+
user = users.get(username)
178+
sm.set_current_user(user)
179+
175180
if intra_communication_flag:
176-
monkeypatch.setenv("INTRA_COMMUNICATION_BASE64", "test1234")
181+
monkeypatch.setenv("INTRA_COMMUNICATION_BASE64", "server_intra_com_val")
182+
sm.set_current_user(User("server_intra_com_val", []))
177183
else:
178184
monkeypatch.delenv("INTRA_COMMUNICATION_BASE64", False)
179185

180-
sm = security_manager
181186
resources = feature_views
182187

183-
user = users.get(username)
184-
sm.set_current_user(user)
185-
186188
result = []
187189
if raise_error_in_permit:
188190
with pytest.raises(PermissionError):

0 commit comments

Comments
 (0)