Skip to content

Commit 8d3174c

Browse files
Access keys fixes
Nonexisting unit tests... are a bad idea. Sanity testing showed several of the access key functions were horribly broken. Also the tests were slightly misleading in their mocking of the ACCESS_KEY_ID. Namely, the user-supplied NAME was being used where in practice it would not actually work.
1 parent 92512cb commit 8d3174c

File tree

3 files changed

+32
-18
lines changed

3 files changed

+32
-18
lines changed

keen/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,7 @@ def update_access_key_permissions(access_key_id, permissions):
576576
_initialize_client_from_environment()
577577
return _client.update_access_key_permissions(access_key_id, permissions)
578578

579-
def update_access_key_options(self, access_key_id, options):
579+
def update_access_key_options(access_key_id, options):
580580
"""
581581
Replaces all of the options on the access key but does not change
582582
non-option properties such as permissions or the key's name.

keen/api.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ def _update_access_key_pair(self, access_key_id, key, val):
333333
current_access_key = self.get_access_key(access_key_id)
334334

335335
# Copy and only change the single parameter.
336-
payload_dict = _build_access_key_dict(current_access_key)
336+
payload_dict = KeenApi._build_access_key_dict(current_access_key)
337337
payload_dict[key] = val
338338

339339
# Now just treat it like a full update.
@@ -362,13 +362,13 @@ def add_access_key_permissions(self, access_key_id, permissions):
362362
current_access_key = self.get_access_key(access_key_id)
363363

364364
# Copy and only change the single parameter.
365-
payload_dict = _build_access_key_dict(current_access_key)
365+
payload_dict = KeenApi._build_access_key_dict(current_access_key)
366366

367367
# Turn into sets to avoid duplicates.
368-
old_permissions = set(payload_dict["permissions"])
368+
old_permissions = set(payload_dict["permitted"])
369369
new_permissions = set(permissions)
370370
combined_permissions = old_permissions.union(new_permissions)
371-
payload_dict["permissions"] = list(combined_permissions)
371+
payload_dict["permitted"] = list(combined_permissions)
372372

373373
# Now just treat it like a full update.
374374
return self.update_access_key_full(access_key_id, **payload_dict)
@@ -389,13 +389,13 @@ def remove_access_key_permissions(self, access_key_id, permissions):
389389
current_access_key = self.get_access_key(access_key_id)
390390

391391
# Copy and only change the single parameter.
392-
payload_dict = _build_access_key_dict(current_access_key)
392+
payload_dict = KeenApi._build_access_key_dict(current_access_key)
393393

394394
# Turn into sets to avoid duplicates.
395-
old_permissions = set(payload_dict["permissions"])
395+
old_permissions = set(payload_dict["permitted"])
396396
removal_permissions = set(permissions)
397-
reduced_permissions = old_permissions.difference_update(removal_permissions)
398-
payload_dict["permissions"] = list(reduced_permissions)
397+
reduced_permissions = old_permissions.difference(removal_permissions)
398+
payload_dict["permitted"] = list(reduced_permissions)
399399

400400
# Now just treat it like a full update.
401401
return self.update_access_key_full(access_key_id, **payload_dict)
@@ -411,7 +411,7 @@ def update_access_key_permissions(self, access_key_id, permissions):
411411
:param access_key_id: the 'key' value of the access key to change the permissions of
412412
:param permissions: the new list of permissions for this key
413413
"""
414-
return self._update_access_key_pair(access_key_id, "permissions", permission)
414+
return self._update_access_key_pair(access_key_id, "permitted", permissions)
415415

416416
@requires_key(KeenKeys.MASTER)
417417
def update_access_key_options(self, access_key_id, options):

keen/tests/access_key_tests.py

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
class AccessKeyTests(BaseTestCase):
1111

1212
ACCESS_KEY_NAME = "Bob_Key"
13+
ACCESS_KEY_ID = "320104AEFFC569EEE60BCAC9BB064DFF9897E391AB8C59608AC0869AFD291B4E"
1314
ACCESS_KEY_RESPONSE = MockedResponse(
1415
status_code=201,
1516
json_response={'name': "Bob_Key",
@@ -73,24 +74,24 @@ def test_list_access_keys(self, get):
7374
@patch("requests.Session.get")
7475
def test_get_access_key(self, get):
7576
get.return_value = self.ACCESS_KEY_RESPONSE
76-
resp = keen.get_access_key(self.ACCESS_KEY_NAME)
77-
self.assertEqual("{0}/{1}".format(self.keys_uri_prefix, self.ACCESS_KEY_NAME), get.call_args[0][0])
77+
resp = keen.get_access_key(self.ACCESS_KEY_ID)
78+
self.assertEqual("{0}/{1}".format(self.keys_uri_prefix, self.ACCESS_KEY_ID), get.call_args[0][0])
7879
self._assert_proper_permissions(get, keen.master_key)
7980
self.assertEqual(resp, self.ACCESS_KEY_RESPONSE.json())
8081

8182
@patch("requests.Session.post")
8283
def test_revoke_access_key(self, post):
8384
post.return_value = self.NO_CONTENT_RESPONSE
84-
resp = keen.revoke_access_key(self.ACCESS_KEY_NAME)
85-
self.assertEqual("{0}/{1}/revoke".format(self.keys_uri_prefix, self.ACCESS_KEY_NAME), post.call_args[0][0])
85+
resp = keen.revoke_access_key(self.ACCESS_KEY_ID)
86+
self.assertEqual("{0}/{1}/revoke".format(self.keys_uri_prefix, self.ACCESS_KEY_ID), post.call_args[0][0])
8687
self._assert_proper_permissions(post, keen.master_key)
8788
self.assertEqual(resp, self.NO_CONTENT_RESPONSE.json())
8889

8990
@patch("requests.Session.post")
9091
def test_unrevoke_access_key(self, post):
9192
post.return_value = self.NO_CONTENT_RESPONSE
92-
resp = keen.unrevoke_access_key(self.ACCESS_KEY_NAME)
93-
self.assertEqual("{0}/{1}/unrevoke".format(self.keys_uri_prefix, self.ACCESS_KEY_NAME), post.call_args[0][0])
93+
resp = keen.unrevoke_access_key(self.ACCESS_KEY_ID)
94+
self.assertEqual("{0}/{1}/unrevoke".format(self.keys_uri_prefix, self.ACCESS_KEY_ID), post.call_args[0][0])
9495
self._assert_proper_permissions(post, keen.master_key)
9596
self.assertEqual(resp, self.NO_CONTENT_RESPONSE.json())
9697

@@ -101,11 +102,24 @@ def test_update_access_key_full(self, post):
101102
# well.
102103
post.return_value = self.UPDATED_ACCESS_KEY_RESPONSE
103104
options_dict = {"queries": self.UPDATED_ACCESS_KEY_RESPONSE.json_response["options"]["queries"]}
104-
resp = keen.update_access_key_full(self.ACCESS_KEY_NAME,
105+
resp = keen.update_access_key_full(self.ACCESS_KEY_ID,
105106
name=self.UPDATED_ACCESS_KEY_RESPONSE.json_response["name"],
106107
is_active=self.UPDATED_ACCESS_KEY_RESPONSE.json_response["is_active"],
107108
permitted=self.UPDATED_ACCESS_KEY_RESPONSE.json_response["permitted"],
108109
options=options_dict)
109-
self.assertEqual("{0}/{1}".format(self.keys_uri_prefix, self.ACCESS_KEY_NAME), post.call_args[0][0])
110+
self.assertEqual("{0}/{1}".format(self.keys_uri_prefix, self.ACCESS_KEY_ID), post.call_args[0][0])
110111
self._assert_proper_permissions(post, keen.master_key)
111112
self.assertEqual(resp, self.UPDATED_ACCESS_KEY_RESPONSE.json())
113+
114+
@patch("requests.Session.get")
115+
@patch("requests.Session.post")
116+
def test_sanity_of_update_functions(self, post, get):
117+
post.return_value = self.UPDATED_ACCESS_KEY_RESPONSE
118+
get.return_value = self.UPDATED_ACCESS_KEY_RESPONSE
119+
# Ensure at the very least that the other access key functions don't crash when run.
120+
keen.update_access_key_name(self.ACCESS_KEY_ID, name="Marzipan")
121+
keen.add_access_key_permissions(self.ACCESS_KEY_ID, ["writes"])
122+
keen.remove_access_key_permissions(self.ACCESS_KEY_ID, ["writes"])
123+
keen.update_access_key_permissions(self.ACCESS_KEY_ID, ["writes", "cached_queries"])
124+
keen.update_access_key_options(self.ACCESS_KEY_ID, options={})
125+
self.assertTrue(True) # Best test assertion ever.

0 commit comments

Comments
 (0)