Feature: BeforeScopeInterface (3.next)

jamisonbryant commented 1 year ago

Description

In our app we would like the ability to run pre-authorization checks on Scopes, something that is not currently possible because the BeforePolicyInterface doesn't work on Scopes. This PR adds the BeforeScopeInterface and updates the AuthorizationService to check for it and call beforeScope() just like it does for BeforePolicyInterface and before().

References #259

Changes Made

Added the BeforeScopeInterface with a single method beforeScope().
Updated the AuthorizationService::applyScope() method to check for BeforeScopeInterface and call beforeScope() prior to passing control to the scope handler.
Updated tests.

Benefits

Adds the ability to run logic before scopes are applied, something that is not currently possible with the plugin.

Checklist

[x] Tests written and passing
[x] Code follows coding standards
[x] Documentation updated if necessary - NOW DONE
[x] Spelling/grammar check done

jamisonbryant commented 1 year ago

Dreadfully sorry for being rude, but is there anything I can do to expedite review & merge of this branch and #261 for this feature? (already rebased it)

My job needs this feature, and I am leaving for paternity leave after Friday. I would love to have this merged in (even to a n.next branch) so they do not have to use my fork for the next couple months :)

LordSimal commented 1 year ago

I would like to leave this up to @ADmad and/or @markstory how they want to handle this, but usually features which were added in previous versions will be merged "upwards" so if we get the 2.next PR in then this 3.next PR is not needed.

So basically if you get your PR fixed up to not have unnencessary commits in them, then we can merge it to 2.next and this feature will be added to the next minor release of this plugin (2.x as well as 3.x)

But in the meantime I would say its totally fine to keep your forked branch in the composer.json so that at least your feature works. Your co-workers will just have to adjust the composer.json after this feature has been released.

markstory commented 1 year ago

With #264 merged, I'm going to close this as the 2->3 merge will take care of getting this to 3.x

cakephp / authorization