Add view helper and controller plugin to pull the current identity from ... #2650

Closed
wants to merge 2 commits into
from

Projects

None yet

5 participants

@superdweebie

...an AuthenticationService

@superdweebie superdweebie referenced this pull request in doctrine/DoctrineModule Oct 3, 2012
Merged

Add authentication storage factory #95

@Ocramius
Member
Ocramius commented Oct 3, 2012

Nice to see that nothing got wasted :)
Anyway, I think you can just use mock objects instead of adding assets. Either way is fine by me anyway...

@superdweebie

Yeah, I'm still getting the hang of mock objects.

@bakura10 bakura10 commented on the diff Oct 3, 2012
library/Zend/Mvc/Controller/Plugin/Identity.php
+
+namespace Zend\Mvc\Controller\Plugin;
+
+use Zend\Authentication\AuthenticationService;
+use Zend\Mvc\Exception;
+
+/**
+ * Controller plugin to fetch the authenticated identity.
+ *
+ * @category Zend
+ * @package Zend_Mvc
+ * @subpackage Controller
+ */
+class Identity extends AbstractPlugin
+{
+
@bakura10
bakura10 Oct 3, 2012

Remove this empty line.

@bakura10 bakura10 commented on the diff Oct 3, 2012
library/Zend/Mvc/Controller/Plugin/Identity.php
+
+use Zend\Authentication\AuthenticationService;
+use Zend\Mvc\Exception;
+
+/**
+ * Controller plugin to fetch the authenticated identity.
+ *
+ * @category Zend
+ * @package Zend_Mvc
+ * @subpackage Controller
+ */
+class Identity extends AbstractPlugin
+{
+
+ /**
+ *
@bakura10
bakura10 Oct 3, 2012

Remove this empty line (and all the others for the properties and functions, we don't have an initial empty lines in CS.

@bakura10 bakura10 commented on the diff Oct 3, 2012
library/Zend/Mvc/Controller/Plugin/Identity.php
+ * @var \Zend\Authentication\AuthenticationService
+ */
+ protected $authenticationService;
+
+ /**
+ *
+ * @return \Zend\Authentication\AuthenticationService
+ */
+ public function getAuthenticationService()
+ {
+ return $this->authenticationService;
+ }
+
+ /**
+ *
+ * @param \Zend\Authentication\AuthenticationService $authetnicationService
@bakura10
bakura10 Oct 3, 2012

Typo in the name (you wrote autheTNication). Moreover, as you added the use statement, you can simply use "AuthenticationService" as well in each docblock.

@weierophinney
Member

This should also include a factor in the HelperPluginManager for injecting the AuthenticationService instance. (I'll likely add this on merge). Otherwise, looks very good!

@bakura10
bakura10 commented Oct 3, 2012

I thought about that Matthew (injecting the AuthenticationService in the constructor as it is a hard depencency). However how can you fetch the AuthenticationService in the ServiceManager ? Is there a standard key for it ?

@weierophinney weierophinney added a commit that referenced this pull request Oct 3, 2012
@weierophinney weierophinney [#2650] CS fixes
- Removed unnecessary empty lines and empty docblock lines
- Removed unnecessary imports (and resolved class names relative to existing
  imports or current namespace)
- Ensured all docblocks were present
- Minor flow fixes
7f86eac
@weierophinney weierophinney added a commit that referenced this pull request Oct 3, 2012
@weierophinney weierophinney [#2650] Added factories for identity plugins
- In View HelperPluginManager and Mvc Controller\PluginManager
- Includes tests for expected behavior
9150bcc
@weierophinney
Member

@bakura10 Typically, if the service is not a core MVC service, we define it using the FQCN (we do this with translator, db adapter, and navigation already). I did the same here -- I have it check for Zend\Authentication\AuthenticationService, and, if present, inject it.

@bakura10
bakura10 commented Oct 3, 2012

Right. So lets add it in the controller as a hard dependency, so that we can remove the check in invoke !

Envoyé de mon iPhone

Le 3 oct. 2012 à 21:12, Matthew Weier O'Phinney notifications@github.com a écrit :

@bakura10 Typically, if the service is not a core MVC service, we define it using the FQCN (we do this with translator, db adapter, and navigation already). I did the same here -- I have it check for Zend\Authentication\AuthenticationService, and, if present, inject it.


Reply to this email directly or view it on GitHub.

@internalsystemerror

Can I request an amendment to allow controllers to easily call clearIdentity() by using the controller plugin?. What I'm thinking, in the controller plugin class, is:

public function __invoke($clear = false)
{
    /** check for authenticationService ommited **/
    if (!$this->authenticationService->hasIdentity()) {
        return null; // Should this return false if $this->identity() will be used to check for valid id?
    }
    if (true === clear) {
        return $this->authenticationService()->clearIdentity();
    }
    return $this->authenticationService()->getIdentity();
}

So I've included an optional boolean to check whether the id should be cleared or returned. Thoughts?

ise

@superdweebie

It's not a bad idea, but it think it might be better to use another method, rather than __invoke, for clarity. @internalsystemerror if we use your suggestion, then this would clear the identity:

$this->identity(true)

Which I think it confusing. I would prefer:

$this->identity()->clear()

Also, if this change is made, the controller plugin should also be updated.

@internalsystemerror

@superdweebie I agree. But with using __invoke() and returning an entity from getIdentity(), this will require another layer, hence why i thought to avoid that? I can fork develop and make that change if required?

@weierophinney weierophinney added a commit to zendframework/zend-view that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#2650] CS fixes
- Removed unnecessary empty lines and empty docblock lines
- Removed unnecessary imports (and resolved class names relative to existing
  imports or current namespace)
- Ensured all docblocks were present
- Minor flow fixes
db5d423
@weierophinney weierophinney added a commit to zendframework/zend-view that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#2650] Added factories for identity plugins
- In View HelperPluginManager and Mvc Controller\PluginManager
- Includes tests for expected behavior
a7503a6
@weierophinney weierophinney added a commit to zendframework/zend-view that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'feature/identity-plugins' into develop 6564ce7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment