Commit graph

15 commits

Author SHA1 Message Date
Rémy Coutable
0b5b3ec3a4
Remove useless permission checks in Gitlab::Checks::ChangeAccess
Signed-off-by: Rémy Coutable <remy@rymai.me>
2017-01-12 17:37:14 -05:00
Lin Jen-Shin
0f0738e788 Merge remote-tracking branch 'upstream/master' into feature/1376-allow-write-access-deploy-keys
* upstream/master: (538 commits)
  Reject blank environment vcariables in Gitlab::Git::RevList
  Add online terminal documentation
  Add changelog entry
  Add terminal UI and controller actions
  Fix specs
  Even out padding on plus button in breadcrumb menu
  Update font size of detail page header to 14px
  Update CHANGELOG.md for 8.13.10
  Update CHANGELOG.md for 8.14.5
  Fix Route#rename_children behavior
  Remove inline-block styling from status
  Add terminals to the Kubernetes deployment service
  Add a ReactiveCaching concern for use in the KubernetesService
  Add xterm.js 2.1.0 and a wrapper class to the asset pipeline
  Remove unnecessary hidden svg elements for icons.
  Fix consistent typo in environment.js
  Use a block to insert extra check for authenticate_build!
  Align milestone column header with count number
  Add Wiki import to BB importer
  Make CI badge hitboxes better match container
  ...
2016-12-20 20:54:40 +08:00
Timothy Andrew
e394d2872a Implement final review comments from @rymai.
- `raise "string"` raises a `RuntimeError` - no need to be explicit
- Remove top-level comment in the `RevList` class
- Use `%w()` instead of `%w[]`
- Extract an `environment_variables` method to cache `env.slice(*ALLOWED_VARIABLES)`
- Use `start_with?` for env variable validation instead of regex match
- Validation specs for each allowed environment variable were identical. Build them dynamically.
- Minor change to `popen3` expectation.
2016-12-16 23:32:25 +05:30
Timothy Andrew
c937aec1f7 Check the exit code while invoking git in the force push check.
Previously, we were calling out to `popen` without asserting on the returned
exit-code. Now we raise a `RuntimeError` if the exit code is non-zero.
2016-12-16 23:32:25 +05:30
Timothy Andrew
f82d549d26 Accept environment variables from the pre-receive script.
1. Starting version 2.11, git changed the way the pre-receive flow works.

  - Previously, the new potential objects would be added to the main repo. If the
    pre-receive passes, the new objects stay in the repo but are linked up. If
    the pre-receive fails, the new objects stay orphaned in the repo, and are
    cleaned up during the next `git gc`.

  - In 2.11, the new potential objects are added to a temporary "alternate object
    directory", that git creates for this purpose. If the pre-receive passes, the
    objects from the alternate object directory are migrated to the main repo. If
    the pre-receive fails the alternate object directory is simply deleted.

2. In our workflow, the pre-recieve script (in `gitlab-shell) calls the
   `/allowed` endpoint, which calls out directly to git to perform
   various checks. These direct calls to git do _not_ have the necessary
   environment variables set which allow access to the "alternate object
   directory" (explained above). Therefore these calls to git are not able to
   access any of the new potential objects to be added during this push.

3. We fix this by accepting the relevant environment variables
   (GIT_ALTERNATE_OBJECT_DIRECTORIES, GIT_OBJECT_DIRECTORY) on the
   `/allowed` endpoint, and then include these environment variables while
   calling out to git.

4. This commit includes (whitelisted) these environment variables while making
   the "force push" check. A `Gitlab::Git::RevList` module is extracted to
   prevent `ForcePush` from being littered with these checks.
2016-12-16 23:32:25 +05:30
Lin Jen-Shin
8c1a01e05f We never check user privilege if it's a deploy key 2016-11-18 03:48:23 +08:00
Paco Guzman
08871cc36a Avoid protected branches checks when verifying access without branch name
GitlabShell verify access sending ‘_any’ as the changes made on the git command, in those cases Gitlab::Checks::ChangeAccess won’t receive a branch_name so we don’t need to check for access to the 
protected branches on that repository. So we avoid some git operations in case the are not cached (empty_repo?) and some database lookups to get protected branches.

These request is happening in every push.
2016-09-13 11:50:13 +02:00
Alejandro Rodríguez
2f06027dc3 Change the order of the access rules to check simpler first, and add specs 2016-08-12 18:27:42 -04:00
Scott Le
6109daf480 api for generating new merge request
DRY code + fix rubocop

Add more test cases

Append to changelog

DRY changes list

find_url service for merge_requests

use GET for getting merge request links

remove files

rename to get_url_service

reduce loop

add test case for cross project

refactor tiny thing

update changelog
2016-08-11 23:37:00 +07:00
Ahmad Sherif
41b25e5a0d Limit git rev-list output count to one in forced push check 2016-07-19 21:22:58 +02:00
Rémy Coutable
2cf7f09b1e
Revert "Revert "Merge branch '18193-developers-can-merge' into 'master' ""
This reverts commit 530f5158e2.

See !4892.

Signed-off-by: Rémy Coutable <remy@rymai.me>
2016-07-18 10:16:56 +02:00
Robert Speicher
530f5158e2 Revert "Merge branch '18193-developers-can-merge' into 'master' "
This reverts commit 9ca633eb4c, reversing
changes made to fb229bbf79.
2016-07-13 13:57:30 -05:00
Timothy Andrew
bb81f2afc1 Implement last round of review comments from !4892.
1. Fix typos, minor styling errors.

2. Use single quotes rather than double quotes in `user_access_spec`.

3. Test formatting.
2016-07-13 14:18:05 +05:30
Timothy Andrew
ea9e8f4609 Move all "checks" under GitLab::Checks.
- https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4892#note_12892160
- This is more consistent.
2016-07-13 13:24:56 +05:30
Timothy Andrew
495db09653 Enforce "developers can merge" during pre-receive.
1. When a merge request is being merged, save the merge commit SHA in
   the `in_progress_merge_commit_sha` database column.

2. The `pre-receive` hook looks for any locked (in progress) merge
   request with `in_progress_merge_commit_sha` matching the `newrev` it
   is passed.

3. If it finds a matching MR, the merge is legitimate.

4. Update `git_access_spec` to test the behaviour we added here. Also
   refactored this spec a bit to make it easier to add more contexts / conditions.
2016-07-13 13:24:56 +05:30