diff --git a/buildtrigger/gitlabhandler.py b/buildtrigger/gitlabhandler.py index 7abedc2d0..3decdca07 100644 --- a/buildtrigger/gitlabhandler.py +++ b/buildtrigger/gitlabhandler.py @@ -309,9 +309,22 @@ class GitLabBuildTrigger(BuildTriggerHandler): def repo_view(repo): # Because *anything* can be None in GitLab API! permissions = repo.get('permissions') or {} + group_access = permissions.get('group_access') or {} project_access = permissions.get('project_access') or {} - access_level = project_access.get('access_level') or 0 + + missing_group_access = permissions.get('group_access') is None + missing_project_access = permissions.get('project_access') is None + + access_level = max(group_access.get('access_level') or 0, + project_access.get('access_level') or 0) + has_admin_permission = _ACCESS_LEVEL_MAP.get(access_level, ("", False))[1] + if missing_group_access or missing_project_access: + # Default to has permission if we cannot check the permissions. This will allow our users + # to select the repository and then GitLab's own checks will ensure that the webhook is + # added only if allowed. + # TODO: Do we want to display this differently in the UI? + has_admin_permission = True view = { 'name': repo['path'], diff --git a/buildtrigger/test/gitlabmock.py b/buildtrigger/test/gitlabmock.py index c15f42a6b..9ac201b10 100644 --- a/buildtrigger/test/gitlabmock.py +++ b/buildtrigger/test/gitlabmock.py @@ -12,7 +12,7 @@ def get_gitlab_trigger(dockerfile_path=''): 'username': 'knownuser' }) - trigger._get_authorized_client = get_mock_gitlab(with_nullavatar=False) + trigger._get_authorized_client = get_mock_gitlab(with_nulls=False) return trigger def adddeploykey_mock(project_id, name, public_key): @@ -29,10 +29,15 @@ def get_currentuser_mock(): def project(namespace, name, is_org=False): project_access = None - if namespace == 'knownuser': - project_access = { - 'access_level': 50, - } + if name != 'null': + if namespace == 'knownuser': + project_access = { + 'access_level': 50, + } + else: + project_access = { + 'access_level': 0, + } data = { 'id': '%s/%s' % (namespace, name), @@ -51,13 +56,14 @@ def project(namespace, name, is_org=False): 'public': name != 'somerepo', 'permissions': { 'project_access': project_access, + 'group_access': {'access_level': 0}, }, 'owner': { 'avatar_url': 'avatarurl', } } - if name == 'nullavatar': + if name == 'null': del data['owner']['avatar_url'] data['namespace']['avatar'] = None elif is_org: @@ -66,11 +72,11 @@ def project(namespace, name, is_org=False): return data -def getprojects_mock(with_nullavatar=False): - if with_nullavatar: +def getprojects_mock(with_nulls=False): + if with_nulls: def _getprojs(page=1, per_page=100): return [ - project('someorg', 'nullavatar', is_org=True), + project('someorg', 'null', is_org=True), ] return _getprojs @@ -183,7 +189,7 @@ def getrawfile_mock(repo_id, branch_name, path): return False -def get_mock_gitlab(with_nullavatar=False): +def get_mock_gitlab(with_nulls=False): def _get_mock(): mock_gitlab = Mock() mock_gitlab.host = 'https://bitbucket.org' @@ -191,7 +197,7 @@ def get_mock_gitlab(with_nullavatar=False): mock_gitlab.currentuser = Mock(side_effect=get_currentuser_mock) mock_gitlab.getusers = Mock(side_effect=getusers_mock) - mock_gitlab.getprojects = Mock(side_effect=getprojects_mock(with_nullavatar)) + mock_gitlab.getprojects = Mock(side_effect=getprojects_mock(with_nulls)) mock_gitlab.getproject = Mock(side_effect=getproject_mock) mock_gitlab.getbranches = Mock(side_effect=getbranches_mock) diff --git a/buildtrigger/test/test_gitlabhandler.py b/buildtrigger/test/test_gitlabhandler.py index f8a8931c6..fae6f4989 100644 --- a/buildtrigger/test/test_gitlabhandler.py +++ b/buildtrigger/test/test_gitlabhandler.py @@ -37,8 +37,15 @@ def test_lookup_user(email, expected_response, gitlab_trigger): assert gitlab_trigger.lookup_user(email) == expected_response +def test_null_permissions(gitlab_trigger): + gitlab_trigger._get_authorized_client = get_mock_gitlab(with_nulls=True) + sources = gitlab_trigger.list_build_sources_for_namespace('someorg') + source = sources[0] + assert source['has_admin_permissions'] + + def test_null_avatar(gitlab_trigger): - gitlab_trigger._get_authorized_client = get_mock_gitlab(with_nullavatar=True) + gitlab_trigger._get_authorized_client = get_mock_gitlab(with_nulls=True) namespace_data = gitlab_trigger.list_build_source_namespaces() expected = { 'avatar_url': None,