My plan is to deprecate the base-permission attribute and leave it
functioning as-is. The new attribute would be used in the trunk and in
any new web applications.
As usual, I will try to document everything as best I can.
Adrian Crum
Sandglass Software
www.sandglass-software.com
On 10/15/2013 8:59 AM, Jacques Le Roux wrote:
I did not look into all the details but this proposition sounds good to me.
I always wondered about OFBTOOLS and was not even aware about NONE.
I don't mind removing NONE (since it was undocumented) but it would be good to
have now a documentation about base-permission empty allows all accesses.
Of course having a ofbiz-component.xsd better documented would be a plus, but I
will not ask for the moon :)
Jacques
Ashish Vijaywargiya wrote:
Hello Adrian,
Your proposal looks good to me and yes it will make things much simpler.
+1
On Tuesday 15 October 2013 04:46 AM, Adrian Crum wrote:
Currently, the web application authorization is a jumbled up mess. I
would like to make things much simpler. The areas of improvement are
described below...
1. Configuring web application authorization is confusing:
<webapp name="party"
title="Party"
...
base-permission="OFBTOOLS,PARTYMGR"
mount-point="/partymgr"/>
In order for a user to access the "party" web application, they must
have two permissions: one starting with "OFBTOOLS_" and ending with
"_VIEW", and another starting with "PARTYMGR_" and ending with
"_VIEW". The boolean AND of the base-permission list was a constant
source of frustration for new users - we were getting regular mail
about that - but adding more information to the schema seems to have
solved that confusion.
The undocumented magic value "NONE" means the web application does not
require authorization.
Even worse is how the implementation interprets the configured
permission list. If I create the permission "PARTY_MGR_VIEW" and
change the web application configuration to:
<webapp name="party"
title="Party"
...
base-permission="OFBTOOLS,PARTY_MGR"
mount-point="/partymgr"/>
it doesn't work - because the implementation strips out the middle
bits of the permission string and checks for the permission
"PARTY_VIEW". This "feature" is confusing.
I believe the original intent was to give a user implicit permission
to use a web application if they already had a similar permission. I
don't think that is a good design, and the need to add the "OFBTOOLS"
permission to every web application configuration proves that. It
would be better to EXPLICITLY grant a user permission to use (or
access) a web application:
<webapp name="party"
title="Party"
...
access-permission="PARTY_MGR_VIEW"
mount-point="/partymgr"/>
This will prevent the unintentional side effect of giving users access
to the web application when they have similar permissions that are
needed for invoking services, etc.
If the access-permission attribute is empty or missing, then no
authorization is required to use the web application.
2. The web application authorization implementation has NO
encapsulation. The implementation is scattered everywhere. In
LoginWorker.java:
ComponentConfig.WebappInfo info =
ComponentConfig.getWebAppInfo(serverId, contextPath);
if (info != null) {
for (String permission: info.getBasePermission()) {
if (!"NONE".equals(permission) &&
!security.hasEntityPermission(permission, "_VIEW", userLogin)) {
return false;
}
}
} else {
Debug.logInfo("No webapp configuration found for : " + serverId +
" / " + contextPath, module);
}
The model of the webapp element is in the base component, and the code
(behavior) for that model is in the webapp component. That makes sense
from a separation-of-concerns perspective - the base component doesn't
"know" about web applications, so it merely generates a model of the
element, and the webapp component "knows" what to do with the model.
But that ideal separation breaks down when the base component
manipulates the model contents instead of just creating the model.
That is why the webapp base-permission list doesn't behave as
expected. In ComponentConfig.java (in the base component):
String basePermStr = element.getAttribute("base-permission");
if (UtilValidate.isNotEmpty(basePermStr)) {
this.basePermission = basePermStr.split(",");
} else {
this.basePermission = new String[] { "NONE" };
}
for (int i = 0; i < this.basePermission.length; i++) {
this.basePermission[i] = this.basePermission[i].trim();
if (this.basePermission[i].indexOf('_') != -1) {
this.basePermission[i] = this.basePermission[i].substring(0,
this.basePermission[i].indexOf('_'));
}
}
Clearly, that code does not belong there. Instead, it belongs in
LoginWorker.java (in the webapp component).
The code in LoginWorker.java is duplicated in every visual theme. In
Flat Grey's appbar.ftl:
<#assign displayApps =
Static["org.ofbiz.base.component.ComponentConfig"].getAppBarWebInfos(ofbizServerName,
"main")>
<#assign displaySecondaryApps =
Static["org.ofbiz.base.component.ComponentConfig"].getAppBarWebInfos(ofbizServerName,
"secondary")>
...
<#assign permissions = display.getBasePermission()>
<#list permissions as perm>
<#if (perm != "NONE" && !security.hasEntityPermission(perm, "_VIEW",
session))>
<#-- User must have ALL permissions in the base-permission list -->
<#assign permission = false>
</#if>
</#list>
Why are we doing that??!!
I propose we encapsulate all of the user web application authorization
code in LoginWorker.java. This will provide a central location to
determine if a user is authorized to access a web application, and it
will help make web application authorization behavior more consistent.
So, the code fragment in Flat Grey's appbar.ftl becomes:
<#assign displayApps =
Static["org.ofbiz.webapp.control.LoginWorker"].getAppBarWebInfos(userLogin,
ofbizServerName, "main")>
<#assign displaySecondaryApps =
Static["org.ofbiz.webapp.control.LoginWorker"].getAppBarWebInfos(userLogin,
ofbizServerName, "secondary")>
The lists contain only the web applications the user is authorized to
access.
What do you think?
