Feature request: Allow Table policies to use BeforePolicyInterface

[jamisonbryant]

Hello, I noticed while implementing a Table policy recently (specifically a Policy Scope) I can't attach the BeforePolicyInterface and expect the before() method to get called prior to the execution of the scopeFoo() method.

It appears as if this is because the AuthorizationService::applyScope() method does not check if the policy is an instance of BeforePolicyInterface like performCheck() does. But other than that, the methods look pretty darn similar.

Was this decision to not check for the Interface by design? If so, what was the motivation? And if not, would this change be beneficial? I could certainly use it to allow my superadmins to skip scope limitations on Queries that I apply to normal users.

Also happy to take a first stab at a PR, if this change is OK with the maintainers.

[markstory]

Was this decision to not check for the Interface by design? If so, what was the motivation? And if not, would this change be beneficial?

No it wasn't intentional, using the same interface before applyScope could be ok. If a policy implements both record policy methods and applyScope though the before method would become messy. Should we have a new method for beforeScope?

Adding methods to existing interfaces isn't possible because of backwards compatibility, but we could add a new interface for BeforeScopePolicy and merge the two interfaces in the future :shrug:

[jamisonbryant]

we could add a new interface for BeforeScopePolicy

This I like. I'm happy to tackle this, if desired. I don't think it would be super difficult.

and merge the two interfaces in the future 🤷

Would this even be strictly necessary? I would think it would be OK to acknowledge that although a scope is a policy, it's a different kind of policy and so a different interface is necessary.

[jamisonbryant]

Description for the beforeScope method (first attempt):

If boolean value is returned, the scope application will be skipped and the unmodified resource will be returned. In case of null, the scope will be applied.

I have two questions:

1. I'm getting hung up on "boolean value" because technically a ResultInterface is not a boolean. Unless it is understood that a Result with status = true is the same as boolean true, and same for status = false?
2. Assuming the answer to the previous question is 'yes', what should be done when true or false are returned? It makes sense that returning null means the scope application will still be run. But in the context of a scope, I'm not sure what it should do when a boolean is returned.

[markstory]

Would this even be strictly necessary? I would think it would be OK to acknowledge that although a scope is a policy, it's a different kind of policy and so a different interface is necessary.

Good point, we don't have to merge the interfaces, as you wouldn't want to implement beforeScope on an entity policy.

I'm getting hung up on "boolean value" because technically a ResultInterface is not a boolean. Unless it is understood that a Result with status = true is the same as boolean true, and same for status = false?

What if beforeScope return the modified scope resource or null if the before hook didn't apply? That would allow before hooks to augment or replace the scoped resource if that is required. This also emulates before where it can replace the result by returning a bool.

[jamisonbryant]

PR is up, I made sure to run all the checks locally so it should be clean and ready for review.

[github-actions[bot]]

This issue is stale because it has been open for 120 days with no activity. Remove the stale label or comment or this will be closed in 15 days

[jamisonbryant]

PR was closed because the port to 3.x was handled elsewhere. Confirmed the BeforeScopeInterface is present in 3.x branches. Closing this issue as resolved :)