Merge branch 'omniauth-csrf' into 'master'

Protect OmniAuth request phase against CSRF.

Addresses #2268.

See merge request !1793

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)
diff --git a/Gemfile b/Gemfile
index c5d7089750e..ca110d8732f 100644
--- a/Gemfile
+++ b/Gemfile
@@ -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