From fc4a1f47de2efec1483f892c5c66a47b791a17df Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Wed, 30 Oct 2024 12:57:05 +0000 Subject: [PATCH 1/5] RHCLOUD-31117 Added ternary operator to handle no username being passed when filtering Principals --- rbac/management/principal/it_service.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rbac/management/principal/it_service.py b/rbac/management/principal/it_service.py index 31871fde..628d101d 100644 --- a/rbac/management/principal/it_service.py +++ b/rbac/management/principal/it_service.py @@ -250,10 +250,11 @@ def get_service_accounts(self, user: User, options: dict[str, Any] = {}) -> Tupl if specified_usernames: usernames = specified_usernames.split(",") - # If "match_criteria" is specified, only the first username is taken into account. + # If "match_criteria" is specified, only the first username is taken into account if a username is provided, + # if no user is specified the current user is provided for username to subvert 500 error. match_criteria = options.get("match_criteria") if match_criteria: - username = usernames[0] + username = user.username if not usernames else usernames[0] if match_criteria == "partial": service_account_principals = service_account_principals.filter(username__startswith=username) From ba6cbf7dd7eced23724ec4c93d13b5e21acc506c Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Fri, 1 Nov 2024 16:30:07 +0000 Subject: [PATCH 2/5] Updated filtering to skip if unless match_criteria and usernames are provided & added tests for each option when filtering --- rbac/management/principal/it_service.py | 8 +-- tests/management/principal/test_it_service.py | 50 +++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/rbac/management/principal/it_service.py b/rbac/management/principal/it_service.py index 628d101d..8a40aaeb 100644 --- a/rbac/management/principal/it_service.py +++ b/rbac/management/principal/it_service.py @@ -250,11 +250,11 @@ def get_service_accounts(self, user: User, options: dict[str, Any] = {}) -> Tupl if specified_usernames: usernames = specified_usernames.split(",") - # If "match_criteria" is specified, only the first username is taken into account if a username is provided, - # if no user is specified the current user is provided for username to subvert 500 error. + # If "match_criteria" is specified and the usernames list is not empty, + # only the first username is taken into account match_criteria = options.get("match_criteria") - if match_criteria: - username = user.username if not usernames else usernames[0] + if match_criteria and usernames: + username = usernames[0] if match_criteria == "partial": service_account_principals = service_account_principals.filter(username__startswith=username) diff --git a/tests/management/principal/test_it_service.py b/tests/management/principal/test_it_service.py index 0a8d94c6..25b2398c 100644 --- a/tests/management/principal/test_it_service.py +++ b/tests/management/principal/test_it_service.py @@ -1570,3 +1570,53 @@ def test_merge_principals_it_service_accounts(self) -> None: f' "{expected_first_username}" or "{expected_second_username}", got the following service account:' f" {filtered_result}", ) + + @override_settings(IT_BYPASS_IT_CALLS=True) + def test_principal_filtering_without_username_match_criteria_exact_skipped(self): + """Test the function under test skips the expected service account when filtering by exact match criteria without usernames""" + user = User() + user.account = self.tenant.account_id + user.org_id = self.tenant.org_id + + # Set the options for the filter. + options = { + "limit": 10, + "offset": 0, + "match_criteria": "exact", + "usernames": None, + } + + # Call the function under test. + result, count = self.it_service.get_service_accounts(user=user, options=options) + print(result) + print(count) + self.assertEqual( + 0, + count, + "unexpected number of service accounts fetched for the tenant", + ) + + @override_settings(IT_BYPASS_IT_CALLS=True) + def test_principal_filtering_without_username_match_criteria_partial_skipped(self): + """Test the function under test skips the expected service account when match_criteria is exact without usernames""" + user = User() + user.account = self.tenant.account_id + user.org_id = self.tenant.org_id + + # Set the options for the filter. + options = { + "limit": 10, + "offset": 0, + "match_criteria": "partial", + "usernames": None, + } + + # Call the function under test. + result, count = self.it_service.get_service_accounts(user=user, options=options) + print(result) + print(count) + self.assertEqual( + 0, + count, + "unexpected number of service accounts fetched for the tenant", + ) From 95757ff3d36556cb4d27c83ac06ea888cc4f2047 Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Wed, 30 Oct 2024 12:57:05 +0000 Subject: [PATCH 3/5] RHCLOUD-31117 Added ternary operator to handle no username being passed when filtering Principals --- rbac/management/principal/it_service.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rbac/management/principal/it_service.py b/rbac/management/principal/it_service.py index ac05c229..e11fa003 100644 --- a/rbac/management/principal/it_service.py +++ b/rbac/management/principal/it_service.py @@ -250,10 +250,11 @@ def get_service_accounts(self, user: User, options: dict[str, Any] = {}) -> Tupl if specified_usernames: usernames = specified_usernames.split(",") - # If "match_criteria" is specified, only the first username is taken into account. + # If "match_criteria" is specified, only the first username is taken into account if a username is provided, + # if no user is specified the current user is provided for username to subvert 500 error. match_criteria = options.get("match_criteria") if match_criteria: - username = usernames[0] + username = user.username if not usernames else usernames[0] if match_criteria == "partial": service_account_principals = service_account_principals.filter(username__startswith=username) From 55839147e42860de82aa1252825b780a8bfd5e66 Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Fri, 1 Nov 2024 16:30:07 +0000 Subject: [PATCH 4/5] Updated filtering to skip if unless match_criteria and usernames are provided & added tests for each option when filtering --- rbac/management/principal/it_service.py | 8 +-- tests/management/principal/test_it_service.py | 50 +++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/rbac/management/principal/it_service.py b/rbac/management/principal/it_service.py index e11fa003..c7788e0e 100644 --- a/rbac/management/principal/it_service.py +++ b/rbac/management/principal/it_service.py @@ -250,11 +250,11 @@ def get_service_accounts(self, user: User, options: dict[str, Any] = {}) -> Tupl if specified_usernames: usernames = specified_usernames.split(",") - # If "match_criteria" is specified, only the first username is taken into account if a username is provided, - # if no user is specified the current user is provided for username to subvert 500 error. + # If "match_criteria" is specified and the usernames list is not empty, + # only the first username is taken into account match_criteria = options.get("match_criteria") - if match_criteria: - username = user.username if not usernames else usernames[0] + if match_criteria and usernames: + username = usernames[0] if match_criteria == "partial": service_account_principals = service_account_principals.filter(username__startswith=username) diff --git a/tests/management/principal/test_it_service.py b/tests/management/principal/test_it_service.py index 0a8d94c6..25b2398c 100644 --- a/tests/management/principal/test_it_service.py +++ b/tests/management/principal/test_it_service.py @@ -1570,3 +1570,53 @@ def test_merge_principals_it_service_accounts(self) -> None: f' "{expected_first_username}" or "{expected_second_username}", got the following service account:' f" {filtered_result}", ) + + @override_settings(IT_BYPASS_IT_CALLS=True) + def test_principal_filtering_without_username_match_criteria_exact_skipped(self): + """Test the function under test skips the expected service account when filtering by exact match criteria without usernames""" + user = User() + user.account = self.tenant.account_id + user.org_id = self.tenant.org_id + + # Set the options for the filter. + options = { + "limit": 10, + "offset": 0, + "match_criteria": "exact", + "usernames": None, + } + + # Call the function under test. + result, count = self.it_service.get_service_accounts(user=user, options=options) + print(result) + print(count) + self.assertEqual( + 0, + count, + "unexpected number of service accounts fetched for the tenant", + ) + + @override_settings(IT_BYPASS_IT_CALLS=True) + def test_principal_filtering_without_username_match_criteria_partial_skipped(self): + """Test the function under test skips the expected service account when match_criteria is exact without usernames""" + user = User() + user.account = self.tenant.account_id + user.org_id = self.tenant.org_id + + # Set the options for the filter. + options = { + "limit": 10, + "offset": 0, + "match_criteria": "partial", + "usernames": None, + } + + # Call the function under test. + result, count = self.it_service.get_service_accounts(user=user, options=options) + print(result) + print(count) + self.assertEqual( + 0, + count, + "unexpected number of service accounts fetched for the tenant", + ) From c2e30dcaf7df0008e6f4531c0fc9e49a6931a4a4 Mon Sep 17 00:00:00 2001 From: EvanCasey13 Date: Thu, 9 Jan 2025 12:17:51 +0000 Subject: [PATCH 5/5] docstring fix for test --- tests/management/principal/test_it_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/management/principal/test_it_service.py b/tests/management/principal/test_it_service.py index 25b2398c..3064a3c0 100644 --- a/tests/management/principal/test_it_service.py +++ b/tests/management/principal/test_it_service.py @@ -1598,7 +1598,7 @@ def test_principal_filtering_without_username_match_criteria_exact_skipped(self) @override_settings(IT_BYPASS_IT_CALLS=True) def test_principal_filtering_without_username_match_criteria_partial_skipped(self): - """Test the function under test skips the expected service account when match_criteria is exact without usernames""" + """Test the function under test skips the expected service account when match_criteria is partial without usernames""" user = User() user.account = self.tenant.account_id user.org_id = self.tenant.org_id