Make solidus_auth_devise for backend work without solidus_frontend

[Kingdutch]

I see a few commits that address frontend specific things being loaded but as far as I can tell solidus_auth_devise is still broken when only solidus_core and solidus_backend are installed.

For the inital issue report please see: solidusio/solidus#1286

What I have found so far:
Because the :spree_user scope for backend is created in the :admin namespace devise will create it and its helpers as admin_spree_user.

In lib/spree/authentication_helpers.rb on master there are conditionals for the paths but spree_current_user is defined to use current_spree_user (the default Devise helper) which does not exist in the described setup.

Changing current_spree_user to current_admin_spree_user fixes this part of the issue so there should probably be another conditional included here.

This makes the authentication function able to find the current spree user. However, this confuses Devise because in config/routes.rb the paths for the admin scope are defined on the spree_user scope instead of the admin_spree_user scope and thus they can not be found. (devise_for does take any namespaces into account when determining what to pass to devise_scope but this is not done in the manual invocation of devise_scope in the routes file).

By changing devise_scope on line 50 to :admin_spree_user this error is fixed.

We are now at a point where the initial navigation to /admin will redirect to /admin/login.

A new error occurs here: on line 28 of lib/views/backend/spree/admin/user_sessions/new.html.erb the helper function recover_password_path is called. However the recover_password path is never defined for the admin user scope. It is however defined when the frontend is available.

There are two possible fixes:
Either remove the link from the admin login page which is easiest or copy the routes from the frontend to also be used on the backend scope.

Choosing for now the simplest option and removing the links I continue my quest. The login screen is correctly presented however it won't properly submit because of a call to authenticate_spree_user! in Spree::Admin::UserSessionsController. Changing the scope here and on the if statement that checks if authentication was succesful (line 15) will fix the errors upon login form submission.

I would be glad to provide a pull request with all the changes. My main problem is that I'm not sure about the intentions in the initial implementation:

• Should there be two different scopes so admins and base users are properly separated?
• Should there be just one scope but have two inadvertently been created.

This is the point at which I'm stuck. My feeling says that the second option is intended. My fixes however are intended towards the first option. As an alternative, setting the admin namespace as: nil will prevent Devise from prefixing the scope with admin. This however breaks other places that do use admin_login_path.

What is the intended way forward?
Apologies for the wall of text, I've tried to document my research best as possible.

[bbuchalter]

Apologies for the wall of text, I've tried to document my research best as possible.

@Kingdutch thanks for this thoughtful and detailed issue write up. This gem has been struggling to find it's place in core/frontend/backend for sometime and the more folks who exercise it the better.

Should there be two different scopes so admins and base users are properly separated?

My understanding is that there are intentionally two different scopes, but we'll need to hear about this from the maintainer @stewart for final word. However it plays out, we should update the README to speak to this issue.

[stewart]

Hey, @Kingdutch - as Brian said, thanks for the write-up. It's greatly appreciated.

solidus_auth_devise is in a very tricky spot as an extension, since it needs to coexist with both the backend + frontend, while not being part of core. As you've seen, there's a lot of conditional code to accomodate this.

I've been thinking about this a bit more over the past few weeks, and I think I've found the most useful solution going forward for the project.

There's a few pending fixups that need a release. If you've got patches for the issues you described above I'll gladly accept them, if not I'll write some myself to attempt to cover these issues. These fixes will be baked into a v1.5.0 release.

v1.6.0 will consist of any further fixes, as well as deprecation warnings for conditional or strangely named methods (spree_current_user renamed to current_user, etc.) that will be removed or replaced in a future release. These include the front-end-conditional helper methods.

From there, version 2.0.0 of solidus_auth_devise will seek to remove as much of this conditional code where possible (in particular for routes and helpers), and stick to defining a single spree_user Devise scope. All of the methods marked as deprecated in v1.6.0 would be removed here as well. This should dramatically simplify the extension, and alleviate most of the pain points users have been encountering.

Additionally, explicit (opt-out) configuration options would also provide a better solution for including decorators and the like. Better documentation of these would make adding solidus_auth_devise to an existing app, or customizing it, less confusing to new users.

Does this sound agreeable to you?

[Kingdutch]

If I understand correctly this would mean that we would move from two scopes to a single scope that would be used for both 'normal' users (customers) as well as admin users. Which would then be separated by the role system that is in core if I understand correctly.

I'm still not really sure then why solidus_auth_devise currently has two scopes (although that's what I'd probably do if I had to write this myself).

The advantage is that with one scope you could probably get rid of a lot of conditional code because the authentication point would always be the same whether for normal login or for admin usage.

I must admit, my knowledge of solidus_auth_devise at this point is a bit foggy.

The step in v1.6.0 to rename spree_current_user to current_user isn't really needed for me. Also there is no spree_current_user method in solidus_auth_devise. That's the method that solidus_core uses. Devise defines a current_spree_user method though. However as the scope that is used by solidus_auth_devise is spree that seems like the proper function name. It would allow people using solidus next to other parts that also use devise to keep those scopes separate should they so desire.

So your plan sounds good, I'm just not entirely clear on where we'll end up.

[stewart]

solidus_auth_devise provides the spree_current_user method.

[gbrlcustodio]

Is this problem solved in the latest version?

[aaronlifton]

Still not solved - we ended up forking this gem - https://github.com/dstld/solidus_auth_devise

[mathportillo]

this would be really useful, can't aaron solution be merged back?