diff options
author | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2015-05-14 17:22:26 +0300 |
---|---|---|
committer | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2015-05-14 17:22:26 +0300 |
commit | c2ee828c19cb245809647428334b8ef215536a0d (patch) | |
tree | 27a00bc43a61ad5a07a6577281cbb21ea71371d3 | |
parent | 910794bae5a91479f41468ebc345db680a33b20e (diff) | |
parent | b17f36f040a18ff6700881c56607ba6df436f652 (diff) |
Merge branch 'omniauth-csrf' into 'master'
Protect OmniAuth request phase against CSRF.
Addresses #2268.
See merge request !1793
-rw-r--r-- | CHANGELOG | 3 | ||||
-rw-r--r-- | Gemfile | 2 | ||||
-rw-r--r-- | Gemfile.lock | 8 | ||||
-rw-r--r-- | app/views/devise/shared/_omniauth_box.html.haml | 4 | ||||
-rw-r--r-- | app/views/profiles/accounts/show.html.haml | 2 | ||||
-rw-r--r-- | config/initializers/7_omniauth.rb | 5 | ||||
-rw-r--r-- | lib/omni_auth/request_forgery_protection.rb | 66 |
7 files changed, 82 insertions, 8 deletions
diff --git a/CHANGELOG b/CHANGELOG index a06509c7c79..b9811039736 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -42,6 +42,9 @@ v 7.11.0 (unreleased) - Task lists are now usable in comments, and will show up in Markdown previews. - Fix bug where avatar filenames were not actually deleted from the database during removal (Stan Hu) - Fix bug where Slack service channel was not saved in admin template settings. (Stan Hu) + - Protect OmniAuth request phase against CSRF. + - + - - Move snippets UI to fluid layout - Improve UI for sidebar. Increase separation between navigation and content - Improve new project command options (Ben Bodenmiller) @@ -23,7 +23,7 @@ gem "pg", group: :postgres # Auth gem "devise", '3.2.4' gem "devise-async", '0.9.0' -gem 'omniauth', "~> 1.1.3" +gem 'omniauth', "~> 1.2.2" gem 'omniauth-google-oauth2' gem 'omniauth-twitter' gem 'omniauth-github' diff --git a/Gemfile.lock b/Gemfile.lock index 9940ab15242..ef7487ae5bc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -362,9 +362,9 @@ GEM rack (~> 1.2) octokit (3.7.0) sawyer (~> 0.6.0, >= 0.5.3) - omniauth (1.1.4) - hashie (>= 1.2, < 3) - rack + omniauth (1.2.2) + hashie (>= 1.2, < 4) + rack (~> 1.0) omniauth-bitbucket (0.0.2) multi_json (~> 1.7) omniauth (~> 1.1) @@ -751,7 +751,7 @@ DEPENDENCIES newrelic_rpm nprogress-rails octokit (= 3.7.0) - omniauth (~> 1.1.3) + omniauth (~> 1.2.2) omniauth-bitbucket omniauth-github omniauth-gitlab diff --git a/app/views/devise/shared/_omniauth_box.html.haml b/app/views/devise/shared/_omniauth_box.html.haml index 8dce0b16936..f8ba9d80ae8 100644 --- a/app/views/devise/shared/_omniauth_box.html.haml +++ b/app/views/devise/shared/_omniauth_box.html.haml @@ -5,6 +5,6 @@ - providers.each do |provider| %span.light - if default_providers.include?(provider) - = link_to oauth_image_tag(provider), omniauth_authorize_path(resource_name, provider), class: 'oauth-image-link' + = link_to oauth_image_tag(provider), omniauth_authorize_path(resource_name, provider), method: :post, class: 'oauth-image-link' - else - = link_to provider.to_s.titleize, omniauth_authorize_path(resource_name, provider), class: "btn", "data-no-turbolink" => "true" + = link_to provider.to_s.titleize, omniauth_authorize_path(resource_name, provider), method: :post, class: "btn", "data-no-turbolink" => "true" diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index 6ac60b01f85..06bad7dd84a 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -62,7 +62,7 @@ - enabled_social_providers.each do |provider| .btn-group = link_to oauth_image_tag(provider), omniauth_authorize_path(User, provider), - class: "btn btn-lg #{'active' if oauth_active?(provider)}" + method: :post, class: "btn btn-lg #{'active' if oauth_active?(provider)}" - if oauth_active?(provider) = link_to unlink_profile_account_path(provider: provider), method: :delete, class: 'btn btn-lg' do = icon('close') diff --git a/config/initializers/7_omniauth.rb b/config/initializers/7_omniauth.rb index 8f6c5673103..103aa06ca32 100644 --- a/config/initializers/7_omniauth.rb +++ b/config/initializers/7_omniauth.rb @@ -10,3 +10,8 @@ if Gitlab::LDAP::Config.enabled? alias_method server['provider_name'], :ldap end end + +OmniAuth.config.allowed_request_methods = [:post] +OmniAuth.config.before_request_phase do |env| + OmniAuth::RequestForgeryProtection.new(env).call +end diff --git a/lib/omni_auth/request_forgery_protection.rb b/lib/omni_auth/request_forgery_protection.rb new file mode 100644 index 00000000000..3557522d3c9 --- /dev/null +++ b/lib/omni_auth/request_forgery_protection.rb @@ -0,0 +1,66 @@ +# Protects OmniAuth request phase against CSRF. + +module OmniAuth + # Based on ActionController::RequestForgeryProtection. + class RequestForgeryProtection + def initialize(env) + @env = env + end + + def request + @request ||= ActionDispatch::Request.new(@env) + end + + def session + request.session + end + + def reset_session + request.reset_session + end + + def params + request.params + end + + def call + verify_authenticity_token + end + + def verify_authenticity_token + if !verified_request? + Rails.logger.warn "Can't verify CSRF token authenticity" if Rails.logger + handle_unverified_request + end + end + + private + + def protect_against_forgery? + ApplicationController.allow_forgery_protection + end + + def request_forgery_protection_token + ApplicationController.request_forgery_protection_token + end + + def forgery_protection_strategy + ApplicationController.forgery_protection_strategy + end + + def verified_request? + !protect_against_forgery? || request.get? || request.head? || + form_authenticity_token == params[request_forgery_protection_token] || + form_authenticity_token == request.headers['X-CSRF-Token'] + end + + def handle_unverified_request + forgery_protection_strategy.new(self).handle_unverified_request + end + + # Sets the token value for the current session. + def form_authenticity_token + session[:_csrf_token] ||= SecureRandom.base64(32) + end + end +end |