From 3e6dbcd7dd0c69b1bce28199f4ac726b8039a547 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: May 20 2019 09:19:16 +0000
Subject: [PATCH 1/4] Adjust HTML indentations


Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/pagure/templates/repo_new_pull_request.html b/pagure/templates/repo_new_pull_request.html
index a5d0aab..fef1c0b 100644
--- a/pagure/templates/repo_new_pull_request.html
+++ b/pagure/templates/repo_new_pull_request.html
@@ -193,10 +193,10 @@
           </div>
         </div>
         <div class="card-body">
-            <textarea class="form-control" rows=8 id="initial_comment" name="initial_comment"
+          <textarea class="form-control" rows=8 id="initial_comment" name="initial_comment"
             placeholder="Describe your changes" tabindex=1>
             {{- form.initial_comment.data if form.initial_comment.data else '' -}}
-            </textarea>
+          </textarea>
           {% if form.initial_comment.errors %}
           <span class="float-right text-danger">
             <small>
@@ -206,24 +206,24 @@
             </small>
           </span>
           {% endif %}
-            <div id="preview" class="p-1">
-            </div>
+          <div id="preview" class="p-1">
           </div>
-          <div class="card-footer bg-light">
-            <div class="d-flex align-items-center">
-              <small>Comments use <a href="https://docs.pagure.org/pagure/usage/markdown.html"
-                  target="_blank" rel="noopener noreferrer" class="notblue">Markdown Syntax</a></small>
-              <div class="ml-auto">
-                <div class="btn-group">
-                    <input type="submit" class="btn btn-primary" value="Create Pull Request"{%
-                      if not diff %} disabled title="There appear to be no diff, so nothing to request pulling"{% endif %}>
+        </div>
+        <div class="card-footer bg-light">
+          <div class="d-flex align-items-center">
+            <small>Comments use <a href="https://docs.pagure.org/pagure/usage/markdown.html"
+                target="_blank" rel="noopener noreferrer" class="notblue">Markdown Syntax</a></small>
+            <div class="ml-auto">
+              <div class="btn-group">
+                  <input type="submit" class="btn btn-primary" value="Create Pull Request"{%
+                    if not diff %} disabled title="There appear to be no diff, so nothing to request pulling"{% endif %}>
 
-                </div>
               </div>
             </div>
           </div>
         </div>
-        {{ form.csrf_token }}
+      </div>
+      {{ form.csrf_token }}
     </div>
 
 
diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py
index 335cc9c..858e12d 100644
--- a/tests/test_pagure_flask_ui_fork.py
+++ b/tests/test_pagure_flask_ui_fork.py
@@ -2558,7 +2558,7 @@ index 0000000..2a552bb
                 """<textarea class="form-control" rows=8 id="initial_comment" name="initial_comment"
             placeholder="Describe your changes" tabindex=1>
 More information</textarea>
-            <div id="preview" class="p-1">""",
+          <div id="preview" class="p-1">""",
                 output_text,
             )
             self.assertIn(

From e180e7ed38a944f087063a31c51ba6ac12bb715c Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: May 20 2019 11:52:52 +0000
Subject: [PATCH 2/4] Add support for allowing the maintainers of the target project rebase


If the person opening the PR checks this checkbox, pagure will by-pass
the auth check when pushing to the project the PR originates from and
thus allow rebasing.
Otherwise, rebase would only be allowed on project the maintainer has
commit access to, which is likely not the case for most forks.
In addition, since we do not know what the fork is used for, it could be
that the fork is used by the contributor as a basis for their own
deployment, we want the permission to rebase the fork to be explicit to
avoid changing the fork without the contributor's knowledge.

Fixes https://pagure.io/pagure/issue/4456

Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/alembic/versions/d7589827abbb_add_support_for_allow_rebase.py b/alembic/versions/d7589827abbb_add_support_for_allow_rebase.py
new file mode 100644
index 0000000..32407aa
--- /dev/null
+++ b/alembic/versions/d7589827abbb_add_support_for_allow_rebase.py
@@ -0,0 +1,34 @@
+"""Add support for allow_rebase
+
+Revision ID: d7589827abbb
+Revises: 802047d28f89
+Create Date: 2019-05-09 16:25:58.971712
+
+"""
+
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = 'd7589827abbb'
+down_revision = '802047d28f89'
+
+
+def upgrade():
+    ''' Add the column allow_rebase to the table pull_requests.
+    '''
+    op.add_column(
+        'pull_requests',
+        sa.Column('allow_rebase', sa.Boolean, default=False, nullable=True)
+    )
+    op.execute('''UPDATE "pull_requests" SET allow_rebase=False;''')
+    op.alter_column(
+        'pull_requests', 'allow_rebase',
+        nullable=False, existing_nullable=True)
+
+
+def downgrade():
+    ''' Remove the column allow_rebase from the table pull_requests.
+    '''
+    op.drop_column('pull_requests', 'allow_rebase')
diff --git a/pagure/forms.py b/pagure/forms.py
index cdfa8c4..eb8db07 100644
--- a/pagure/forms.py
+++ b/pagure/forms.py
@@ -315,6 +315,11 @@ class RequestPullForm(PagureForm):
     initial_comment = wtforms.TextAreaField(
         "Initial Comment", [wtforms.validators.Optional()]
     )
+    allow_rebase = wtforms.BooleanField(
+        "Allow rebasing",
+        [wtforms.validators.Optional()],
+        false_values=FALSE_VALUES,
+    )
 
 
 class RemoteRequestPullForm(RequestPullForm):
diff --git a/pagure/lib/git.py b/pagure/lib/git.py
index c0e6d02..30c927c 100644
--- a/pagure/lib/git.py
+++ b/pagure/lib/git.py
@@ -1964,13 +1964,23 @@ def rebase_pull_request(session, request, username):
         # Push the changes
         _log.info("Pushing %s to %s", branch_ref.name, request.branch_from)
         try:
-            tempclone.push(
-                username,
-                branch_ref.name,
-                request.branch_from,
-                pull_request=request,
-                force=True,
-            )
+            if request.allow_rebase:
+                tempclone.push(
+                    username,
+                    branch_ref.name,
+                    request.branch_from,
+                    pull_request=request,
+                    force=True,
+                    internal="yes",
+                )
+            else:
+                tempclone.push(
+                    username,
+                    branch_ref.name,
+                    request.branch_from,
+                    pull_request=request,
+                    force=True,
+                )
         except subprocess.CalledProcessError as err:
             _log.debug(
                 "Rebase FAILED: {cmd} returned code {code} with the "
diff --git a/pagure/lib/model.py b/pagure/lib/model.py
index e4b660d..fd230a3 100644
--- a/pagure/lib/model.py
+++ b/pagure/lib/model.py
@@ -1960,6 +1960,8 @@ class PullRequest(BASE):
         nullable=True,
     )
 
+    allow_rebase = sa.Column(sa.Boolean, default=False, nullable=False)
+
     # While present this column isn't used anywhere yet
     private = sa.Column(sa.Boolean, nullable=False, default=False)
 
diff --git a/pagure/lib/query.py b/pagure/lib/query.py
index 137c868..1601de1 100644
--- a/pagure/lib/query.py
+++ b/pagure/lib/query.py
@@ -1859,6 +1859,7 @@ def new_pull_request(
     title,
     user,
     initial_comment=None,
+    allow_rebase=False,
     repo_from=None,
     remote_git=None,
     requestuid=None,
@@ -1887,6 +1888,7 @@ def new_pull_request(
         branch_from=branch_from,
         title=title,
         initial_comment=initial_comment or None,
+        allow_rebase=allow_rebase,
         user_id=user_obj.id,
         status=status,
         commit_start=commit_start,
diff --git a/pagure/templates/repo_new_pull_request.html b/pagure/templates/repo_new_pull_request.html
index fef1c0b..213d40e 100644
--- a/pagure/templates/repo_new_pull_request.html
+++ b/pagure/templates/repo_new_pull_request.html
@@ -208,6 +208,15 @@
           {% endif %}
           <div id="preview" class="p-1">
           </div>
+          <div class="form-control">
+            <label for="allow_rebase">Allow rebasing</label>
+            <label class="c-input c-checkbox">
+              <input checked id="allow_rebase" name="allow_rebase" type="checkbox" value="y">
+            </label>
+            <small class="text-muted">
+              Let the maintainer of the target project to rebase the pull-request
+            </small>
+          </div>
         </div>
         <div class="card-footer bg-light">
           <div class="d-flex align-items-center">
diff --git a/pagure/templates/repo_pull_request.html b/pagure/templates/repo_pull_request.html
index 8d3e99e..60c25d8 100644
--- a/pagure/templates/repo_pull_request.html
+++ b/pagure/templates/repo_pull_request.html
@@ -179,7 +179,7 @@
           </a>
           <div id="merge-alert" class="text-xs-center dropdown-menu dropdown-menu-right p-0" style="min-width:400px">
              <div class="alert text-center mb-0">
-                {% if pull_request.status == 'Open' and g.repo_committer %}
+                {% if pull_request.status == 'Open' and g.repo_committer and pull_request.allow_rebase %}
                 <small id="merge-alert-message"></small>
                 <form action="{{ url_for('ui_ns.merge_request_pull',
                         repo=repo.name,
diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py
index 6262e20..ce43d23 100644
--- a/pagure/ui/fork.py
+++ b/pagure/ui/fork.py
@@ -1712,6 +1712,7 @@ def new_request_pull(
                 repo_from=repo,
                 title=form.title.data,
                 initial_comment=initial_comment,
+                allow_rebase=form.allow_rebase.data,
                 user=flask.g.fas_user.username,
                 commit_start=commit_start,
                 commit_stop=commit_stop,
@@ -1756,6 +1757,8 @@ def new_request_pull(
 
     if not flask.g.repo_committer:
         form = None
+    elif flask.request.method == "GET":
+        form.allow_rebase.data = True
 
     # if the pull request we are creating only has one commit,
     # we automatically fill out the form fields for the PR with

From 6b4e5077af7cd0891203ffe6047da986ac48a502 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: May 20 2019 11:52:52 +0000
Subject: [PATCH 3/4] Fix description of the API error EPULLREQUESTSDISABLED


Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py
index 13e28df..d6218de 100644
--- a/pagure/api/__init__.py
+++ b/pagure/api/__init__.py
@@ -81,7 +81,7 @@ class APIERROR(enum.Enum):
     EISSUENOTALLOWED = "You are not allowed to view this issue"
     EPRNOTALLOWED = "You are not allowed to view this pull-request"
     EPULLREQUESTSDISABLED = (
-        "Pull-Request have been deactivated for this " "project"
+        "Pull-Request have been deactivated for this project"
     )
     ENOREQ = "Pull-Request not found"
     ENOPRCLOSE = (

From b2d89792d5040bbd741e783585bb24d7fd306871 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: May 20 2019 11:52:52 +0000
Subject: [PATCH 4/4] Do not allow rebase via the API if the PR does not allow it


Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py
index d6218de..ce067a8 100644
--- a/pagure/api/__init__.py
+++ b/pagure/api/__init__.py
@@ -122,6 +122,7 @@ class APIERROR(enum.Enum):
     ETRACKERREADONLY = "The issue tracker of this project is read-only"
     ENOPRSTATS = "No statistics could be computed for this PR"
     EUBLOCKED = "You have been blocked from this project"
+    EREBASENOTALLOWED = "You are not authorized to rebase this pull-request"
 
 
 def get_authorized_api_project(session, repo, user=None, namespace=None):
diff --git a/pagure/api/fork.py b/pagure/api/fork.py
index 0f0ba2d..fb2a4a7 100644
--- a/pagure/api/fork.py
+++ b/pagure/api/fork.py
@@ -665,11 +665,16 @@ def api_pull_request_rebase(repo, requestid, username=None, namespace=None):
     repo = _get_repo(repo, username, namespace)
     _check_pull_request(repo)
     _check_token(repo)
-    _get_request(repo, requestid)
+    request = _get_request(repo, requestid)
 
     if not is_repo_committer(repo):
         raise pagure.exceptions.APIError(403, error_code=APIERROR.ENOPRCLOSE)
 
+    if not request.allow_rebase:
+        raise pagure.exceptions.APIError(
+            403, error_code=APIERROR.EREBASENOTALLOWED
+        )
+
     task = pagure.lib.tasks.rebase_pull_request.delay(
         repo.name,
         namespace,
diff --git a/tests/test_pagure_flask_api.py b/tests/test_pagure_flask_api.py
index 2cb96c0..e311da8 100644
--- a/tests/test_pagure_flask_api.py
+++ b/tests/test_pagure_flask_api.py
@@ -241,7 +241,7 @@ class PagureFlaskApitests(tests.SimplePagureTest):
         output = self.app.get("/api/0/-/error_codes")
         self.assertEqual(output.status_code, 200)
         data = json.loads(output.get_data(as_text=True))
-        self.assertEqual(len(data), 36)
+        self.assertEqual(len(data), 37)
         self.assertEqual(
             sorted(data.keys()),
             sorted(
@@ -282,6 +282,7 @@ class PagureFlaskApitests(tests.SimplePagureTest):
                     "ETRACKERDISABLED",
                     "ETRACKERREADONLY",
                     "EUBLOCKED",
+                    "EREBASENOTALLOWED",
                 ]
             ),
         )
diff --git a/tests/test_pagure_flask_rebase.py b/tests/test_pagure_flask_rebase.py
index 240047f..2284339 100644
--- a/tests/test_pagure_flask_rebase.py
+++ b/tests/test_pagure_flask_rebase.py
@@ -68,6 +68,7 @@ class PagureRebasetests(tests.Modeltests):
             branch_to="master",
             title="PR from the test branch",
             user="pingou",
+            allow_rebase=True,
         )
         self.session.commit()
         self.assertEqual(req.id, 1)
@@ -400,5 +401,153 @@ class PagureRebasetests(tests.Modeltests):
         )
 
 
+class PagureRebaseNotAllowedtests(tests.Modeltests):
+    """ Tests rebasing pull-request in pagure """
+
+    maxDiff = None
+
+    @patch("pagure.lib.notify.send_email", MagicMock(return_value=True))
+    def setUp(self):
+        """ Set up the environnment, ran before every tests. """
+        super(PagureRebaseNotAllowedtests, self).setUp()
+
+        pagure.config.config["REQUESTS_FOLDER"] = None
+        tests.create_projects(self.session)
+        tests.create_projects_git(os.path.join(self.path, "repos"), bare=True)
+        tests.create_projects_git(
+            os.path.join(self.path, "requests"), bare=True
+        )
+        tests.add_content_to_git(
+            os.path.join(self.path, "repos", "test.git"),
+            branch="master",
+            content="foobarbaz",
+            filename="testfile",
+        )
+        tests.add_content_to_git(
+            os.path.join(self.path, "repos", "test.git"),
+            branch="test",
+            content="foobar",
+            filename="sources",
+        )
+        tests.add_readme_git_repo(os.path.join(self.path, "repos", "test.git"))
+
+        # Create a PR for these changes
+        project = pagure.lib.query.get_authorized_project(self.session, "test")
+        req = pagure.lib.query.new_pull_request(
+            session=self.session,
+            repo_from=project,
+            branch_from="test",
+            repo_to=project,
+            branch_to="master",
+            title="PR from the test branch",
+            user="pingou",
+            allow_rebase=False,
+        )
+        self.session.commit()
+        self.assertEqual(req.id, 1)
+        self.assertEqual(req.title, "PR from the test branch")
+
+        self.project = pagure.lib.query.get_authorized_project(
+            self.session, "test"
+        )
+        self.assertEqual(len(project.requests), 1)
+        self.request = self.project.requests[0]
+
+    def test_rebase_api_ui_logged_in(self):
+        """ Test the rebase PR API endpoint when logged in from the UI and
+        its outcome. """
+
+        user = tests.FakeUser(username="pingou")
+        with tests.user_set(self.app.application, user):
+            # Get the merge status first so it's cached and can be refreshed
+            csrf_token = self.get_csrf()
+            data = {"requestid": self.request.uid, "csrf_token": csrf_token}
+            output = self.app.post("/pv/pull-request/merge", data=data)
+            self.assertEqual(output.status_code, 200)
+            data = json.loads(output.get_data(as_text=True))
+            self.assertEqual(
+                data,
+                {
+                    "code": "MERGE",
+                    "message": "The pull-request can be merged with "
+                    "a merge commit",
+                    "short_code": "With merge",
+                },
+            )
+
+            output = self.app.post("/api/0/test/pull-request/1/rebase")
+            self.assertEqual(output.status_code, 403)
+            data = json.loads(output.get_data(as_text=True))
+            self.assertEqual(
+                data,
+                {
+                    "error": "You are not authorized to rebase this pull-request",
+                    "error_code": "EREBASENOTALLOWED",
+                },
+            )
+
+    def test_rebase_api_ui_logged_in_different_user(self):
+        """ Test the rebase PR API endpoint when logged in from the UI and
+        its outcome. """
+        # Add 'foo' to the project 'test' so 'foo' can rebase the PR
+        repo = pagure.lib.query._get_project(self.session, "test")
+        msg = pagure.lib.query.add_user_to_project(
+            session=self.session, project=repo, new_user="foo", user="pingou"
+        )
+        self.session.commit()
+        self.assertEqual(msg, "User added")
+
+        user = tests.FakeUser(username="foo")
+        with tests.user_set(self.app.application, user):
+            # Get the merge status first so it's cached and can be refreshed
+            csrf_token = self.get_csrf()
+            data = {"requestid": self.request.uid, "csrf_token": csrf_token}
+            output = self.app.post("/pv/pull-request/merge", data=data)
+            self.assertEqual(output.status_code, 200)
+            data = json.loads(output.get_data(as_text=True))
+            self.assertEqual(
+                data,
+                {
+                    "code": "MERGE",
+                    "message": "The pull-request can be merged with "
+                    "a merge commit",
+                    "short_code": "With merge",
+                },
+            )
+
+            output = self.app.post("/api/0/test/pull-request/1/rebase")
+            self.assertEqual(output.status_code, 403)
+            data = json.loads(output.get_data(as_text=True))
+            self.assertEqual(
+                data,
+                {
+                    "error": "You are not authorized to rebase this pull-request",
+                    "error_code": "EREBASENOTALLOWED",
+                },
+            )
+
+    def test_rebase_api_api_logged_in(self):
+        """ Test the rebase PR API endpoint when using an API token and
+        its outcome. """
+
+        tests.create_tokens(self.session)
+        tests.create_tokens_acl(self.session)
+
+        headers = {"Authorization": "token aaabbbcccddd"}
+
+        output = self.app.post(
+            "/api/0/test/pull-request/1/rebase", headers=headers
+        )
+        self.assertEqual(output.status_code, 403)
+        data = json.loads(output.get_data(as_text=True))
+        self.assertEqual(
+            data,
+            {
+                "error": "You are not authorized to rebase this pull-request",
+                "error_code": "EREBASENOTALLOWED",
+            },
+        )
+
+
 if __name__ == "__main__":
     unittest.main(verbosity=2)

