Done in revision 1546891.
Adrian Crum
Sandglass Software
www.sandglass-software.com
On 10/14/2013 7:16 PM, 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?
