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.

    @Override
    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);
    }

    @Override
    public DataAccessLimits getAccessLimits(Authentication user,
ResourceInfo resource) {
        if (hideResource(resource)) {
            return hide();

        super.getAccessLimits(user, resource);
    }

    @Override
    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 <[email protected]> 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:
>
>     @Override
>     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 <[email protected]>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
>>
>> * getAccessLimits(Layer):
>>
>>  1. if the workspace of the layer does not match the local workspace, hide
>> access
>>  2. if the layer does not match the local layer, same as (1)
>>  3. 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 <[email protected]>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 <
>>> [email protected]> wrote:
>>>
>>>> On Tue, Mar 1, 2011 at 1:19 AM, David Winslow <[email protected]>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.
------------------------------------------------------------------------------
Free Software Download: Index, Search & Analyze Logs and other IT data in 
Real-Time with Splunk. Collect, index and harness all the fast moving IT data 
generated by your applications, servers and devices whether physical, virtual
or in the cloud. Deliver compliance at lower cost and gain new business 
insights. http://p.sf.net/sfu/splunk-dev2dev 
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Reply via email to