Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: BeforeScopeInterface (3.next) #260

Conversation

jamisonbryant
Copy link
Contributor

@jamisonbryant jamisonbryant commented Nov 8, 2023

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

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

@ADmad ADmad changed the base branch from 3.x to 3.next November 8, 2023 15:02
@jamisonbryant
Copy link
Contributor Author

jamisonbryant commented Nov 9, 2023

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
Copy link
Contributor

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.

@jamisonbryant jamisonbryant changed the title Feature: BeforeScopeInterface (3.x) Feature: BeforeScopeInterface (3.next) Nov 10, 2023
@markstory markstory added this to the 3.x milestone Nov 10, 2023
@markstory
Copy link
Member

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

@markstory markstory closed this Nov 10, 2023
@jamisonbryant jamisonbryant deleted the feature/before-scope-interface branch January 22, 2024 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants