Skip to content

Allow setting a different user class for backend users - #6329

Draft
elia wants to merge 4 commits into
mainfrom
elia/admin-user
Draft

Allow setting a different user class for backend users#6329
elia wants to merge 4 commits into
mainfrom
elia/admin-user

Conversation

@elia

@elia elia commented Oct 5, 2025

Copy link
Copy Markdown
Member

Summary

Solidus has always used a single user model for both storefront customers and
backoffice operators, with access decided purely by roles. One table, one set of
validations, one authentication configuration — so any weakness in customer
signup is a weakness in admin access.

This PR adds Spree.admin_user_class and routes every admin/backoffice lookup
through it, so an application can separate the two. It defaults to
Spree.user_class, so nothing changes until you opt in.

Separating the models buys three things:

  • Security — privilege escalation across the boundary stops being possible
    when the two sets of records are siloed.
  • Authentication — admins and customers usually want different rules
    (domain-restricted Google signup for admins, magic links for customers,
    stricter password policies). Separate models mean separate Devise scopes.
  • Impersonation — impersonating a customer gets much simpler when the two
    authentication systems don't overlap.

What's in here

Spree.admin_user_class is a mattr_accessor alongside Spree.user_class,
with the same String/Symbol-only contract and a admin_user_class_name
counterpart for lazy resolution. It returns Spree.user_class when unset.

Call sites updated to use it: the classic backend (users, orders, search, store
credits, API key controllers, the matching views and the store-credit-events
helper), the users menu item condition, and Solidus Admin (users, orders and
store-credits controllers plus the users/users-and-roles components). The
install generator's initializer template now writes both settings.

Spree::UserClassHandle — the lazy proxy that defers resolving a class name
until runtime, because user models often load before the initializer runs — is
generalized into Spree::ClassProxy, with UserClassHandle and a new
AdminUserClassHandle as thin subclasses. UserClassHandle behaviour is
unchanged.

The :admin_user factory moves out of user_factory.rb, where it was nested
inside :user and therefore always built a Spree.user_class, into its own
file built on AdminUserClassHandle.

New doc at core/docs/admin_user_class.md.

What this deliberately does not do

Setting Spree.admin_user_class changes which model the admin interfaces read
and write. It does not give that model authentication. There is no generator
for a separate admin Devise scope yet, so an application pointing this at a
dedicated model has to wire up admin login itself.

That's the intended next step — a generator rather than a new extension — but
it's out of scope here to keep this reviewable.

Three unrelated test-environment fixes that used to live on this branch have been
extracted to #6532. This PR is now only the feature.

Behaviour changes worth flagging

  • create(:admin_user) now builds Spree.admin_user_class. A no-op while
    that falls back to Spree.user_class, but applications configuring a distinct
    admin model will get that model in specs.
  • The install generator writes Spree.admin_user_class explicitly, pointing
    it at the same class as Spree.user_class. Generated apps therefore never
    exercise the fallback.
  • The users menu item condition now tests Spree.admin_user_class, so the
    Users menu is gated on admin access to the admin user model.
  • Admin i18n now resolves against the admin user model. The users index
    breadcrumb, the Users menu label and the email / roles / lifetime-value column
    headers all call model_name.human or human_attribute_name on
    Spree.admin_user_class. Configure a separate AdminUser and those strings
    come from activerecord.models.admin_user and
    activerecord.attributes.admin_user.* instead of the spree/user keys — so a
    dedicated admin model needs its own translations to avoid humanized fallbacks.
    Worth a reviewer's opinion on whether these labels should keep pointing at
    Spree.user_class.
  • The users index breadcrumb previously hardcoded Spree::LegacyUser; it now
    follows the configured class, which is a fix in its own right for apps with a
    custom user model.

How to test

Default behaviour is unchanged — the existing suite covers that, since
Spree.admin_user_class resolves to Spree.user_class everywhere.

To exercise the split, point it at a separate model in
config/initializers/spree.rb:

Spree.user_class = 'YourApp::User'
Spree.admin_user_class = 'YourApp::AdminUser'

The admin user index, order customer search, store credits and the Users menu
should all operate on AdminUser, while storefront and checkout continue on
User. Note you'll need to provide admin authentication yourself — see above.

Factory specs:

cd core && bundle exec rspec spec/lib/spree/core/testing_support/

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📑 I have documented new code with YARD.
  • ✅ I have added automated tests to cover my changes.
  • 🛣️ Guides will need updating once the authentication generator lands.

@elia elia self-assigned this Oct 5, 2025
@github-actions github-actions Bot added changelog:solidus_backend Changes to the solidus_backend gem changelog:solidus_core Changes to the solidus_core gem changelog:solidus_admin labels Oct 5, 2025
@codecov

codecov Bot commented Oct 5, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.58333% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.13%. Comparing base (ccc5ad8) to head (cae97a3).

Files with missing lines Patch % Lines
core/lib/spree/core.rb 55.55% 4 Missing ⚠️
...app/controllers/solidus_admin/orders_controller.rb 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6329      +/-   ##
==========================================
+ Coverage   91.31%   92.13%   +0.82%     
==========================================
  Files         813     1036     +223     
  Lines       17088    21172    +4084     
==========================================
+ Hits        15604    19507    +3903     
- Misses       1484     1665     +181     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jarednorman

Copy link
Copy Markdown
Member

This seems like a good idea to me. This separation could be useful for some marketplace stores we work on as well.

@elia

elia commented Oct 8, 2025

Copy link
Copy Markdown
Member Author

@jarednorman wanted to do this for a long time, and never got the time until I was able to delegate 😅
If there's buy in I'll give it some more (compute) time and finish this first iteration, later I think we can introduce to a better default for new apps.

cc @rainerdema @spaghetticode

@elia
elia force-pushed the elia/admin-user branch from da9f17b to 6d2dfbe Compare October 9, 2025 15:35
@elia
elia force-pushed the elia/admin-user branch 2 times, most recently from 3d18008 to 3482d51 Compare August 2, 2026 14:36
@elia elia mentioned this pull request Aug 2, 2026
6 tasks
elia and others added 4 commits August 3, 2026 19:02
…o user_class. Update initializer template, menu conditions, controllers, views, and helper.
`Spree::UserClassHandle` existed to defer resolving `Spree.user_class`
until runtime, since classes can be loaded before the initializer sets
it. `Spree.admin_user_class` needs exactly the same treatment, so pull
the deferral into a reusable `Spree::ClassProxy`.

The proxy takes the setting's name plus a block that reads the current
value, and both handles become three-line subclasses. Naming the setting
means the "has not been set yet" error points at whichever one is
missing rather than always blaming `Spree.user_class`.

`UserClassHandle#to_s` is unchanged in behaviour: it still reads
`Spree.user_class_name` and still raises the same message. Only
`AdminUserClassHandle` is new.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `:admin_user` factory was nested inside `:user`, so it inherited
`Spree::UserClassHandle` and always built a `Spree.user_class`. Now that
admin users can be a separate model, it should follow
`Spree.admin_user_class` instead.

Move it to its own file, built on `Spree::AdminUserClassHandle`, and give
it its own spec. It no longer inherits from `:user`, so the `email`,
`password` and `password_confirmation` attributes are repeated here; the
`:with_api_key` and `:with_orders` traits are not, since an admin user
model is not assumed to have an API key or orders.

This changes what the existing callers of `create(:admin_user)` build.
It is a no-op today because `Spree.admin_user_class` falls back to
`Spree.user_class`, but applications configuring a dedicated admin user
model will get that model in specs from now on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Explain why the setting exists, not only what it does: sharing one model
between customers and administrators means customer signup and admin
access share validations and authentication, which is the problem this
separation solves.

Placed under core/docs/ following the per-gem convention of admin/docs/
and storefront/docs/, since the setting is defined in core.

Records two constraints that are easy to trip over: both settings must be
given a String or Symbol rather than a Class, because they are resolved
lazily; and the install generator writes both explicitly, pointing them
at the same class, so the fallback is not what a generated application
relies on.

Also states what is still missing, so nobody reads the setting as more
than it is: it redirects which model the admin interfaces use, but
provides no authentication for that model and no generator for a separate
admin Devise scope.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@elia
elia force-pushed the elia/admin-user branch from 3482d51 to cae97a3 Compare August 3, 2026 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:solidus_admin changelog:solidus_backend Changes to the solidus_backend gem changelog:solidus_core Changes to the solidus_core gem

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants