Skip to content

Commit c1b5c45

Browse files
committed
Add 'protect_from_forgery` to turn on Rail's built-in protection.
Since the Doorkeeper controllers inherit from Doorkeeper::Application (which inherits directly from ActionController::Base) and not ApplicationController, they never call `protect_from_forgery`, which means that non-GET methods don’t validate CSRF tokens. Thus, it’s possible for an attacker to host a form on an arbitrary URL, and if a users is logged into a site that uses Doorkeeper visits the URL, the attacker can grant access to a application on that site.
1 parent ed26117 commit c1b5c45

File tree

2 files changed

+29
-0
lines changed

2 files changed

+29
-0
lines changed

app/controllers/doorkeeper/application_controller.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@ module Doorkeeper
22
class ApplicationController < ActionController::Base
33
include Helpers::Controller
44

5+
if ::Rails.version.to_i < 4
6+
protect_from_forgery
7+
else
8+
protect_from_forgery with: :exception
9+
end
10+
511
helper 'doorkeeper/dashboard'
612
end
713
end

spec/requests/endpoints/authorization_spec.rb

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,27 @@
5151
i_should_see_translated_error_message :unsupported_response_type
5252
end
5353
end
54+
55+
context 'forgery protection enabled' do
56+
before do
57+
ActionController::Base.allow_forgery_protection = true
58+
end
59+
60+
after do
61+
ActionController::Base.allow_forgery_protection = false
62+
end
63+
64+
background do
65+
create_resource_owner
66+
sign_in
67+
end
68+
69+
scenario 'raises exception on forged requests' do
70+
ActionController::Base.any_instance.should_receive(:handle_unverified_request)
71+
post "/oauth/authorize",
72+
client_id: @client.uid,
73+
redirect_uri: @client.redirect_uri,
74+
response_type: 'code'
75+
end
76+
end
5477
end

0 commit comments

Comments
 (0)