Carlo, Thanks for your detailed testing and thinking about this problem.
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. 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. 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. 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). -- 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 >
_______________________________________________ Geoserver-devel mailing list Geoserver-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geoserver-devel
