core: restructure and improve acl routines (issue #463). #464

Merged
merged 4 commits into from Nov 23, 2012

Projects

None yet

2 participants

@kaos
Zotonic member

I'd like this reviewed before I commit it (for issue #463):

It started off with adding a guard to z_acl:wm_is_authorized/2.
Along with that I wanted to add some -spec's for that family of functions,
which led to adding -type's in webzmachine and m_rsc.

Also, it felt backwards to require the id to be integer at this stage,
which turned out to affect the acl modules handling the authorization.

Also, m_rsc:get_acl_props/2 always returns a #acl_props{} record when
given a integer id, even if no resource with that id exists, so to be
consistent I updated it so that also applies when presented with a
resource name.

This is a rather big chunk of changes in one commit, but they are rather
entangled, at least when applied in the order I made them.

@kaos kaos core: restructure and improve acl routines (issue #463).
It started off with adding a guard to z_acl:wm_is_authorized/2.
Along with that I wanted to add some -spec's for that family of functions,
which led to adding -type's in webzmachine and m_rsc.

Also, it felt backwards to require the id to be integer at this stage,
which turned out to affect the acl modules handling the authorization.

Also, m_rsc:get_acl_props/2 always returns a #acl_props{} record when
given a integer id, even if no resource with that id exists, so to be
consistent I updated it so that also applies when presented with a
resource name.

This is a rather big chunk of changes in one commit, but they are rather
entangled, at least when applied in the order I made them.
8a4bb5a
@kaos
Zotonic member

I've done some smoke testing on a default blog site using acl_adminsonly and a unpublished resource.
That seems to work fine, so it ought to work fine for other cases too...

@arjan
Zotonic member

This all looks very reasonable to me.
I'll test a bit with the simple roles ACL module.

@arjan
Zotonic member

I think these changes look good.
Would be nice to add them to the documentation of controller_template as well, though.

And I noticed that the acl dispatch option is not documented at all (it can have is_auth and logoff values)

👍

@kaos
Zotonic member

Nice :)

I'll fixup the docs, and if that looks good, I'll merge it in.

And, acl can also be a action, object pair, or a list of such entries (i.e. acl list... ;)

@arjan
Zotonic member

Yep, you're right, I looked over that :)

@kaos
Zotonic member

I hope the docs work, I made this on a machine where I've not setup sphinx... so I may hold on to the merge until I've been able to check it out first...

@kaos kaos merged commit 2320699 into zotonic:master Nov 23, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment