Skip to content
This repository has been archived by the owner on Jul 3, 2020. It is now read-only.

[WIP] Update factory tests #312

Merged
merged 10 commits into from
Nov 10, 2015
Merged

Conversation

bakura10
Copy link
Member

@bakura10 bakura10 commented Nov 8, 2015

Some factories proved to be not useful anymore, as we likely remove the dependency to Zend\Authentication and will instead pass the identity as part of the AuthorizationService is granted.

@bakura10
Copy link
Member Author

bakura10 commented Nov 8, 2015

@danizord , while trying to re-understand the code, I can see that the "matchIdentityRoles" is no longer used now. It used to be needed because of ZendDeveloperTools, but now that it has gone, it does not seem to be needed. Any idea of why it could be useful now?

@bakura10
Copy link
Member Author

bakura10 commented Nov 8, 2015

I'm not sure to understand how the flow would work actually. The current identity is provided by the "RoleService", which itself has a dependency to an IdentityProviderInterface (which used to use Zend\Authentication).

Because we are using a middleware approach, I'd think that the good approach would be to have a middleware that would do the authentication logic and store the logged user (if any) as part of the request attributes. However how the RoleService would retrieve the request to get the identity?

The workflow is a bit confusing for me :(.

@bakura10 bakura10 mentioned this pull request Nov 9, 2015
@bakura10
Copy link
Member Author

I'm going to merge that (not passing yet, but I don't want too big PR).

bakura10 added a commit that referenced this pull request Nov 10, 2015
@bakura10 bakura10 merged commit f4cac29 into ZF-Commons:develop Nov 10, 2015
@bakura10 bakura10 deleted the update-factory-tests branch November 10, 2015 23:13
@danizord danizord added this to the 3.0.0 milestone Mar 30, 2016
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 this pull request may close these issues.

3 participants