Navigation Menu

Skip to content

Instantly share code, notes, and snippets.

@carlobeltrame
Last active May 22, 2019 07:48
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
  • Save carlobeltrame/8dd5b5e6279d91d1e3c181cb9086666a to your computer and use it in GitHub Desktop.
Save carlobeltrame/8dd5b5e6279d91d1e3c181cb9086666a to your computer and use it in GitHub Desktop.

Acceptance Criteria

  • For every layer you can create/delete impersonal API Tokens
  • A token can be described by a name and a description
  • A token is attached to a certain layer, its rights are derived from said layer the token's settings
  • API Token can only be created and edited by people with :layer_and_below_full and :layer_full permissions
  • per Token I can configure whether a specific API token can
    • read People on Layer
    • read People on Layer and below
    • read Groups (API to be extended)
    • read Events (API to come)
  • Events and courses can be filtered by year
  • UI matches Mockup more or less
  • Last accessed date works
  • The token value stays the same when changing permissions, description or name
  • A token can not be used anymore once it has been deleted
  • When deleting an existing token and then recreating it with the exact same settings, the token value is different
  • API should always return JSON, and return the same for parameters style and headers style calls
  • The events permission should be called "Events and courses", and in German "Anlässe und Kurse". (Note the term Anlass instead of Event)
  • API Documentation should be updated
  • All API endpoints work the way they are documented
  • Additional requirement / future work: CORS header Access-Control-Allow-Origin should be set on all status 200 API responses

Explanations of unmet criteria

❌ API should always return JSON, and return the same for parameters style and headers style calls

Under all circumstances, using parameters style (.json + token query param) and headers style (X-Token + Accept JSON) requests should give the same responses. However currently, for authenticated but unauthorized parameters style requests, the API returns HTML responses with a login form. The corresponding responses when using headers style requests are a simple JSON object { "email": null, "password": null }, which is not beautiful but okay if documented.

Steps to reproduce
GET https://pbs.puzzle.ch/groups/3
Accept: application/json
X-Token: soPFCnKNN5HNeLWAP59PYsRBsaExN_RSezyFrRR7jknxC1ofzQ

yields the response:

{
  "email": null,
  "password": null
}

while

GET https://pbs.puzzle.ch/groups/3.json?token=soPFCnKNN5HNeLWAP59PYsRBsaExN_RSezyFrRR7jknxC1ofzQ

yields the response:

<!DOCTYPE html> <html lang='de'> <head> <meta charset='utf-8'> <title>MiData PBS/MSdS/MSS - Anmelden</title> <meta...

❌ The events permission should be called "Events and courses", and in German "Anlässe und Kurse". (Note the term Anlass instead of Event)

Either this permission should be renamed or a separate permission type for courses should be created.

❌ API Documentation should be updated

The exact required parameters for querying the API with a service account token is not documented at https://github.com/hitobito/hitobito/blob/master/doc/development/05_rest_api.md yet.

Expected

Add something like the following to the documentation:

There are two possible styles for querying the API with a service account token.

Headers style:

GET https://[hitobito domain]/groups/1
X-Token: [service account token]
Accept: application/json

Parameters style:

GET https://[hitobito domain]/groups/1.json?token=[service account token]

Also document which permissions are necessary for which operations (unclear to me was only that the groups permission not only disables access to subgroups, but also to the group to which the token belongs).

All API endpoints work the way they are documented

The following is a list of all cases where the API does not work as expected, when using service account tokens to access it.

❌ Root group endpoint does not work

Accessing the root group endpoint /groups yields the same result as authorization failures regardless of token permissions and even if the token is defined on the root group. In case this is by design, remove the endpoint.

Steps to reproduce
GET https://pbs.puzzle.ch/groups.json?token=wfZPuvZBugipVHeBdWzsbsoLNcasc1xQ-zQKwhyHy5_jAy1hBg

❌ Course endpoint does not work

All requests to the /groups/{id}/events/course endpoint yield the same result as authorization failures regardless of token permissions and even if the token is defined on the root group.

Steps to reproduce

Accessing courses with a token with full permissions:

GET https://pbs.puzzle.ch/groups/1/events/course.json?token=jF4ozjEsSjk12dqFacx9yu4stmbzryy38QyM9PdRNeLfanJwSg

Caution when testing manually: Use an incognito window or make sure that your browser is not logged in to the hitobito instance you are testing. The API currently also reads any active session cookies.

❌ Additional requirement / future work: CORS header Access-Control-Allow-Origin should be set on all status 200 API responses

This is necessary so the API can be used from JavaScript applications in browsers.

Proposition / Expected

Each token could have a list of allowed accessing sources (protocol + (sub)domain + port) for CORS, and the CORS header Access-Control-Allow-Origin is set by the server.

@Michael-Schaer
Copy link

I can confirm all the problems you describe. I see the highest priority in the following points:

  • Most events are not returned in the API
  • Event permission is ignored completely
  • Course endpoint does not work

@RolandStuder
Copy link

Ok, I went through the list:

  1. API should always return JSON: Agreed, this is not very elegant.
  2. API works the same with parameters style (.json + token query param) and with headers style: Kind of a repetition of the first point
  3. The events permission should be called "Events and courses", and in German "Anlässe und Kurse": Well this is a semantic issue, as events, both are the a certain subscall and the main class. But yeah, the label can be easily changed
  4. Security: CORS header Access-Control-Allow-Origin: This is a new requirement. For the moment, it's is up to the consuming application to handle this. So the API has to be consumed server side.
  5. API Documentation should be updated: Thx for the suggestion, Pull Request are always welcome :-)
  6. Optional / future work: Events and courses can be filtered by year: Actually the API has a more flexible filter, as you can set a start_date / end_date to filter the coursies, the filter parameter to restrict just to the events of the selected layer is also available, see (https://github.com/hitobito/hitobito/blob/master/doc/development/05_rest_api.md#parameters)
    1.Root group endpoint does not work: ok, I think the root group access point should always be available, there is no sensitive Information in there anyway.
  7. Most events are not returned in the API: We chose a new default scope when introducing the API. We return events, which have not ended yet. So all current and future events show up, not the ones within the yearly scope. I see how this is confusing, also because it was not mentioned in the documentation. I think the scope makes sense, but probably we loose more with confusing people with the different behaviour than we win with the new default. What is your opinion on this?
  8. Event permission is ignored completely: Was not able to reproduce this fully. With your examples it indeed was like this, then I removed and granted permissions again. Now I can not reproduce the problem
  9. Course endpoint does not work: Indeed it doesn't, maybe an issue of the wagon overwriting the default behaviour.

Thanks for the detailed analysis.

@carlobeltrame
Copy link
Author

Thanks for having a look. I updated the list.

  • merged points 1. & 2. into one
  • about 5.: I can gladly make a PR to the documentation as soon as the state of the API has been more or less finalized.
  • about 6.: my bad, I completely overlooked this in the documentation. Just tested it and it looks good on first glance.
  • about the root group endpoint: I am not sure there will never be any sensitive information in the root group. I suggest you take extra care with e.g. the crisis feature and the crisis team members, which should not be publicly disclosed. And there may be other more or less confidential information in other tenants I don't know about.
  • about 8.: I think I mis-tested this. When doing it in a browser window that not is logged in in the GUI, it works as expected. I think the problem is more that the cookies of the browser should be ignored when using the JSON API. But this is not a big problem, for now just a thing to keep in mind when manually testing the API.
  • about 9.: The course endpoint actually works when I have a session cookie. So it might just be a permission issue.

@RolandStuder
Copy link

  • 9 to be fixed
  • 7 to be fixed: keep it more consistent

@carlobeltrame
Copy link
Author

I can confirm that points 9 and 7 are fixed now on pbs.puzzle.ch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment