glpi icon indicating copy to clipboard operation
glpi copied to clipboard

High-level API

Open cconard96 opened this issue 2 years ago • 9 comments

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -

The current API is very low-level and is directly reliant on the GLPI PHP classes (CommonDBTM classes). While this provides a lot of access, it requires the users to know a lot about the GLPI internals like class names. The documentation itself is also low-level and does not reveal the required internals information.

This API is intended to be more user friendly with stricter control over the endpoints available and their internal logic. While a lot of the endpoints can be dynamic to support multiple types of items, it does mean that there would be more to maintain if new types are added or if they are changed. It is intended for this to be closer to what the old Webservices plugin provided, while covering even more functionality.

The routing is done with a simple, custom router rather than a library. This was done to leave the decision open in case a library was chosen later for the regular web routes. The Symfony router component was used as inspiration but it could also be easily replaced with Slim.

Currently, the following endpoints are added and can be accessed via /api.php (varying levels of functionality):

  • /doc Displays the dynamically generated OpenAPI schema as JSON or as a Swagger UI depending on how it is requested. The UI is default, but if you use the Accept request header set to application/json or add '.json' to the path, you can get the raw schema.
  • A special default route which handles when you request a non-existing route. It also handles the OPTIONS HTTP method for all other routes.
  • /sesson Start or end a session. Supports basic HTTP authentication and user tokens. No app_token support is added yet. In the current API, the app_token was provided every request instead of just during authentication. Is that needed?
  • /Assets/{itemtype} List/search items of the specified type. Currently, this works with all CommonDBTM types but it is intended that this would be restricted to only assets.
  • /Assets/{itemtype}/{id} Get, update or delete a specific item.
  • /Assistance/{itemtype} List/search Tickets, Changes, or Problems.
  • /Assistance/{itemtype}/{id} Get, update or delete a specific Ticket, Change or Problem.
  • /Assistance/{itemtype}/{id}/Timeline Get all timeline items in order.
  • /Assistance/{itemtype}/{id}/Timeline/{subitem_type} Get all timeline items of a specific type (user friendly type names used here like Task, Followup, Document, Solution, Validation, and Log)
  • /Assistance/{itemtype}/{id}/Timeline/{subitem_type}/{subitem_id} Get, update or delete a specific timeline item
  • /status List all supported system status checker "services"
  • /status/{service} Get the status of a specific service. You can specify "all" as the service to get the full system status.
  • / Displays a welcome message, the API version(s) information, and a simplified list of the available routes (path, requirements, etc). If the user is not yet authenticated, only the public routes like /doc and /session are shown. This should help guide users in the right direction when they are new to the API.

A look at the documentation Swagger UI: Selection_095

You can test/use the API directly from this UI too which should be helpful.

cconard96 avatar Jul 17 '22 23:07 cconard96

Note, this work REQUIRES PHP 8 to work. This was done mainly to be able to use the PHP 8 attributes for a Route attribute rather than having to define routes in a separate class/function and to avoid having to use something like Doctrine Annotations.

cconard96 avatar Jul 17 '22 23:07 cconard96

Note, this work REQUIRES PHP 8 to work. This was done mainly to be able to use the PHP 8 attributes for a Route attribute rather than having to define routes in a separate class/function and to avoid having to use something like Doctrine Annotations.

PHP 7.4 is now only patched for security issues and this security support will end on november.

For now, most of GLPI 10.0 installations are done on PHP 7.4 image I guess it is related to the fact that it is the default PHP version on the used OS.

  • RHEL 8.3 (10/2020) : PHP 7.4
  • RHEL 8.4 (05/2021) : PHP 7.4
  • RHEL 8.5 (11/2021) : PHP 7.4
  • RHEL 8.6 (05/2022) : PHP 8.0
  • Debian Buster (07/2019) : PHP 7.3
  • Debian Bullseye (08/2021) : PHP 7.4
  • Ubuntu 20.04 (04/2020) : PHP 7.4
  • Ubuntu 22.04 (04/2022) : PHP 8.1

The only OS that will not propose PHP 8+ for again a few months is Debian. IMHO, it is not a problem, as administrators could rely on Ondřej Surý packages to get an up-to-date PHP version. They could also wait for Debian Bookworm release, which would probably be available soon after GLPI 10.1.

@orthagh @trasher Would you agree to drop PHP 7.4 support on GLPI 10.1 ?

cedric-anne avatar Jul 18 '22 10:07 cedric-anne

Would you agree to drop PHP 7.4 support on GLPI 10.1 ?

It should be ok as time will pass before releasing the new major version but we can be sure at 100%. People tends to not upgrade immediately their servers (not i agree with this).

Some comments btw:

I'm not sure they accept this kind of scenarios (but i don't know much companies using debian, most are on redhat env)

  • could we list what requires php 8 ? dependencies, current code ?

  • can symfony/polyfill get around the issue ?

orthagh avatar Jul 18 '22 11:07 orthagh

  • could we list what requires php 8 ? dependencies, current code ?
  • can symfony/polyfill get around the issue ?

The idea was to be able to use PHP 8 attributes. We could replace it by either:

  • a static mapping (good performances, but requires to be manually maintained, which can result in typos, missing declarations, ...),
  • usage of doctrine/annotations, or any similar lib (introduces new dependency and requires cache to have good performances).

In both case, we would probably want to rewrite this part in GLPI 10.2 to use PHP 8 attributes, so I would prefere usage of a static mapping, which can be declared as private and will not be subject to a BC-break when we will want to remove it.

cedric-anne avatar Jul 18 '22 11:07 cedric-anne

* could we list what requires php 8 ? dependencies, current code ?
* can symfony/polyfill get around the issue ?

The initial "requirement" was the Attributes. Other things from PHP 8 that were used since the targeted version changed are:

  • Short closures (one-line fn() => {})
  • match statement
  • Named arguments (To avoid a lot of useless Route attribute parameters when the defaults are wanted)
  • Class property promotion (Less code needed for setting properties within a constructor)
  • str_starts_with, str_ends_with, and str_contains which should already have polyfills

The Symfony/polyfill library adds an Attribute class, but I don't see any way that it patches the Reflection classes to be able to parse them. I think it is just to be able to use the constants TARGET_* and prevent errors from being thrown. The attribute declaration is just ignored by the interpreter.

No new PHP dependencies were added.

It wouldn't be possible to just add polyfills to provide compatibility for all of these features. If we have to continue supporting PHP 7.4, it could be changed.

  • Replace short closures with regular closures
  • Replace match statement with switch
  • Replace named arguments with regular ones, and specify the default values for any in-between parameters
  • Replace inline properties in the constructor
  • Replace Route attribute with a regular class and Doctrine Annotations

cconard96 avatar Jul 18 '22 11:07 cconard96

Even when using Attributes, there may be a point when caching the routes (not the attributes themselves) will be desired. Currently, performance is not bad, but as more routes get added, that may change.

I'd rather avoid statically defining routes if possible so I would prefer using the Doctrine library as the alternative. The doctrine cache may not be needed at all and if performance suffers, it would still be preferred to cache the compiled route paths themselves.

The Router currently gets all Route attributes from all controllers and creates an array of RoutePaths which have a "compiled path" which is the path with the arguments replaced with their requirement regex patterns and all the other information needed to route requests to the correct method.

cconard96 avatar Jul 18 '22 11:07 cconard96

  • Class property promotion (Less code needed for setting properties within a constructor)

Not sure using this one in GLPI is a good idea. All developpers are not using advanced PHP IDEs (or IDEs that are handling this correctly), and will probably not think to look at constructor everytime they will search for a class property.

cedric-anne avatar Aug 08 '22 13:08 cedric-anne

Hi! Thanks for your efforts and the excellent work! I wasn't able to grasp the practical differences between the old and the new Rest APIs here, but I'm just a very basic Api user. Trying to give a constructive feedback, as such a basic user I just wanted to highlight one thing which makes the use of the original Rest API difficult, with an example:

You GET a Computer: the result is a very good, complete JSON with all the nested details (IPs, OS,...). However, you aren't able to upload this JSON with the same format to update/insert an Asset. Some few nested parts work, some not, and you need to make several requests to add something that can be read with 1 request (as example, as always, the IPs ;-) ).

On the other side, with the method used by the GLPI Agent (also just a POST call with a JSON ;-) ) it is possible to upload a whole asset in one call (but there seem to be other limitations, e.g. additional fields). Seems to be completely different code for quite the same task (and both need to be maintained for every core change).

Best! Mirko

Mirkk avatar Aug 12 '22 08:08 Mirkk

I wasn't able to grasp the practical differences between the old and the new Rest APIs here, but I'm just a very basic Api user.

A good and simple example of a benefit of this API design over the existing one is how linked/related items are handled. For example, when creating a Ticket followup in the old API, you have to use '/ITILFollowup' and manually specify the itemtype of "Ticket" and items_id of the Ticket's ID in the input. In this API, you would use '/Ticket/10/Followup` where '10' is the Ticket ID and only need to specify properties like the content.

Another example is Ticket actors. In the current API, you have to use '/Ticket_User', '/Group_Ticket', '/Supplier_Ticket' endpoints depending on the actor type you want to add and manually specify the tickets_id in the input. How it works in the new API (as it is currently but may change before release), it is just '/Ticket/10/TeamMember' where you specify the role (assigned, requester, observer), type (user, group, supplier), and ID (user, group, supplier ID). You do not need to know the internal class names used to represent the relationships like 'Ticket_User'.

cconard96 avatar Aug 16 '22 23:08 cconard96

To use Swagger UI for API testing:

  1. Navigate to /api.php/doc relative path on your GLPI
  2. Click Authorize at the top of the page, to the right of the servers dropdown
  3. Enter either your GLPI username and password or your user token and click "Authorize"
  4. Close the authorization popup
  5. Click the Session group and expand the POST /Session endpoint
  6. Click "Try it out" and then "Execute"
  7. Copy the session_token
  8. Click the Authorize button at the top of the page again and paste the session_token into the related field and click "Authorize"
  9. All endpoints should now use the session_token you entered and be showing a locked lock icon (endpoint requires authentication and you are authenticated) or no lock icon at all (no authentication needed for this endpoint).

cconard96 avatar Oct 20 '22 21:10 cconard96

I'm not sure why the lint test is failing. Even adding "getItemTypeForTable" to the require checker config doesn't fix it and that function is used elsewhere without issue.

cconard96 avatar Oct 27 '22 11:10 cconard96

I'm not sure why the lint test is failing. Even adding "getItemTypeForTable" to the require checker config doesn't fix it and that function is used elsewhere without issue.

You use a different case.

cedric-anne avatar Oct 27 '22 13:10 cedric-anne

This PR is an initial implementation only as it is reaching 10,000 lines. Most of the backend should be done now, so future PRs can focus on the implementation of new endpoints and expanding the response schemas (to include more relations/links for example).

cconard96 avatar Nov 06 '22 20:11 cconard96

i checked out again today this pr, and i got

Warning: Array to string conversion in /var/www/glpi/src/Http/Response.php on line 97

content of $headers

Array
  (
      [Content-Type] => Array
          (
              [0] => text/html
          )
  
      [Cache-Control] => Array
          (
              [0] => public, max-age=86400
          )
  
  )

orthagh avatar Nov 08 '22 13:11 orthagh

To return on the discussion about the custom header dedicated to passing session token. Apart the oauth mechanism we already discussed (and what i think is the good way but we can keep a legacy way with a custom header), i think we should avoid any exotic syntax.

So i suggest the key Glpi-Session-Token instead of session_token. This to:

  • avoid any issue with underscores
  • keep a predictable syntax similar to other http headers
  • identify with a prefix this is a custom header provided by GLPI.

orthagh avatar Nov 08 '22 14:11 orthagh

Some last comments and questions, i guess we're almost good:

  • Is the new base url with api.php is required ? we had previously apirest.php. suggestion to add something like version parsing in url, to get these scenarios

    • apirest.php (without version) -> legacy api
    • apirest.php/v1/ -> legacy api
    • apirest.php/v2/ -> new api I'll not be picky about this, but i found we may be do better here.
  • TeamMember wording, did you choose this term to avoid something ? I prefer to stick to "Actor" term to have something predictable with core and UI terms

  • While testing a little more, i saw there's no parameters hinting for most endpoints apart generic ones (like itemtype for posting itilobjects). Is it possible for "fixed" endpoints (like Entity/Users/etc) to have parameters documentation ? I checked this part after reading again your statement above regarding name field. and so nowhere it's indicated you must use username .

  • Not sure you can do something, but i missed my glpi was setup with a ipv6 url (http://[::1]/glpi/). Trying to use the api ui with ipv4 url displays correctly the interface but "try" buttons fails with 500 errors. I saw in my browser console there was a cors issue. Anyway, this is something we must absolutely check, CORS is mandatory for an API.

orthagh avatar Nov 10 '22 13:11 orthagh

* Is the new base url with api.php is required ? we had previously apirest.php.
  suggestion to add something like version parsing in url, to get these scenarios
  
  * apirest.php (without version) -> legacy api
  * apirest.php/v1/ -> legacy api
  * apirest.php/v2/ -> new api
    I'll not be picky about this, but i found we may be do better here.

Not required, but apirest.php naming was used in the past I'm guessing just because there was also a XML-RPC API that had to have a different endpoint. Having just api.php would be simpler, and match what you would see in many other REST APIs especially if the web server is configured so the php extension isn't needed.

* TeamMember wording, did you choose this term to avoid something ? I prefer to stick to "Actor" term to have something predictable with core and UI terms

It was chosen because it matches the name of the existing code interface for managing actors/team members for ITIL Objects, Projects, and Project Tasks. In some cases, the teams members are referred to as "actors" but in other cases they could be "managers" (Projects) or something else. It also just sounded more natural to talk about everyone involved with the ITIL Object, Project or Project Task as a Team and then the individuals in the team as Team Members. The endpoint name can be changed or aliased, and it doesn't matter to me that much either way.

I am looking into the last two points.

cconard96 avatar Nov 10 '22 14:11 cconard96

and match what you would see in many other REST APIs

That's a good point.

the version management may be still a good addition for accessing legacy api. The default becomes (without specified version) V2. And we may think about deprecating old file (apirest.php)

it doesn't matter to me that much either way.

Same, let other give their opinion on this

orthagh avatar Nov 10 '22 14:11 orthagh

Hi, If you allow us, I'd like to ask for the legacy mode approach for GLPI 10.1. API Legacy mode will give us the time to migrate our integrations. Otherwise, a breaking change on 10.1 will be very bad news, at least. Thanks.

OscarBeiro avatar Nov 10 '22 15:11 OscarBeiro

Hi, If you allow us, I'd like to ask for the legacy mode approach for GLPI 10.1. API Legacy mode will give us the time to migrate our integrations. Otherwise, a breaking change on 10.1 will be very bad news, at least. Thanks.

There are no plans to drop the current API, especially when it offers so much more access. This new API would be the "preferred" one for users since it would be more strict in what endpoints are available, the strict auto-documenting schemas, and not needing to know search options or class names, but they can always use the current API for more advanced needs.

I think if apirest.php is kept as-is it will prevent any breaking changes with API URIs. The new api.php file can handle routing legacy API requests if it sees its URI start with /v1/.

cconard96 avatar Nov 10 '22 15:11 cconard96

Thanks Curtis, Changing the Endpoint URL is not a big issue. Maybe I misunderstood some comments.

OscarBeiro avatar Nov 10 '22 15:11 OscarBeiro

yes my point was to deprecate the url /apirest.php to prefer api.php (and avoid two base url) The legacy api should be kept as it offers more low level control

orthagh avatar Nov 10 '22 15:11 orthagh

I tried also to debug a little the CORS issue. Current state is not sufficient.

somethin i found is when i try to authenticate the api from a different url than the one configured (like 127.0.0.1 vs localhost), the browser calls an OPTIONS request on the /session endpoint (where this verb is not available) but not when i call from the good url.

OPTIONS requests are all handled by a single route \Glpi\Api\HL\Controller\CoreController::defaultRoute and at least weren't intended to be supported on individual routes since the specific methods allowed could be determined globally by looking at the compiled route information (Rather than saying all methods are allowed when only some would be supported).

The Allow Origin response header should be added using the Middleware\SecurityResponseMiddleware.php. As with the legacy API, the header is only added if $_SERVER['HTTP_ORIGIN'] is set.

cconard96 avatar Nov 14 '22 11:11 cconard96

CORS was not working because the preflight requests were going to a route that was set to only allow authenticated requests and preflights don't send the required headers for that. The server was responding with a 401 response. There was also an issue with handling response middleware in general. It seems to be working now.

cconard96 avatar Nov 15 '22 11:11 cconard96

ok good job for CORS, i confirm it's ok now.

Some last comments:

  • is it possible to paginate response ? (maybe a parameter should be present)
  • same for ordering ?
  • you talk initially about RSQL filtering, this is not illustrated in swagger ?
  • example response explains the ok return code should be 200, in fact, when trying, it's 206 (and annotated as "Undocumented")
  • i didn't succeed to get the Asset/{itemtype}[/{id}] endpoints working. I tested with Computer and It fails with an error : "Value must follow pattern List"
  • Not really urgent (and can be addressed in another pr), but we miss a lot of dropdown foreign keys. The first i can think is Category for tickets (i can see status and entity informations, but that's all). I think everybody will asks about their dropdows very quickly
  • Html content are sent encoded with html entities. Until we remove all stuff related to auto escaping, i think we should auto decode before return the content.
  • GET TeamMember response doesn't include id of the member
  • GET /Administration/Group doesn't include entity information

A big one, and i'm sorry to not see it before. But the initial goal was to have a persistent API with non changing parameters. After exploring a little more, it appears that for example in timeline endpoint, it's a copy of database field ? Can you confirm this ? Because, we may have the same issue as the old database, any change in our db model will impact the results of API.

orthagh avatar Nov 15 '22 13:11 orthagh

Some last comments:

* is it possible to paginate response ? (maybe a parameter should be present)

Done by default with the list_limit preference. The API should also handle start and limit query parameters well. Responses already return a 206 status if the returned result is paginated and the response also has the Content-Ranger header set to inform you the range returned and total count.

* same for ordering ?

Indeed, it looks like I missed implementing that.

* you talk initially about RSQL filtering, this is not illustrated in swagger ?

Indeed, there may need to be a generic documentation page accompanying the Swagger documentation. I don't know a good way to integrate such documentation with this.

A quick summary

The RSQL filtering system supports the following operators:

  • == - Equivalent to
  • != - Not equivalent to
  • =in= - In
  • =out= - Not in
  • =lt= - Less than
  • =le= - Less than or equal to
  • =gt= - Greater than
  • =ge= - Greater than or equal to
  • =like= - Like
  • =ilike= - Case-insensitive like
  • =isnull= - Is null
  • =notnull= - Is not null
  • =empty= - Is null or empty string
  • =notempty= - Is not null or empty string

Filters are provided using the filter parameter on any of the search endpoints in the form of field + operator + value. Example: ?filter[]=name==test&filter[]=entity.id=in=(0,1) This filter means name equals "test" and the entity ID is either "0" or "1".

The accepted fields are exactly as shown in the response. Sub-items in the response are separated by a period.

* example response explains the ok return code should be 200, in fact, when trying, it's 206 (and annotated as "Undocumented")

206 response means the response is partial (paginated). Indeed, that isn't a documented response currently.

* i didn't succeed to get the Asset/{itemtype}[/{id}] endpoints working. I tested with `Computer` and It fails with an error : "Value must follow pattern List"

/Assets/Computer/10 for Computer with ID 10

* Not really urgent (and can be addressed in another pr), but we miss a lot of dropdown foreign keys. The first i can think is Category for tickets (i can see status and entity informations, but that's all). I think everybody will asks about their dropdows very quickly
* Html content are sent encoded with html entities. Until we remove all stuff related to auto escaping, i think we should auto decode before return the content.

* GET TeamMember response doesn't include id of the member

* GET /Administration/Group doesn't include entity information

Most of the relations are missing. The main focus of this PR was the underlying routing, search and documentation generation systems so future PRs can focus on creating and expanding the endpoints.

A big one, and i'm sorry to not see it before. But the initial goal was to have a persistent API with non changing parameters. After exploring a little more, it appears that for example in timeline endpoint, it's a copy of database field ? Can you confirm this ? Because, we may have the same issue as the old database, any change in our db model will impact the results of API.

The schemas are not directly tied to the database. The fields in the schemas do default to database columns of the same name, but can be decoupled (and are already in some cases) by specifying x-field in the schema properties. Schema mapping also does not use search options at all. Some work may need done if dropped DB fields should still show in responses, but be empty, but renamed fields can be re-mapped and new fields won't show unless the schema in the API controllers is updated.

cconard96 avatar Nov 15 '22 13:11 cconard96

ITIL timeline IS currently relying on CommonITILObject::getTimelineItems. That could be reworked to use a specific schema like the other endpoints.

cconard96 avatar Nov 15 '22 13:11 cconard96

IMHO, object schemas should be moved in Facade object, to make them more easier to find and to have less declarative code in controllers.

For instance, a facade could look like this:

namespace Glpi\Api\HL\Facades;

#[Facade(itemtype: \User::class)]
final class UserFacade
{
    #[Property(
        type: Doc\Schema::TYPE_INTEGER,
        format: Doc\Schema::FORMAT_INTEGER_INT64,
        description: 'ID',
        x-readonly: true
    )]
    public int $id;
}

cedric-anne avatar Nov 30 '22 15:11 cedric-anne

I've not really thought about how plugins should be allowed to expand schemas yet, but I don't know how it could be done easily when using a facade with properties.

With the tags plugins, I imagine it would add a new schema property tags which maps to an array of tags related to the asset. With using an array, they have full control. With a class, PHP doesn't let you add properties dynamically beyond the magic methods and even then, there is no way to add the attribute information.

cconard96 avatar Dec 02 '22 09:12 cconard96

Hi everyone. Thank you for the FR. It will make life a lot easier for a n00b like me. I am familiar (sort of) with the way presented above, but I am having REAL difficulty interacting with the old API. Question: I know this is still early days, but how do I get my hands on this feature in its current state? I currently have 10.0.5 stable installed on my system and therefore only get a 404 when trying to access http://myinstance.com/glpi/api.php/doc. But this is only a dev machine to develop integration features. And I make daily backups, so stability is not AS important. Also, is this perhaps a feature for the 10.1 milestone? Thank you again.

hanserasmus avatar Jan 23 '23 18:01 hanserasmus