search

moqui / moqui-framework

Use Moqui Framework to build enterprise applications based on Java. It includes tools for databases (relational, graph, document), local and web services, web and other UI with screens and forms, security, file/resource access, scripts, templates, l10n, caching, logging, search, rules, workflow, multi-instance, and integration.
http://www.moqui.org
Other
284 stars 204 forks source link

ArtifactAuthz deny will not override allow #110

Closed chunlinyao closed 8 years ago

chunlinyao commented 8 years ago

User may belong to multiple user groups, If different user groups have deny and allow authz on same artifact, the deny will not override allow. current code will return true if an allow authz exists and no denyaacv before it. https://github.com/moqui/moqui-framework/blob/ec6d7cc46a59ef7702b393d9f52522a7f58732bb/framework/src/main/groovy/org/moqui/impl/context/ArtifactExecutionFacadeImpl.groovy#L346

Because the order of aacvList not guarantees for deny before allow.

BTW: If there's multiple allow authz (eg. one has inheritAuthz="Y", another has inheritAuthz="N"), Using which one is indeterminable.

jonesde commented 8 years ago

Good point, that is an issue. I made some changes in commit #3b724c2 that should address this...

chunlinyao commented 8 years ago

Confirmed. Another question: If a parent screen has require-authentication="anonymous-view" all subscreens will be accessable even if require-authentication="true", Is this a desired behavior?

jonesde commented 8 years ago

The only place this is used so far in the moqui repositories is in the PopCommerceRoot.xml screen (mounted in the screen tree on /popc) and the Profile screens under it. The current behavior is to still require authentication but authorization remains open, ie no authorization is required for screens under that.

I don't know if it is the best possible behavior, but that is the current behavior so that applications like the PopCommerce ecommerce screens can require authentication but not require that users be added to a group that has authorization for the screens.

chunlinyao commented 8 years ago

Despite whether it is the best behavior, I think we should document the behavior in xsd.

jonesde commented 8 years ago

I added a better explanation of the behavior in commit #0a08279.