Replacement options for dispatcher->until()

[askvortsov1]

Functionality Discussion

Is your feature request related to a problem? Please describe.
Certain extendable features currently use ->until() with. While this has some benefits, there are also several significant issues, namely:

1. We are looking to transition away from non-domain events in favor of our more robust extender API
2. Only the first extension to return a non-null value is considered. This has security implications, as in Applying the "reply to discussions" permission to a tag may break the "lock" mode #1832. In my opinion, permissions should NOT depend on extension boot order (which is a separate issue).
3. There is no way for an extension to override a value returned by another extension.

I've compiled a list of all instances where ->until() is used here: #2015 (comment)

Describe the solution you'd like
I have come up with 2 possible ideas this far. In my opinion, at this stage, the actual implementation is less important than deciding how to handle this situation.

1. Consider ALL responses returned by extensions. This is inspired by Symfony's voter approach. When requesting this replacment to a ->until() check, the code would indicate an 'evaluation mode'. Depending on the mode, return:
   a. true if any true values are present
   b. false if any false values are present
   c. whichever of true or false has more returned, with true returned if there is a tie
   d. whichever of true or false has more returned, with false returned if there is a tie

2. Extensions can return one of 4 options:
   a. $this->allow()
   b. $this->allow()
   a. $this->forceAllowallow()
   a. $this->forceDeny()

Any $this->forceDeny()s returned would override, if not present, any $this->forceAllow()s would override, if not present any $this->allow()s would override, and so on. This is my preferred approach as it's intuitive and flexible. One potential implementation is included in #2056

EDIT:

Some time after writing this, I realized that this would not necessarily work for ->until() methods that don't return a boolean (such as GetDisplayName). For those cases, I think that the extender should be refactored to use a driver, and a driver should be selectable in the admin configuration, like we do with mail drivers.

[askvortsov1]

Franz and I had a very productive discussion today, here are my takeaways for how to procees:

Permissions and Policies.

The policies system will face a broaded refactor, discusssed here: #2092. However, so far the 4 tiered decision system (allow, deny, forceallow, forceDeny) seems like a good approach. Which values we actually pick to represent this might be controversial, but should not affect forward compatibility. At this point, I'm thinking of true for ->allow(), false for ->deny() (which should help with back compatibility), and the strings FORCE_ALLOW and FORCE_DENY for ->forceAllow() and ->forceDeny(), respectively.

Model relationships (planned in Models Extender) (Also, why do we limit it to only 1 custom relationship right now lol?)

https://github.com/flarum/core/blob/0d208dc4437c0cc551c556d2c2cc7c12bda369ae/src/Database/AbstractModel.php#L140

A replacement is proposed here: #2100. until is unnecessary.

Serialization Relationships (planned in API Serializer Extender)

https://github.com/flarum/core/blob/0d208dc4437c0cc551c556d2c2cc7c12bda369ae/src/Api/Serializer/AbstractSerializer.php#L131

Will be implemented the same way as model relationships.

Whether Discussions and Posts are private

https://github.com/flarum/core/blob/d492579638fb52dafbfe65f1f36a95eb6047f7f3/src/Post/Post.php#L102
https://github.com/flarum/core/blob/8877bf97c4b42d8093146039d652262f21c5717d/src/Discussion/Discussion.php#L116

In this case, marking something as private should NOT be overriden. In other words, if ANY extension returns true, discussion or post will be marked as private. Overriding/working around this behavior would be more suitable for policies controlling access to private discussions/posts.

Checking for flooding

https://github.com/flarum/core/blob/b91e9032849446d86f6bec0dc1fa6f6cf0146c8a/src/Post/Floodgate.php#L51

It doesn't really make sense to return false here, that would just essentially disable the flood checker, which is better done through other methods. Similarly to the proposal for private models, if any extender returns true, flooding will be reported.

However, there should be a method for extensions to disable flood checkers introduced by other extensions. For that reason, each flood checker will be assigned a string id as a second parameter in the extender. There will be a method on the extender called disableFloodingChecker, which will take such a string id as a parameter. During runtime, any extending flood checkers that have been disabled will not be called.

Getting users' display names

https://github.com/flarum/core/blob/d492579638fb52dafbfe65f1f36a95eb6047f7f3/src/User/User.php#L312

This doesn't make sense as until to begin with, and will be replaced with a driver-based system. For now, the first registered driver will be used, but later versions of Flarum (or extensions) will incude a UI for admins to select which display name driver to use.

Checking passwords

https://github.com/flarum/core/blob/d492579638fb52dafbfe65f1f36a95eb6047f7f3/src/User/User.php#L323

Here, true, false, and null are all valid responses. This system faces the same concerns as Policies. However, because this is used much less, we can start with just allow() and deny(), and add other options (like the force stuff) as it becomes relevant.

[luceos]

Can't we move the Floodgate throttling to a middleware? So that extensions can pluck that from the middleware stack and others could inject their own? Sounds like a lot of bloat to add unique id's to floodgates..

[askvortsov1]

Perhaps, but that would require a rewrite of how floodgate works (I believe). I would be open to that though.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum.
In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

[askvortsov1]

This can be further discussed in the relevant issues/PRs for particular extenders.