Problem/Motivation

It's possible to have access to create or edit a registration, but anything you do on the registration form will lead to a validation error. This is obviously a hostile UX.

This is because RegistrationAccessHandler and RegisterAccessCheck only checks that the host is configured for registration, not enabled for registration; but the validation cares about enabled too.

Steps to reproduce

Disable registration for a host by unchecking the Enable box on the settings edit page for the host.
Masquerade as an ordinary user already registered.
Try to edit your registration.

Proposed resolution

The access and validation need to align more closely.

Remaining tasks

User interface changes

API changes

Data model changes

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

jonathanshaw created an issue. See original summary.

jonathanshaw’s picture

I don't have a specific proposal for this yet, I'm just filing this issue to flag this as a @todo.

I find the intertwined and often not DRY logic for access, HostEntityInterface::isEnabledForRegistration(), HostEntityInterface::isUserRegistered(), and RegistrationConstraintValidator rather scary. I suspect that a fundamental refactor is needed in this area. It's as if this module has done an absolutely fantastic job of growing to handle all kinds of use cases, but the growth has got to the point where it shows that one of the existing pillars needs an overhaul.

I'm actively thinking about this and hope to make a proposal.

jonathanshaw’s picture

jonathanshaw’s picture

Title: Access & validation mismatch editing a registration on a disabled host » Access & validation mismatch creating/editing a registration on a disabled host
Issue summary: View changes

Widened this to include creating registrations too.

jonathanshaw’s picture

If we can do #3464717: HostEntity::isUserRegistered() and isEmailRegistered() don't consider edge cases then my question becomes:
could we make use of these new HostEntity methods in RegisterAccessCheck and RegistrationAccessControlHandler?

One possible concern is that they may be expensive, and so performance when viewing a list of hosts might degrade. I'm tempted to think we should do it anyway, rely on the render caching, and add more specific custom caching if we need to.

john.oltman’s picture

Issue summary: View changes
john.oltman’s picture

Version: 3.1.x-dev » 3.3.x-dev
Assigned: Unassigned » john.oltman
Related issues: +#3464736: Replace HostEntity::isEnabledForRegistration

There are a number of things in play here:

* Refactoring to share more code between access and validation - I like this idea but let's set this aside since https://www.drupal.org/project/registration/issues/3464736 covers it

* Registration is disabled after the form has already been displayed, e.g. capacity is reached while a new registration form is being filled in - yes, bad UX since there will be an error no matter what the user inputs, but generally, you cannot rely on access control to avoid constraint checks for this reason

* Some validation is dependent on submitted values, e.g. in many cases the registrant is unknown until submit - some checks can only occur in validation, for example the "allow multiple registrations for the same user" check

That leaves us with:

* A non-administrator attempts to edit a registration after registration is disabled, and the disabling occurs before the Edit tab is rendered - we can do something about this one; currently only "update" permission is checked in access control - the constraint validator has a special check for this use case. I think the validator is correct in doing this check, but it could be added to access control as well.

john.oltman’s picture

New registrations are checked by RegisterAccessCheck - although it only checks the main status switch in the settings, that should generally be accurate, and the form displays a message instead of the fields when it isn't. So going to leave that use case alone.

john.oltman’s picture

Title: Access & validation mismatch creating/editing a registration on a disabled host » Access control allows non-admins to edit registrations on disabled hosts

  • john.oltman committed 8e8034c1 on 3.3.x
    #3464134: Access control allows non-admins to edit registrations on...

  • john.oltman committed a2303aa7 on 3.1.x
    #3464134: Access control allows non-admins to edit registrations on...
john.oltman’s picture

Assigned: john.oltman » Unassigned
Status: Active » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.