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

Rails API Application compatibility (and support) #4947

Closed
colinross opened this issue Sep 27, 2018 · 14 comments
Closed

Rails API Application compatibility (and support) #4947

colinross opened this issue Sep 27, 2018 · 14 comments

Comments

@colinross
Copy link
Contributor

This issue is meant to track compatibility and support for 'Rails API Applications' by which I specifically mean Rails applications built with API only support, ala

Rails.application.config.api_only = true
Reference: https://guides.rubyonrails.org/api_app.html

Current behavior

With a boilerplate rails app built with api-only support (rails new my_api --api ) devise throws some errors under testing, mainly from missing middleware that is not provided for api-only applications.
At first look, the main culprits tend to be the flash middleware and redirect/cookie middlewares.

Expected behavior

Devise should treat all requests as 'non-navigational' and 'non-flashing' if loaded within a Rails api-only application without configuration (assuming it can be done while maintaining full compatibility with a 'full' rails application)

Proposal

I've looked into this issue with a dummy rails-api app, and as I pointed out about, much of it has to do with missing middleware. Luckily, most of the calls to that middleware are guarded in the current code by the helper methods is_navigational_format? and is_flashing_format? (the later is by default just a call to the former). I'm suggesting that we make is_flashing_format? aware of the type of application it is loaded into.

How to determine the nature of the rails application is a bit tricky, which at least 3 ways I can see:

  1. Presence of and value of Rails.application.config.api_only
  2. Detecting loaded middleware
  3. Detecting the parent controller

Detecting Rails.application.config.api_only can be troublesome as many applications could be hybrids, consisting of both api-only and non-api controllers. This also puts a big assumption that legacy applications have properly and fully migrated over their configuration.

Detecting loaded middleware is also troublesome, as we can't tell if the middleware is loaded due to the nature of the controller, or, like devise does itself, loaded manually as a 'helper library' of behavior.

Detecting the parent controller I think, is the best solution with the least probability of false positives. Not constrained to just the direct parent ala Devise.parent_controller.constantize, but we should be able to say that if ActionController::API is in the hierarchy, we should treat it as an API controller, and not attempt to flash or set cookies (API controllers allow for redirects).

Again, my goal is not to change devise to be api-centric, just to make it reasonably work with api apps out of the box without errors.

@moyuanhuang
Copy link

+1 I'm trying to use devise as the authentication solution for an API only rails app and was stuck setting it up. Would be really nice if Devise can support API only rails app. @colinross In the meantime, devise_token_auth might be an alternative if you really need it.

@colinross
Copy link
Contributor Author

colinross commented Oct 3, 2018

I'm progressing with checking out the current situation in more detail along with a PR for a small tweak on the is_flashing_format? method (in the devise controller and the default failure app).
I will also include a note in the docs about installing devise ontop of a rails api application.

If we are just talking about 'not throwing errors' level of compatibility, along with the tweaks to is_flashing_format?, you only really need to add respond_to :json to your ApplicationController (or w/e controller Devise inherits from) in order for the responders part to work.
edit: Oh, and setting Accept: 'application/json' in your request.

So far in my testing (I've done password resets, login/logout, doing registration next). You don't hit any errors, and while some responses may just be blank since they don't have flashing, it still works, albeit with less than optimal messaging.

The biggest crux I've encountered so far is how to handle sessions, which I think is really where this discussion should focus, but for the future.

I say future because if you enable HTTP auth, you don't really need sessions and I see that as acceptable from a base compatibility point of view. HTTP auth isn't the best practice or most secure way but assuming use under https, I would assert it is enough for compatibility-level use.

With a fairly trivial pair of configuration changes
config.http_authenticatable = true and config.http_authenticatable_on_xhr = false you can get it working. I'm not too savy with the xhr code so i'm not even sure that is a requirement either.

  • Clean up tweaks and validate tests (and add some for the situation)
  • Update docs about adding respond_to :json; (use of http auth for compatibilty?)

@tegon
Copy link
Member

tegon commented Oct 3, 2018

Hi, @colinross thanks for digging into this.

I've also used in the past and didn't get many issues either. Now, I don't think we should support other authentication options - e.g. token-based, JWT, and so on - which I think is mostly what people are looking for when they ask for this. Those are very application-specific so we'd have to come up with a good API that would allow people to extend it without much trouble.
The problem is that it would be very hard to add that kind of support with the code we have today, so we'd probably end up rewriting a lot of stuff. I think Devise does a good job as an authentication gem for HTML applications, so we can keep it that way.

The idea to add documentation explaining how to add that custom behavior is very good though. We could include the things you mentioned and also that some custom authentication methods (e.g. token) can be achieved by using a custom Warden strategy.

So I'd say that for your PR fixing the cases where is_flashing_format? is not being called as you mentioned is enough. Later we can include that documentation in the Wiki.

@qortex
Copy link

qortex commented Oct 28, 2018

Just did that too. @colinross : true, that's how I went. I'll try to overwrite the flash_message methods to add their content to the responses. I guess it could help.

For authentication, I then use Doorkeeper to get oauth tokens, and it works well. Kind of tricky to get everything to work. But you then get both Devise features, and stateless authentication with Doorkeeper which is nice for the API behaviour.

The main roadblock I encountered was this: doorkeeper-gem/doorkeeper#1160. Don't know if the compatibility issue should be fixed on Devise or Doorkeeper side though.

@tegon
Copy link
Member

tegon commented Nov 13, 2018

Closed via #4950

@emersonthis
Copy link
Contributor

emersonthis commented Jul 16, 2019

I was about to open an issue proposing documentation on how to run Devise in (Rails) API mode, but I stumbled onto this thread first...

I've been using Devise to authenticate my API app that has a React front end. Aside from a couple of the "gotchas" mentioned above, it seemed to work pretty well right out of the box. I started looking for documentation because I seem to keep having to re-authenticate on mobile safari, but aside from that issue, it has worked so well that I never realize it wasn't supposed to be supported, which is what I understood from this discussion as well as this one.

If Devise does not support Rails API mode, maybe we could update the docs to be a little more explicit about that. I submitted a documentation PR a while back mentioning one of the "gotchas" associated with API mode, and I didn't even realize that was an unsupported use case. At the very least, maybe that part of the docs should be expanded to clearly state that Devise is not recommended for API mode. Or if I misunderstood, and it is supported, we could talk a bit about how that should work (which configurations, complimentary gems, etc).

There's a lot of conflicting information out there for someone Googling this issue, and I didn't find anything in the official docs.

So in conclusion...
Regardless of what the official position is on API mode, it feels like the official docs should become the authoritative source of truth on the matter one way or the other. I'd be happy to draft something once I feel like I personally understand the position on this topic.

@tegon
Copy link
Member

tegon commented Aug 1, 2019

@emersonthis Throughout history, it seems like patches were accepted to fix some of those gotchas but there was never a big effort to really support the API mode. That's why I say it's not officially supported. I really don't know the existing limitations and issues with applications that use the library in that way. However, since there were patches accepted in the past to make the life of those users easier, I do accept changes like the one you sent before that might help them.

Unfortunately, I can't tell you the history reasons since I only started to maintain the Gem in the past two years.

I agree with you that stating this in the documentation would be great to make it more clear to everyone else. If you can send us a pull request with those changes, I'd happy to review it.

Thanks and sorry for the trouble.

@emersonthis
Copy link
Contributor

Sounds like we basically want to incorporate what you said above into the docs. If I understood correctly, the key points would be:

  • The library might work in API mode, but we're not entirely sure...
  • The primary focus is form-based authentication using cookies
  • There's other libraries out there that add support for tokens, etc... which might make more sense for an API project

Does that sound about right?

@tegon
Copy link
Member

tegon commented Aug 2, 2019

@emersonthis Yep, that sounds good!

@colinross
Copy link
Contributor Author

Not to be overly pedantic here, but I feel that support for API mode is there in the sense that a vanilla Rails app running in api-mode with Devise loaded should not conflict with each others functionality. That is not that same as out-of-the-box support for directly using Devise as the authentication layer for your API, as I think it is a slippery slope for Devise to dictate API-authentication strategies. The fact is that there are countless ways (strategies) for authentication of APIs.

I feel this discussion is very much akin to previous ones about having 'token authentication' baked into Devise as a strategy, which (imho correctly) was removed ( http://blog.plataformatec.com.br/2013/08/devise-3-1-now-with-more-secure-defaults/ )
Warden is agnostic to how you authenticate while providing a great framework.
Devise gets a bit more opinionated on this (email/password and adding much of the crud of user records), but if you want to go beyond username/password, you must provide your own strategy.

Many popular, opinionated strategies get bundled up as gems that enhanced Devise, and that is great, as it allows Devise to not have to be the chooser of winners/loosers as far as strategies and authentication algorithms go, nor having to maintain specific strategies.

Devise supports usage in api-mode, what it doesn't support/provide is an API-specific strategy baked into the gem.

@emersonthis
Copy link
Contributor

emersonthis commented Aug 2, 2019 via email

@colinross
Copy link
Contributor Author

colinross commented Aug 6, 2019 via email

@tegon
Copy link
Member

tegon commented Aug 30, 2019

@emersonthis @colinross You both made great points. About the following:

the library should be compatible with Rails 'API' mode

When you say compatible do you mean that it shouldn't raise any exceptions?

I know that some of you have used the Gem with API mode and confirmed that it works to a certain degree, but it seems to me that the effort to support non-HTML requests has been small throughout the time (though I don't know the historical reasons). Some patches were accepted to fix specific issues, but there are not many tests for the JSON format; there's no dummy app that runs in Rails API mode and some of the conditionals added to not perform redirects in API mode did not include tests.

So I'm not confident that things are going to keep working after new features of bug fixes are added.
That's my main concern of saying that the API mode is supported or that Devise is compatible with it. As long as we define what we mean with compatible, or that we actually include more tests for the behaviors we wish should be supported, I think it's ok.

@emersonthis
Copy link
Contributor

emersonthis commented Aug 31, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants