Have gitlab default to True on permissions if they are missing

This allows the repositories to be selected in the UI, if we are unsure whether the user has permission. Since gitlab will do the check anyway, this is safe, although not a great user experience if they chose an invalid repository, but we can't really do much about that.
This commit is contained in:
Joseph Schorr 2017-05-02 13:11:57 -04:00
parent a78d5fb9ff
commit bf41aedc9c
3 changed files with 39 additions and 13 deletions

View file

@ -309,9 +309,22 @@ class GitLabBuildTrigger(BuildTriggerHandler):
def repo_view(repo): def repo_view(repo):
# Because *anything* can be None in GitLab API! # Because *anything* can be None in GitLab API!
permissions = repo.get('permissions') or {} permissions = repo.get('permissions') or {}
group_access = permissions.get('group_access') or {}
project_access = permissions.get('project_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] 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 = { view = {
'name': repo['path'], 'name': repo['path'],

View file

@ -12,7 +12,7 @@ def get_gitlab_trigger(dockerfile_path=''):
'username': 'knownuser' 'username': 'knownuser'
}) })
trigger._get_authorized_client = get_mock_gitlab(with_nullavatar=False) trigger._get_authorized_client = get_mock_gitlab(with_nulls=False)
return trigger return trigger
def adddeploykey_mock(project_id, name, public_key): def adddeploykey_mock(project_id, name, public_key):
@ -29,10 +29,15 @@ def get_currentuser_mock():
def project(namespace, name, is_org=False): def project(namespace, name, is_org=False):
project_access = None project_access = None
if name != 'null':
if namespace == 'knownuser': if namespace == 'knownuser':
project_access = { project_access = {
'access_level': 50, 'access_level': 50,
} }
else:
project_access = {
'access_level': 0,
}
data = { data = {
'id': '%s/%s' % (namespace, name), 'id': '%s/%s' % (namespace, name),
@ -51,13 +56,14 @@ def project(namespace, name, is_org=False):
'public': name != 'somerepo', 'public': name != 'somerepo',
'permissions': { 'permissions': {
'project_access': project_access, 'project_access': project_access,
'group_access': {'access_level': 0},
}, },
'owner': { 'owner': {
'avatar_url': 'avatarurl', 'avatar_url': 'avatarurl',
} }
} }
if name == 'nullavatar': if name == 'null':
del data['owner']['avatar_url'] del data['owner']['avatar_url']
data['namespace']['avatar'] = None data['namespace']['avatar'] = None
elif is_org: elif is_org:
@ -66,11 +72,11 @@ def project(namespace, name, is_org=False):
return data return data
def getprojects_mock(with_nullavatar=False): def getprojects_mock(with_nulls=False):
if with_nullavatar: if with_nulls:
def _getprojs(page=1, per_page=100): def _getprojs(page=1, per_page=100):
return [ return [
project('someorg', 'nullavatar', is_org=True), project('someorg', 'null', is_org=True),
] ]
return _getprojs return _getprojs
@ -183,7 +189,7 @@ def getrawfile_mock(repo_id, branch_name, path):
return False return False
def get_mock_gitlab(with_nullavatar=False): def get_mock_gitlab(with_nulls=False):
def _get_mock(): def _get_mock():
mock_gitlab = Mock() mock_gitlab = Mock()
mock_gitlab.host = 'https://bitbucket.org' 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.currentuser = Mock(side_effect=get_currentuser_mock)
mock_gitlab.getusers = Mock(side_effect=getusers_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.getproject = Mock(side_effect=getproject_mock)
mock_gitlab.getbranches = Mock(side_effect=getbranches_mock) mock_gitlab.getbranches = Mock(side_effect=getbranches_mock)

View file

@ -37,8 +37,15 @@ def test_lookup_user(email, expected_response, gitlab_trigger):
assert gitlab_trigger.lookup_user(email) == expected_response 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): 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() namespace_data = gitlab_trigger.list_build_source_namespaces()
expected = { expected = {
'avatar_url': None, 'avatar_url': None,