REST API: sharing of configuration snippets #921

Namoshek · 2016-09-06T09:48:58Z

Purpose

REST API description for the service that allows for sharing of configuration snippets.

Additional info

I checked the format with Drafter and generated a HTML documentation with the Apairy CLI.

TODO

Authentication endpoint
Database endpoint

Checklist

commit messages are fine (with references to issues)
I ran all tests and everything went fine
meta data is updated (e.g. README.md of plugins)

@markus2330 please review my pull request

Namoshek · 2016-09-06T09:50:04Z

So far I was unable to find a way to put schemata into a variable or something similar, so I copied it for each response. But I really don't like it doing this way. Maybe @omnidan has an idea?

markus2330 · 2016-09-06T12:08:14Z

What was the output of drafter (warnings?) and is the HTML docu available somewhere?

markus2330 · 2016-09-06T12:08:58Z

doc/rest_api/snippet_sharing/api-description.apib

+
+## Authenticate [POST]
+
+The API will try to authenticate the user by the submitted credentials. In case of success a session token (JWT) will be returned, which can be used for further requests to the API.


linebreaks?

API Blueprints need an empty line between +Body and the example output, for example. So in order to have a more general layout, I decided to use 3 line breaks in front of each first level heading, 2 line breaks in front of each second heading, and so on.

JWT is what i decided to use, because it is very convenient to use in combination with SPA-frontends. And it actually doesn't make a difference for the user, he just has to know how to submit it.

Ok, but isn't JWT the cause of the dependency problems? Did you look into the build-in auth from cppcms? Does JWT work with --cookie from curl? Maybe we open a seperate issue for authentification?

The JWT can easily be added to the Authorization header via curl, I did that before. I also added functionality so you can use it as additional GET parameter (?token=the_session_cookie) in case a separate header is not wanted.

Hence I don't really see it as dependency problem at all. I'm using the Boost library for other things as well (e.g. string splitting) and the Cryptlite "library" is only a header collection providing a more natural interface for cryptographic tasks.

But no, I didn't really look for CppCMS authentication stuff, but I also don't expect it to have some. Documentation doesn't give hints about it at all.

dependency problem Cryptlite

Yes, the problem is also not really "ours", but its a problem for every distribution. E.g. in Debian maintainers need to write a licence file where every copied source file must be described. Thus we should avoid distributing source files of other licences/authors. Furthermore it creates huge problems if there are security bugs. cppcms provides crypto and also much more.

Boost is ok if you need more than provided by [booster](http://cppcms.com/cppcms_ref/latest/namespacebooster.html and especially "booster::locale", the author of cppcms is also author of boost::locale, where a full copy is included in booster). The only reason against boost is the bloat of the binary size, because cppcms already uses an internal version of boost and if we use one more version the binary size grows again. But you can keep it, if you do not find an booster replacement in every case. (One problem of boost is that they do not really care about compatibility, please avoid these parts)

For crypto it is required to compile CppCMS with more dependencies (e.g. OpenSSL), so it would only be a shift of dependencies in my eyes. There is also no documentation on how to use it anywhere (yes I looked this up before).

The main problem I have with getting rid of the Cryptlite lib is that using cryptographic functions requires lots of initialization and stuff that is done by this library. So I would basically have to write it on my own again, achieving nothing else than work. Why should I do that? License problems of other people are not my business, to be honest.

For crypto it is required to compile CppCMS with more dependencies (e.g. OpenSSL), so it would only be a shift of dependencies in my eyes. There is also no documentation on how to use it anywhere (yes I looked this up before).

Cppcms on debian is already compiled with libgcrypt11, are there any problems with how it is compiled? It is not only a shift of dependencies. When we use libgcrypt, security bugs are libgcrypt's problems, if we distribute source, every problem is our problem. And cppcms even seems to have different security backends (similar to what @petermax2 made for Elektra's crypto plugin), i.e. if libgcrypt support end, we can switch easily.

And what do you mean with no docu?

Tutorial: http://cppcms.com/wikipp/en/page/cppcms_1x_sessions

Config: http://cppcms.com/wikipp/en/page/cppcms_1x_config#session

I read on sessions in CppCMS before, but they are cookie based only. I've never seen cookie sessions been used for any other API before, so I doubt this is good design..? Of course it would be more comfortable because it is there already, but there must be a reason why APIs use JWTs, oAuth and other mechanism in favour of cookies.

I understand your problem with the dependencies and I'll see what I can do about it, but I'm not promising anything. I was talking about crypto docu by the way, not sessions. I'm using crypto also for other taks (e.g. password hashing). The only docu I could find is the generated doxygen docu that doesn't give much info at all.

Maybe you can find out why they prefer other methods, just because others are doing it is not a good reason.

I'll see what I can do about it

Thank you.

Namoshek · 2016-09-06T12:25:37Z

What was the output of drafter (warnings?) and is the HTML docu available somewhere?

Drafter ends with OK. I uploaded the current HTML version to my server.

markus2330 · 2016-09-06T14:18:33Z

doc/rest_api/snippet_sharing/api-description.apib

+                        "type": "string"
+                    },
+                    "rank": {
+                        "description": "The authenticated users rank",


The rank is used for a very basic permission system. It allows to set a rank between 1 and 3, whereas 1 is a user, 2 something like a moderator and 3 an admin. Higher ranks will allow to delete and modify content of other users as well.

Thank you, can you add this to the API description, too? (Maybe as enum?)

Changed in the upcoming version, ty for the catch!

…on for possible messages, missing API functions)

Namoshek · 2016-09-07T10:25:11Z

I pushed a new version of the API description with a lot more details and better syntax. I also updated the rendered version on my server. As soon as the API works, we could add the host to the blueprint, which would add console support to the rendered documentation. Maybe it would also be a good idea to return the docu instead of the current plain html API description when accessing the API root?

markus2330 · 2016-09-07T12:12:38Z

Generated API looks nice (it seems to be located on apiary.io though)

Maybe it would also be a good idea to return the docu instead

A CSV like format (or JSON if you prefer it) with links to docu and versions for every method would be another way? Returning the whole docu seems to be hardly usable, or do you mean such a Apiary.Embed like it is done on your web server currently?

@omnidan Can you also take a look at the API?

markus2330 · 2016-09-07T12:23:08Z

Two small questions:

What are the scopes for? (Database)
Why is DELETE only possible for admin? Why should a user not be able to remove what he/she uploaded before?

Namoshek · 2016-09-07T12:41:06Z

Generated API looks nice (it seems to be located on apiary.io though)

Where have you seen anything about Apiary in the generated docs?

A CSV like format (or JSON if you prefer it) with links to docu and versions for every method would be another way? Returning the whole docu seems to be hardly usable, or do you mean such a Apiary.Embed like it is done on your web server currently?

I was talking about a generated doc like on my server at the moment. I don't think that REST services normally offer something like a WSDL. Of course I could just return the blueprint, but I think the generated doc is more helpful?

What are the scopes for? (Database)

Basically the idea was that it is easier searchable if there is some very crudely grouping involved in the highest hierarchy level, but with tagging (which idea came a bit later) it is probably not as useful anymore. Do you think I should remove this level?

Why is DELETE only possible for admin? Why should a user not be able to remove what he/she uploaded before?

It is only a docu mistake, of course the user should be able to delete own stuff. Thanks for the catch! 👍

markus2330 · 2016-09-07T12:56:04Z

Where have you seen anything about Apiary in the generated docs?

My browser tells me if content from other domains is loaded.

generated doc is more helpful?

Yes, if its similar to what you have on your webpage, it sounds useful. But then it is not really API anymore. There should be also some programmatic way to get some information, at least the version of the API.

Do you think I should remove this level?

This is a difficult question and depends on how it is solved in the web ui. Basically it is a question of user experience: If it is easy to use the scopes, and it is helpful for retrieving snippets, we should have it. If users are annoyed by it and only use tags anyway, it is better if we do not have it. Maybe the following question helps you for deciding: Will it be clear for a user when to use scopes and when to use tags?

Namoshek · 2016-09-07T13:05:22Z

My browser tells me if content from other domains is loaded.

Interesting. Well, it is generated by the Apiary-CLI, so it is quite likely that it is the same sort of documentation than they use for their service. There are other tools available too, I can try some of them as well.

Yes, if its similar to what you have on your webpage, it sounds useful. But then it is not really API anymore. There should be also some programmatic way to get some information, at least the version of the API.

To @omnidan you said version for the API is not really that important...?!

This is a difficult question and depends on how it is solved in the web ui. Basically it is a question of user experience: If it is easy to use the scopes, and it is helpful for retrieving snippets, we should have it. If users are annoyed by it and only use tags anyway, it is better if we do not have it. Maybe the following question helps you for deciding: Will it be clear for a user when to use scopes and when to use tags?

I guess yes, but without a good set of possible scopes it will be hard to find the right one. So I will get rid of it.

…appscope to scope, fixed description of DELETE entry method)

…erations)

…cation/json)

…more error codes, added new search parameters)

Namoshek · 2016-09-08T09:57:26Z

My browser tells me if content from other domains is loaded.

Interesting. Well, it is generated by the Apiary-CLI, so it is quite likely that it is the same sort of documentation than they use for their service. There are other tools available too, I can try some of them as well.

I tried the aglio doc generator which offers two different templates, both can be found on my server:

It works quite well too and the layout seems nice as well, but one or two things work different (e.g. many {?optional} parameters are not converted into ?param1&param2&param3, but rather in to ?param1?param2?param3. Biggest problem is though that he ommits a lot of output for the request attributes and parameters for example. That is a lot metter with the Apiary CLI.

omnidan · 2016-09-08T19:51:42Z

So far I was unable to find a way to put schemata into a variable or something similar, so I copied it for each response. But I really don't like it doing this way. Maybe @omnidan has an idea?

I haven't done this either, but the api-blueprint examples have helped me a lot when writing blueprints before. Maybe you can find something similar there?

If it is easy to use the scopes, and it is helpful for retrieving snippets, we should have it. If users are annoyed by it and only use tags anyway, it is better if we do not have it.

I think it will be hard to define scopes for us if we want to make them predefined. If they're user defined, I would definitely go for tags instead (much easier to maintain, more flexible, most snippets don't have one category, e.g. apache, php, wordpress)

@omnidan Can you also take a look at the API?

I just had a look at the API, here are my comments:

Authentication endpoint could be a /user resource (more RESTful)
- especially setrank is a little weird for a REST API (could be PUT /user/:id to edit a certain user or PUT /user to edit your own account)
- /auth is fine, because it's special (and /user/auth might conflict with /user/:id), /register could simply be POST /user, though
why does the HTTP status code need to also be part of the response body? (redundancy?)
Database endpoint is basically the Snippets endpoint? (where you get snippets from other users) Maybe a different name would be more appropriate here?

Namoshek · 2016-09-09T14:53:29Z

I haven't done this either, but the api-blueprint examples have helped me a lot when writing blueprints before. Maybe you can find something similar there?

Of course I found and used them already. It is possible to replace some code (e.g. the attributes of responses) by a model, but for the different i18n descriptions I'd have to define this one attribute every time either way. And if I replace the other 3 lines for example, the i18n is afterwards always on top, because the substitution for the model is appended to the list instead of prepended... that doesn't look nice. So for now I kept it quite c&p like. Most important would be a solution to only store schemata once, because they are freaking large, but this seems impossible.

Edit: After some trying I found out that schema declaration seems not necessary at all. When generating the doc with the apiary-cli, schema descriptions are generated automatically. That saves a lot of lines.

I think it will be hard to define scopes for us if we want to make them predefined. If they're user defined, I would definitely go for tags instead...

Thanks for your opinion, I'll go with tags instead!

Authentication endpoint could be a /user resource (more RESTful)

especially setrank is a little weird for a REST API (could be PUT /user/:id to edit a certain user or PUT /user to edit your own account)

/auth is fine, because it's special (and /user/auth might conflict with /user/:id), /register could simply be POST /user, though

Good point there. But concept wise (and also logically) I'm not so sure if it is a good idea to open some operations for the /user resource to the public (e.g. POST for registration) and some not (e.g. PUT for setrank/edit). But I could imagine that a mix of both, /auth and /auth/register like now and the rest like you suggested, could work quite well. Of course I could also create a POST /user resource that is only applicable for admins and that is used by /auth/register internally...

why does the HTTP status code need to also be part of the response body? (redundancy?)

That was part of the decision to put a textual description of the status in the response body as well. Theoretically this should be unnecessary, that is right. I also cannot really think of any reason why it should be necessary, all JS (front-end) libs that I know and use can handle headers and the response codes there. I think I'll remove them if no one can give me a good reason to not do so.

Database endpoint is basically the Snippets endpoint? (where you get snippets from other users) Maybe a different name would be more appropriate here?

Suggestion? 😄
Internally I'm often using the word repository, which seems equally in-/appropriate to me. snippets doesn't really sound that well either to be honest. It is also not really the name of a resource, but rather a prefix to avoid name collission (see /auth and /user). Therefore the naming is not too important in my eyes, but of course I'm open for suggestions!

omnidan · 2016-09-25T20:55:54Z

If you use 204 for PUT/DELETE, then you should definitely use 201 for POST as you are already using more specific response codes.

Alright, I changed that.

My PUT/DELETE requests also return a response with a i18n string when they succeed. It is not entirely necessary to be honest, but somehow I think it is better to have a common way to transport i18n keys than mixing them within frontend and backend. If I would return an empty response, I'd have to define the success key in the frontend and error keys in the backend, that doesn't seem to be a good design. What do you think?

You're right. Although, for my API, adding an i18n field everywhere doesn't make much sense as I never have a "success message", I only care about the data and errors in case they happen.

I tried it this way, but there are several problems:

Ah, I see, I thought it was inheriting the type. Re-writing only the i18n value:

        + i18n (string) - a unique token representing above error description message; can be used for internationalization in frontends

fixes all the issues except the dividing line (I don't think it's so bad that it's at the beginning), which seems to be a bug in apiary. I think that's still more readable than copy&pasting all of the attributes.

…tructure)

Namoshek · 2016-09-26T06:36:46Z

Ok, I changed it as well. I hope it will not cause troubles with Dredd later on.

markus2330 · 2016-09-26T15:48:38Z

doc/api_blueprints/snippet-sharing.apib

@@ -1,4 +1,5 @@
 FORMAT: 1A
+HOST: api.libelektra.org


Maybe "webapi.libelektra.org" or "restapi" is better?

I only added it as placeholder for the moment. I'll have to change it to some localhost url for testing, that's why I needed it. We can change it when the API and the deployment decisions are final.

…also contains plugin name)

…request and responses, removed format list from entry list resource, added plugin name to conversion list of entry details)

…t export)

…ayer in snippet details response)

…the entry list + removed description, which is a detail)

…te entry function)

…ts list + small structure change)

…1000000)

… root of api)

markus2330 · 2016-11-23T17:02:27Z

Build seems to fail?

Please remove "work in progress" if it is ready to review.

Namoshek · 2016-11-23T18:42:24Z

ELEKTRA CHECK FORMATTING

diff --git a/src/error/exporterrors.cpp b/src/error/exporterrors.cpp
index 63f79f9..63c9a64 100644
--- a/src/error/exporterrors.cpp
+++ b/src/error/exporterrors.cpp

Must be something broken on the master branch, can't be my fault.

I changed the host, so lets see if the tests still fail.

markus2330 · 2016-11-29T08:59:18Z

And the last piece! Thanks!

REST service: api-description draft

f0ff0fd

markus2330 reviewed Sep 6, 2016
View reviewed changes

REST service: api-description update (added enums, detailed descripti…

b75f927

…on for possible messages, missing API functions)

Namoshek added 2 commits September 7, 2016 17:43

REST service: api-description updated (removed normal scope, renamed …

dad8f60

…appscope to scope, fixed description of DELETE entry method)

REST service: api-description updated (changed description of some op…

6c47830

…erations)

markus2330 mentioned this pull request Sep 7, 2016

web ui: status? #252

Closed

Namoshek added 2 commits September 8, 2016 09:27

REST service: api-description updated (switch request format to appli…

e8de3fd

…cation/json)

REST service: api-description updated (added regex for inputs, added …

9f83497

…more error codes, added new search parameters)

Marvin and others added 2 commits September 9, 2016 17:28

REST service: api-description updated (removed status in response body)

c7891d1

REST service: api-description updated (removed schema descriptions)

28eb673

REST service: api-description updated (moved error response to data s…

7a59b04

…tructure)

markus2330 suggested changes Sep 26, 2016

View reviewed changes

markus2330 mentioned this pull request Sep 26, 2016

elektra-web HTTP API #912

Merged

7 tasks

Namoshek added 8 commits October 3, 2016 08:47

rest-backend: api-description updated (list of supported formats now …

1e59666

…also contains plugin name)

rest-backend: api-description updated (added example values for most …

712d520

…request and responses, removed format list from entry list resource, added plugin name to conversion list of entry details)

rest-backend: api-description updated (additional error for raw forma…

06d61ae

…t export)

rest-backend: api-description updated (formats resource fixed)

b2f3aac

rest-backend: api-description updated (fixed error code to match schema)

00824b2

rest-backend: api-description updated (removed not necessary 'data' l…

4d033c2

…ayer in snippet details response)

rest-backend: api-description updated (added more key-information to …

072cc16

…the entry list + removed description, which is a detail)

rest-backend: api-description updated (added validated flag to exports)

1a32ff8

Namoshek changed the title ~~REST API: description draft for sharing of conf snippets~~ REST API: sharing of configuration snippets Oct 10, 2016

Namoshek added 7 commits October 11, 2016 15:56

rest-backend: api-description updated (users sortable by rank)

35048fc

rest-backend: api-description updated (user permission check for crea…

ad2eccb

…te entry function)

rest-backend: api-description updated (added plugin statuses to forma…

e79d841

…ts list + small structure change)

rest-backend: api-description updated (added tags to entry output)

d9b5a02

rest-backend: api-description updated

0e1e28c

rest-backend: api-description updated (changed row limit from 200 to …

586abb1

…1000000)

rest-backend: api-description updated (status code & header fixed for…

0d1396e

… root of api)

rest-backend: api-description updated (changed host)

8bb42cc

Namoshek added ready to merge and removed work in progress labels Nov 23, 2016

markus2330 approved these changes Nov 29, 2016

View reviewed changes

markus2330 merged commit 18db87b into ElektraInitiative:master Nov 29, 2016

Namoshek deleted the rest-service-api-desc branch December 9, 2016 14:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

REST API: sharing of configuration snippets #921

REST API: sharing of configuration snippets #921

Namoshek commented Sep 6, 2016 •

edited

Loading

Namoshek commented Sep 6, 2016

markus2330 commented Sep 6, 2016

markus2330 Sep 6, 2016

markus2330 Sep 6, 2016

Namoshek Sep 6, 2016

markus2330 Sep 6, 2016

Namoshek Sep 6, 2016

markus2330 Sep 7, 2016

Namoshek Sep 7, 2016

markus2330 Sep 7, 2016

Namoshek Sep 8, 2016

markus2330 Sep 8, 2016

Namoshek commented Sep 6, 2016

markus2330 Sep 6, 2016

Namoshek Sep 6, 2016

markus2330 Sep 6, 2016

Namoshek Sep 7, 2016

Namoshek commented Sep 7, 2016

markus2330 commented Sep 7, 2016

markus2330 commented Sep 7, 2016

Namoshek commented Sep 7, 2016 •

edited

Loading

markus2330 commented Sep 7, 2016

Namoshek commented Sep 7, 2016

Namoshek commented Sep 8, 2016 •

edited

Loading

omnidan commented Sep 8, 2016 •

edited by Namoshek

Loading

Namoshek commented Sep 9, 2016 •

edited

Loading

omnidan commented Sep 25, 2016

Namoshek commented Sep 26, 2016

markus2330 Sep 26, 2016

Namoshek Oct 2, 2016

markus2330 commented Nov 23, 2016

Namoshek commented Nov 23, 2016

markus2330 commented Nov 29, 2016


		## Authenticate [POST]

		The API will try to authenticate the user by the submitted credentials. In case of success a session token (JWT) will be returned, which can be used for further requests to the API.

REST API: sharing of configuration snippets #921

REST API: sharing of configuration snippets #921

Conversation

Namoshek commented Sep 6, 2016 • edited Loading

Purpose

Additional info

TODO

Checklist

Namoshek commented Sep 6, 2016

markus2330 commented Sep 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Namoshek commented Sep 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Namoshek commented Sep 7, 2016

markus2330 commented Sep 7, 2016

markus2330 commented Sep 7, 2016

Namoshek commented Sep 7, 2016 • edited Loading

markus2330 commented Sep 7, 2016

Namoshek commented Sep 7, 2016

Namoshek commented Sep 8, 2016 • edited Loading

omnidan commented Sep 8, 2016 • edited by Namoshek Loading

Namoshek commented Sep 9, 2016 • edited Loading

omnidan commented Sep 25, 2016

Namoshek commented Sep 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markus2330 commented Nov 23, 2016

Namoshek commented Nov 23, 2016

markus2330 commented Nov 29, 2016

Namoshek commented Sep 6, 2016 •

edited

Loading

Namoshek commented Sep 7, 2016 •

edited

Loading

Namoshek commented Sep 8, 2016 •

edited

Loading

omnidan commented Sep 8, 2016 •

edited by Namoshek

Loading

Namoshek commented Sep 9, 2016 •

edited

Loading