Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for sending and receiving the auth token via a server cookie #1453

Merged
merged 8 commits into from
Mar 11, 2021
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ test/dummy/.sass-cache
test/dummy/config/application.yml
coverage
.idea
.byebug_history
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-by that popped up while debugging a test

.irb_history
.ruby-version
.ruby-gemset
Expand Down
32 changes: 28 additions & 4 deletions app/controllers/devise_token_auth/concerns/set_user_by_token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,20 @@ def set_user_by_token(mapping = nil)
access_token_name = DeviseTokenAuth.headers_names[:'access-token']
client_name = DeviseTokenAuth.headers_names[:'client']

# gets values from cookie if configured and present
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can all this new cookie code be in a new module like set_user_by_cookie or something like that, so we don't pollute more this big file, you can include the concern/module in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can, my only push back would be that hopefully in the future we'll allow turning off headers (for the security reasons mentioned in point two of the description). In that case, it might feel a bit disjointed to have the header logic in set_user_by_token while the cookie logic is in set_user_by_cookie, when really both logics deal with handling the token. In that future world, would we move the header logic to another concern called set_user_by_headers? If so, wouldn't set_user_by_token just become a skeleton file?

Copy link
Contributor Author

@theblang theblang Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, see my recent realization. That may clean up the amount of noise in this file considerably.

parsed_auth_cookie = {}
if DeviseTokenAuth.cookie_enabled
auth_cookie = request.cookies[DeviseTokenAuth.cookie_name]
if auth_cookie.present?
parsed_auth_cookie = JSON.parse(auth_cookie)
end
end

# parse header for values necessary for authentication
uid = request.headers[uid_name] || params[uid_name]
uid = request.headers[uid_name] || params[uid_name] || parsed_auth_cookie[uid_name]
@token = DeviseTokenAuth::TokenFactory.new unless @token
@token.token ||= request.headers[access_token_name] || params[access_token_name]
@token.client ||= request.headers[client_name] || params[client_name]
@token.token ||= request.headers[access_token_name] || params[access_token_name] || parsed_auth_cookie[access_token_name]
@token.client ||= request.headers[client_name] || params[client_name] || parsed_auth_cookie[client_name]

# client isn't required, set to 'default' if absent
@token.client ||= 'default'
Expand Down Expand Up @@ -101,6 +110,10 @@ def update_auth_header
# update the response header
response.headers.merge!(auth_header)

# set a server cookie if configured
if DeviseTokenAuth.cookie_enabled
set_cookie(auth_header)
end
else
unless @resource.reload.valid?
@resource = @resource.class.find(@resource.to_param) # errors remain after reload
Expand All @@ -123,11 +136,22 @@ def refresh_headers
# cleared by sign out in the meantime
return if @used_auth_by_token && @resource.tokens[@token.client].nil?

_auth_header_from_batch_request = auth_header_from_batch_request

# update the response header
response.headers.merge!(auth_header_from_batch_request)
response.headers.merge!(_auth_header_from_batch_request)

# set a server cookie if configured
if DeviseTokenAuth.cookie_enabled
set_cookie(_auth_header_from_batch_request)
end
end # end lock
end

def set_cookie(auth_header)
cookies[DeviseTokenAuth.cookie_name] = DeviseTokenAuth.cookie_attributes.merge(value: auth_header.to_json)
end

def is_batch_request?(user, client)
!params[:unbatch] &&
user.tokens[client] &&
Expand Down
6 changes: 6 additions & 0 deletions app/controllers/devise_token_auth/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ def destroy
user.tokens.delete(client)
user.save!

if DeviseTokenAuth.cookie_enabled
# If a cookie is set with a domain specified then it must be deleted with that domain specified
# See https://api.rubyonrails.org/classes/ActionDispatch/Cookies.html
cookies.delete(DeviseTokenAuth.cookie_name, domain: DeviseTokenAuth.cookie_attributes[:domain])
end

yield user if block_given?

render_destroy_success
Expand Down
3 changes: 3 additions & 0 deletions docs/config/initialization.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ The following settings are available for configuration in `config/initializers/d
| **`enable_standard_devise_support`** (`false`) | By default, only Bearer Token authentication is implemented out of the box. If, however, you wish to integrate with legacy Devise authentication, you can do so by enabling this flag. NOTE: This feature is highly experimental! |
| **`remove_tokens_after_password_reset`** (`false`) | By default, old tokens are not invalidated when password is changed. Enable this option if you want to make passwords updates to logout other devices. |
| **`default_callbacks`** (`true`) | By default User model will include the `DeviseTokenAuth::Concerns::UserOmniauthCallbacks` concern, which has `email`, `uid` validations & `uid` synchronization callbacks. |
| **`cookie_enabled`** (`false`) | Specifies if DeviseTokenAuth should send and receive the auth token in a cookie.
| **`cookie_name`** (`"auth_cookie"`) | Sets the name of the cookie containing the auth token.
| **`cookie_attributes`** (`{}`) | Sets attributes for the cookie containing the auth token (ex. `domain`, `secure`, `httponly`, `same_site`, `expires`, `encrypt`). See [this Rails doc](https://api.rubyonrails.org/classes/ActionDispatch/Cookies.html) for what values can be passed to `ActionDispatch::Cookies`. See [this MDN doc](https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies) for additional information on HTTP cookie attributes.
| **`bypass_sign_in`** (`true`) | By default DeviseTokenAuth will not check user's `#active_for_authentication?` which includes confirmation check on each call (it will do it only on sign in). If you want it to be validated on each request (for example, to be able to deactivate logged in users on the fly), set it to false. |
| **`send_confirmation_email`** (`false`) | By default DeviseTokenAuth will not send confirmation email, even when including devise confirmable module. If you want to use devise confirmable module and send email, set it to true. (This is a setting for compatibility) |
| **`require_client_password_reset_token`** (`false`) | By default, the password-reset confirmation link redirects to the client with valid session credentials as querystring params. With this option enabled, the redirect will NOT include the valid session credentials. Instead the redirect will include a password_reset_token querystring param that can be used to reset the users password. Once the user has reset their password, the password-reset success response headers will contain valid session credentials. |
Expand Down
6 changes: 6 additions & 0 deletions lib/devise_token_auth/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ class Engine < ::Rails::Engine
:remove_tokens_after_password_reset,
:default_callbacks,
:headers_names,
:cookie_enabled,
:cookie_name,
:cookie_attributes,
:bypass_sign_in,
:send_confirmation_email,
:require_client_password_reset_token
Expand All @@ -47,6 +50,9 @@ class Engine < ::Rails::Engine
'expiry': 'expiry',
'uid': 'uid',
'token-type': 'token-type' }
self.cookie_enabled = false
self.cookie_name = 'auth_cookie'
self.cookie_attributes = {}
self.bypass_sign_in = true
self.send_confirmation_email = false
self.require_client_password_reset_token = false
Expand Down
43 changes: 28 additions & 15 deletions test/controllers/devise_token_auth/registrations_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,17 @@

class DeviseTokenAuth::RegistrationsControllerTest < ActionDispatch::IntegrationTest
describe DeviseTokenAuth::RegistrationsController do

def mock_registration_params
{
email: Faker::Internet.email,
password: 'secret123',
password_confirmation: 'secret123',
confirm_success_url: Faker::Internet.url,
unpermitted_param: '(x_x)'
}
end

describe 'Validate non-empty body' do
before do
# need to post empty data
Expand Down Expand Up @@ -41,13 +52,7 @@ class DeviseTokenAuth::RegistrationsControllerTest < ActionDispatch::Integration
@mails_sent = ActionMailer::Base.deliveries.count

post '/auth',
params: {
email: Faker::Internet.email,
password: 'secret123',
password_confirmation: 'secret123',
confirm_success_url: Faker::Internet.url,
unpermitted_param: '(x_x)'
}
params: mock_registration_params

@resource = assigns(:resource)
@data = JSON.parse(response.body)
Expand Down Expand Up @@ -87,24 +92,32 @@ class DeviseTokenAuth::RegistrationsControllerTest < ActionDispatch::Integration
before do
@original_duration = Devise.allow_unconfirmed_access_for
Devise.allow_unconfirmed_access_for = nil
post '/auth',
params: {
email: Faker::Internet.email,
password: 'secret123',
password_confirmation: 'secret123',
confirm_success_url: Faker::Internet.url,
unpermitted_param: '(x_x)'
}
end

test 'auth headers were returned in response' do
post '/auth', params: mock_registration_params
assert response.headers['access-token']
assert response.headers['token-type']
assert response.headers['client']
assert response.headers['expiry']
assert response.headers['uid']
end

describe 'using auth cookie' do
before do
DeviseTokenAuth.cookie_enabled = true
end

test 'auth cookie was returned in response' do
post '/auth', params: mock_registration_params
assert response.cookies[DeviseTokenAuth.cookie_name]
end

after do
DeviseTokenAuth.cookie_enabled = false
end
end

after do
Devise.allow_unconfirmed_access_for = @original_duration
end
Expand Down
49 changes: 39 additions & 10 deletions test/controllers/devise_token_auth/sessions_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@ class DeviseTokenAuth::SessionsControllerTest < ActionController::TestCase

describe 'success' do
before do
post :create,
params: {
email: @existing_user.email,
password: @existing_user.password
}
@user_session_params = {
email: @existing_user.email,
password: @existing_user.password
}

post :create, params: @user_session_params

@resource = assigns(:resource)
@data = JSON.parse(response.body)
Expand All @@ -35,17 +36,27 @@ class DeviseTokenAuth::SessionsControllerTest < ActionController::TestCase
assert_equal @existing_user.email, @data['data']['email']
end

describe 'using auth cookie' do
before do
DeviseTokenAuth.cookie_enabled = true
end

test 'request should return auth cookie' do
post :create, params: @user_session_params
assert response.cookies[DeviseTokenAuth.cookie_name]
end

after do
DeviseTokenAuth.cookie_enabled = false
end
end

describe "with multiple clients and headers don't change in each request" do
before do
# Set the max_number_of_devices to a lower number
# to expedite tests! (Default is 10)
DeviseTokenAuth.max_number_of_devices = 2
DeviseTokenAuth.change_headers_on_each_request = false

@user_session_params = {
email: @existing_user.email,
password: @existing_user.password
}
end

test 'should limit the maximum number of concurrent devices' do
Expand Down Expand Up @@ -159,6 +170,24 @@ def @controller.reset_session
test 'session was destroyed' do
assert_equal true, @controller.reset_session_called
end

describe 'using auth cookie' do
before do
DeviseTokenAuth.cookie_enabled = true
@auth_token = @existing_user.create_new_auth_token
@controller.send(:cookies)[DeviseTokenAuth.cookie_name] = { value: @auth_token.to_json }
end

test 'auth cookie was destroyed' do
assert_equal @auth_token.to_json, @controller.send(:cookies)[DeviseTokenAuth.cookie_name] # sanity check
delete :destroy, format: :json
assert_nil @controller.send(:cookies)[DeviseTokenAuth.cookie_name]
end

after do
DeviseTokenAuth.cookie_enabled = false
end
end
end

describe 'unauthed user sign out' do
Expand Down