Skip to content
This repository has been archived by the owner on Mar 22, 2021. It is now read-only.

Fix security flaw and add Soft(optional) authentication feature #171

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
13 changes: 11 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ _Note: the `authenticate_user` method uses the `current_user` method. Overwritin

You can do the exact same thing for any entity. E.g. for `Admin`, use `authenticate_admin` and `current_admin` instead.

If you're using a namespaced model, Knock won't be able to infer it automatically from the method name. Instead you can use `authenticate_for` directly like this:
If you're using a namespaced model, Knock won't be able to infer it automatically from the method name. Instead you can use `set_authenticate_for` directly like this:

```ruby
class ApplicationController < ActionController::Base
Expand All @@ -128,7 +128,7 @@ class ApplicationController < ActionController::Base
private

def authenticate_v1_user
authenticate_for V1::User
set_authenticate_for V1::User
end
end
```
Expand All @@ -143,6 +143,15 @@ Then you get the current user by calling `current_v1_user` instead of `current_u

### Customization

#### Soft (Optional) Authentication

For use in controllers where authentication is optional, use `soft_authenticate_user` instead of `authenticate_user` and use `set_soft_authenticate_for V1::User` instead of `set_authenticate_for V1::User` as in the examples in the [Usage section](#Usage) above.

Users will have access even if no token or an invalid token is passed.

NB: `current_user` will only be available if a valid token is passed and authenticated.


#### Via the entity model

The entity model (e.g. `User`) can implement specific methods to provide
Expand Down
17 changes: 15 additions & 2 deletions lib/knock/authenticable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,31 @@ def authenticate_for entity_class
public_send(getter_name)
end

def set_authenticate_for entity_class
unauthorized_entity(entity_class.to_s) unless authenticate_for entity_class
end

def set_soft_authenticate_for entity_class
authenticate_for entity_class
end

private

def token
params[:token] || token_from_request_headers
end

def method_missing(method, *args)
prefix, entity_name = method.to_s.split('_', 2)
values = method.to_s.split('_', 3)
if (values.size) == 3 && ("#{values.first}_#{values.second}" == 'soft_authenticate')
prefix, entity_name = 'soft_authenticate', values.last
else
prefix, entity_name = method.to_s.split('_', 2)
end
case prefix
when 'authenticate'
unauthorized_entity(entity_name) unless authenticate_entity(entity_name)
when 'current'
when 'current', 'soft_authenticate'
authenticate_entity(entity_name)
else
super
Expand Down
12 changes: 12 additions & 0 deletions test/dummy/app/controllers/admin_custom_auth_soft_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
class AdminCustomAuthSoftController < ApplicationController
before_action :soft_authenticate_admin

def index
head :ok
end

private
def soft_authenticate_admin
set_soft_authenticate_for Admin
end
end
13 changes: 13 additions & 0 deletions test/dummy/app/controllers/admin_custom_auth_strict_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
class AdminCustomAuthStrictController < ApplicationController
before_action :authenticate_admin


def index
head :ok
end

private
def authenticate_admin
set_authenticate_for Admin
end
end
9 changes: 9 additions & 0 deletions test/dummy/app/controllers/soft_admin_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class SoftAdminController < ApplicationController
before_action :soft_authenticate_admin


def index
head :ok
end

end
3 changes: 3 additions & 0 deletions test/dummy/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@
resource :current_user

resources :admin_protected
resources :soft_admin, only: [:index]
resources :composite_name_entity_protected
resources :custom_unauthorized_entity
resources :guest_protected
resources :protected_resources
resources :vendor_protected
resources :admin_custom_auth_strict, only: [:index]
resources :admin_custom_auth_soft, only: [:index]

namespace :v1 do
resources :test_namespaced
Expand Down
2 changes: 1 addition & 1 deletion test/dummy/db/migrate/20150713101607_create_users.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class CreateUsers < ActiveRecord::Migration
class CreateUsers < ActiveRecord::Migration[4.2]
def change
create_table :users do |t|
t.string :email, unique: true, null: false
Expand Down
2 changes: 1 addition & 1 deletion test/dummy/db/migrate/20160519075733_create_admins.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class CreateAdmins < ActiveRecord::Migration
class CreateAdmins < ActiveRecord::Migration[4.2]
def change
create_table :admins do |t|
t.string :email
Expand Down
2 changes: 1 addition & 1 deletion test/dummy/db/migrate/20160522051816_create_vendors.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class CreateVendors < ActiveRecord::Migration
class CreateVendors < ActiveRecord::Migration[4.2]
def change
create_table :vendors do |t|
t.string :email
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class CreateCompositeNameEntities < ActiveRecord::Migration
class CreateCompositeNameEntities < ActiveRecord::Migration[4.2]
def change
create_table :composite_name_entities do |t|
t.string :email
Expand Down
2 changes: 1 addition & 1 deletion test/dummy/db/migrate/20161127203222_create_v1_users.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class CreateV1Users < ActiveRecord::Migration
class CreateV1Users < ActiveRecord::Migration[4.2]
def change
create_table :v1_users do |t|

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
require 'test_helper'

class AdminCustomAuthSoftControllerTest < ActionController::TestCase
def valid_auth
@admin = admins(:one)
@token = Knock::AuthToken.new(payload: { sub: @admin.id }).token
@request.env['HTTP_AUTHORIZATION'] = "Bearer #{@token}"
end

def invalid_token_auth
@token = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9'
@request.env['HTTP_AUTHORIZATION'] = "Bearer #{@token}"
end

def invalid_entity_auth
@token = Knock::AuthToken.new(payload: { sub: 0 }).token
@request.env['HTTP_AUTHORIZATION'] = "Bearer #{@token}"
end

test "responds with success" do
get :index
assert_response :success
end

test "responds with success to invalid token" do
invalid_token_auth
get :index
assert_response :success
end

test "responds with success to invalid entity" do
invalid_entity_auth
get :index
assert_response :success
end

test "responds with success if authenticated" do
valid_auth
get :index
assert_response :success
end

test "has a current_admin after authentication" do
valid_auth
get :index
assert_response :success
assert @controller.current_admin.id == @admin.id
end

test "has no current_admin if no authentication" do
get :index
assert_response :success
assert @controller.current_admin.nil?
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
require 'test_helper'

class AdminCustomAuthStrictControllerTest < ActionController::TestCase
def valid_auth
@admin = admins(:one)
@token = Knock::AuthToken.new(payload: { sub: @admin.id }).token
@request.env['HTTP_AUTHORIZATION'] = "Bearer #{@token}"
end

def invalid_token_auth
@token = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9'
@request.env['HTTP_AUTHORIZATION'] = "Bearer #{@token}"
end

def invalid_entity_auth
@token = Knock::AuthToken.new(payload: { sub: 0 }).token
@request.env['HTTP_AUTHORIZATION'] = "Bearer #{@token}"
end

test "responds with unauthorized" do
get :index
assert_response :unauthorized
end

test "responds with unauthorized to invalid token" do
invalid_token_auth
get :index
assert_response :unauthorized
end

test "responds with unauthorized to invalid entity" do
invalid_entity_auth
get :index
assert_response :unauthorized
end

test "responds with success if authenticated" do
valid_auth
get :index
assert_response :success
end

test "has a current_admin after authentication" do
valid_auth
get :index
assert_response :success
assert @controller.current_admin.id == @admin.id
end
end
55 changes: 55 additions & 0 deletions test/dummy/test/controllers/soft_admin_controller_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
require 'test_helper'

class SoftAdminControllerTest < ActionController::TestCase
def valid_auth
@admin = admins(:one)
@token = Knock::AuthToken.new(payload: { sub: @admin.id }).token
@request.env['HTTP_AUTHORIZATION'] = "Bearer #{@token}"
end

def invalid_token_auth
@token = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9'
@request.env['HTTP_AUTHORIZATION'] = "Bearer #{@token}"
end

def invalid_entity_auth
@token = Knock::AuthToken.new(payload: { sub: 0 }).token
@request.env['HTTP_AUTHORIZATION'] = "Bearer #{@token}"
end

test "responds with success" do
get :index
assert_response :success
end

test "responds with success to invalid token" do
invalid_token_auth
get :index
assert_response :success
end

test "responds with success to invalid entity" do
invalid_entity_auth
get :index
assert_response :success
end

test "responds with success if authenticated" do
valid_auth
get :index
assert_response :success
end

test "has a current_admin after authentication" do
valid_auth
get :index
assert_response :success
assert @controller.current_admin.id == @admin.id
end

test "has no current_admin if no authentication" do
get :index
assert_response :success
assert @controller.current_admin.nil?
end
end