knock icon indicating copy to clipboard operation
knock copied to clipboard

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

Open serco-chen opened this issue 6 years ago • 1 comments

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 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.

serco-chen avatar Oct 16 '18 06:10 serco-chen

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

fabio-padovani89 avatar Oct 18 '18 16:10 fabio-padovani89