Skip to content

Commit

Permalink
Extract the provider from the uid param.
Browse files Browse the repository at this point in the history
  Previously, the `provider` was concatenated to the `uid` and passed around
  between requests via a single `uid` parameter.  This combined `uid` was
  split back apart when querying for the resource_class via its authentications.

  Instead of concatenating and then splitting, use an additional `provider`
  param in conjunction with the existing `uid` param.
  • Loading branch information
Evan-M committed Feb 13, 2018
1 parent 90e1270 commit d7aaa80
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,13 @@ def set_user_by_token(mapping=nil)

# gets the headers names, which was set in the initialize file
uid_name = DeviseTokenAuth.headers_names[:'uid']
provider_name = DeviseTokenAuth.headers_names[:'provider']
access_token_name = DeviseTokenAuth.headers_names[:'access-token']
client_name = DeviseTokenAuth.headers_names[:'client']

# parse header for values necessary for authentication
uid = request.headers[uid_name] || params[uid_name]
provider = request.headers[provider_name] || params[provider_name]
@token ||= request.headers[access_token_name] || params[access_token_name]
@client_id ||= request.headers[client_name] || params[client_name]

Expand Down Expand Up @@ -80,8 +82,7 @@ def set_user_by_token(mapping=nil)
# NOTE: By searching for the user by an identifier instead of by token, we
# mitigate timing attacks
#
@provider_id, @provider = uid.split # e.g. ["12345", "facebook"] or ["[email protected]", "email"]
resource = rc.find_resource(@provider_id, @provider)
resource = rc.find_resource(uid, provider)

if resource && resource.valid_token?(@token, @client_id)
# sign_in with bypass: true will be deprecated in the next version of Devise
Expand Down
3 changes: 2 additions & 1 deletion app/models/devise_token_auth/concerns/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,8 @@ def build_auth_header(token, client_id='default', provider_id, provider)
DeviseTokenAuth.headers_names[:"token-type"] => "Bearer",
DeviseTokenAuth.headers_names[:"client"] => client_id,
DeviseTokenAuth.headers_names[:"expiry"] => expiry.to_s,
DeviseTokenAuth.headers_names[:"uid"] => "#{provider_id} #{provider}"
DeviseTokenAuth.headers_names[:"provider"] => provider.to_s,
DeviseTokenAuth.headers_names[:"uid"] => provider_id
}
end

Expand Down
1 change: 1 addition & 0 deletions lib/devise_token_auth/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class Engine < ::Rails::Engine
:'client' => 'client',
:'expiry' => 'expiry',
:'uid' => 'uid',
:'provider' => 'provider',
:'token-type' => 'token-type' }
self.bypass_sign_in = true

Expand Down
2 changes: 2 additions & 0 deletions test/controllers/demo_group_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class DemoGroupControllerTest < ActionDispatch::IntegrationTest
@resp_client_id = response.headers['client']
@resp_expiry = response.headers['expiry']
@resp_uid = response.headers['uid']
@resp_provider = response.headers['provider']
end

test 'request is successful' do
Expand Down Expand Up @@ -96,6 +97,7 @@ class DemoGroupControllerTest < ActionDispatch::IntegrationTest
@resp_client_id = response.headers['client']
@resp_expiry = response.headers['expiry']
@resp_uid = response.headers['uid']
@resp_provider = response.headers['provider']
end

test 'request is successful' do
Expand Down
7 changes: 6 additions & 1 deletion test/controllers/demo_mang_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class DemoMangControllerTest < ActionDispatch::IntegrationTest
@resp_client_id = response.headers['client']
@resp_expiry = response.headers['expiry']
@resp_uid = response.headers['uid']
@resp_provider = response.headers['provider']
end

describe 'devise mappings' do
Expand Down Expand Up @@ -67,7 +68,11 @@ class DemoMangControllerTest < ActionDispatch::IntegrationTest
end

it "should return the user's uid in the auth header" do
assert_equal "#{@resource.uid} email", @resp_uid
assert_equal @resource.uid, @resp_uid
end

it "should return 'email' provider in the auth header" do
assert_equal 'email', @resp_provider
end

it 'should not treat this request as a batch request' do
Expand Down
17 changes: 16 additions & 1 deletion test/controllers/demo_user_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class DemoUserControllerTest < ActionDispatch::IntegrationTest
@resp_client_id = response.headers['client']
@resp_expiry = response.headers['expiry']
@resp_uid = response.headers['uid']
@resp_provider = response.headers['provider']
end

describe 'devise mappings' do
Expand Down Expand Up @@ -68,7 +69,11 @@ class DemoUserControllerTest < ActionDispatch::IntegrationTest
end

it "should return the user's uid in the auth header" do
assert_equal "#{@resource.uid} email", @resp_uid
assert_equal @resource.uid, @resp_uid
end

it "should return 'email' provider in the auth header" do
assert_equal 'email', @resp_provider
end

it 'should not treat this request as a batch request' do
Expand Down Expand Up @@ -488,6 +493,7 @@ class DemoUserControllerTest < ActionDispatch::IntegrationTest
@resp_client_id = response.headers['client']
@resp_expiry = response.headers['expiry']
@resp_uid = response.headers['uid']
@resp_provider = response.headers['provider']
end

describe 'devise mappings' do
Expand Down Expand Up @@ -535,6 +541,10 @@ class DemoUserControllerTest < ActionDispatch::IntegrationTest
it "should return the user's uid in the auth header" do
assert @resp_uid
end

it "should return the user's auth provider in the auth header" do
assert @resp_provider
end
end

describe 'existing Warden authentication with ignored token data' do
Expand All @@ -552,6 +562,7 @@ class DemoUserControllerTest < ActionDispatch::IntegrationTest
@resp_client_id = response.headers['client']
@resp_expiry = response.headers['expiry']
@resp_uid = response.headers['uid']
@resp_provider = response.headers['provider']
end

describe 'devise mappings' do
Expand Down Expand Up @@ -595,6 +606,10 @@ class DemoUserControllerTest < ActionDispatch::IntegrationTest
it "should not return the token user's uid in the auth header" do
refute_equal @resp_uid, @auth_headers['uid']
end

it "should return the user's auth provider in the auth (response) header" do
assert @resp_provider
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -839,6 +839,7 @@ class DeviseTokenAuth::RegistrationsControllerTest < ActionDispatch::Integration
assert response.headers['client']
assert response.headers['expiry']
assert response.headers['uid']
assert response.headers['provider']
end

test 'response token is valid' do
Expand Down

0 comments on commit d7aaa80

Please sign in to comment.