> > *So far I don't see how the existence of SOAP complicates things :)*
=> See below list of what a potential v2 of the Rest API could/should fix. I don't see how this can be fixed while keeping SOAP and REST in the same class definition: *Security - 2.4. Never expose sensitive information on URLs* Reference: See https://restfulapi.net/security-essentials/ Quote: *Usernames, passwords, session tokens, and API keys should not appear in the URL, as this can be captured in web server logs, which makes them easily exploitable. *(aka == do not use GET parameters for sensitive information) Problematic OpenMeetings REST API: - UserService::login => Sends both username and password as GET / URL parameters - All methods (except a few) => Send security token as GET / URL parameter => Severity *High *=> Essentially no Client side JavaScript client should ever use our API. Anybody can use a simple DNS sniffer to read the blank password or get a god mode security token. Or for example you can read the security credentials by trailing the access log, since the full URL including GET parameters will be logged. *Security - 2.3. Use Password Hash* Reference: See https://restfulapi.net/security-essentials/ Self explanatory. => Severity *Medium/High *I think similarly important but its not as High Severity as 2.4 *Using appropriate HTTP response codes* See: https://restfulapi.net/http-status-codes/ We currently return HTTP 200 no matter if the call was successful or not (eg SID expired or wrong). We only return 500 in case there was a server error (actually that's is not quite true, I think there might be cases where application errors also throw 500) We should use appropriate HTTP codes for different kind of scenarios: Eg: Use 4xx errors for user errors: - Return 401 if login fails, 403 is SID expired or invalid - Return 400 if a BadRequest was sent (eg parameter validation fails) - Return 404 in case you request an item but none was found Use 5xx errors only for real server side errors, eg internal server error (eg database down) => Severity *Medium - *I think this would be better to be fixed, but compared to above 2.4 it's probably not quite as high *Appropriate usage of HTTP Methods GET just returns, doesn't action anything* See: https://restfulapi.net/http-methods/ We use a few examples where the HTTP Method is "GET" but it actually creates or actions something. For example: UserServier::login -> GET method. This should be a POST. Simply also because you should never expose user/pass as get parameters. RoomService::kick -> Currently "GET", but this is clearly doing something. It creates a request that kicks users. (And then returns something). But it is doing something. There are more examples for this. I think some methods have been made a GET method just because they have a single parameter. But that is not the criteria to use GET or POST. GET should never change any state or action something. => Severity *Medium - *I think this would be better to be fixed, but compared to above 2.4 it's probably not quite as high *POST methods should create single object in post body * There is a good explanation what we currently do around "Form params", see: https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/POST#example This is probably more debatable but I would just suggest a single request body in JSON format and any other "meta" information as a potential GET parameter. Example: UserService::add - String SID => should be not a Get parameter, but probably a header parameter - User user => Post body, plain JSON - Boolean confirm => Meta data, simple GET Parameter. Won't be "saved" with the object, but just some additional information This also has the advantage that we simply can use Accept and Content-Type:application/json headers for all methods input and output. => Severity *Medium - *I think this would be better to be fixed, but compared to above 2.4 it's probably not quite as high => I can't see how realistically we would be able to just change the current API with the above while its SOAP and REST in a single API. The differences are just too big. *I'm not sure if SOAP is being used in any integration so we can drop SOAP > support* => I would suggest to neither drop nor introduce breaking changes into current APIs. Neither SOAP nor REST. But rather deprecated and slowly phase out over time. Eg deprecated and then after 3 major releases drop. That gives reasonable notice to people to migrate using a new version of the API. Just because there isn't a reply to this email doesn't mean there is nobody using it. I would VOTE for a better API > Please step in and let's discuss how to make it better :) See above list. Those are the main things I can think of. And I think those would be good candidates to introduce a v2 rest only version of the API. And then -over time- deprecate the old SOAP/REST API and savely over 2-3 major releases then drop it. Thanks Seb Sebastian Wagner Director Arrakeen Solutions, OM-Hosting.com http://arrakeen-solutions.co.nz/ https://om-hosting.com - Cloud & Server Hosting for HTML5 Video-Conferencing OpenMeetings <https://www.youracclaim.com/badges/da4e8828-743d-4968-af6f-49033f10d60a/public_url> <https://www.youracclaim.com/badges/b7e709c6-aa87-4b02-9faf-099038475e36/public_url> On Thu, 23 Sept 2021 at 20:43, Maxim Solodovnik <solomax...@gmail.com> wrote: > > > On Thu, 23 Sept 2021 at 04:47, seba.wag...@gmail.com < > seba.wag...@gmail.com> wrote: > >> Hi all! >> >> I think we may try to solve too many things at the same time in this >> discussion. But also in our API. It just seems things are a bit too tightly >> coupled. >> >> For example: >> *We can't change/update REST methods, cause they affect SOAP calls. >> Currently we have a just a single class that does both* >> => It was easy when the API was simple. But then over time it actually >> became harder, cause we discovered that under the covers Soap and Rest >> representation is different. >> > > So far I don't see how the existence of SOAP complicates things :) > > > >> *We introduced REST 10years ago and the definitions of REST changed over >> time* >> => We created the OpenMeetings REST API over 10years(!!) ago. At that >> time the definition of REST was quite fluent. By now there is quite a >> different and more strict understanding of what Rest means. And how HTTP >> methods (GET/POST/PUT/DELETE), resource paths ( having nouns not verbs as >> resource paths, eg: /users/$identifier), using the right kind of >> request/response message structure or for example how >> authentication/security works. >> >> Things have changed. And I actually think by now it is taking more time >> and effort to try to get Soap/Rest into 1 class. Then separating them. As >> well as it's just maybe a long time since we had a major uplift. >> >> How about we do the following: >> >> 1) We keep the current SOAP/REST API structure as is >> => Minimise rework / No breaking changes >> => We accept some minor quirks around some *documentation only* >> annotations in order to document it in a way people can use it >> >> 2) For v7.0.0 or v8.0.0 we introduce a v2 of the Rest API >> But: >> => v2 is REST only. The existing/v1 API is the SOAP/REST API. And SOAP >> stays where it is. That way we have less work in trying to make 2 things >> into 1. >> > > I'm not sure if SOAP is being used in any integration > so we can drop SOAP support, but IMO this is another question :) > > >> => Soap and Rest can still use the same "adapter" class in order to >> achieve 'feature parity'. But we do _not_ attempt for example that method >> parameters need to match >> => Having a single v2 REST only interface makes it also easier to write >> integration tests. Cause you only need to test the REST interface. >> => There isn't really any issue with the SOAP interface. SOAP hasn't >> changed in the last 10years. There isn't any need to go for a v2 in the >> SOAP API >> >> 3) We agree _before_ adding a v2 what REST "guidelines" we adopt >> => Instead of arguing what kind of HTTP method, security headers, POST >> body parameters we adopt a guideline/standard document. And simply comply >> with that standard. >> => This will also make it a lot easier to: >> A) Integrate with it, cause people are used to the >> format/standard/guidelines and there is less discussion needed >> B) The tooling will be much easier, because all the code generators, >> documentation tools, CXF-RS, json-mappers we are using are referencing a >> similar or same guidelines/standard. So we not constantly need to customise >> things to fit into how the existing OpenMeetings API works >> An example of a REST guideline/standard that we could adopt is: OpenAPI >> 3.0.x (https://swagger.io/specification/) + Restful recommendations ( >> https://restfulapi.net/) >> >> @Maxim Solodovnik <solomax...@gmail.com> what do you think? I think more >> than 10years is enough for a single version of an API. I think by now it >> actually is _less_ work to have a new v2 Rest only version of the API then >> trying to somehow create a SOAP/REST API and try to enhance that into a >> "RESTful" way but not break it at the same time. >> > > I would VOTE for a better API > Please step in and let's discuss how to make it better :) > > >> Thanks, >> Seb >> >> Sebastian Wagner >> Director Arrakeen Solutions, OM-Hosting.com >> http://arrakeen-solutions.co.nz/ >> https://om-hosting.com - Cloud & Server Hosting for HTML5 >> Video-Conferencing OpenMeetings >> >> <https://www.youracclaim.com/badges/da4e8828-743d-4968-af6f-49033f10d60a/public_url> >> <https://www.youracclaim.com/badges/b7e709c6-aa87-4b02-9faf-099038475e36/public_url> >> >> >> On Thu, 23 Sept 2021 at 02:24, Daniel Baker <i...@collisiondetection.biz> >> wrote: >> >>> Can you not employ an extra programmer? >>> On 22/09/2021 13:24, Maxim Solodovnik wrote: >>> >>> >>> >>> On Wed, 22 Sept 2021 at 19:20, Ali Alhaidary < >>> ali.alhaid...@the5stars.org> wrote: >>> >>>> >>>> On 9/22/21 3:14 PM, Maxim Solodovnik wrote: >>>> >>>> I only can do manual testing here :( >>>> >>>> What is manual testing? >>>> >>> >>> I'm installing everything >>> setting up integration and do clicking :))) >>> >>> >>>> IMO these changes (if we will be able to do them) worth to be done >>>> >>>> what is IMO ? >>>> >>> >>> In My Opinion :) >>> >>>> Why I raise some old design issues: we can do changes now and let the >>>> API unchanged for another several years :))) >>>> >>>> What is several years ;-) >>>> >>> >>> Well I believe REST API was changed 2-3 times, so we are trying to keep >>> it stable >>> v1/v2 etc. approach can also be applied >>> the problem here: I don't have enough time to maintain more than one >>> version :(( >>> >>> >>>> On Wed, 22 Sept 2021 at 19:09, Ali Alhaidary < >>>> ali.alhaid...@the5stars.org> wrote: >>>> >>>>> The issue here is that, It is a lot of work, and, a lot of testing >>>>> that follows. We are not a direct API users, however, moodle plugin is. >>>>> Along the road, things could break in such change. So, if you see this >>>>> change is the the way forward, I am in with as usual a dedicated >>>>> production >>>>> server for selected teaches/students as long as the old work (mainly >>>>> recordings) is not lost, and, only one environment is used (as is now), >>>>> i.e. moodle plugin can handle all the communication. >>>>> >>>>> The issue is being discussed by only three people, how many others are >>>>> using these APIs ? How many apps are up and running on them now ? looking >>>>> at the moodle plugin downloads, >>>>> https://moodle.org/plugins/mod_openmeetings/stats there is a peak >>>>> during the past year, and I am sure the case is the same with other LMS >>>>> and >>>>> custom built apps, keeping in mind that OM can work exceptional good by >>>>> itself. >>>>> >>>>> Ali >>>>> >>>>> >>>>> On 9/22/21 2:16 PM, Maxim Solodovnik wrote: >>>>> >>>>> These changes are only being discussed >>>>> Nothing is broken, yet :)))) >>>>> we can @Deprecate these old methods and/or move it to some prefixed URL >>>>> so API users will need to change base URL from >>>>> https://localhost:5443/openmeetings to >>>>> https://localhost:5443/openmeetings/v1 >>>>> >>>>> On Wed, 22 Sept 2021 at 13:14, seba.wag...@gmail.com < >>>>> seba.wag...@gmail.com> wrote: >>>>> >>>>>> @Ali Alhaidary <ali.alhaid...@the5stars.org> >>>>>> The other alternative to fix the issue AND make it backwards >>>>>> compatible would be to have a /v2 version of the API >>>>>> >>>>>> So all endpoints would be duplicated to have version /v2 of the API >>>>>> (with maybe some other fixes) >>>>>> and the current API stays the same. But would not receive any >>>>>> improvements anymore/deprecated. >>>>>> >>>>>> But that would be quite a bit of work. But yeah, that is what people >>>>>> do when they want to avoid breaking changes. Need to do versioning. >>>>>> >>>>>> Thanks >>>>>> Seb >>>>>> >>>>>> >>>>>> Sebastian Wagner >>>>>> Director Arrakeen Solutions, OM-Hosting.com >>>>>> http://arrakeen-solutions.co.nz/ >>>>>> https://om-hosting.com - Cloud & Server Hosting for HTML5 >>>>>> Video-Conferencing OpenMeetings >>>>>> >>>>>> <https://www.youracclaim.com/badges/da4e8828-743d-4968-af6f-49033f10d60a/public_url> >>>>>> <https://www.youracclaim.com/badges/b7e709c6-aa87-4b02-9faf-099038475e36/public_url> >>>>>> >>>>>> >>>>>> On Wed, 22 Sept 2021 at 18:10, Ali Alhaidary < >>>>>> ali.alhaid...@the5stars.org> wrote: >>>>>> >>>>>>> We are using OM in production with moodle front end, we can not >>>>>>> tolerate downtime neither with OM or its plugin (that needs fixing, but >>>>>>> living with), and to tell you the truth, I do not see it as 'broken' >>>>>>> from >>>>>>> that angle. >>>>>>> >>>>>>> So my answer is B. >>>>>>> >>>>>>> Ali >>>>>>> On 9/22/21 2:10 AM, seba.wag...@gmail.com wrote: >>>>>>> >>>>>>> It is broken. The problem is the fix will be a breaking change that >>>>>>> will require 3rd party integration code to be fixed. Not a big fix, but >>>>>>> a >>>>>>> fix. Eg the Moodle Plugin requires some minor changes. >>>>>>> >>>>>>> The workaround is to write some additional wrapper code to make it >>>>>>> backwards compatible. Which is also a bit confusing. >>>>>>> >>>>>>> I also don't understand quite if you answer is pro or contra >>>>>>> changing the response. >>>>>>> >>>>>>> So is your statement: >>>>>>> A) Yes, lets fix it to align our JSON response with what the >>>>>>> schema/method signature says. We don't like wrapper objects. And I am >>>>>>> happy >>>>>>> that people have to change their integration code to use newer versions >>>>>>> of >>>>>>> OpenMeetings. >>>>>>> B) No, lets leave it like this for now and we do whatever other >>>>>>> additional code we need to write to workaround so that our documentation >>>>>>> and schema matches what the actual API responses look like >>>>>>> >>>>>>> If you could please clarify if you are A, B. Or if you don't mind >>>>>>> either way/no strong opinion :) >>>>>>> >>>>>>> Thanks >>>>>>> Seb >>>>>>> >>>>>>> >>>>>>> Sebastian Wagner >>>>>>> Director Arrakeen Solutions, OM-Hosting.com >>>>>>> http://arrakeen-solutions.co.nz/ >>>>>>> https://om-hosting.com - Cloud & Server Hosting for HTML5 >>>>>>> Video-Conferencing OpenMeetings >>>>>>> >>>>>>> <https://www.youracclaim.com/badges/da4e8828-743d-4968-af6f-49033f10d60a/public_url> >>>>>>> <https://www.youracclaim.com/badges/b7e709c6-aa87-4b02-9faf-099038475e36/public_url> >>>>>>> >>>>>>> >>>>>>> On Wed, 22 Sept 2021 at 10:59, Ali Alhaidary < >>>>>>> ali.alhaid...@the5stars.org> wrote: >>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> We have an old saying 'If it is not broken, do not fix it' ;-) >>>>>>>> >>>>>>>> Ali >>>>>>>> On 9/22/21 12:46 AM, seba.wag...@gmail.com wrote: >>>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> as discussed in the comments section in >>>>>>>> https://github.com/apache/openmeetings/commit/4daf7c1f53738cd786dc976114cc5278b4f05f4f#comments >>>>>>>> >>>>>>>> >>>>>>>> we would like to propose a breaking change for the OpenMeetings >>>>>>>> Json/Rest API in v7.0.0 >>>>>>>> >>>>>>>> Problem: JSON response wrapping >>>>>>>> Currently CXF-RS is configured to wrap the JSON response into >>>>>>>> another object. >>>>>>>> >>>>>>>> Example: Method signature: public List<AppointmentDTO> range(...) { >>>>>>>> ... } (Example taken from >>>>>>>> https://github.com/apache/openmeetings/blob/master/openmeetings-webservice/src/main/java/org/apache/openmeetings/webservice/CalendarWebService.java#L111 >>>>>>>> ) >>>>>>>> >>>>>>>> OLD/CURRENT JSON Response: >>>>>>>> { >>>>>>>> "appointmentDTO": >>>>>>>> [ >>>>>>>> { >>>>>>>> itemXYZ: 123, ... >>>>>>>> } >>>>>>>> ] >>>>>>>> } >>>>>>>> >>>>>>>> >>>>>>>> Proposed NEW/UPDATED JSON Response: >>>>>>>> // no wrapping object around it, just return list >>>>>>>> [ >>>>>>>> { >>>>>>>> itemXYZ: 123, ... >>>>>>>> } >>>>>>>> ] >>>>>>>> >>>>>>>> Reasoning: The wrapping "{ "appointmentDTO": ... }" should be >>>>>>>> dropped from the json response body. "appointmentDTO" is generated but >>>>>>>> it >>>>>>>> is not in any schema definition or method signature. Cause there is >>>>>>>> nothing >>>>>>>> in the method signature that would tell anybody where " >>>>>>>> "appointmentDTO": >>>>>>>> [" is coming from. Other than by testing the API call and finding out >>>>>>>> by >>>>>>>> try and error. >>>>>>>> >>>>>>>> CXF-RS allows configuring our Web Service to NOT generate that >>>>>>>> wrapping element. And turn this behaviour off and just generate the >>>>>>>> list. >>>>>>>> See "dropRootName" in the CXF docs at: >>>>>>>> >>>>>>>> https://cxf.apache.org/docs/jax-rs-data-bindings.html#JAXRSDataBindings-WrappingandUnwrappingJSONsequences >>>>>>>> >>>>>>>> *This affects all methods returning a JSON response body (which is >>>>>>>> pretty much every API Method)* >>>>>>>> >>>>>>>> Please reply to this email if you have concerns, questions or >>>>>>>> objections. >>>>>>>> >>>>>>>> Thanks! >>>>>>>> Seb >>>>>>>> >>>>>>>> Sebastian Wagner >>>>>>>> Director Arrakeen Solutions, OM-Hosting.com >>>>>>>> http://arrakeen-solutions.co.nz/ >>>>>>>> https://om-hosting.com - Cloud & Server Hosting for HTML5 >>>>>>>> Video-Conferencing OpenMeetings >>>>>>>> >>>>>>>> <https://www.youracclaim.com/badges/da4e8828-743d-4968-af6f-49033f10d60a/public_url> >>>>>>>> <https://www.youracclaim.com/badges/b7e709c6-aa87-4b02-9faf-099038475e36/public_url> >>>>>>>> >>>>>>>> >>>>> >>>>> -- >>>>> Best regards, >>>>> Maxim >>>>> >>>>> >>>> >>>> -- >>>> Best regards, >>>> Maxim >>>> >>>> >>> >>> -- >>> Best regards, >>> Maxim >>> >>> > > -- > Best regards, > Maxim >
