Ciao Jody, > > 1. Given the results, is it a bug? >> > Am I correct in thinking that the given endpoint cannot be used for > editing; or is it possible to update the entire file in one go and have it > preserve order? If the existing endpoint cannot be used with POST or PUT > then I would consider this a bug to be fixed. >
given the documentation (and the code implementation): > For reference from the openapi documentation page: > https://docs.geoserver.org/latest/en/api/#1.0.0/security.yaml > Geoserver is supporting the GET and the POST but not respecting the order (which is not the insertion even when xml is used). The PUT implements a PATCH pattern where you can modify only existing rows. This thread is mainly to add the full Rest PUT functionality where an user with granted PUT permission to /security/acl/rest endpoint can have the full control of the rest resource replacing it in one shot and respecting the order, no matter if the body is xml or json. The branch is implementing this under /security/acl/rest/update We can easily change the endpoint as it could clash with an ant path rule to be delete,post,put,get (pls ref to the swagger documentation) There may also be a second bug: > Andrea is correct about the JSON representation not providing a list to > represent order as significant. This may be the point to do a breaking > change for the JSON representation. > Yes indeed I'm trying with XML but even with that the insertion order is not respected and natural order is imposed. Json is not enforcing nor supporting an order but it is also not forbidding to make the correct use of the order when necessary. For an API like this, where the order is important, I think the order should be kept even when using Json as a container, > 2. Should I open a ticket or we should reopen the previous one? >> > > I think it would be kinder to reopen the previous ticket; so it is clear > that the attempted fix does not work. > Okay, thanks, I'm on hold. > 3. I see a function which is intentionally limiting only to ADMIN role >> these kind of calls checkUserIsAdmin(); >> > > Are you seeking to setup a new built-in role? In my thinking a user having > admin access grants ability to update the system (it should not matter if > they are using the REST API or the Admin Console). > I'm thinking of having a managerial role for some users below the superuser role ADMIN and I was thinking why f.e. the GROUP_ADMIN can't be used to share some system management permission over the platform? I've also tested (with no luck) inheriting my MNG from GROUP_ADMIN role to get more power but I can only see a few things more in the UI, on the REST /security/acl everything looks limited intentionally to the ADMIN role. So, I was wondering: it's actually impossible to control the permission level of the users under /security/acl without having to grant or inherit from the ADMIN role, is this something which can be discussed? Thank you for sharing your thoughts. Best, Carlo > > -- > Jody Garnett > > > On Oct 26, 2023 at 10:51:11 AM, carlo cancellieri < > geo.ccancelli...@gmail.com> wrote: > >> Dear Andrea and List, >> thank you for your reply and your explanations. >> >> Unfortunately we've *tested all the *documented* combinations* (json, >> xml) with no luck over the *geoserver** main branch or releases 2.17.2 >> and 2.23.2*. >> Let me share an example: >> >> Given a default* rest.properties:* >> ----------------------------------------------------------- >> /**;POST,DELETE,PUT=ADMIN >> /**;GET=ADMIN >> ----------------------------------------------------------- >> >> performing a *POST with XML* body (same will happen with json): >> >> curl -X *POST* >> http://localhost:8080/geoserver/rest/rest/security/acl/rest.xml -H >> "accept: application/json" -H "content-type: application/xml" -u >> admin:geoserver -d " >> <rules> >> *<rule resource="/xyz/**:GET">ROLE_AUTHENTICATED</rule>* >> </rules>" >> >> The *resulting rest.properties* file looks correct as the order is >> respected: >> ----------------------------------------------------------- >> /**;POST,DELETE,PUT=ADMIN >> /**;GET=ADMIN >> */xyz/**;GET=ROLE_AUTHENTICATED* >> ----------------------------------------------------------- >> The rule is appended to the end as expected and looks correct. >> >> Performing a *POST with XML* body like below (same will happen with >> json) everything changes: >> >> curl -X *POST* >> http://localhost:8080/geoserver/rest/rest/security/acl/rest.xml -H >> "accept: application/json" -H "content-type: application/xml" -u >> admin:geoserver -d " >> <rules> >> *<rule resource="/abc/**:GET">ROLE_AUTHENTICATED</rule>* >> </rules>" >> >> *Example 2*: - Pushing a new rule in *the rest.properties* file: >> ----------------------------------------------------------- >> /**;POST,DELETE,PUT=ADMIN >> * /abc/**;GET=ROLE_AUTHENTICATED* >> /**;GET=ADMIN >> /xyz/**;GET=ROLE_AUTHENTICATED >> ----------------------------------------------------------- >> It goes in the middle, so it is not appended anymore to the end (should >> be the key natural order but I'm not sure). >> >> *Example 2*: - Manually fixing the order in *the rest.properties* file >> (before the rest call): >> ----------------------------------------------------------- >> /abc/**;GET=ROLE_AUTHENTICATED >> /xyz/**;GET=ROLE_AUTHENTICATED >> /**;GET=ADMIN >> /**;POST,DELETE,PUT=ADMIN >> ----------------------------------------------------------- >> >> Then performing a *PUT with XML* body to update a rule: >> >> curl -X PUT >> http://localhost:8080/geoserver/rest/rest/security/acl/rest.xml -H >> "accept: application/json" -H "content-type: type: application/xml" -u >> admin:geoserver -d " >> <?xml version=\"1.0\" encoding=\"UTF-8\"?> >> <rules> >> *<rule resource=\"/abc/**:GET\">ROLE_AUTHENTICATED,TEST</rule>* >> </rules>" >> >> The *resulting rest.properties file is rewritten *messing up the work >> done manually >> ----------------------------------------------------------- >> /**;POST,DELETE,PUT=ADMIN >> /abc/**;GET=ROLE_AUTHENTICATED >> /**;GET=ADMIN >> /xyz/**;GET=ROLE_AUTHENTICATED,*TEST* >> ----------------------------------------------------------- >> >> TEST role is added to the rule but the *order is changed again*! >> >> For reference from the openapi documentation page: >> https://docs.geoserver.org/latest/en/api/#1.0.0/security.yaml >> >> In the patch I'm preparing I was thinking about fixing the situation by >> adding an additional endpoint as you (Andrea) suggested. >> >> */security/acl/rest/update* >> >> Performing a PUT on the rest (the above) endpoint the user may be able to >> send the entire properties file content updating the current rest resource >> (inserting, appending, deleting and updating in one call). >> >> This will allow people with rights to /security/acl/rest to properly >> update the rest.properties file in the way they prefer to respect the order >> and being able to also *insert,* not only to append to the end making >> the management of the *rest* resource easier and the update operation >> atomic. >> >> Example: >> >> Given the following rest.properties file, we want to insert something >> after row 2 >> --------------------------- >> /rest/**;GET=ROLE_AUTHENTICATED >> /**;POST,DELETE,PUT=ADMIN >> /**;GET=ADMIN >> --------------------------- >> >> We can get the entire file with a GET and than insert the row in the body >> resending the whole body updated: >> curl -X *PUT* >> http://localhost:8080/geoserver/rest/rest/security/acl/rest.xml -H >> "accept: application/json" -H "content-type: type: application/xml" -u >> admin:geoserver -d " >> <?xml version=\"1.0\" encoding=\"UTF-8\"?> >> <rules> >> <rule resource=\"/rest/**:GET\">ROLE_AUTHENTICATED</rule> >> *<rule resource=\"/rest/**:PUT,POST,DELETE\">ROLE_AUTHENTICATED</rule>* >> <rule resource=\"/**:POST,DELETE,PUT\">ADMIN</rule> >> <rule resource=\"/**:GET\">ADMIN</rule> >> </rules>" >> >> Will result in: >> --------------------------- >> /rest/**;GET=ROLE_AUTHENTICATED >> */rest/**;**PUT,**POST,DELETE=ROLE_AUTHENTICATED* >> /**;POST,DELETE,PUT=ADMIN >> /**;GET=ADMIN >> --------------------------- >> >> This is the branch with the changes I was planning to do a lot more: >> - checking roles during the load from the rest file or api to validate >> that the role already exists >> - filter by the ROLE so an user can only modify the rules where the role >> is applied >> - file upload endpoint >> - provide an interface >> >> This is why a big chunk of the work has been pushed into a dedicated >> class to better represent, validate and manage the model content (AntRule). >> Here is a diff against the geoserver master, all the old tests are >> running, I'm only missing the new endpoint test which will come. >> >> >> https://github.com/geoserver/geoserver/compare/main...ccancellieri:geoserver:rest_access_rule_editing >> >> I'm open to implement the tests and document as well as change the >> current implementation. >> I'd love to have an endpoint to update the rest configuration. >> >> Questions: >> 1. Given the results, is it a bug? >> 2. Should I open a ticket or we should reopen the previous one? >> 3. I see a function which is intentionally limiting only to ADMIN role >> these kind of calls checkUserIsAdmin(); this is also affecting us as we >> would like to use a MNG group to manage rest permissions which may not be >> entitled to do all the rest ADMIN can do, so I'm trying to inherit MNG from >> GROUP_ADMIN but even doing this the check is not allowing an user of MNG to >> work under /security/acl/... how can this better managed? >> >> *NOTE*: using the main branch I'm seeing that sometimes changing the >> group permission from the UI a tomcat restart is required to make changes >> effective while in the past we used it with no issue afaik. >> >> Best, >> C. >> >> >> >> Il giorno mer 25 ott 2023 alle ore 00:28 Andrea Aime < >> andrea.a...@geosolutionsgroup.com> ha scritto: >> >>> Hi Carlo, >>> I can confirm that all PUT operations in the whole GeoServer API have >>> been operating like PATCH since day one (around 2007?), >>> the ACL one is no exception. There is no way to fully overwrite any >>> resource like a PUT would do. >>> To achieve what you want you'd first have to delete everything and then >>> start over.... to be honest I don't see an easy way to do that. >>> >>> Maybe modify the controller method for PUT to have a new "replace" mode >>> as a query parameter? >>> Currently it just seems to be trying to locate existing rules that match >>> and update them, which means, it would preserve the order of the rules as >>> they were in the file, rather than the one you provided. >>> >>> Also about the ordering... are you using JSON or XML? Remember that JSON >>> is not order preserving, a representation >>> like this one has no order by definition (a JSON object is "an unordered >>> collection of zero or more name/value pairs"): >>> >>> { >>> "/**:GET":"ADMIN", >>> "/**:POST,DELETE,PUT":"ADMIN" >>> } >>> >>> while this one does have order instead (by XML definition): >>> >>> <rules> >>> <rule resource="/**:GET">ADMIN</rule> >>> <rule resource="/**:POST,DELETE,PUT">ADMIN</rule> >>> </rules> >>> >>> The default JSON representation of this resource really does not make >>> sense for this use case, should be a list.... using Map in the backend is >>> not a good fit for this case. >>> >>> Cheers >>> Andrea >>> >>> On Tue, Oct 24, 2023 at 11:07 PM carlo cancellieri < >>> geo.ccancelli...@gmail.com> wrote: >>> >>>> Dear List, >>>> looking for a REST api to update the rest.properties file I'm using >>>> the features provided by: >>>> https://github.com/geoserver/geoserver/wiki/GSIP-120 >>>> >>>> Unfortunately it seems that it's still affected by the ordering problem. >>>> I see it should be solved here >>>> https://osgeo-org.atlassian.net/browse/GEOS-5139 >>>> but I'm testing the main branch now and every time a post is >>>> performed the order of the file is messed up and all the settings (in terms >>>> of rule ordering) are lost. >>>> >>>> In addition to that it's not possible to operate with a real PUT >>>> operation replacing all the rules, it looks like it has been implemented >>>> more like a PATCH where only existing rules can be updated with a PUT. >>>> >>>> I've a branch with some ideas which is trying to address all the >>>> problems with minimum changes but I'm still confused about this rest api, >>>> and I don't want to go too far if this is not an issue.... >>>> >>>> I'd love to have the confirmation that this is still an open issue or >>>> I'm doing something wrong. >>>> >>>> Thank you all. >>>> >>>> Best, >>>> Carlo >>>> >>>> -- >>>> Carlo Cancellieri >>>> *Skype*: ccancellieri >>>> *Twitter*: @cancellieric >>>> *LinkedIn*: http://it.linkedin.com/in/ccancellieri/ >>>> _______________________________________________ >>>> Geoserver-devel mailing list >>>> Geoserver-devel@lists.sourceforge.net >>>> https://lists.sourceforge.net/lists/listinfo/geoserver-devel >>>> >>> >>> >>> -- >>> >>> Regards, >>> >>> Andrea Aime >>> >>> == >>> GeoServer Professional Services from the experts! >>> >>> Visit http://bit.ly/gs-services-us for more information. >>> == >>> >>> Ing. Andrea Aime >>> @geowolf >>> Technical Lead >>> >>> GeoSolutions Group >>> phone: +39 0584 962313 >>> >>> fax: +39 0584 1660272 >>> >>> mob: +39 339 8844549 >>> >>> https://www.geosolutionsgroup.com/ >>> >>> 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 >>> >> >> >> -- >> Carlo Cancellieri >> *Skype*: ccancellieri >> *Twitter*: @cancellieric >> *LinkedIn*: http://it.linkedin.com/in/ccancellieri/ >> _______________________________________________ >> Geoserver-devel mailing list >> Geoserver-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/geoserver-devel >> > -- Carlo Cancellieri *Skype*: ccancellieri *Twitter*: @cancellieric *LinkedIn*: http://it.linkedin.com/in/ccancellieri/
_______________________________________________ Geoserver-devel mailing list Geoserver-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geoserver-devel
