Skip to content

Commit

Permalink
Fix security vulnerability alert CVE-2015-9284
Browse files Browse the repository at this point in the history
Github issued a security vulnerability alert related to OmniAuth.

A Cross-Site Request Forgery vulnerability in the request phase in
OmniAuth, allows an attacker to gain full access to a user's account
on a site that uses OmniAuth.

Not sure how this affects EVE Online's SSO but the fix is
straight-forward so let's stay on the safe side.

This commit adds CSRF verification during OmniAuth's request phase.

See https://nvd.nist.gov/vuln/detail/CVE-2015-9284
See omniauth/omniauth#960

Closes #128
  • Loading branch information
lunohodov committed May 30, 2019
1 parent 033680d commit b017fcf
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 4 deletions.
7 changes: 5 additions & 2 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@ def link_to_eve_online_sso_help(text = "What is this?")

def eve_online_sso_button(size = :small)
size = :small unless %i[small large].include?(size)
style = "border: 0; padding: 0; display: inline-block; vertical-align: top;"

link_to "/auth/eve_online_sso" do
# rubocop:disable Metrics/LineLength
button_to("/auth/eve_online_sso", method: :post, remote: false, style: style) do
image_tag(
"https://web.ccpgamescdn.com/eveonlineassets/developers/eve-sso-login-black-#{size}.png", # rubocop:disable Metrics/LineLength
"https://web.ccpgamescdn.com/eveonlineassets/developers/eve-sso-login-black-#{size}.png",
alt: "Log in with EVE Online",
)
end
# rubocop:enable Metrics/LineLength
end
end
39 changes: 39 additions & 0 deletions app/middleware/request_forgery_protection_token_verification.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# See https://github.com/omniauth/omniauth/pull/809
#
# Provides a callable method that verifies Cross-Site Request Forgery
# protection token.
#
# This class includes `ActionController::RequestForgeryProtection`
# directly and utilizes `verified_request?` method to match the way Rails
# performs token verification in Rails controllers.
#
# If you like to learn more about how Rails generate and verify authenticity
# token, you can find the source code at
# https://github.com/rails/rails/blob/v5.2.2/actionpack/lib/action_controller/metal/request_forgery_protection.rb#L217-L240.
class RequestForgeryProtectionTokenVerification
include ActiveSupport::Configurable
include ActionController::RequestForgeryProtection

# `ActionController::RequestForgeryProtection` contains a few configurable
# options. As we want to make sure that our configuration is the same as
# what being set in `ActionController::Base`, we should make all out
# configuration methods to delegate to `ActionController::Base`.
config.each_key do |configuration_name|
define_method configuration_name do
ActionController::Base.config[configuration_name]
end
end

def call(env)
@request = ActionDispatch::Request.new(env)

unless verified_request?
raise ActionController::InvalidAuthenticityToken
end
end

private

attr_reader :request
delegate :params, :session, to: :request
end
4 changes: 4 additions & 0 deletions config/initializers/omniauth.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
end

Rails.application.config.middleware.use OmniAuth::Builder do
OmniAuth.config.allowed_request_methods = [:post]
OmniAuth.config.before_request_phase =
RequestForgeryProtectionTokenVerification.new

provider :developer unless Rails.env.production?
provider :eve_online_sso, setup: SETUP_PROC
end
4 changes: 2 additions & 2 deletions spec/features/index_page_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
visit "/"

within(".cover-try") do
expect(find_link("Log in with EVE Online")).to be_visible
expect(find_button("Log in with EVE Online")).to be_visible
end

within(".masthead") do
expect(find_link("Log in with EVE Online")).to be_visible
expect(find_button("Log in with EVE Online")).to be_visible
end
end

Expand Down

0 comments on commit b017fcf

Please sign in to comment.