From 850bbd528b1741cf2d8ab7fae0333dd1eb47bdb9 Mon Sep 17 00:00:00 2001
From: Martin Basti <mbasti@redhat.com>
Date: Mar 29 2017 20:08:44 +0000
Subject: [PATCH 1/9] issues api: deduplicate of getting repository


Replace copy paste with function call

---

diff --git a/pagure/api/issue.py b/pagure/api/issue.py
index f1397a5..9edb8c4 100644
--- a/pagure/api/issue.py
+++ b/pagure/api/issue.py
@@ -26,6 +26,28 @@ from pagure.api import (
 )
 
 
+def _get_repo(repo_name, username=None, namespace=None):
+    """Check if repository exists and get repository name
+    :param repo_name: name of repository
+    :param username:
+    :param namespace:
+    :raises pagure.exceptions.APIError: when repository doesn't exists or is disabled
+    :return: repository name
+    """
+    repo = pagure.lib.get_project(
+        SESSION, repo_name, user=username, namespace=namespace)
+
+    if repo is None:
+        raise pagure.exceptions.APIError(
+            404, error_code=APIERROR.ENOPROJECT)
+
+    if not repo.settings.get('issue_tracker', True):
+        raise pagure.exceptions.APIError(
+            404, error_code=APIERROR.ETRACKERDISABLED)
+
+    return repo
+
+
 @API.route('/<repo>/new_issue', methods=['POST'])
 @API.route('/<namespace>/<repo>/new_issue', methods=['POST'])
 @API.route('/fork/<username>/<repo>/new_issue', methods=['POST'])
@@ -96,17 +118,8 @@ def api_new_issue(repo, username=None, namespace=None):
         }
 
     """
-    repo = pagure.lib.get_project(
-        SESSION, repo, user=username, namespace=namespace)
     output = {}
-
-    if repo is None:
-        raise pagure.exceptions.APIError(
-            404, error_code=APIERROR.ENOPROJECT)
-
-    if not repo.settings.get('issue_tracker', True):
-        raise pagure.exceptions.APIError(
-            404, error_code=APIERROR.ETRACKERDISABLED)
+    repo = _get_repo(repo, username, namespace)
 
     if flask.g.token.project and repo != flask.g.token.project:
         raise pagure.exceptions.APIError(
@@ -288,16 +301,7 @@ def api_view_issues(repo, username=None, namespace=None):
         }
 
     """
-
-    repo = pagure.lib.get_project(
-        SESSION, repo, user=username, namespace=namespace)
-
-    if repo is None:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT)
-
-    if not repo.settings.get('issue_tracker', True):
-        raise pagure.exceptions.APIError(
-            404, error_code=APIERROR.ETRACKERDISABLED)
+    repo = _get_repo(repo, username, namespace)
 
     assignee = flask.request.args.get('assignee', None)
     author = flask.request.args.get('author', None)
@@ -454,15 +458,7 @@ def api_view_issue(repo, issueid, username=None, namespace=None):
     if str(comments).lower() in ['0', 'False']:
         comments = False
 
-    repo = pagure.lib.get_project(
-        SESSION, repo, user=username, namespace=namespace)
-
-    if repo is None:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT)
-
-    if not repo.settings.get('issue_tracker', True):
-        raise pagure.exceptions.APIError(
-            404, error_code=APIERROR.ETRACKERDISABLED)
+    repo = _get_repo(repo, username, namespace)
 
     issue_id = issue_uid = None
     try:
@@ -541,15 +537,7 @@ def api_view_issue_comment(
 
     """  # noqa
 
-    repo = pagure.lib.get_project(
-        SESSION, repo, user=username, namespace=namespace)
-
-    if repo is None:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT)
-
-    if not repo.settings.get('issue_tracker', True):
-        raise pagure.exceptions.APIError(
-            404, error_code=APIERROR.ETRACKERDISABLED)
+    repo = _get_repo(repo, username, namespace)
 
     issue_id = issue_uid = None
     try:
@@ -637,17 +625,9 @@ def api_change_status_issue(repo, issueid, username=None, namespace=None):
         }
 
     """
-    repo = pagure.lib.get_project(
-        SESSION, repo, user=username, namespace=namespace)
-
     output = {}
 
-    if repo is None:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT)
-
-    if not repo.settings.get('issue_tracker', True):
-        raise pagure.exceptions.APIError(
-            404, error_code=APIERROR.ETRACKERDISABLED)
+    repo = _get_repo(repo, username, namespace)
 
     if api_authenticated():
         if repo != flask.g.token.project:
@@ -768,17 +748,8 @@ def api_change_milestone_issue(repo, issueid, username=None, namespace=None):
         }
 
     """
-    repo = pagure.lib.get_project(
-        SESSION, repo, user=username, namespace=namespace)
-
     output = {}
-
-    if repo is None:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT)
-
-    if not repo.settings.get('issue_tracker', True):
-        raise pagure.exceptions.APIError(
-            404, error_code=APIERROR.ETRACKERDISABLED)
+    repo = _get_repo(repo, username, namespace)
 
     if api_authenticated():
         if repo != flask.g.token.project:
@@ -887,16 +858,8 @@ def api_comment_issue(repo, issueid, username=None, namespace=None):
         }
 
     """
-    repo = pagure.lib.get_project(
-        SESSION, repo, user=username, namespace=namespace)
     output = {}
-
-    if repo is None:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT)
-
-    if not repo.settings.get('issue_tracker', True):
-        raise pagure.exceptions.APIError(
-            404, error_code=APIERROR.ETRACKERDISABLED)
+    repo = _get_repo(repo, username, namespace)
 
     if api_authenticated():
         if flask.g.token.project and repo != flask.g.token.project:
@@ -986,16 +949,8 @@ def api_assign_issue(repo, issueid, username=None, namespace=None):
         }
 
     """
-    repo = pagure.lib.get_project(
-        SESSION, repo, user=username, namespace=namespace)
     output = {}
-
-    if repo is None:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT)
-
-    if not repo.settings.get('issue_tracker', True):
-        raise pagure.exceptions.APIError(
-            404, error_code=APIERROR.ETRACKERDISABLED)
+    repo = _get_repo(repo, username, namespace)
 
     if api_authenticated():
         if repo != flask.g.token.project:
@@ -1103,16 +1058,8 @@ def api_subscribe_issue(repo, issueid, username=None, namespace=None):
         }
 
     """  # noqa
-    repo = pagure.lib.get_project(
-        SESSION, repo, user=username, namespace=namespace)
     output = {}
-
-    if repo is None:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT)
-
-    if not repo.settings.get('issue_tracker', True):
-        raise pagure.exceptions.APIError(
-            404, error_code=APIERROR.ETRACKERDISABLED)
+    repo = _get_repo(repo, username, namespace)
 
     if api_authenticated():
         if repo != flask.g.token.project:
@@ -1205,17 +1152,8 @@ def api_update_custom_field(
         }
 
     """  # noqa
-    repo = pagure.lib.get_project(
-        SESSION, repo, user=username, namespace=namespace)
-
     output = {}
-
-    if repo is None:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT)
-
-    if not repo.settings.get('issue_tracker', True):
-        raise pagure.exceptions.APIError(
-            404, error_code=APIERROR.ETRACKERDISABLED)
+    repo = _get_repo(repo, username, namespace)
 
     if api_authenticated():
         if repo != flask.g.token.project:

From 3646a69d374317b6c4f9aacd434e6dfbf11b5f89 Mon Sep 17 00:00:00 2001
From: Martin Basti <mbasti@redhat.com>
Date: Mar 29 2017 20:08:44 +0000
Subject: [PATCH 2/9] issues api: deduplicate checking of token


Instead of copy paste a function can be used

---

diff --git a/pagure/api/issue.py b/pagure/api/issue.py
index 9edb8c4..471e623 100644
--- a/pagure/api/issue.py
+++ b/pagure/api/issue.py
@@ -48,6 +48,17 @@ def _get_repo(repo_name, username=None, namespace=None):
     return repo
 
 
+def _check_token(repo):
+    """Check if token is valid for the repo
+    :param repo: repository name
+    :raises pagure.exceptions.APIError: when token is not valid for repo
+    """
+    if api_authenticated():
+        if repo != flask.g.token.project:
+            raise pagure.exceptions.APIError(
+                401, error_code=APIERROR.EINVALIDTOK)
+
+
 @API.route('/<repo>/new_issue', methods=['POST'])
 @API.route('/<namespace>/<repo>/new_issue', methods=['POST'])
 @API.route('/fork/<username>/<repo>/new_issue', methods=['POST'])
@@ -472,10 +483,7 @@ def api_view_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 api_authenticated():
-        if repo != flask.g.token.project:
-            raise pagure.exceptions.APIError(
-                401, error_code=APIERROR.EINVALIDTOK)
+    _check_token(repo)
 
     if issue.private and not is_repo_committer(repo) \
             and (not api_authenticated() or
@@ -551,10 +559,7 @@ def api_view_issue_comment(
     if issue is None or issue.project != repo:
         raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE)
 
-    if api_authenticated():
-        if repo != flask.g.token.project:
-            raise pagure.exceptions.APIError(
-                401, error_code=APIERROR.EINVALIDTOK)
+    _check_token(repo)
 
     if issue.private and not is_repo_committer(issue.project) \
             and (not api_authenticated() or
@@ -629,10 +634,7 @@ def api_change_status_issue(repo, issueid, username=None, namespace=None):
 
     repo = _get_repo(repo, username, namespace)
 
-    if api_authenticated():
-        if repo != flask.g.token.project:
-            raise pagure.exceptions.APIError(
-                401, error_code=APIERROR.EINVALIDTOK)
+    _check_token(repo)
 
     issue = pagure.lib.search_issues(SESSION, repo, issueid=issueid)
 
@@ -751,10 +753,7 @@ def api_change_milestone_issue(repo, issueid, username=None, namespace=None):
     output = {}
     repo = _get_repo(repo, username, namespace)
 
-    if api_authenticated():
-        if repo != flask.g.token.project:
-            raise pagure.exceptions.APIError(
-                401, error_code=APIERROR.EINVALIDTOK)
+    _check_token(repo)
 
     issue = pagure.lib.search_issues(SESSION, repo, issueid=issueid)
 
@@ -952,10 +951,7 @@ def api_assign_issue(repo, issueid, username=None, namespace=None):
     output = {}
     repo = _get_repo(repo, username, namespace)
 
-    if api_authenticated():
-        if repo != flask.g.token.project:
-            raise pagure.exceptions.APIError(
-                401, error_code=APIERROR.EINVALIDTOK)
+    _check_token(repo)
 
     issue = pagure.lib.search_issues(SESSION, repo, issueid=issueid)
 
@@ -1061,10 +1057,7 @@ def api_subscribe_issue(repo, issueid, username=None, namespace=None):
     output = {}
     repo = _get_repo(repo, username, namespace)
 
-    if api_authenticated():
-        if repo != flask.g.token.project:
-            raise pagure.exceptions.APIError(
-                401, error_code=APIERROR.EINVALIDTOK)
+    _check_token(repo)
 
     issue = pagure.lib.search_issues(SESSION, repo, issueid=issueid)
 
@@ -1155,10 +1148,7 @@ def api_update_custom_field(
     output = {}
     repo = _get_repo(repo, username, namespace)
 
-    if api_authenticated():
-        if repo != flask.g.token.project:
-            raise pagure.exceptions.APIError(
-                401, error_code=APIERROR.EINVALIDTOK)
+    _check_token(repo)
 
     issue = pagure.lib.search_issues(SESSION, repo, issueid=issueid)
 

From c45eac984538703059dfe15681abc6b01eb48ca9 Mon Sep 17 00:00:00 2001
From: Martin Basti <mbasti@redhat.com>
Date: Mar 29 2017 20:08:44 +0000
Subject: [PATCH 3/9] issues API: performace: fail early with invalid token


Fail early with invalid token and don't waste precious database operations

---

diff --git a/pagure/api/issue.py b/pagure/api/issue.py
index 471e623..0540ba8 100644
--- a/pagure/api/issue.py
+++ b/pagure/api/issue.py
@@ -313,6 +313,7 @@ def api_view_issues(repo, username=None, namespace=None):
 
     """
     repo = _get_repo(repo, username, namespace)
+    _check_token(repo)
 
     assignee = flask.request.args.get('assignee', None)
     author = flask.request.args.get('author', None)
@@ -350,9 +351,6 @@ def api_view_issues(repo, username=None, namespace=None):
     private = False
     # If user is authenticated, show him/her his/her private tickets
     if api_authenticated():
-        if repo != flask.g.token.project:
-            raise pagure.exceptions.APIError(
-                401, error_code=APIERROR.EINVALIDTOK)
         private = flask.g.fas_user.username
     # If user is repo committer, show all tickets included the private ones
     if is_repo_committer(repo):
@@ -470,6 +468,7 @@ def api_view_issue(repo, issueid, username=None, namespace=None):
         comments = False
 
     repo = _get_repo(repo, username, namespace)
+    _check_token(repo)
 
     issue_id = issue_uid = None
     try:
@@ -483,8 +482,6 @@ def api_view_issue(repo, issueid, username=None, namespace=None):
     if issue is None or issue.project != repo:
         raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE)
 
-    _check_token(repo)
-
     if issue.private and not is_repo_committer(repo) \
             and (not api_authenticated() or
                  not issue.user.user == flask.g.fas_user.username):
@@ -546,6 +543,7 @@ def api_view_issue_comment(
     """  # noqa
 
     repo = _get_repo(repo, username, namespace)
+    _check_token(repo)
 
     issue_id = issue_uid = None
     try:
@@ -559,8 +557,6 @@ def api_view_issue_comment(
     if issue is None or issue.project != repo:
         raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE)
 
-    _check_token(repo)
-
     if issue.private and not is_repo_committer(issue.project) \
             and (not api_authenticated() or
                  not issue.user.user == flask.g.fas_user.username):

From aac5b57a26c0bfef569c4c04df334344e8e6f339 Mon Sep 17 00:00:00 2001
From: Martin Basti <mbasti@redhat.com>
Date: Mar 29 2017 20:08:44 +0000
Subject: [PATCH 4/9] issues api: deduplicate getting issues


This copy paste can be replaced by function

---

diff --git a/pagure/api/issue.py b/pagure/api/issue.py
index 0540ba8..e56918d 100644
--- a/pagure/api/issue.py
+++ b/pagure/api/issue.py
@@ -59,6 +59,22 @@ def _check_token(repo):
                 401, error_code=APIERROR.EINVALIDTOK)
 
 
+def _get_issue(repo, issueid, issueuid=None):
+    """Get issue and check permissions
+    :param repo: repository name
+    :param issueid: issue ID
+    :param issueuid: issue Unique ID
+    :raises pagure.exceptions.APIError: when issues doesn't exists
+    :return: issue
+    """
+    issue = pagure.lib.search_issues(SESSION, repo, issueid=issueid, issueuid=issueuid)
+
+    if issue is None or issue.project != repo:
+        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE)
+
+    return issue
+
+
 @API.route('/<repo>/new_issue', methods=['POST'])
 @API.route('/<namespace>/<repo>/new_issue', methods=['POST'])
 @API.route('/fork/<username>/<repo>/new_issue', methods=['POST'])
@@ -476,11 +492,7 @@ def api_view_issue(repo, issueid, username=None, namespace=None):
     except (ValueError, TypeError):
         issue_uid = issueid
 
-    issue = pagure.lib.search_issues(
-        SESSION, repo, issueid=issue_id, issueuid=issue_uid)
-
-    if issue is None or issue.project != repo:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE)
+    issue = _get_issue(repo, issue_id, issueuid=issue_uid)
 
     if issue.private and not is_repo_committer(repo) \
             and (not api_authenticated() or
@@ -551,11 +563,7 @@ def api_view_issue_comment(
     except (ValueError, TypeError):
         issue_uid = issueid
 
-    issue = pagure.lib.search_issues(
-        SESSION, repo, issueid=issue_id, issueuid=issue_uid)
-
-    if issue is None or issue.project != repo:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE)
+    issue = _get_issue(repo, issue_id, issueuid=issue_uid)
 
     if issue.private and not is_repo_committer(issue.project) \
             and (not api_authenticated() or
@@ -632,10 +640,7 @@ def api_change_status_issue(repo, issueid, username=None, namespace=None):
 
     _check_token(repo)
 
-    issue = pagure.lib.search_issues(SESSION, repo, issueid=issueid)
-
-    if issue is None or issue.project != repo:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE)
+    issue = _get_issue(repo, issueid)
 
     if issue.private and not is_repo_committer(repo) \
             and (not api_authenticated() or
@@ -751,10 +756,7 @@ def api_change_milestone_issue(repo, issueid, username=None, namespace=None):
 
     _check_token(repo)
 
-    issue = pagure.lib.search_issues(SESSION, repo, issueid=issueid)
-
-    if issue is None or issue.project != repo:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE)
+    issue = _get_issue(repo, issueid)
 
     if issue.private and not is_repo_committer(repo) \
             and (not api_authenticated() or
@@ -861,10 +863,7 @@ def api_comment_issue(repo, issueid, username=None, namespace=None):
             raise pagure.exceptions.APIError(
                 401, error_code=APIERROR.EINVALIDTOK)
 
-    issue = pagure.lib.search_issues(SESSION, repo, issueid=issueid)
-
-    if issue is None or issue.project != repo:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE)
+    issue = _get_issue(repo, issueid)
 
     if issue.private and not is_repo_committer(repo) \
             and (not api_authenticated() or
@@ -949,10 +948,7 @@ def api_assign_issue(repo, issueid, username=None, namespace=None):
 
     _check_token(repo)
 
-    issue = pagure.lib.search_issues(SESSION, repo, issueid=issueid)
-
-    if issue is None or issue.project != repo:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE)
+    issue = _get_issue(repo, issueid)
 
     if issue.private and not is_repo_committer(repo) \
             and (not api_authenticated() or
@@ -1055,10 +1051,7 @@ def api_subscribe_issue(repo, issueid, username=None, namespace=None):
 
     _check_token(repo)
 
-    issue = pagure.lib.search_issues(SESSION, repo, issueid=issueid)
-
-    if issue is None or issue.project != repo:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE)
+    issue = _get_issue(repo, issueid)
 
     if issue.private and not is_repo_admin(repo) \
             and (not api_authenticated() or
@@ -1146,10 +1139,7 @@ def api_update_custom_field(
 
     _check_token(repo)
 
-    issue = pagure.lib.search_issues(SESSION, repo, issueid=issueid)
-
-    if issue is None or issue.project != repo:
-        raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE)
+    issue = _get_issue(repo, issueid)
 
     if issue.private and not is_repo_admin(repo) \
             and (not api_authenticated() or

From 2ed3b42e42c31ac6593c036002f68813e0f00384 Mon Sep 17 00:00:00 2001
From: Martin Basti <mbasti@redhat.com>
Date: Mar 29 2017 20:08:44 +0000
Subject: [PATCH 5/9] unify argument of is_repo_commiter


Various sources of repo has been used: variable repo and issue.project

IMO is safer to use issue.project in case that accidentaly an issue is
taken from different repo.

---

diff --git a/pagure/api/issue.py b/pagure/api/issue.py
index e56918d..6b15f78 100644
--- a/pagure/api/issue.py
+++ b/pagure/api/issue.py
@@ -67,7 +67,8 @@ def _get_issue(repo, issueid, issueuid=None):
     :raises pagure.exceptions.APIError: when issues doesn't exists
     :return: issue
     """
-    issue = pagure.lib.search_issues(SESSION, repo, issueid=issueid, issueuid=issueuid)
+    issue = pagure.lib.search_issues(
+        SESSION, repo, issueid=issueid, issueuid=issueuid)
 
     if issue is None or issue.project != repo:
         raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE)
@@ -494,7 +495,7 @@ def api_view_issue(repo, issueid, username=None, namespace=None):
 
     issue = _get_issue(repo, issue_id, issueuid=issue_uid)
 
-    if issue.private and not is_repo_committer(repo) \
+    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(
@@ -642,7 +643,7 @@ def api_change_status_issue(repo, issueid, username=None, namespace=None):
 
     issue = _get_issue(repo, issueid)
 
-    if issue.private and not is_repo_committer(repo) \
+    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(
@@ -758,7 +759,7 @@ def api_change_milestone_issue(repo, issueid, username=None, namespace=None):
 
     issue = _get_issue(repo, issueid)
 
-    if issue.private and not is_repo_committer(repo) \
+    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(
@@ -865,7 +866,7 @@ def api_comment_issue(repo, issueid, username=None, namespace=None):
 
     issue = _get_issue(repo, issueid)
 
-    if issue.private and not is_repo_committer(repo) \
+    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(
@@ -950,7 +951,7 @@ def api_assign_issue(repo, issueid, username=None, namespace=None):
 
     issue = _get_issue(repo, issueid)
 
-    if issue.private and not is_repo_committer(repo) \
+    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(

From 8c3d8a0bb49e8ff8fa054b48ef2ce0a0cd49db9d Mon Sep 17 00:00:00 2001
From: Martin Basti <mbasti@redhat.com>
Date: Mar 29 2017 20:08:44 +0000
Subject: [PATCH 6/9] issues API: deduplicate issue access check


---

diff --git a/pagure/api/issue.py b/pagure/api/issue.py
index 6b15f78..9de7d2d 100644
--- a/pagure/api/issue.py
+++ b/pagure/api/issue.py
@@ -76,6 +76,23 @@ def _get_issue(repo, issueid, issueuid=None):
     return issue
 
 
+def _check_issue_access_repo_commiter(issue):
+    """Check if user can access issue. Must be repo commiter
+    or author to see private issues.
+    :param issue: issue object
+    :raises pagure.exceptions.APIError: when access denied
+    """
+    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(
+            403, error_code=APIERROR.EISSUENOTALLOWED)
+
+
 @API.route('/<repo>/new_issue', methods=['POST'])
 @API.route('/<namespace>/<repo>/new_issue', methods=['POST'])
 @API.route('/fork/<username>/<repo>/new_issue', methods=['POST'])
@@ -494,12 +511,7 @@ def api_view_issue(repo, issueid, username=None, namespace=None):
         issue_uid = issueid
 
     issue = _get_issue(repo, issue_id, issueuid=issue_uid)
-
-    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(
-            403, error_code=APIERROR.EISSUENOTALLOWED)
+    _check_issue_access_repo_commiter(issue)
 
     jsonout = flask.jsonify(
         issue.to_json(public=True, with_comments=comments))
@@ -565,12 +577,7 @@ def api_view_issue_comment(
         issue_uid = issueid
 
     issue = _get_issue(repo, issue_id, issueuid=issue_uid)
-
-    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(
-            403, error_code=APIERROR.EISSUENOTALLOWED)
+    _check_issue_access_repo_commiter(issue)
 
     comment = pagure.lib.get_issue_comment(SESSION, issue.uid, commentid)
     if not comment:
@@ -642,12 +649,8 @@ def api_change_status_issue(repo, issueid, username=None, namespace=None):
     _check_token(repo)
 
     issue = _get_issue(repo, issueid)
+    _check_issue_access_repo_commiter(issue)
 
-    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(
-            403, error_code=APIERROR.EISSUENOTALLOWED)
 
     status = pagure.lib.get_issue_statuses(SESSION)
     form = pagure.forms.StatusForm(
@@ -758,12 +761,7 @@ def api_change_milestone_issue(repo, issueid, username=None, namespace=None):
     _check_token(repo)
 
     issue = _get_issue(repo, issueid)
-
-    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(
-            403, error_code=APIERROR.EISSUENOTALLOWED)
+    _check_issue_access_repo_commiter(issue)
 
     form = pagure.forms.MilestoneForm(
         milestones=repo.milestones.keys(),
@@ -865,12 +863,7 @@ def api_comment_issue(repo, issueid, username=None, namespace=None):
                 401, error_code=APIERROR.EINVALIDTOK)
 
     issue = _get_issue(repo, issueid)
-
-    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(
-            403, error_code=APIERROR.EISSUENOTALLOWED)
+    _check_issue_access_repo_commiter(issue)
 
     form = pagure.forms.CommentForm(csrf_enabled=False)
     if form.validate_on_submit():
@@ -950,12 +943,7 @@ def api_assign_issue(repo, issueid, username=None, namespace=None):
     _check_token(repo)
 
     issue = _get_issue(repo, issueid)
-
-    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(
-            403, error_code=APIERROR.EISSUENOTALLOWED)
+    _check_issue_access_repo_commiter(issue)
 
     form = pagure.forms.AssignIssueForm(csrf_enabled=False)
     if form.validate_on_submit():

From ac2f71b9618afb9afd15d18dd687bc07d705d2f2 Mon Sep 17 00:00:00 2001
From: Martin Basti <mbasti@redhat.com>
Date: Mar 29 2017 20:08:44 +0000
Subject: [PATCH 7/9] issues API: require repo_commiter not repo_admin privileges


repo_admin requirement is too strong limitation because issues may
be updated using different API endpoints just with repo_commiter
permission.

---

diff --git a/pagure/api/issue.py b/pagure/api/issue.py
index 9de7d2d..f03f050 100644
--- a/pagure/api/issue.py
+++ b/pagure/api/issue.py
@@ -1041,12 +1041,7 @@ def api_subscribe_issue(repo, issueid, username=None, namespace=None):
     _check_token(repo)
 
     issue = _get_issue(repo, issueid)
-
-    if issue.private and not is_repo_admin(repo) \
-            and (not api_authenticated() or
-                 not issue.user.user == flask.g.fas_user.username):
-        raise pagure.exceptions.APIError(
-            403, error_code=APIERROR.EISSUENOTALLOWED)
+    _check_issue_access_repo_commiter(issue)
 
     form = pagure.forms.SubscribtionForm(csrf_enabled=False)
     if form.validate_on_submit():
@@ -1129,12 +1124,7 @@ def api_update_custom_field(
     _check_token(repo)
 
     issue = _get_issue(repo, issueid)
-
-    if issue.private and not is_repo_admin(repo) \
-            and (not api_authenticated() or
-                 not issue.user.user == flask.g.fas_user.username):
-        raise pagure.exceptions.APIError(
-            403, error_code=APIERROR.EISSUENOTALLOWED)
+    _check_issue_access_repo_commiter(issue)
 
     fields = {k.name: k for k in repo.issue_keys}
     if field not in fields:

From c00ccc205d32f11dd5a67eefd43433a0dd4d2da1 Mon Sep 17 00:00:00 2001
From: Martin Basti <mbasti@redhat.com>
Date: Mar 29 2017 20:08:44 +0000
Subject: [PATCH 8/9] issues API: update _check_token to support non-project tokens


Add option to diferentiate between project token and non-project token

---

diff --git a/pagure/api/issue.py b/pagure/api/issue.py
index f03f050..2d79705 100644
--- a/pagure/api/issue.py
+++ b/pagure/api/issue.py
@@ -48,13 +48,18 @@ def _get_repo(repo_name, username=None, namespace=None):
     return repo
 
 
-def _check_token(repo):
+def _check_token(repo, project_token=True):
     """Check if token is valid for the repo
     :param repo: repository name
+    :param project_token: set True when project token is required,
+        otherwise any token can be used
     :raises pagure.exceptions.APIError: when token is not valid for repo
     """
     if api_authenticated():
-        if repo != flask.g.token.project:
+        if (
+            (project_token or flask.g.token.project) and
+            repo != flask.g.token.project
+        ):
             raise pagure.exceptions.APIError(
                 401, error_code=APIERROR.EINVALIDTOK)
 
@@ -856,11 +861,7 @@ def api_comment_issue(repo, issueid, username=None, namespace=None):
     """
     output = {}
     repo = _get_repo(repo, username, namespace)
-
-    if api_authenticated():
-        if flask.g.token.project and repo != flask.g.token.project:
-            raise pagure.exceptions.APIError(
-                401, error_code=APIERROR.EINVALIDTOK)
+    _check_token(repo, project_token=False)
 
     issue = _get_issue(repo, issueid)
     _check_issue_access_repo_commiter(issue)

From 213dd9b7602f539a6194f2e7c6d154692a2d0564 Mon Sep 17 00:00:00 2001
From: Martin Basti <mbasti@redhat.com>
Date: Mar 29 2017 20:08:44 +0000
Subject: [PATCH 9/9] issues API: extract issue tracker check to separate function


_check_issue_tracker now checks if tracker is enabled

---

diff --git a/pagure/api/issue.py b/pagure/api/issue.py
index 2d79705..8324180 100644
--- a/pagure/api/issue.py
+++ b/pagure/api/issue.py
@@ -41,12 +41,18 @@ def _get_repo(repo_name, username=None, namespace=None):
         raise pagure.exceptions.APIError(
             404, error_code=APIERROR.ENOPROJECT)
 
+    return repo
+
+
+def _check_issue_tracker(repo):
+    """Check if issue tracker is enabled for repository
+    :param repo: repository
+    :raises pagure.exceptions.APIError: when issue tracker is disabled
+    """
     if not repo.settings.get('issue_tracker', True):
         raise pagure.exceptions.APIError(
             404, error_code=APIERROR.ETRACKERDISABLED)
 
-    return repo
-
 
 def _check_token(repo, project_token=True):
     """Check if token is valid for the repo
@@ -170,6 +176,7 @@ def api_new_issue(repo, username=None, namespace=None):
     """
     output = {}
     repo = _get_repo(repo, username, namespace)
+    _check_issue_tracker(repo)
 
     if flask.g.token.project and repo != flask.g.token.project:
         raise pagure.exceptions.APIError(
@@ -352,6 +359,7 @@ def api_view_issues(repo, username=None, namespace=None):
 
     """
     repo = _get_repo(repo, username, namespace)
+    _check_issue_tracker(repo)
     _check_token(repo)
 
     assignee = flask.request.args.get('assignee', None)
@@ -507,6 +515,7 @@ def api_view_issue(repo, issueid, username=None, namespace=None):
         comments = False
 
     repo = _get_repo(repo, username, namespace)
+    _check_issue_tracker(repo)
     _check_token(repo)
 
     issue_id = issue_uid = None
@@ -573,6 +582,7 @@ def api_view_issue_comment(
     """  # noqa
 
     repo = _get_repo(repo, username, namespace)
+    _check_issue_tracker(repo)
     _check_token(repo)
 
     issue_id = issue_uid = None
@@ -650,7 +660,7 @@ def api_change_status_issue(repo, issueid, username=None, namespace=None):
     output = {}
 
     repo = _get_repo(repo, username, namespace)
-
+    _check_issue_tracker(repo)
     _check_token(repo)
 
     issue = _get_issue(repo, issueid)
@@ -762,7 +772,7 @@ def api_change_milestone_issue(repo, issueid, username=None, namespace=None):
     """
     output = {}
     repo = _get_repo(repo, username, namespace)
-
+    _check_issue_tracker(repo)
     _check_token(repo)
 
     issue = _get_issue(repo, issueid)
@@ -861,6 +871,7 @@ def api_comment_issue(repo, issueid, username=None, namespace=None):
     """
     output = {}
     repo = _get_repo(repo, username, namespace)
+    _check_issue_tracker(repo)
     _check_token(repo, project_token=False)
 
     issue = _get_issue(repo, issueid)
@@ -940,7 +951,7 @@ def api_assign_issue(repo, issueid, username=None, namespace=None):
     """
     output = {}
     repo = _get_repo(repo, username, namespace)
-
+    _check_issue_tracker(repo)
     _check_token(repo)
 
     issue = _get_issue(repo, issueid)
@@ -1038,7 +1049,7 @@ def api_subscribe_issue(repo, issueid, username=None, namespace=None):
     """  # noqa
     output = {}
     repo = _get_repo(repo, username, namespace)
-
+    _check_issue_tracker(repo)
     _check_token(repo)
 
     issue = _get_issue(repo, issueid)
@@ -1121,7 +1132,7 @@ def api_update_custom_field(
     """  # noqa
     output = {}
     repo = _get_repo(repo, username, namespace)
-
+    _check_issue_tracker(repo)
     _check_token(repo)
 
     issue = _get_issue(repo, issueid)

