simple_token_authentication icon indicating copy to clipboard operation
simple_token_authentication copied to clipboard

fallback: :exception with JSON returns empty error message

Open iampeter opened this issue 9 years ago • 6 comments

I am forcing JSON response and using fallback: :exception, and the response is

{
"error": ""
}

although the code in ExceptionCallbackHandler says

throw(:warden, scope: entity.name_underscore.to_sym) if controller.send("current_#{entity.name_underscore}").nil?

Why doesn't it return any message?

I have tried doing it this way:

  class UnauthenticatedException < Exception
  end

  rescue_from UnauthenticatedException do |exception|
    render json: { error: "Authentication error", code: 1 }, status: 401
  end

  class ApiExceptionFallbackHandler
    # Notifies the failure of authentication to Warden in the same DEvise does.
    # Does result in an HTTP 401 response in a Devise context.
    def fallback!(controller, entity)
      raise UnauthenticatedException.new if controller.send("current_#{entity.name_underscore}").nil?
    end
  end

  @@fallback_authentication_handler = ApiExceptionFallbackHandler.new

  acts_as_token_authentication_handler_for ApiKey

and it works, but am bot sure if raising and Exception instead of throw(:warden) is a legitimate way here.

iampeter avatar Nov 14 '15 18:11 iampeter

Hi @iampeter,

I'm reading about this, but it seems (see these examples) that you can provide a message when thowing the :warden symbol. I would try:

# lib/simple_token_authentication/exception_fallback_handler.rb

module SimpleTokenAuthentication
  class ExceptionFallbackHandler
    # Notifies the failure of authentication to Warden in the same Devise does.
    # Does result in an HTTP 401 response in a Devise context.
    def fallback!(controller, entity)
      throw(:warden, scope: entity.name_underscore.to_sym, message: 'Invalid authentication credentials.') if controller.send("current_#{entity.name_underscore}").nil?
    end
  end
end

Indeed, message: :invalid_token_authentication_credentials might be enough if you define the message in your locales file (idea).

Unless I'm wrong, Devise will get this message all the way through your JSON response.

gonzalo-bulnes avatar Nov 15 '15 03:11 gonzalo-bulnes

About raising an exception vs. throwing a symbol: I found all three answers to this SO question helpful.

In this case, having to handle uncorrect or missing credentials is expected, and undoubtly part of the token authentication job. That's why I would avoid relying on exceptions to do it. That being said, I was not familiar with the distinction myself and I think I'll refactor some code here and there ; )

gonzalo-bulnes avatar Nov 15 '15 04:11 gonzalo-bulnes

Hi @iampeter,

Were you able to try passing a message when throwing :warden?

gonzalo-bulnes avatar Jan 27 '16 16:01 gonzalo-bulnes

@gonzalo-bulnes nope. that's why I'm doing it like mentioned in my original comment

iampeter avatar Jan 28 '16 10:01 iampeter

@iampeter @gonzalo-bulnes Is this issue is fixed and released? I am also getting following error message for invalid email and authentication token.

{
    "error": ""
}

gkunwar avatar Aug 11 '16 11:08 gkunwar

Hi @gkunwar,

No, I haven't been able to dig further and understand how Devise does handle the error messages through Warden. If you can progress towards understanding what is missing in the suggestion I wrote above, I'd be glad to work with you to make that a releasable fix. (@krazedkrish That's true for you too!)

While I find @iampeter's workaround fine and clean, it is a workaround and does not really fix the issue.

Indeed, another workaround could be to use an after_failed_token_authentication hook. That wouldn't fix the issue either, but is a more general piece of code that it could make sense to release independently. See #217.

In both cases, I'm happy to spend time understanding the issue and the Devise code better, but I need help and feedback from people experiencing the issue to make sure we don't release anything that wouldn't make sense in practice. Does it sound fair?

gonzalo-bulnes avatar Aug 12 '16 05:08 gonzalo-bulnes