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

[Security] AclProvider #100

Merged
merged 30 commits into from Feb 10, 2012
Merged

Conversation

havvg
Copy link
Member

@havvg havvg commented Jan 31, 2012

Add an implementation for ACL of the Security component.

@havvg
Copy link
Member Author

havvg commented Feb 1, 2012

Tests will require this pull request to be merged propelorm/Propel#276

*
* @return AclClass
*/
public static function fromAclObjectIdentity(ObjectIdentityInterface $objectIdentity, PropelPDO $con = null)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static public

@willdurand
Copy link

Awesome work!

@willdurand
Copy link

@havvg Is it good to merge for you?

@havvg
Copy link
Member Author

havvg commented Feb 5, 2012

I would like to add handling of the AclCacheInterface and add an init acl command before merging.

@havvg
Copy link
Member Author

havvg commented Feb 6, 2012

@willdurand

I got a question on the schema part. I added a acl_schema.xml file, which would be generated and inserted into the database, even though the user might not want to use the acl. Any ideas on how to skip that schema file, but provide a way to initiate the database?

I thought of a propel:init-acl command, but I'm not sure on how to accomplish this in a nice way, got no deep insight on this part of Propel right now. Can you hint me to some starting points? It's the missing piece right now before this can be merged.

@willdurand
Copy link

Interesting question, the command init-acl is fine. I'm +1 for that.
You should rename acl_schema.xml to acl.xml, that way the bundle won't try to load it, and the command will know this name to just generate model classes + sql for this schema.

@havvg
Copy link
Member Author

havvg commented Feb 7, 2012

@willdurand If you are good with the last commit (the command), then it's good to merge.

I moved the schema file on directory up, as the target file needs to be named "_schem.xml" and this was the easiest way with least impact.

@willdurand
Copy link

You may need to rebase this PR

The schema is compatible with the distributed schema of the Security component.
* fix ObjectIdentityQuery::findGrandChildren
* change default inherited value to true to comply with database default value
The MutableAclProviderInterface expects to remove all child ACLs upon deletion.

* fix createAcl to check for existing parent
* re-factor Tests\Model\Acl\TestCase to Tests\AclTestCase sharing between Model\Acl and Security\Acl tests
* fix schema setting auditing flags to true by default
* fix MutableAcl inserting field based ACEs
* fix MutableAclProvider removing class based ACEs if last object identity is being removed
* fix EntryQuery error message if non-object has been provided in list of SecurityIdentity
* fix (un)serialize of (Field)Entry
* fix usage of existing database entries
* update provider class to AuditableAclProvider
* change auditing default to be more reasonable, 90% use case: log failures only
* add transformers to Model\Acl\Entry converting from/to Security\Acl\Domain\Entry
* fix MutableAclProvider to use getAcl method instead of creating MutableAcl directly
* re-factor MutableAcl::updateAce and MutableAclProvider::persistAcl to use Entry transformers
* move acl_schema.xml into Resources to avoid being retrieved by default
* re-factor parent tasks methods to use separated methods for re-usage
@havvg
Copy link
Member Author

havvg commented Feb 7, 2012

Rebased onto master.

@havvg
Copy link
Member Author

havvg commented Feb 7, 2012

@willdurand Good to go.

@willdurand
Copy link

Can you write some documentation on propelorm.github.com ?

willdurand added a commit that referenced this pull request Feb 10, 2012
@willdurand willdurand merged commit 4f0ccd5 into propelorm:master Feb 10, 2012
@willdurand
Copy link

Many many many thanks @havvg !!!

@havvg
Copy link
Member Author

havvg commented Feb 10, 2012

No promises on the "when", but I will add some docs for propelorm.github.com. Basic docs are in the README so far.

@willdurand
Copy link

I've added a very first draft: http://www.propelorm.org/cookbook/symfony2/the-symfony2-security-component-and-propel.html#acl_implementation (just got text fom the README).

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.

None yet

2 participants