From 3078b13e7248d5f0df1e1093ebfb8f401d234784 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 30 Jun 2015 21:38:26 -0400 Subject: [PATCH 1/9] Gem updates for security issues - sprockets (rails dependency, but we need to specify a version to pull in fixes) - sass-rails (no security issues, but required an update to meet new sprockets version requirement) - rest-client (coveralls dependency) --- Gemfile | 12 +++++++++++- Gemfile.lock | 51 ++++++++++++++++++++++++++++++--------------------- 2 files changed, 41 insertions(+), 22 deletions(-) diff --git a/Gemfile b/Gemfile index cebe957965f..368cadc97d7 100644 --- a/Gemfile +++ b/Gemfile @@ -2,6 +2,10 @@ source "https://rubygems.org" gem 'rails', '4.1.11' +# Specify a sprockets version due to security issue +# See https://groups.google.com/forum/#!topic/rubyonrails-security/doAVp0YaTqY +gem 'sprockets', '~> 2.12.3' + # Default values for AR models gem "default_value_for", "~> 3.0.0" @@ -181,7 +185,7 @@ gem 'mousetrap-rails' # Detect and convert string character encoding gem 'charlock_holmes' -gem "sass-rails", '~> 4.0.2' +gem "sass-rails", '~> 4.0.5' gem "coffee-rails" gem "uglifier" gem 'turbolinks', '~> 2.5.0' @@ -234,6 +238,12 @@ group :development, :test do gem 'rubocop', '0.28.0', require: false gem 'spinach-rails' + # rest-client is a coveralls dependency and not used directly in GitLab, but + # we specify a version here to pick up some security fixes. + # See https://github.com/rest-client/rest-client/issues/369 + # and http://www.osvdb.org/show/osvdb/117461 + gem 'rest-client', '~> 1.8.0' + # Prevent occasions where minitest is not bundled in packaged versions of ruby (see #3826) gem 'minitest', '~> 5.3.0' diff --git a/Gemfile.lock b/Gemfile.lock index 6d39e3b94c5..e300de42116 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -113,12 +113,12 @@ GEM colorize (0.5.8) columnize (0.9.0) connection_pool (2.1.0) - coveralls (0.7.0) - multi_json (~> 1.3) - rest-client - simplecov (>= 0.7) - term-ansicolor - thor + coveralls (0.8.2) + json (~> 1.8) + rest-client (>= 1.6.8, < 2) + simplecov (~> 0.10.0) + term-ansicolor (~> 1.3) + thor (~> 0.19.1) crack (0.4.2) safe_yaml (~> 1.0.0) creole (0.3.8) @@ -149,6 +149,8 @@ GEM diff-lcs (1.2.5) diffy (3.0.3) docile (1.1.5) + domain_name (0.5.24) + unf (>= 0.0.5, < 1.0.0) doorkeeper (2.1.3) railties (>= 3.2) dotenv (0.9.0) @@ -322,6 +324,8 @@ GEM html-pipeline (1.11.0) activesupport (>= 2) nokogiri (~> 1.4) + http-cookie (1.0.2) + domain_name (~> 0.5) http_parser.rb (0.5.3) httparty (0.13.3) json (~> 1.8) @@ -377,6 +381,7 @@ GEM net-scp (1.2.1) net-ssh (>= 2.6.5) net-ssh (2.9.2) + netrc (0.10.3) newrelic_rpm (3.9.4.245) nokogiri (1.6.6.2) mini_portile (~> 0.6.0) @@ -525,8 +530,10 @@ GEM request_store (1.0.5) rerun (0.10.0) listen (~> 2.7, >= 2.7.3) - rest-client (1.6.7) - mime-types (>= 1.16) + rest-client (1.8.0) + http-cookie (>= 1.0.2, < 2.0) + mime-types (>= 1.16, < 3.0) + netrc (~> 0.7) rinku (1.7.3) rotp (1.6.1) rouge (1.7.7) @@ -577,10 +584,10 @@ GEM sanitize (2.1.0) nokogiri (>= 1.4.4) sass (3.2.19) - sass-rails (4.0.3) + sass-rails (4.0.5) railties (>= 4.0.0, < 5.0) - sass (~> 3.2.0) - sprockets (~> 2.8, <= 2.11.0) + sass (~> 3.2.2) + sprockets (~> 2.8, < 3.0) sprockets-rails (~> 2.0) sawyer (0.6.0) addressable (~> 2.3.5) @@ -608,11 +615,11 @@ GEM ice_cube (= 0.11.1) sidekiq (>= 3.0.0) simple_oauth (0.1.9) - simplecov (0.9.0) + simplecov (0.10.0) docile (~> 1.1.0) - multi_json - simplecov-html (~> 0.8.0) - simplecov-html (0.8.0) + json (~> 1.8) + simplecov-html (~> 0.10.0) + simplecov-html (0.10.0) sinatra (1.4.4) rack (~> 1.4) rack-protection (~> 1.4) @@ -637,12 +644,12 @@ GEM spring (>= 0.9.1) spring-commands-teaspoon (0.0.2) spring (>= 0.9.1) - sprockets (2.11.0) + sprockets (2.12.4) hike (~> 1.2) multi_json (~> 1.0) rack (~> 1.0) tilt (~> 1.1, != 1.3.0) - sprockets-rails (2.3.1) + sprockets-rails (2.3.2) actionpack (>= 3.0) activesupport (>= 3.0) sprockets (>= 2.8, < 4.0) @@ -657,8 +664,8 @@ GEM teaspoon-jasmine (2.2.0) teaspoon (>= 1.0.0) temple (0.6.7) - term-ansicolor (1.2.2) - tins (~> 0.8) + term-ansicolor (1.3.2) + tins (~> 1.0) terminal-table (1.4.5) test_after_commit (0.2.2) thin (1.6.1) @@ -680,7 +687,7 @@ GEM mime-types (~> 1.19) multi_json (~> 1.7) twitter-stream (~> 0.1) - tins (0.13.1) + tins (1.5.4) trollop (2.1.2) turbolinks (2.5.3) coffee-rails @@ -826,12 +833,13 @@ DEPENDENCIES redis-rails request_store rerun (~> 0.10.0) + rest-client (~> 1.8.0) rqrcode-rails3 rspec-rails (~> 3.3.0) rubocop (= 0.28.0) rugments (~> 1.0.0.beta8) sanitize (~> 2.0) - sass-rails (~> 4.0.2) + sass-rails (~> 4.0.5) sdoc seed-fu select2-rails @@ -849,6 +857,7 @@ DEPENDENCIES spring-commands-rspec (~> 1.0.0) spring-commands-spinach (~> 1.0.0) spring-commands-teaspoon (~> 0.0.2) + sprockets (~> 2.12.3) stamp state_machine task_list (= 1.0.2) From 9e841cf1a894c5bb88a470ffae0f59f12e6be8c2 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 1 Jul 2015 14:25:22 +0200 Subject: [PATCH 2/9] Add Troubleshooting section to SAML doc. --- doc/integration/saml.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/integration/saml.md b/doc/integration/saml.md index a8cc5c8f74a..4aa6dbe758a 100644 --- a/doc/integration/saml.md +++ b/doc/integration/saml.md @@ -75,3 +75,8 @@ At a minimum the IdP *must* provide a claim containing the user's email address, On the sign in page there should now be a SAML button below the regular sign in form. Click the icon to begin the authentication process. If everything goes well the user will be returned to GitLab and will be signed in. +## Troubleshooting + +If you see a "500 error" in GitLab when you are redirected back from the SAML sign in page, this likely indicates that GitLab could not get the email address for the SAML user. + +Make sure the IdP provides a claim containing the user's email address, using claim name 'email' or 'mail'. The email will be used to automatically generate the GitLab username. From 2662c7932a493e766eec3225b368286b71598ce9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Reynolds?= Date: Tue, 30 Jun 2015 23:08:10 -0300 Subject: [PATCH 3/9] commit hashes are monospaced --- app/assets/stylesheets/generic/gfm.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/assets/stylesheets/generic/gfm.scss b/app/assets/stylesheets/generic/gfm.scss index 8fac5e534fa..bd9200ace23 100644 --- a/app/assets/stylesheets/generic/gfm.scss +++ b/app/assets/stylesheets/generic/gfm.scss @@ -19,3 +19,7 @@ height: 14em; } } + +.gfm-commit, .gfm-commit_range { + font-family: $monospace_font; +} From c67edaee103cf78c09c7ed8ae0a02e4aaf6cb1fb Mon Sep 17 00:00:00 2001 From: Daniel Gerhardt Date: Wed, 1 Jul 2015 20:56:01 +0200 Subject: [PATCH 4/9] Fix changelog for !767 and !794 These changes did not make it into the 7.12.0 release. The entry for !794 is already correctly listed for 7.12.1 so it has been removed. --- CHANGELOG | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 616b41a4269..f1be68aba74 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 7.13.0 (unreleased) + - Fix external issue tracker hook/test for HTTPS URLs (Daniel Gerhardt) - Fix order of issues imported form GitHub (Hiroyuki Sato) - Bump rugments to 1.0.0beta8 to fix C prototype function highlighting (Jonathon Reinhart) - Fix Merge Request webhook to properly fire "merge" action when accepted from the web UI @@ -44,12 +45,10 @@ v 7.12.0 - Disable changing of target branch in new merge request page when a branch has already been specified (Stan Hu) - Fix post-receive errors on a push when an external issue tracker is configured (Stan Hu) - Update oauth button logos for Twitter and Google to recommended assets - - Fix hooks for web based events with external issue references (Daniel Gerhardt) - Update browser gem to version 0.8.0 for IE11 support (Stan Hu) - Fix timeout when rendering file with thousands of lines. - Add "Remember me" checkbox to LDAP signin form. - Add session expiration delay configuration through UI application settings - - Fix external issue tracker hook/test for HTTPS URLs (Daniel Gerhardt) - Don't notify users mentioned in code blocks or blockquotes. - Omit link to generate labels if user does not have access to create them (Stan Hu) - Show warning when a comment will add 10 or more people to the discussion. From b643fa16039b9da8a52527e1a657401415684f21 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 1 Jul 2015 22:51:28 -0700 Subject: [PATCH 5/9] Remove link leading to a 404 error in Deploy Keys page Closes #1866 --- CHANGELOG | 1 + app/views/admin/deploy_keys/index.html.haml | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 616b41a4269..7a6b71ff140 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 7.13.0 (unreleased) + - Remove link leading to a 404 error in Deploy Keys page (Stan Hu) - Fix order of issues imported form GitHub (Hiroyuki Sato) - Bump rugments to 1.0.0beta8 to fix C prototype function highlighting (Jonathon Reinhart) - Fix Merge Request webhook to properly fire "merge" action when accepted from the web UI diff --git a/app/views/admin/deploy_keys/index.html.haml b/app/views/admin/deploy_keys/index.html.haml index 6405a69fad3..2bf1689cbc6 100644 --- a/app/views/admin/deploy_keys/index.html.haml +++ b/app/views/admin/deploy_keys/index.html.haml @@ -16,8 +16,7 @@ - @deploy_keys.each do |deploy_key| %tr %td - = link_to admin_deploy_key_path(deploy_key) do - %strong= deploy_key.title + %strong= deploy_key.title %td %code.key-fingerprint= deploy_key.fingerprint %td From cd5e79e98b7f08a6fa6999d25e158073fee03048 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 2 Jul 2015 11:53:31 +0200 Subject: [PATCH 6/9] Correctly show anonymous authorized applications under Profile > Applications. --- CHANGELOG | 1 + .../oauth/authorized_applications_controller.rb | 7 ++++++- app/controllers/profiles_controller.rb | 3 ++- .../authorized_applications/_delete_form.html.haml | 9 +++++++-- app/views/profiles/applications.html.haml | 9 +++++++++ 5 files changed, 25 insertions(+), 4 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index f1be68aba74..d15b5d830a8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -30,6 +30,7 @@ v 7.13.0 (unreleased) - Reporter role can manage issue tracker now: edit any issue, set assignee or milestone and manage labels - Better performance for pages with events list, issues list and commits list - Faster automerge check and merge itself when source and target branches are in same repository + - Correctly show anonymous authorized applications under Profile > Applications. v 7.12.1 - Fix error when deleting a user who has projects (Stan Hu) diff --git a/app/controllers/oauth/authorized_applications_controller.rb b/app/controllers/oauth/authorized_applications_controller.rb index 3ab6def511c..4193ac11399 100644 --- a/app/controllers/oauth/authorized_applications_controller.rb +++ b/app/controllers/oauth/authorized_applications_controller.rb @@ -4,7 +4,12 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio layout 'profile' def destroy - Doorkeeper::AccessToken.revoke_all_for(params[:id], current_resource_owner) + if params[:token_id].present? + current_resource_owner.oauth_authorized_tokens.find(params[:token_id]).revoke + else + Doorkeeper::AccessToken.revoke_all_for(params[:id], current_resource_owner) + end + redirect_to applications_profile_url, notice: I18n.t(:notice, scope: [:doorkeeper, :flash, :authorized_applications, :destroy]) end end diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index b4af9e490ed..5382a6cf6ac 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -11,7 +11,8 @@ class ProfilesController < Profiles::ApplicationController def applications @applications = current_user.oauth_applications @authorized_tokens = current_user.oauth_authorized_tokens - @authorized_apps = @authorized_tokens.map(&:application).uniq + @authorized_anonymous_tokens = @authorized_tokens.reject(&:application) + @authorized_apps = @authorized_tokens.map(&:application).uniq - [nil] end def update diff --git a/app/views/doorkeeper/authorized_applications/_delete_form.html.haml b/app/views/doorkeeper/authorized_applications/_delete_form.html.haml index 4bba72167e3..bfa95ce79a7 100644 --- a/app/views/doorkeeper/authorized_applications/_delete_form.html.haml +++ b/app/views/doorkeeper/authorized_applications/_delete_form.html.haml @@ -1,4 +1,9 @@ - submit_btn_css ||= 'btn btn-link btn-remove' -= form_tag oauth_authorized_application_path(application) do +- if defined?(token) + - path = oauth_authorized_application_path(0, token_id: token) +- else + - path = oauth_authorized_application_path(application) + += form_tag path do %input{:name => "_method", :type => "hidden", :value => "delete"}/ - = submit_tag 'Revoke', onclick: "return confirm('Are you sure?')", class: 'btn btn-link btn-remove btn-sm' \ No newline at end of file + = submit_tag 'Revoke', onclick: "return confirm('Are you sure?')", class: 'btn btn-link btn-remove btn-sm' diff --git a/app/views/profiles/applications.html.haml b/app/views/profiles/applications.html.haml index 2c4f0804f0b..d2fad31eca2 100644 --- a/app/views/profiles/applications.html.haml +++ b/app/views/profiles/applications.html.haml @@ -56,5 +56,14 @@ %td= token.created_at %td= token.scopes %td= render 'doorkeeper/authorized_applications/delete_form', application: app + - @authorized_anonymous_tokens.each do |token| + %tr + %td + Anonymous + %div.help-block + %em Authorization was granted by entering your username and password in the application. + %td= token.created_at + %td= token.scopes + %td= render 'doorkeeper/authorized_applications/delete_form', token: token - else %p.light You dont have any authorized applications From 3e738e3b9aeae5620116109258c4d4da84180e7e Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 1 Jul 2015 22:26:14 -0700 Subject: [PATCH 7/9] Add support for unlocking users in admin settings Closes https://github.com/gitlabhq/gitlabhq/issues/9381 --- CHANGELOG | 1 + app/controllers/admin/users_controller.rb | 8 ++++++++ app/views/admin/users/index.html.haml | 2 ++ app/views/admin/users/show.html.haml | 8 ++++++++ config/routes.rb | 1 + spec/controllers/admin/users_controller_spec.rb | 15 +++++++++++++++ 6 files changed, 35 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 616b41a4269..d7d12034e3b 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 7.13.0 (unreleased) + - Add support for unlocking users in admin settings (Stan Hu) - Fix order of issues imported form GitHub (Hiroyuki Sato) - Bump rugments to 1.0.0beta8 to fix C prototype function highlighting (Jonathon Reinhart) - Fix Merge Request webhook to properly fire "merge" action when accepted from the web UI diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index ec29c320654..7a683098df3 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -47,6 +47,14 @@ class Admin::UsersController < Admin::ApplicationController end end + def unlock + if user.unlock_access! + redirect_to :back, alert: "Successfully unlocked" + else + redirect_to :back, alert: "Error occurred. User was not unlocked" + end + end + def create opts = { force_random_password: true, diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml index 9c1bec7c84d..b0d31170704 100644 --- a/app/views/admin/users/index.html.haml +++ b/app/views/admin/users/index.html.haml @@ -93,6 +93,8 @@ = link_to 'Unblock', unblock_admin_user_path(user), method: :put, class: "btn btn-xs btn-success" - else = link_to 'Block', block_admin_user_path(user), data: {confirm: 'USER WILL BE BLOCKED! Are you sure?'}, method: :put, class: "btn btn-xs btn-warning" + - if user.access_locked? + = link_to 'Unlock', unlock_admin_user_path(user), method: :put, class: "btn btn-xs btn-success", data: { confirm: 'Are you sure?' } - if user.can_be_removed? = link_to 'Destroy', [:admin, user], data: { confirm: "USER #{user.name} WILL BE REMOVED! All tickets linked to this user will also be removed! Maybe block the user instead? Are you sure?" }, method: :delete, class: "btn btn-xs btn-remove" = paginate @users, theme: "gitlab" diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml index 2662b3569ec..8c6b8e851c4 100644 --- a/app/views/admin/users/show.html.haml +++ b/app/views/admin/users/show.html.haml @@ -131,6 +131,14 @@ %li Owned groups will be left %br = link_to 'Block user', block_admin_user_path(@user), data: { confirm: 'USER WILL BE BLOCKED! Are you sure?' }, method: :put, class: "btn btn-warning" + - if @user.access_locked? + .panel.panel-info + .panel-heading + This account has been locked + .panel-body + %p This user has been temporarily locked due to excessive number of failed logins. You may manually unlock the account. + %br + = link_to 'Unlock user', unlock_admin_user_path(@user), method: :put, class: "btn btn-info", data: { confirm: 'Are you sure?' } .panel.panel-danger .panel-heading diff --git a/config/routes.rb b/config/routes.rb index 33f55dde476..f904c975733 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -158,6 +158,7 @@ Gitlab::Application.routes.draw do put :team_update put :block put :unblock + put :unlock delete 'remove/:email_id', action: 'remove_email', as: 'remove_email' end end diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index f27e861e175..550a91a79e2 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -21,4 +21,19 @@ describe Admin::UsersController do expect { User.find(user.id) }.to raise_exception(ActiveRecord::RecordNotFound) end end + + describe 'PUT unlock/:id' do + let(:user) { create(:user) } + + before do + request.env["HTTP_REFERER"] = "/" + user.lock_access! + end + + it 'unlocks user' do + put :unlock, id: user.username + user.reload + expect(user.access_locked?).to be_falsey + end + end end From 87ac590078c6c219a8afb6feac6e41d75dca1287 Mon Sep 17 00:00:00 2001 From: catatsuy Date: Tue, 16 Jun 2015 21:22:06 +0900 Subject: [PATCH 8/9] 'created_at DESC' is performed twice If you are already sorting in descending order in the created_at, it is run twice when you run the .recent. It has passed in the string 'created_at DESC'. Ruby on Rails is directly given to the SQL. It is a slow query in MySQL. --- app/models/event.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/event.rb b/app/models/event.rb index c9a88ffa8e0..78f16c6304e 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -44,7 +44,7 @@ class Event < ActiveRecord::Base after_create :reset_project_activity # Scopes - scope :recent, -> { order("created_at DESC") } + scope :recent, -> { order(created_at: :desc) } scope :code_push, -> { where(action: PUSHED) } scope :in_projects, ->(project_ids) { where(project_id: project_ids).recent } scope :with_associations, -> { includes(project: :namespace) } From 63a4dd53920882cf993459b3483d70073e75fae0 Mon Sep 17 00:00:00 2001 From: catatsuy Date: Thu, 2 Jul 2015 21:29:17 +0900 Subject: [PATCH 9/9] add CHANGELOG --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 7763ccf7c70..1c16ff728a5 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -32,6 +32,7 @@ v 7.13.0 (unreleased) - Better performance for pages with events list, issues list and commits list - Faster automerge check and merge itself when source and target branches are in same repository - Correctly show anonymous authorized applications under Profile > Applications. + - Query Optimization in MySQL. v 7.12.1 - Fix error when deleting a user who has projects (Stan Hu)