-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Sharing Devise on Multiple Engines #2827
Comments
Thanks @Bharat311! Question: Why does Devise need to be mounted multiple times? Do you really want a different authentication end-point for each engine? This proposed change is definitely a big change on how Devise works and given the use case, I am more inclined to say it is an use case we don't support in Devise. |
@josevalim, thanks for the quick response. In my application, we have several small apps that are using the same backend. The functionality of the sub-apps are mutually exclusive so we have separated them out to engines. And yes, we really want a different authentication end-point for each of them. Actually we have 3 different roles in our app - Customer, Merchant and Admin and each of these roles has been extracted out to a separate engine as they perform features which are unique to them, so it not ideal to have them in the same app. Using separate engines for each of them, we are able to segregate the functionality, and it makes a lot of sense as we have dedicated engine for each role. I did some changes to Devise in my own fork. I would be obliged if you take a look and let me know your views on them. Its been sometime since we have been using it successfully on production. https://github.com/Bharat311/devise/tree/f-mixin |
I have added some comments. It seems a nice solution that still keeps the best of both worlds but I am still unsure due to the meta-programming involved. :( |
@josevalim, oh sorry, forgot to add the file. Just added it: |
Hi @josevalim, i think we can remove the metaprogramming altogether by removing the generator class. We could provide a config variable generate_controllers (defaults: true) which will define whether Devise provides the controllers. ( Though, I am not sure as to what could be the better way to do this. ) Now instead of having definitions in the controllers we can have them in the mixins and have devise controllers file like:
For cases such as mine, we simply need to set the generate_controllers to false and define our own controllers per engine using the mixins.
This way making minimum changes to the existing structure of Devise, we are also solving our issue. What are your views regarding this? The general idea could be taken from the code here: |
@Bharat311 I am sorry but we decided to not merge those changes. It is a very specific corner case but it adds a bunch of complexity to Devise source. Thank you for the discussion! |
I am running into the same problem :( |
@abezzub you can check out our Devise fork here which supports this https://github.com/Bharat311/devise/tree/f-mixin (note In order to use it, the routes for each user model will need to live in their own Rails Engine. Then you can specify the engines in # The engines which should be Devise-aware:
config.controller_scopes = [:my_engine, :engine_b, :engine_c] In each engine's MyEngine::Engine.routes.draw do
devise_for :customer_users, class_name: 'MyNamespace::CustomerUser', path: '/users', controllers: {
registrations: 'engine_a/registrations',
sessions: 'engine_a/sessions',
confirmations: 'engine_a/confirmations',
passwords: 'engine_a/passwords'
}, path_names: { registration: 'me' }
authenticated :customer_user do
# your user routes
end
end @Bharat311 please correct if wrong |
We have a similar problem :/ Didn't realize this was unsupported b/c it was not written on the wiki. So I added an entry here: |
We have exactly the same requirement as @Bharat311! There are regular and admin users for the website and we have a separate engine for admins mounted in the main app. The mixin approach will work well for us. |
I also find myself wishing this were possible. It has been very useful/promising to use Engines to organize mostly separate apps that all reference the same data, but I'm unable to do so, because Devise breaks in this use case. I could look into alternate authentication for the Engines, but it sure would be much nicer to just use Devise for everything. |
You can look at how Spree Commerce does it. It uses multiple engines, including one for devise auth. It is easy to extend using more engines as extensions. |
I am running into the same issue of having one modular Rails monolith with multiple engines - and multiple models like Admin and User that have different requirements for the authentication setup. Is there by now a solution by Devise - which would allow to use the gem and not a fork or monkey-patches? Or is this still unsolved and needs to be worked around? |
Could this be addressed by allowing the devise_for :regular_user, parent_controller: "RegularUser::ApplicationController"
devise_for :admin_user, parent_controller: "AdminUser::ApplicationController" Posting this because I ran into a similar issue when trying to set up two categories of users, albeit on the same monolith. The use case is a multi-sided marketplace where each category of user has a similar set of paths (i.e. "/", "/profile", "/foobar", etc) that each route to different controllers based on the type of user. This is important as an additional layer of security to prevent producers from seeing consumer's data and vice-versa. I'm using authenticate :consumer_user do
root "consumer_user/home#index"
end
authenticate :producer_user do
root "producer_user/home#index"
end This works great, except that devise wants to inherit from its config's
As a result, all devise controller inherit from class ApplicationController < ActionController::Base
def analytics(authenticatable: current_consumer_user || current_producer_user)
@analytics ||=
case authenticatable
when ->(a) { a.is_a?(ConsumerUser) }
ConsumerUser::Analytics.new(
consumer_user: authenticatable,
browser: browser.name
)
when ->(a) { a.is_a?(ProducerUser) }
ProducerUser::Analytics.new(
producer_user: authenticatable,
browser: browser.name
)
end
end
end It's not the end of the world, but it does require these polymorphic disambiguators that can make things a bit hard to reason about. If there's interest from the team I can work to put together a PR to allow for a dynamic |
Hi, I have an application which has multiple rails engines mounted on it. Some of these engines use devise for authentication.
Now, it is easy to setup devise to work with one engine - by setting the 'router_name' and 'parent_controller' in config/initializer/devise.rb file. But since, in this case, each DeviseController must inherit from each engine's own Application Controller and defining multiple devise initializer files simply overrides the values.
I think an optimum solution for this is to allow devise the ability to be able to mount to multiple engines. One way i propose for this is to have the devise provides Mixins for each controller and we should have a generator class that generates the controllers classes using those mixins and for each engine, which can be easily configured in the initializer file.
Another solution for this problem is to separate out all authentication to a single engine (and it can be configured to use devise easily), but that would require moving/copying of the layouts and js, css, etc. assets as well to that authentication engine, thus violating the DRY principle.
The text was updated successfully, but these errors were encountered: