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

JWT authentication #70

Closed
wants to merge 14 commits into from
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,20 @@ login_from(provider) # Tries to login from the external provider's callback
create_from(provider) # Create the user in the local app database
```

### JWT authentication

```ruby
jwt_require_auth # This is a before action
jwt_auth(email, password) # => return json web token
jwt_encode # This method creating JWT token by payload
jwt_decode # This method decoding JWT token
jwt_from_header # Take token from header, by key defined in config
jwt_user_data(token = jwt_from_header) # Return user data which decoded from token
jwt_user_id # Return user id from user data if id present.
jwt_not_authenticated # This method called if user not authenticated

```

### Remember Me

```ruby
Expand Down
34 changes: 33 additions & 1 deletion lib/generators/sorcery/templates/initializer.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# The first thing you need to configure is which modules you need in your app.
# The default is nothing which will include only core features (password encryption, login/logout).
# Available submodules are: :user_activation, :http_basic_auth, :remember_me,
# :reset_password, :session_timeout, :brute_force_protection, :activity_logging, :external
# :reset_password, :session_timeout, :brute_force_protection, :activity_logging, :external, :jwt_auth
Rails.application.config.sorcery.submodules = []

# Here you can configure each submodule's features.
Expand Down Expand Up @@ -442,6 +442,38 @@
# Default: `:uid`
#
# user.provider_uid_attribute_name =

# -- jwt_auth --

# Parameters passed for generating payload part of token
# Default: [:id]
#
# config.jwt_user_params =

# Header name which will parsed
# Default: `Authorization`
#
# config.jwt_headers_key =

# Key on which returned user data
# Default: :user_data
#
# config.jwt_user_data_key =

# Key on which returned token
# Default: :auth_token
#
# config.jwt_auth_token_key =

# A flag that specifies whether to perform a database query to set the current_user
# Default: true
#
# config.jwt_set_user =

# Secret key for token generation
# Default: nil
#
# config.jwt_secret_key = '<%= SecureRandom.hex(64) %>'
end

# This line must come after the 'user config' block.
Expand Down
1 change: 1 addition & 0 deletions lib/sorcery.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ module Submodules
require 'sorcery/controller/submodules/http_basic_auth'
require 'sorcery/controller/submodules/activity_logging'
require 'sorcery/controller/submodules/external'
require 'sorcery/controller/submodules/jwt_auth'
end
end

Expand Down
16 changes: 15 additions & 1 deletion lib/sorcery/controller/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ class << self
attr_accessor :after_failed_login
attr_accessor :before_logout
attr_accessor :after_logout
attr_accessor :jwt_user_params
attr_accessor :jwt_headers_key
attr_accessor :jwt_user_data_key
attr_accessor :jwt_auth_token_key
# If true, will set user by request to db.
# If false will use data from jwt_user_params without executing db requests.
attr_accessor :jwt_set_user
attr_accessor :jwt_secret_key

def init!
@defaults = {
Expand All @@ -30,7 +38,13 @@ def init!
:@before_logout => [],
:@after_logout => [],
:@save_return_to_url => true,
:@cookie_domain => nil
:@cookie_domain => nil,
:@jwt_user_params => [:id],
:@jwt_headers_key => 'Authorization',
:@jwt_user_data_key => :user_data,
:@jwt_auth_token_key => :auth_token,
:@jwt_set_user => true,
:@jwt_secret_key => ''
}
end

Expand Down
75 changes: 75 additions & 0 deletions lib/sorcery/controller/submodules/jwt_auth.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
module Sorcery
module Controller
module Submodules
module JwtAuth
def self.included(base)
base.send(:include, InstanceMethods)
end

module InstanceMethods
# This method return generated token if user can be authenticated
def jwt_auth(*credentials)
user = user_class.authenticate(*credentials)
if user
user_params = Config.jwt_user_params.each_with_object({}) do |val, acc|
acc[val] = user.public_send(val)
end
{ Config.jwt_user_data_key => user_params,
Config.jwt_auth_token_key => jwt_encode(user_params) }

Choose a reason for hiding this comment

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

Maybe it's an idea to replace user_id with a jti (SecureRandom.uuid) and implement a whitelisting strategy to support login from multiple devices and independent logout. This implies that on login, a record is created in the whitelists table and that on logout this record gets deleted.

Also i'm missing expiration from this implementation. The generated jwt keys are valid indefinitely. The duration a key is persisted could be set from the config for instance.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jovanmaric I don't know why user_id is here though the token includes user_id as the subject claim ( of course, it's needed to decode to acquire it ) .

I'd like to remove and only returns a token like eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJzdWIiOjQyLCJleHAiOjE1NDc3MTkyMDAsImlhdCI6MTU0NzQ2MDAwMH0.QM5mTkYiDwI-10cEOq4b_bfrwe99BRuef6pnIB-jqIk.

What do you think about it?

And I would appreciate it if @wilddima could answer why Config.jwt_user_data_key => user_params is needed.

end
end

# To be used as a before_action.
def jwt_require_auth
jwt_not_authenticated && return unless jwt_user_id

@current_user = Config.jwt_set_user ? User.find(jwt_user_id) : jwt_user_data
rescue JWT::VerificationError, JWT::DecodeError
jwt_not_authenticated && return
end

# This method creating JWT token by payload
def jwt_encode(payload)
JWT.encode(payload, Config.jwt_secret_key)
Copy link
Contributor

Choose a reason for hiding this comment

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

You should specify an algorithm to use, else the created JWT is unsigned!

See here: https://github.com/jwt/ruby-jwt#algorithms-and-usage

I think it should be a config option, with "HS256" as its default value.

The same algorithm parameter has to be passed to JWT.decode as well.

end

# This method decoding JWT token
# Return nil if token incorrect
def jwt_decode(token)
HashWithIndifferentAccess.new(
JWT.decode(token, Config.jwt_secret_key)[0]
)
rescue JWT::DecodeError
nil

Choose a reason for hiding this comment

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

Shouldn't this be removed, considering that def jwt_require_auth catches JWT::DecodeError, and omits an unauthorized_error?

Copy link
Contributor

Choose a reason for hiding this comment

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

@jovanmaric
I agree with that and changed like this:
9e02135

end

# Take token from header, by key defined in config
# With memoization
def jwt_from_header
@jwt_header_token ||= request.headers[Config.jwt_headers_key]
end

# Return user data which decoded from token
# With memoization
def jwt_user_data(token = jwt_from_header)
@jwt_user_data ||= jwt_decode(token)
end

# Return user id from user data if id present.
# Else return nil
def jwt_user_id
jwt_user_data.try(:[], :id)
end

# This method called if user not authenticated
def jwt_not_authenticated
respond_to do |format|
format.html { not_authenticated }
format.json { render json: { status: 401 }, status: 401 }
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO the message should not be the status code because it's verbose.
It would be better if the api response is like this (just an example)

format.json {
  render json: {
    "error": {
      "message": "Need to login first.",
    }
  }, 
  status: 401
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I also suggest you use status: :unauthorized to make it more explicit.

Choose a reason for hiding this comment

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

How about making this changeable, instead of hardcoding the return message

Copy link
Contributor

Choose a reason for hiding this comment

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

It's difficult to make an error message changeable because of the design. The cases are different when this method is called.

I use ruby-jwt error messages and I want it to be overridden if someone wants to change the message.

See this commit for details: ebihara99999@9e02135

end
end
end
end
end
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to set the new line.

1 change: 1 addition & 0 deletions sorcery.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Gem::Specification.new do |s|
s.add_dependency 'oauth', '~> 0.4', '>= 0.4.4'
s.add_dependency 'oauth2', '~> 1.0', '>= 0.8.0'
s.add_dependency 'bcrypt', '~> 3.1'
s.add_dependency 'jwt', '~> 1.5'

Choose a reason for hiding this comment

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

The latest is 2.1.0


s.add_development_dependency 'yard', '~> 0.6.0'
s.add_development_dependency 'timecop'
Expand Down
92 changes: 92 additions & 0 deletions spec/controllers/controller_jwt_auth_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
require 'spec_helper'

describe SorceryController, type: :controller do
let!(:user) { double('user', id: 42) }
before(:each) do
request.env['HTTP_ACCEPT'] = "application/json" if ::Rails.version < '5.0.0'
end

describe 'with jwt auth features' do
let(:user_email) { 'test@test.test' }
let(:user_password) { 'testpass' }
let(:auth_token) { 'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpZCI6NDJ9.rrjj-sXvbIjT8y4MLGb88Cv7XvfpJXj-HEgaBimT_-0' }
let(:response_data) do
{
user_data: { id: user.id },
auth_token: auth_token
}
end

before(:all) do
sorcery_reload!([:jwt_auth])
end

describe '#jwt_auth' do
context 'when success' do
before do
allow(User).to receive(:authenticate).with(user_email, user_password).and_return(user)

post :test_jwt_auth, params: { email: user_email, password: user_password }
end

it 'assigns user to @token variable' do
expect(assigns[:token]).to eq response_data
end
end

context 'when fails' do
before do
allow(User).to receive(:authenticate).with(user_email, user_password).and_return(nil)

post :test_jwt_auth, params: { email: user_email, password: user_password }
end

it 'assigns user to @token variable' do
expect(assigns[:token]).to eq nil
end
end
end

describe '#jwt_require_auth' do
context 'when success' do
before do
allow(User).to receive(:find).with(user.id).and_return(user)
allow(user).to receive(:set_last_activity_at)
end

it 'does return 200' do
request.headers.merge! Authorization: auth_token

get :some_action_jwt, format: :json

expect(response.status).to eq(200)
end
end

context 'when fails' do
let(:user_email) { 'test@test.test' }
let(:user_password) { 'testpass' }

context 'without auth header' do
it 'does return 401' do
get :some_action_jwt, format: :json

expect(response.status).to eq(401)
end
end

context 'with incorrect auth header' do
let(:incorrect_header) { '123.123.123' }

it 'does return 401' do
request.headers.merge! Authorization: incorrect_header

get :some_action_jwt, format: :json

expect(response.status).to eq(401)
end
end
end
end
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to set the new line.

10 changes: 10 additions & 0 deletions spec/rails_app/app/controllers/sorcery_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ class SorceryController < ActionController::Base

before_action :require_login_from_http_basic, only: [:test_http_basic_auth]
before_action :require_login, only: [:test_logout, :test_logout_with_force_forget_me, :test_should_be_logged_in, :some_action]
before_action :jwt_require_auth, only: [:some_action_jwt]

def index; end

Expand Down Expand Up @@ -367,4 +368,13 @@ def test_create_from_provider_with_block
redirect_to 'blu', alert: 'Failed!'
end
end

def test_jwt_auth
@token = jwt_auth(params[:email], params[:password])
head :ok
end

def some_action_jwt
head :ok
end
end
2 changes: 2 additions & 0 deletions spec/rails_app/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,7 @@
post :test_login_with_remember
get :test_create_from_provider_with_block
get :login_at_test_with_state
post :test_jwt_auth
get :some_action_jwt
end
end