From 967335cccb508d7abc706fd0b82e4200725b8b1f Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Feb 15 2017 10:28:57 +0000 Subject: [PATCH 1/21] Project Level Access Control - Admin, Commit, Ticket --- diff --git a/alembic/versions/987edda096f5_access_id_in_user_projects.py b/alembic/versions/987edda096f5_access_id_in_user_projects.py new file mode 100644 index 0000000..6e053b9 --- /dev/null +++ b/alembic/versions/987edda096f5_access_id_in_user_projects.py @@ -0,0 +1,99 @@ +"""access_id in user_projects + +Revision ID: 987edda096f5 +Revises: 38581a8fbae2 +Create Date: 2016-07-05 18:21:14.771273 + +""" + +# revision identifiers, used by Alembic. +revision = '987edda096f5' +down_revision = '38581a8fbae2' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ''' Add a foreign key in user_projects and projects_groups + table for access_levels + ''' + + op.add_column( + 'user_projects', + sa.Column( + 'access', + sa.String(255), + sa.ForeignKey( + 'access_levels.access', + onupdate='CASCADE', + ondelete='CASCADE', + ), + nullable=True, + ), + ) + op.execute('UPDATE "user_projects" SET access=\'admin\'') + op.alter_column( + 'user_projects', + 'access', + nullable=False, + existing_nullable=True, + ) + + # for groups + op.add_column( + 'projects_groups', + sa.Column( + 'access', + sa.String(255), + sa.ForeignKey( + 'access_levels.access', + onupdate='CASCADE', + ondelete='CASCADE', + ), + nullable=True, + ), + ) + op.execute('UPDATE "projects_groups" SET access=\'admin\'') + op.alter_column( + 'projects_groups', + 'access', + nullable=False, + existing_nullable=True, + ) + + # alter the constraints + op.drop_constraint('user_projects_project_id_key', 'user_projects') + op.create_unique_constraint( + None, + 'user_projects', + ["project_id", "user_id", "access"] + ) + + op.drop_constraint('projects_groups_pkey', 'projects_groups') + op.create_primary_key( + None, + 'projects_groups', + ['project_id', 'group_id', 'access'], + ) + + + +def downgrade(): + ''' Remove column access_id from user_projects and projects_groups ''' + + # this removes the current constraints as well. + op.drop_column('user_projects', 'access') + op.drop_column('projects_groups', 'access') + + # recreate the previous constraints + op.create_unique_constraint( + None, + 'user_projects', + ['project_id', 'user_id'], + ) + op.create_primary_key( + None, + 'projects_groups', + ['project_id', 'group_id'], + ) diff --git a/pagure/__init__.py b/pagure/__init__.py index 2123379..2f7ccdb 100644 --- a/pagure/__init__.py +++ b/pagure/__init__.py @@ -305,6 +305,52 @@ def is_repo_admin(repo_obj): usergrps = [ usr.user + for grp in repo_obj.admin_groups + for usr in grp.users] + + return user == repo_obj.user.user or ( + user in [usr.user for usr in repo_obj.admins] + ) or (user in usergrps) + + +def is_repo_committer(repo_obj): + """ Return whether the user is a committer of the provided repo. """ + if not authenticated(): + return False + + user = flask.g.fas_user.username + + admin_users = APP.config.get('PAGURE_ADMIN_USERS', []) + if not isinstance(admin_users, list): + admin_users = [admin_users] + if user in admin_users: + return True + + usergrps = [ + usr.user + for grp in repo_obj.committer_groups + for usr in grp.users] + + return user == repo_obj.user.user or ( + user in [usr.user for usr in repo_obj.committers] + ) or (user in usergrps) + + +def is_repo_user(repo_obj): + """ Return whether the user has some access in the provided repo. """ + if not authenticated(): + return False + + user = flask.g.fas_user.username + + admin_users = APP.config.get('PAGURE_ADMIN_USERS', []) + if not isinstance(admin_users, list): + admin_users = [admin_users] + if user in admin_users: + return True + + usergrps = [ + usr.user for grp in repo_obj.groups for usr in grp.users] @@ -434,6 +480,8 @@ def set_variables(): flask.g.reponame = get_repo_path(flask.g.repo) flask.g.repo_obj = pygit2.Repository(flask.g.reponame) flask.g.repo_admin = is_repo_admin(flask.g.repo) + flask.g.repo_committer = is_repo_committer(flask.g.repo) + flask.g.repo_user = is_repo_user(flask.g.repo) flask.g.branches = sorted(flask.g.repo_obj.listall_branches()) items_per_page = APP.config['ITEM_PER_PAGE'] diff --git a/pagure/api/fork.py b/pagure/api/fork.py index 9bc5083..043833c 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -15,7 +15,7 @@ from sqlalchemy.exc import SQLAlchemyError import pagure import pagure.exceptions import pagure.lib -from pagure import APP, SESSION, is_repo_admin +from pagure import APP, SESSION, is_repo_committer from pagure.api import API, api_method, api_login_required, APIERROR @@ -327,7 +327,7 @@ def api_pull_request_merge(repo, requestid, username=None, namespace=None): if not request: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOREQ) - if not is_repo_admin(repo): + if not is_repo_committer(repo): raise pagure.exceptions.APIError(403, error_code=APIERROR.ENOPRCLOSE) if repo.settings.get('Only_assignee_can_merge_pull-request', False): @@ -414,7 +414,7 @@ def api_pull_request_close(repo, requestid, username=None, namespace=None): if not request: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOREQ) - if not is_repo_admin(repo): + if not is_repo_committer(repo): raise pagure.exceptions.APIError(403, error_code=APIERROR.ENOPRCLOSE) try: diff --git a/pagure/api/issue.py b/pagure/api/issue.py index afc3b18..31afab4 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -16,9 +16,9 @@ from sqlalchemy.exc import SQLAlchemyError import pagure import pagure.exceptions import pagure.lib -from pagure import ( - APP, SESSION, is_repo_admin, api_authenticated, urlpattern -) + +from pagure import APP, SESSION, is_repo_committer, api_authenticated + from pagure.api import ( API, api_method, api_login_required, api_login_optional, APIERROR ) @@ -288,8 +288,8 @@ def api_view_issues(repo, username=None, namespace=None): raise pagure.exceptions.APIError( 401, error_code=APIERROR.EINVALIDTOK) private = flask.g.fas_user.username - # If user is repo admin, show all tickets included the private ones - if is_repo_admin(repo): + # If user is repo committer, show all tickets included the private ones + if is_repo_committer(repo): private = None if status is not None: @@ -426,7 +426,7 @@ def api_view_issue(repo, issueid, username=None, namespace=None): raise pagure.exceptions.APIError( 401, error_code=APIERROR.EINVALIDTOK) - if issue.private and not is_repo_admin(repo) \ + if issue.private and not is_repo_committer(repo) \ and (not api_authenticated() or not issue.user.user == flask.g.fas_user.username): raise pagure.exceptions.APIError( @@ -513,7 +513,7 @@ def api_view_issue_comment( raise pagure.exceptions.APIError( 401, error_code=APIERROR.EINVALIDTOK) - if issue.private and not is_repo_admin(issue.project) \ + if issue.private and not is_repo_committer(issue.project) \ and (not api_authenticated() or not issue.user.user == flask.g.fas_user.username): raise pagure.exceptions.APIError( @@ -604,7 +604,7 @@ def api_change_status_issue(repo, issueid, username=None, namespace=None): if issue is None or issue.project != repo: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE) - if issue.private and not is_repo_admin(repo) \ + if issue.private and not is_repo_committer(repo) \ and (not api_authenticated() or not issue.user.user == flask.g.fas_user.username): raise pagure.exceptions.APIError( @@ -731,7 +731,7 @@ def api_comment_issue(repo, issueid, username=None, namespace=None): if issue is None or issue.project != repo: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE) - if issue.private and not is_repo_admin(repo) \ + if issue.private and not is_repo_committer(repo) \ and (not api_authenticated() or not issue.user.user == flask.g.fas_user.username): raise pagure.exceptions.APIError( @@ -830,7 +830,7 @@ def api_assign_issue(repo, issueid, username=None, namespace=None): if issue is None or issue.project != repo: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE) - if issue.private and not is_repo_admin(repo) \ + if issue.private and not is_repo_committer(repo) \ and (not api_authenticated() or not issue.user.user == flask.g.fas_user.username): raise pagure.exceptions.APIError( diff --git a/pagure/forms.py b/pagure/forms.py index a9cc117..1e88704 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -439,6 +439,18 @@ class AddUserForm(PagureForm): 'Username *', [wtforms.validators.Required()] ) + access = wtforms.TextField( + 'Access Level *', + [wtforms.validators.Required()] + ) + + +class AddUserToGroupForm(PagureForm): + ''' Form to add a user to a pagure group. ''' + user = wtforms.TextField( + 'Username *', + [wtforms.validators.Required()] + ) class AssignIssueForm(PagureForm): @@ -458,6 +470,10 @@ class AddGroupForm(PagureForm): wtforms.validators.Regexp(STRICT_REGEX, flags=re.IGNORECASE) ] ) + access = wtforms.TextField( + 'Access Level *', + [wtforms.validators.Required()] + ) class ConfirmationForm(PagureForm): diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index c338725..a360322 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -121,7 +121,7 @@ def pull_request_add_comment(): @PV.route('/ticket/comment/', methods=['PUT']) @localonly def ticket_add_comment(): - """ Add a comment to a pull-request. + """ Add a comment to an issue. """ pform = pagure.forms.ProjectCommentForm(csrf_enabled=False) if not pform.validate_on_submit(): @@ -142,7 +142,7 @@ def ticket_add_comment(): admin = False if user_obj: admin = user_obj == issue.project.user.user or ( - user_obj in [user.user for user in issue.project.users]) + user_obj in [user.user for user in issue.project.committers]) if issue.private and user_obj and not admin \ and not issue.user.user == user_obj.username: diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 4657093..485fd52 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -918,21 +918,46 @@ def add_deploykey_to_project(session, project, ssh_key, pushaccess, user): return 'Deploy key added' -def add_user_to_project(session, project, new_user, user): - ''' Add a specified user to a specified project. ''' +def add_user_to_project(session, project, new_user, user, access='admin'): + ''' Add a specified user to a specified project with a specified access''' + new_user_obj = get_user(session, new_user) user_obj = get_user(session, user) - users = set([user.user for user in project.users]) + users = set([user_.user for user_ in + get_project_users(session, project, access, combine=False)]) users.add(project.user.user) + if new_user in users: raise pagure.exceptions.PagureException( - 'This user is already listed on this project.' + 'This user is already listed on this project with the same access' + ) + + # user has some access on project, so update to new access + if new_user_obj in project.users: + access_obj = get_obj_access(session, project, new_user_obj) + access_obj.access = access + session.add(access_obj) + session.flush() + + pagure.lib.notify.log( + project, + topic='project.user.access.updated', + msg=dict( + project=project.to_json(public=True), + new_user=new_user_obj.username, + new_access=access, + agent=user_obj.username, + ), + redis=REDIS, ) + return 'User access updated' + project_user = model.ProjectUser( project_id=project.id, user_id=new_user_obj.id, + access=access, ) session.add(project_user) # Make sure we won't have SQLAlchemy error before we continue @@ -944,6 +969,7 @@ def add_user_to_project(session, project, new_user, user): msg=dict( project=project.to_json(public=True), new_user=new_user_obj.username, + access=access, agent=user_obj.username, ), redis=REDIS, @@ -953,8 +979,10 @@ def add_user_to_project(session, project, new_user, user): def add_group_to_project( - session, project, new_group, user, create=False, is_admin=False): - ''' Add a specified group to a specified project. ''' + session, project, new_group, user, access='admin', + create=False, is_admin=False): + ''' Add a specified group to a specified project with some access ''' + user_obj = search_user(session, username=user) if not user_obj: raise pagure.exceptions.PagureException( @@ -984,15 +1012,39 @@ def add_group_to_project( 'You are not allowed to add a group of users to this project' ) - groups = set([group.group_name for group in project.groups]) + groups = set([group.group_name for group in + get_project_groups(session, project, access, combine=False)]) + if new_group in groups: raise pagure.exceptions.PagureException( - 'This group is already associated to this project.' + 'This group already has this access on this project' + ) + + # the group already has some access, update to new access + if group_obj in project.groups: + access_obj = get_obj_access(session, project, group_obj) + access_obj.access = access + session.add(access_obj) + session.flush() + + pagure.lib.notify.log( + project, + topic='project.group.access.updated', + msg=dict( + project=project.to_json(public=True), + new_group=group_obj.group_name, + new_access=access, + agent=user, + ), + redis=REDIS, ) + return 'Group access updated' + project_group = model.ProjectGroup( project_id=project.id, group_id=group_obj.id, + access=access, ) session.add(project_group) # Make sure we won't have SQLAlchemy error before we continue @@ -1004,6 +1056,7 @@ def add_group_to_project( msg=dict( project=project.to_json(public=True), new_group=group_obj.group_name, + access=access, agent=user, ), redis=REDIS, @@ -1816,29 +1869,31 @@ def search_projects( sub_q2 = session.query( model.Project.id ).filter( - # User got commit right + # User got admin rights sqlalchemy.and_( model.User.user == username, model.User.id == model.ProjectUser.user_id, - model.ProjectUser.project_id == model.Project.id + model.ProjectUser.project_id == model.Project.id, + model.ProjectUser.access == 'admin' ) ) sub_q3 = session.query( model.Project.id ).filter( - # User created a group that has commit right + # User created a group that has admin rights sqlalchemy.and_( model.User.user == username, model.PagureGroup.user_id == model.User.id, model.PagureGroup.group_type == 'user', model.PagureGroup.id == model.ProjectGroup.group_id, model.Project.id == model.ProjectGroup.project_id, + model.ProjectGroup.access == 'admin', ) ) sub_q4 = session.query( model.Project.id ).filter( - # User is part of a group that has commit right + # User is part of a group that has admin right sqlalchemy.and_( model.User.user == username, model.PagureUserGroup.user_id == model.User.id, @@ -1846,6 +1901,7 @@ def search_projects( model.PagureGroup.group_type == 'user', model.PagureGroup.id == model.ProjectGroup.group_id, model.Project.id == model.ProjectGroup.project_id, + model.ProjectGroup.access == 'admin', ) ) @@ -3300,7 +3356,11 @@ def get_pull_request_of_user(session, username): sqlalchemy.and_( model.User.user == username, model.User.id == model.ProjectUser.user_id, - model.ProjectUser.project_id == model.Project.id + model.ProjectUser.project_id == model.Project.id, + sqlalchemy.or_( + model.ProjectUser.access == 'admin', + model.ProjectUser.access == 'commit', + ) ) ) sub_q3 = session.query( @@ -3313,6 +3373,10 @@ def get_pull_request_of_user(session, username): model.PagureGroup.group_type == 'user', model.PagureGroup.id == model.ProjectGroup.group_id, model.Project.id == model.ProjectGroup.project_id, + sqlalchemy.or_( + model.ProjectGroup.access == 'admin', + model.ProjectGroup.access == 'commit', + ) ) ) sub_q4 = session.query( @@ -3326,6 +3390,10 @@ def get_pull_request_of_user(session, username): model.PagureGroup.group_type == 'user', model.PagureGroup.id == model.ProjectGroup.group_id, model.Project.id == model.ProjectGroup.project_id, + sqlalchemy.or_( + model.ProjectGroup.access == 'admin', + model.ProjectGroup.access == 'commit', + ) ) ) @@ -3901,3 +3969,132 @@ def tokenize_search_string(pattern): remaining += finalize_token(token, custom_search) return custom_search, remaining.strip() + + +def get_access_levels(session): + ''' Returns all the access levels a user/group can have for a project ''' + + access_level_objs = session.query(model.AccessLevels).all() + return [access_level.access for access_level in access_level_objs] + + +def get_project_users(session, project_obj, access, combine=True): + ''' Returns the list of users/groups of the project according + to the given access. + + :arg session: the session to use to connect to the database. + :arg project_obj: SQLAlchemy object of Project class. + :arg access: the access level to query for, can be: 'admin', + 'commit' or 'ticket'. + :type access: string + :arg combine: The access levels have some hierarchy - + like: all the users having commit access also has + ticket access and the admins have all the access + that commit and ticket access users have. If combine + is set to False, this function will only return those + users which have the given access and no other access. + ex: if access is 'ticket' and combine is True, it will + return all the users with ticket access which includes + all the committers and admins. If combine were False, + it would have returned only the users with ticket access + and would not have included committers and admins. + :type combine: boolean + ''' + + if access not in ['admin', 'commit', 'ticket']: + return None + + if combine: + if access == 'admin': + return project_obj.admins + elif access == 'commit': + return project_obj.committers + elif access == 'ticket': + return project_obj.users + else: + if access == 'admin': + return project_obj.admins + elif access == 'commit': + committers = set(project_obj.committers) + admins = set(project_obj.admins) + return list(committers - admins) + elif access == 'ticket': + committers = set(project_obj.committers) + admins = set(project_obj.admins) + users = set(project_obj.users) + return list(users - committers - admins) + + +def get_project_groups(session, project_obj, access, combine=True): + ''' Returns the list of groups of the project according + to the given access. + + :arg session: the session to use to connect to the database. + :arg project_obj: SQLAlchemy object of Project class. + :arg access: the access level to query for, can be: 'admin', + 'commit' or 'ticket'. + :type access: string + :arg combine: The access levels have some hierarchy - + like: all the groups having commit access also has + ticket access and the admin_groups have all the access + that committer_groups and ticket access groups have. + If combine is set to False, this function will only return + those groups which have the given access and no other access. + ex: if access is 'ticket' and combine is True, it will + return all the groups with ticket access which includes + all the committer_groups and admin_groups. If combine were False, + it would have returned only the groups with ticket access + and would not have included committer_groups and admin_groups. + :type combine: boolean + ''' + + if access not in ['admin', 'commit', 'ticket']: + return None + + if combine: + if access == 'admin': + return project_obj.admin_groups + elif access == 'commit': + return project_obj.committer_groups + elif access == 'ticket': + return project_obj.groups + else: + if access == 'admin': + return project_obj.admin_groups + elif access == 'commit': + committers = set(project_obj.committer_groups) + admins = set(project_obj.admin_groups) + return list(committers - admins) + elif access == 'ticket': + committers = set(project_obj.committer_groups) + admins = set(project_obj.admin_groups) + groups = set(project_obj.groups) + return list(groups - committers - admins) + + +def get_obj_access(session, project_obj, obj): + ''' Returns the level of access the user/group has on the project. + + :arg session: the session to use to connect to the database. + :arg project_obj: SQLAlchemy object of Project class + :arg obj: SQLAlchemy object of either User or PagureGroup class + ''' + + if isinstance(obj, model.User): + query = session.query( + model.ProjectUser + ).filter( + model.ProjectUser.project_id == project_obj.id + ).filter( + model.ProjectUser.user_id == obj.id + ) + else: + query = session.query( + model.ProjectGroup + ).filter( + model.ProjectGroup.project_id == project_obj.id + ).filter( + model.ProjectGroup.group_id == obj.id + ) + + return query.first() diff --git a/pagure/lib/git.py b/pagure/lib/git.py index d81d2ea..6156f9e 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -91,7 +91,7 @@ def write_gitolite_acls(session, configfile): config = [] groups = {} for project in session.query(model.Project).all(): - for group in project.groups: + for group in project.committer_groups: if group.group_name not in groups: groups[group.group_name] = [ user.username for user in group.users] @@ -103,11 +103,11 @@ def write_gitolite_acls(session, configfile): config.append('repo %s%s' % (repos, project.fullname)) if repos not in ['tickets/', 'requests/']: config.append(' R = @all') - if project.groups: + if project.committer_groups: config.append(' RW+ = @%s' % ' @'.join([ - group.group_name for group in project.groups])) + group.group_name for group in project.committer_groups])) config.append(' RW+ = %s' % project.user.user) - for user in project.users: + for user in project.committers: if user != project.user: config.append(' RW+ = %s' % user.user) for deploykey in project.deploykeys: diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 4815849..1950b85 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -142,6 +142,22 @@ def create_default_status(session, acls=None): session.rollback() ERROR_LOG.debug('ACL %s could not be added', acl) + for access in ['ticket', 'commit', 'admin']: + access_obj = AccessLevels(access=access) + session.add(access_obj) + try: + session.commit() + except SQLAlchemyError: + session.rollback() + ERROR_LOG.debug('Access level %s could not be added', access) + + +class AccessLevels(BASE): + ''' Different access levels a user/group can have for a project ''' + __tablename__ = 'access_levels' + + access = sa.Column(sa.String(255), primary_key=True) + class StatusIssue(BASE): """ Stores the status a ticket can have. @@ -365,6 +381,25 @@ class Project(BASE): backref='co_projects' ) + admins = relation( + 'User', + secondary="user_projects", + primaryjoin="projects.c.id==user_projects.c.project_id", + secondaryjoin="and_(users.c.id==user_projects.c.user_id,\ + user_projects.c.access=='admin')", + backref='co_projects_admins' + ) + + committers = relation( + 'User', + secondary="user_projects", + primaryjoin="projects.c.id==user_projects.c.project_id", + secondaryjoin="and_(users.c.id==user_projects.c.user_id,\ + or_(user_projects.c.access=='commit',\ + user_projects.c.access=='admin'))", + backref='co_projects_committers' + ) + groups = relation( "PagureGroup", secondary="projects_groups", @@ -376,6 +411,25 @@ class Project(BASE): ) ) + admin_groups = relation( + "PagureGroup", + secondary="projects_groups", + primaryjoin="projects.c.id==projects_groups.c.project_id", + secondaryjoin="and_(pagure_group.c.id==projects_groups.c.group_id,\ + projects_groups.c.access=='admin')", + backref="projects_admin_groups", + ) + + committer_groups = relation( + "PagureGroup", + secondary="projects_groups", + primaryjoin="projects.c.id==projects_groups.c.project_id", + secondaryjoin="and_(pagure_group.c.id==projects_groups.c.group_id,\ + or_(projects_groups.c.access=='admin',\ + projects_groups.c.access=='commit'))", + backref="projects_committer_groups", + ) + unwatchers = relation( "Watcher", primaryjoin="and_(Project.id==Watcher.project_id, " @@ -619,7 +673,7 @@ class ProjectUser(BASE): __tablename__ = 'user_projects' __table_args__ = ( - sa.UniqueConstraint('project_id', 'user_id'), + sa.UniqueConstraint('project_id', 'user_id', 'access'), ) id = sa.Column(sa.Integer, primary_key=True) @@ -636,6 +690,12 @@ class ProjectUser(BASE): ), nullable=False, index=True) + access = sa.Column( + sa.String(255), + sa.ForeignKey( + 'access_levels.access', onupdate='CASCADE', ondelete='CASCADE', + ), + nullable=False) class DeployKey(BASE): @@ -1740,6 +1800,12 @@ class ProjectGroup(BASE): 'pagure_group.id', ), primary_key=True) + access = sa.Column( + sa.String(255), + sa.ForeignKey( + 'access_levels.access', onupdate='CASCADE', ondelete='CASCADE', + ), + nullable=False) # Constraints __table_args__ = (sa.UniqueConstraint('project_id', 'group_id'),) diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py index b7cde73..83c6ecb 100644 --- a/pagure/lib/notify.py +++ b/pagure/lib/notify.py @@ -108,7 +108,7 @@ def _get_emails_for_obj(obj): if user.default_email: emails.add(user.default_email) - # Add people in groups with commits access to the project: + # Add people in groups with any access to the project: for group in obj.project.groups: if group.creator.default_email: emails.add(group.creator.default_email) diff --git a/pagure/templates/_formhelper.html b/pagure/templates/_formhelper.html index ae5da5b..33af8dc 100644 --- a/pagure/templates/_formhelper.html +++ b/pagure/templates/_formhelper.html @@ -144,7 +144,7 @@ {% endif %} - {% if id != 0 and g.fas_user and g.repo_admin or ( + {% if id != 0 and g.fas_user and g.repo_committer or ( comment.parent.status in [True, 'Open'] and g.fas_user.username == comment.user.username) %} {% endif %} - {% if id != 0 and g.fas_user and g.repo_admin or ( + {% if id != 0 and g.fas_user and g.repo_committer or ( comment.parent.status in [True, 'Open'] and g.fas_user.username == comment.user.username) %} - {{ form.csrf_token }} - - + {% for access in access_users %} + {% for user in access_users[access] %} +
  • + + +   {{ user.user }} + + ({{access}}) +
    + + {{ form.csrf_token }} +
    +
    • + {% endfor %} {% endfor %} - {% for group in repo.groups %} -
  • - - -   {{ group.group_name }} - -
    - - {{ form.csrf_token }} -
    -
    • + {% for access in access_groups %} + {% for group in access_groups[access] %} +
  • + + +   {{ group.group_name }} + + ({{access}}) +
    + + {{ form.csrf_token }} +
    +
    • + {% endfor %} {% endfor %} diff --git a/pagure/ui/filters.py b/pagure/ui/filters.py index ae5e36f..dbcb611 100644 --- a/pagure/ui/filters.py +++ b/pagure/ui/filters.py @@ -30,7 +30,7 @@ from pygments.filters import VisibleWhitespaceFilter import pagure.exceptions import pagure.lib import pagure.forms -from pagure import (APP, SESSION, authenticated, is_repo_admin) +from pagure import (APP, SESSION, authenticated, is_repo_committer) # Jinja filters @@ -170,7 +170,7 @@ def format_loc(loc, commit=None, filename=None, tree_id=None, prequest=None, status in ['true', 'open'] and comment.user.user == flask.g.fas_user.username ) - or is_repo_admin(comment.parent.project)): + or is_repo_committer(comment.parent.project)): templ_delete = tpl_delete % ({'commentid': comment.id}) templ_edit = tpl_edit % ({ 'edit_url': flask.url_for( diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index d179eb3..d8f90e3 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -434,7 +434,7 @@ def request_pull_edit(repo, requestid, username=None, namespace=None): if request.status != 'Open': flask.abort(400, 'Pull-request is already closed') - if not flask.g.repo_admin \ + if not flask.g.repo_committer \ and flask.g.fas_user.username != request.user.username: flask.abort(403, 'You are not allowed to edit this pull-request') @@ -619,7 +619,7 @@ def pull_request_drop_comment( if (flask.g.fas_user.username != comment.user.username or comment.parent.status is False) \ - and not flask.g.repo_admin: + and not flask.g.repo_committer: flask.abort( 403, 'You are not allowed to remove this comment from ' @@ -679,7 +679,7 @@ def pull_request_edit_comment( if (flask.g.fas_user.username != comment.user.username or comment.parent.status != 'Open') \ - and not flask.g.repo_admin: + and not flask.g.repo_committer: flask.abort(403, 'You are not allowed to edit the comment') form = pagure.forms.EditCommentForm() @@ -771,7 +771,7 @@ def merge_request_pull(repo, requestid, username=None, namespace=None): if not request: flask.abort(404, 'Pull-request not found') - if not flask.g.repo_admin: + if not flask.g.repo_committer: flask.abort( 403, 'You are not allowed to merge pull-request for this project') @@ -844,7 +844,7 @@ def cancel_request_pull(repo, requestid, username=None, namespace=None): if not request: flask.abort(404, 'Pull-request not found') - if not flask.g.repo_admin \ + if not flask.g.repo_committer \ and not flask.g.fas_user.username == request.user.username: flask.abort( 403, @@ -904,7 +904,7 @@ def set_assignee_requests(repo, requestid, username=None, namespace=None): if request.status != 'Open': flask.abort(403, 'Pull-request closed') - if not flask.g.repo_admin: + if not flask.g.repo_committer: flask.abort(403, 'You are not allowed to assign this pull-request') form = pagure.forms.ConfirmationForm() @@ -1044,10 +1044,10 @@ def new_request_pull( 'view_repo', username=username, repo=repo.name, namespace=namespace)) - repo_admin = flask.g.repo_admin + repo_committer = flask.g.repo_committer form = pagure.forms.RequestPullForm() - if form.validate_on_submit() and repo_admin: + if form.validate_on_submit() and repo_committer: try: if repo.settings.get( 'Enforce_signed-off_commits_in_pull-request', False): @@ -1108,7 +1108,7 @@ def new_request_pull( 'We could not save all the info, please try again', 'error') - if not repo_admin: + if not flask.g.repo_committer: form = None # if the pull request we are creating only has one commit, @@ -1177,8 +1177,6 @@ def new_remote_request_pull(repo, username=None, namespace=None): parentpath = flask.g.reponame orig_repo = flask.g.repo_obj - repo_admin = flask.g.repo_admin - form = pagure.forms.RemoteRequestPullForm() if form.validate_on_submit(): branch_from = form.branch_from.data.strip() diff --git a/pagure/ui/groups.py b/pagure/ui/groups.py index 2c05070..b365ad1 100644 --- a/pagure/ui/groups.py +++ b/pagure/ui/groups.py @@ -65,7 +65,7 @@ def view_group(group): flask.abort(404, 'Group not found') # Add new user to the group if asked - form = pagure.forms.AddUserForm() + form = pagure.forms.AddUserToGroupForm() if pagure.authenticated() and form.validate_on_submit() \ and pagure.APP.config.get('ENABLE_GROUP_MNGT', False): diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index 710b659..6d1b74b 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -88,7 +88,7 @@ def update_issue(repo, issueid, username=None, namespace=None): if issue is None or issue.project != repo: flask.abort(404, 'Issue not found') - if issue.private and not flask.g.repo_admin \ + if issue.private and not flask.g.repo_committer \ and (not authenticated() or not issue.user.user == flask.g.fas_user.username): flask.abort( @@ -110,8 +110,6 @@ def update_issue(repo, issueid, username=None, namespace=None): ) if form.validate_on_submit(): - repo_admin = flask.g.repo_admin - if flask.request.form.get('drop_comment'): commentid = flask.request.form.get('drop_comment') @@ -122,7 +120,7 @@ def update_issue(repo, issueid, username=None, namespace=None): if (flask.g.fas_user.username != comment.user.username or comment.parent.status != 'Open') \ - and not flask.g.repo_admin: + and not flask.g.repo_committer: flask.abort( 403, 'You are not allowed to remove this comment from ' @@ -212,7 +210,7 @@ def update_issue(repo, issueid, username=None, namespace=None): # The status field can be updated by both the admin and the # person who opened the ticket. # Update status - if repo_admin or flask.g.fas_user.username == issue.user.user: + if flask.g.repo_user or flask.g.fas_user.username == issue.user.user: if new_status in status: msgs = pagure.lib.edit_issue( SESSION, @@ -231,7 +229,7 @@ def update_issue(repo, issueid, username=None, namespace=None): # All the other meta-data can be changed only by admins # while other field will be missing for non-admin and thus # reset if we let them - if repo_admin: + if flask.g.repo_user: # Adjust (add/remove) tags msgs = pagure.lib.update_tags( SESSION, issue, tags, @@ -240,11 +238,9 @@ def update_issue(repo, issueid, username=None, namespace=None): ) messages = messages.union(set(msgs)) - # The meta-data can be changed by admins and issue creator, - # where issue creators can only change status of their issue while - # other fields will be missing for non-admin and thus reset if we - # let them - if repo_admin: + # The meta-data can only be changed by admins, which means they + # will be missing for non-admin and thus reset if we let them + if flask.g.repo_user: # Assign or update assignee of the ticket message = pagure.lib.add_issue_assignee( SESSION, @@ -624,7 +620,7 @@ def view_issues(repo, username=None, namespace=None): if authenticated(): private = flask.g.fas_user.username # If user is repo admin, show all tickets included the private ones - if flask.g.repo_admin: + if flask.g.repo_committer: private = None if str(status).lower() in ['all']: @@ -745,8 +741,9 @@ def view_roadmap(repo, username=None, namespace=None): # If user is authenticated, show him/her his/her private tickets if authenticated(): private = flask.g.fas_user.username - # If user is repo admin, show all tickets included the private ones - if flask.g.repo_admin: + + # If user is repo committer, show all tickets included the private ones + if flask.g.repo_committer: private = None all_milestones = sorted(list(repo.milestones.keys())) @@ -961,7 +958,7 @@ def view_issue(repo, issueid, username=None, namespace=None): if issue is None or issue.project != repo: flask.abort(404, 'Issue not found') - if issue.private and not flask.g.repo_admin \ + if issue.private and not flask.g.repo_committer \ and (not authenticated() or not issue.user.user == flask.g.fas_user.username): flask.abort( @@ -1002,6 +999,7 @@ def view_issue(repo, issueid, username=None, namespace=None): knowns_keys=knowns_keys, subscribers=pagure.lib.get_watch_list(SESSION, issue), attachments=issue.attachments, + subscribed=subscribed, ) @@ -1025,7 +1023,7 @@ def delete_issue(repo, issueid, username=None, namespace=None): if issue is None or issue.project != repo: flask.abort(404, 'Issue not found') - if not flask.g.repo_admin: + if not flask.g.repo_committer: flask.abort( 403, 'You are not allowed to remove tickets of this project') @@ -1083,7 +1081,7 @@ def edit_issue(repo, issueid, username=None, namespace=None): if issue is None or issue.project != repo: flask.abort(404, 'Issue not found') - if not (flask.g.repo_admin + if not (flask.g.repo_committer or flask.g.fas_user.username == issue.user.username): flask.abort( 403, 'You are not allowed to edit issues for this project') @@ -1368,7 +1366,7 @@ def edit_comment_issue( if (flask.g.fas_user.username != comment.user.username or comment.parent.status != 'Open') \ - and not flask.g.repo_admin: + and not flask.g.repo_user: flask.abort(403, 'You are not allowed to edit this comment') form = pagure.forms.EditCommentForm() diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index c3d672a..537e33a 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -1077,11 +1077,53 @@ def view_settings(repo, username=None, namespace=None): if flask.request.method == 'GET' and branchname: branches_form.branches.data = branchname + access_users = { + 'admin': pagure.lib.get_project_users( + SESSION, + project_obj=repo, + access='admin', + combine=False + ), + 'commit': pagure.lib.get_project_users( + SESSION, + project_obj=repo, + access='commit', + combine=False + ), + 'ticket': pagure.lib.get_project_users( + SESSION, + project_obj=repo, + access='ticket', + combine=False + ), + } + access_groups = { + 'admin': pagure.lib.get_project_groups( + SESSION, + project_obj=repo, + access='admin', + combine=False + ), + 'commit': pagure.lib.get_project_groups( + SESSION, + project_obj=repo, + access='commit', + combine=False + ), + 'ticket': pagure.lib.get_project_groups( + SESSION, + project_obj=repo, + access='ticket', + combine=False + ), + } return flask.render_template( 'settings.html', select='settings', username=username, repo=repo, + access_users=access_users, + access_groups=access_groups, form=form, tag_form=tag_form, branches_form=branches_form, @@ -1612,8 +1654,7 @@ def remove_user(repo, userid, username=None, namespace=None): userids = [str(user.id) for user in repo.users] if str(userid) not in userids: - flask.flash( - 'User does not have commit rights, or cannot have them removed', 'error') + flask.flash('User does not have any access on the repo', 'error') return flask.redirect(flask.url_for( '.view_settings', repo=repo.name, username=username, namespace=repo.namespace,) @@ -1743,6 +1784,7 @@ def add_user(repo, username=None, namespace=None): SESSION, repo, new_user=form.user.data, user=flask.g.fas_user.username, + access=form.access.data, ) SESSION.commit() pagure.lib.git.generate_gitolite_acls() @@ -1758,11 +1800,13 @@ def add_user(repo, username=None, namespace=None): APP.logger.exception(err) flask.flash('User could not be added', 'error') + access_levels = pagure.lib.get_access_levels(SESSION) return flask.render_template( 'add_user.html', form=form, username=username, repo=repo, + access_levels=access_levels, ) @@ -1866,6 +1910,7 @@ def add_group_project(repo, username=None, namespace=None): SESSION, repo, new_group=form.group.data, user=flask.g.fas_user.username, + access=form.access.data, create=not pagure.APP.config.get('ENABLE_GROUP_MNGT', False), is_admin=pagure.is_admin(), ) @@ -1883,11 +1928,13 @@ def add_group_project(repo, username=None, namespace=None): APP.logger.exception(err) flask.flash('Group could not be added', 'error') + access_levels = pagure.lib.get_access_levels(SESSION) return flask.render_template( 'add_group_project.html', form=form, username=username, repo=repo, + access_levels=access_levels, ) @@ -1981,7 +2028,7 @@ def add_token(repo, username=None, namespace=None): repo = flask.g.repo - if not flask.g.repo_admin: + if not flask.g.repo_committer: flask.abort( 403, 'You are not allowed to change the settings for this project') @@ -2177,7 +2224,7 @@ def delete_branch(repo, branchname, username=None, namespace=None): reponame = flask.g.reponame repo_obj = flask.g.repo_obj - if not flask.g.repo_admin: + if not flask.g.repo_committer: flask.abort( 403, 'You are not allowed to delete branch for this project') From c33f8f92453637dc8a7e7666c888ec5e70ed212e Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Feb 15 2017 10:28:57 +0000 Subject: [PATCH 2/21] Add new exception: AccessLevelNotFound Signed-off-by: Vivek Anand --- diff --git a/pagure/exceptions.py b/pagure/exceptions.py index edcc943..6892623 100644 --- a/pagure/exceptions.py +++ b/pagure/exceptions.py @@ -30,6 +30,12 @@ class ProjectBlackListedException(PagureException): pass +class AccessLevelNotFound(PagureException): + ''' Exception raised when the access level asked is not allowed on pagure + ''' + pass + + class FileNotFoundException(PagureException): ''' Exception thrown when trying to create a repository that already exists. From 9cf0dfae1c7dbdce609f45affd6e7e2e0ee1224b Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Feb 15 2017 10:28:57 +0000 Subject: [PATCH 3/21] Raise an exception when access level is not found Signed-off-by: Vivek Anand --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 485fd52..ea88ac6 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -4002,7 +4002,8 @@ def get_project_users(session, project_obj, access, combine=True): ''' if access not in ['admin', 'commit', 'ticket']: - return None + raise pagure.exceptions.AccessLevelNotFound( + 'The access level does not exist') if combine: if access == 'admin': @@ -4049,7 +4050,8 @@ def get_project_groups(session, project_obj, access, combine=True): ''' if access not in ['admin', 'commit', 'ticket']: - return None + raise pagure.exceptions.AccessLevelNotFound( + 'The access level does not exist') if combine: if access == 'admin': From e9c47b511b7c44f68a415b4cfca69a338743f5ff Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Feb 15 2017 10:28:57 +0000 Subject: [PATCH 4/21] Add missing import: urlpattern in api/issues Signed-off-by: Vivek Anand --- diff --git a/pagure/api/issue.py b/pagure/api/issue.py index 31afab4..25bcce2 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -17,8 +17,9 @@ import pagure import pagure.exceptions import pagure.lib -from pagure import APP, SESSION, is_repo_committer, api_authenticated - +from pagure import ( + APP, SESSION, is_repo_committer, api_authenticated, urlpattern +) from pagure.api import ( API, api_method, api_login_required, api_login_optional, APIERROR ) From 7d9e04bf99a348809f8238c7e8bf43d07e7bb6e7 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 15 2017 10:28:57 +0000 Subject: [PATCH 5/21] Add a contributors property to the projects and list them on the front page --- diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 1950b85..bdde911 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -12,6 +12,7 @@ __requires__ = ['SQLAlchemy >= 0.8', 'jinja2 >= 2.4'] import pkg_resources import datetime +import collections import logging import json import operator @@ -637,6 +638,18 @@ class Project(BASE): Issue.private == False ).count() + @property + def contributors(self): + """ Return the dict presenting the different contributors of the + project based on their access level. + """ + contributors = collections.defaultdict(list) + + for user in self.user_projects: + contributors[user.access].append(user.user) + + return contributors + def to_json(self, public=False, api=False): ''' Return a representation of the project as JSON. ''' @@ -697,6 +710,12 @@ class ProjectUser(BASE): ), nullable=False) + project = relation( + 'Project', remote_side=[Project.id], + backref='user_projects') + + user = relation('User', backref='user_projects') + class DeployKey(BASE): """ Stores information about deployment keys. diff --git a/pagure/templates/repo_info.html b/pagure/templates/repo_info.html index e7f4e11..f641861 100644 --- a/pagure/templates/repo_info.html +++ b/pagure/templates/repo_info.html @@ -33,7 +33,7 @@ git push -u origin master {% else %}
    - {% if readme %} + {% if readme %}
    {% if safe %} {{ readme | noJS |safe }} @@ -49,27 +49,31 @@ git push -u origin master
    - {% endif %} + {% endif %} {% endif %}
    -
    Owners
    +
    Contributors
    - {% for user in repo.admins %} - + {% for access in repo.contributors %} + {% for user in repo.contributors[access] %} + + {% endfor %} {% endfor %} {% for group in repo.admin_groups %}
    From c1c345fc4c03e46f2d2f6f28557b1988b5ce18d2 Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Feb 15 2017 10:28:57 +0000 Subject: [PATCH 6/21] Allow repos in index page if the user has commit access Signed-off-by: Vivek Anand --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index ea88ac6..7201418 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1869,31 +1869,37 @@ def search_projects( sub_q2 = session.query( model.Project.id ).filter( - # User got admin rights + # User got admin or commit right sqlalchemy.and_( model.User.user == username, model.User.id == model.ProjectUser.user_id, model.ProjectUser.project_id == model.Project.id, - model.ProjectUser.access == 'admin' + sqlalchemy.or_( + model.ProjectUser.access == 'admin', + model.ProjectUser.access == 'commit', + ) ) ) sub_q3 = session.query( model.Project.id ).filter( - # User created a group that has admin rights + # User created a group that has admin or commit right sqlalchemy.and_( model.User.user == username, model.PagureGroup.user_id == model.User.id, model.PagureGroup.group_type == 'user', model.PagureGroup.id == model.ProjectGroup.group_id, model.Project.id == model.ProjectGroup.project_id, - model.ProjectGroup.access == 'admin', + sqlalchemy.or_( + model.ProjectGroup.access == 'admin', + model.ProjectGroup.access == 'commit', + ) ) ) sub_q4 = session.query( model.Project.id ).filter( - # User is part of a group that has admin right + # User is part of a group that has admin or commit right sqlalchemy.and_( model.User.user == username, model.PagureUserGroup.user_id == model.User.id, @@ -1901,7 +1907,11 @@ def search_projects( model.PagureGroup.group_type == 'user', model.PagureGroup.id == model.ProjectGroup.group_id, model.Project.id == model.ProjectGroup.project_id, - model.ProjectGroup.access == 'admin', + sqlalchemy.or_( + model.ProjectUser.access == 'admin', + model.ProjectUser.access == 'commit', + ) + ) ) From ca33a1604fe2825956f3cd932215fb7110a215dc Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Feb 15 2017 10:28:57 +0000 Subject: [PATCH 7/21] Add call to create tables in alembic script Signed-off-by: Vivek Anand --- diff --git a/alembic/versions/987edda096f5_access_id_in_user_projects.py b/alembic/versions/987edda096f5_access_id_in_user_projects.py index 6e053b9..469303e 100644 --- a/alembic/versions/987edda096f5_access_id_in_user_projects.py +++ b/alembic/versions/987edda096f5_access_id_in_user_projects.py @@ -12,13 +12,33 @@ down_revision = '38581a8fbae2' from alembic import op import sqlalchemy as sa +from sqlalchemy.sql import table, column +acl = table ( + 'access_levels', + column('access', sa.String(255)) +) def upgrade(): ''' Add a foreign key in user_projects and projects_groups table for access_levels ''' + # To allow N + 2 migrations easier + # without going through N + 1 + # Make sure, we have all the tables + op.create_table( + 'access_levels', + sa.Column('access', sa.String(255), primary_key=True) + ) + op.bulk_insert( + acl, + [ + {'access': 'ticket'}, + {'access': 'commit'}, + {'access': 'admin'}, + ], + ) op.add_column( 'user_projects', sa.Column( @@ -78,7 +98,6 @@ def upgrade(): ) - def downgrade(): ''' Remove column access_id from user_projects and projects_groups ''' @@ -97,3 +116,4 @@ def downgrade(): 'projects_groups', ['project_id', 'group_id'], ) + op.drop_table('access_levels') From 48704eeac28025fb5d9ac70f5241779db39c404b Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Feb 15 2017 10:28:57 +0000 Subject: [PATCH 8/21] Access level docs in add user and add group page Signed-off-by: Vivek Anand --- diff --git a/pagure/templates/add_group_project.html b/pagure/templates/add_group_project.html index 3554323..7a4a45d 100644 --- a/pagure/templates/add_group_project.html +++ b/pagure/templates/add_group_project.html @@ -47,6 +47,29 @@ {{ form.csrf_token }}

    +

    Access Levels

    +

    + Ticket: A user or a group with this level of access can only edit metadata + of an issue. This includes changing the status of an issue, adding/removing + tags from them, adding/removing assignees and every other option which can + be accessed when you click "Edit Metadata" button in an issue page. However, + this user can not "create" a new tag or "delete" an existing tag because, + that would involve access to settings page of the project which this user + won't have. It also won't be able to "delete" the issue because, it falls + outside of "Edit Metadata". +

    +

    + Commit: A user or a group with this level of access can do everything what + a user/group with ticket access can do + it can do everything on the project + which doesn't include access to settings page. It can "Edit Metadata" of an issue + just like a user with ticket access would do, can merge a pull request, can push + to the main repository directly, delete an issue, cancel a pull request etc. +

    +

    + Admin: The user/group with this access has access to everything on the project. + All the "users" of the project that have been added till now are having this access. + They can change the settings of the project, add/remove users/groups on the project. +

    diff --git a/pagure/templates/add_user.html b/pagure/templates/add_user.html index 2cbd771..43b4bb0 100644 --- a/pagure/templates/add_user.html +++ b/pagure/templates/add_user.html @@ -45,6 +45,29 @@ {{ form.csrf_token }}

    +

    Access Levels

    +

    + Ticket: A user or a group with this level of access can only edit metadata + of an issue. This includes changing the status of an issue, adding/removing + tags from them, adding/removing assignees and every other option which can + be accessed when you click "Edit Metadata" button in an issue page. However, + this user can not "create" a new tag or "delete" an existing tag because, + that would involve access to settings page of the project which this user + won't have. It also won't be able to "delete" the issue because, it falls + outside of "Edit Metadata". +

    +

    + Commit: A user or a group with this level of access can do everything what + a user/group with ticket access can do + it can do everything on the project + which doesn't include access to settings page. It can "Edit Metadata" of an issue + just like a user with ticket access would do, can merge a pull request, can push + to the main repository directly, delete an issue, cancel a pull request etc. +

    +

    + Admin: The user/group with this access has access to everything on the project. + All the "users" of the project that have been added till now are having this access. + They can change the settings of the project, add/remove users/groups on the project. +

    From 70f75adac556b9f950f14831d8d7f87a2d1a44b0 Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Feb 15 2017 10:28:57 +0000 Subject: [PATCH 9/21] Update Access button in front of user/group in settings Signed-off-by: Vivek Anand --- diff --git a/pagure/templates/settings.html b/pagure/templates/settings.html index df34f3d..c25c0c4 100644 --- a/pagure/templates/settings.html +++ b/pagure/templates/settings.html @@ -434,6 +434,19 @@ {{ form.csrf_token }} +
    + + {{ form.csrf_token }} +
    {% endfor %} {% endfor %} @@ -459,6 +472,20 @@ {{ form.csrf_token }} +
    + + {{ form.csrf_token }} +
    + {% endfor %} {% endfor %} From fc2d4050a0918ed5486e600960967f9da926bb20 Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Feb 15 2017 10:28:57 +0000 Subject: [PATCH 10/21] Don't use form for accessing add_user/add_group_project page Signed-off-by: Vivek Anand --- diff --git a/pagure/templates/settings.html b/pagure/templates/settings.html index c25c0c4..5ed2634 100644 --- a/pagure/templates/settings.html +++ b/pagure/templates/settings.html @@ -434,19 +434,15 @@ {{ form.csrf_token }} -
    - - {{ form.csrf_token }} -
    + {% endfor %} {% endfor %} @@ -472,19 +468,15 @@ {{ form.csrf_token }} -
    - - {{ form.csrf_token }} -
    + {% endfor %} From 963f9a30b0b93bc752afb11166118df41387819e Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Feb 15 2017 10:28:57 +0000 Subject: [PATCH 11/21] Pre-fill the user/group field if that usre/group's access is being updated Signed-off-by: Vivek Anand --- diff --git a/pagure/templates/add_group_project.html b/pagure/templates/add_group_project.html index 7a4a45d..ca8cc08 100644 --- a/pagure/templates/add_group_project.html +++ b/pagure/templates/add_group_project.html @@ -82,24 +82,29 @@ src="{{ url_for('static', filename='selectize.min.js') }}"> {% endblock %} diff --git a/pagure/templates/add_user.html b/pagure/templates/add_user.html index 091b98c..dd93ce4 100644 --- a/pagure/templates/add_user.html +++ b/pagure/templates/add_user.html @@ -12,7 +12,7 @@ repo.namespace + '/' if repo.namespace }}{{ repo.name }}{% endblock %} {% block repo %} -
    +
    Add user to the {{repo.name}} project @@ -26,15 +26,11 @@ + value="" style="margin-bottom:1%"/> @@ -102,12 +98,13 @@ $( document ).ready(function() { }); } else { $("#user").attr("value", user_to_update); + $("#user").attr("readonly", true); + var user_access = "{{ user_access }}"; + if (user_access !== "None") { + $("#" + "{{ user_access.access }}").attr("selected", "selected"); + } } - $('#access').selectize({ - sortField: 'text', - }); - }); {% endblock %} diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 53c54e3..44ef6d8 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -1778,14 +1778,17 @@ def add_user(repo, username=None, namespace=None): user_to_update = flask.request.args.get('user', '').strip() user_to_update_obj = None + user_access = None if user_to_update: user_to_update_obj = pagure.lib.search_user( - SESSION, - username=user_to_update) + SESSION, username=user_to_update) + user_access = pagure.lib.get_obj_access( + SESSION, repo, user_to_update_obj) # The requested user is not found if user_to_update_obj is None: user_to_update = None + user_access = None form = pagure.forms.AddUserForm() @@ -1819,6 +1822,7 @@ def add_user(repo, username=None, namespace=None): repo=repo, access_levels=access_levels, user_to_update=user_to_update, + user_access=user_access, ) @@ -1916,14 +1920,17 @@ def add_group_project(repo, username=None, namespace=None): group_to_update = flask.request.args.get('group', '').strip() group_to_update_obj = None + group_access = None if group_to_update: group_to_update_obj = pagure.lib.search_groups( - SESSION, - group_name=group_to_update) + SESSION, group_name=group_to_update) + group_access = pagure.lib.get_obj_access( + SESSION, repo, group_to_update_obj) # The requested group is not found if group_to_update_obj is None: group_to_update = None + group_access = None form = pagure.forms.AddGroupForm() @@ -1959,6 +1966,7 @@ def add_group_project(repo, username=None, namespace=None): repo=repo, access_levels=access_levels, group_to_update=group_to_update, + group_access=group_access, ) From c553853213fa536fd3d018f3b1ec99c47b8f8a05 Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Feb 15 2017 10:28:57 +0000 Subject: [PATCH 16/21] Change text to Update in case we are updating user/group's access Signed-off-by: Vivek Anand --- diff --git a/pagure/templates/add_group_project.html b/pagure/templates/add_group_project.html index 27d2a37..00ea291 100644 --- a/pagure/templates/add_group_project.html +++ b/pagure/templates/add_group_project.html @@ -16,7 +16,7 @@
    -
    +
    Add group to the {{repo.name}} project
    @@ -39,7 +39,7 @@

    - + {{ form.csrf_token }}

    @@ -105,6 +105,8 @@ $( document ).ready(function() { if (group_access !== "None") { $("#" + "{{ group_access.access }}").attr("selected", "selected"); } + $("#card-topic").html("Update group access in {{repo.name}}"); + $("#add_update_button").attr("value", "Update"); } }); diff --git a/pagure/templates/add_user.html b/pagure/templates/add_user.html index dd93ce4..d57c7ec 100644 --- a/pagure/templates/add_user.html +++ b/pagure/templates/add_user.html @@ -14,7 +14,7 @@ {% block repo %}
    -
    +
    Add user to the {{repo.name}} project
    @@ -37,7 +37,7 @@

    - + {{ form.csrf_token }}

    @@ -103,6 +103,8 @@ $( document ).ready(function() { if (user_access !== "None") { $("#" + "{{ user_access.access }}").attr("selected", "selected"); } + $("#card-topic").html("Update user access in {{repo.name}}"); + $("#add_update_button").attr("value", "Update"); } }); From 8b76cbce6ca1dceac10bc5712c8f08a0b33a577e Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Feb 15 2017 10:28:58 +0000 Subject: [PATCH 17/21] It's group name not username for group in addgroup page Signed-off-by: Vivek Anand --- diff --git a/pagure/templates/add_group_project.html b/pagure/templates/add_group_project.html index 00ea291..42ef076 100644 --- a/pagure/templates/add_group_project.html +++ b/pagure/templates/add_group_project.html @@ -26,7 +26,7 @@ }}" method="post">
    - + From 57b3bbd357035e1d46aa310db1e8728e47e1c26f Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Feb 15 2017 10:28:58 +0000 Subject: [PATCH 18/21] Adjust alembic revision id after deploy keys Signed-off-by: Vivek Anand --- diff --git a/alembic/versions/987edda096f5_access_id_in_user_projects.py b/alembic/versions/987edda096f5_access_id_in_user_projects.py index 469303e..0b201f5 100644 --- a/alembic/versions/987edda096f5_access_id_in_user_projects.py +++ b/alembic/versions/987edda096f5_access_id_in_user_projects.py @@ -1,14 +1,14 @@ """access_id in user_projects Revision ID: 987edda096f5 -Revises: 38581a8fbae2 +Revises: 8a3b10926153 Create Date: 2016-07-05 18:21:14.771273 """ # revision identifiers, used by Alembic. revision = '987edda096f5' -down_revision = '38581a8fbae2' +down_revision = '8a3b10926153' from alembic import op import sqlalchemy as sa From 78333a07d04706617d8dabbeb14d81f62aa76e40 Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Feb 15 2017 10:28:58 +0000 Subject: [PATCH 19/21] check for access of group and not user when filtering the projects according to group Signed-off-by: Vivek Anand --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 7201418..be0ae3d 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1908,8 +1908,8 @@ def search_projects( model.PagureGroup.id == model.ProjectGroup.group_id, model.Project.id == model.ProjectGroup.project_id, sqlalchemy.or_( - model.ProjectUser.access == 'admin', - model.ProjectUser.access == 'commit', + model.ProjectGroup.access == 'admin', + model.ProjectGroup.access == 'commit', ) ) From 3decd287a00c5eec5cf65d6ddd96721baa120c0e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 15 2017 10:28:58 +0000 Subject: [PATCH 20/21] Fix typo in the alembic migration --- diff --git a/alembic/versions/987edda096f5_access_id_in_user_projects.py b/alembic/versions/987edda096f5_access_id_in_user_projects.py index 0b201f5..3181586 100644 --- a/alembic/versions/987edda096f5_access_id_in_user_projects.py +++ b/alembic/versions/987edda096f5_access_id_in_user_projects.py @@ -83,7 +83,7 @@ def upgrade(): ) # alter the constraints - op.drop_constraint('user_projects_project_id_key', 'user_projects') + op.drop_constraint('user_projects_project_id_fkey', 'user_projects') op.create_unique_constraint( None, 'user_projects', From 058fe858a95262d1dca713dd8e5311365f1a3037 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 15 2017 10:28:58 +0000 Subject: [PATCH 21/21] Fix the unit-tests for the ACLs change --- diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index 6d1b74b..25b633b 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -999,7 +999,6 @@ def view_issue(repo, issueid, username=None, namespace=None): knowns_keys=knowns_keys, subscribers=pagure.lib.get_watch_list(SESSION, issue), attachments=issue.attachments, - subscribed=subscribed, ) diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index 2454ef5..816c0c3 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -264,13 +264,37 @@ class PagureFlaskRepotests(tests.Modeltests): 'user': 'ralph', } + # Missing access output = self.app.post('/test/adduser', data=data) self.assertEqual(output.status_code, 200) + self.assertIn( + 'Add user - test - Pagure', output.data) self.assertTrue('Add user to the' in output.data) data['csrf_token'] = csrf_token output = self.app.post('/test/adduser', data=data) self.assertEqual(output.status_code, 200) + self.assertIn( + 'Add user - test - Pagure', output.data) + self.assertIn('Add user to the', output.data) + + data = { + 'user': 'ralph', + 'access': 'commit', + } + + # No CSRF + output = self.app.post('/test/adduser', data=data) + self.assertEqual(output.status_code, 200) + self.assertIn( + 'Add user - test - Pagure', output.data) + + # Unknown user + data['csrf_token'] = csrf_token + output = self.app.post('/test/adduser', data=data) + self.assertEqual(output.status_code, 200) + self.assertIn( + 'Add user - test - Pagure', output.data) self.assertIn('Add user to the', output.data) self.assertIn( '\n No user "ralph" ' @@ -412,18 +436,33 @@ class PagureFlaskRepotests(tests.Modeltests): 'group': 'ralph', } + # Missing CSRF output = self.app.post('/test/addgroup', data=data) self.assertEqual(output.status_code, 200) - self.assertTrue('Add group to the' in output.data) + self.assertIn( + 'Add group - test - Pagure', output.data) + self.assertIn('Add group to the', output.data) + # Missing access data['csrf_token'] = csrf_token output = self.app.post('/test/addgroup', data=data) self.assertEqual(output.status_code, 200) - self.assertTrue('Add group to the' in output.data) + self.assertIn( + 'Add group - test - Pagure', output.data) + self.assertIn('Add group to the', output.data) + + # Unknown group + data['access'] = 'ticket' + output = self.app.post('/test/addgroup', data=data) + self.assertEqual(output.status_code, 200) + self.assertIn( + 'Add group - test - Pagure', output.data) + self.assertIn('Add group to the', output.data) self.assertIn( '\n No group ralph found.', output.data) + # All good data['group'] = 'foo' output = self.app.post( '/test/addgroup', data=data, follow_redirects=True) @@ -609,8 +648,8 @@ class PagureFlaskRepotests(tests.Modeltests): 'Settings - test - Pagure', output.data) self.assertIn('

    Settings for test

    ', output.data) self.assertIn( - '\n User does not have commit rights, ' - 'or cannot have them removed', output.data) + '\n User does not have any ' + 'access on the repo', output.data) # Add an user to a project repo = pagure.lib.get_project(self.session, 'test') diff --git a/tests/test_pagure_flask_ui_repo_slash_name.py b/tests/test_pagure_flask_ui_repo_slash_name.py index 63207f8..432430b 100644 --- a/tests/test_pagure_flask_ui_repo_slash_name.py +++ b/tests/test_pagure_flask_ui_repo_slash_name.py @@ -113,7 +113,7 @@ class PagureFlaskSlashInNametests(tests.Modeltests): self.assertEqual(output.status_code, 200) self.assertIn( '
    \n ' - '
    Owners
    ', output.data) + '
    Contributors
    ', output.data) self.assertIn( '

    The Project Creator has not pushed any code yet

    ', output.data) @@ -156,7 +156,7 @@ class PagureFlaskSlashInNametests(tests.Modeltests): self.assertEqual(output.status_code, 200) self.assertIn( '
    \n ' - '
    Owners
    ', output.data) + '
    Contributors
    ', output.data) self.assertIn( '

    The Project Creator has not pushed any code yet

    ', output.data) @@ -188,7 +188,7 @@ class PagureFlaskSlashInNametests(tests.Modeltests): self.assertEqual(output.status_code, 200) self.assertIn( '
    \n ' - '
    Owners
    ', output.data) + '
    Contributors
    ', output.data) # We can't create the project `forks/test` the normal way self.assertRaises( @@ -227,7 +227,7 @@ class PagureFlaskSlashInNametests(tests.Modeltests): self.assertEqual(output.status_code, 200) self.assertIn( '
    \n ' - '
    Owners
    ', output.data) + '
    Contributors
    ', output.data) self.assertIn('Add sources file for testing', output.data) self.assertIn( 'Overview - forks/test - Pagure', output.data) diff --git a/tests/test_pagure_flask_ui_slash_branch_name.py b/tests/test_pagure_flask_ui_slash_branch_name.py index 0e42dd7..4106461 100644 --- a/tests/test_pagure_flask_ui_slash_branch_name.py +++ b/tests/test_pagure_flask_ui_slash_branch_name.py @@ -141,7 +141,7 @@ class PagureFlaskSlashInBranchtests(tests.Modeltests): self.assertEqual(output.status_code, 200) self.assertIn( '
    \n ' - '
    Owners
    ', output.data) + '
    Contributors
    ', output.data) @patch('pagure.lib.notify.send_email') def test_view_repo_branch(self, send_email): @@ -162,7 +162,7 @@ class PagureFlaskSlashInBranchtests(tests.Modeltests): self.assertEqual(output.status_code, 200) self.assertIn( '
    \n ' - '
    Owners
    ', output.data) + '
    Contributors
    ', output.data) @patch('pagure.lib.notify.send_email') def test_view_commits(self, send_email):