per-Controller auth strategies

[braindeaf]

Adding BlahController.strategies config to allow only specific strategies to be run on a per-controller basis. The best example of this is we have at least 5 strategies for our User class, but when it comes to our API controllers we only want :jwt to run, ignoring database_authenticatable, yubi_key, etc.

This was my initial solution, before this pull request.

https://robl.me/posts/the-magical-devise-journey

[xijo]

I'd like to bump this PR.

We have the same use case: API should only use token authentication, rest of the app tries all approaches. And I'd guess we're not be the only ones..

Anything speaking against integrating it into devise? Happy to help with the PR in case anything is still missing.

[braindeaf]

We are using this branch in production so it's been really invaluable.

[oehlschl]

Thoughts on making this a controller instance method as opposed to a class attribute? Agreed that this would be useful -- I'm in the middle of implementing something similar for a multi-tenant app with a single scope but a few separate auth mechanisms -- but I think it could be helpful to have the request context available if possible.

I realize Warden encourages you to locate strategy eligibility in the strategy itself, but if we did want to pick explicitly in the controller based on the request or some runtime information, that would be possible with an instance method but not a class attribute.

Making this an instance method would also make this function a little closer to the way the auth_options work in Devise::SessionsController#create.: https://github.com/heartcombo/devise/blob/main/app/controllers/devise/sessions_controller.rb#L47

[braindeaf]

That seems perfectly reasonable. I'll take a look.

[braindeaf]

changed on your recommendation

[oehlschl]

You'll likely also want to respect any per-controller strategies here (and add self.class.strategies_for to this .authenticate call in Devise::SessionsController#create):

https://github.com/heartcombo/devise/blob/main/app/controllers/devise/sessions_controller.rb#L19

[oehlschl]

I guess that Devise::SessionsController probably doesn't include these controller helpers, but as part of this work, it may be worth considering how to allow custom strategies in that controller. (It would be a mildly more complete solution, even if it's just adding an empty auth_strategies or strategies method to that controller and respecting it in the authenticate call.)

[braindeaf]

it is now possible to override the SessionsController too :)

[oehlschl]

Just for the record, another solution I've seen encouraged for this problem is to set something in the Rails/request env in the controller and check it in the Strategy. Ex:

class Api::Users::SessionsController
  prepend_before_action :skip_token_strategy!, only: :create
  
  private def skip_token_strategy!
    request.env['devise.skip_token_strategy'] = true
  end
end

class Devise::Strategies::Token
  def valid?
    !env['devise.skip_token_strategy']
  end
end

Not suggesting anything one way or the other; just anticipating what other responses to this proposal might be. (Ex: "This can already be done, and strategy eligibility / validity should be determined in the Strategy itself.")

[braindeaf]

Not sure how I feel about that solution. A Strategy should be able to understand what is valid or not sure. Say, if it's a JWT token request and maybe forcing the request to be JSON. That I could understand. But deciding on whether to force the request to be invalid for some other arbitrary reason seems unreasonable to inject a reason to cause the Strategy to fail, not to mention that it will run every Strategy which is unnecessary if you know you only want to run one of them.

Not sure what the hell has happened after my merge of upstream, now I have 23 file changes :S