The call though to checking resource from the getAccessLimits(Layer) need not go through to the delegate, although i understand that in the original it does. This is what I would expect with my understanding of the new api. There is probably some redundant checking here but I don’t understand the life cycle of the api and what we can be sure gets called when.
@anonymised.com
public DataAccessLimits getAccessLimits(Authentication user, LayerInfo layer) {
if (LocalLayer.get() != null && !LocalLayer.get().equals(layer)) {
return hide();
}
if (hideResource(layer.getResource())) {
return hide();
}
return super.getAccessLimits(user, layer);
}
@anonymised.com
public DataAccessLimits getAccessLimits(Authentication user, ResourceInfo resource) {
if (hideResource(resource)) {
return hide();
super.getAccessLimits(user, resource);
}
@anonymised.com
public WorkspaceAccessLimits getAccessLimits(Authentication user, WorkspaceInfo workspace) {
if (hideWorkspace(workspace)) {
return new WorkspaceAccessLimits(CatalogMode.HIDE, false, false);
} else {
return super.getAccessLimits(user, workspace);
}
}
boolean hideResource(ResourceInfo resource) {
if (LocalLayer.get() != null) {
for (LayerInfo l : resource.getCatalog().getLayers(resource)) {
if (!l.equals(LocalLayer.get())) {
return true;
}
}
}
return hideWorkspace(resource.getStore().getWorkspace());
}
boolean hideWorkspace(Workspace workspace) {
if (LocalWorkspace.get() != null && !LocalWorkspace.get().equals(workspace)) {
return true;
}
return false;
}
On Thu, Mar 3, 2011 at 7:45 AM, David Winslow <dwinslow@anonymised.com> wrote:
The “mixing” of limits comes from the existing DataAccessManager, where getAccessLimits(LayerInfo) depends on the result of getAccessLimits(ResourceInfo). Here’s the code as it is in LocalWorkspaceDataAccessManager on 2.1.x:
@anonymised.com
public boolean canAccess(Authentication user, LayerInfo layer, AccessMode mode) {
if (super.canAccess(user, layer, mode)) {
if (LocalLayer.get() != null && !LocalLayer.get().equals(layer)) {
return false;
}
return this.canAccess(user, layer.getResource(), mode);
}
return false;
}
If that should just be checking the workspace, then I can do the same in my patch.
–
David Winslow
OpenGeo - http://opengeo.org/
On Wed, Mar 2, 2011 at 7:50 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:
Sorry, I am have kind of lost hold of this thread. And I am not really familiar with how the new ResourceAccessManager stuff works. The old api was easy to implement as it just involves returning true or false for each canAccess method.
I would think the LW wrapper should be simple… and no need to intersect or whatever access constraints. The logic should be more or less:
- getAccessLimts(WOrkspace):
If the workspace being access does not match the local workspace, return limits that hide access. Otherwise pass through to the delegate
- if the workspace of the layer does not match the local workspace, hide access
- if the layer does not match the local layer, same as (1)
- if none of the above delegate to the delegate
- getAccessLimits(Resource):
Basically apply 1 and 2 from the layer case with every layer that publishes the resource. If any of them are not (since there are usually only one) hide the resource. Otherwise again continue to the delegate.
So I am not seeing where any mixing of access limits should come into play…
On Wed, Mar 2, 2011 at 9:17 AM, David Winslow <dwinslow@anonymised.com> wrote:
I was hoping Justin would comment on this behavior, as it is there only because I was trying to emulate the behavior of the original LocalWorkspaceDataAccessManager in the LW ResourceAccessManager. As far as I can tell from reading code the changes you suggest should be fine, so I can work up a patch implementing them.
–
David Winslow
OpenGeo - http://opengeo.org/
On Tue, Mar 1, 2011 at 3:27 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:
On Tue, Mar 1, 2011 at 1:19 AM, David Winslow <dwinslow@anonymised.com> wrote:
Ok this was simple and made the code smaller so I went ahead and refactored to use a GeometryFilter.
Unless there’s any objections, I’ll go ahead and commit the latest patch on the ticket tomorrow.
Hi,
had a look at the current patch, I think I spotted a couple other issues.
The most serious one is that LocalWorkspaceDataAccessManager perform alien logic
that was not part of the wrapped access manager in case the layer/resource can be
exposed: I don’t see a need to perform the intersection between the layer rights
and the wrapped resource rights, and same goes for the resource vs workspace.
The code should simply take what the wrapped resource access manager returned
and pass it back without extra processing, it’s not its role to ensure there is proper
right containment between workspace, layer and resource.
Also consider that the current system allows for overrides, so one can have no rights
to the workspace as a whole but there can be an override for a specific layer, as far
as I can see the current code will break that behavior.
The second issue is more of a worry. When intersecting two geometries you don’t
check whether the result is an empty collection (symptom of no intersection).
In that case the filter should probably be flipped to Filter.EXCLUDE instead of
asking for an intersection with an empty geometry.
However, about this one I’m not completely sure, it may end up working fine as is.
Cheers
Andrea
–
Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 962313
mob: +39 333 8128928
http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.youtube.com/user/GeoSolutionsIT
http://www.linkedin.com/in/andreaaime
http://twitter.com/geowolf
–
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
–
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
