gitlab-org--gitlab-foss/lib
Rémy Coutable 9ca633eb4c Merge branch '18193-developers-can-merge' into 'master'
Allow developers to merge into a protected branch without having push access

## What does this MR do?

Adds a "Developers can merge" checkbox to protected branches much like the "Developers can push" checkbox. When the checkbox is enabled, a developer can merge MRs into that protected branch from the Web UI and from the command-line (any push that is entirely composed of merge commits is allowed).

## Are there points in the code the reviewer needs to double check?

- This MR refactors the `GitAccess` module, moving parts of it to `UserAccess` and the new `ChangeAccessCheck`.
- This MR refactors `GitAccessSpec`, which generates a "matrix" of tests.
- The main logic "developers can merge" should be straightforward enough.
- The commits are fairly atomic, and the commit messages are descriptive regarding the motivations behind every change.

## Why was this MR needed?

A significant portion of this feature was implemented in !4220 (thanks, @mvestergaard!) ; I'm wrapping it up.

## What are the relevant issue numbers?

#18193 
Closes #967 

## Screenshots

![1](/uploads/c636e88ba38628211754e7cf122b0dc4/1.png)
![2](/uploads/5ed1e7917e2f36853a479faa565b022a/2.png)
![3](/uploads/0d202ba42e8dc6aade7bc6ac8db41ee6/3.png)

## TODO

- [ ]  #18193 !4892 Add "developers can merge" as an option for protected branches
    - [x]  Review existing code
    - [x]  Fix build
    - [x]  Implementation / refactoring
        - [x]  Clean up `GitAccess`
        - [x]  Clean up `protected_branches.js.coffee`
        - [x]  Figure out authorization issue
            - If we try to merge code into a protected branch for a user who doesn't have access to that branch, an auth check will fail
            - We need to get around this, somehow
            - [x]  Try detecting merge commits and allowing those
        - [x]  A push with regular commits _and_ merge commits should fail
            - [x]  Figure out a solution
            - [x]  Extensive tests for `MergeCommitCheck`
    - [x]  Add tests
        - [x]  Untested parts of original MR
    - [x]  Improve the checks in `/allowed`
        -  @dzaporozhets's proposal:
            - commits in push == commits in merge request
            - branch to push == target branch of merge request
            - merge request has required amount of approves (ee only)
            - merge commit in push == merge commit we created when merged via UI
            - save merge commit sha in database and compare with `newrev`
        - my proposal
            - /allowed finds all open merge requests with the appropriate target branch
            - For each MR, compare the commit SHAs in the MR to the commit SHAs in the change set
            - If we have a match, compare the diff of the matching MR to the diff of the change set
            - If we still have a match, the merge is legit
        - [x]  Wait for replies on my proposal
        - [x]  Pick a strategy
        - [x]  Implementation
            - [x]  Save `in_progress_merge_commit_sha`
            - [x]  Check `in_progress_merge_commit_sha`
            - [x]  Clear `in_progress_merge_commit_sha`
        - [x]  Test / refactor
    - [x]  Merge conflicts
    - [x]  Verify workflows
        - [x]  Developer with 'developer can merge' on:
            - [x]  Can merge an MR from the Web UI
            - [x]  Error message for conflicts in the Web UI
            - [x]  Cannot merge an MR from the command line (HTTP)
            - [x]  Cannot merge an MR from the command line (SSH)
            - [x]  Cannot modify the branch otherwise
        - [x]  Developer with 'developer can merge' off:
            - [x]  Cannot merge an MR from the Web UI
            - [x]  Error message for conflicts in the Web UI
            - [x]  Cannot merge an MR from the command line (HTTP)
            - [x]  Cannot merge an MR from the command line (SSH)
            - [x]  Cannot modify the branch otherwise
        - [x]  New projects created could have have "Developers can merge" turned on automatically for the default branch
    - [x]  CHANGELOG
    - [x]  Fix build
    - [x]  Wait for [build](42624e3d53/builds) to pass
    - [x]  Screenshots
    - [x]  Assign to endboss
    - [x]  Respond to @dbalexandre's comments
        - [x]  Duplicated line, this is equals to line 26.
        - [x]  We aren't using any of these helpers in this migration, we can remove the include.
        - [x]  What do you think to add a default value for this column to avoid the Three-state Boolean Problem?
        - [x]  group all checks under Gitlab::Checks
        - [x]  You have a default value for developers_can_merge column, but your migration doesn't add it.
        - [x]  What do you think to rename Partially protected to anything else?
    - [x]  Fix conflicts
    - [x]  Make sure [build](b1cfd42f20/builds) passes
    - [ ]  Wait for merge

See merge request !4892
2016-07-13 11:14:57 +00:00
..
api Refactor `Gitlab::GitAccess` 2016-07-13 13:24:56 +05:30
assets
backup Refactor repository paths handling to allow multiple git mount points 2016-06-29 22:30:31 -04:00
banzai ObjectRenderer doesn't crash when no objects to cache with Rails.cache.read_multi 2016-07-13 11:19:21 +02:00
ci Merge branch 'refactor/ci-config-move-global-entries' into 'master' 2016-07-05 08:37:16 +00:00
container_registry Show proper image ID on registry page 2016-06-21 13:08:10 +02:00
gitlab Implement last round of review comments from !4892. 2016-07-13 14:18:05 +05:30
json_web_token Revert "Fix merge conflicts - squashed commit" 2016-06-03 11:10:17 +02:00
omni_auth
rouge/formatters Enable Style/IdenticalConditionalBranches Rubocop cop 2016-07-08 11:04:04 +02:00
support Defend against 'Host' header injection 2016-07-12 19:50:20 +02:00
tasks Merge branch 'update-gemoji' into 'master' 2016-07-05 17:08:35 +00:00
banzai.rb Object renderer read_multi rendered entries from Cache 2016-07-12 14:35:29 +02:00
disable_email_interceptor.rb Enable Style/EmptyLines cop, remove redundant ones 2016-07-01 21:56:17 +02:00
event_filter.rb Revert "Fix merge conflicts - squashed commit" 2016-06-03 11:10:17 +02:00
extracts_path.rb
file_size_validator.rb Get rid of more requires, which causes warnings when code is reloaded 2016-04-19 11:48:10 +02:00
file_streamer.rb
gitlab.rb Update `Gitlab.com?` to support staging 2016-06-27 15:10:36 -04:00
gt_one_coercion.rb
repository_cache.rb
static_model.rb
unfold_form.rb
uploaded_file.rb Enable Style/EmptyLines cop, remove redundant ones 2016-07-01 21:56:17 +02:00
version_check.rb