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

split responsibilities of acts_as_token_authentication_handler_for method #379

Closed
mtoygar opened this issue Jan 15, 2021 · 1 comment
Closed

Comments

@mtoygar
Copy link

mtoygar commented Jan 15, 2021

as far as I understand acts_as_token_authentication_handler_for does 2 things. First, it does add authenticate_#{entity.name_underscore}_from_token method. Second, uses one of these methods as a controller filter.

This approach becomes a problem when an app supports more than one authentication method. Our app needs to decide the authentication method using the headers on the request. We have a filter with the name authenticate_user_with_multiple_system!. This method actually authenticates the user either with JWT, doorkeeper or devise. However when we use acts_as_token_authentication_handler_for method another filter is automatically added. I believe that the api of this gem would be more flexible and beneficial if acts_as_token_authentication_handler_for method only defines the filter hooks and application developer use the filter method explicitly later on.

Below I share a code snippet to demonstrate the auth logic with more than one authentication method

def authenticate_user_with_multiple_system!
  if valid_doorkeeper_header?
    # auth with doorkeeper
  elsif valid_devise_header?
    # auth with devise
    # use `authenticate_user_from_token` method here
  else
    # unauthorized
  end
end

With the current API authenticate_user_from_token or authenticate_user_from_token! methods runs no matter what. We can skip it using skip_before_action but I believe that using authenticate_user_from_token directly is way more explicit.

My current workaround (Actually I dont try this yet but I believe that it should work)

skip_before_action :authenticate_user_from_token # or authenticate_user_from_token! depending on the fallback
before_action authenticate_user_with_multiple_system!
@mtoygar
Copy link
Author

mtoygar commented Jan 18, 2021

migrated to discussions
#381

@mtoygar mtoygar closed this as completed Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant