Hello Andrea,
thanks for your review, pr contains now your feedbacks.

Il giorno gio 31 ott 2019 alle ore 18:16 Andrea Aime <
[email protected]> ha scritto:

> Hi,
> after checking the code I've realized that there is another important
> issue to be considered: who can edit the rules?
> Right now, only a full admin can edit security access rules, as that page
> is not visible to workspace admins.... but they
> can see the layers, so this GSIP actually makes workspace admins able to
> edit security rules, while before they could
> have not.
> I would make the tabs visible only to a full admin by default, and if you
> really need it to be available to workspace admins
> too, I'd suggest to add a configuration to enable/disable it (defaulting
> to disabled, to avoid security breaches on upgrades).
>
> Also, looking at the UI and code, it seems that "admin" rights can be set
> on layers and layer groups... but I believe this
> is misleading, admin level access are performed only on workspaces, in the
> UI and REST API, if memory serves me right...
> the documentation still has a note about it:
> https://docs.geoserver.org/stable/en/user/security/layer.html#access-modes
>
> I suggest to check if this is still the case, and if so, don't offer admin
> rights in the UI for layers and groups... that said, the current
> UI allows to setup admin rights on layers... I believe due to the GUI
> being dumb... of well, not a big deal in the end, it's misleading
> but "documented", see what you want to do about it (eventually nothing).
>
> Cheers
> Andrea
>
>
>
> On Thu, Oct 31, 2019 at 4:55 PM Andrea Aime <[email protected]>
> wrote:
>
>> Hi Nuno,
>> just read the proposal. I'm generally good with ut, but the module
>> capabilities bit worries me, it seems
>> the design only considered GeoFence as a possible ResourceAccessManager
>> alternative.
>> In fact, there is plenty of ResourceAccessManager around (it's one of the
>> bits we customize the most in
>> custom setups), but the code, as written, will keep on displaying the
>> tabs even if the they can't be used, unless
>> those modules are upgraded to expose the right module metadata (most of
>> the time, they don't have a module
>> metadata at all).
>>
>> The security subsystem in SecureCatalogImpl works by looking in the
>> classpath for a ResourceAccessManager
>> implementation, and if one is found, then it's used, otherwise the
>> default one is used:
>>
>> https://github.com/geoserver/geoserver/blob/e0771561a5e131f33387b0a0197b3c7bb577848f/src/main/src/main/java/org/geoserver/security/SecureCatalogImpl.java#L103
>>
>> Given the new UI makes sense only if the DefaultResourceAccessManager is
>> used, I'd suggest
>> a simpler alternative: ask SecureCatalogImpl what it is using (there is
>> only one SecureCatalogImpl in the classpath).
>>
>> Now, the ResourceAccessManager used is already available via a getter,
>> but it can be wrapped, and the wrapper
>> interface is not unwrapping friendly. Options:
>>
>>    - Add a simple boolean field "usingDefaultAccessManager" in
>>    SecureCatalogImpl that would be set while creating the
>>    DefaultResourceAccessManager, and a getter to get it
>>    - Or if you prefer, make the resorce access manager wrappers
>>    unwrapping friendly, and if you get a wrapper back from the catalog, query
>>    it and check if it's wrapping a DefaultResourceAccessManager. See also the
>>    java.sql.Wrapper class.
>>
>> I'm also going to have a look at the PR and add extra feedback there.
>>
>> Cheers
>> Andrea
>>
>>
>> On Thu, Oct 10, 2019 at 11:36 AM [email protected] <
>> [email protected]> wrote:
>>
>>> Dear all,
>>>
>>> I would like to submit to your attention the following geoserver
>>> improvement proposal:
>>>
>>> https://github.com/geoserver/geoserver/wiki/GSIP-182.
>>>
>>> Best regards,
>>> Marco Volpini
>>> _______________________________________________
>>> Geoserver-devel mailing list
>>> [email protected]
>>> https://lists.sourceforge.net/lists/listinfo/geoserver-devel
>>>
>>
>>
>> --
>>
>> Regards, Andrea Aime == GeoServer Professional Services from the experts!
>> Visit http://goo.gl/it488V for more information. == Ing. Andrea Aime
>> @geowolf Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054
>> Massarosa (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339
>> 8844549 http://www.geo-solutions.it http://twitter.com/geosolutions_it
>> ------------------------------------------------------- *Con riferimento
>> alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 -
>> Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni
>> circostanza inerente alla presente email (il suo contenuto, gli eventuali
>> allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i
>> destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per
>> errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le
>> sarei comunque grato se potesse darmene notizia. This email is intended
>> only for the person or entity to which it is addressed and may contain
>> information that is privileged, confidential or otherwise protected from
>> disclosure. We remind that - as provided by European Regulation 2016/679
>> “GDPR” - copying, dissemination or use of this e-mail or the information
>> herein by anyone other than the intended recipient is prohibited. If you
>> have received this email by mistake, please notify us immediately by
>> telephone or e-mail.*
>>
>
>
> --
>
> Regards, Andrea Aime == GeoServer Professional Services from the experts!
> Visit http://goo.gl/it488V for more information. == Ing. Andrea Aime
> @geowolf Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054
> Massarosa (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339
> 8844549 http://www.geo-solutions.it http://twitter.com/geosolutions_it
> ------------------------------------------------------- *Con riferimento
> alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 -
> Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni
> circostanza inerente alla presente email (il suo contenuto, gli eventuali
> allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i
> destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per
> errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le
> sarei comunque grato se potesse darmene notizia. This email is intended
> only for the person or entity to which it is addressed and may contain
> information that is privileged, confidential or otherwise protected from
> disclosure. We remind that - as provided by European Regulation 2016/679
> “GDPR” - copying, dissemination or use of this e-mail or the information
> herein by anyone other than the intended recipient is prohibited. If you
> have received this email by mistake, please notify us immediately by
> telephone or e-mail.*
>
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Reply via email to