solidusio / solidus_auth_devise

🔑 Devise authentication for your Solidus store.
http://solidus.io
BSD 3-Clause "New" or "Revised" License
52 stars 124 forks source link

RFC: Support Backend-Only Stores #96

Closed stewart closed 6 years ago

stewart commented 7 years ago

This PR introduces some changes to solidus_auth_devise supporting Solidus stores that use solidus_backend, but have custom front-ends (read: do not use solidus_frontend).

The routes for password reset have also been added to the admin scope to allow the full Devise recovery flow to be followed. A corresponding change to the Devise initialiser is required to allow Devise to pick up on the Spree routes. Additionally, the configuration of the admin routes has been updated.

dgra commented 7 years ago

@stewart You may be able to continue to use the authenticate_spree_user, admin_spree_user, etc if you add singular: :spree_user to the devise_for like

      devise_for(:spree_user, {
        class_name: 'Spree::User',
        controllers: {
          sessions: 'spree/admin/user_sessions',
          passwords: 'spree/admin/user_passwords'
        },
        skip: [:unlocks, :omniauth_callbacks, :registrations],
        path_names: { sign_out: 'logout' },
        path_prefix: :user,
        singular: :spree_user # This line was added.
      })

Also, you may not, I didn't get too far into it.

Nice work!

stewart commented 7 years ago

I've updated this per @dgra's suggestion - now uses the singular option to use the same resource name for both frontend and backend.

Additionally, I've removed the auto-generated routes, as we already generate pretty alternatives.

This is the routes diff from an example application, between the last release and this PR:

diff --git a/before b/after
index d49ee08..7842ffa 100644
--- a/before
+++ b/after
@@ -2,18 +2,14 @@ Prefix Verb URI Pattern Controller#Action
  spree      /           Spree::Core::Engine

 Routes for Spree::Core::Engine:
-                               new_admin_spree_user_session GET    /user/spree_user/sign_in(.:format)                                                                 spree/admin/user_sessions#new
-                                   admin_spree_user_session POST   /user/spree_user/sign_in(.:format)                                                                 spree/admin/user_sessions#create
-                           destroy_admin_spree_user_session GET    /user/spree_user/logout(.:format)                                                                  spree/admin/user_sessions#destroy
-                              new_admin_spree_user_password GET    /user/spree_user/password/new(.:format)                                                            spree/admin/user_passwords#new
-                             edit_admin_spree_user_password GET    /user/spree_user/password/edit(.:format)                                                           spree/admin/user_passwords#edit
-                                  admin_spree_user_password PATCH  /user/spree_user/password(.:format)                                                                spree/admin/user_passwords#update
-                                                            PUT    /user/spree_user/password(.:format)                                                                spree/admin/user_passwords#update
-                                                            POST   /user/spree_user/password(.:format)                                                                spree/admin/user_passwords#create
                                          admin_unauthorized GET    /admin/authorization_failure(.:format)                                                             spree/admin/user_sessions#authorization_failure
                                                 admin_login GET    /admin/login(.:format)                                                                             spree/admin/user_sessions#new
                                    admin_create_new_session POST   /admin/login(.:format)                                                                             spree/admin/user_sessions#create
                                                admin_logout GET    /admin/logout(.:format)                                                                            spree/admin/user_sessions#destroy
+                                     admin_recover_password GET    /admin/password/recover(.:format)                                                                  spree/admin/user_passwords#new
+                                       admin_reset_password POST   /admin/password/recover(.:format)                                                                  spree/admin/user_passwords#create
+                                        admin_edit_password GET    /admin/password/change(.:format)                                                                   spree/admin/user_passwords#edit
+                                      admin_update_password PUT    /admin/password/change(.:format)                                                                   spree/admin/user_passwords#update
                                          admin_search_users GET    /admin/search/users(.:format)                                                                      spree/admin/search#users
                                       admin_search_products GET    /admin/search/products(.:format)                                                                   spree/admin/search#products
                                       home_admin_dashboards GET    /admin/dashboards/home(.:format)                                                                   spree/admin/dashboards#home
bbuchalter commented 7 years ago

👍 but I think we could use a rebase of the commits which toggle the name of the routes.

acreilly commented 7 years ago

Has this been approved to merge?

tvdeyen commented 6 years ago

@stewart I'm still a huge fan of this change! Would you mind to rebase and have a look into the failing specs?

stewart commented 6 years ago

Rebased again - there are a couple of outstanding failures when paired with Solidus' master branch (these are present on this repo's master as well), but it passes with all released versions.

If there's anything more that needs to be done to get this PR ready-to-go, please let me know.

tvdeyen commented 6 years ago

@stewart thanks. #113 should fix the current master builds. Would you mind to review, merge if satisfied and rebase your branch after that?

stewart commented 6 years ago

Rebased once more and green across the board. Based on the two 👍s already given, I'm going to merge this.

tvdeyen commented 6 years ago

👍