nsarno / knock

Seamless JWT authentication for Rails API
MIT License
2.07k stars 253 forks source link

Namespaced model could cause a severe security issue if implemented as the doc suggests #228

Open serco-chen opened 6 years ago

serco-chen commented 6 years ago

This is what doc suggests

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:


class ApplicationController < ActionController::Base
include Knock::Authenticable

private

def authenticate_v1_user authenticate_for V1::User end end

class SecuredController < ApplicationController before_action :authenticate_v1_user end



This gem relies on [method_missing](https://github.com/nsarno/knock/blob/master/lib/knock/authenticable.rb#L14) to do the actuall authentication work. 

However `authenticate_v1_user` defined in `ApplicationController` will override it and return a `nil` when lacking a valid token, what you really need is a `head(:unauthorized)` response.

I could be wrong since I'm not familiar with the gem. IMO this is a big security issue.
fabio-padovani89 commented 6 years ago

I totally agree with @serco-chen.

I suggest you to update the documentation with something like:

def authenticate_v1_user
  unauthorized_entity('V1::User') unless authenticate_entity('V1::User')
end