Skip to content
This repository has been archived by the owner on May 14, 2018. It is now read-only.

Why are Guards passed securityService instead of serviceLocator? #43

Closed
Chionsas opened this issue Nov 2, 2012 · 2 comments · Fixed by #44
Closed

Why are Guards passed securityService instead of serviceLocator? #43

Chionsas opened this issue Nov 2, 2012 · 2 comments · Fixed by #44

Comments

@Chionsas
Copy link

Chionsas commented Nov 2, 2012

I'm having issue with /Service/Authorize.php:

38: $this->addRoleProvider(new $class($options, $serviceLocator));
44: $this->addResourceProvider(new $class($options, $serviceLocator));
50: $this->addRuleProvider(new $class($options, $serviceLocator));
61: $this->addGuard(new $class($options, $this)); <-- $this here is the BjyAuthorize\Service\Authorize

Is there a specific reason for Guards being passed the Service instead of the Locator?
The two Guards bundled with the module make any use of the Authorize Service passed.
As the Authorize Service doesn't have a getServiceLocator() method and the 'sm' property is set to private, there is no way to access the Service Locator from within a Guard. That creates a problem when one wants to retrieve something (say, a router config) from the Service Locator.

I see two ways of fixing this:

  1. Change addGuard() calls [line 61] to use $serviceLocator instead of $this and change Guard classes as appropriate (simply rename variables). This will not affect bundled code as the bundled Guards do not currently make use of the second argument anyway. It, however, can force custom Guards use $this->serviceLocator->get(''BjyAuthorize\Service\Authorize') instead of $this->securityService if they're currently using it. I favor this approach, as it introduces consistency.
  2. Add getServiceLocator() method to the Service\Authorize class that simply returns $this->sm.

It's a simpel fix. I can attach a patch or make a pull request if you see this issue as valid.

I'm currently passing my custom Guard inside the bjyauthorize config twice - once as a resource provider and once as a rule provider. This gets me the Service Locator as the second argument when the constructor is called :)

@bjyoungblood
Copy link
Owner

Fix #1 is looks like the best approach. I think there was a fix for this in progress, but it never got finished. I'll gladly take a pull request.

@bjyoungblood
Copy link
Owner

Merged #44

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants