RegistrationAccessControlHandler checks 'administer own $type registration' incorrectly

So this is a simple fix, if one is willing to accept the change of behavior.

I think the fix I propose here is sound but wrong.

This permission is intrinsically confusing because of its name. Rather than fixing it to make it adhere to what we said it has been doing, but which it hasn't, lets take advantage of the fact that no one is relying on for its intended purpose, to simply kill it. Let's remove the permission, and replace it with an 'administer own host registration' permission.

Part of the problem is that we also have 'manage own $type_id registration' which is documented as having the same semantic as 'administer own $type_id registration'.

We could change the documentation for 'administer own $type_id registration' to match how it is actually used (i.e. own means the registrant not the host editor) as it does for view, update and delete. But then we'd still be left with 'manage own $type_id registration' dangling with its own odd semantic.

Perhaps that could be deprecated in favor of a new 'manage host registration'.

For now, changing the documentation for 'administer own $type_id registration' to match its actual behavior seems to be the simplest solution. From a security point of view this is relatively benign as the user is getting a more restricted access than the documentation used to imply they would get.

N.B. the docs 'for administer own $type_id registration' say 'Manage registrations and registration settings of this type for host entities to which a user has edit access.' Currently the code allows the user to administer the settings but not the registrations.

One problem with removing 'administer own $type_id registration' altogether is that we still have 'manage own $type_id registration' so the odd semantic lingers on.

I'm finally understanding why this is so confusing. For 'administer own $type_id registration' the description is:
"View, edit and delete own registrations of this type. Manage registrations and registration settings of this type for host entities to which a user has edit access."

And this is in fact how it works.

What's confusing is that the semantic of own is effectively different in the 2 sentences of the description: in the first sentence "View, edit and delete own registrations" it means registrations that one is the registrant for; in the second sentence it means the same as it does for "manage own $type_id registrations", registrations for host entities you can edit.

Additionally confusing is that I would naively have understand 'Manage registrations' to mean being able to do things like view, update and edit registrations. But actually all it really means "is able to view the manage registrations route'.

So the code does match the docs, but what these permissions are doing is so weird that I've found it very hard to understand or believe.

Updated the IS with new proposal now I understand this is not a bug per se.

@jonathanshaw thanks for digging into this and sorry that it is confusing. What happened is I tried in my original port from D7 to keep the original permission names and meanings to the degree possible in a D9 context, to preserve the ability to easily migrate. Thus some confusion was created from the beginning, and then as requests to make things more granular came in, it snowballed. I would love to coordinate on a new structure for a future 3.5 or 4.x version of the module. Perhaps we can deal with the existing structure by creating a documentation page on d.org (I have an issue for this in the issue queue) to explain the nuances.

I would love to coordinate on a new structure for a future 3.5 or 4.x version of the module.

Great, let's do it. I suggest we discuss on a messy way in this issue, and once we understand what's needed better, we can maybe make a new roadmap or subissues.

My current thinking is that we have 3 problematic permisisons:
"administer own $type_id registration"
"administer own $type_id registration settings"
"manage own $type_id registration"

We should create 2 new permissions to replace these:
"manage host registrations"
"administer host registrations"

We should change the functioning of "administer own $type_id registration" so that it only addresses administering registrations the user is the registrant of; this can be done in a fully BC way by giving anyone who has it the "manage own $type_id registration" permission as well.

We then need to get rid of "administer own $type_id registration settings" and "manage own $type_id registration". This is impossible to do in a fully BC way. Perhaps the best solution is to use hook_requirements to block upgrading the module if those permissions are still in use, forcing sitebuilders to take action.

Perhaps we can deal with the existing structure by creating a documentation page on d.org (I have an issue for this in the issue queue)

I will have a look at it.

I wonder if we can close this and combine it into https://www.drupal.org/project/registration/issues/3479435 ✨ Create additional host permissions Active

I think we should do this in this issue, before or after ✨ Create additional host permissions Active :

We should change the functioning of "administer own $type_id registration" so that it only addresses administering registrations the user is the registrant of; this can be done in a fully BC way by giving anyone who has it the "manage own $type_id registration" permission as well.

And then we should create a new issue to discuss (and maybe implement but only after ✨ Create additional host permissions Active ) this:

We then need to get rid of "administer own $type_id registration settings" and "manage own $type_id registration". This is impossible to do in a fully BC way. Perhaps the best solution is to use hook_requirements to block upgrading the module if those permissions are still in use, forcing sitebuilders to take action.

So there's 3 separable issues here. We could combine, but I tend to assume seperating is better if possible.

Thanks @jonathan - this is basically ready - just a few minor nits in the tests. The above makes it look like a lot of work but it's basically the same issues that repeat and I wanted to be sure I flagged all the occurrences.

Re ManageRegistrationsAccessCheck

"Do we need a couple new examples now - for administer own conference registration settings - with host update TRUE and FALSE - and expected being TRUE and FALSE respectively."

I don't think it's as simple as that. We would need to add a new scenario if I understand the existing data provider logic correctly. And I'm not sure that's worth doing given I've opened 🐛 Remove "administer own $type_id registration settings" & "manage own $type_id registration" Active as a follow up.

This was ready but then I realized that somewhere along the way "manage own $bundle registration' became too powerful and was allowing settings access. As implied in the description for 'manage $bundle registration settings' permission, settings access requires this additional permission when using either 'manage $bundle registration' or 'manage own $bundle registration' permission. I don't want to have yet another update hook in a separate issue, so we'll have to fix that in this issue. I will work on it.

I thought there might be an issue with settings access control but on further review it looks okay. But to be sure I'll add some tests, which are missing, to RegistrationSettingsAccessTest. Currently there are no tests for "manage own". I'll add some to this MR.

All tests are passing. I want to add one more test, for the update hook, proving that a user with 'administer own' lost the ability to do certain things before the update hook runs, and regains them after the update hook runs. Will add that within the next day or two.

Skipped the update hook tests since new "host" replacement permissions are imminent and want this merged before then. Can always add a test later.