From 571ba5a7feb870b7aa711d5a6fc6d4d53d92a4c5 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@gitlab.com>
Date: Fri, 24 Apr 2015 17:03:18 +0200
Subject: [PATCH] Protect OmniAuth request phase against CSRF.
---
CHANGELOG | 2 +-
Gemfile | 2 +-
Gemfile.lock | 8 +--
.../devise/shared/_omniauth_box.html.haml | 4 +-
app/views/profiles/accounts/show.html.haml | 2 +-
config/initializers/7_omniauth.rb | 5 ++
lib/omni_auth/request_forgery_protection.rb | 62 +++++++++++++++++++
7 files changed, 76 insertions(+), 9 deletions(-)
create mode 100644 lib/omni_auth/request_forgery_protection.rb
diff --git a/CHANGELOG b/CHANGELOG
index 0d6b0f2d942..0c55a58a47c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -14,7 +14,7 @@ v 7.11.0 (unreleased)
- Add project activity atom feed.
- Don't crash when an MR from a fork has a cross-reference comment from the target project on of its commits.
- Include commit comments in MR from a forked project.
- -
+ - Protect OmniAuth request phase against CSRF.
-
-
- Move snippets UI to fluid layout
diff --git a/Gemfile b/Gemfile
index 460a0f93a96..50630a4cb58 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 f0f8601a760..62488cc61f7 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -354,9 +354,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)
@@ -734,7 +734,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 5bffb4acc1d..7cc6008e48e 100644
--- a/app/views/profiles/accounts/show.html.haml
+++ b/app/views/profiles/accounts/show.html.haml
@@ -34,7 +34,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
%i.fa.fa-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..cbbb686473c
--- /dev/null
+++ b/lib/omni_auth/request_forgery_protection.rb
@@ -0,0 +1,62 @@
+# Protects OmniAuth request phase against CSRF.
+
+module OmniAuth
+ # Based from ActionController::RequestForgeryProtection.
+ class RequestForgeryProtection
+ def initialize(env)
+ @env = env
+ end
+
+ def request
+ @request ||= ActionDispatch::Request.new(@env)
+ end
+
+ def session
+ request.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
--
GitLab