From d7d644a42814e46915f8517f4fdbc8d10e52cfa5 Mon Sep 17 00:00:00 2001 From: Kelvin Date: Fri, 4 Mar 2016 12:58:05 +0300 Subject: [PATCH 01/69] Prepend letter v to GitLab Workhorse version numbers --- doc/update/patch_versions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/update/patch_versions.md b/doc/update/patch_versions.md index a10e62877b..9bb007d4f4 100644 --- a/doc/update/patch_versions.md +++ b/doc/update/patch_versions.md @@ -47,7 +47,7 @@ sudo -u git -H git checkout v`cat /home/git/gitlab/GITLAB_SHELL_VERSION` -b v`ca ```bash cd /home/git/gitlab-workhorse sudo -u git -H git fetch -sudo -u git -H git checkout `cat /home/git/gitlab/GITLAB_WORKHORSE_VERSION` -b `cat /home/git/gitlab/GITLAB_WORKHORSE_VERSION` +sudo -u git -H git checkout v`cat /home/git/gitlab/GITLAB_WORKHORSE_VERSION` -b v`cat /home/git/gitlab/GITLAB_WORKHORSE_VERSION` sudo -u git -H make ``` From de95ab6257fcb38f109d9d1aea03618ae9981738 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Sat, 16 Apr 2016 18:03:20 -0600 Subject: [PATCH 02/69] Enable the UrlFormat and UrlQuotes SCSS Linters. --- .scss-lint.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.scss-lint.yml b/.scss-lint.yml index 835a4a88c4..bddb48eb09 100644 --- a/.scss-lint.yml +++ b/.scss-lint.yml @@ -244,11 +244,11 @@ linters: # URLs should be valid and not contain protocols or domain names. UrlFormat: - enabled: false + enabled: true # URLs should always be enclosed within quotes. UrlQuotes: - enabled: false + enabled: true # Properties, like color and font, are easier to read and maintain # when defined using variables rather than literals. From d3462e711c0b3cc17ef47e1ffffa6f40f98b5e98 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 22 Apr 2016 23:19:55 +0200 Subject: [PATCH 03/69] Fix issue with impersonation --- .../admin/application_controller.rb | 8 +- .../admin/impersonation_controller.rb | 38 -------- .../admin/impersonations_controller.rb | 28 ++++++ app/controllers/admin/users_controller.rb | 16 ++++ app/views/layouts/header/_default.html.haml | 2 +- config/routes.rb | 6 +- .../admin/impersonation_controller_spec.rb | 19 ---- .../admin/impersonations_controller_spec.rb | 95 +++++++++++++++++++ .../admin/users_controller_spec.rb | 49 +++++++++- 9 files changed, 192 insertions(+), 69 deletions(-) delete mode 100644 app/controllers/admin/impersonation_controller.rb create mode 100644 app/controllers/admin/impersonations_controller.rb delete mode 100644 spec/controllers/admin/impersonation_controller_spec.rb create mode 100644 spec/controllers/admin/impersonations_controller_spec.rb diff --git a/app/controllers/admin/application_controller.rb b/app/controllers/admin/application_controller.rb index 9083bfb41c..cf795d977c 100644 --- a/app/controllers/admin/application_controller.rb +++ b/app/controllers/admin/application_controller.rb @@ -6,12 +6,6 @@ class Admin::ApplicationController < ApplicationController layout 'admin' def authenticate_admin! - return render_404 unless current_user.is_admin? - end - - def authorize_impersonator! - if session[:impersonator_id] - User.find_by!(username: session[:impersonator_id]).admin? - end + render_404 unless current_user.is_admin? end end diff --git a/app/controllers/admin/impersonation_controller.rb b/app/controllers/admin/impersonation_controller.rb deleted file mode 100644 index bf98af7861..0000000000 --- a/app/controllers/admin/impersonation_controller.rb +++ /dev/null @@ -1,38 +0,0 @@ -class Admin::ImpersonationController < Admin::ApplicationController - skip_before_action :authenticate_admin!, only: :destroy - - before_action :user - before_action :authorize_impersonator! - - def create - if @user.blocked? - flash[:alert] = "You cannot impersonate a blocked user" - - redirect_to admin_user_path(@user) - else - session[:impersonator_id] = current_user.username - session[:impersonator_return_to] = admin_user_path(@user) - - warden.set_user(user, scope: 'user') - - flash[:alert] = "You are impersonating #{user.username}." - - redirect_to root_path - end - end - - def destroy - redirect = session[:impersonator_return_to] - - warden.set_user(user, scope: 'user') - - session[:impersonator_return_to] = nil - session[:impersonator_id] = nil - - redirect_to redirect || root_path - end - - def user - @user ||= User.find_by!(username: params[:id] || session[:impersonator_id]) - end -end diff --git a/app/controllers/admin/impersonations_controller.rb b/app/controllers/admin/impersonations_controller.rb new file mode 100644 index 0000000000..1ca3dd8228 --- /dev/null +++ b/app/controllers/admin/impersonations_controller.rb @@ -0,0 +1,28 @@ +class Admin::ImpersonationsController < Admin::ApplicationController + skip_before_action :authenticate_admin! + before_action :authenticate_impersonator! + + def destroy + redirect_path = admin_user_path(current_user) + + warden.set_user(impersonator, scope: :user) + + session[:impersonator_id] = nil + + redirect_to redirect_path + end + + private + + def user + @user ||= User.find(params[:id]) + end + + def impersonator + @impersonator ||= User.find(session[:impersonator_id]) if session[:impersonator_id] + end + + def authenticate_impersonator! + render_404 unless impersonator && impersonator.is_admin? && !impersonator.blocked? + end +end diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 9abf08d0e1..b8976fa09a 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -31,6 +31,22 @@ class Admin::UsersController < Admin::ApplicationController user end + def impersonate + if user.blocked? + flash[:alert] = "You cannot impersonate a blocked user" + + redirect_to admin_user_path(user) + else + session[:impersonator_id] = current_user.id + + warden.set_user(user, scope: :user) + + flash[:alert] = "You are now impersonating #{user.username}" + + redirect_to root_path + end + end + def block if user.block redirect_back_or_admin_user(notice: "Successfully blocked") diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 3beb8ff7c0..cde9e1b918 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -15,7 +15,7 @@ - if current_user - if session[:impersonator_id] %li.impersonation - = link_to stop_impersonation_admin_users_path, method: :delete, title: 'Stop Impersonation', data: { toggle: 'tooltip', placement: 'bottom', container: 'body' } do + = link_to admin_impersonation_path, method: :delete, title: 'Stop Impersonation', data: { toggle: 'tooltip', placement: 'bottom', container: 'body' } do = icon('user-secret fw') - if current_user.is_admin? %li diff --git a/config/routes.rb b/config/routes.rb index 2f820aafed..9c6fb682fc 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -212,8 +212,6 @@ Rails.application.routes.draw do resources :keys, only: [:show, :destroy] resources :identities, except: [:show] - delete 'stop_impersonation' => 'impersonation#destroy', on: :collection - member do get :projects get :keys @@ -223,12 +221,14 @@ Rails.application.routes.draw do put :unblock put :unlock put :confirm - post 'impersonate' => 'impersonation#create' + post :impersonate patch :disable_two_factor delete 'remove/:email_id', action: 'remove_email', as: 'remove_email' end end + resource :impersonation, only: :destroy + resources :abuse_reports, only: [:index, :destroy] resources :spam_logs, only: [:index, :destroy] diff --git a/spec/controllers/admin/impersonation_controller_spec.rb b/spec/controllers/admin/impersonation_controller_spec.rb deleted file mode 100644 index d7a7ba1c5b..0000000000 --- a/spec/controllers/admin/impersonation_controller_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -require 'spec_helper' - -describe Admin::ImpersonationController do - let(:admin) { create(:admin) } - - before do - sign_in(admin) - end - - describe 'CREATE #impersonation when blocked' do - let(:blocked_user) { create(:user, state: :blocked) } - - it 'does not allow impersonation' do - post :create, id: blocked_user.username - - expect(flash[:alert]).to eq 'You cannot impersonate a blocked user' - end - end -end diff --git a/spec/controllers/admin/impersonations_controller_spec.rb b/spec/controllers/admin/impersonations_controller_spec.rb new file mode 100644 index 0000000000..8b48c6b6ea --- /dev/null +++ b/spec/controllers/admin/impersonations_controller_spec.rb @@ -0,0 +1,95 @@ +require 'spec_helper' + +describe Admin::ImpersonationsController do + let(:impersonator) { create(:admin) } + let(:user) { create(:user) } + + describe "DELETE destroy" do + context "when not signed in" do + it "redirects to the sign in page" do + delete :destroy + + expect(response).to redirect_to(new_user_session_path) + end + end + + context "when signed in" do + before do + sign_in(user) + end + + context "when not impersonating" do + it "responds with status 404" do + delete :destroy + + expect(response.status).to eq(404) + end + + it "doesn't sign us in" do + delete :destroy + + expect(warden.user).to eq(user) + end + end + + context "when impersonating" do + before do + session[:impersonator_id] = impersonator.id + end + + context "when the impersonator is not impersonator (anymore)" do + before do + impersonator.admin = false + impersonator.save + end + + it "responds with status 404" do + delete :destroy + + expect(response.status).to eq(404) + end + + it "doesn't sign us in as the impersonator" do + delete :destroy + + expect(warden.user).to eq(user) + end + end + + context "when the impersonator is admin" do + context "when the impersonator is blocked" do + before do + impersonator.block! + end + + it "responds with status 404" do + delete :destroy + + expect(response.status).to eq(404) + end + + it "doesn't sign us in as the impersonator" do + delete :destroy + + expect(warden.user).to eq(user) + end + end + + context "when the impersonator is not blocked" do + it "redirects to the impersonated user's page" do + delete :destroy + + expect(response).to redirect_to(admin_user_path(user)) + end + + it "signs us in as the impersonator" do + delete :destroy + + expect(warden.user).to eq(impersonator) + end + end + end + end + end + end +end diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 9ef8ba1b09..ce2a62ae1f 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -2,9 +2,10 @@ require 'spec_helper' describe Admin::UsersController do let(:user) { create(:user) } + let(:admin) { create(:admin) } before do - sign_in(create(:admin)) + sign_in(admin) end describe 'DELETE #user with projects' do @@ -112,4 +113,50 @@ describe Admin::UsersController do patch :disable_two_factor, id: user.to_param end end + + describe "POST impersonate" do + context "when the user is blocked" do + before do + user.block! + end + + it "shows a notice" do + post :impersonate, id: user.username + + expect(flash[:alert]).to eq("You cannot impersonate a blocked user") + end + + it "doesn't sign us in as the user" do + post :impersonate, id: user.username + + expect(warden.user).to eq(admin) + end + end + + context "when the user is not blocked" do + it "stores the impersonator in the session" do + post :impersonate, id: user.username + + expect(session[:impersonator_id]).to eq(admin.id) + end + + it "signs us in as the user" do + post :impersonate, id: user.username + + expect(warden.user).to eq(user) + end + + it "redirects to root" do + post :impersonate, id: user.username + + expect(response).to redirect_to(root_path) + end + + it "shows a notice" do + post :impersonate, id: user.username + + expect(flash[:alert]).to eq("You are now impersonating #{user.username}") + end + end + end end From b992e2520cc93b3161f6e7c46dde66f10fe13a12 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 22 Apr 2016 21:46:47 +0000 Subject: [PATCH 04/69] Fix typo --- spec/controllers/admin/impersonations_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/admin/impersonations_controller_spec.rb b/spec/controllers/admin/impersonations_controller_spec.rb index 8b48c6b6ea..eb82476b17 100644 --- a/spec/controllers/admin/impersonations_controller_spec.rb +++ b/spec/controllers/admin/impersonations_controller_spec.rb @@ -37,7 +37,7 @@ describe Admin::ImpersonationsController do session[:impersonator_id] = impersonator.id end - context "when the impersonator is not impersonator (anymore)" do + context "when the impersonator is not admin (anymore)" do before do impersonator.admin = false impersonator.save From 0ab98a8a407cb9764c9c28554c8f077906a9eb9b Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 22 Apr 2016 21:55:49 +0000 Subject: [PATCH 05/69] Remove unused method --- app/controllers/admin/impersonations_controller.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/controllers/admin/impersonations_controller.rb b/app/controllers/admin/impersonations_controller.rb index 1ca3dd8228..2d64923c47 100644 --- a/app/controllers/admin/impersonations_controller.rb +++ b/app/controllers/admin/impersonations_controller.rb @@ -14,10 +14,6 @@ class Admin::ImpersonationsController < Admin::ApplicationController private - def user - @user ||= User.find(params[:id]) - end - def impersonator @impersonator ||= User.find(session[:impersonator_id]) if session[:impersonator_id] end From c6c985bc690c88f2819787824a50b22cc86cacf4 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 22 Apr 2016 21:58:09 +0000 Subject: [PATCH 06/69] Store original user in variable --- app/controllers/admin/impersonations_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/impersonations_controller.rb b/app/controllers/admin/impersonations_controller.rb index 2d64923c47..2db824c87e 100644 --- a/app/controllers/admin/impersonations_controller.rb +++ b/app/controllers/admin/impersonations_controller.rb @@ -3,13 +3,13 @@ class Admin::ImpersonationsController < Admin::ApplicationController before_action :authenticate_impersonator! def destroy - redirect_path = admin_user_path(current_user) + original_user = current_user warden.set_user(impersonator, scope: :user) session[:impersonator_id] = nil - redirect_to redirect_path + redirect_to admin_user_path(original_user) end private From 03ae2cdbff49d4f72d32529963a2173c7308da40 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 24 Apr 2016 20:07:59 -0700 Subject: [PATCH 07/69] Filter confidential issues from milestones API if user does not have access Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/15579 --- CHANGELOG | 2 ++ lib/api/milestones.rb | 10 ++++++- spec/requests/api/milestones_spec.rb | 40 +++++++++++++++++++++++++++- 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 2d1c561fb8..4ce16b4449 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -8,6 +8,8 @@ v 8.7.1 (unreleased) - Fix license detection to detect all license files, not only known licenses. !3878 - Use the `can?` helper instead of `current_user.can?`. !3882 - Prevent users from deleting Webhooks via API they do not own + - Use the `can?` helper instead of `current_user.can?` + - Filter confidential issues from milestones API if user does not have access v 8.7.0 - Gitlab::GitAccess and Gitlab::GitAccessWiki are now instrumented diff --git a/lib/api/milestones.rb b/lib/api/milestones.rb index 84b4d4cdd6..132043cf3f 100644 --- a/lib/api/milestones.rb +++ b/lib/api/milestones.rb @@ -105,7 +105,15 @@ module API authorize! :read_milestone, user_project @milestone = user_project.milestones.find(params[:milestone_id]) - present paginate(@milestone.issues), with: Entities::Issue, current_user: current_user + + finder_params = { + project_id: user_project.id, + milestone_title: @milestone.title, + state: 'all' + } + + issues = IssuesFinder.new(current_user, finder_params).execute + present paginate(issues), with: Entities::Issue, current_user: current_user end end diff --git a/spec/requests/api/milestones_spec.rb b/spec/requests/api/milestones_spec.rb index 344f0fe0b7..cb9c3dde5e 100644 --- a/spec/requests/api/milestones_spec.rb +++ b/spec/requests/api/milestones_spec.rb @@ -127,7 +127,7 @@ describe API::API, api: true do describe 'GET /projects/:id/milestones/:milestone_id/issues' do before do - milestone.issues << create(:issue) + milestone.issues << create(:issue, project: project) end it 'should return project issues for a particular milestone' do get api("/projects/#{project.id}/milestones/#{milestone.id}/issues", user) @@ -141,4 +141,42 @@ describe API::API, api: true do expect(response.status).to eq(401) end end + + describe 'confidential issues' do + it 'should return confidential issues to team members' do + public_project = create(:project, :public) + user = create(:user) + milestone = create(:milestone, project: public_project) + issue = create(:issue, project: public_project) + confidential_issue = create(:issue, confidential: true, project: public_project) + public_project.team << [user, :developer] + milestone.issues << issue + milestone.issues << confidential_issue + + get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", user) + + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.size).to eq(2) + expect(json_response.map { |issue| issue['id'] }).to include(issue.id, confidential_issue.id) + end + + it 'should not return confidential issues to regular users' do + public_project = create(:project, :public) + normal_user = create(:user) + milestone = create(:milestone, project: public_project) + issue = create(:issue, project: public_project) + confidential_issue = create(:issue, confidential: true, project: public_project) + public_project.team << [user, :developer] + milestone.issues << issue + milestone.issues << confidential_issue + + get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", normal_user) + + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.size).to eq(1) + expect(json_response.map { |issue| issue['id'] }).to include(issue.id) + end + end end From 5fc6a7dc14aa05748049eebd0a53d5e615cd1a9a Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Mon, 25 Apr 2016 13:51:17 +0200 Subject: [PATCH 08/69] Update using_docker_build.md, clarify the 'privileged' mode requirement [ci skip] --- doc/ci/docker/using_docker_build.md | 74 +++++++++++++++++++++-------- 1 file changed, 53 insertions(+), 21 deletions(-) diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md index 4b1788a9af..bb2a6d1137 100644 --- a/doc/ci/docker/using_docker_build.md +++ b/doc/ci/docker/using_docker_build.md @@ -8,7 +8,7 @@ This is one of new trends in Continuous Integration/Deployment to: 1. create application image, 1. run test against created image, -1. push image to remote registry, +1. push image to remote registry, 1. deploy server from pushed image It's also useful in case when your application already has the `Dockerfile` that can be used to create and test image: @@ -46,22 +46,22 @@ GitLab Runner then executes build scripts as `gitlab-runner` user. For more information how to install Docker on different systems checkout the [Supported installations](https://docs.docker.com/installation/). 3. Add `gitlab-runner` user to `docker` group: - + ```bash $ sudo usermod -aG docker gitlab-runner ``` 4. Verify that `gitlab-runner` has access to Docker: - + ```bash $ sudo -u gitlab-runner -H docker info ``` - + You can now verify that everything works by adding `docker info` to `.gitlab-ci.yml`: ```yaml before_script: - docker info - + build_image: script: - docker build -t my-docker-image . @@ -88,24 +88,56 @@ In order to do that follow the steps: --token RUNNER_TOKEN \ --executor docker \ --description "My Docker Runner" \ - --docker-image "gitlab/dind:latest" \ + --docker-image "docker:latest" \ --docker-privileged ``` - - The above command will register new Runner to use special [gitlab/dind](https://registry.hub.docker.com/u/gitlab/dind/) image which is provided by GitLab Inc. - The image at the start runs Docker daemon in [docker-in-docker](https://blog.docker.com/2013/09/docker-can-now-run-within-docker/) mode. -1. You can now use `docker` from build script: - - ```yaml - before_script: - - docker info - - build_image: - script: - - docker build -t my-docker-image . - - docker run my-docker-image /script/to/run/tests + The above command will register a new Runner to use special `docker:latest` image which is provided by Docker + creators. **Notice that it's using the `privileged` mode to start build and service containers.** If you want to use + [docker-in-docker](https://blog.docker.com/2013/09/docker-can-now-run-within-docker/) mode, you always have to use + `privileged = true` in your docker containers. + + The above command will create a `config.toml` entry similar to this: + + ``` + [[runners]] + url = "https://gitlab.com/ci" + token = TOKEN + executor = "docker" + [runners.docker] + tls_verify = false + image = "docker:latest" + privileged = true + disable_cache = false + volumes = ["/cache"] + [runners.cache] + Insecure = false ``` -1. However, by enabling `--docker-privileged` you are effectively disables all security mechanisms of containers and exposing your host to privilege escalation which can lead to container breakout. -For more information, check out [Runtime privilege](https://docs.docker.com/reference/run/#runtime-privilege-linux-capabilities-and-lxc-configuration). \ No newline at end of file + If you want to use Shared Runners available on your GitLab CE/EE installation, to build docker images, then + make sure that your Shared Runners configuration have `privileged` mode set to `true`. + +1. You can now use `docker` from build script: + + ```yaml + image: docker:latest + + services: + - docker:dind + + before_script: + - docker info + + build: + stage: build + script: + - docker build -t my-docker-image . + - docker run my-docker-image /script/to/run/tests + ``` + +1. However, by enabling `--docker-privileged` you are effectively disables all security mechanisms of containers and + exposing your host to privilege escalation which can lead to container breakout. + + For more information, check out [Runtime privilege](https://docs.docker.com/reference/run/#runtime-privilege-linux-capabilities-and-lxc-configuration). + +An example project using this approach can be found here: https://gitlab.com/gitlab-examples/docker. From 5542c9456c44816d45caba42b00389da0b1d1f26 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 25 Apr 2016 14:42:17 +0100 Subject: [PATCH 09/69] Updated project group sharing UI Closes #13991 --- app/assets/stylesheets/framework/selects.scss | 12 ++- .../stylesheets/framework/variables.scss | 4 +- app/assets/stylesheets/pages/profile.scss | 17 +--- app/assets/stylesheets/pages/settings.scss | 14 ++++ .../doorkeeper/applications/index.html.haml | 4 +- app/views/profiles/keys/_key.html.haml | 2 +- app/views/profiles/keys/_key_table.html.haml | 2 +- .../projects/group_links/index.html.haml | 79 ++++++++++--------- 8 files changed, 72 insertions(+), 62 deletions(-) create mode 100644 app/assets/stylesheets/pages/settings.scss diff --git a/app/assets/stylesheets/framework/selects.scss b/app/assets/stylesheets/framework/selects.scss index eae5f062dd..6efc6ec1e4 100644 --- a/app/assets/stylesheets/framework/selects.scss +++ b/app/assets/stylesheets/framework/selects.scss @@ -7,13 +7,11 @@ .select2-choice { background: #fff; border-color: $input-border; - border-color: $border-white-light; height: 35px; padding: $gl-vert-padding $gl-btn-padding; font-size: $gl-font-size; line-height: 1.42857143; - - @include border-radius($border-radius-default); + border-radius: $border-radius-base; .select2-arrow { background-image: none; @@ -199,6 +197,14 @@ } } +.select2-highlighted { + .group-result { + .group-path { + color: #fff; + } + } +} + .group-result { .group-image { float: left; diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 30ca27ab10..300f941bda 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -71,8 +71,8 @@ $gl-avatar-size: 40px; $error-exclamation-point: #e62958; $border-radius-default: 2px; $btn-transparent-color: #8f8f8f; -$ssh-key-icon-color: #8f8f8f; -$ssh-key-icon-size: 18px; +$settings-icon-color: #8f8f8f; +$settings-icon-size: 18px; $provider-btn-group-border: #e5e5e5; $provider-btn-not-active-color: #4688f1; diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index a9656e5cae..01f9847962 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -18,7 +18,8 @@ } .account-btn-link, -.profile-settings-sidebar a { +.profile-settings-sidebar a, +.settings-sidebar a { color: $md-link-color; } @@ -123,12 +124,6 @@ } } -.key-icon { - color: $ssh-key-icon-color; - font-size: $ssh-key-icon-size; - line-height: 42px; -} - .key-created-at { line-height: 42px; } @@ -180,14 +175,6 @@ } } -.profile-settings-message { - line-height: 32px; - color: $warning-message-color; - background-color: $warning-message-bg; - border: 1px solid $warning-message-border; - border-radius: $border-radius-base; -} - .oauth-applications { form { display: inline-block; diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss new file mode 100644 index 0000000000..67dbeca7bb --- /dev/null +++ b/app/assets/stylesheets/pages/settings.scss @@ -0,0 +1,14 @@ +.settings-list-icon { + color: $settings-icon-color; + font-size: $settings-icon-size; + line-height: 42px; +} + +.settings-message { + padding: 5px; + line-height: 1.3; + color: $warning-message-color; + background-color: $warning-message-bg; + border: 1px solid $warning-message-border; + border-radius: $border-radius-base; +} diff --git a/app/views/doorkeeper/applications/index.html.haml b/app/views/doorkeeper/applications/index.html.haml index 79df17ba61..3998e66f40 100644 --- a/app/views/doorkeeper/applications/index.html.haml +++ b/app/views/doorkeeper/applications/index.html.haml @@ -44,7 +44,7 @@ = icon('pencil') = render 'delete_form', application: application, small: true - else - .profile-settings-message.text-center + .settings-message.text-center You don't have any applications .oauth-authorized-applications.prepend-top-20.append-bottom-default - if user_oauth_applications? @@ -78,5 +78,5 @@ %td= token.scopes %td= render 'doorkeeper/authorized_applications/delete_form', token: token - else - .profile-settings-message.text-center + .settings-message.text-center You don't have any authorized applications diff --git a/app/views/profiles/keys/_key.html.haml b/app/views/profiles/keys/_key.html.haml index 4dbaa662b6..3276db6692 100644 --- a/app/views/profiles/keys/_key.html.haml +++ b/app/views/profiles/keys/_key.html.haml @@ -1,6 +1,6 @@ %li.key-list-item .pull-left.append-right-10 - = icon 'key', class: "key-icon hidden-xs" + = icon 'key', class: "settings-list-icon hidden-xs" .key-list-item-info = link_to path_to_key(key, is_admin), class: "title" do = key.title diff --git a/app/views/profiles/keys/_key_table.html.haml b/app/views/profiles/keys/_key_table.html.haml index 296cafa6e3..e78763bdcb 100644 --- a/app/views/profiles/keys/_key_table.html.haml +++ b/app/views/profiles/keys/_key_table.html.haml @@ -4,7 +4,7 @@ %ul.well-list = render partial: 'profiles/keys/key', collection: @keys, locals: { is_admin: is_admin } - else - %p.profile-settings-message.text-center + %p.settings-message.text-center - if is_admin There are no SSH keys associated with this account. - else diff --git a/app/views/projects/group_links/index.html.haml b/app/views/projects/group_links/index.html.haml index 13f5fc141f..627419c1b1 100644 --- a/app/views/projects/group_links/index.html.haml +++ b/app/views/projects/group_links/index.html.haml @@ -1,41 +1,44 @@ - page_title "Groups" -%h3.page_title Share project with other groups -%p.light - Projects can be stored in only one group at once. However you can share a project with other groups here. -%hr -- if @group_links.present? - .enabled-groups.panel.panel-default - .panel-heading - Already shared with - %ul.well-list - - @group_links.each do |group_link| - - group = group_link.group - %li - .pull-right - = link_to namespace_project_group_link_path(@project.namespace, @project, group_link), method: :delete, class: 'btn btn-sm' do - %i.icon-remove - disable sharing - = link_to group do - %strong - %i.icon-folder-open - = group.name - %br - .light up to #{group_link.human_access} - - -.available-groups - %h4 - Can be shared with - %div - = form_tag namespace_project_group_links_path(@project.namespace, @project), method: :post, class: 'form-horizontal' do +.row.prepend-top-default + .col-lg-3.settings-sidebar + %h4.prepend-top-0 + Share project with other groups + %p + Projects can be stored in only one group at once. However you can share a project with other groups here. + .col-lg-9 + %h5.prepend-top-0 + Set a group to share + = form_tag namespace_project_group_links_path(@project.namespace, @project), method: :post do .form-group - = label_tag :link_group_id, 'Group', class: 'control-label' - .col-sm-10 - = groups_select_tag(:link_group_id, skip_group: @project.group.try(:path)) + = label_tag :link_group_id, "Group", class: "label-light" + = groups_select_tag(:link_group_id, skip_group: @project.group.try(:path)) .form-group - = label_tag :link_group_access, 'Max access level', class: 'control-label' - .col-sm-10 - = select_tag :link_group_access, options_for_select(ProjectGroupLink.access_options, ProjectGroupLink.default_access), class: "form-control" - .form-actions - = submit_tag "Share", class: "btn btn-create" - + = label_tag :link_group_access, "Max access level", class: "label-light" + = select_tag :link_group_access, options_for_select(ProjectGroupLink.access_options, ProjectGroupLink.default_access), class: "form-control" + = submit_tag "Share", class: "btn btn-create" + .col-sm-12 + %hr + .col-lg-3.settings-sidebar + %h4.prepend-top-0 + .col-lg-9.append-bottom-default + %h5.prepend-top-0 + Groups you share with (#{@group_links.size}) + - if @group_links.present? + %ul.well-list + - @group_links.each do |group_link| + - group = group_link.group + %li + .pull-left.append-right-10.hidden-xs + = icon("folder-open-o", class: "settings-list-icon") + .pull-left + = link_to group do + = group.name + %br + up to #{group_link.human_access} + .pull-right + = link_to namespace_project_group_link_path(@project.namespace, @project, group_link), method: :delete, class: "btn btn-transparent" do + %span.sr-only disable sharing + = icon("trash") + - else + .settings-message.text-center + There are no groups with access to your project, add one in the form above From 172947f3660ee695e5126a8319fb82cbca1fa360 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 25 Apr 2016 14:55:03 +0100 Subject: [PATCH 10/69] Updated select box to match design --- app/assets/stylesheets/framework/forms.scss | 18 ++++++++++++++++++ app/views/projects/group_links/index.html.haml | 4 +++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/framework/forms.scss b/app/assets/stylesheets/framework/forms.scss index 54cb546111..558b133f59 100644 --- a/app/assets/stylesheets/framework/forms.scss +++ b/app/assets/stylesheets/framework/forms.scss @@ -78,6 +78,24 @@ label { border-radius: 3px; } +.select-wrapper { + position: relative; + + .caret { + position: absolute; + right: 10px; + top: $gl-padding; + color: $gray-darkest; + pointer-events: none; + } +} + +.select-control { + padding-left: 10px; + padding-right: 10px; + -webkit-appearance: none; +} + .form-control-inline { display: inline; } diff --git a/app/views/projects/group_links/index.html.haml b/app/views/projects/group_links/index.html.haml index 627419c1b1..13cdac2102 100644 --- a/app/views/projects/group_links/index.html.haml +++ b/app/views/projects/group_links/index.html.haml @@ -14,7 +14,9 @@ = groups_select_tag(:link_group_id, skip_group: @project.group.try(:path)) .form-group = label_tag :link_group_access, "Max access level", class: "label-light" - = select_tag :link_group_access, options_for_select(ProjectGroupLink.access_options, ProjectGroupLink.default_access), class: "form-control" + .select-wrapper + = select_tag :link_group_access, options_for_select(ProjectGroupLink.access_options, ProjectGroupLink.default_access), class: "form-control select-control" + %span.caret = submit_tag "Share", class: "btn btn-create" .col-sm-12 %hr From 756185533cfef42f85f5ad5cf687768e9ed82bb5 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 25 Apr 2016 16:26:41 +0100 Subject: [PATCH 11/69] Project deploy keys updated UI Closes #13992 --- app/assets/stylesheets/pages/projects.scss | 27 ++++++- .../projects/deploy_keys_controller.rb | 31 ++++---- .../deploy_keys/_deploy_key.html.haml | 45 +++++------- .../projects/deploy_keys/_form.html.haml | 31 ++++---- .../projects/deploy_keys/index.html.haml | 73 +++++++++---------- 5 files changed, 110 insertions(+), 97 deletions(-) diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index fcca9d4faf..9561ac3905 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -202,8 +202,31 @@ min-width: 200px; } -.deploy-project-label { - margin: 1px; +.deploy-key-content { + @media (min-width: $screen-sm-min) { + float: left; + + &:last-child { + float: right; + } + } +} + +.deploy-key-projects { + @media (min-width: $screen-sm-min) { + line-height: 42px; + } +} + +a.deploy-project-label { + padding: 5px; + margin-right: 5px; + color: $gl-gray; + background-color: $row-hover; + + &:hover { + color: $gl-link-color; + } } .vs-public { diff --git a/app/controllers/projects/deploy_keys_controller.rb b/app/controllers/projects/deploy_keys_controller.rb index 7d09288bc8..83d5ced9be 100644 --- a/app/controllers/projects/deploy_keys_controller.rb +++ b/app/controllers/projects/deploy_keys_controller.rb @@ -7,31 +7,24 @@ class Projects::DeployKeysController < Projects::ApplicationController layout "project_settings" def index - @enabled_keys = @project.deploy_keys - - @available_keys = accessible_keys - @enabled_keys - @available_project_keys = current_user.project_deploy_keys - @enabled_keys - @available_public_keys = DeployKey.are_public - @enabled_keys - - # Public keys that are already used by another accessible project are already - # in @available_project_keys. - @available_public_keys -= @available_project_keys + @key = DeployKey.new + set_index_vars end def new - @key = @project.deploy_keys.new - - respond_with(@key) + redirect_to namespace_project_deploy_keys_path(@project.namespace, + @project) end def create @key = DeployKey.new(deploy_key_params) + set_index_vars if @key.valid? && @project.deploy_keys << @key redirect_to namespace_project_deploy_keys_path(@project.namespace, @project) else - render "new" + render "index" end end @@ -51,6 +44,18 @@ class Projects::DeployKeysController < Projects::ApplicationController protected + def set_index_vars + @enabled_keys ||= @project.deploy_keys + + @available_keys ||= accessible_keys - @enabled_keys + @available_project_keys ||= current_user.project_deploy_keys - @enabled_keys + @available_public_keys ||= DeployKey.are_public - @enabled_keys + + # Public keys that are already used by another accessible project are already + # in @available_project_keys. + @available_public_keys -= @available_project_keys + end + def accessible_keys @accessible_keys ||= current_user.accessible_deploy_keys end diff --git a/app/views/projects/deploy_keys/_deploy_key.html.haml b/app/views/projects/deploy_keys/_deploy_key.html.haml index 8d66bae8cd..450aaeb367 100644 --- a/app/views/projects/deploy_keys/_deploy_key.html.haml +++ b/app/views/projects/deploy_keys/_deploy_key.html.haml @@ -1,32 +1,27 @@ %li - .pull-right + .pull-left.append-right-10.hidden-xs + = icon "key", class: "key-icon" + .deploy-key-content.key-list-item-info + %strong.title + = deploy_key.title + .description + = deploy_key.fingerprint + .deploy-key-content.prepend-left-default.deploy-key-projects + - deploy_key.projects.each do |project| + - if can?(current_user, :read_project, project) + = link_to namespace_project_path(project.namespace, project), class: "label deploy-project-label" do + = project.name_with_namespace + .deploy-key-content + %span.key-created-at + created #{time_ago_with_tooltip(deploy_key.created_at)} + .visible-xs-block.visible-sm-block - if @available_keys.include?(deploy_key) - = link_to enable_namespace_project_deploy_key_path(@project.namespace, @project, deploy_key), class: 'btn btn-sm', method: :put do - = icon('plus') + = link_to enable_namespace_project_deploy_key_path(@project.namespace, @project, deploy_key), class: "btn btn-sm prepend-left-10", method: :put do Enable - else - if deploy_key.destroyed_when_orphaned? && deploy_key.almost_orphaned? - = link_to 'Remove', disable_namespace_project_deploy_key_path(@project.namespace, @project, deploy_key), data: { confirm: 'You are going to remove deploy key. Are you sure?'}, method: :put, class: "btn btn-remove delete-key btn-sm pull-right" + = link_to disable_namespace_project_deploy_key_path(@project.namespace, @project, deploy_key), data: { confirm: "You are going to remove deploy key. Are you sure?" }, method: :put, class: "btn btn-warning btn-sm prepend-left-10" do + Remove - else - = link_to disable_namespace_project_deploy_key_path(@project.namespace, @project, deploy_key), class: 'btn btn-sm', method: :put do - = icon('power-off') + = link_to disable_namespace_project_deploy_key_path(@project.namespace, @project, deploy_key), class: "btn btn-warning btn-sm prepend-left-10", method: :put do Disable - - = icon('key') - %strong= deploy_key.title - %br - %code.key-fingerprint= deploy_key.fingerprint - - %p.light.prepend-top-10 - - if deploy_key.public? - %span.label.label-info.deploy-project-label - Public deploy key - - - deploy_key.projects.each do |project| - - if can?(current_user, :read_project, project) - %span.label.label-gray.deploy-project-label - = link_to namespace_project_path(project.namespace, project) do - = project.name_with_namespace - - %small.pull-right - Created #{time_ago_with_tooltip(deploy_key.created_at)} diff --git a/app/views/projects/deploy_keys/_form.html.haml b/app/views/projects/deploy_keys/_form.html.haml index f6565f8583..894c36a96d 100644 --- a/app/views/projects/deploy_keys/_form.html.haml +++ b/app/views/projects/deploy_keys/_form.html.haml @@ -1,18 +1,13 @@ -%div - = form_for [@project.namespace.becomes(Namespace), @project, @key], url: namespace_project_deploy_keys_path, html: { class: 'deploy-key-form form-horizontal js-requires-input' } do |f| - = form_errors(@key) - - .form-group - = f.label :title, class: "control-label" - .col-sm-10= f.text_field :title, class: 'form-control', autofocus: true, required: true - .form-group - = f.label :key, class: "control-label" - .col-sm-10 - %p.light - Paste a machine public key here. Read more about how to generate it - = link_to "here", help_page_path("ssh", "README") - = f.text_area :key, class: "form-control thin_area", rows: 5, required: true - - .form-actions - = f.submit 'Create Deploy Key', class: "btn-create btn" - = link_to "Cancel", namespace_project_deploy_keys_path(@project.namespace, @project), class: "btn btn-cancel" += form_for [@project.namespace.becomes(Namespace), @project, @key], url: namespace_project_deploy_keys_path, html: { class: "js-requires-input" } do |f| + = form_errors(@key) + .form-group + = f.label :title, class: "label-light" + = f.text_field :title, class: 'form-control', autofocus: true, required: true + .form-group + = f.label :key, class: "label-light" + = f.text_area :key, class: "form-control", rows: 5, required: true + .form-group + %p.light.append-bottom-0 + Paste a machine public key here. Read more about how to generate it + = link_to "here", help_page_path("ssh", "README") + = f.submit "Add key", class: "btn-create btn" diff --git a/app/views/projects/deploy_keys/index.html.haml b/app/views/projects/deploy_keys/index.html.haml index 8e24c778b7..d82ef31ab4 100644 --- a/app/views/projects/deploy_keys/index.html.haml +++ b/app/views/projects/deploy_keys/index.html.haml @@ -1,43 +1,38 @@ - page_title "Deploy Keys" -%h3.page-title - Deploy keys allow read-only access to the repository - - = link_to new_namespace_project_deploy_key_path(@project.namespace, @project), class: "btn btn-new pull-right", title: "New Deploy Key" do - %i.fa.fa-plus - New Deploy Key - -%p.light - Deploy keys can be used for CI, staging or production servers. - You can create a deploy key or add an existing one - -%hr.clearfix - -.row - .col-md-6.enabled-keys - %h5 - %strong.cgreen Enabled deploy keys - for this project - %ul.bordered-list - = render @enabled_keys - - if @enabled_keys.blank? - .light-well - .nothing-here-block Create a #{link_to 'new deploy key', new_namespace_project_deploy_key_path(@project.namespace, @project)} or add an existing one - .col-md-6.available-keys - - # If there are available public deploy keys but no available project deploy keys, only public deploy keys are shown. - - if @available_project_keys.any? || @available_public_keys.blank? - %h5 - %strong Deploy keys - from projects you have access to - %ul.bordered-list +.row.prepend-top-default + .col-lg-3.profile-settings-sidebar + %h4.prepend-top-0 + = page_title + %p + Deploy keys allow read-only access to your repository. Deploy keys can be used for CI, staging or production servers. You can create a deploy key or add an existing one. + .col-lg-9 + %h5.prepend-top-0 + Create a new deploy key for this project + = render "form" + .col-lg-9.col-lg-offset-3 + %hr + .col-lg-9.col-lg-offset-3.append-bottom-default + %h5.prepend-top-0 + Enabled deploy keys for this project (#{@enabled_keys.size}) + - if @enabled_keys.any? + %ul.well-list + = render @enabled_keys + - else + .profile-settings-message.text-center + No deploy keys found. Create one with the form above or add existing one below. + .col-lg-9.col-lg-offset-3.append-bottom-default + %h5.prepend-top-0 + Deploy keys from projects you have access to (#{@available_project_keys.size}) + - if @available_project_keys.any? + %ul.well-list = render @available_project_keys - - if @available_project_keys.blank? - .light-well - .nothing-here-block Deploy keys from projects you have access to will be displayed here - - - if @available_public_keys.any? - %h5 - %strong Public deploy keys - available to any project - %ul.bordered-list + - else + .profile-settings-message.text-center + No deploy keys from your projects could be found. Create one with the form above or add existing one below. + - if @available_public_keys.any? + .col-lg-9.col-lg-offset-3.append-bottom-default + %h5.prepend-top-0 + Public deploy keys available to any project (#{@available_public_keys.size}) + %ul.well-list = render @available_public_keys From ca988b2be7a8c1144cb050b517fa4c763f5ca496 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 25 Apr 2016 16:56:17 +0100 Subject: [PATCH 12/69] Fixed failing tests --- app/views/projects/group_links/index.html.haml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/views/projects/group_links/index.html.haml b/app/views/projects/group_links/index.html.haml index 13cdac2102..2b904544f2 100644 --- a/app/views/projects/group_links/index.html.haml +++ b/app/views/projects/group_links/index.html.haml @@ -18,11 +18,9 @@ = select_tag :link_group_access, options_for_select(ProjectGroupLink.access_options, ProjectGroupLink.default_access), class: "form-control select-control" %span.caret = submit_tag "Share", class: "btn btn-create" - .col-sm-12 + .col-lg-9.col-lg-offset-3 %hr - .col-lg-3.settings-sidebar - %h4.prepend-top-0 - .col-lg-9.append-bottom-default + .col-lg-9.col-lg-offset-3.append-bottom-default.enabled-groups %h5.prepend-top-0 Groups you share with (#{@group_links.size}) - if @group_links.present? From 97bd349146bfb3acef77ec413cd0def552d00472 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 25 Apr 2016 11:49:52 +0200 Subject: [PATCH 13/69] Improve Milestones API specs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- CHANGELOG | 2 - spec/requests/api/milestones_spec.rb | 55 ++++++++++++---------------- 2 files changed, 23 insertions(+), 34 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 4ce16b4449..2d1c561fb8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -8,8 +8,6 @@ v 8.7.1 (unreleased) - Fix license detection to detect all license files, not only known licenses. !3878 - Use the `can?` helper instead of `current_user.can?`. !3882 - Prevent users from deleting Webhooks via API they do not own - - Use the `can?` helper instead of `current_user.can?` - - Filter confidential issues from milestones API if user does not have access v 8.7.0 - Gitlab::GitAccess and Gitlab::GitAccessWiki are now instrumented diff --git a/spec/requests/api/milestones_spec.rb b/spec/requests/api/milestones_spec.rb index cb9c3dde5e..241995041b 100644 --- a/spec/requests/api/milestones_spec.rb +++ b/spec/requests/api/milestones_spec.rb @@ -140,43 +140,34 @@ describe API::API, api: true do get api("/projects/#{project.id}/milestones/#{milestone.id}/issues") expect(response.status).to eq(401) end - end - describe 'confidential issues' do - it 'should return confidential issues to team members' do - public_project = create(:project, :public) - user = create(:user) - milestone = create(:milestone, project: public_project) - issue = create(:issue, project: public_project) - confidential_issue = create(:issue, confidential: true, project: public_project) - public_project.team << [user, :developer] - milestone.issues << issue - milestone.issues << confidential_issue + describe 'confidential issues' do + let(:public_project) { create(:project, :public) } + let(:milestone) { create(:milestone, project: public_project) } + let(:issue) { create(:issue, project: public_project) } + let(:confidential_issue) { create(:issue, confidential: true, project: public_project) } + before do + public_project.team << [user, :developer] + milestone.issues << issue << confidential_issue + end - get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", user) + it 'returns confidential issues to team members' do + get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", user) - expect(response.status).to eq(200) - expect(json_response).to be_an Array - expect(json_response.size).to eq(2) - expect(json_response.map { |issue| issue['id'] }).to include(issue.id, confidential_issue.id) - end + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.size).to eq(2) + expect(json_response.map { |issue| issue['id'] }).to include(issue.id, confidential_issue.id) + end - it 'should not return confidential issues to regular users' do - public_project = create(:project, :public) - normal_user = create(:user) - milestone = create(:milestone, project: public_project) - issue = create(:issue, project: public_project) - confidential_issue = create(:issue, confidential: true, project: public_project) - public_project.team << [user, :developer] - milestone.issues << issue - milestone.issues << confidential_issue + it 'does not return confidential issues to regular users' do + get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", create(:user)) - get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", normal_user) - - expect(response.status).to eq(200) - expect(json_response).to be_an Array - expect(json_response.size).to eq(1) - expect(json_response.map { |issue| issue['id'] }).to include(issue.id) + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.size).to eq(1) + expect(json_response.map { |issue| issue['id'] }).to include(issue.id) + end end end end From 54493b9aaef6b2209889a414cf3a26da131eb0ca Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 25 Apr 2016 17:45:05 +0100 Subject: [PATCH 14/69] Updated tests for deploy keys --- app/views/projects/deploy_keys/index.html.haml | 10 ++++------ features/project/deploy_keys.feature | 1 - features/steps/project/deploy_keys.rb | 14 +++++++------- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/app/views/projects/deploy_keys/index.html.haml b/app/views/projects/deploy_keys/index.html.haml index d82ef31ab4..e230834e8b 100644 --- a/app/views/projects/deploy_keys/index.html.haml +++ b/app/views/projects/deploy_keys/index.html.haml @@ -12,7 +12,7 @@ = render "form" .col-lg-9.col-lg-offset-3 %hr - .col-lg-9.col-lg-offset-3.append-bottom-default + .col-lg-9.col-lg-offset-3.append-bottom-default.deploy-keys %h5.prepend-top-0 Enabled deploy keys for this project (#{@enabled_keys.size}) - if @enabled_keys.any? @@ -21,8 +21,7 @@ - else .profile-settings-message.text-center No deploy keys found. Create one with the form above or add existing one below. - .col-lg-9.col-lg-offset-3.append-bottom-default - %h5.prepend-top-0 + %h5.prepend-top-default Deploy keys from projects you have access to (#{@available_project_keys.size}) - if @available_project_keys.any? %ul.well-list @@ -30,9 +29,8 @@ - else .profile-settings-message.text-center No deploy keys from your projects could be found. Create one with the form above or add existing one below. - - if @available_public_keys.any? - .col-lg-9.col-lg-offset-3.append-bottom-default - %h5.prepend-top-0 + - if @available_public_keys.any? + %h5.prepend-top-default Public deploy keys available to any project (#{@available_public_keys.size}) %ul.well-list = render @available_public_keys diff --git a/features/project/deploy_keys.feature b/features/project/deploy_keys.feature index 47cf774094..960b4100ee 100644 --- a/features/project/deploy_keys.feature +++ b/features/project/deploy_keys.feature @@ -21,7 +21,6 @@ Feature: Project Deploy Keys Scenario: I add new deploy key Given I visit project deploy keys page - When I click 'New Deploy Key' And I submit new deploy key Then I should be on deploy keys page And I should see newly created deploy key diff --git a/features/steps/project/deploy_keys.rb b/features/steps/project/deploy_keys.rb index a4d6c9a1b8..83b9ef4839 100644 --- a/features/steps/project/deploy_keys.rb +++ b/features/steps/project/deploy_keys.rb @@ -8,19 +8,19 @@ class Spinach::Features::ProjectDeployKeys < Spinach::FeatureSteps end step 'I should see project deploy key' do - page.within '.enabled-keys' do + page.within '.deploy-keys' do expect(page).to have_content deploy_key.title end end step 'I should see other project deploy key' do - page.within '.available-keys' do + page.within '.deploy-keys' do expect(page).to have_content other_deploy_key.title end end step 'I should see public deploy key' do - page.within '.available-keys' do + page.within '.deploy-keys' do expect(page).to have_content public_deploy_key.title end end @@ -32,7 +32,7 @@ class Spinach::Features::ProjectDeployKeys < Spinach::FeatureSteps step 'I submit new deploy key' do fill_in "deploy_key_title", with: "laptop" fill_in "deploy_key_key", with: "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAzrEJUIR6Y03TCE9rIJ+GqTBvgb8t1jI9h5UBzCLuK4VawOmkLornPqLDrGbm6tcwM/wBrrLvVOqi2HwmkKEIecVO0a64A4rIYScVsXIniHRS6w5twyn1MD3sIbN+socBDcaldECQa2u1dI3tnNVcs8wi77fiRe7RSxePsJceGoheRQgC8AZ510UdIlO+9rjIHUdVN7LLyz512auAfYsgx1OfablkQ/XJcdEwDNgi9imI6nAXhmoKUm1IPLT2yKajTIC64AjLOnE0YyCh6+7RFMpiMyu1qiOCpdjYwTgBRiciNRZCH8xIedyCoAmiUgkUT40XYHwLuwiPJICpkAzp7Q== user@laptop" - click_button "Create" + click_button "Add key" end step 'I should be on deploy keys page' do @@ -40,7 +40,7 @@ class Spinach::Features::ProjectDeployKeys < Spinach::FeatureSteps end step 'I should see newly created deploy key' do - page.within '.enabled-keys' do + page.within '.deploy-keys' do expect(page).to have_content(deploy_key.title) end end @@ -56,7 +56,7 @@ class Spinach::Features::ProjectDeployKeys < Spinach::FeatureSteps end step 'I should only see the same deploy key once' do - page.within '.available-keys' do + page.within '.deploy-keys' do expect(page).to have_selector('ul li', count: 1) end end @@ -66,7 +66,7 @@ class Spinach::Features::ProjectDeployKeys < Spinach::FeatureSteps end step 'I click attach deploy key' do - page.within '.available-keys' do + page.within '.deploy-keys' do click_link 'Enable' end end From d5267dfd0dac8e4cab4919bf8aca611de3a5497b Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 24 Apr 2016 21:45:26 -0700 Subject: [PATCH 15/69] Prevent private snippets in public/internal projects from being leaked via API Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/15580 --- app/finders/snippets_finder.rb | 2 +- lib/api/project_snippets.rb | 15 ++-- spec/requests/api/project_snippets_spec.rb | 87 ++++++++++++++++++++++ spec/requests/api/projects_spec.rb | 2 +- 4 files changed, 99 insertions(+), 7 deletions(-) diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb index a41172816b..01cbf91c65 100644 --- a/app/finders/snippets_finder.rb +++ b/app/finders/snippets_finder.rb @@ -51,7 +51,7 @@ class SnippetsFinder snippets = project.snippets.fresh if current_user - if project.team.member?(current_user.id) + if project.team.member?(current_user.id) || current_user.admin? snippets else snippets.public_and_internal diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb index 22ce3c6a06..ce1bf0d26d 100644 --- a/lib/api/project_snippets.rb +++ b/lib/api/project_snippets.rb @@ -11,6 +11,11 @@ module API end not_found! end + + def snippets_for_current_user + finder_params = { filter: :by_project, project: user_project } + SnippetsFinder.new.execute(current_user, finder_params) + end end # Get a project snippets @@ -20,7 +25,7 @@ module API # Example Request: # GET /projects/:id/snippets get ":id/snippets" do - present paginate(user_project.snippets), with: Entities::ProjectSnippet + present paginate(snippets_for_current_user), with: Entities::ProjectSnippet end # Get a project snippet @@ -31,7 +36,7 @@ module API # Example Request: # GET /projects/:id/snippets/:snippet_id get ":id/snippets/:snippet_id" do - @snippet = user_project.snippets.find(params[:snippet_id]) + @snippet = snippets_for_current_user.find(params[:snippet_id]) present @snippet, with: Entities::ProjectSnippet end @@ -73,7 +78,7 @@ module API # Example Request: # PUT /projects/:id/snippets/:snippet_id put ":id/snippets/:snippet_id" do - @snippet = user_project.snippets.find(params[:snippet_id]) + @snippet = snippets_for_current_user.find(params[:snippet_id]) authorize! :update_project_snippet, @snippet attrs = attributes_for_keys [:title, :file_name, :visibility_level] @@ -97,7 +102,7 @@ module API # DELETE /projects/:id/snippets/:snippet_id delete ":id/snippets/:snippet_id" do begin - @snippet = user_project.snippets.find(params[:snippet_id]) + @snippet = snippets_for_current_user.find(params[:snippet_id]) authorize! :update_project_snippet, @snippet @snippet.destroy rescue @@ -113,7 +118,7 @@ module API # Example Request: # GET /projects/:id/snippets/:snippet_id/raw get ":id/snippets/:snippet_id/raw" do - @snippet = user_project.snippets.find(params[:snippet_id]) + @snippet = snippets_for_current_user.find(params[:snippet_id]) env['api.format'] = :txt content_type 'text/plain' diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 3722ddf5a3..9706d060cf 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -15,4 +15,91 @@ describe API::API, api: true do expect(json_response['expires_at']).to be_nil end end + + describe 'GET /projects/:project_id/snippets/' do + it 'all snippets available to team member' do + project = create(:project, :public) + user = create(:user) + project.team << [user, :developer] + public_snippet = create(:project_snippet, :public, project: project) + internal_snippet = create(:project_snippet, :internal, project: project) + private_snippet = create(:project_snippet, :private, project: project) + + get api("/projects/#{project.id}/snippets/", user) + + expect(response.status).to eq(200) + expect(json_response.size).to eq(3) + expect(json_response.map{ |snippet| snippet['id']} ).to include(public_snippet.id, internal_snippet.id, private_snippet.id) + end + + it 'hides private snippets from regular user' do + project = create(:project, :public) + user = create(:user) + create(:project_snippet, :private, project: project) + + get api("/projects/#{project.id}/snippets/", user) + expect(response.status).to eq(200) + expect(json_response.size).to eq(0) + end + end + + describe 'POST /projects/:project_id/snippets/' do + it 'creates a new snippet' do + admin = create(:admin) + project = create(:project) + params = { + title: 'Test Title', + file_name: 'test.rb', + code: 'puts "hello world"', + visibility_level: Gitlab::VisibilityLevel::PUBLIC + } + + post api("/projects/#{project.id}/snippets/", admin), params + + expect(response.status).to eq(201) + snippet = ProjectSnippet.find(json_response['id']) + expect(snippet.content).to eq(params[:code]) + expect(snippet.title).to eq(params[:title]) + expect(snippet.file_name).to eq(params[:file_name]) + expect(snippet.visibility_level).to eq(params[:visibility_level]) + end + end + + describe 'PUT /projects/:project_id/snippets/:id/' do + it 'updates snippet' do + admin = create(:admin) + snippet = create(:project_snippet, author: admin) + new_content = 'New content' + + put api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/", admin), code: new_content + + expect(response.status).to eq(200) + snippet.reload + expect(snippet.content).to eq(new_content) + end + end + + describe 'DELETE /projects/:project_id/snippets/:id/' do + it 'deletes snippet' do + admin = create(:admin) + snippet = create(:project_snippet, author: admin) + + delete api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/", admin) + + expect(response.status).to eq(200) + end + end + + describe 'GET /projects/:project_id/snippets/:id/raw' do + it 'returns raw text' do + admin = create(:admin) + snippet = create(:project_snippet, author: admin) + + get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/raw", admin) + + expect(response.status).to eq(200) + expect(response.content_type).to eq 'text/plain' + expect(response.body).to eq(snippet.content) + end + end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index fccd08bd6d..66193eac05 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -11,7 +11,7 @@ describe API::API, api: true do let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } let(:project2) { create(:project, path: 'project2', creator_id: user.id, namespace: user.namespace) } let(:project3) { create(:project, path: 'project3', creator_id: user.id, namespace: user.namespace) } - let(:snippet) { create(:project_snippet, author: user, project: project, title: 'example') } + let(:snippet) { create(:project_snippet, :public, author: user, project: project, title: 'example') } let(:project_member) { create(:project_member, :master, user: user, project: project) } let(:project_member2) { create(:project_member, :developer, user: user3, project: project) } let(:user4) { create(:user) } From ef340f6e777875e1bbb38752e64ba7bea3ab2f31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 21 Apr 2016 17:13:14 +0200 Subject: [PATCH 16/69] Ensure URL in all Service subclasses are valid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/helpers/issues_helper.rb | 48 +++++--- .../project_services/buildkite_service.rb | 4 +- .../project_services/issue_tracker_service.rb | 2 +- app/models/project_services/jira_service.rb | 2 + app/models/project_services/slack_service.rb | 2 +- spec/helpers/issues_helper_spec.rb | 36 ++++++ .../project_services/bamboo_service_spec.rb | 109 ++++++------------ .../buildkite_service_spec.rb | 17 +++ .../builds_email_service_spec.rb | 81 ++++++------- .../project_services/campfire_service_spec.rb | 42 +++++++ .../custom_issue_tracker_service_spec.rb | 49 ++++++++ .../project_services/drone_ci_service_spec.rb | 13 +-- .../emails_on_push_service_spec.rb | 17 +++ .../external_wiki_service_spec.rb | 17 ++- .../project_services/flowdock_service_spec.rb | 14 +++ .../gemnasium_service_spec.rb | 16 +++ .../gitlab_issue_tracker_service_spec.rb | 14 +++ .../project_services/hipchat_service_spec.rb | 14 +++ .../project_services/irker_service_spec.rb | 14 ++- .../project_services/jira_service_spec.rb | 26 ++++- .../pivotaltracker_service_spec.rb | 42 +++++++ .../project_services/pushover_service_spec.rb | 20 ++-- .../project_services/redmine_service_spec.rb | 49 ++++++++ .../project_services/slack_service_spec.rb | 17 ++- .../project_services/teamcity_service_spec.rb | 109 ++++++------------ .../issue_tracker_service_shared_example.rb | 7 ++ 26 files changed, 539 insertions(+), 242 deletions(-) create mode 100644 spec/models/project_services/campfire_service_spec.rb create mode 100644 spec/models/project_services/custom_issue_tracker_service_spec.rb create mode 100644 spec/models/project_services/emails_on_push_service_spec.rb rename spec/models/{ => project_services}/external_wiki_service_spec.rb (78%) create mode 100644 spec/models/project_services/pivotaltracker_service_spec.rb create mode 100644 spec/models/project_services/redmine_service_spec.rb create mode 100644 spec/support/issue_tracker_service_shared_example.rb diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index afe1e11a0d..198d39455d 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -16,31 +16,49 @@ module IssuesHelper def url_for_project_issues(project = @project, options = {}) return '' if project.nil? - if options[:only_path] - project.issues_tracker.project_path - else - project.issues_tracker.project_url - end + url = + if options[:only_path] + project.issues_tracker.project_path + else + project.issues_tracker.project_url + end + + # Ensure we return a valid URL to prevent possible XSS. + URI.parse(url).to_s + rescue URI::InvalidURIError + '' end def url_for_new_issue(project = @project, options = {}) return '' if project.nil? - if options[:only_path] - project.issues_tracker.new_issue_path - else - project.issues_tracker.new_issue_url - end + url = + if options[:only_path] + project.issues_tracker.new_issue_path + else + project.issues_tracker.new_issue_url + end + + # Ensure we return a valid URL to prevent possible XSS. + URI.parse(url).to_s + rescue URI::InvalidURIError + '' end def url_for_issue(issue_iid, project = @project, options = {}) return '' if project.nil? - if options[:only_path] - project.issues_tracker.issue_path(issue_iid) - else - project.issues_tracker.issue_url(issue_iid) - end + url = + if options[:only_path] + project.issues_tracker.issue_path(issue_iid) + else + project.issues_tracker.issue_url(issue_iid) + end + + # Ensure we return a valid URL to prevent possible XSS. + URI.parse(url).to_s + rescue URI::InvalidURIError + '' end def bulk_update_milestone_options diff --git a/app/models/project_services/buildkite_service.rb b/app/models/project_services/buildkite_service.rb index 3efbfd2eec..861cc974ec 100644 --- a/app/models/project_services/buildkite_service.rb +++ b/app/models/project_services/buildkite_service.rb @@ -26,7 +26,7 @@ class BuildkiteService < CiService prop_accessor :project_url, :token, :enable_ssl_verification - validates :project_url, presence: true, if: :activated? + validates :project_url, presence: true, url: true, if: :activated? validates :token, presence: true, if: :activated? after_save :compose_service_hook, if: :activated? @@ -91,7 +91,7 @@ class BuildkiteService < CiService { type: 'text', name: 'project_url', placeholder: "#{ENDPOINT}/example/project" }, - + { type: 'checkbox', name: 'enable_ssl_verification', title: "Enable SSL verification" } diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index 25045224ce..c5501e0641 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -21,7 +21,7 @@ class IssueTrackerService < Service - validates :project_url, :issues_url, :new_issue_url, presence: true, if: :activated? + validates :project_url, :issues_url, :new_issue_url, presence: true, url: true, if: :activated? default_value_for :category, 'issue_tracker' diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index 1ed42c4f3e..b4418ba928 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -28,6 +28,8 @@ class JiraService < IssueTrackerService prop_accessor :username, :password, :api_url, :jira_issue_transition_id, :title, :description, :project_url, :issues_url, :new_issue_url + validates :api_url, presence: true, url: true, if: :activated? + before_validation :set_api_url, :set_jira_issue_transition_id before_update :reset_password diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index fd65027f08..7092b75754 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -22,7 +22,7 @@ class SlackService < Service prop_accessor :webhook, :username, :channel boolean_accessor :notify_only_broken_builds - validates :webhook, presence: true, if: :activated? + validates :webhook, presence: true, url: true, if: :activated? def initialize_properties if properties.nil? diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index 543593cf38..bffe2c18b6 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -30,6 +30,18 @@ describe IssuesHelper do expect(url_for_project_issues).to eq "" end + it 'returns an empty string if project_url is invalid' do + expect(project).to receive_message_chain('issues_tracker.project_url') { 'javascript:alert("foo");' } + + expect(url_for_project_issues(project)).to eq '' + end + + it 'returns an empty string if project_path is invalid' do + expect(project).to receive_message_chain('issues_tracker.project_path') { 'javascript:alert("foo");' } + + expect(url_for_project_issues(project, only_path: true)).to eq '' + end + describe "when external tracker was enabled and then config removed" do before do @project = ext_project @@ -68,6 +80,18 @@ describe IssuesHelper do expect(url_for_issue(issue.iid)).to eq "" end + it 'returns an empty string if issue_url is invalid' do + expect(project).to receive_message_chain('issues_tracker.issue_url') { 'javascript:alert("foo");' } + + expect(url_for_issue(issue.iid, project)).to eq '' + end + + it 'returns an empty string if issue_path is invalid' do + expect(project).to receive_message_chain('issues_tracker.issue_path') { 'javascript:alert("foo");' } + + expect(url_for_issue(issue.iid, project, only_path: true)).to eq '' + end + describe "when external tracker was enabled and then config removed" do before do @project = ext_project @@ -105,6 +129,18 @@ describe IssuesHelper do expect(url_for_new_issue).to eq "" end + it 'returns an empty string if issue_url is invalid' do + expect(project).to receive_message_chain('issues_tracker.new_issue_url') { 'javascript:alert("foo");' } + + expect(url_for_new_issue(project)).to eq '' + end + + it 'returns an empty string if issue_path is invalid' do + expect(project).to receive_message_chain('issues_tracker.new_issue_path') { 'javascript:alert("foo");' } + + expect(url_for_new_issue(project, only_path: true)).to eq '' + end + describe "when external tracker was enabled and then config removed" do before do @project = ext_project diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb index 31b2c90122..e771f35811 100644 --- a/spec/models/project_services/bamboo_service_spec.rb +++ b/spec/models/project_services/bamboo_service_spec.rb @@ -27,86 +27,51 @@ describe BambooService, models: true do end describe 'Validations' do - describe '#bamboo_url' do - it 'does not validate the presence of bamboo_url if service is not active' do - bamboo_service = service - bamboo_service.active = false + subject { service } - expect(bamboo_service).not_to validate_presence_of(:bamboo_url) + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:build_key) } + it { is_expected.to validate_presence_of(:bamboo_url) } + it_behaves_like 'issue tracker service URL attribute', :bamboo_url + + describe '#username' do + it 'does not validate the presence of username if password is nil' do + subject.password = nil + + expect(subject).not_to validate_presence_of(:username) + end + + it 'validates the presence of username if password is present' do + subject.password = 'secret' + + expect(subject).to validate_presence_of(:username) + end end - it 'validates the presence of bamboo_url if service is active' do - bamboo_service = service - bamboo_service.active = true + describe '#password' do + it 'does not validate the presence of password if username is nil' do + subject.username = nil - expect(bamboo_service).to validate_presence_of(:bamboo_url) + expect(subject).not_to validate_presence_of(:password) + end + + it 'validates the presence of password if username is present' do + subject.username = 'john' + + expect(subject).to validate_presence_of(:password) + end end end - describe '#build_key' do - it 'does not validate the presence of build_key if service is not active' do - bamboo_service = service - bamboo_service.active = false + context 'when service is inactive' do + before { subject.active = false } - expect(bamboo_service).not_to validate_presence_of(:build_key) - end - - it 'validates the presence of build_key if service is active' do - bamboo_service = service - bamboo_service.active = true - - expect(bamboo_service).to validate_presence_of(:build_key) - end - end - - describe '#username' do - it 'does not validate the presence of username if service is not active' do - bamboo_service = service - bamboo_service.active = false - - expect(bamboo_service).not_to validate_presence_of(:username) - end - - it 'does not validate the presence of username if username is nil' do - bamboo_service = service - bamboo_service.active = true - bamboo_service.password = nil - - expect(bamboo_service).not_to validate_presence_of(:username) - end - - it 'validates the presence of username if service is active and username is present' do - bamboo_service = service - bamboo_service.active = true - bamboo_service.password = 'secret' - - expect(bamboo_service).to validate_presence_of(:username) - end - end - - describe '#password' do - it 'does not validate the presence of password if service is not active' do - bamboo_service = service - bamboo_service.active = false - - expect(bamboo_service).not_to validate_presence_of(:password) - end - - it 'does not validate the presence of password if username is nil' do - bamboo_service = service - bamboo_service.active = true - bamboo_service.username = nil - - expect(bamboo_service).not_to validate_presence_of(:password) - end - - it 'validates the presence of password if service is active and username is present' do - bamboo_service = service - bamboo_service.active = true - bamboo_service.username = 'john' - - expect(bamboo_service).to validate_presence_of(:password) - end + it { is_expected.not_to validate_presence_of(:build_key) } + it { is_expected.not_to validate_presence_of(:bamboo_url) } + it { is_expected.not_to validate_presence_of(:username) } + it { is_expected.not_to validate_presence_of(:password) } end end diff --git a/spec/models/project_services/buildkite_service_spec.rb b/spec/models/project_services/buildkite_service_spec.rb index 88cd624877..60364df201 100644 --- a/spec/models/project_services/buildkite_service_spec.rb +++ b/spec/models/project_services/buildkite_service_spec.rb @@ -26,6 +26,23 @@ describe BuildkiteService, models: true do it { is_expected.to have_one :service_hook } end + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:project_url) } + it { is_expected.to validate_presence_of(:token) } + it_behaves_like 'issue tracker service URL attribute', :project_url + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:project_url) } + it { is_expected.not_to validate_presence_of(:token) } + end + end + describe 'commits methods' do before do @project = Project.new diff --git a/spec/models/project_services/builds_email_service_spec.rb b/spec/models/project_services/builds_email_service_spec.rb index 7c23c2efcc..236df8f047 100644 --- a/spec/models/project_services/builds_email_service_spec.rb +++ b/spec/models/project_services/builds_email_service_spec.rb @@ -1,76 +1,71 @@ require 'spec_helper' describe BuildsEmailService do - let(:build) { create(:ci_build) } - let(:data) { Gitlab::BuildDataBuilder.build(build) } - let!(:project) { create(:project, :public, ci_id: 1) } - let(:service) { described_class.new(project: project, active: true) } + let(:data) { Gitlab::BuildDataBuilder.build(create(:ci_build)) } + + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:recipients) } + + context 'when pusher is added' do + before { subject.add_pusher = true } + + it { is_expected.not_to validate_presence_of(:recipients) } + end + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:recipients) } + end + end describe '#execute' do it 'sends email' do - service.recipients = 'test@gitlab.com' + subject.recipients = 'test@gitlab.com' data[:build_status] = 'failed' + expect(BuildEmailWorker).to receive(:perform_async) - service.execute(data) + + subject.execute(data) end it 'does not send email with succeeded build and notify_only_broken_builds on' do - expect(service).to receive(:notify_only_broken_builds).and_return(true) + expect(subject).to receive(:notify_only_broken_builds).and_return(true) data[:build_status] = 'success' + expect(BuildEmailWorker).not_to receive(:perform_async) - service.execute(data) + + subject.execute(data) end it 'does not send email with failed build and build_allow_failure is true' do data[:build_status] = 'failed' data[:build_allow_failure] = true + expect(BuildEmailWorker).not_to receive(:perform_async) - service.execute(data) + + subject.execute(data) end it 'does not send email with unknown build status' do data[:build_status] = 'foo' + expect(BuildEmailWorker).not_to receive(:perform_async) - service.execute(data) + + subject.execute(data) end it 'does not send email when recipients list is empty' do - service.recipients = ' ,, ' + subject.recipients = ' ,, ' data[:build_status] = 'failed' + expect(BuildEmailWorker).not_to receive(:perform_async) - service.execute(data) - end - end - describe 'validations' do - - context 'when pusher is not added' do - before { service.add_pusher = false } - - it 'does not allow empty recipient input' do - service.recipients = '' - expect(service.valid?).to be false - end - - it 'does allow non-empty recipient input' do - service.recipients = 'test@example.com' - expect(service.valid?).to be true - end - - end - - context 'when pusher is added' do - before { service.add_pusher = true } - - it 'does allow empty recipient input' do - service.recipients = '' - expect(service.valid?).to be true - end - - it 'does allow non-empty recipient input' do - service.recipients = 'test@example.com' - expect(service.valid?).to be true - end + subject.execute(data) end end end diff --git a/spec/models/project_services/campfire_service_spec.rb b/spec/models/project_services/campfire_service_spec.rb new file mode 100644 index 0000000000..3e6da42803 --- /dev/null +++ b/spec/models/project_services/campfire_service_spec.rb @@ -0,0 +1,42 @@ +# == Schema Information +# +# Table name: services +# +# id :integer not null, primary key +# type :string(255) +# title :string(255) +# project_id :integer +# created_at :datetime +# updated_at :datetime +# active :boolean default(FALSE), not null +# properties :text +# template :boolean default(FALSE) +# push_events :boolean default(TRUE) +# issues_events :boolean default(TRUE) +# merge_requests_events :boolean default(TRUE) +# tag_push_events :boolean default(TRUE) +# note_events :boolean default(TRUE), not null +# + +require 'spec_helper' + +describe CampfireService, models: true do + describe 'Associations' do + it { is_expected.to belong_to :project } + it { is_expected.to have_one :service_hook } + end + + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:token) } + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:token) } + end + end +end diff --git a/spec/models/project_services/custom_issue_tracker_service_spec.rb b/spec/models/project_services/custom_issue_tracker_service_spec.rb new file mode 100644 index 0000000000..ff976f8ec5 --- /dev/null +++ b/spec/models/project_services/custom_issue_tracker_service_spec.rb @@ -0,0 +1,49 @@ +# == Schema Information +# +# Table name: services +# +# id :integer not null, primary key +# type :string(255) +# title :string(255) +# project_id :integer +# created_at :datetime +# updated_at :datetime +# active :boolean default(FALSE), not null +# properties :text +# template :boolean default(FALSE) +# push_events :boolean default(TRUE) +# issues_events :boolean default(TRUE) +# merge_requests_events :boolean default(TRUE) +# tag_push_events :boolean default(TRUE) +# note_events :boolean default(TRUE), not null +# + +require 'spec_helper' + +describe CustomIssueTrackerService, models: true do + describe 'Associations' do + it { is_expected.to belong_to :project } + it { is_expected.to have_one :service_hook } + end + + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:project_url) } + it { is_expected.to validate_presence_of(:issues_url) } + it { is_expected.to validate_presence_of(:new_issue_url) } + it_behaves_like 'issue tracker service URL attribute', :project_url + it_behaves_like 'issue tracker service URL attribute', :issues_url + it_behaves_like 'issue tracker service URL attribute', :new_issue_url + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:project_url) } + it { is_expected.not_to validate_presence_of(:issues_url) } + it { is_expected.not_to validate_presence_of(:new_issue_url) } + end + end +end diff --git a/spec/models/project_services/drone_ci_service_spec.rb b/spec/models/project_services/drone_ci_service_spec.rb index a2cf68a9e3..3a8e67438f 100644 --- a/spec/models/project_services/drone_ci_service_spec.rb +++ b/spec/models/project_services/drone_ci_service_spec.rb @@ -28,25 +28,18 @@ describe DroneCiService, models: true do describe 'validations' do context 'active' do - before { allow(subject).to receive(:activated?).and_return(true) } + before { subject.active = true } it { is_expected.to validate_presence_of(:token) } it { is_expected.to validate_presence_of(:drone_url) } - it { is_expected.to allow_value('ewf9843kdnfdfs89234n').for(:token) } - it { is_expected.to allow_value('http://ci.example.com').for(:drone_url) } - it { is_expected.not_to allow_value('this is not url').for(:drone_url) } - it { is_expected.not_to allow_value('http//noturl').for(:drone_url) } - it { is_expected.not_to allow_value('ftp://ci.example.com').for(:drone_url) } + it_behaves_like 'issue tracker service URL attribute', :drone_url end context 'inactive' do - before { allow(subject).to receive(:activated?).and_return(false) } + before { subject.active = false } it { is_expected.not_to validate_presence_of(:token) } it { is_expected.not_to validate_presence_of(:drone_url) } - it { is_expected.to allow_value('ewf9843kdnfdfs89234n').for(:token) } - it { is_expected.to allow_value('http://drone.example.com').for(:drone_url) } - it { is_expected.to allow_value('ftp://drone.example.com').for(:drone_url) } end end diff --git a/spec/models/project_services/emails_on_push_service_spec.rb b/spec/models/project_services/emails_on_push_service_spec.rb new file mode 100644 index 0000000000..e6f78898c8 --- /dev/null +++ b/spec/models/project_services/emails_on_push_service_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' + +describe EmailsOnPushService do + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:recipients) } + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:recipients) } + end + end +end diff --git a/spec/models/external_wiki_service_spec.rb b/spec/models/project_services/external_wiki_service_spec.rb similarity index 78% rename from spec/models/external_wiki_service_spec.rb rename to spec/models/project_services/external_wiki_service_spec.rb index d37978720b..5fe5ea7d2d 100644 --- a/spec/models/external_wiki_service_spec.rb +++ b/spec/models/project_services/external_wiki_service_spec.rb @@ -28,13 +28,18 @@ describe ExternalWikiService, models: true do it { should have_one :service_hook } end - describe "Validations" do - context "active" do - before do - subject.active = true - end + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } - it { should validate_presence_of :external_wiki_url } + it { is_expected.to validate_presence_of(:external_wiki_url) } + it_behaves_like 'issue tracker service URL attribute', :external_wiki_url + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:external_wiki_url) } end end diff --git a/spec/models/project_services/flowdock_service_spec.rb b/spec/models/project_services/flowdock_service_spec.rb index ff7fbcaa00..b7e627e651 100644 --- a/spec/models/project_services/flowdock_service_spec.rb +++ b/spec/models/project_services/flowdock_service_spec.rb @@ -26,6 +26,20 @@ describe FlowdockService, models: true do it { is_expected.to have_one :service_hook } end + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:token) } + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:token) } + end + end + describe "Execute" do let(:user) { create(:user) } let(:project) { create(:project) } diff --git a/spec/models/project_services/gemnasium_service_spec.rb b/spec/models/project_services/gemnasium_service_spec.rb index ecb3ccb167..a08f1ac229 100644 --- a/spec/models/project_services/gemnasium_service_spec.rb +++ b/spec/models/project_services/gemnasium_service_spec.rb @@ -26,6 +26,22 @@ describe GemnasiumService, models: true do it { is_expected.to have_one :service_hook } end + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:token) } + it { is_expected.to validate_presence_of(:api_key) } + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:token) } + it { is_expected.not_to validate_presence_of(:api_key) } + end + end + describe "Execute" do let(:user) { create(:user) } let(:project) { create(:project) } diff --git a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb index 3518dbd172..7a1f106d6e 100644 --- a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb +++ b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb @@ -26,6 +26,20 @@ describe GitlabIssueTrackerService, models: true do it { is_expected.to have_one :service_hook } end + describe 'Validations' do + context 'when service is active' do + subject { described_class.new(project: create(:project), active: true) } + + it { is_expected.to validate_presence_of(:issues_url) } + it_behaves_like 'issue tracker service URL attribute', :issues_url + end + + context 'when service is inactive' do + subject { described_class.new(project: create(:project), active: false) } + + it { is_expected.not_to validate_presence_of(:issues_url) } + end + end describe 'project and issue urls' do let(:project) { create(:project) } diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index d878162a22..6fb5cad501 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -26,6 +26,20 @@ describe HipchatService, models: true do it { is_expected.to have_one :service_hook } end + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:token) } + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:token) } + end + end + describe "Execute" do let(:hipchat) { HipchatService.new } let(:user) { create(:user, username: 'username') } diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb index b783b1a576..4ee022a517 100644 --- a/spec/models/project_services/irker_service_spec.rb +++ b/spec/models/project_services/irker_service_spec.rb @@ -29,14 +29,16 @@ describe IrkerService, models: true do end describe 'Validations' do - before do - subject.active = true - subject.properties['recipients'] = _recipients + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:recipients) } end - context 'active' do - let(:_recipients) { nil } - it { should validate_presence_of :recipients } + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:recipients) } end end diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 2f8193170a..5309cfb99f 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -26,6 +26,30 @@ describe JiraService, models: true do it { is_expected.to have_one :service_hook } end + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:api_url) } + it { is_expected.to validate_presence_of(:project_url) } + it { is_expected.to validate_presence_of(:issues_url) } + it { is_expected.to validate_presence_of(:new_issue_url) } + it_behaves_like 'issue tracker service URL attribute', :api_url + it_behaves_like 'issue tracker service URL attribute', :project_url + it_behaves_like 'issue tracker service URL attribute', :issues_url + it_behaves_like 'issue tracker service URL attribute', :new_issue_url + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:api_url) } + it { is_expected.not_to validate_presence_of(:project_url) } + it { is_expected.not_to validate_presence_of(:issues_url) } + it { is_expected.not_to validate_presence_of(:new_issue_url) } + end + end + describe "Execute" do let(:user) { create(:user) } let(:project) { create(:project) } @@ -72,7 +96,7 @@ describe JiraService, models: true do context "when a password was previously set" do before do - @jira_service = JiraService.create( + @jira_service = JiraService.create!( project: create(:project), properties: { api_url: 'http://jira.example.com/rest/api/2', diff --git a/spec/models/project_services/pivotaltracker_service_spec.rb b/spec/models/project_services/pivotaltracker_service_spec.rb new file mode 100644 index 0000000000..f37edd4d97 --- /dev/null +++ b/spec/models/project_services/pivotaltracker_service_spec.rb @@ -0,0 +1,42 @@ +# == Schema Information +# +# Table name: services +# +# id :integer not null, primary key +# type :string(255) +# title :string(255) +# project_id :integer +# created_at :datetime +# updated_at :datetime +# active :boolean default(FALSE), not null +# properties :text +# template :boolean default(FALSE) +# push_events :boolean default(TRUE) +# issues_events :boolean default(TRUE) +# merge_requests_events :boolean default(TRUE) +# tag_push_events :boolean default(TRUE) +# note_events :boolean default(TRUE), not null +# + +require 'spec_helper' + +describe PivotaltrackerService, models: true do + describe 'Associations' do + it { is_expected.to belong_to :project } + it { is_expected.to have_one :service_hook } + end + + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:token) } + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:token) } + end + end +end diff --git a/spec/models/project_services/pushover_service_spec.rb b/spec/models/project_services/pushover_service_spec.rb index 96039f9491..555d9757b4 100644 --- a/spec/models/project_services/pushover_service_spec.rb +++ b/spec/models/project_services/pushover_service_spec.rb @@ -27,14 +27,20 @@ describe PushoverService, models: true do end describe 'Validations' do - context 'active' do - before do - subject.active = true - end + context 'when service is active' do + before { subject.active = true } - it { is_expected.to validate_presence_of :api_key } - it { is_expected.to validate_presence_of :user_key } - it { is_expected.to validate_presence_of :priority } + it { is_expected.to validate_presence_of(:api_key) } + it { is_expected.to validate_presence_of(:user_key) } + it { is_expected.to validate_presence_of(:priority) } + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:api_key) } + it { is_expected.not_to validate_presence_of(:user_key) } + it { is_expected.not_to validate_presence_of(:priority) } end end diff --git a/spec/models/project_services/redmine_service_spec.rb b/spec/models/project_services/redmine_service_spec.rb new file mode 100644 index 0000000000..7d14f6e828 --- /dev/null +++ b/spec/models/project_services/redmine_service_spec.rb @@ -0,0 +1,49 @@ +# == Schema Information +# +# Table name: services +# +# id :integer not null, primary key +# type :string(255) +# title :string(255) +# project_id :integer +# created_at :datetime +# updated_at :datetime +# active :boolean default(FALSE), not null +# properties :text +# template :boolean default(FALSE) +# push_events :boolean default(TRUE) +# issues_events :boolean default(TRUE) +# merge_requests_events :boolean default(TRUE) +# tag_push_events :boolean default(TRUE) +# note_events :boolean default(TRUE), not null +# + +require 'spec_helper' + +describe RedmineService, models: true do + describe 'Associations' do + it { is_expected.to belong_to :project } + it { is_expected.to have_one :service_hook } + end + + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:project_url) } + it { is_expected.to validate_presence_of(:issues_url) } + it { is_expected.to validate_presence_of(:new_issue_url) } + it_behaves_like 'issue tracker service URL attribute', :project_url + it_behaves_like 'issue tracker service URL attribute', :issues_url + it_behaves_like 'issue tracker service URL attribute', :new_issue_url + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:project_url) } + it { is_expected.not_to validate_presence_of(:issues_url) } + it { is_expected.not_to validate_presence_of(:new_issue_url) } + end + end +end diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb index 478d59be08..a97b756013 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/project_services/slack_service_spec.rb @@ -26,13 +26,18 @@ describe SlackService, models: true do it { is_expected.to have_one :service_hook } end - describe "Validations" do - context "active" do - before do - subject.active = true - end + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } - it { is_expected.to validate_presence_of :webhook } + it { is_expected.to validate_presence_of(:webhook) } + it_behaves_like 'issue tracker service URL attribute', :webhook + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:webhook) } end end diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb index bc7423cee6..ad24b89517 100644 --- a/spec/models/project_services/teamcity_service_spec.rb +++ b/spec/models/project_services/teamcity_service_spec.rb @@ -27,86 +27,51 @@ describe TeamcityService, models: true do end describe 'Validations' do - describe '#teamcity_url' do - it 'does not validate the presence of teamcity_url if service is not active' do - teamcity_service = service - teamcity_service.active = false + subject { service } - expect(teamcity_service).not_to validate_presence_of(:teamcity_url) + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:build_type) } + it { is_expected.to validate_presence_of(:teamcity_url) } + it_behaves_like 'issue tracker service URL attribute', :teamcity_url + + describe '#username' do + it 'does not validate the presence of username if password is nil' do + subject.password = nil + + expect(subject).not_to validate_presence_of(:username) + end + + it 'validates the presence of username if password is present' do + subject.password = 'secret' + + expect(subject).to validate_presence_of(:username) + end end - it 'validates the presence of teamcity_url if service is active' do - teamcity_service = service - teamcity_service.active = true + describe '#password' do + it 'does not validate the presence of password if username is nil' do + subject.username = nil - expect(teamcity_service).to validate_presence_of(:teamcity_url) + expect(subject).not_to validate_presence_of(:password) + end + + it 'validates the presence of password if username is present' do + subject.username = 'john' + + expect(subject).to validate_presence_of(:password) + end end end - describe '#build_type' do - it 'does not validate the presence of build_type if service is not active' do - teamcity_service = service - teamcity_service.active = false + context 'when service is inactive' do + before { subject.active = false } - expect(teamcity_service).not_to validate_presence_of(:build_type) - end - - it 'validates the presence of build_type if service is active' do - teamcity_service = service - teamcity_service.active = true - - expect(teamcity_service).to validate_presence_of(:build_type) - end - end - - describe '#username' do - it 'does not validate the presence of username if service is not active' do - teamcity_service = service - teamcity_service.active = false - - expect(teamcity_service).not_to validate_presence_of(:username) - end - - it 'does not validate the presence of username if username is nil' do - teamcity_service = service - teamcity_service.active = true - teamcity_service.password = nil - - expect(teamcity_service).not_to validate_presence_of(:username) - end - - it 'validates the presence of username if service is active and username is present' do - teamcity_service = service - teamcity_service.active = true - teamcity_service.password = 'secret' - - expect(teamcity_service).to validate_presence_of(:username) - end - end - - describe '#password' do - it 'does not validate the presence of password if service is not active' do - teamcity_service = service - teamcity_service.active = false - - expect(teamcity_service).not_to validate_presence_of(:password) - end - - it 'does not validate the presence of password if username is nil' do - teamcity_service = service - teamcity_service.active = true - teamcity_service.username = nil - - expect(teamcity_service).not_to validate_presence_of(:password) - end - - it 'validates the presence of password if service is active and username is present' do - teamcity_service = service - teamcity_service.active = true - teamcity_service.username = 'john' - - expect(teamcity_service).to validate_presence_of(:password) - end + it { is_expected.not_to validate_presence_of(:build_type) } + it { is_expected.not_to validate_presence_of(:teamcity_url) } + it { is_expected.not_to validate_presence_of(:username) } + it { is_expected.not_to validate_presence_of(:password) } end end diff --git a/spec/support/issue_tracker_service_shared_example.rb b/spec/support/issue_tracker_service_shared_example.rb new file mode 100644 index 0000000000..b6d7436c36 --- /dev/null +++ b/spec/support/issue_tracker_service_shared_example.rb @@ -0,0 +1,7 @@ +RSpec.shared_examples 'issue tracker service URL attribute' do |url_attr| + it { is_expected.to allow_value('https://example.com').for(url_attr) } + + it { is_expected.not_to allow_value('example.com').for(url_attr) } + it { is_expected.not_to allow_value('ftp://example.com').for(url_attr) } + it { is_expected.not_to allow_value('herp-and-derp').for(url_attr) } +end From cd0750e0457f26f8165be301ad628e1830bd1e40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 25 Apr 2016 15:46:15 +0200 Subject: [PATCH 17/69] Prevent private project name and namespace from leaking in the new MR view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #15591. Signed-off-by: Rémy Coutable --- app/services/merge_requests/build_service.rb | 3 +++ spec/features/merge_requests/create_new_mr_spec.rb | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index fa34753c4f..3544752d47 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -7,6 +7,9 @@ module MergeRequests merge_request.can_be_created = false merge_request.compare_commits = [] merge_request.source_project = project unless merge_request.source_project + + merge_request.target_project = nil unless can?(current_user, :read_project, merge_request.target_project) + merge_request.target_project ||= (project.forked_from_project || project) merge_request.target_branch ||= merge_request.target_project.default_branch diff --git a/spec/features/merge_requests/create_new_mr_spec.rb b/spec/features/merge_requests/create_new_mr_spec.rb index 00b60bd0e7..e296078bad 100644 --- a/spec/features/merge_requests/create_new_mr_spec.rb +++ b/spec/features/merge_requests/create_new_mr_spec.rb @@ -30,4 +30,14 @@ feature 'Create New Merge Request', feature: true, js: true do expect(page).to have_content 'git checkout -b orphaned-branch origin/orphaned-branch' end + + context 'when target project cannot be viewed by the current user' do + it 'does not leak the private project name & namespace' do + private_project = create(:project, :private) + + visit new_namespace_project_merge_request_path(project.namespace, project, merge_request: { target_project_id: private_project.id }) + + expect(page).not_to have_content private_project.to_reference + end + end end From 180c45b37ee16c24ddcb5c26a631dbca4ac76335 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Fri, 22 Apr 2016 13:25:32 +0300 Subject: [PATCH 18/69] AwardsHandler follows code style conventions --- app/assets/javascripts/awards_handler.coffee | 66 +++++++++++--------- app/assets/javascripts/notes.js.coffee | 8 +-- app/views/votes/_votes_block.html.haml | 18 +++--- 3 files changed, 50 insertions(+), 42 deletions(-) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index fcba981872..21bd19ef07 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -1,5 +1,5 @@ class @AwardsHandler - constructor: (@get_emojis_url, @post_emoji_url, @noteable_type, @noteable_id, @unicodes) -> + constructor: (@getEmojisUrl, @postEmojiUrl, @noteableType, @noteableId, @unicodes) -> $(".js-add-award").on "click", (event) => event.stopPropagation() event.preventDefault() @@ -23,13 +23,13 @@ class @AwardsHandler .find(".icon") .data "emoji" - if emoji is "thumbsup" and awards_handler.didUserClickEmoji $(this), "thumbsdown" - awards_handler.addAward "thumbsdown" + if emoji is "thumbsup" and awardsHandler.didUserClickEmoji $(this), "thumbsdown" + awardsHandler.addAward "thumbsdown" - else if emoji is "thumbsdown" and awards_handler.didUserClickEmoji $(this), "thumbsup" - awards_handler.addAward "thumbsup" + else if emoji is "thumbsdown" and awardsHandler.didUserClickEmoji $(this), "thumbsup" + awardsHandler.addAward "thumbsup" - awards_handler.addAward emoji + awardsHandler.addAward emoji $(this).trigger 'blur' @@ -47,7 +47,7 @@ class @AwardsHandler $("#emoji_search").focus() else $('.js-add-award').addClass "is-loading" - $.get @get_emojis_url, (response) => + $.get @getEmojisUrl, (response) => $('.js-add-award').removeClass "is-loading" $(".js-award-holder").append response setTimeout => @@ -99,25 +99,25 @@ class @AwardsHandler emojiIcon.remove() removeMeFromAuthorList: (emoji) -> - award_block = @findEmojiIcon(emoji).parent() - authors = award_block + awardBlock = @findEmojiIcon(emoji).parent() + authors = awardBlock .attr("data-original-title") .split(", ") authors.splice(authors.indexOf("me"),1) - award_block + awardBlock .closest(".js-emoji-btn") .attr("data-original-title", authors.join(", ")) - @resetTooltip(award_block) + @resetTooltip(awardBlock) addMeToAuthorList: (emoji) -> - award_block = @findEmojiIcon(emoji).parent() - origTitle = award_block.attr("data-original-title").trim() + awardBlock = @findEmojiIcon(emoji).parent() + origTitle = awardBlock.attr("data-original-title").trim() authors = [] if origTitle authors = origTitle.split(', ') authors.push("me") - award_block.attr("data-original-title", authors.join(", ")) - @resetTooltip(award_block) + awardBlock.attr("data-original-title", authors.join(", ")) + @resetTooltip(awardBlock) resetTooltip: (award) -> award.tooltip("destroy") @@ -139,20 +139,28 @@ class @AwardsHandler "" ) - emoji_node = $(nodes.join("\n")) + $(nodes.join("\n")) .insertBefore(".js-award-holder") .find(".emoji-icon") .data("emoji", emoji) $('.award-control').tooltip() resolveNameToCssClass: (emoji) -> - "emoji-#{@unicodes[emoji]}" + emoji_icon = $(".emoji-menu-content [data-emoji='#{emoji}']") + + if emoji_icon.length > 0 + unicodeName = emoji_icon.data('unicode-name') + else + # Find by alias + unicodeName = $(".emoji-menu-content [data-aliases*=':#{emoji}:']").data('unicode-name') + + "emoji-#{unicodeName}" postEmoji: (emoji, callback) -> - $.post @post_emoji_url, { note: { + $.post @postEmojiUrl, { note: { note: ":#{emoji}:" - noteable_type: @noteable_type - noteable_id: @noteable_id + noteable_type: @noteableType + noteable_id: @noteableId }},(data) -> if data.ok callback.call() @@ -166,21 +174,21 @@ class @AwardsHandler }, 200) addEmojiToFrequentlyUsedList: (emoji) -> - frequently_used_emojis = @getFrequentlyUsedEmojis() - frequently_used_emojis.push(emoji) - $.cookie('frequently_used_emojis', frequently_used_emojis.join(","), { expires: 365 }) + frequentlyUsedEmojis = @getFrequentlyUsedEmojis() + frequentlyUsedEmojis.push(emoji) + $.cookie('frequently_used_emojis', frequentlyUsedEmojis.join(","), { expires: 365 }) getFrequentlyUsedEmojis: -> - frequently_used_emojis = ($.cookie('frequently_used_emojis') || "").split(",") - _.compact(_.uniq(frequently_used_emojis)) + frequentlyUsedEmojis = ($.cookie('frequently_used_emojis') || "").split(",") + _.compact(_.uniq(frequentlyUsedEmojis)) renderFrequentlyUsedBlock: -> if $.cookie('frequently_used_emojis') - frequently_used_emojis = @getFrequentlyUsedEmojis() + frequentlyUsedEmojis = @getFrequentlyUsedEmojis() ul = $("
    ") - for emoji in frequently_used_emojis + for emoji in frequentlyUsedEmojis do (emoji) -> $(".emoji-menu-content [data-emoji='#{emoji}']").closest("li").clone().appendTo(ul) @@ -196,8 +204,8 @@ class @AwardsHandler if term # Generate a search result block h5 = $("
    ").text("Search results").addClass("emoji-search") - found_emojis = @searchEmojis(term).show() - ul = $("
      ").addClass("emoji-menu-list emoji-menu-search").append(found_emojis) + foundEmojis = @searchEmojis(term).show() + ul = $("
        ").addClass("emoji-menu-list emoji-menu-search").append(foundEmojis) $(".emoji-menu-content ul, .emoji-menu-content h5").hide() $(".emoji-menu-content").append(h5).append(ul) else diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 82e210fed7..efb3e8e219 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -167,8 +167,8 @@ class @Notes return if note.award - awards_handler.addAwardToEmojiBar(note.note) - awards_handler.scrollToAwards() + awardsHandler.addAwardToEmojiBar(note.note) + awardsHandler.scrollToAwards() # render note if it not present in loaded list # or skip if rendered @@ -373,11 +373,11 @@ class @Notes new GLForm form if scrollTo? and myLastNote? - # scroll to the bottom + # scroll to the bottom # so the open of the last element doesn't make a jump $('html, body').scrollTop($(document).height()); $('html, body').animate({ - scrollTop: myLastNote.offset().top - 150 + scrollTop: myLastNote.offset().top - 150 }, 500, -> $noteText = form.find(".js-note-text") $noteText.focus() diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index 59e1279869..4beb874644 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -15,16 +15,16 @@ - if current_user :javascript - var get_emojis_url = "#{emojis_path}"; - var post_emoji_url = "#{award_toggle_namespace_project_notes_path(@project.namespace, @project)}"; - var noteable_type = "#{votable.class.name.underscore}"; - var noteable_id = "#{votable.id}"; + var getEmojisUrl = "#{emojis_path}"; + var postEmojiUrl = "#{award_toggle_namespace_project_notes_path(@project.namespace, @project)}"; + var noteableType = "#{votable.class.name.underscore}"; + var noteableId = "#{votable.id}"; var unicodes = #{AwardEmoji.unicode.to_json}; - window.awards_handler = new AwardsHandler( - get_emojis_url, - post_emoji_url, - noteable_type, - noteable_id, + window.awardsHandler = new AwardsHandler( + getEmojisUrl, + postEmojiUrl, + noteableType, + noteableId, unicodes ); From 311c859d11f2ca6bcfc0bba04e778535dc4e0f83 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 25 Apr 2016 22:37:25 +0300 Subject: [PATCH 19/69] awards_handler: single quotes --- app/assets/javascripts/awards_handler.coffee | 128 +++++++++---------- 1 file changed, 64 insertions(+), 64 deletions(-) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index 21bd19ef07..bf95e06b4e 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -1,58 +1,58 @@ class @AwardsHandler constructor: (@getEmojisUrl, @postEmojiUrl, @noteableType, @noteableId, @unicodes) -> - $(".js-add-award").on "click", (event) => + $('.js-add-award').on 'click', (event) => event.stopPropagation() event.preventDefault() @showEmojiMenu() - $("html").on 'click', (event) -> - if !$(event.target).closest(".emoji-menu").length - if $(".emoji-menu").is(":visible") - $(".emoji-menu").removeClass "is-visible" + $('html').on 'click', (event) -> + if !$(event.target).closest('.emoji-menu').length + if $('.emoji-menu').is(':visible') + $('.emoji-menu').removeClass 'is-visible' - $(".awards") - .off "click" - .on "click", ".js-emoji-btn", @handleClick + $('.awards') + .off 'click' + .on 'click', '.js-emoji-btn', @handleClick @renderFrequentlyUsedBlock() handleClick: (e) -> e.preventDefault() emoji = $(this) - .find(".icon") - .data "emoji" + .find('.icon') + .data 'emoji' - if emoji is "thumbsup" and awardsHandler.didUserClickEmoji $(this), "thumbsdown" - awardsHandler.addAward "thumbsdown" + if emoji is 'thumbsup' and awardsHandler.didUserClickEmoji $(this), 'thumbsdown' + awardsHandler.addAward 'thumbsdown' - else if emoji is "thumbsdown" and awardsHandler.didUserClickEmoji $(this), "thumbsup" - awardsHandler.addAward "thumbsup" + else if emoji is 'thumbsdown' and awardsHandler.didUserClickEmoji $(this), 'thumbsup' + awardsHandler.addAward 'thumbsup' awardsHandler.addAward emoji $(this).trigger 'blur' didUserClickEmoji: (that, emoji) -> - if $(that).siblings("button:has([data-emoji=#{emoji}])").attr("data-original-title") - $(that).siblings("button:has([data-emoji=#{emoji}])").attr("data-original-title").indexOf('me') > -1 + if $(that).siblings("button:has([data-emoji=#{emoji}])").attr('data-original-title') + $(that).siblings("button:has([data-emoji=#{emoji}])").attr('data-original-title').indexOf('me') > -1 showEmojiMenu: -> - if $(".emoji-menu").length - if $(".emoji-menu").is ".is-visible" - $(".emoji-menu").removeClass "is-visible" - $("#emoji_search").blur() + if $('.emoji-menu').length + if $('.emoji-menu').is '.is-visible' + $('.emoji-menu').removeClass 'is-visible' + $('#emoji_search').blur() else - $(".emoji-menu").addClass "is-visible" - $("#emoji_search").focus() + $('.emoji-menu').addClass 'is-visible' + $('#emoji_search').focus() else - $('.js-add-award').addClass "is-loading" + $('.js-add-award').addClass 'is-loading' $.get @getEmojisUrl, (response) => - $('.js-add-award').removeClass "is-loading" - $(".js-award-holder").append response + $('.js-add-award').removeClass 'is-loading' + $('.js-award-holder').append response setTimeout => - $(".emoji-menu").addClass "is-visible" - $("#emoji_search").focus() + $('.emoji-menu').addClass 'is-visible' + $('#emoji_search').focus() @setupSearch() , 200 @@ -60,7 +60,7 @@ class @AwardsHandler @postEmoji emoji, => @addAwardToEmojiBar(emoji) - $(".emoji-menu").removeClass "is-visible" + $('.emoji-menu').removeClass 'is-visible' addAwardToEmojiBar: (emoji) -> @addEmojiToFrequentlyUsedList(emoji) @@ -69,9 +69,9 @@ class @AwardsHandler if @isActive(emoji) @decrementCounter(emoji) else - counter = @findEmojiIcon(emoji).siblings(".js-counter") + counter = @findEmojiIcon(emoji).siblings('.js-counter') counter.text(parseInt(counter.text()) + 1) - counter.parent().addClass("active") + counter.parent().addClass('active') @addMeToAuthorList(emoji) else @createEmoji(emoji) @@ -80,47 +80,47 @@ class @AwardsHandler @findEmojiIcon(emoji).length > 0 isActive: (emoji) -> - @findEmojiIcon(emoji).parent().hasClass("active") + @findEmojiIcon(emoji).parent().hasClass('active') decrementCounter: (emoji) -> - counter = @findEmojiIcon(emoji).siblings(".js-counter") + counter = @findEmojiIcon(emoji).siblings('.js-counter') emojiIcon = counter.parent() if parseInt(counter.text()) > 1 counter.text(parseInt(counter.text()) - 1) - emojiIcon.removeClass("active") + emojiIcon.removeClass('active') @removeMeFromAuthorList(emoji) - else if emoji == "thumbsup" || emoji == "thumbsdown" - emojiIcon.tooltip("destroy") + else if emoji == 'thumbsup' || emoji == 'thumbsdown' + emojiIcon.tooltip('destroy') counter.text(0) - emojiIcon.removeClass("active") + emojiIcon.removeClass('active') @removeMeFromAuthorList(emoji) else - emojiIcon.tooltip("destroy") + emojiIcon.tooltip('destroy') emojiIcon.remove() removeMeFromAuthorList: (emoji) -> awardBlock = @findEmojiIcon(emoji).parent() authors = awardBlock - .attr("data-original-title") - .split(", ") - authors.splice(authors.indexOf("me"),1) + .attr('data-original-title') + .split(', ') + authors.splice(authors.indexOf('me'),1) awardBlock - .closest(".js-emoji-btn") - .attr("data-original-title", authors.join(", ")) + .closest('.js-emoji-btn') + .attr('data-original-title', authors.join(', ')) @resetTooltip(awardBlock) addMeToAuthorList: (emoji) -> awardBlock = @findEmojiIcon(emoji).parent() - origTitle = awardBlock.attr("data-original-title").trim() + origTitle = awardBlock.attr('data-original-title').trim() authors = [] if origTitle authors = origTitle.split(', ') - authors.push("me") - awardBlock.attr("data-original-title", authors.join(", ")) + authors.push('me') + awardBlock.attr('data-original-title', authors.join(', ')) @resetTooltip(awardBlock) resetTooltip: (award) -> - award.tooltip("destroy") + award.tooltip('destroy') # "destroy" call is asynchronous and there is no appropriate callback on it, this is why we need to set timeout. setTimeout (-> @@ -140,16 +140,16 @@ class @AwardsHandler ) $(nodes.join("\n")) - .insertBefore(".js-award-holder") - .find(".emoji-icon") - .data("emoji", emoji) + .insertBefore('.js-award-holder') + .find('.emoji-icon') + .data('emoji', emoji) $('.award-control').tooltip() resolveNameToCssClass: (emoji) -> - emoji_icon = $(".emoji-menu-content [data-emoji='#{emoji}']") + emojiIcon = $(".emoji-menu-content [data-emoji='#{emoji}']") - if emoji_icon.length > 0 - unicodeName = emoji_icon.data('unicode-name') + if emojiIcon.length > 0 + unicodeName = emojiIcon.data('unicode-name') else # Find by alias unicodeName = $(".emoji-menu-content [data-aliases*=':#{emoji}:']").data('unicode-name') @@ -176,40 +176,40 @@ class @AwardsHandler addEmojiToFrequentlyUsedList: (emoji) -> frequentlyUsedEmojis = @getFrequentlyUsedEmojis() frequentlyUsedEmojis.push(emoji) - $.cookie('frequently_used_emojis', frequentlyUsedEmojis.join(","), { expires: 365 }) + $.cookie('frequently_used_emojis', frequentlyUsedEmojis.join(','), { expires: 365 }) getFrequentlyUsedEmojis: -> - frequentlyUsedEmojis = ($.cookie('frequently_used_emojis') || "").split(",") + frequentlyUsedEmojis = ($.cookie('frequently_used_emojis') || '').split(',') _.compact(_.uniq(frequentlyUsedEmojis)) renderFrequentlyUsedBlock: -> if $.cookie('frequently_used_emojis') frequentlyUsedEmojis = @getFrequentlyUsedEmojis() - ul = $("
          ") + ul = $('
            ') for emoji in frequentlyUsedEmojis do (emoji) -> - $(".emoji-menu-content [data-emoji='#{emoji}']").closest("li").clone().appendTo(ul) + $(".emoji-menu-content [data-emoji='#{emoji}']").closest('li').clone().appendTo(ul) - $("input.emoji-search").after(ul).after($("
            ").text("Frequently used")) + $('input.emoji-search').after(ul).after($('
            ').text('Frequently used')) setupSearch: -> - $("input.emoji-search").keyup (ev) => + $('input.emoji-search').keyup (ev) => term = $(ev.target).val() # Clean previous search results - $("ul.emoji-menu-search, h5.emoji-search").remove() + $('ul.emoji-menu-search, h5.emoji-search').remove() if term # Generate a search result block - h5 = $("
            ").text("Search results").addClass("emoji-search") + h5 = $('
            ').text('Search results').addClass('emoji-search') foundEmojis = @searchEmojis(term).show() - ul = $("
              ").addClass("emoji-menu-list emoji-menu-search").append(foundEmojis) - $(".emoji-menu-content ul, .emoji-menu-content h5").hide() - $(".emoji-menu-content").append(h5).append(ul) + ul = $('
                ').addClass('emoji-menu-list emoji-menu-search').append(foundEmojis) + $('.emoji-menu-content ul, .emoji-menu-content h5').hide() + $('.emoji-menu-content').append(h5).append(ul) else - $(".emoji-menu-content").children().show() + $('.emoji-menu-content').children().show() searchEmojis: (term)-> $(".emoji-menu-content [data-emoji*='#{term}']").closest("li").clone() From 2e072034a6a30eaf9478875140c541b9b072f7aa Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Tue, 26 Apr 2016 11:54:37 +0200 Subject: [PATCH 20/69] Send 'admin emails' weekly, not daily Daily seems to be to spammy, so let's default to weekly instead. --- config/gitlab.yml.example | 4 ++-- config/initializers/1_settings.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index d9c15f8140..20d65a8f33 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -168,9 +168,9 @@ production: &base # once per hour you will have concurrent 'git fsck' jobs. repository_check_worker: cron: "20 * * * *" - # Send admin emails once a day + # Send admin emails once a week admin_email_worker: - cron: "0 0 * * *" + cron: "0 0 * * 0" # Remove outdated repository archives repository_archive_cache_worker: diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 10c25044b7..e32f7c15e6 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -245,7 +245,7 @@ Settings.cron_jobs['repository_check_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['repository_check_worker']['cron'] ||= '20 * * * *' Settings.cron_jobs['repository_check_worker']['job_class'] = 'RepositoryCheck::BatchWorker' Settings.cron_jobs['admin_email_worker'] ||= Settingslogic.new({}) -Settings.cron_jobs['admin_email_worker']['cron'] ||= '0 0 * * *' +Settings.cron_jobs['admin_email_worker']['cron'] ||= '0 0 * * 0' Settings.cron_jobs['admin_email_worker']['job_class'] = 'AdminEmailWorker' Settings.cron_jobs['repository_archive_cache_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['repository_archive_cache_worker']['cron'] ||= '0 * * * *' From 4c772d9bc849e21349d6b01701db9beccf903c44 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 26 Apr 2016 10:56:18 +0100 Subject: [PATCH 21/69] Project webhooks updated UI Closes #13993 --- .../stylesheets/framework/typography.scss | 10 ++ .../projects/hooks/_project_hook.html.haml | 15 ++ app/views/projects/hooks/index.html.haml | 161 +++++++++--------- 3 files changed, 101 insertions(+), 85 deletions(-) create mode 100644 app/views/projects/hooks/_project_hook.html.haml diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 0a5b4b8834..b2535ddf4b 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -205,6 +205,10 @@ h1, h2, h3, h4, h5, h6 { font-weight: 600; } +.light-header { + font-weight: 600; +} + /** CODE **/ pre { font-family: $monospace_font; @@ -259,3 +263,9 @@ h1, h2, h3, h4 { color: $gl-gray; } } + +.text-right-lg { + @media (min-width: $screen-lg-min) { + text-align: right; + } +} diff --git a/app/views/projects/hooks/_project_hook.html.haml b/app/views/projects/hooks/_project_hook.html.haml new file mode 100644 index 0000000000..62eba5888a --- /dev/null +++ b/app/views/projects/hooks/_project_hook.html.haml @@ -0,0 +1,15 @@ +%li + .row + .col-md-8.col-lg-7 + %strong.light-header= hook.url + %div + - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events).each do |trigger| + - if hook.send(trigger) + %span.label.label-gray.deploy-project-label= trigger.titleize + .col-md-4.col-lg-5.text-right-lg.prepend-top-5 + %span.append-right-10.inline + SSL Verification: #{hook.enable_ssl_verification ? "enabled" : "disabled"} + = link_to "Test", test_namespace_project_hook_path(@project.namespace, @project, hook), class: "btn btn-sm" + = link_to namespace_project_hook_path(@project.namespace, @project, hook), data: { confirm: 'Are you sure?'}, method: :delete, class: "btn btn-transparent" do + %span.sr-only Remove + = icon('trash') diff --git a/app/views/projects/hooks/index.html.haml b/app/views/projects/hooks/index.html.haml index aae3abcad4..6f1ee20943 100644 --- a/app/views/projects/hooks/index.html.haml +++ b/app/views/projects/hooks/index.html.haml @@ -1,88 +1,79 @@ - page_title "Webhooks" -%h3.page-title - Webhooks +.row.prepend-top-default + .col-lg-3.profile-settings-sidebar + %h4.prepend-top-0 + = page_title + %p + #{link_to "Webhooks", help_page_path("web_hooks", "web_hooks")} can be + used for binding events when something is happening within the project. + .col-lg-9.append-bottom-default + %h5.prepend-top-0 + Add new webhook + = form_for [@project.namespace.becomes(Namespace), @project, @hook], as: :hook, url: namespace_project_hooks_path(@project.namespace, @project) do |f| + = form_errors(@hook) -%p.light - #{link_to "Webhooks ", help_page_path("web_hooks", "web_hooks"), class: "vlink"} can be - used for binding events when something is happening within the project. - -%hr.clearfix - -= form_for [@project.namespace.becomes(Namespace), @project, @hook], as: :hook, url: namespace_project_hooks_path(@project.namespace, @project), html: { class: 'form-horizontal' } do |f| - = form_errors(@hook) - - .form-group - = f.label :url, "URL", class: 'control-label' - .col-sm-10 - = f.text_field :url, class: "form-control", placeholder: 'http://example.com/trigger-ci.json' - .form-group - = f.label :url, "Trigger", class: 'control-label' - .col-sm-10.prepend-top-10 - %div - = f.check_box :push_events, class: 'pull-left' - .prepend-left-20 - = f.label :push_events, class: 'list-label' do - %strong Push events - %p.light - This url will be triggered by a push to the repository - %div - = f.check_box :tag_push_events, class: 'pull-left' - .prepend-left-20 - = f.label :tag_push_events, class: 'list-label' do - %strong Tag push events - %p.light - This url will be triggered when a new tag is pushed to the repository - %div - = f.check_box :note_events, class: 'pull-left' - .prepend-left-20 - = f.label :note_events, class: 'list-label' do - %strong Comments - %p.light - This url will be triggered when someone adds a comment - %div - = f.check_box :issues_events, class: 'pull-left' - .prepend-left-20 - = f.label :issues_events, class: 'list-label' do - %strong Issues events - %p.light - This url will be triggered when an issue is created/updated/merged - %div - = f.check_box :merge_requests_events, class: 'pull-left' - .prepend-left-20 - = f.label :merge_requests_events, class: 'list-label' do - %strong Merge Request events - %p.light - This url will be triggered when a merge request is created/updated/merged - %div - = f.check_box :build_events, class: 'pull-left' - .prepend-left-20 - = f.label :build_events, class: 'list-label' do - %strong Build events - %p.light - This url will be triggered when the build status changes - .form-group - = f.label :enable_ssl_verification, "SSL verification", class: 'control-label checkbox' - .col-sm-10 - .checkbox - = f.label :enable_ssl_verification do - = f.check_box :enable_ssl_verification - %strong Enable SSL verification - .form-actions - = f.submit "Add Webhook", class: "btn btn-create" - --if @hooks.any? - .panel.panel-default - .panel-heading + .form-group + = f.label :url, "URL", class: "label-light" + = f.text_field :url, class: "form-control", placeholder: "http://example.com/trigger-ci.json" + .form-group + = f.label :url, "Trigger", class: "label-light" + %div + = f.check_box :push_events, class: "pull-left" + .prepend-left-20 + = f.label :push_events, class: "label-light append-bottom-0" do + Push events + %p.light + This url will be triggered by a push to the repository + %div + = f.check_box :tag_push_events, class: "pull-left" + .prepend-left-20 + = f.label :tag_push_events, class: "label-light append-bottom-0" do + Tag push events + %p.light + This url will be triggered when a new tag is pushed to the repository + %div + = f.check_box :note_events, class: "pull-left" + .prepend-left-20 + = f.label :note_events, class: "label-light append-bottom-0" do + Comments + %p.light + This url will be triggered when someone adds a comment + %div + = f.check_box :issues_events, class: "pull-left" + .prepend-left-20 + = f.label :issues_events, class: "label-light append-bottom-0" do + Issues events + %p.light + This url will be triggered when an issue is created/updated/merged + %div + = f.check_box :merge_requests_events, class: "pull-left" + .prepend-left-20 + = f.label :merge_requests_events, class: "label-light append-bottom-0" do + Merge Request events + %p.light + This url will be triggered when a merge request is created/updated/merged + %div + = f.check_box :build_events, class: "pull-left" + .prepend-left-20 + = f.label :build_events, class: "label-light append-bottom-0" do + Build events + %p.light + This url will be triggered when the build status changes + .form-group + = f.label :enable_ssl_verification, "SSL verification", class: "label-light" + %div + = f.check_box :enable_ssl_verification, class: "pull-left" + .prepend-left-20 + = f.label :enable_ssl_verification, class: "label-light append-bottom-0" do + Enable SSL verification + = f.submit "Add Webhook", class: "btn btn-create" + %hr + %h5.prepend-top-default Webhooks (#{@hooks.count}) - %ul.content-list - - @hooks.each do |hook| - %li - .controls - = link_to 'Test Hook', test_namespace_project_hook_path(@project.namespace, @project, hook), class: "btn btn-sm btn-grouped" - = link_to 'Remove', namespace_project_hook_path(@project.namespace, @project, hook), data: { confirm: 'Are you sure?'}, method: :delete, class: "btn btn-remove btn-sm btn-grouped" - .monospace= hook.url - %div - - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events).each do |trigger| - - if hook.send(trigger) - %span.label.label-gray= trigger.titleize - %span.label.label-gray SSL Verification: #{hook.enable_ssl_verification ? "enabled" : "disabled"} + - if @hooks.any? + %ul.well-list + - @hooks.each do |hook| + = render "project_hook", hook: hook + - else + %p.profile-settings-message.text-center.append-bottom-0 + No webhooks found, add one in the form above. From b651303ea63874e7cc9428c09770bee138cc68ec Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 26 Apr 2016 14:14:34 +0100 Subject: [PATCH 22/69] Updated tests --- features/steps/project/hooks.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/features/steps/project/hooks.rb b/features/steps/project/hooks.rb index 4994df589a..b1ffe7f7b4 100644 --- a/features/steps/project/hooks.rb +++ b/features/steps/project/hooks.rb @@ -48,12 +48,12 @@ class Spinach::Features::ProjectHooks < Spinach::FeatureSteps step 'I click test hook button' do stub_request(:post, @hook.url).to_return(status: 200) - click_link 'Test Hook' + click_link 'Test' end step 'I click test hook button with invalid URL' do stub_request(:post, @hook.url).to_raise(SocketError) - click_link 'Test Hook' + click_link 'Test' end step 'hook should be triggered' do From a3d6b61063b8a3864ba9823166fa08fdeeb9d2d9 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 26 Apr 2016 15:50:31 +0100 Subject: [PATCH 23/69] Protected branches UI Closes #14027 --- app/assets/stylesheets/framework/common.scss | 1 + .../stylesheets/framework/typography.scss | 4 ++ app/assets/stylesheets/pages/projects.scss | 17 ++++-- .../_branches_list.html.haml | 56 ++++++++++-------- .../protected_branches/index.html.haml | 58 ++++++++++--------- 5 files changed, 77 insertions(+), 59 deletions(-) diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 2ade341c9d..3386523dbf 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -11,6 +11,7 @@ .prepend-top-10 { margin-top: 10px } .prepend-top-default { margin-top: $gl-padding !important; } .prepend-top-20 { margin-top: 20px } +.prepend-left-5 { margin-left: 5px } .prepend-left-10 { margin-left: 10px } .prepend-left-default { margin-left: $gl-padding; } .prepend-left-20 { margin-left: 20px } diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 0a5b4b8834..64b1c24738 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -205,6 +205,10 @@ h1, h2, h3, h4, h5, h6 { font-weight: 600; } +.light-header { + font-weight: 600; +} + /** CODE **/ pre { font-family: $monospace_font; diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index fcca9d4faf..0dd44cdc14 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -256,12 +256,6 @@ } } -table.table.protected-branches-list tr.no-border { - th, td { - border: 0; - } -} - .project-import .btn { float: left; margin-right: 10px; @@ -474,3 +468,14 @@ pre.light-well { color: #fff; } } + +.protected-branches-list { + a { + color: $gl-gray; + font-weight: 600; + + &:hover { + color: $gl-link-color; + } + } +} diff --git a/app/views/projects/protected_branches/_branches_list.html.haml b/app/views/projects/protected_branches/_branches_list.html.haml index f68449b186..b9e9dd8aae 100644 --- a/app/views/projects/protected_branches/_branches_list.html.haml +++ b/app/views/projects/protected_branches/_branches_list.html.haml @@ -1,35 +1,41 @@ -- unless @branches.empty? - %br - %h4 Already Protected: - .table-holder +%h5.prepend-top-0 + Already Protected (#{@branches.size}) +- if @branches.empty? + %p.profile-settings-message.text-center + No branches are protected, protect a branch with the form above. +- else + - can_admin_project = can?(current_user, :admin_project, @project) + .table-responsive %table.table.protected-branches-list + %colgroup + %col{ width: "30%" } + %col{ width: "30%" } + %col{ width: "25%" } + - if can_admin_project + %col %thead - %tr.no-border + %tr %th Branch - %th Developers can push %th Last commit - %th - + %th Developers can push + - if can_admin_project + %th %tbody - @branches.each do |branch| - @url = namespace_project_protected_branch_path(@project.namespace, @project, branch) %tr %td - = link_to namespace_project_commits_path(@project.namespace, @project, branch.name) do - %strong= branch.name - - if @project.root_ref?(branch.name) - %span.label.label-info default + = link_to(branch.name, namespace_project_commits_path(@project.namespace, @project, branch.name)) + - if @project.root_ref?(branch.name) + %span.label.label-info.prepend-left-5 default + %td + - if commit = branch.commit + = link_to(commit.short_id, namespace_project_commit_path(@project.namespace, @project, commit.id), class: 'commit_short_id') + #{time_ago_with_tooltip(commit.committed_date)} + - else + (branch was removed from repository) + %td + = check_box_tag("developers_can_push", branch.id, branch.developers_can_push, data: { url: @url }) + - if can_admin_project %td - = check_box_tag "developers_can_push", branch.id, branch.developers_can_push, "data-url" => @url - %td - - if commit = branch.commit - = link_to namespace_project_commit_path(@project.namespace, @project, commit.id), class: 'commit_short_id' do - = commit.short_id - · - #{time_ago_with_tooltip(commit.committed_date)} - - else - (branch was removed from repository) - %td - .pull-right - - if can? current_user, :admin_project, @project - = link_to 'Unprotect', [@project.namespace.becomes(Namespace), @project, branch], data: { confirm: 'Branch will be writable for developers. Are you sure?' }, method: :delete, class: "btn btn-remove btn-sm" + = link_to 'Unprotect', [@project.namespace.becomes(Namespace), @project, branch], data: { confirm: 'Branch will be writable for developers. Are you sure?' }, method: :delete, class: "btn btn-warning btn-sm" diff --git a/app/views/projects/protected_branches/index.html.haml b/app/views/projects/protected_branches/index.html.haml index 653b02da4d..c7d317dbae 100644 --- a/app/views/projects/protected_branches/index.html.haml +++ b/app/views/projects/protected_branches/index.html.haml @@ -1,31 +1,33 @@ - page_title "Protected branches" -%h3.page-title Protected branches -%p.light Keep stable branches secure and force developers to use Merge Requests -%hr -.well - %p Protected branches are designed to - %ul - %li prevent pushes from everybody except #{link_to "masters", help_page_path("permissions", "permissions"), class: "vlink"} - %li prevent anyone from force pushing to the branch - %li prevent anyone from deleting the branch - %p Read more about #{link_to "project permissions", help_page_path("permissions", "permissions"), class: "underlined-link"} +.row.prepend-top-default.append-bottom-default + .col-lg-3 + %h4.prepend-top-0 + = page_title + %p Keep stable branches secure and force developers to use Merge Requests + .col-lg-9 + %h5.prepend-top-0 + Protect a branch + .account-well.append-bottom-default + %p.light-header.append-bottom-0 Protected branches are designed to + %ul + %li prevent pushes from everybody except #{link_to "masters", help_page_path("permissions", "permissions"), class: "vlink"} + %li prevent anyone from force pushing to the branch + %li prevent anyone from deleting the branch + %p.append-bottom-0 Read more about #{link_to "project permissions", help_page_path("permissions", "permissions"), class: "underlined-link"} + - if can? current_user, :admin_project, @project + = form_for [@project.namespace.becomes(Namespace), @project, @protected_branch] do |f| + = form_errors(@protected_branch) -- if can? current_user, :admin_project, @project - = form_for [@project.namespace.becomes(Namespace), @project, @protected_branch], html: { class: 'form-horizontal' } do |f| - = form_errors(@protected_branch) - - .form-group - = f.label :name, "Branch", class: 'control-label' - .col-sm-10 - = f.select(:name, @project.open_branches.map { |br| [br.name, br.name] } , {include_blank: true}, {class: "select2", data: {placeholder: "Select branch"}}) - .form-group - .col-sm-offset-2.col-sm-10 - .checkbox - = f.label :developers_can_push do - = f.check_box :developers_can_push - %strong Developers can push - .help-block Allow developers to push to this branch - .form-actions - = f.submit 'Protect', class: "btn-create btn" -= render 'branches_list' + .form-group + = f.label :name, "Branch", class: "label-light" + = f.select(:name, @project.open_branches.map { |br| [br.name, br.name] } , {include_blank: true}, {class: "select2", data: {placeholder: "Select branch"}}) + .form-group + = f.check_box :developers_can_push, class: "pull-left" + .prepend-left-20 + = f.label :developers_can_push, "Developers can push", class: "label-light append-bottom-0" + %p.light.append-bottom-0 + Allow developers to push to this branch + = f.submit "Protect", class: "btn-create btn" + %hr + = render "branches_list" From be67a4843cc37790402404650cb96a6f02552b54 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 26 Apr 2016 12:55:38 -0400 Subject: [PATCH 24/69] Prevent privilege escalation via notes API Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/15577 --- app/services/notes/create_service.rb | 11 ++++++++ spec/requests/api/notes_spec.rb | 41 +++++++++++++++++++++------- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 2bb312bb25..0158699481 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -5,6 +5,8 @@ module Notes note.author = current_user note.system = false + return unless valid_project?(note) + if note.save # Finish the harder work in the background NewNoteWorker.perform_in(2.seconds, note.id, params) @@ -13,5 +15,14 @@ module Notes note end + + private + + def valid_project?(note) + return false unless project + return true if note.for_commit? + + note.noteable.try(:project) == project + end end end diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index ec9eda0a2e..49091fc0f4 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe API::API, api: true do include ApiHelpers let(:user) { create(:user) } - let!(:project) { create(:project, namespace: user.namespace ) } + let!(:project) { create(:project, namespace: user.namespace) } let!(:issue) { create(:issue, project: project, author: user) } let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) } let!(:snippet) { create(:project_snippet, project: project, author: user) } @@ -45,7 +45,7 @@ describe API::API, api: true do end it "should return a 404 error when issue id not found" do - get api("/projects/#{project.id}/issues/123/notes", user) + get api("/projects/#{project.id}/issues/12345/notes", user) expect(response.status).to eq(404) end @@ -106,7 +106,7 @@ describe API::API, api: true do end it "should return a 404 error if issue note not found" do - get api("/projects/#{project.id}/issues/#{issue.id}/notes/123", user) + get api("/projects/#{project.id}/issues/#{issue.id}/notes/12345", user) expect(response.status).to eq(404) end @@ -134,7 +134,7 @@ describe API::API, api: true do end it "should return a 404 error if snippet note not found" do - get api("/projects/#{project.id}/snippets/#{snippet.id}/notes/123", user) + get api("/projects/#{project.id}/snippets/#{snippet.id}/notes/12345", user) expect(response.status).to eq(404) end end @@ -191,6 +191,27 @@ describe API::API, api: true do expect(response.status).to eq(401) end end + + context 'when user does not have access to create noteable' do + let(:private_issue) { create(:issue, project: create(:project, :private)) } + + ## + # We are posting to project user has access to, but we use issue id + # from a different project, see #15577 + # + before do + post api("/projects/#{project.id}/issues/#{private_issue.id}/notes", user), + body: 'Hi!' + end + + it 'responds with 500' do + expect(response.status).to eq 500 + end + + it 'does not create new note' do + expect(private_issue.notes.reload).to be_empty + end + end end describe "POST /projects/:id/noteable/:noteable_id/notes to test observer on create" do @@ -211,7 +232,7 @@ describe API::API, api: true do end it 'should return a 404 error when note id not found' do - put api("/projects/#{project.id}/issues/#{issue.id}/notes/123", user), + put api("/projects/#{project.id}/issues/#{issue.id}/notes/12345", user), body: 'Hello!' expect(response.status).to eq(404) end @@ -233,7 +254,7 @@ describe API::API, api: true do it 'should return a 404 error when note id not found' do put api("/projects/#{project.id}/snippets/#{snippet.id}/"\ - "notes/123", user), body: "Hello!" + "notes/12345", user), body: "Hello!" expect(response.status).to eq(404) end end @@ -248,7 +269,7 @@ describe API::API, api: true do it 'should return a 404 error when note id not found' do put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/"\ - "notes/123", user), body: "Hello!" + "notes/12345", user), body: "Hello!" expect(response.status).to eq(404) end end @@ -268,7 +289,7 @@ describe API::API, api: true do end it 'returns a 404 error when note id not found' do - delete api("/projects/#{project.id}/issues/#{issue.id}/notes/123", user) + delete api("/projects/#{project.id}/issues/#{issue.id}/notes/12345", user) expect(response.status).to eq(404) end @@ -288,7 +309,7 @@ describe API::API, api: true do it 'returns a 404 error when note id not found' do delete api("/projects/#{project.id}/snippets/#{snippet.id}/"\ - "notes/123", user) + "notes/12345", user) expect(response.status).to eq(404) end @@ -308,7 +329,7 @@ describe API::API, api: true do it 'returns a 404 error when note id not found' do delete api("/projects/#{project.id}/merge_requests/"\ - "#{merge_request.id}/notes/123", user) + "#{merge_request.id}/notes/12345", user) expect(response.status).to eq(404) end From eede0323453190440a8d738b5eab0723f54dee65 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 22 Apr 2016 12:43:10 -0700 Subject: [PATCH 25/69] Backport GitHub Enterprise import support from EE These changes were pulled from GitLab EE to support configuring an alternative API URL than the default https://api.github.com. In addition, the `verify_ssl` flag allows users to disable SSL cert checking. One modification: add a default `args` option if it does not exist to avoid breaking existing configurations. --- CHANGELOG | 1 + config/gitlab.yml.example | 2 + config/initializers/1_settings.rb | 24 +++++++++++ doc/integration/github.md | 14 +++++++ lib/gitlab/github_import/client.rb | 15 +++++-- .../import/github_controller_spec.rb | 2 + spec/lib/gitlab/github_import/client_spec.rb | 40 +++++++++++++++++-- spec/services/projects/import_service_spec.rb | 13 ++++-- 8 files changed, 101 insertions(+), 10 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index e52e52691c..e899877380 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -8,6 +8,7 @@ v 8.8.0 (unreleased) - Replace Devise Async with Devise ActiveJob integration. !3902 (Connor Shea) - Allow "NEWS" and "CHANGES" as alternative names for CHANGELOG. !3768 (Connor Shea) - Added button to toggle whitespaces changes on diff view + - Backport GitLab Enterprise support from EE v 8.7.1 (unreleased) - Throttle the update of `project.last_activity_at` to 1 minute. !3848 diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index d9c15f8140..2790f1c8a2 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -350,6 +350,8 @@ production: &base # - { name: 'github', # app_id: 'YOUR_APP_ID', # app_secret: 'YOUR_APP_SECRET', + # url: "https://github.com/", + # verify_ssl: true, # args: { scope: 'user:email' } } # - { name: 'bitbucket', # app_id: 'YOUR_APP_ID', diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 10c25044b7..3964165518 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -140,6 +140,30 @@ Settings.omniauth.cas3['session_duration'] ||= 8.hours Settings.omniauth['session_tickets'] ||= Settingslogic.new({}) Settings.omniauth.session_tickets['cas3'] = 'ticket' +# Fill out omniauth-gitlab settings. It is needed for easy set up GHE or GH by just specifying url. + +github_default_url = "https://github.com" +github_settings = Settings.omniauth['providers'].find { |provider| provider["name"] == "github"} + +if github_settings + # For compatibility with old config files (before 7.8) + # where people dont have url in github settings + if github_settings['url'].blank? + github_settings['url'] = github_default_url + end + + github_settings["args"] ||= Settingslogic.new({}) + + if github_settings["url"].include?(github_default_url) + github_settings["args"]["client_options"] = OmniAuth::Strategies::GitHub.default_options[:client_options] + else + github_settings["args"]["client_options"] = { + "site" => File.join(github_settings["url"], "api/v3"), + "authorize_url" => File.join(github_settings["url"], "login/oauth/authorize"), + "token_url" => File.join(github_settings["url"], "login/oauth/access_token") + } + end +end Settings['shared'] ||= Settingslogic.new({}) Settings.shared['path'] = File.expand_path(Settings.shared['path'] || "shared", Rails.root) diff --git a/doc/integration/github.md b/doc/integration/github.md index 1890edd7a4..ac17e2069f 100644 --- a/doc/integration/github.md +++ b/doc/integration/github.md @@ -60,12 +60,26 @@ GitHub will generate an application ID and secret key for you to use. For installation from source: + For GitHub.com: + ``` - { name: 'github', app_id: 'YOUR_APP_ID', app_secret: 'YOUR_APP_SECRET', args: { scope: 'user:email' } } ``` + + For GitHub Enterprise: + + ``` + - { name: 'github', app_id: 'YOUR_APP_ID', + app_secret: 'YOUR_APP_SECRET', + url: "https://github.example.com/", + args: { scope: 'user:email' } } + ``` + + __Replace `https://github.example.com/` with your GitHub URL__ + 1. Change 'YOUR_APP_ID' to the client ID from the GitHub application page from step 7. 1. Change 'YOUR_APP_SECRET' to the client secret from the GitHub application page from step 7. diff --git a/lib/gitlab/github_import/client.rb b/lib/gitlab/github_import/client.rb index 74d1529e1f..67988ea346 100644 --- a/lib/gitlab/github_import/client.rb +++ b/lib/gitlab/github_import/client.rb @@ -7,12 +7,19 @@ module Gitlab @client = ::OAuth2::Client.new( config.app_id, config.app_secret, - github_options + github_options.merge(ssl: { verify: config['verify_ssl'] }) ) if access_token ::Octokit.auto_paginate = true - @api = ::Octokit::Client.new(access_token: access_token) + + @api = ::Octokit::Client.new( + access_token: access_token, + api_endpoint: github_options[:site], + connection_options: { + ssl: { verify: config['verify_ssl'] } + } + ) end end @@ -42,11 +49,11 @@ module Gitlab private def config - Gitlab.config.omniauth.providers.find{|provider| provider.name == "github"} + Gitlab.config.omniauth.providers.find { |provider| provider.name == "github" } end def github_options - OmniAuth::Strategies::GitHub.default_options[:client_options].to_h.symbolize_keys + config["args"]["client_options"].deep_symbolize_keys end end end diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index bbf8adef53..bcc713dce2 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -22,6 +22,8 @@ describe Import::GithubController do token = "asdasd12345" allow_any_instance_of(Gitlab::GithubImport::Client). to receive(:get_token).and_return(token) + allow_any_instance_of(Gitlab::GithubImport::Client). + to receive(:github_options).and_return({}) stub_omniauth_provider('github') get :callback diff --git a/spec/lib/gitlab/github_import/client_spec.rb b/spec/lib/gitlab/github_import/client_spec.rb index 49d8cdf431..7c21cbe96d 100644 --- a/spec/lib/gitlab/github_import/client_spec.rb +++ b/spec/lib/gitlab/github_import/client_spec.rb @@ -2,15 +2,49 @@ require 'spec_helper' describe Gitlab::GithubImport::Client, lib: true do let(:token) { '123456' } - let(:client) { Gitlab::GithubImport::Client.new(token) } + let(:github_provider) { Settingslogic.new('app_id' => 'asd123', 'app_secret' => 'asd123', 'name' => 'github', 'args' => { 'client_options' => {} }) } + + subject(:client) { described_class.new(token) } before do - Gitlab.config.omniauth.providers << OpenStruct.new(app_id: "asd123", app_secret: "asd123", name: "github") + allow(Gitlab.config.omniauth).to receive(:providers).and_return([github_provider]) end - it 'all OAuth2 client options are symbols' do + it 'convert OAuth2 client options to symbols' do client.client.options.keys.each do |key| expect(key).to be_kind_of(Symbol) end end + + it 'does not crash (e.g. Settingslogic::MissingSetting) when verify_ssl config is not present' do + expect { client.api }.not_to raise_error + end + + context 'allow SSL verification to be configurable on API' do + before do + github_provider['verify_ssl'] = false + end + + it 'uses supplied value' do + expect(client.client.options[:connection_opts][:ssl]).to eq({ verify: false }) + expect(client.api.connection_options[:ssl]).to eq({ verify: false }) + end + end + + context 'when provider does not specity an API endpoint' do + it 'uses GitHub root API endpoint' do + expect(client.api.api_endpoint).to eq 'https://api.github.com/' + end + end + + context 'when provider specify a custom API endpoint' do + before do + github_provider['args']['client_options']['site'] = 'https://github.company.com/' + end + + it 'uses the custom API endpoint' do + expect(OmniAuth::Strategies::GitHub).not_to receive(:default_options) + expect(client.api.api_endpoint).to eq 'https://github.company.com/' + end + end end diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb index 32bf3acf48..7f2dcdab96 100644 --- a/spec/services/projects/import_service_spec.rb +++ b/spec/services/projects/import_service_spec.rb @@ -112,9 +112,16 @@ describe Projects::ImportService, services: true do def stub_github_omniauth_provider provider = OpenStruct.new( - name: 'github', - app_id: 'asd123', - app_secret: 'asd123' + 'name' => 'github', + 'app_id' => 'asd123', + 'app_secret' => 'asd123', + 'args' => { + 'client_options' => { + 'site' => 'https://github.com/api/v3', + 'authorize_url' => 'https://github.com/login/oauth/authorize', + 'token_url' => 'https://github.com/login/oauth/access_token' + } + } ) Gitlab.config.omniauth.providers << provider From c47183d78723a30733dc465f72e37299d8628363 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 26 Apr 2016 21:02:24 -0700 Subject: [PATCH 26/69] Fixes for review of GitHub Enterprise backport changes --- config/initializers/1_settings.rb | 6 +++--- doc/integration/github.md | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 3964165518..6eb144d043 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -143,7 +143,7 @@ Settings.omniauth.session_tickets['cas3'] = 'ticket' # Fill out omniauth-gitlab settings. It is needed for easy set up GHE or GH by just specifying url. github_default_url = "https://github.com" -github_settings = Settings.omniauth['providers'].find { |provider| provider["name"] == "github"} +github_settings = Settings.omniauth['providers'].find { |provider| provider["name"] == "github" } if github_settings # For compatibility with old config files (before 7.8) @@ -158,9 +158,9 @@ if github_settings github_settings["args"]["client_options"] = OmniAuth::Strategies::GitHub.default_options[:client_options] else github_settings["args"]["client_options"] = { - "site" => File.join(github_settings["url"], "api/v3"), + "site" => File.join(github_settings["url"], "api/v3"), "authorize_url" => File.join(github_settings["url"], "login/oauth/authorize"), - "token_url" => File.join(github_settings["url"], "login/oauth/access_token") + "token_url" => File.join(github_settings["url"], "login/oauth/access_token") } end end diff --git a/doc/integration/github.md b/doc/integration/github.md index ac17e2069f..e1f9242fd0 100644 --- a/doc/integration/github.md +++ b/doc/integration/github.md @@ -78,7 +78,7 @@ GitHub will generate an application ID and secret key for you to use. args: { scope: 'user:email' } } ``` - __Replace `https://github.example.com/` with your GitHub URL__ + __Replace `https://github.example.com/` with your GitHub URL.__ 1. Change 'YOUR_APP_ID' to the client ID from the GitHub application page from step 7. From 420b95bd4dcffbe87938251bf497382b6437b155 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 27 Apr 2016 09:11:11 +0100 Subject: [PATCH 27/69] Changed settings icon color variable --- app/assets/stylesheets/framework/variables.scss | 1 - app/assets/stylesheets/pages/settings.scss | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 300f941bda..ad54e3ff5a 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -71,7 +71,6 @@ $gl-avatar-size: 40px; $error-exclamation-point: #e62958; $border-radius-default: 2px; $btn-transparent-color: #8f8f8f; -$settings-icon-color: #8f8f8f; $settings-icon-size: 18px; $provider-btn-group-border: #e5e5e5; $provider-btn-not-active-color: #4688f1; diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index 67dbeca7bb..3fb7008571 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -1,5 +1,5 @@ .settings-list-icon { - color: $settings-icon-color; + color: $gl-placeholder-color; font-size: $settings-icon-size; line-height: 42px; } From bdb86ea6cfe16cd3c1e8197744fa3328ba93ecf2 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 18 Apr 2016 09:44:22 +0200 Subject: [PATCH 28/69] Render canceled status if any of the jobs canceled This status will be returned only when there are no failed jobs that are not allowed to fail. --- app/models/concerns/statuseable.rb | 2 +- spec/models/concerns/statuseable_spec.rb | 26 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/app/models/concerns/statuseable.rb b/app/models/concerns/statuseable.rb index 8a293b7b76..3ef91caad4 100644 --- a/app/models/concerns/statuseable.rb +++ b/app/models/concerns/statuseable.rb @@ -18,7 +18,7 @@ module Statuseable WHEN (#{builds})=0 THEN NULL WHEN (#{builds})=(#{success})+(#{ignored}) THEN 'success' WHEN (#{builds})=(#{pending}) THEN 'pending' - WHEN (#{builds})=(#{canceled}) THEN 'canceled' + WHEN (#{builds})=(#{canceled})+(#{success})+(#{ignored}) THEN 'canceled' WHEN (#{builds})=(#{skipped}) THEN 'skipped' WHEN (#{running})+(#{pending})>0 THEN 'running' ELSE 'failed' diff --git a/spec/models/concerns/statuseable_spec.rb b/spec/models/concerns/statuseable_spec.rb index dacbd3034c..8e0a2a2cbd 100644 --- a/spec/models/concerns/statuseable_spec.rb +++ b/spec/models/concerns/statuseable_spec.rb @@ -61,9 +61,35 @@ describe Statuseable do let(:statuses) do [create(type, status: :success), create(type, status: :canceled)] end + + it { is_expected.to eq 'canceled' } + end + + context 'one failed and one canceled' do + let(:statuses) do + [create(type, status: :failed), create(type, status: :canceled)] + end + it { is_expected.to eq 'failed' } end + context 'one failed but allowed to fail and one canceled' do + let(:statuses) do + [create(type, status: :failed, allow_failure: true), + create(type, status: :canceled)] + end + + it { is_expected.to eq 'canceled' } + end + + context 'one running one canceled' do + let(:statuses) do + [create(type, status: :running), create(type, status: :canceled)] + end + + it { is_expected.to eq 'running' } + end + context 'all canceled' do let(:statuses) do [create(type, status: :canceled), create(type, status: :canceled)] From 1cf8723d2ff22b096799c74b7ec0eb6b72c33f0b Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 27 Apr 2016 14:04:59 +0100 Subject: [PATCH 29/69] Updated triggers UI Closes #14090 --- .../framework/tw_bootstrap_variables.scss | 2 +- .../projects/triggers/_trigger.html.haml | 8 +- app/views/projects/triggers/index.html.haml | 119 +++++++++--------- 3 files changed, 63 insertions(+), 66 deletions(-) diff --git a/app/assets/stylesheets/framework/tw_bootstrap_variables.scss b/app/assets/stylesheets/framework/tw_bootstrap_variables.scss index c72af5dad0..d91e35911d 100644 --- a/app/assets/stylesheets/framework/tw_bootstrap_variables.scss +++ b/app/assets/stylesheets/framework/tw_bootstrap_variables.scss @@ -153,7 +153,7 @@ $nav-link-padding: 13px $gl-padding; //== Code // //## -$pre-bg: #f8fafc !default; +$pre-bg: #f4f7fd !default; $pre-color: $gl-gray !default; $pre-border-color: #e7e9ed; diff --git a/app/views/projects/triggers/_trigger.html.haml b/app/views/projects/triggers/_trigger.html.haml index 48b3b5c992..112b51712e 100644 --- a/app/views/projects/triggers/_trigger.html.haml +++ b/app/views/projects/triggers/_trigger.html.haml @@ -1,7 +1,6 @@ %tr %td - .clearfix - %span.monospace= trigger.token + %span.monospace= trigger.token %td - if trigger.last_trigger_request @@ -9,6 +8,5 @@ - else Never - %td - .pull-right - = link_to 'Revoke', namespace_project_trigger_path(@project.namespace, @project, trigger), data: { confirm: 'Are you sure?'}, method: :delete, class: "btn btn-danger btn-sm btn-grouped" + %td.text-right + = link_to 'Revoke', namespace_project_trigger_path(@project.namespace, @project, trigger), data: { confirm: 'Are you sure?'}, method: :delete, class: "btn btn-warning btn-sm" diff --git a/app/views/projects/triggers/index.html.haml b/app/views/projects/triggers/index.html.haml index bd346c4b8e..f91885b216 100644 --- a/app/views/projects/triggers/index.html.haml +++ b/app/views/projects/triggers/index.html.haml @@ -1,71 +1,70 @@ - page_title "Triggers" -%h3.page-title - Triggers -%p.light - Triggers can be used to force a rebuild of a specific branch or tag with an API call. +.row.prepend-top-default.append-bottom-default + .col-lg-3 + %h4.prepend-top-0 + = page_title + %p + Triggers can be used to force a rebuild of a specific branch or tag with an API call. + .col-lg-9 + %h5.prepend-top-0 + Your triggers + - if @triggers.any? + .table-responsive + %table.table + %thead + %th Token + %th Last used + %th + = render partial: 'trigger', collection: @triggers, as: :trigger + - else + %p.profile-settings-message.text-center.append-bottom-default + There are no triggers to use, add one by the button below. -%hr.clearfix + = form_for @trigger, url: url_for(controller: 'projects/triggers', action: 'create') do |f| + = f.submit "Add Trigger", class: 'btn btn-success' --if @triggers.any? - .table-holder - %table.table - %thead - %th Token - %th Last used - %th - = render partial: 'trigger', collection: @triggers, as: :trigger -- else - %h4 No triggers + %h5.prepend-top-default + Use CURL -= form_for @trigger, url: url_for(controller: 'projects/triggers', action: 'create'), html: { class: 'form-horizontal' } do |f| - .clearfix - = f.submit "Add Trigger", class: 'btn btn-success pull-right' - -%hr.clearfix - --if @triggers.any? - %h3 - Use CURL - - %p.light - Copy the token above and set your branch or tag name. This is the reference that will be rebuild. + %p.light + Copy the token above and set your branch or tag name. This is the reference that will be rebuild. - %pre - :plain - curl -X POST \ - -F token=TOKEN \ - -F ref=REF_NAME \ - #{builds_trigger_url(@project.id)} - %h3 - Use .gitlab-ci.yml + %pre + :plain + curl -X POST \ + -F token=TOKEN \ + -F ref=REF_NAME \ + #{builds_trigger_url(@project.id)} + %h5.prepend-top-default + Use .gitlab-ci.yml - %p.light - Copy the snippet to - %i .gitlab-ci.yml - of dependent project. - At the end of your build it will trigger this project to rebuilt. + %p.light + Copy the snippet to + %i .gitlab-ci.yml + of dependent project. + At the end of your build it will trigger this project to rebuilt. - %pre - :plain - trigger: - type: deploy - script: - - "curl -X POST -F token=TOKEN -F ref=REF_NAME #{builds_trigger_url(@project.id)}" - %h3 - Pass build variables + %pre + :plain + trigger: + type: deploy + script: + - "curl -X POST -F token=TOKEN -F ref=REF_NAME #{builds_trigger_url(@project.id)}" + %h5.prepend-top-default + Pass build variables - %p.light - Add - %strong variables[VARIABLE]=VALUE - to API request. - The value of variable could then be used to distinguish triggered build from normal one. + %p.light + Add + %strong variables[VARIABLE]=VALUE + to API request. + The value of variable could then be used to distinguish triggered build from normal one. - %pre - :plain - curl -X POST \ - -F token=TOKEN \ - -F "ref=REF_NAME" \ - -F "variables[RUN_NIGHTLY_BUILD]=true" \ - #{builds_trigger_url(@project.id)} + %pre.append-bottom-0 + :plain + curl -X POST \ + -F token=TOKEN \ + -F "ref=REF_NAME" \ + -F "variables[RUN_NIGHTLY_BUILD]=true" \ + #{builds_trigger_url(@project.id)} From 1d6fdcea95accde7502ace3dc94f79435edfd5c7 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 25 Apr 2016 17:08:10 -0700 Subject: [PATCH 30/69] Disable Rack Attack if admin disables it in config file Grack::Auth already checks this variable. These holdouts were not disabled, leading to confusion when debugging a customer issue. --- config/initializers/rack_attack.rb.example | 3 ++- config/initializers/rack_attack_git_basic_auth.rb | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/config/initializers/rack_attack.rb.example b/config/initializers/rack_attack.rb.example index b1bbcca1d6..30d05f1615 100644 --- a/config/initializers/rack_attack.rb.example +++ b/config/initializers/rack_attack.rb.example @@ -17,8 +17,9 @@ paths_to_be_protected = [ # Create one big regular expression that matches strings starting with any of # the paths_to_be_protected. paths_regex = Regexp.union(paths_to_be_protected.map { |path| /\A#{Regexp.escape(path)}/ }) +rack_attack_enabled = Gitlab.config.rack_attack.git_basic_auth['enabled'] -unless Rails.env.test? +unless Rails.env.test? || !rack_attack_enabled Rack::Attack.throttle('protected paths', limit: 10, period: 60.seconds) do |req| if req.post? && req.path =~ paths_regex req.ip diff --git a/config/initializers/rack_attack_git_basic_auth.rb b/config/initializers/rack_attack_git_basic_auth.rb index bbbfed6832..6a72182617 100644 --- a/config/initializers/rack_attack_git_basic_auth.rb +++ b/config/initializers/rack_attack_git_basic_auth.rb @@ -1,4 +1,6 @@ -unless Rails.env.test? +rack_attack_enabled = Gitlab.config.rack_attack.git_basic_auth['enabled'] + +unless Rails.env.test? || !rack_attack_enabled # Tell the Rack::Attack Rack middleware to maintain an IP blacklist. We will # update the blacklist from Grack::Auth#authenticate_user. Rack::Attack.blacklist('Git HTTP Basic Auth') do |req| From 129c6fbc2d4ce72db8f4dc1ded9b92ce3fcbde50 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Wed, 27 Apr 2016 09:52:20 -0500 Subject: [PATCH 31/69] Add Safari search box bug fix to changelog --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index e52e52691c..ca5b6b4081 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -16,6 +16,7 @@ v 8.7.1 (unreleased) - Use the `can?` helper instead of `current_user.can?`. !3882 - Prevent users from deleting Webhooks via API they do not own - Fix Error 500 due to stale cache when projects are renamed or transferred + - Update width of search box to fix Safari bug. !3900 (Jedidiah) v 8.7.0 - Gitlab::GitAccess and Gitlab::GitAccessWiki are now instrumented From d2f898e7eadc2b4b3d2afb839818c5bc886ce62f Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 27 Apr 2016 14:50:24 -0500 Subject: [PATCH 32/69] Fix Todos test --- spec/features/todos/todos_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/todos/todos_spec.rb b/spec/features/todos/todos_spec.rb index 248e004ba6..3354f52929 100644 --- a/spec/features/todos/todos_spec.rb +++ b/spec/features/todos/todos_spec.rb @@ -68,12 +68,12 @@ describe 'Dashboard Todos', feature: true do describe 'completing last todo from last page', js: true do it 'redirects to the previous page' do visit dashboard_todos_path(page: 2) - expect(page).to have_content(Todo.first.body) + expect(page).to have_css("#todo_#{Todo.last.id}") click_link('Done') expect(current_path).to eq dashboard_todos_path - expect(page).to have_content(Todo.last.body) + expect(page).to have_css("#todo_#{Todo.first.id}") end end end From e99cf05875af4627e532fee77bd22574dde240d7 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 27 Apr 2016 22:04:40 -0700 Subject: [PATCH 33/69] Use ActionDispatch Remote IP for Akismet checking Previously all remote IPs appeared at 127.0.0.1, which made Akismet not very useful. Using the ActionDispatch Remote IP (http://api.rubyonrails.org/classes/ActionDispatch/RemoteIp.html) should provide more reliable results. Closes #16629 --- CHANGELOG | 1 + lib/api/issues.rb | 4 ++-- lib/gitlab/akismet_helper.rb | 12 ++++++++++-- spec/lib/gitlab/akismet_helper_spec.rb | 2 +- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index e52e52691c..6f931eb92f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.8.0 (unreleased) - Remove future dates from contribution calendar graph. + - Use ActionDispatch Remote IP for Akismet checking - Fix error when visiting commit builds page before build was updated - Add 'l' shortcut to open Label dropdown on issuables and 'i' to create new issue on a project - Updated search UI diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 8aa08fd5ac..4092874948 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -24,8 +24,8 @@ module API def create_spam_log(project, current_user, attrs) params = attrs.merge({ - source_ip: env['REMOTE_ADDR'], - user_agent: env['HTTP_USER_AGENT'], + source_ip: client_ip(env), + user_agent: user_agent(env), noteable_type: 'Issue', via_api: true }) diff --git a/lib/gitlab/akismet_helper.rb b/lib/gitlab/akismet_helper.rb index b366c89889..04676fdb74 100644 --- a/lib/gitlab/akismet_helper.rb +++ b/lib/gitlab/akismet_helper.rb @@ -9,14 +9,22 @@ module Gitlab Gitlab.config.gitlab.url) end + def client_ip(env) + env['action_dispatch.remote_ip'].to_s + end + + def user_agent(env) + env['HTTP_USER_AGENT'] + end + def check_for_spam?(project, user) akismet_enabled? && !project.team.member?(user) end def is_spam?(environment, user, text) client = akismet_client - ip_address = environment['REMOTE_ADDR'] - user_agent = environment['HTTP_USER_AGENT'] + ip_address = client_ip(environment) + user_agent = user_agent(environment) params = { type: 'comment', diff --git a/spec/lib/gitlab/akismet_helper_spec.rb b/spec/lib/gitlab/akismet_helper_spec.rb index 9858935180..53f5d6c5c8 100644 --- a/spec/lib/gitlab/akismet_helper_spec.rb +++ b/spec/lib/gitlab/akismet_helper_spec.rb @@ -24,7 +24,7 @@ describe Gitlab::AkismetHelper, type: :helper do describe '#is_spam?' do it 'returns true for spam' do environment = { - 'REMOTE_ADDR' => '127.0.0.1', + 'action_dispatch.remote_ip' => '127.0.0.1', 'HTTP_USER_AGENT' => 'Test User Agent' } From 8b817f594015ad730f426bf6bbf69dab6cc7e563 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Sat, 23 Jan 2016 11:03:16 +0100 Subject: [PATCH 34/69] Users can use the feature proposal label inside the issue [ci skip] --- CONTRIBUTING.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 24cd586453..39b3048899 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -135,8 +135,9 @@ For feature proposals for EE, open an issue on the In order to help track the feature proposals, we have created a [`feature proposal`][fpl] label. For the time being, users that are not members -of the project cannot add labels. You can instead ask one of the [core team][core-team] -members to add the label `feature proposal` to the issue. +of the project cannot add labels. You can instead ask one of the [core team][] +members to add the label `feature proposal` to the issue or add the following +code snippet right after your description in a new line: `~"feature proposal"`. Please keep feature proposals as small and simple as possible, complex ones might be edited to make them small and simple. From 8b09dafb16726168ab78ff425c7c3d7c668ac5a5 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Thu, 28 Apr 2016 13:04:53 +0300 Subject: [PATCH 35/69] Copyedit `using_docker_build.md` and fix links --- doc/ci/docker/using_docker_build.md | 37 +++++++++++++++++++---------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md index bb2a6d1137..5fb086b1dd 100644 --- a/doc/ci/docker/using_docker_build.md +++ b/doc/ci/docker/using_docker_build.md @@ -75,15 +75,19 @@ For more information please checkout [On Docker security: `docker` group conside ## 2. Use docker-in-docker executor -Second approach is to use special Docker image with all tools installed (`docker` and `docker-compose`) and run build script in context of that image in privileged mode. +The second approach is to use the special Docker image with all tools installed +(`docker` and `docker-compose`) and run the build script in context of that +image in privileged mode. + In order to do that follow the steps: 1. Install [GitLab Runner](https://gitlab.com/gitlab-org/gitlab-ci-multi-runner/#installation). -1. Register GitLab Runner from command line to use `docker` and `privileged` mode: +1. Register GitLab Runner from the command line to use `docker` and `privileged` + mode: ```bash - $ sudo gitlab-runner register -n \ + sudo gitlab-runner register -n \ --url https://gitlab.com/ci \ --token RUNNER_TOKEN \ --executor docker \ @@ -92,10 +96,11 @@ In order to do that follow the steps: --docker-privileged ``` - The above command will register a new Runner to use special `docker:latest` image which is provided by Docker - creators. **Notice that it's using the `privileged` mode to start build and service containers.** If you want to use - [docker-in-docker](https://blog.docker.com/2013/09/docker-can-now-run-within-docker/) mode, you always have to use - `privileged = true` in your docker containers. + The above command will register a new Runner to use the special + `docker:latest` image which is provided by Docker. **Notice that it's using + the `privileged` mode to start the build and service containers.** If you + want to use [docker-in-docker] mode, you always have to use `privileged = true` + in your Docker containers. The above command will create a `config.toml` entry similar to this: @@ -114,8 +119,9 @@ In order to do that follow the steps: Insecure = false ``` - If you want to use Shared Runners available on your GitLab CE/EE installation, to build docker images, then - make sure that your Shared Runners configuration have `privileged` mode set to `true`. + If you want to use the Shared Runners available on your GitLab CE/EE + installation in order to build Docker images, then make sure that your + Shared Runners configuration has the `privileged` mode set to `true`. 1. You can now use `docker` from build script: @@ -126,7 +132,7 @@ In order to do that follow the steps: - docker:dind before_script: - - docker info + - docker info build: stage: build @@ -135,9 +141,14 @@ In order to do that follow the steps: - docker run my-docker-image /script/to/run/tests ``` -1. However, by enabling `--docker-privileged` you are effectively disables all security mechanisms of containers and - exposing your host to privilege escalation which can lead to container breakout. +1. However, by enabling `--docker-privileged` you are effectively disabling all + the security mechanisms of containers and exposing your host to privilege + escalation which can lead to container breakout. - For more information, check out [Runtime privilege](https://docs.docker.com/reference/run/#runtime-privilege-linux-capabilities-and-lxc-configuration). + For more information, check out the official Docker documentation on + [Runtime privilege and Linux capabilities][docker-cap]. An example project using this approach can be found here: https://gitlab.com/gitlab-examples/docker. + +[docker-in-docker]: https://blog.docker.com/2013/09/docker-can-now-run-within-docker/ +[docker-cap]: https://docs.docker.com/reference/run/#runtime-privilege-and-linux-capabilities From 4fcf2fb9e779cbc8ce872a15c5c7657471d1ddba Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 28 Apr 2016 11:35:46 +0100 Subject: [PATCH 36/69] Updated spacing between notification label and button Closes #16552 --- app/views/profiles/emails/index.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/profiles/emails/index.html.haml b/app/views/profiles/emails/index.html.haml index 57527361eb..6f7fefdb46 100644 --- a/app/views/profiles/emails/index.html.haml +++ b/app/views/profiles/emails/index.html.haml @@ -45,4 +45,4 @@ %span.label.label-info Public Email - if email.email === current_user.notification_email %span.label.label-info Notification Email - = link_to 'Remove', profile_email_path(email), data: { confirm: 'Are you sure?'}, method: :delete, class: 'btn btn-sm btn-remove pull-right' + = link_to 'Remove', profile_email_path(email), data: { confirm: 'Are you sure?'}, method: :delete, class: 'btn btn-sm btn-warning prepend-left-10' From 7518330aa197930282121ac489a01987c20b6e00 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 19 Apr 2016 12:36:57 +0200 Subject: [PATCH 37/69] Refactor ci commit specs by adding context blocks --- spec/models/ci/commit_spec.rb | 146 ++++++++++++++++++---------------- 1 file changed, 77 insertions(+), 69 deletions(-) diff --git a/spec/models/ci/commit_spec.rb b/spec/models/ci/commit_spec.rb index 82c18aaa01..0bc7722ce6 100644 --- a/spec/models/ci/commit_spec.rb +++ b/spec/models/ci/commit_spec.rb @@ -158,97 +158,105 @@ describe Ci::Commit, models: true do stub_ci_commit_yaml_file(YAML.dump(yaml)) end - it 'properly creates builds' do - expect(create_builds).to be_truthy - expect(commit.builds.pluck(:name)).to contain_exactly('build') - expect(commit.builds.pluck(:status)).to contain_exactly('pending') - commit.builds.running_or_pending.each(&:success) + context 'when builds are successful' do + it 'properly creates builds' do + expect(create_builds).to be_truthy + expect(commit.builds.pluck(:name)).to contain_exactly('build') + expect(commit.builds.pluck(:status)).to contain_exactly('pending') + commit.builds.running_or_pending.each(&:success) - expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending') - commit.builds.running_or_pending.each(&:success) + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending') + commit.builds.running_or_pending.each(&:success) - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending') - expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy') - commit.builds.running_or_pending.each(&:success) + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending') + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy') + commit.builds.running_or_pending.each(&:success) - expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup') - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'pending') - commit.builds.running_or_pending.each(&:success) + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'pending') + commit.builds.running_or_pending.each(&:success) - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'success') - commit.reload - expect(commit.status).to eq('success') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'success') + commit.reload + expect(commit.status).to eq('success') + end end - it 'properly creates builds when test fails' do - expect(create_builds).to be_truthy - expect(commit.builds.pluck(:name)).to contain_exactly('build') - expect(commit.builds.pluck(:status)).to contain_exactly('pending') - commit.builds.running_or_pending.each(&:success) + context 'when test job fails' do + it 'properly creates builds' do + expect(create_builds).to be_truthy + expect(commit.builds.pluck(:name)).to contain_exactly('build') + expect(commit.builds.pluck(:status)).to contain_exactly('pending') + commit.builds.running_or_pending.each(&:success) - expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending') - commit.builds.running_or_pending.each(&:drop) + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending') + commit.builds.running_or_pending.each(&:drop) - expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure') - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending') - commit.builds.running_or_pending.each(&:success) + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending') + commit.builds.running_or_pending.each(&:success) - expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'pending') - commit.builds.running_or_pending.each(&:success) + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'pending') + commit.builds.running_or_pending.each(&:success) - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'success') - commit.reload - expect(commit.status).to eq('failed') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'success') + commit.reload + expect(commit.status).to eq('failed') + end end - it 'properly creates builds when test and test_failure fails' do - expect(create_builds).to be_truthy - expect(commit.builds.pluck(:name)).to contain_exactly('build') - expect(commit.builds.pluck(:status)).to contain_exactly('pending') - commit.builds.running_or_pending.each(&:success) + context 'when test and test_failure jobs fail' do + it 'properly creates builds' do + expect(create_builds).to be_truthy + expect(commit.builds.pluck(:name)).to contain_exactly('build') + expect(commit.builds.pluck(:status)).to contain_exactly('pending') + commit.builds.running_or_pending.each(&:success) - expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending') - commit.builds.running_or_pending.each(&:drop) + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending') + commit.builds.running_or_pending.each(&:drop) - expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure') - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending') - commit.builds.running_or_pending.each(&:drop) + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending') + commit.builds.running_or_pending.each(&:drop) - expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'pending') - commit.builds.running_or_pending.each(&:success) + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'pending') + commit.builds.running_or_pending.each(&:success) - expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'success') - commit.reload - expect(commit.status).to eq('failed') + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'success') + commit.reload + expect(commit.status).to eq('failed') + end end - it 'properly creates builds when deploy fails' do - expect(create_builds).to be_truthy - expect(commit.builds.pluck(:name)).to contain_exactly('build') - expect(commit.builds.pluck(:status)).to contain_exactly('pending') - commit.builds.running_or_pending.each(&:success) + context 'when deploy job fails' do + it 'properly creates builds' do + expect(create_builds).to be_truthy + expect(commit.builds.pluck(:name)).to contain_exactly('build') + expect(commit.builds.pluck(:status)).to contain_exactly('pending') + commit.builds.running_or_pending.each(&:success) - expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending') - commit.builds.running_or_pending.each(&:success) + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending') + commit.builds.running_or_pending.each(&:success) - expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy') - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending') - commit.builds.running_or_pending.each(&:drop) + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending') + commit.builds.running_or_pending.each(&:drop) - expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup') - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'pending') - commit.builds.running_or_pending.each(&:success) + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'pending') + commit.builds.running_or_pending.each(&:success) - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'success') - commit.reload - expect(commit.status).to eq('failed') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'success') + commit.reload + expect(commit.status).to eq('failed') + end end end end From 60492caa98d7da6d1dd04710c9bc3f762bdceebd Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 19 Apr 2016 12:47:34 +0200 Subject: [PATCH 38/69] Add tests exercising builds scheduler after cancel --- spec/models/ci/commit_spec.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/spec/models/ci/commit_spec.rb b/spec/models/ci/commit_spec.rb index 0bc7722ce6..6055c142e2 100644 --- a/spec/models/ci/commit_spec.rb +++ b/spec/models/ci/commit_spec.rb @@ -258,6 +258,24 @@ describe Ci::Commit, models: true do expect(commit.status).to eq('failed') end end + + context 'when build is canceled in the second stage' do + it 'does not schedule builds after build has been canceled' do + expect(create_builds).to be_truthy + expect(commit.builds.pluck(:name)).to contain_exactly('build') + expect(commit.builds.pluck(:status)).to contain_exactly('pending') + commit.builds.running_or_pending.each(&:success) + + expect(commit.builds.running_or_pending).to_not be_empty + + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending') + commit.builds.running_or_pending.each(&:cancel) + + expect(commit.builds.running_or_pending).to be_empty + expect(commit.reload.status).to eq('canceled') + end + end end end From 83883f426e5d00d56db086bceb4d570ee7a3ebe6 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 19 Apr 2016 13:19:20 +0200 Subject: [PATCH 39/69] Add Changelog entry for build status canceled fix --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index e52e52691c..ba40707a58 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.8.0 (unreleased) + - Make build status canceled if any of the jobs was canceled and none failed - Remove future dates from contribution calendar graph. - Fix error when visiting commit builds page before build was updated - Add 'l' shortcut to open Label dropdown on issuables and 'i' to create new issue on a project From da4aaaacd785789cc787d6df481de80d4edd58e6 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Thu, 28 Apr 2016 14:06:28 +0300 Subject: [PATCH 40/69] Fix links to core team page [ci skip] --- CONTRIBUTING.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 39b3048899..084c2d616a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -38,7 +38,7 @@ source edition, and GitLab Enterprise Edition (EE) which is our commercial edition. Throughout this guide you will see references to CE and EE for abbreviation. -If you have read this guide and want to know how the GitLab [core team][core-team] +If you have read this guide and want to know how the GitLab [core team] operates please see [the GitLab contributing process](PROCESS.md). ## Contributor license agreement @@ -135,7 +135,7 @@ For feature proposals for EE, open an issue on the In order to help track the feature proposals, we have created a [`feature proposal`][fpl] label. For the time being, users that are not members -of the project cannot add labels. You can instead ask one of the [core team][] +of the project cannot add labels. You can instead ask one of the [core team] members to add the label `feature proposal` to the issue or add the following code snippet right after your description in a new line: `~"feature proposal"`. @@ -345,8 +345,7 @@ is it will be merged (quickly). After that you can send more MRs to enhance it. For examples of feedback on merge requests please look at already [closed merge requests][closed-merge-requests]. If you would like quick feedback on your merge request feel free to mention one of the Merge Marshalls in the -[core team][core-team] or one of the -[Merge request coaches](https://about.gitlab.com/team/). +[core team] or one of the [Merge request coaches](https://about.gitlab.com/team/). Please ensure that your merge request meets the contribution acceptance criteria. When having your code reviewed and when reviewing merge requests please take the @@ -498,7 +497,7 @@ reported by emailing `contact@gitlab.com`. This Code of Conduct is adapted from the [Contributor Covenant][contributor-covenant], version 1.1.0, available at [http://contributor-covenant.org/version/1/1/0/](http://contributor-covenant.org/version/1/1/0/). -[core-team]: https://about.gitlab.com/core-team/ +[core team]: https://about.gitlab.com/core-team/ [getting-help]: https://about.gitlab.com/getting-help/ [codetriage]: http://www.codetriage.com/gitlabhq/gitlabhq [up-for-grabs]: https://gitlab.com/gitlab-org/gitlab-ce/issues?label_name=up-for-grabs From 40a25732e9c36b50ee7682b4b0ab4e323343ea65 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 28 Apr 2016 15:15:59 +0200 Subject: [PATCH 41/69] Reorder asserts is ci commits specs for consistency --- spec/models/ci/commit_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/ci/commit_spec.rb b/spec/models/ci/commit_spec.rb index 6055c142e2..a747aa0844 100644 --- a/spec/models/ci/commit_spec.rb +++ b/spec/models/ci/commit_spec.rb @@ -169,8 +169,8 @@ describe Ci::Commit, models: true do expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending') commit.builds.running_or_pending.each(&:success) - expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending') expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending') commit.builds.running_or_pending.each(&:success) expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup') From c73bf28143a8fb6adf7322bb301e28b20fb6bee2 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Wed, 20 Jan 2016 10:57:12 +0100 Subject: [PATCH 42/69] Replace gitlab-workhorse with GitLab Workhorse where appropriate --- lib/support/init.d/gitlab | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/support/init.d/gitlab b/lib/support/init.d/gitlab index d95e7023d2..31b00ff128 100755 --- a/lib/support/init.d/gitlab +++ b/lib/support/init.d/gitlab @@ -173,7 +173,7 @@ check_stale_pids(){ fi fi if [ "$hpid" != "0" ] && [ "$gitlab_workhorse_status" != "0" ]; then - echo "Removing stale gitlab-workhorse pid. This is most likely caused by gitlab-workhorse crashing the last time it ran." + echo "Removing stale GitLab Workhorse pid. This is most likely caused by GitLab Workhorse crashing the last time it ran." if ! rm "$gitlab_workhorse_pid_path"; then echo "Unable to remove stale pid, exiting" exit 1 @@ -208,7 +208,7 @@ start_gitlab() { echo "Starting GitLab Sidekiq" fi if [ "$gitlab_workhorse_status" != "0" ]; then - echo "Starting gitlab-workhorse" + echo "Starting GitLab Workhorse" fi if [ "$mail_room_enabled" = true ] && [ "$mail_room_status" != "0" ]; then echo "Starting GitLab MailRoom" @@ -232,7 +232,7 @@ start_gitlab() { fi if [ "$gitlab_workhorse_status" = "0" ]; then - echo "The gitlab-workhorse is already running with pid $spid, not restarting" + echo "The GitLab Workhorse is already running with pid $spid, not restarting" else # No need to remove a socket, gitlab-workhorse does this itself. # Because gitlab-workhorse has multiple executables we need to fix @@ -271,7 +271,7 @@ stop_gitlab() { RAILS_ENV=$RAILS_ENV bin/background_jobs stop fi if [ "$gitlab_workhorse_status" = "0" ]; then - echo "Shutting down gitlab-workhorse" + echo "Shutting down GitLab Workhorse" kill -- $(cat $gitlab_workhorse_pid_path) fi if [ "$mail_room_enabled" = true ] && [ "$mail_room_status" = "0" ]; then @@ -320,9 +320,9 @@ print_status() { printf "The GitLab Sidekiq job dispatcher is \033[31mnot running\033[0m.\n" fi if [ "$gitlab_workhorse_status" = "0" ]; then - echo "The gitlab-workhorse with pid $hpid is running." + echo "The GitLab Workhorse with pid $hpid is running." else - printf "The gitlab-workhorse is \033[31mnot running\033[0m.\n" + printf "The GitLab Workhorse is \033[31mnot running\033[0m.\n" fi if [ "$mail_room_enabled" = true ]; then if [ "$mail_room_status" = "0" ]; then From 435b1c043e8722c0f42d2b248283b950dbd5a357 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 28 Apr 2016 17:03:03 +0200 Subject: [PATCH 43/69] Reuse more popular colors Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/framework/header.scss | 6 +++--- app/assets/stylesheets/framework/nav.scss | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index d14317f9c4..5fa10d29a8 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -26,9 +26,9 @@ header { z-index: 100; margin-bottom: 0; min-height: $header-height; - background-color: $gray-light; + background-color: $background-color; border: none; - border-bottom: 1px solid #eee; + border-bottom: 1px solid $border-color; .container-fluid { width: 100% !important; @@ -47,7 +47,7 @@ header { text-align: center; &:hover, &:focus, &:active { - background-color: $gray-light; + background-color: $background-color; } } diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss index 38bea9d462..5fe687dcec 100644 --- a/app/assets/stylesheets/framework/nav.scss +++ b/app/assets/stylesheets/framework/nav.scss @@ -187,7 +187,7 @@ } .layout-nav { - background: $gray-light; + background: $background-color; border-bottom: 1px solid $border-color; .controls { From a94b71c301c8fa0741ba7b4958c30d04f7f10341 Mon Sep 17 00:00:00 2001 From: connorshea Date: Sun, 10 Apr 2016 14:47:01 -0600 Subject: [PATCH 44/69] Upgrade Doorkeeper from 2.2.2 to 3.1.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I’d rather upgrade one major version at a time, so Doorkeeper needs to be upgraded to 3.x before it can be upgraded to 4.x (which includes Rails 5 support). Changelog: https://github.com/doorkeeper-gem/doorkeeper/blob/master/NEWS.md#310 --- Gemfile | 2 +- Gemfile.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 7882e467f8..25c13fda57 100644 --- a/Gemfile +++ b/Gemfile @@ -19,7 +19,7 @@ gem "pg", '~> 0.18.2', group: :postgres # Authentication libraries gem 'devise', '~> 3.5.4' -gem 'doorkeeper', '~> 2.2.0' +gem 'doorkeeper', '~> 3.1' gem 'omniauth', '~> 1.3.1' gem 'omniauth-auth0', '~> 1.4.1' gem 'omniauth-azure-oauth2', '~> 0.0.6' diff --git a/Gemfile.lock b/Gemfile.lock index 91d89b4875..9c97426cb5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -173,7 +173,7 @@ GEM diff-lcs (1.2.5) diffy (3.0.7) docile (1.1.5) - doorkeeper (2.2.2) + doorkeeper (3.1.0) railties (>= 3.2) dropzonejs-rails (0.7.2) rails (> 3.1) @@ -922,7 +922,7 @@ DEPENDENCIES devise (~> 3.5.4) devise-two-factor (~> 2.0.0) diffy (~> 3.0.3) - doorkeeper (~> 2.2.0) + doorkeeper (~> 3.1) dropzonejs-rails (~> 0.7.1) email_reply_parser (~> 0.5.8) email_spec (~> 1.6.0) From ad99404d2588e182a115d89b0b457d957e8ca7b6 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 14 Apr 2016 12:44:35 +0200 Subject: [PATCH 45/69] Properly handle bigger files --- CHANGELOG | 1 + app/helpers/blob_helper.rb | 4 ++-- app/models/blob.rb | 8 +++++++ app/models/snippet.rb | 4 ++++ app/views/projects/blob/_text.html.haml | 25 +++++++++++++++------- app/views/shared/_file_highlight.html.haml | 5 +++-- lib/gitlab/highlight.rb | 13 +++++++---- 7 files changed, 44 insertions(+), 16 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index dccf218dc9..38e440a72c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -11,6 +11,7 @@ v 8.8.0 (unreleased) - Allow "NEWS" and "CHANGES" as alternative names for CHANGELOG. !3768 (Connor Shea) - Added button to toggle whitespaces changes on diff view - Backport GitLab Enterprise support from EE + - Files over 5MB can only be viewed in their raw form, files over 1MB without highlighting !3718 v 8.7.1 (unreleased) - Throttle the update of `project.last_activity_at` to 1 minute. !3848 diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index a4d7c425d0..474c6f2737 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -3,8 +3,8 @@ module BlobHelper Gitlab::Highlight.new(blob_name, blob_content, nowrap: nowrap) end - def highlight(blob_name, blob_content, nowrap: false) - Gitlab::Highlight.highlight(blob_name, blob_content, nowrap: nowrap) + def highlight(blob_name, blob_content, nowrap: false, plain: false) + Gitlab::Highlight.highlight(blob_name, blob_content, nowrap: nowrap, plain: plain) end def no_highlight_files diff --git a/app/models/blob.rb b/app/models/blob.rb index 72e6c5fa3f..0fea6b7f57 100644 --- a/app/models/blob.rb +++ b/app/models/blob.rb @@ -19,6 +19,14 @@ class Blob < SimpleDelegator new(blob) end + def no_highlighting? + size && size > 1.megabyte + end + + def only_display_raw? + size && size > 5.megabytes + end + def svg? text? && language && language.name == 'SVG' end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index b96e393728..0fd0806192 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -112,6 +112,10 @@ class Snippet < ActiveRecord::Base visibility_level end + def no_highlighting? + content.lines.count > 1000 + end + class << self # Searches for snippets with a matching title or file name. # diff --git a/app/views/projects/blob/_text.html.haml b/app/views/projects/blob/_text.html.haml index d09cd73558..b1769759dc 100644 --- a/app/views/projects/blob/_text.html.haml +++ b/app/views/projects/blob/_text.html.haml @@ -1,10 +1,19 @@ -- blob.load_all_data!(@repository) -- if markup?(blob.name) - .file-content.wiki - = render_markup(blob.name, blob.data) +- if blob.only_display_raw? + .file-content.code + .nothing-here-block + File too large, you can + = succeed '.' do + = link_to 'view the raw file', namespace_project_raw_path(@project.namespace, @project, @id), target: '_blank' + - else - - unless blob.empty? - = render 'shared/file_highlight', blob: blob + - blob.load_all_data!(@repository) + + - if markup?(blob.name) + .file-content.wiki + = render_markup(blob.name, blob.data) - else - .file-content.code - .nothing-here-block Empty file + - if blob.empty? + .file-content.code + .nothing-here-block Empty file + - else + = render 'shared/file_highlight', blob: blob diff --git a/app/views/shared/_file_highlight.html.haml b/app/views/shared/_file_highlight.html.haml index 57856031d6..37dcf39c06 100644 --- a/app/views/shared/_file_highlight.html.haml +++ b/app/views/shared/_file_highlight.html.haml @@ -1,12 +1,13 @@ .file-content.code.js-syntax-highlight .line-numbers - if blob.data.present? + - link_icon = icon('link') - blob.data.each_line.each_with_index do |_, index| - offset = defined?(first_line_number) ? first_line_number : 1 - i = index + offset -# We're not using `link_to` because it is too slow once we get to thousands of lines. %a.diff-line-num{href: "#L#{i}", id: "L#{i}", 'data-line-number' => i} - %i.fa.fa-link + = link_icon = i .blob-content{data: {blob_id: blob.id}} - = highlight(blob.name, blob.data) + = highlight(blob.name, blob.data, plain: blob.no_highlighting?) diff --git a/lib/gitlab/highlight.rb b/lib/gitlab/highlight.rb index cac7644232..280120b0f9 100644 --- a/lib/gitlab/highlight.rb +++ b/lib/gitlab/highlight.rb @@ -1,7 +1,8 @@ module Gitlab class Highlight - def self.highlight(blob_name, blob_content, nowrap: true) - new(blob_name, blob_content, nowrap: nowrap).highlight(blob_content, continue: false) + def self.highlight(blob_name, blob_content, nowrap: true, plain: false) + new(blob_name, blob_content, nowrap: nowrap). + highlight(blob_content, continue: false, plain: plain) end def self.highlight_lines(repository, ref, file_name) @@ -17,8 +18,12 @@ module Gitlab @lexer = Rouge::Lexer.guess(filename: blob_name, source: blob_content).new rescue Rouge::Lexers::PlainText end - def highlight(text, continue: true) - @formatter.format(@lexer.lex(text, continue: continue)).html_safe + def highlight(text, continue: true, plain: false) + if plain + @formatter.format(Rouge::Lexers::PlainText.lex(text)).html_safe + else + @formatter.format(@lexer.lex(text, continue: continue)).html_safe + end rescue @formatter.format(Rouge::Lexers::PlainText.lex(text)).html_safe end From 0426647069397d57d46c66de41c8fa4fad65e0bb Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 21 Apr 2016 16:34:00 +0200 Subject: [PATCH 46/69] Load the "New Branch" button asynchronously This button depends on Issue#can_be_worked_on? which in turn depends on Issue#related_branches. By rendering this button asynchronously we can finally remove all usages of Issue#related_branches and Issue#referenced_merge_requests from the issues "show" page. --- CHANGELOG | 2 ++ app/assets/javascripts/issue.js.coffee | 23 +++++++++++++++++++ app/assets/stylesheets/framework/buttons.scss | 4 ++++ app/controllers/projects/issues_controller.rb | 16 +++++++++++-- .../projects/issues/_new_branch.html.haml | 16 +++++++++---- config/routes.rb | 1 + .../features/issues/new_branch_button_spec.rb | 11 +++++---- 7 files changed, 62 insertions(+), 11 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index dccf218dc9..b41ddc8e57 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -20,6 +20,8 @@ v 8.7.1 (unreleased) - Prevent users from deleting Webhooks via API they do not own - Fix Error 500 due to stale cache when projects are renamed or transferred - Update width of search box to fix Safari bug. !3900 (Jedidiah) + - The "New Branch" button is now loaded asynchronously + - Use the `can?` helper instead of `current_user.can?` v 8.7.0 - Gitlab::GitAccess and Gitlab::GitAccessWiki are now instrumented diff --git a/app/assets/javascripts/issue.js.coffee b/app/assets/javascripts/issue.js.coffee index c7d74a12f9..157361404e 100644 --- a/app/assets/javascripts/issue.js.coffee +++ b/app/assets/javascripts/issue.js.coffee @@ -12,6 +12,7 @@ class @Issue @initMergeRequests() @initRelatedBranches() + @initCanCreateBranch() initTaskList: -> $('.detail-page-description .js-task-list-container').taskList('enable') @@ -92,3 +93,25 @@ class @Issue .success (data) -> if 'html' of data $container.html(data.html) + + initCanCreateBranch: -> + $container = $('div#new-branch') + + # If the user doesn't have the required permissions the container isn't + # rendered at all. + return unless $container + + $.getJSON($container.data('path')) + .error -> + $container.find('.checking').hide() + $container.find('.unavailable').show() + + new Flash('Failed to check if a new branch can be created.', 'alert') + .success (data) -> + if data.can_create_branch + $container.find('.checking').hide() + $container.find('.available').show() + $container.find('a').attr('disabled', false) + else + $container.find('.checking').hide() + $container.find('.unavailable').show() diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss index 18a74fe21a..062da397b6 100644 --- a/app/assets/stylesheets/framework/buttons.scss +++ b/app/assets/stylesheets/framework/buttons.scss @@ -139,6 +139,10 @@ pointer-events: auto !important; } + &[disabled] { + pointer-events: none !important; + } + .caret { margin-left: 5px; } diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 7d4fc361ce..9face235ba 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -3,8 +3,8 @@ class Projects::IssuesController < Projects::ApplicationController include IssuableActions before_action :module_enabled - before_action :issue, - only: [:edit, :update, :show, :referenced_merge_requests, :related_branches] + before_action :issue, only: [:edit, :update, :show, :referenced_merge_requests, + :related_branches, :can_create_branch] # Allow read any issue before_action :authorize_read_issue!, only: [:show] @@ -139,6 +139,18 @@ class Projects::IssuesController < Projects::ApplicationController end end + def can_create_branch + can_create = current_user && + can?(current_user, :push_code, @project) && + @issue.can_be_worked_on?(current_user) + + respond_to do |format| + format.json do + render json: { can_create_branch: can_create } + end + end + end + def bulk_update result = Issues::BulkUpdateService.new(project, current_user, bulk_update_params).execute redirect_back_or_default(default: { action: 'index' }, options: { notice: "#{result[:count]} issues updated" }) diff --git a/app/views/projects/issues/_new_branch.html.haml b/app/views/projects/issues/_new_branch.html.haml index 6da8e4f33a..469429ccf3 100644 --- a/app/views/projects/issues/_new_branch.html.haml +++ b/app/views/projects/issues/_new_branch.html.haml @@ -1,5 +1,13 @@ -- if current_user && can?(current_user, :push_code, @project) && @issue.can_be_worked_on?(current_user) +- if can?(current_user, :push_code, @project) .pull-right - = link_to namespace_project_branches_path(@project.namespace, @project, branch_name: @issue.to_branch_name, issue_iid: @issue.iid), method: :post, class: 'btn has-tooltip', title: @issue.to_branch_name do - = icon('code-fork') - New Branch + #new-branch{'data-path' => can_create_branch_namespace_project_issue_path(@project.namespace, @project, @issue)} + = link_to namespace_project_branches_path(@project.namespace, @project, branch_name: @issue.to_branch_name, issue_iid: @issue.iid), method: :post, class: 'btn has-tooltip', title: @issue.to_branch_name, disabled: 'disabled' do + .checking + %i.fa.fa-spinner.fa-spin + Checking branches + .available(style="display: none") + %i.fa.fa-code-fork + New branch + .unavailable(style="display: none") + %i.fa.fa-exclamation-triangle + New branch unavailable diff --git a/config/routes.rb b/config/routes.rb index d664434e1a..5ce1f49ec6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -710,6 +710,7 @@ Rails.application.routes.draw do post :toggle_subscription get :referenced_merge_requests get :related_branches + get :can_create_branch end collection do post :bulk_update diff --git a/spec/features/issues/new_branch_button_spec.rb b/spec/features/issues/new_branch_button_spec.rb index 9219b76754..16e188d2a8 100644 --- a/spec/features/issues/new_branch_button_spec.rb +++ b/spec/features/issues/new_branch_button_spec.rb @@ -11,10 +11,10 @@ feature 'Start new branch from an issue', feature: true do login_as(user) end - it 'shown the new branch button', js: false do + it 'shows the new branch button', js: true do visit namespace_project_issue_path(project.namespace, project, issue) - expect(page).to have_link "New Branch" + expect(page).to have_css('#new-branch .available') end context "when there is a referenced merge request" do @@ -34,16 +34,17 @@ feature 'Start new branch from an issue', feature: true do end it "hides the new branch button", js: true do - expect(page).not_to have_link "New Branch" + expect(page).not_to have_css('#new-branch .available') expect(page).to have_content /1 Related Merge Request/ end end end context "for visiters" do - it 'no button is shown', js: false do + it 'no button is shown', js: true do visit namespace_project_issue_path(project.namespace, project, issue) - expect(page).not_to have_link "New Branch" + + expect(page).not_to have_css('#new-branch') end end end From 9ef18c6c8bdcdf859845da66d7246872f90fef35 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 28 Apr 2016 22:58:38 +0200 Subject: [PATCH 47/69] Replace outdated bluish colors with default bg color --- app/assets/stylesheets/framework/blocks.scss | 2 +- app/assets/stylesheets/framework/tw_bootstrap_variables.scss | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/framework/blocks.scss b/app/assets/stylesheets/framework/blocks.scss index 62b2af0dbf..e72e4aa47e 100644 --- a/app/assets/stylesheets/framework/blocks.scss +++ b/app/assets/stylesheets/framework/blocks.scss @@ -1,5 +1,5 @@ .light-well { - background-color: #f8fafc; + background-color: $background-color; padding: 15px; } diff --git a/app/assets/stylesheets/framework/tw_bootstrap_variables.scss b/app/assets/stylesheets/framework/tw_bootstrap_variables.scss index d91e35911d..371c1bf17e 100644 --- a/app/assets/stylesheets/framework/tw_bootstrap_variables.scss +++ b/app/assets/stylesheets/framework/tw_bootstrap_variables.scss @@ -153,8 +153,8 @@ $nav-link-padding: 13px $gl-padding; //== Code // //## -$pre-bg: #f4f7fd !default; +$pre-bg: $background-color !default; $pre-color: $gl-gray !default; -$pre-border-color: #e7e9ed; +$pre-border-color: $border-color; $table-bg-accent: $background-color; From 519a791ef9a90541a1bb3825790bd46fd57756f9 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Thu, 28 Apr 2016 15:08:23 -0600 Subject: [PATCH 48/69] Prevent Rails filtered parameters from leaking to Sentry. As described in their Docs: https://docs.getsentry.com/on-premise/clients/ruby/integrations/rails/ --- config/initializers/sentry.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/config/initializers/sentry.rb b/config/initializers/sentry.rb index e87899b2d5..74fef7cadf 100644 --- a/config/initializers/sentry.rb +++ b/config/initializers/sentry.rb @@ -15,6 +15,9 @@ if Rails.env.production? Raven.configure do |config| config.dsn = current_application_settings.sentry_dsn config.release = Gitlab::REVISION + + # Sanitize fields based on those sanitized from Rails. + config.sanitize_fields = Rails.application.config.filter_parameters.map(&:to_s) end end end From c50ce3ba2905ab9f3431467c0118d3cf16fbfea2 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Thu, 28 Apr 2016 21:13:21 -0700 Subject: [PATCH 49/69] Fix the GitHub Omniauth instructions --- doc/integration/github.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/integration/github.md b/doc/integration/github.md index e1f9242fd0..e7497e475c 100644 --- a/doc/integration/github.md +++ b/doc/integration/github.md @@ -9,7 +9,9 @@ GitHub will generate an application ID and secret key for you to use. 1. Navigate to your individual user settings or an organization's settings, depending on how you want the application registered. It does not matter if the application is registered as an individual or an organization - that is entirely up to you. -1. Select "Applications" in the left menu. +1. Select "OAuth applications" in the left menu. + +1. If you already have applications listed, switch to the "Developer applications" tab. 1. Select "Register new application". From ded3b02f3353708386d6960c5c146bf2a2de86db Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Tue, 26 Apr 2016 12:22:52 +0100 Subject: [PATCH 50/69] Use a better message when milestone is newly created make some changes for the checks to determine when the messages should be displayed add item to changelog and also integration test make some changes to the test make some changes --- CHANGELOG | 1 + app/models/concerns/milestoneish.rb | 2 +- app/views/projects/milestones/show.html.haml | 7 ++-- spec/features/milestone_spec.rb | 35 ++++++++++++++++++++ 4 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 spec/features/milestone_spec.rb diff --git a/CHANGELOG b/CHANGELOG index d453ae19e7..bba0941573 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -7,6 +7,7 @@ v 8.8.0 (unreleased) - Fix error when visiting commit builds page before build was updated - Add 'l' shortcut to open Label dropdown on issuables and 'i' to create new issue on a project - Updated search UI + - Display informative message when new milestone is created - Replace Devise Async with Devise ActiveJob integration. !3902 (Connor Shea) - Allow "NEWS" and "CHANGES" as alternative names for CHANGELOG. !3768 (Connor Shea) - Added button to toggle whitespaces changes on diff view diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb index 5b8e3f654e..7bcc78247b 100644 --- a/app/models/concerns/milestoneish.rb +++ b/app/models/concerns/milestoneish.rb @@ -8,7 +8,7 @@ module Milestoneish end def complete?(user = nil) - total_items_count(user) == closed_items_count(user) + total_items_count(user) > 0 && total_items_count(user) == closed_items_count(user) end def percent_complete(user = nil) diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml index be63875ab3..56543ccd06 100644 --- a/app/views/projects/milestones/show.html.haml +++ b/app/views/projects/milestones/show.html.haml @@ -42,9 +42,12 @@ = preserve do = markdown @milestone.description -- if @milestone.complete?(current_user) && @milestone.active? +- if @milestone.total_items_count(current_user).zero? .alert.alert-success.prepend-top-default - %span All issues for this milestone are closed. You may close milestone now. + %span Assign some issues to this milestone. +- elsif @milestone.complete?(current_user) && @milestone.active? + .alert.alert-success.prepend-top-default + %span All issues for this milestone are closed. You may close this milestone now. = render 'shared/milestones/summary', milestone: @milestone, project: @project = render 'shared/milestones/tabs', milestone: @milestone diff --git a/spec/features/milestone_spec.rb b/spec/features/milestone_spec.rb new file mode 100644 index 0000000000..c2c7acff3e --- /dev/null +++ b/spec/features/milestone_spec.rb @@ -0,0 +1,35 @@ +require 'rails_helper' + +feature 'Milestone', feature: true do + include WaitForAjax + + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + let(:milestone) { create(:milestone, project: project, title: 8.7) } + + before do + project.team << [user, :master] + login_as(user) + end + + feature 'Create a milestone' do + scenario 'should show an informative message for a new issue' do + visit new_namespace_project_milestone_path(project.namespace, project) + page.within '.milestone-form' do + fill_in "milestone_title", with: '8.7' + end + find('input[name="commit"]').click + + expect(find('.alert-success')).to have_content('Assign some issues to this milestone.') + end + end + + feature 'Open a milestone with closed issues' do + scenario 'should show an informative message' do + create(:issue, title: "Bugfix1", project: project, milestone: milestone, state: "closed") + visit namespace_project_milestone_path(project.namespace, project, milestone) + + expect(find('.alert-success')).to have_content('All issues for this milestone are closed. You may close this milestone now.') + end + end +end From 0ea9e477229830c7fb82bd6beaaf8fcdd9d0129e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 29 Apr 2016 12:21:20 +0200 Subject: [PATCH 51/69] Remove deprecated CI badge helper Closes #13446 --- app/helpers/ci_badge_helper.rb | 13 ------------- 1 file changed, 13 deletions(-) delete mode 100644 app/helpers/ci_badge_helper.rb diff --git a/app/helpers/ci_badge_helper.rb b/app/helpers/ci_badge_helper.rb deleted file mode 100644 index 27386133e3..0000000000 --- a/app/helpers/ci_badge_helper.rb +++ /dev/null @@ -1,13 +0,0 @@ -module CiBadgeHelper - def markdown_badge_code(project, ref) - url = status_ci_project_url(project, ref: ref, format: 'png') - link = namespace_project_commits_path(project.namespace, project, ref) - "[![build status](#{url})](#{link})" - end - - def html_badge_code(project, ref) - url = status_ci_project_url(project, ref: ref, format: 'png') - link = namespace_project_commits_path(project.namespace, project, ref) - "" - end -end From 39ed7c18e9881416a7ca8b689a3e1fdde91f12c2 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Fri, 29 Apr 2016 12:44:23 +0200 Subject: [PATCH 52/69] Fixed CHANGELOG for 8.7.1/8.7.2 [ci skip] --- CHANGELOG | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index d453ae19e7..569185fa2a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -13,7 +13,10 @@ v 8.8.0 (unreleased) - Backport GitLab Enterprise support from EE - Files over 5MB can only be viewed in their raw form, files over 1MB without highlighting !3718 -v 8.7.1 (unreleased) +v 8.7.2 + - The "New Branch" button is now loaded asynchronously + +v 8.7.1 - Throttle the update of `project.last_activity_at` to 1 minute. !3848 - Fix .gitlab-ci.yml parsing issue when hidde job is a template without script definition. !3849 - Fix license detection to detect all license files, not only known licenses. !3878 @@ -21,7 +24,6 @@ v 8.7.1 (unreleased) - Prevent users from deleting Webhooks via API they do not own - Fix Error 500 due to stale cache when projects are renamed or transferred - Update width of search box to fix Safari bug. !3900 (Jedidiah) - - The "New Branch" button is now loaded asynchronously - Use the `can?` helper instead of `current_user.can?` v 8.7.0 From 28dcdb27795de79337df7aeff1107114cdffc2f9 Mon Sep 17 00:00:00 2001 From: Matt Oakes Date: Mon, 14 Mar 2016 23:33:00 +0000 Subject: [PATCH 53/69] Support supressing text file diffs on the default branch with .gitattributes This is a combination of 3 commits. - Update the bare repositories info/attributes if the default branch is updated - Check the diff attributes of a file before showing a diff - Update CHANGELOG --- CHANGELOG | 1 + Gemfile.lock | 6 ++--- app/models/project.rb | 1 + app/models/repository.rb | 10 ++++++++ app/services/git_push_service.rb | 9 +++++++ app/views/projects/diffs/_file.html.haml | 26 ++++++++++---------- spec/models/repository_spec.rb | 10 ++++++++ spec/services/git_push_service_spec.rb | 30 ++++++++++++++++++++++++ 8 files changed, 77 insertions(+), 16 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index d453ae19e7..370b2856b4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -12,6 +12,7 @@ v 8.8.0 (unreleased) - Added button to toggle whitespaces changes on diff view - Backport GitLab Enterprise support from EE - Files over 5MB can only be viewed in their raw form, files over 1MB without highlighting !3718 + - Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes) v 8.7.1 (unreleased) - Throttle the update of `project.last_activity_at` to 1 minute. !3848 diff --git a/Gemfile.lock b/Gemfile.lock index 91d89b4875..1dcda0daff 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -184,7 +184,7 @@ GEM encryptor (1.3.0) equalizer (0.0.11) erubis (2.7.0) - escape_utils (1.1.0) + escape_utils (1.1.1) eventmachine (1.0.8) excon (0.45.4) execjs (2.6.0) @@ -334,7 +334,7 @@ GEM json get_process_mem (0.2.0) gherkin-ruby (0.3.2) - github-linguist (4.7.5) + github-linguist (4.7.6) charlock_holmes (~> 0.7.3) escape_utils (~> 1.1.0) mime-types (>= 1.19) @@ -351,7 +351,7 @@ GEM posix-spawn (~> 0.3) gitlab_emoji (0.3.1) gemojione (~> 2.2, >= 2.2.1) - gitlab_git (10.0.0) + gitlab_git (10.0.1) activesupport (~> 4.0) charlock_holmes (~> 0.7.3) github-linguist (~> 4.7.0) diff --git a/app/models/project.rb b/app/models/project.rb index 0420c6a61a..5c6c36e6b3 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -901,6 +901,7 @@ class Project < ActiveRecord::Base repository.rugged.references.create('HEAD', "refs/heads/#{branch}", force: true) + repository.copy_gitattributes(branch) reload_default_branch end diff --git a/app/models/repository.rb b/app/models/repository.rb index d495c8d18f..b4319297e4 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -938,6 +938,16 @@ class Repository raw_repository.ls_files(actual_ref) end + def copy_gitattributes(ref) + actual_ref = ref || root_ref + begin + raw_repository.copy_gitattributes(actual_ref) + true + rescue Gitlab::Git::Repository::InvalidRef + false + end + end + def main_language return if empty? || rugged.head_unborn? diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 1e1be8cd04..b7af80055b 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -42,7 +42,12 @@ class GitPushService < BaseService # Collect data for this git push @push_commits = @project.repository.commits_between(params[:oldrev], params[:newrev]) process_commit_messages + + # Update the bare repositories info/attributes file using the contents of the default branches + # .gitattributes file + update_gitattributes if is_default_branch? end + # Update merge requests that may be affected by this push. A new branch # could cause the last commit of a merge request to change. update_merge_requests @@ -54,6 +59,10 @@ class GitPushService < BaseService perform_housekeeping end + def update_gitattributes + @project.repository.copy_gitattributes(params[:ref]) + end + def update_main_language # Performance can be bad so for now only check main_language once # See https://gitlab.com/gitlab-org/gitlab-ce/issues/14937 diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index 83a8d7ae9b..0f04fc5d33 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -40,19 +40,19 @@ = view_file_btn(diff_commit.id, diff_file, project) .diff-content.diff-wrap-lines - -# Skipp all non non-supported blobs + - # Skip all non non-supported blobs - return unless blob.respond_to?('text?') - if diff_file.too_large? - .nothing-here-block - This diff could not be displayed because it is too large. - - else - - if blob_text_viewable?(blob) - - if diff_view == 'parallel' - = render "projects/diffs/parallel_view", diff_file: diff_file, project: project, blob: blob, index: i - - else - = render "projects/diffs/text_file", diff_file: diff_file, index: i - - elsif blob.image? - - old_file = project.repository.prev_blob_for_diff(diff_commit, diff_file) - = render "projects/diffs/image", diff_file: diff_file, old_file: old_file, file: blob, index: i, diff_refs: diff_refs + .nothing-here-block This diff could not be displayed because it is too large. + - elsif blob_text_viewable?(blob) && !project.repository.diffable?(blob) + .nothing-here-block This diff was suppressed by a .gitattributes entry. + - elsif blob_text_viewable?(blob) + - if diff_view == 'parallel' + = render "projects/diffs/parallel_view", diff_file: diff_file, project: project, blob: blob, index: i - else - .nothing-here-block No preview for this file type + = render "projects/diffs/text_file", diff_file: diff_file, index: i + - elsif blob.image? + - old_file = project.repository.prev_blob_for_diff(diff_commit, diff_file) + = render "projects/diffs/image", diff_file: diff_file, old_file: old_file, file: blob, index: i, diff_refs: diff_refs + - else + .nothing-here-block No preview for this file type diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index a306cc4aef..397bb5a802 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -795,6 +795,16 @@ describe Repository, models: true do end + describe "#copy_gitattributes" do + it 'returns true with a valid ref' do + expect(repository.copy_gitattributes('master')).to be_truthy + end + + it 'returns false with an invalid ref' do + expect(repository.copy_gitattributes('invalid')).to be_falsey + end + end + describe "#main_language" do it 'shows the main language of the project' do expect(repository.main_language).to eq("Ruby") diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index b40a5c1c81..eeab540c2f 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -201,6 +201,36 @@ describe GitPushService, services: true do end + describe "Updates git attributes" do + context "for default branch" do + it "calls the copy attributes method for the first push to the default branch" do + expect(project.repository).to receive(:copy_gitattributes).with('master') + + execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master') + end + + it "calls the copy attributes method for changes to the default branch" do + expect(project.repository).to receive(:copy_gitattributes).with('refs/heads/master') + + execute_service(project, user, 'oldrev', 'newrev', 'refs/heads/master') + end + end + + context "for non-default branch" do + before do + # Make sure the "default" branch is different + allow(project).to receive(:default_branch).and_return('not-master') + end + + it "does not call copy attributes method" do + expect(project.repository).not_to receive(:copy_gitattributes) + + execute_service(project, user, @oldrev, @newrev, @ref) + end + end + end + + describe "Webhooks" do context "execute webhooks" do it "when pushing a branch for the first time" do From 3811eb0ba1e8c0f318060696853b1cab7f99dfa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 26 Apr 2016 12:54:49 +0200 Subject: [PATCH 54/69] Fix error when trying to create a wiki page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #15527. Signed-off-by: Rémy Coutable --- CHANGELOG | 3 +- app/controllers/projects/wikis_controller.rb | 4 +- app/services/wiki_pages/create_service.rb | 3 +- .../wiki/user_creates_wiki_page_spec.rb | 87 +++++++++++++++++++ .../wiki/user_updates_wiki_page_spec.rb | 46 ++++++++++ 5 files changed, 139 insertions(+), 4 deletions(-) create mode 100644 spec/features/projects/wiki/user_creates_wiki_page_spec.rb create mode 100644 spec/features/projects/wiki/user_updates_wiki_page_spec.rb diff --git a/CHANGELOG b/CHANGELOG index ed151df9e2..bcbcdfc3fc 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,8 +15,9 @@ v 8.8.0 (unreleased) - Files over 5MB can only be viewed in their raw form, files over 1MB without highlighting !3718 - Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes) -v 8.7.2 +v 8.7.2 (unreleased) - The "New Branch" button is now loaded asynchronously + - Fix error 500 when trying to create a wiki page v 8.7.1 - Throttle the update of `project.last_activity_at` to 1 minute. !3848 diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index c02bc28ace..0d6c32fabd 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -40,10 +40,10 @@ class Projects::WikisController < Projects::ApplicationController end def update - @page = @project_wiki.find_page(params[:id]) - return render('empty') unless can?(current_user, :create_wiki, @project) + @page = @project_wiki.find_page(params[:id]) + if @page = WikiPages::UpdateService.new(@project, current_user, wiki_params).execute(@page) redirect_to( namespace_project_wiki_path(@project.namespace, @project, @page), diff --git a/app/services/wiki_pages/create_service.rb b/app/services/wiki_pages/create_service.rb index 988c663b9d..24a817c06c 100644 --- a/app/services/wiki_pages/create_service.rb +++ b/app/services/wiki_pages/create_service.rb @@ -1,7 +1,8 @@ module WikiPages class CreateService < WikiPages::BaseService def execute - page = WikiPage.new(@project.wiki) + project_wiki = ProjectWiki.new(@project, current_user) + page = WikiPage.new(project_wiki) if page.create(@params) execute_hooks(page, 'create') diff --git a/spec/features/projects/wiki/user_creates_wiki_page_spec.rb b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb new file mode 100644 index 0000000000..d709fe8046 --- /dev/null +++ b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb @@ -0,0 +1,87 @@ +require 'spec_helper' + +feature 'Projects > Wiki > User creates wiki page', feature: true do + let(:user) { create(:user) } + + background do + project.team << [user, :master] + login_as(user) + + visit namespace_project_path(project.namespace, project) + click_link 'Wiki' + end + + context 'wiki project is in the user namespace' do + let(:project) { create(:project, namespace: user.namespace) } + + context 'when wiki is empty' do + scenario 'user can create a new wiki page from the wiki home page' do + expect(page).to have_content('Home · Edit Page') + + fill_in :wiki_content, with: 'My awesome wiki!' + click_button 'Create page' + + expect(page).to have_content("Home · last edited by #{user.name}") + expect(page).to have_content('My awesome wiki!') + end + end + + context 'when wiki is not empty' do + before do + WikiPages::CreateService.new(project, user, title: 'home', content: 'Home page').execute + end + + scenario 'user can create a new wiki page', js: true do + click_link 'New Page' + + fill_in :new_wiki_path, with: 'foo' + click_button 'Create Page' + + expect(page).to have_content('Foo · Edit Page') + + fill_in :wiki_content, with: 'My awesome wiki!' + click_button 'Create page' + + expect(page).to have_content("Foo · last edited by #{user.name}") + expect(page).to have_content('My awesome wiki!') + end + end + end + + context 'wiki project is in the user namespace' do + let(:project) { create(:project, namespace: create(:group, :public)) } + + context 'when wiki is empty' do + scenario 'user can create a new wiki page from the wiki home page' do + expect(page).to have_content('Home · Edit Page') + + fill_in :wiki_content, with: 'My awesome wiki!' + click_button 'Create page' + + expect(page).to have_content("Home · last edited by #{user.name}") + expect(page).to have_content('My awesome wiki!') + end + end + + context 'when wiki is not empty' do + before do + WikiPages::CreateService.new(project, user, title: 'home', content: 'Home page').execute + end + + scenario 'user can create a new wiki page', js: true do + click_link 'New Page' + + fill_in :new_wiki_path, with: 'foo' + click_button 'Create Page' + + expect(page).to have_content('Foo · Edit Page') + + fill_in :wiki_content, with: 'My awesome wiki!' + click_button 'Create page' + + expect(page).to have_content("Foo · last edited by #{user.name}") + expect(page).to have_content('My awesome wiki!') + end + end + end +end diff --git a/spec/features/projects/wiki/user_updates_wiki_page_spec.rb b/spec/features/projects/wiki/user_updates_wiki_page_spec.rb new file mode 100644 index 0000000000..b69d6b46ac --- /dev/null +++ b/spec/features/projects/wiki/user_updates_wiki_page_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +feature 'Projects > Wiki > User updates wiki page', feature: true do + let(:user) { create(:user) } + + background do + project.team << [user, :master] + login_as(user) + + visit namespace_project_path(project.namespace, project) + WikiPages::CreateService.new(project, user, title: 'home', content: 'Home page').execute + click_link 'Wiki' + end + + context 'wiki project is in the user namespace' do + let(:project) { create(:project, namespace: user.namespace) } + + scenario 'user can update the wiki home page' do + click_link 'Edit' + + expect(page).to have_content('Home · Edit Page') + + fill_in :wiki_content, with: 'My awesome wiki!' + click_button 'Save changes' + + expect(page).to have_content("Home · last edited by #{user.name}") + expect(page).to have_content('My awesome wiki!') + end + end + + context 'wiki project is in the user namespace' do + let(:project) { create(:project, namespace: create(:group, :public)) } + + scenario 'user can update the wiki home page' do + click_link 'Edit' + + expect(page).to have_content('Home · Edit Page') + + fill_in :wiki_content, with: 'My awesome wiki!' + click_button 'Save changes' + + expect(page).to have_content("Home · last edited by #{user.name}") + expect(page).to have_content('My awesome wiki!') + end + end +end From b2ec176539dd7caf8ae463776175e00c80199470 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 28 Apr 2016 10:32:32 +0100 Subject: [PATCH 55/69] Removes duplicates from the label dropdown It concats the duplicate labels into a single label in the dropdown with the color being a gradient of the differnet colors being concatted. Closes #16555 --- .../javascripts/labels_select.js.coffee | 40 ++++++++++++++++++- spec/features/dashboard/label_filter_spec.rb | 29 ++++++++++++++ 2 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 spec/features/dashboard/label_filter_spec.rb diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 7d61cd9bf7..995fd76860 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -163,6 +163,21 @@ class @LabelsSelect $.ajax( url: labelUrl ).done (data) -> + data = _.chain data + .groupBy (label) -> + label.title + .map (label) -> + color = _.map label, (dup) -> + dup.color + + return { + id: label[0].id + title: label[0].title + color: color + duplicate: color.length > 1 + } + .value() + if $dropdown.hasClass 'js-extra-options' if showNo data.unshift( @@ -178,6 +193,7 @@ class @LabelsSelect if data.length > 2 data.splice 2, 0, 'divider' + callback data renderRow: (label) -> @@ -192,11 +208,31 @@ class @LabelsSelect if $dropdown.hasClass('js-multiselect') and removesAll selectedClass.push 'dropdown-clear-active' - color = if label.color? then "" else "" + if label.duplicate + spacing = 100 / label.color.length + + # Reduce the colors to 4 + label.color = label.color.filter (color, i) -> + i < 4 + + color = _.map(label.color, (color, i) -> + percentFirst = Math.floor(spacing * i) + percentSecond = Math.floor(spacing * (i + 1)) + "#{color} #{percentFirst}%,#{color} #{percentSecond}% " + ).join(',') + color = "linear-gradient(#{color})" + else + if label.color? + color = label.color[0] + + if color + colorEl = "" + else + colorEl = '' "
              • - #{color} + #{colorEl} #{_.escape(label.title)}
              • " diff --git a/spec/features/dashboard/label_filter_spec.rb b/spec/features/dashboard/label_filter_spec.rb new file mode 100644 index 0000000000..24e83d4401 --- /dev/null +++ b/spec/features/dashboard/label_filter_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe 'Dashboard > label filter', feature: true, js: true do + let(:user) { create(:user) } + let(:project) { create(:project, name: 'test', namespace: user.namespace) } + let(:project2) { create(:project, name: 'test2', path: 'test2', namespace: user.namespace) } + let(:label) { create(:label, title: 'bug', color: '#ff0000') } + let(:label2) { create(:label, title: 'bug') } + + before do + project.labels << label + project2.labels << label2 + + login_as(user) + visit issues_dashboard_path + end + + context 'duplicate labels' do + it 'should remove duplicate labels' do + page.within('.labels-filter') do + click_button 'Label' + end + + page.within('.dropdown-menu-labels') do + expect(page).to have_selector('.dropdown-content a', text: 'bug', count: 1) + end + end + end +end From 6e5f0fbad47c28d3cf33fd52296e278ed4625d5a Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Fri, 29 Apr 2016 12:08:53 -0400 Subject: [PATCH 56/69] Update CHANGELOG --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index ed151df9e2..5bd897126b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -14,6 +14,7 @@ v 8.8.0 (unreleased) - Backport GitLab Enterprise support from EE - Files over 5MB can only be viewed in their raw form, files over 1MB without highlighting !3718 - Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes) + - Added multiple colors for labels in dropdowns when dups happen. v 8.7.2 - The "New Branch" button is now loaded asynchronously From f3f820d0bd579bdaba19cf6043aa5fadd4216b98 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 29 Apr 2016 19:02:30 +0200 Subject: [PATCH 57/69] Move modal css to separate file and fix danger text for confirmation modal Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/framework.scss | 1 + app/assets/stylesheets/framework/modal.scss | 22 +++++++++++++++++++++ app/assets/stylesheets/pages/help.scss | 19 ------------------ app/views/shared/_confirm_modal.html.haml | 2 +- 4 files changed, 24 insertions(+), 20 deletions(-) create mode 100644 app/assets/stylesheets/framework/modal.scss diff --git a/app/assets/stylesheets/framework.scss b/app/assets/stylesheets/framework.scss index c85ab9148d..560de9fc0b 100644 --- a/app/assets/stylesheets/framework.scss +++ b/app/assets/stylesheets/framework.scss @@ -25,6 +25,7 @@ @import "framework/lists.scss"; @import "framework/markdown_area.scss"; @import "framework/mobile.scss"; +@import "framework/modal.scss"; @import "framework/nav.scss"; @import "framework/pagination.scss"; @import "framework/progress.scss"; diff --git a/app/assets/stylesheets/framework/modal.scss b/app/assets/stylesheets/framework/modal.scss new file mode 100644 index 0000000000..26ad2870aa --- /dev/null +++ b/app/assets/stylesheets/framework/modal.scss @@ -0,0 +1,22 @@ +.modal-body { + position: relative; + overflow-y: auto; + padding: 15px; + + .form-actions { + margin: -$gl-padding+1; + margin-top: 15px; + } + + .text-danger { + font-weight: bold; + } +} + +body.modal-open { + overflow: hidden; +} + +.modal .modal-dialog { + width: 860px; +} diff --git a/app/assets/stylesheets/pages/help.scss b/app/assets/stylesheets/pages/help.scss index ee95bdf488..4a95b7b852 100644 --- a/app/assets/stylesheets/pages/help.scss +++ b/app/assets/stylesheets/pages/help.scss @@ -55,25 +55,6 @@ } } -.modal-body { - position: relative; - overflow-y: auto; - padding: 15px; - - .form-actions { - margin: -$gl-padding+1; - margin-top: 15px; - } -} - -body.modal-open { - overflow: hidden; -} - -.modal .modal-dialog { - width: 860px; -} - .documentation { padding: 7px; } diff --git a/app/views/shared/_confirm_modal.html.haml b/app/views/shared/_confirm_modal.html.haml index 34241cd8aa..b0fc60573f 100644 --- a/app/views/shared/_confirm_modal.html.haml +++ b/app/views/shared/_confirm_modal.html.haml @@ -7,7 +7,7 @@ Confirmation required .modal-body - %p.cred.lead.js-confirm-text + %p.text-danger.js-confirm-text %p This action can lead to data loss. From f49d86cd5c680ba233e4cb46c51748d83e2f562e Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 29 Apr 2016 13:11:23 -0600 Subject: [PATCH 58/69] Fix some broken links in the documentation [ci skip] --- doc/monitoring/performance/gitlab_configuration.md | 2 +- doc/monitoring/performance/influxdb_configuration.md | 2 +- doc/monitoring/performance/influxdb_schema.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/monitoring/performance/gitlab_configuration.md b/doc/monitoring/performance/gitlab_configuration.md index 90e9930221..771584268d 100644 --- a/doc/monitoring/performance/gitlab_configuration.md +++ b/doc/monitoring/performance/gitlab_configuration.md @@ -37,4 +37,4 @@ Read more on: - [Introduction to GitLab Performance Monitoring](introduction.md) - [InfluxDB Configuration](influxdb_configuration.md) - [InfluxDB Schema](influxdb_schema.md) -- [Grafana Install/Configuration](grafana_configuration.md +- [Grafana Install/Configuration](grafana_configuration.md) diff --git a/doc/monitoring/performance/influxdb_configuration.md b/doc/monitoring/performance/influxdb_configuration.md index 63aa03985e..c30cd2950d 100644 --- a/doc/monitoring/performance/influxdb_configuration.md +++ b/doc/monitoring/performance/influxdb_configuration.md @@ -181,7 +181,7 @@ Read more on: - [Introduction to GitLab Performance Monitoring](introduction.md) - [GitLab Configuration](gitlab_configuration.md) - [InfluxDB Schema](influxdb_schema.md) -- [Grafana Install/Configuration](grafana_configuration.md +- [Grafana Install/Configuration](grafana_configuration.md) [influxdb-retention]: https://docs.influxdata.com/influxdb/v0.9/query_language/database_management/#retention-policy-management [influxdb documentation]: https://docs.influxdata.com/influxdb/v0.9/ diff --git a/doc/monitoring/performance/influxdb_schema.md b/doc/monitoring/performance/influxdb_schema.md index d31b3788f3..41861860b6 100644 --- a/doc/monitoring/performance/influxdb_schema.md +++ b/doc/monitoring/performance/influxdb_schema.md @@ -85,4 +85,4 @@ Read more on: - [Introduction to GitLab Performance Monitoring](introduction.md) - [GitLab Configuration](gitlab_configuration.md) - [InfluxDB Configuration](influxdb_configuration.md) -- [Grafana Install/Configuration](grafana_configuration.md +- [Grafana Install/Configuration](grafana_configuration.md) From e28d1fa3cc2e8c2f696f6740c8a8441100542470 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Fri, 29 Apr 2016 22:21:06 +0200 Subject: [PATCH 59/69] Use a query in Project#protected_branch? This changes Project#protected_branch? to use a query to check if a branch is protected, instead of loading all ProtectedBranch records into memory just to check if the list of names includes a given branch name. --- app/models/project.rb | 2 +- spec/models/project_spec.rb | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/models/project.rb b/app/models/project.rb index 5c6c36e6b3..bbc929e9bd 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -764,7 +764,7 @@ class Project < ActiveRecord::Base # Check if current branch name is marked as protected in the system def protected_branch?(branch_name) - protected_branches_names.include?(branch_name) + protected_branches.where(name: branch_name).any? end def developers_can_push_to_protected_branch?(branch_name) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e33c7d62ff..5b1cf71337 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -798,4 +798,18 @@ describe Project, models: true do end end end + + describe '#protected_branch?' do + let(:project) { create(:empty_project) } + + it 'returns true when a branch is a protected branch' do + project.protected_branches.create!(name: 'foo') + + expect(project.protected_branch?('foo')).to eq(true) + end + + it 'returns false when a branch is not a protected branch' do + expect(project.protected_branch?('foo')).to eq(false) + end + end end From b6a18f1093c88e82d2320d4d3f3c15c549b861be Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Fri, 29 Apr 2016 22:33:56 +0200 Subject: [PATCH 60/69] Tweak checking branches in Project#open_branches This changes 4 things: 1. Project#protected_branches_names has been renamed to Project#protected_branch_names. 2. Project#open_branches uses a Set for the branch names as checking values in a Set is faster than checking values in a (large) Array. 3. Some redundant code in Project#open_branches has been removed. 4. Project#protected_branch_names now uses #pluck instead of #map, removing the need for loading entire DB records into memory. --- CHANGELOG | 1 + app/models/project.rb | 16 +++++++--------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 5bd897126b..121b03be32 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.8.0 (unreleased) + - Project#open_branches has been cleaned up and no longer loads entire records into memory. - Make build status canceled if any of the jobs was canceled and none failed - Remove future dates from contribution calendar graph. - Use ActionDispatch Remote IP for Akismet checking diff --git a/app/models/project.rb b/app/models/project.rb index bbc929e9bd..af62e8ecd9 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -735,19 +735,17 @@ class Project < ActiveRecord::Base end def open_branches - all_branches = repository.branches + # We're using a Set here as checking values in a large Set is faster than + # checking values in a large Array. + protected_set = Set.new(protected_branch_names) - if protected_branches.present? - all_branches.reject! do |branch| - protected_branches_names.include?(branch.name) - end + repository.branches.reject do |branch| + protected_set.include?(branch.name) end - - all_branches end - def protected_branches_names - @protected_branches_names ||= protected_branches.map(&:name) + def protected_branch_names + @protected_branch_names ||= protected_branches.pluck(:name) end def root_ref?(branch) From 8d3debe4cc964d830a2f21a7e950f143cc9008ca Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Thu, 28 Apr 2016 12:12:03 -0600 Subject: [PATCH 61/69] Add more parameters to the filter_parameters config. Adds Sentry DSN, Webhooks, Deploy Keys, etc. Alphabetized the parameters and included line breaks between each parameter. Easier to merge into EE if there are any differences. This also seems to be the more popular syntax for adding new parameters, from what I can find. --- config/application.rb | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index 2e2ed48db0..b602e2b616 100644 --- a/config/application.rb +++ b/config/application.rb @@ -32,7 +32,30 @@ module Gitlab config.encoding = "utf-8" # Configure sensitive parameters which will be filtered from the log file. - config.filter_parameters.push(:password, :password_confirmation, :private_token, :otp_attempt, :variables, :import_url) + # + # Parameters filtered: + # - Password (:password, :password_confirmation) + # - Private tokens (:private_token) + # - Two-factor tokens (:otp_attempt) + # - Repo/Project Import URLs (:import_url) + # - Build variables (:variables) + # - GitLab Pages SSL cert/key info (:certificate, :encrypted_key) + # - Webhook URLs (:hook) + # - Sentry DSN (:sentry_dsn) + # - Deploy keys (:key) + config.filter_parameters += %i( + certificate + encrypted_key + hook + import_url + key + otp_attempt + password + password_confirmation + private_token + sentry_dsn + variables + ) # Enable escaping HTML in JSON. config.active_support.escape_html_entities_in_json = true From 0cd5edf35cfaca74344dd389aadd65f0f179d395 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 27 Apr 2016 04:24:49 -0300 Subject: [PATCH 62/69] Backported minimal safewebhook implementation to GitLab CE --- app/models/hooks/project_hook.rb | 1 + app/models/hooks/service_hook.rb | 1 + app/models/hooks/system_hook.rb | 1 + app/models/hooks/web_hook.rb | 24 ++++++---- .../20160413115152_add_token_to_web_hooks.rb | 5 ++ db/schema.rb | 1 + spec/factories/project_hooks.rb | 4 ++ spec/models/hooks/web_hook_spec.rb | 46 ++++++++++++------- 8 files changed, 58 insertions(+), 25 deletions(-) create mode 100644 db/migrate/20160413115152_add_token_to_web_hooks.rb diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb index bc6e0f98c3..d149511b86 100644 --- a/app/models/hooks/project_hook.rb +++ b/app/models/hooks/project_hook.rb @@ -16,6 +16,7 @@ # note_events :boolean default(FALSE), not null # enable_ssl_verification :boolean default(TRUE) # build_events :boolean default(FALSE), not null +# token :string # class ProjectHook < WebHook diff --git a/app/models/hooks/service_hook.rb b/app/models/hooks/service_hook.rb index 80962264ba..f45145eeb3 100644 --- a/app/models/hooks/service_hook.rb +++ b/app/models/hooks/service_hook.rb @@ -16,6 +16,7 @@ # note_events :boolean default(FALSE), not null # enable_ssl_verification :boolean default(TRUE) # build_events :boolean default(FALSE), not null +# token :string # class ServiceHook < WebHook diff --git a/app/models/hooks/system_hook.rb b/app/models/hooks/system_hook.rb index 15dddcc244..012cc8ec00 100644 --- a/app/models/hooks/system_hook.rb +++ b/app/models/hooks/system_hook.rb @@ -16,6 +16,7 @@ # note_events :boolean default(FALSE), not null # enable_ssl_verification :boolean default(TRUE) # build_events :boolean default(FALSE), not null +# token :string # class SystemHook < WebHook diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index 3a2e4f546f..1e3b481559 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -16,6 +16,7 @@ # note_events :boolean default(FALSE), not null # enable_ssl_verification :boolean default(TRUE) # build_events :boolean default(FALSE), not null +# token :string # class WebHook < ActiveRecord::Base @@ -43,23 +44,17 @@ class WebHook < ActiveRecord::Base if parsed_url.userinfo.blank? response = WebHook.post(url, body: data.to_json, - headers: { - "Content-Type" => "application/json", - "X-Gitlab-Event" => hook_name.singularize.titleize - }, + headers: build_headers(hook_name), verify: enable_ssl_verification) else - post_url = url.gsub("#{parsed_url.userinfo}@", "") + post_url = url.gsub("#{parsed_url.userinfo}@", '') auth = { username: CGI.unescape(parsed_url.user), password: CGI.unescape(parsed_url.password), } response = WebHook.post(post_url, body: data.to_json, - headers: { - "Content-Type" => "application/json", - "X-Gitlab-Event" => hook_name.singularize.titleize - }, + headers: build_headers(hook_name), verify: enable_ssl_verification, basic_auth: auth) end @@ -73,4 +68,15 @@ class WebHook < ActiveRecord::Base def async_execute(data, hook_name) Sidekiq::Client.enqueue(ProjectWebHookWorker, id, data, hook_name) end + + private + + def build_headers(hook_name) + headers = { + 'Content-Type' => 'application/json', + 'X-Gitlab-Event' => hook_name.singularize.titleize + } + headers['X-Gitlab-Token'] = token if token.present? + headers + end end diff --git a/db/migrate/20160413115152_add_token_to_web_hooks.rb b/db/migrate/20160413115152_add_token_to_web_hooks.rb new file mode 100644 index 0000000000..f04225068c --- /dev/null +++ b/db/migrate/20160413115152_add_token_to_web_hooks.rb @@ -0,0 +1,5 @@ +class AddTokenToWebHooks < ActiveRecord::Migration + def change + add_column :web_hooks, :token, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 42457d9235..04aee737e4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1025,6 +1025,7 @@ ActiveRecord::Schema.define(version: 20160421130527) do t.boolean "enable_ssl_verification", default: true t.boolean "build_events", default: false, null: false t.boolean "wiki_page_events", default: false, null: false + t.string "token" end add_index "web_hooks", ["created_at", "id"], name: "index_web_hooks_on_created_at_and_id", using: :btree diff --git a/spec/factories/project_hooks.rb b/spec/factories/project_hooks.rb index 94dd935a03..3195fb3ddc 100644 --- a/spec/factories/project_hooks.rb +++ b/spec/factories/project_hooks.rb @@ -1,5 +1,9 @@ FactoryGirl.define do factory :project_hook do url { FFaker::Internet.uri('http') } + + trait :token do + token { SecureRandom.hex(10) } + end end end diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index 04bc2dcfb1..37a27d73aa 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -43,51 +43,65 @@ describe WebHook, models: true do end describe "execute" do + let(:project) { create(:project) } + let(:project_hook) { create(:project_hook) } + before(:each) do - @project_hook = create(:project_hook) - @project = create(:project) - @project.hooks << [@project_hook] + project.hooks << [project_hook] @data = { before: 'oldrev', after: 'newrev', ref: 'ref' } - WebMock.stub_request(:post, @project_hook.url) + WebMock.stub_request(:post, project_hook.url) + end + + context 'when token is defined' do + let(:project_hook) { create(:project_hook, :token) } + + it 'POSTs to the webhook URL' do + project_hook.execute(@data, 'push_hooks') + expect(WebMock).to have_requested(:post, project_hook.url).with( + headers: { 'Content-Type' => 'application/json', + 'X-Gitlab-Event' => 'Push Hook', + 'X-Gitlab-Token' => project_hook.token } + ).once + end end it "POSTs to the webhook URL" do - @project_hook.execute(@data, 'push_hooks') - expect(WebMock).to have_requested(:post, @project_hook.url).with( - headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Push Hook' } + project_hook.execute(@data, 'push_hooks') + expect(WebMock).to have_requested(:post, project_hook.url).with( + headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'Push Hook' } ).once end it "POSTs the data as JSON" do - @project_hook.execute(@data, 'push_hooks') - expect(WebMock).to have_requested(:post, @project_hook.url).with( - headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Push Hook' } + project_hook.execute(@data, 'push_hooks') + expect(WebMock).to have_requested(:post, project_hook.url).with( + headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'Push Hook' } ).once end it "catches exceptions" do expect(WebHook).to receive(:post).and_raise("Some HTTP Post error") - expect { @project_hook.execute(@data, 'push_hooks') }.to raise_error(RuntimeError) + expect { project_hook.execute(@data, 'push_hooks') }.to raise_error(RuntimeError) end it "handles SSL exceptions" do expect(WebHook).to receive(:post).and_raise(OpenSSL::SSL::SSLError.new('SSL error')) - expect(@project_hook.execute(@data, 'push_hooks')).to eq([false, 'SSL error']) + expect(project_hook.execute(@data, 'push_hooks')).to eq([false, 'SSL error']) end it "handles 200 status code" do - WebMock.stub_request(:post, @project_hook.url).to_return(status: 200, body: "Success") + WebMock.stub_request(:post, project_hook.url).to_return(status: 200, body: "Success") - expect(@project_hook.execute(@data, 'push_hooks')).to eq([true, 'Success']) + expect(project_hook.execute(@data, 'push_hooks')).to eq([true, 'Success']) end it "handles 2xx status codes" do - WebMock.stub_request(:post, @project_hook.url).to_return(status: 201, body: "Success") + WebMock.stub_request(:post, project_hook.url).to_return(status: 201, body: "Success") - expect(@project_hook.execute(@data, 'push_hooks')).to eq([true, 'Success']) + expect(project_hook.execute(@data, 'push_hooks')).to eq([true, 'Success']) end end end From c9577711cee5f9eec699711d39196480f400a746 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 30 Apr 2016 21:14:40 +0200 Subject: [PATCH 63/69] Handle issue move access instead of raising error Closes #15533 --- app/controllers/projects/issues_controller.rb | 2 + .../projects/issues_controller_spec.rb | 39 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 9face235ba..016f5dd000 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -96,6 +96,8 @@ class Projects::IssuesController < Projects::ApplicationController if params[:move_to_project_id].to_i > 0 new_project = Project.find(params[:move_to_project_id]) + return render_404 unless issue.can_move?(current_user, new_project) + move_service = Issues::MoveService.new(project, current_user) @issue = move_service.execute(@issue, new_project) end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index d6e4cd71ce..2b2ad3b941 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -40,6 +40,45 @@ describe Projects::IssuesController do end end + describe 'PUT #update' do + context 'when moving issue to another private project' do + let(:another_project) { create(:project, :private) } + + before do + sign_in(user) + project.team << [user, :developer] + end + + context 'when user has access to move issue' do + before { another_project.team << [user, :reporter] } + + it 'moves issue to another project' do + move_issue + + expect(response).to have_http_status :found + expect(another_project.issues).to_not be_empty + end + end + + context 'when user does not have access to move issue' do + it 'responds with 404' do + move_issue + + expect(response).to have_http_status :not_found + end + end + + def move_issue + put :update, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: issue.iid, + issue: { title: 'New title' }, + move_to_project_id: another_project.id + end + end + end + describe 'Confidential Issues' do let(:project) { create(:project_empty_repo, :public) } let(:assignee) { create(:assignee) } From 00ced598ea65f1b957c43576bc1564ed3f67d749 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Sat, 30 Apr 2016 05:34:31 -0300 Subject: [PATCH 64/69] Added UI to define secret_token for webhook and systemhook Codestyle changes to easy EE merge --- app/controllers/admin/hooks_controller.rb | 8 +++++++- app/controllers/projects/hooks_controller.rb | 14 +++++++++++--- app/views/admin/hooks/index.html.haml | 10 ++++++++-- app/views/projects/hooks/index.html.haml | 5 +++++ 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/hooks_controller.rb b/app/controllers/admin/hooks_controller.rb index 93c4894ea0..4e85b6b4cf 100644 --- a/app/controllers/admin/hooks_controller.rb +++ b/app/controllers/admin/hooks_controller.rb @@ -39,6 +39,12 @@ class Admin::HooksController < Admin::ApplicationController end def hook_params - params.require(:hook).permit(:url, :enable_ssl_verification, :push_events, :tag_push_events) + params.require(:hook).permit( + :enable_ssl_verification, + :push_events, + :tag_push_events, + :token, + :url + ) end end diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb index 5fd4f855de..dfa9bd259e 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/app/controllers/projects/hooks_controller.rb @@ -52,8 +52,16 @@ class Projects::HooksController < Projects::ApplicationController end def hook_params - params.require(:hook).permit(:url, :push_events, :issues_events, - :merge_requests_events, :tag_push_events, :note_events, - :build_events, :enable_ssl_verification) + params.require(:hook).permit( + :build_events, + :enable_ssl_verification, + :issues_events, + :merge_requests_events, + :note_events, + :push_events, + :tag_push_events, + :token, + :url + ) end end diff --git a/app/views/admin/hooks/index.html.haml b/app/views/admin/hooks/index.html.haml index 67d23c8023..7b388cf786 100644 --- a/app/views/admin/hooks/index.html.haml +++ b/app/views/admin/hooks/index.html.haml @@ -13,9 +13,15 @@ = form_errors(@hook) .form-group - = f.label :url, "URL:", class: 'control-label' + = f.label :url, 'URL', class: 'control-label' .col-sm-10 - = f.text_field :url, class: "form-control" + = f.text_field :url, class: 'form-control' + .form-group + = f.label :token, 'Secret Token', class: 'control-label' + .col-sm-10 + = f.text_field :token, class: 'form-control' + %p.help-block + Use this token to validate received payloads .form-group = f.label :url, "Trigger", class: 'control-label' .col-sm-10.prepend-top-10 diff --git a/app/views/projects/hooks/index.html.haml b/app/views/projects/hooks/index.html.haml index 6f1ee20943..36c1d69f06 100644 --- a/app/views/projects/hooks/index.html.haml +++ b/app/views/projects/hooks/index.html.haml @@ -15,6 +15,11 @@ .form-group = f.label :url, "URL", class: "label-light" = f.text_field :url, class: "form-control", placeholder: "http://example.com/trigger-ci.json" + .form-group + = f.label :token, "Secret Token", class: 'label-light' + = f.text_field :token, class: "form-control", placeholder: '' + %p.help-block + Use this token to validate received payloads .form-group = f.label :url, "Trigger", class: "label-light" %div From 1dfd051ec58e5c1a324d3b5493552289a44f1aa0 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 2 May 2016 14:32:26 +0000 Subject: [PATCH 65/69] Remove duplicate entry in the CHANGELOG --- CHANGELOG | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 121b03be32..f1f93a3cb0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -140,7 +140,6 @@ v 8.7.0 - Import GitHub labels - Add option to filter by "Owned projects" on dashboard page - Import GitHub milestones - - Fix emoji catgories in the emoji picker - Execute system web hooks on push to the project - Allow enable/disable push events for system hooks - Fix GitHub project's link in the import page when provider has a custom URL From dc0ff9a17472c71d2e2f5510797976f9ce7cea23 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Mon, 2 May 2016 17:22:03 +0200 Subject: [PATCH 66/69] Fix "remember me" sign in option --- config/initializers/session_store.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/session_store.rb b/config/initializers/session_store.rb index 88cb859871..599dabb9e5 100644 --- a/config/initializers/session_store.rb +++ b/config/initializers/session_store.rb @@ -22,7 +22,7 @@ else key: '_gitlab_session', secure: Gitlab.config.gitlab.https, httponly: true, - expire_after: Settings.gitlab['session_expire_delay'] * 60, + expires_in: Settings.gitlab['session_expire_delay'] * 60, path: (Rails.application.config.relative_url_root.nil?) ? '/' : Gitlab::Application.config.relative_url_root ) end From f1e74de47cd9fcf334617023aaa737c83c98a7f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 2 May 2016 17:49:51 +0200 Subject: [PATCH 67/69] Simplify specs by not over-expecting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- CHANGELOG | 2 +- .../wiki/user_creates_wiki_page_spec.rb | 32 ++++++++----------- .../wiki/user_updates_wiki_page_spec.rb | 18 +++++------ 3 files changed, 23 insertions(+), 29 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index bcbcdfc3fc..efaa5f43d5 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,7 +15,7 @@ v 8.8.0 (unreleased) - Files over 5MB can only be viewed in their raw form, files over 1MB without highlighting !3718 - Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes) -v 8.7.2 (unreleased) +v 8.7.2 - The "New Branch" button is now loaded asynchronously - Fix error 500 when trying to create a wiki page diff --git a/spec/features/projects/wiki/user_creates_wiki_page_spec.rb b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb index d709fe8046..7e6eef6587 100644 --- a/spec/features/projects/wiki/user_creates_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb @@ -11,17 +11,16 @@ feature 'Projects > Wiki > User creates wiki page', feature: true do click_link 'Wiki' end - context 'wiki project is in the user namespace' do + context 'in the user namespace' do let(:project) { create(:project, namespace: user.namespace) } context 'when wiki is empty' do - scenario 'user can create a new wiki page from the wiki home page' do - expect(page).to have_content('Home · Edit Page') - + scenario 'directly from the wiki home page' do fill_in :wiki_content, with: 'My awesome wiki!' click_button 'Create page' - expect(page).to have_content("Home · last edited by #{user.name}") + expect(page).to have_content('Home') + expect(page).to have_content("last edited by #{user.name}") expect(page).to have_content('My awesome wiki!') end end @@ -31,34 +30,32 @@ feature 'Projects > Wiki > User creates wiki page', feature: true do WikiPages::CreateService.new(project, user, title: 'home', content: 'Home page').execute end - scenario 'user can create a new wiki page', js: true do + scenario 'via the "new wiki page" page', js: true do click_link 'New Page' fill_in :new_wiki_path, with: 'foo' click_button 'Create Page' - expect(page).to have_content('Foo · Edit Page') - fill_in :wiki_content, with: 'My awesome wiki!' click_button 'Create page' - expect(page).to have_content("Foo · last edited by #{user.name}") + expect(page).to have_content('Foo') + expect(page).to have_content("last edited by #{user.name}") expect(page).to have_content('My awesome wiki!') end end end - context 'wiki project is in the user namespace' do + context 'in a group namespace' do let(:project) { create(:project, namespace: create(:group, :public)) } context 'when wiki is empty' do - scenario 'user can create a new wiki page from the wiki home page' do - expect(page).to have_content('Home · Edit Page') - + scenario 'directly from the wiki home page' do fill_in :wiki_content, with: 'My awesome wiki!' click_button 'Create page' - expect(page).to have_content("Home · last edited by #{user.name}") + expect(page).to have_content('Home') + expect(page).to have_content("last edited by #{user.name}") expect(page).to have_content('My awesome wiki!') end end @@ -68,18 +65,17 @@ feature 'Projects > Wiki > User creates wiki page', feature: true do WikiPages::CreateService.new(project, user, title: 'home', content: 'Home page').execute end - scenario 'user can create a new wiki page', js: true do + scenario 'via the "new wiki page" page', js: true do click_link 'New Page' fill_in :new_wiki_path, with: 'foo' click_button 'Create Page' - expect(page).to have_content('Foo · Edit Page') - fill_in :wiki_content, with: 'My awesome wiki!' click_button 'Create page' - expect(page).to have_content("Foo · last edited by #{user.name}") + expect(page).to have_content('Foo') + expect(page).to have_content("last edited by #{user.name}") expect(page).to have_content('My awesome wiki!') end end diff --git a/spec/features/projects/wiki/user_updates_wiki_page_spec.rb b/spec/features/projects/wiki/user_updates_wiki_page_spec.rb index b69d6b46ac..ef82d2375d 100644 --- a/spec/features/projects/wiki/user_updates_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_updates_wiki_page_spec.rb @@ -12,34 +12,32 @@ feature 'Projects > Wiki > User updates wiki page', feature: true do click_link 'Wiki' end - context 'wiki project is in the user namespace' do + context 'in the user namespace' do let(:project) { create(:project, namespace: user.namespace) } - scenario 'user can update the wiki home page' do + scenario 'the home page' do click_link 'Edit' - expect(page).to have_content('Home · Edit Page') - fill_in :wiki_content, with: 'My awesome wiki!' click_button 'Save changes' - expect(page).to have_content("Home · last edited by #{user.name}") + expect(page).to have_content('Home') + expect(page).to have_content("last edited by #{user.name}") expect(page).to have_content('My awesome wiki!') end end - context 'wiki project is in the user namespace' do + context 'in a group namespace' do let(:project) { create(:project, namespace: create(:group, :public)) } - scenario 'user can update the wiki home page' do + scenario 'the home page' do click_link 'Edit' - expect(page).to have_content('Home · Edit Page') - fill_in :wiki_content, with: 'My awesome wiki!' click_button 'Save changes' - expect(page).to have_content("Home · last edited by #{user.name}") + expect(page).to have_content('Home') + expect(page).to have_content("last edited by #{user.name}") expect(page).to have_content('My awesome wiki!') end end From f64b82e1da47e0f0b4e0f6e91746ed95a98105dd Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 29 Apr 2016 16:02:28 -0700 Subject: [PATCH 68/69] Support e-mail notifications for comments on project snippets Closes #2334 --- CHANGELOG | 1 + app/mailers/emails/notes.rb | 8 +++ app/models/project_snippet.rb | 2 + app/views/notify/note_snippet_email.html.haml | 1 + app/views/notify/note_snippet_email.text.erb | 8 +++ spec/services/notification_service_spec.rb | 65 ++++++++++++++----- 6 files changed, 69 insertions(+), 16 deletions(-) create mode 100644 app/views/notify/note_snippet_email.html.haml create mode 100644 app/views/notify/note_snippet_email.text.erb diff --git a/CHANGELOG b/CHANGELOG index 87aafb939e..3dfa074eae 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,7 @@ v 8.8.0 (unreleased) - Project#open_branches has been cleaned up and no longer loads entire records into memory. - Make build status canceled if any of the jobs was canceled and none failed - Remove future dates from contribution calendar graph. + - Support e-mail notifications for comments on project snippets - Use ActionDispatch Remote IP for Akismet checking - Fix error when visiting commit builds page before build was updated - Add 'l' shortcut to open Label dropdown on issuables and 'i' to create new issue on a project diff --git a/app/mailers/emails/notes.rb b/app/mailers/emails/notes.rb index cdc40b81ee..96116e916d 100644 --- a/app/mailers/emails/notes.rb +++ b/app/mailers/emails/notes.rb @@ -28,6 +28,14 @@ module Emails mail_answer_thread(@merge_request, note_thread_options(recipient_id)) end + def note_snippet_email(recipient_id, note_id) + setup_note_mail(note_id, recipient_id) + + @snippet = @note.noteable + @target_url = namespace_project_snippet_url(*note_target_url_options) + mail_answer_thread(@snippet, note_thread_options(recipient_id)) + end + private def note_target_url_options diff --git a/app/models/project_snippet.rb b/app/models/project_snippet.rb index 1f7d85a5f3..d48f054615 100644 --- a/app/models/project_snippet.rb +++ b/app/models/project_snippet.rb @@ -22,4 +22,6 @@ class ProjectSnippet < Snippet # Scopes scope :fresh, -> { order("created_at DESC") } + + participant :author, :notes end diff --git a/app/views/notify/note_snippet_email.html.haml b/app/views/notify/note_snippet_email.html.haml new file mode 100644 index 0000000000..2fa2f78466 --- /dev/null +++ b/app/views/notify/note_snippet_email.html.haml @@ -0,0 +1 @@ += render 'note_message' diff --git a/app/views/notify/note_snippet_email.text.erb b/app/views/notify/note_snippet_email.text.erb new file mode 100644 index 0000000000..4d5a406f4b --- /dev/null +++ b/app/views/notify/note_snippet_email.text.erb @@ -0,0 +1,8 @@ +New comment for Snippet <%= @snippet.id %> + +<%= url_for(namespace_project_snippet_url(@snippet.project.namespace, @snippet.project, @snippet, anchor: "note_#{@note.id}")) %> + + +Author: <%= @note.author_name %> + +<%= @note.note %> diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index d7c72dc081..4bbc4ddc3a 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -10,7 +10,7 @@ describe NotificationService, services: true do end describe 'Keys' do - describe :new_key do + describe '#new_key' do let!(:key) { create(:personal_key) } it { expect(notification.new_key(key)).to be_truthy } @@ -22,7 +22,7 @@ describe NotificationService, services: true do end describe 'Email' do - describe :new_email do + describe '#new_email' do let!(:email) { create(:email) } it { expect(notification.new_email(email)).to be_truthy } @@ -147,8 +147,8 @@ describe NotificationService, services: true do ActionMailer::Base.deliveries.clear end - describe :new_note do - it do + describe '#new_note' do + it 'notifies the team members' do notification.new_note(note) # Notify all team members @@ -177,6 +177,39 @@ describe NotificationService, services: true do end end + context 'project snippet note' do + let(:project) { create(:empty_project, :public) } + let(:snippet) { create(:project_snippet, project: project, author: create(:user)) } + let(:note) { create(:note_on_project_snippet, noteable: snippet, project_id: snippet.project.id, note: '@all mentioned') } + + before do + build_team(note.project) + note.project.team << [note.author, :master] + ActionMailer::Base.deliveries.clear + end + + describe '#new_note' do + it 'notifies the team members' do + notification.new_note(note) + + # Notify all team members + note.project.team.members.each do |member| + # User with disabled notification should not be notified + next if member.id == @u_disabled.id + # Author should not be notified + next if member.id == note.author.id + should_email(member) + end + + should_email(note.noteable.author) + should_not_email(note.author) + should_email(@u_mentioned) + should_not_email(@u_disabled) + should_email(@u_not_mentioned) + end + end + end + context 'commit note' do let(:project) { create(:project, :public) } let(:note) { create(:note_on_commit, project: project) } @@ -187,7 +220,7 @@ describe NotificationService, services: true do allow_any_instance_of(Commit).to receive(:author).and_return(@u_committer) end - describe :new_note, :perform_enqueued_jobs do + describe '#new_note, #perform_enqueued_jobs' do it do notification.new_note(note) @@ -230,7 +263,7 @@ describe NotificationService, services: true do ActionMailer::Base.deliveries.clear end - describe :new_issue do + describe '#new_issue' do it do notification.new_issue(issue, @u_disabled) @@ -289,7 +322,7 @@ describe NotificationService, services: true do end end - describe :reassigned_issue do + describe '#reassigned_issue' do it 'emails new assignee' do notification.reassigned_issue(issue, @u_disabled) @@ -419,7 +452,7 @@ describe NotificationService, services: true do end end - describe :close_issue do + describe '#close_issue' do it 'should sent email to issue assignee and issue author' do notification.close_issue(issue, @u_disabled) @@ -435,7 +468,7 @@ describe NotificationService, services: true do end end - describe :reopen_issue do + describe '#reopen_issue' do it 'should send email to issue assignee and issue author' do notification.reopen_issue(issue, @u_disabled) @@ -461,7 +494,7 @@ describe NotificationService, services: true do ActionMailer::Base.deliveries.clear end - describe :new_merge_request do + describe '#new_merge_request' do it do notification.new_merge_request(merge_request, @u_disabled) @@ -483,7 +516,7 @@ describe NotificationService, services: true do end end - describe :reassigned_merge_request do + describe '#reassigned_merge_request' do it do notification.reassigned_merge_request(merge_request, merge_request.author) @@ -498,7 +531,7 @@ describe NotificationService, services: true do end end - describe :relabel_merge_request do + describe '#relabel_merge_request' do let(:label) { create(:label, merge_requests: [merge_request]) } let(:label2) { create(:label) } let!(:subscriber_to_label) { create(:user).tap { |u| label.toggle_subscription(u) } } @@ -527,7 +560,7 @@ describe NotificationService, services: true do end end - describe :closed_merge_request do + describe '#closed_merge_request' do it do notification.close_mr(merge_request, @u_disabled) @@ -542,7 +575,7 @@ describe NotificationService, services: true do end end - describe :merged_merge_request do + describe '#merged_merge_request' do it do notification.merge_mr(merge_request, @u_disabled) @@ -557,7 +590,7 @@ describe NotificationService, services: true do end end - describe :reopen_merge_request do + describe '#reopen_merge_request' do it do notification.reopen_mr(merge_request, @u_disabled) @@ -581,7 +614,7 @@ describe NotificationService, services: true do ActionMailer::Base.deliveries.clear end - describe :project_was_moved do + describe '#project_was_moved' do it do notification.project_was_moved(project, "gitlab/gitlab") From f0c4f727359a5848d12e2097bad6a6a3190943ef Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 2 May 2016 21:47:11 -0400 Subject: [PATCH 69/69] Update CHANGELOG for 8.6.8, 8.5.12, et al. [ci skip] --- CHANGELOG | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 3dfa074eae..b6527780bb 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -18,7 +18,7 @@ v 8.8.0 (unreleased) - Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes) - Added multiple colors for labels in dropdowns when dups happen. -v 8.7.2 +v 8.7.2 (unreleased) - The "New Branch" button is now loaded asynchronously - Fix error 500 when trying to create a wiki page @@ -148,6 +148,19 @@ v 8.7.0 - Add RAW build trace output and button on build page - Add incremental build trace update into CI API +v 8.6.8 + - Prevent privilege escalation via "impersonate" feature + - Prevent privilege escalation via notes API + - Prevent privilege escalation via project webhook API + - Prevent XSS via Git branch and tag names + - Prevent XSS via custom issue tracker URL + - Prevent XSS via `window.opener` + - Prevent XSS via label drop-down + - Prevent information disclosure via milestone API + - Prevent information disclosure via snippet API + - Prevent information disclosure via project labels + - Prevent information disclosure via new merge request page + v 8.6.7 - Fix persistent XSS vulnerability in `commit_person_link` helper - Fix persistent XSS vulnerability in Label and Milestone dropdowns @@ -289,6 +302,17 @@ v 8.6.0 - Trigger a todo for mentions on commits page - Let project owners and admins soft delete issues and merge requests +v 8.5.12 + - Prevent privilege escalation via "impersonate" feature + - Prevent privilege escalation via notes API + - Prevent privilege escalation via project webhook API + - Prevent XSS via Git branch and tag names + - Prevent XSS via custom issue tracker URL + - Prevent XSS via `window.opener` + - Prevent information disclosure via snippet API + - Prevent information disclosure via project labels + - Prevent information disclosure via new merge request page + v 8.5.11 - Fix persistent XSS vulnerability in `commit_person_link` helper @@ -439,6 +463,17 @@ v 8.5.0 - Show label row when filtering issues or merge requests by label (Nuttanart Pornprasitsakul) - Add Todos +v 8.4.10 + - Prevent privilege escalation via "impersonate" feature + - Prevent privilege escalation via notes API + - Prevent privilege escalation via project webhook API + - Prevent XSS via Git branch and tag names + - Prevent XSS via custom issue tracker URL + - Prevent XSS via `window.opener` + - Prevent information disclosure via snippet API + - Prevent information disclosure via project labels + - Prevent information disclosure via new merge request page + v 8.4.9 - Fix persistent XSS vulnerability in `commit_person_link` helper @@ -564,6 +599,15 @@ v 8.4.0 - Add IP check against DNSBLs at account sign-up - Added cache:key to .gitlab-ci.yml allowing to fine tune the caching +v 8.3.9 + - Prevent privilege escalation via "impersonate" feature + - Prevent privilege escalation via notes API + - Prevent privilege escalation via project webhook API + - Prevent XSS via custom issue tracker URL + - Prevent XSS via `window.opener` + - Prevent information disclosure via project labels + - Prevent information disclosure via new merge request page + v 8.3.8 - Fix persistent XSS vulnerability in `commit_person_link` helper @@ -673,6 +717,17 @@ v 8.3.0 - Expose Git's version in the admin area - Show "New Merge Request" buttons on canonical repos when you have a fork (Josh Frye) +v 8.2.5 + - Prevent privilege escalation via "impersonate" feature + - Prevent privilege escalation via notes API + - Prevent privilege escalation via project webhook API + - Prevent XSS via `window.opener` + - Prevent information disclosure via project labels + - Prevent information disclosure via new merge request page + +v 8.2.4 + - Bump Git version requirement to 2.7.4 + v 8.2.3 - Fix application settings cache not expiring after changes (Stan Hu) - Fix Error 500s when creating global milestones with Unicode characters (Stan Hu)