diff --git a/src/appengine/handlers/configuration.py b/src/appengine/handlers/configuration.py index 3a699e79e5c..3a6b2046cee 100644 --- a/src/appengine/handlers/configuration.py +++ b/src/appengine/handlers/configuration.py @@ -120,6 +120,7 @@ def post(self): 'oss_fuzz_robot_github_personal_access_token') platform_group_mappings = request.get('platform_group_mappings') privileged_users = request.get('privileged_users') + privileged_groups = request.get('privileged_groups') blacklisted_users = request.get('blacklisted_users') relax_security_bug_restrictions = request.get( 'relax_security_bug_restrictions') @@ -146,6 +147,7 @@ def post(self): config.jira_url = jira_url config.platform_group_mappings = platform_group_mappings config.privileged_users = privileged_users + config.privileged_groups = privileged_groups config.blacklisted_users = blacklisted_users config.relax_security_bug_restrictions = bool( relax_security_bug_restrictions) diff --git a/src/appengine/libs/access.py b/src/appengine/libs/access.py index bf54ea82905..864860e522d 100644 --- a/src/appengine/libs/access.py +++ b/src/appengine/libs/access.py @@ -29,11 +29,33 @@ def _is_privileged_user(email): if local_config.AuthConfig().get('all_users_privileged'): return True + # Check privileged access from direct user emails. privileged_user_emails = (db_config.get_value('privileged_users') or '').splitlines() - return any( + if any( utils.emails_equal(email, privileged_user_email) - for privileged_user_email in privileged_user_emails) + for privileged_user_email in privileged_user_emails): + return True + + # Check privileged access from google groups. + privileged_groups = (db_config.get_value('privileged_groups') or + '').splitlines() + identity_service = auth.get_identity_api() + for privileged_group in privileged_groups: + # Filter for non-group patterns. + if ('@' not in privileged_group or + utils.is_service_account(privileged_group)): + continue + + group_id = auth.get_google_group_id(privileged_group, identity_service) + if not group_id: + continue + + if auth.check_transitive_group_membership(group_id, email, + identity_service): + return True + + return False def _is_blacklisted_user(email): diff --git a/src/appengine/libs/auth.py b/src/appengine/libs/auth.py index 9188bfeab01..eaff7423471 100644 --- a/src/appengine/libs/auth.py +++ b/src/appengine/libs/auth.py @@ -14,12 +14,14 @@ """Authentication helpers.""" import collections +from urllib import parse from firebase_admin import auth from google.auth.transport import requests as google_requests from google.cloud import ndb from google.oauth2 import id_token -from googleapiclient.discovery import build +from googleapiclient import discovery +from googleapiclient.errors import HttpError import jwt import requests @@ -27,6 +29,7 @@ from clusterfuzz._internal.base import utils from clusterfuzz._internal.config import local_config from clusterfuzz._internal.datastore import data_types +from clusterfuzz._internal.google_cloud_utils import credentials from clusterfuzz._internal.metrics import logs from clusterfuzz._internal.system import environment from libs import helpers @@ -74,7 +77,7 @@ def is_current_user_admin(): @memoize.wrap(memoize.FifoInMemory(1)) def _project_number_from_id(project_id): """Get the project number from project ID.""" - resource_manager = build('cloudresourcemanager', 'v1') + resource_manager = discovery.build('cloudresourcemanager', 'v1') # pylint: disable=no-member result = resource_manager.projects().get(projectId=project_id).execute() if 'projectNumber' not in result: @@ -249,3 +252,48 @@ def decode_claims(session_cookie): return auth.verify_session_cookie(session_cookie, check_revoked=True) except (ValueError, auth.AuthError): raise AuthError('Invalid session cookie.') + + +def get_identity_api() -> discovery.Resource: + """Return cloud identity api client.""" + creds, _ = credentials.get_default() + return discovery.build('cloudidentity', 'v1', credentials=creds) + + +def get_google_group_id(group_email: str, + identity_service: discovery.Resource | None = None + ) -> str | None: + """Retrive a google group ID.""" + if not identity_service: + identity_service = get_identity_api() + + try: + request = identity_service.groups().lookup(groupKey_id=group_email) + response = request.execute() + return response.get('name') + except HttpError: + logs.warning(f"Unable to look up group {group_email}.") + return None + + +def check_transitive_group_membership( + group_id: str, + member: str, + identity_service: discovery.Resource | None = None) -> bool: + """Check if an user is a member of a google group.""" + if not identity_service: + identity_service = get_identity_api() + + try: + query_params = parse.urlencode({ + "query": "member_key_id == '{}'".format(member) + }) + request = identity_service.groups().memberships().checkTransitiveMembership( + parent=group_id) + request.uri += "&" + query_params + response = request.execute() + return response.get('hasMembership', False) + except HttpError: + logs.warning( + f'Unable to check group membership from {member} to {group_id}.') + return False diff --git a/src/appengine/private/components/configuration/configuration.html b/src/appengine/private/components/configuration/configuration.html index d907edfa8cf..a51f54f641f 100644 --- a/src/appengine/private/components/configuration/configuration.html +++ b/src/appengine/private/components/configuration/configuration.html @@ -126,6 +126,17 @@ + + +
+ Privileged groups +
+ + + + + +
diff --git a/src/clusterfuzz/_internal/base/utils.py b/src/clusterfuzz/_internal/base/utils.py index c039de381e8..5cd25d55a49 100644 --- a/src/clusterfuzz/_internal/base/utils.py +++ b/src/clusterfuzz/_internal/base/utils.py @@ -1005,6 +1005,14 @@ def emails_equal(first, second): return normalize_email(first) == normalize_email(second) +def is_service_account(email: str) -> bool: + """Check if an email is a SA based on email address pattern.""" + sa_email = normalize_email(email) + if '@' not in sa_email: + return False + return sa_email.endswith('gserviceaccount.com') + + def parse_delimited(value_or_handle, delimiter, strip=False, remove_empty=False): """Parse a delimter separated value.""" diff --git a/src/clusterfuzz/_internal/datastore/data_types.py b/src/clusterfuzz/_internal/datastore/data_types.py index 47e37bc9dd7..4e5af47dc24 100644 --- a/src/clusterfuzz/_internal/datastore/data_types.py +++ b/src/clusterfuzz/_internal/datastore/data_types.py @@ -818,6 +818,9 @@ class Config(Model): # Privileged users. privileged_users = ndb.TextProperty(default='') + # Privileged groups. + privileged_groups = ndb.TextProperty(default='') + # Blacklisted users. blacklisted_users = ndb.TextProperty(default='') diff --git a/src/clusterfuzz/_internal/system/environment.py b/src/clusterfuzz/_internal/system/environment.py index 0337ddd0519..77b50a9d923 100644 --- a/src/clusterfuzz/_internal/system/environment.py +++ b/src/clusterfuzz/_internal/system/environment.py @@ -1004,7 +1004,7 @@ def set_bot_environment(): def set_local_log_only(): """Force logs to be local-only.""" - # We set this to an empty string because currently the logs + # We set this to an empty string because currently the logs # module does not correctly evaluate env vars # (i.e., 'false' is evaluated to true). set_value('LOG_TO_GCP', '') diff --git a/src/clusterfuzz/_internal/tests/appengine/libs/access_test.py b/src/clusterfuzz/_internal/tests/appengine/libs/access_test.py index c889d9427af..b70dc74473e 100644 --- a/src/clusterfuzz/_internal/tests/appengine/libs/access_test.py +++ b/src/clusterfuzz/_internal/tests/appengine/libs/access_test.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. """access tests.""" + import unittest # pylint: disable=protected-access from unittest import mock @@ -63,21 +64,34 @@ def test_get_none(self): class IsPrivilegedUserTest(unittest.TestCase): """Test _is_privileged_user.""" - _FAKE_CONFIG = 'Test@test.com\n' + _FAKE_CONFIG = { + 'privileged_users': 'Test@test.com\n', + 'privileged_groups': 'test@group.com\n' + } + + def _get_value_mock(self, key): + """Helper to return the config based on the key.""" + return self._FAKE_CONFIG.get(key) def setUp(self): - test_helpers.patch(self, - ['clusterfuzz._internal.config.db_config.get_value']) + test_helpers.patch(self, [ + 'clusterfuzz._internal.config.db_config.get_value', + 'libs.auth.get_google_group_id', + 'libs.auth.check_transitive_group_membership', + 'libs.auth.get_identity_api' + ]) def test_none(self): """Ensure it returns False when config is invalid.""" self.mock.get_value.return_value = None self.assertFalse(access._is_privileged_user('test')) - self.mock.get_value.assert_has_calls([mock.call('privileged_users')]) + self.mock.get_value.assert_has_calls( + [mock.call('privileged_users'), + mock.call('privileged_groups')]) def test_global(self): """Ensure it returns True if an email is permitted globally.""" - self.mock.get_value.return_value = self._FAKE_CONFIG + self.mock.get_value.side_effect = self._get_value_mock self.assertTrue(access._is_privileged_user('test@test.com')) self.mock.get_value.assert_has_calls([mock.call('privileged_users')]) @@ -90,6 +104,40 @@ def test_all_users_privileged(self): }) self.assertTrue(access._is_privileged_user('a@user.com')) + def test_privileged_group(self): + """Test success access for member of privileged group.""" + self.mock.get_value.side_effect = self._get_value_mock + self.mock.get_identity_api.return_value = None + self.mock.get_google_group_id.return_value = 1 + self.mock.check_transitive_group_membership.return_value = True + + self.assertTrue(access._is_privileged_user('usertest@google.com')) + self.mock.get_google_group_id.assert_called_with('test@group.com', None) + self.mock.check_transitive_group_membership.assert_called_with( + 1, 'usertest@google.com', None) + + def test_privileged_group_id_not_available(self): + """Test failed access if privileged group not found.""" + self.mock.get_value.side_effect = self._get_value_mock + self.mock.get_identity_api.return_value = None + self.mock.get_google_group_id.return_value = None + + self.assertFalse(access._is_privileged_user('usertest@google.com')) + self.mock.get_google_group_id.assert_called_with('test@group.com', None) + self.mock.check_transitive_group_membership.assert_not_called() + + def test_not_member_privileged_group(self): + """Test failed access if user not member of privileged group.""" + self.mock.get_value.side_effect = self._get_value_mock + self.mock.get_identity_api.return_value = None + self.mock.get_google_group_id.return_value = 1 + self.mock.check_transitive_group_membership.return_value = False + + self.assertFalse(access._is_privileged_user('usertest@google.com')) + self.mock.get_google_group_id.assert_called_with('test@group.com', None) + self.mock.check_transitive_group_membership.assert_called_with( + 1, 'usertest@google.com', None) + class IsDomainAllowedTest(unittest.TestCase): """Test _is_domain_allowed.""" diff --git a/src/clusterfuzz/_internal/tests/core/base/utils_test.py b/src/clusterfuzz/_internal/tests/core/base/utils_test.py index 09cf6e5491a..69bcfd122a1 100644 --- a/src/clusterfuzz/_internal/tests/core/base/utils_test.py +++ b/src/clusterfuzz/_internal/tests/core/base/utils_test.py @@ -477,6 +477,26 @@ def test_with_domain(self): utils.service_account_email()) +class CheckServiceAccountEmailTest(unittest.TestCase): + """Tests the service account email pattern check. """ + + @parameterized.parameterized.expand([ + 'project-id.domain.com@appspot.gserviceaccount.com', + '123-compute@developer.iam.gserviceaccount.com' + ]) + def test_is_service_account(self, sa_email): + """Test the expected service account email pattern.""" + self.assertTrue(utils.is_service_account(sa_email)) + + @parameterized.parameterized.expand([ + 'test@google.com', 'test@chromium.com', + 'test.not.email.gserviceaccount.com' + ]) + def test_is_not_service_account(self, email): + """Test for email patterns that are not from service accounts.""" + self.assertFalse(utils.is_service_account(email)) + + class SearchBytesInFileTest(unittest.TestCase): """Tests search_bytes_in_file."""