How should plugins create API endpoints

[NateWr]

Summary

There should be a built-in way for plugins to create custom API endpoints and route them to a custom APIHandler which ensures that all API middleware is applied and works seamlessly with the useFetch composable.

Proposal requirements:

• Create new API endpoint from plugin (such as /plugins/generic/my-plugin)
• Route an API endpoint to a custom API handler that applies all middleware and automatically checks the CSRF token on non-GET requests.
• Document a working example in the plugin guide and differentiate when to use APIHandler or PageHandler

Problem

At the moment, I believe plugins can only create new endpoints off of existing endpoints by hooking into APIHandler::endpoints::<entity>. However, sometimes a totally new API endpoint is more sensible. The only way to do this that is currently documented is to use the LoadHandler hook to pass the request to a PageHandler.

This works for GET requests , which can use authorization policies to restrict access and return JSON. However, if a PUT, POST, or DELETE request is handled, the CSRF token must also be checked. A PageHandler expects to find the CSRF token in the request body, but API requests sent by the useFetch composable put the CSRF token into a header at X-Csrf-Token.

More importantly, a PageHandler doesn't automatically check for a CSRF token on PUT, POST, or DELETE requests. It would be easy for a plugin developer to forget to apply this security measure.

Current Alternatives

Plugins can use the following to workaround this problem:

• The CustomQuestions plugin uses the Dispatcher::dispatch hook to create new API endpoints.
• For many plugins, it will make sense to extend off of an existing endpoint. See example.
• Plugins can check the CSRF token manually from a custom PageHandler. See example.
• Plugins can pass the CSRF token in the request body so that a PageHandler will find it. Example: const {data} = useFetch(url, {method: 'POST', body: {...data, csrfToken}}).
• Using an API token will work when the plugin does not need to be deeply integrated within the OJS system. See example.

[NateWr]

Just added this alternative from the CustomQuestions plugin, which uses the Dispatcher::dispatch hook.

Should this be the recommended approach for plugins?

[ewhanson]

I agree this looks like the easiest and most flexible approach for plugins right now. Eventually, it might be nice to have a dedicated hook for specifically adding in API controllers, but that would only be applicable moving forward. @NateWr, have you tried this out? Have you come across any cons to this approach?

[touhidurabir]

@NateWr @ewhanson I can confirm the above mentioned approach does works pretty well with slight modification for 3.5/main branch , see at touhidurabir/apiExample#1 .

Only downside is that this require some good amount of code to setup but this also serve the intended purpose without us making any changes to the core .

[dxL1nus]

We are developing a plugin called ojs-codecheck for OJS 3.5.x and have the same problem.

We need to create our own custom API endpoints, as extending of the existing endpoint doesn't fit our use case.
Ideally we would like to have a general API route for our plugin with multiple sub-routes. In the moment we use the workaround with a LoadHandler that then echos different JSON data, depending on the page and sub-page which was requested.

We also asked about this in the PKP Forum and got the confirmation, that we can keep using the LoadHandler approach for now, but we would still prefer to extend the already existing API of OJS. So we are a fan of the proposed changes by @NateWr.

Kind regards

[jardakotesovec]

Also very interested how this discussions evolves. I was just looking into updating our PluginTemplate to use Vue.js for managing settings, and end up here, because was not sure how to cleanly add API endpoint(s) to the plugin.

[touhidurabir]

A simpler approach would be to introduce a new hook for plugin to inject custom routes which has no association with any existing entity routes .

one such example implementation at main...touhidurabir:pkp-lib:d11991_main and from plugin we can simply use as

Hook::add('APIHandler::endpoints::plugin', function (string $hookName, APIRouter $apiRouter): bool {
    $apiRouter->registerPluginApiControllers([
        // Allow to have a custom API endpoint as 
        // BASE_URL/index.php/CONTEXT_PATH/api/v1/custom-plugin-path/
        new CustomApiController,

        // Allow to have a custom ADMIN API endpoint as 
        // BASE_URL/index.php/index/api/v1/custom-admin-plugin-path/
        new CustomAdminApiController,
    ]);

    return Hook::CONTINUE;
});

it uses a strict API controller's handler path uniqueness check so there should be no collision of plugins having same type of path and remove the possibility of action/data leaking of different plugins with same api route path which is possible with the use of Dispatcher::dispatch hook . Also plugin developer will have less checks to consider .

However this require some modification to the core .

[ewhanson]

Thanks @touhidurabir, I think this would be a good approach moving forward with falling back on the other approach for older versions of OJS. @asmecher, would it be worthwhile putting this into 3.5 or should this go into only main instead?

[asmecher]

@ewhanson, I support this going into stable-3_5_0 (as well as main).

[ewhanson]

@touhidurabir, could you take the idea you outlined above and create an issue to add this into stable-3_5_0 and main? Thanks!

[touhidurabir]

We moved forward with issue #12050 and POC PR . Any suggestions are highly appreciated .

[jardakotesovec]

@touhidurabir @ewhanson And for extending existing endpoint this approach is still recommended approach?

jardakotesovec/backendUiExample#4 (comment)

[touhidurabir]

extending existing endpoint this approach is still recommended approach?

@jardakotesovec I think it depend based on the requirements .

1. if needed to extend any entity based routes, better to use like hook APIHandler::endpoints::<ENTITY>
2. if need something more generic/global, better to use the APIHandler::endpoints::plugin for any kind of route paths .

But the opposite is also possible and we are not going to restrict . Also using this new hook APIHandler::endpoints::plugin it is also possible to completely replace a core existing route also. for example, we can totally replace the entire submissions routes as plugin routes will run first if there is a matching . The same can be done using the Dispatch::dispatch .

[touhidurabir]

Proposed Solution

See the issue Custom API endpoint for plugins and related PRs in the issue description . will be available from 3.5.0-4 and main(upcoming 3.6) .

Use Hook

use the hook APIHandler::endpoints::plugin to inject custom API controller as

Hook::add('APIHandler::endpoints::plugin', function (string $hookName, APIRouter $apiRouter): bool {
    $apiRouter->registerPluginApiControllers([
        // Allow to have a custom API endpoint as 
        // BASE_URL/index.php/CONTEXT_PATH/api/v1/custom-plugin-path/
        new CustomApiController,
    ]);

    return Hook::CONTINUE;
});

The APIHandler::registerPluginApiControllers method will take an array where each element must be an instance of PKPBaseController . Basically register a list of custom API controllers .

When to use (Recommended)

1. plugin require very custom API endpoint
2. Not related to any existing entity.
3. If need to append an API routes to any existing entity, should use APIHandler::endpoints::<ENTITY> hook

Restrictions

1. Core API routes still has higher priority. So for example, if a plugin's custom API controller define the return valuegetHandlerPath method as submissions which does match with existing PKPSubmissionController and as a result, we have match of existing API route paths, it will be executed instead of plugins one .
2. Custom API routes will be executed(e.g. the capability will be transferred over to plugins API controller) only when there is no match of current core routing paths .
3. We have introduced a mechanism to detect path collision and throw exception on that . So for example, if 2 custom API controller has same base handler path defined in controller's getHandlerPath, it will throw an exception at the time to custom API controller registration. This is done to prevent the accidental path collision and data leaking of 2 different plugin with similar path as in those case, we have no way to differentiate and the first match one will be executed which may lead to undesired result .

Alternative approach :

use the Dispatcher::dispatch hook to intervene the process of APIRouter execution process as follow

// Allow to have a custom API endpoint as 
// BASE_URL/index.php/CONTEXT_PATH/api/v1/custom-plugin-path/
Hook::add('Dispatcher::dispatch', function (string $hookName, array $args): bool {
    $request = $args[0]; /** @var PKPRequest $request */
    $router = $request->getRouter();

    if (!$router instanceof APIRouter) {
        return Hook::CONTINUE;
    }

    $requestPath = $request->getRequestPath();
    
    if (!str_contains($requestPath, 'custom-plugin-path')) {
        return Hook::CONTINUE;
    }

    $controller = new CustomApiController;
    $handler = new APIHandler($controller);

    // we can get all the registered routes at this point as if need to run any more extra checks
    // app('router')->getRoutes();

    $router->setHandler($handler);
    $handler->runRoutes();

    return Hook::ABORT;
});

Through the usage of Dispatcher::dispatch hook (as above), it's possible to overcome the restrictions of APIHandler::endpoints::plugin hook . However it's an intervention at very low level of request processing, plugin developer need to be very careful of any core router path collision and security measure .

Possible points to reconsider

It's possible to alter the current implementation very slightly to remove those Restrictions with follow impacts

1. Plugins will get higher priority to run API routes before the core API routes . This open the possibility of accidental override of the core API routes completely and break core behaviour . However it's still possible with the use of Dispatcher::dispatch .
2. Extra processing overhead will be introduced as for every single API request(core or plugin level) , plugins API routes will be checked first . So if there is say, 100 custom API Controller registered, it will check for all those 100 and add a bit overhead to the API route processing mechanism . This is a bit performance overhead introduced here .

[jonasraoni]

We should use events instead of hooks, the only extra benefit of a hook is defining that it should be executed before others, but this isn't a great way to solve the problem (e.g. N hooks attempting to be the first to be executed leads to undefined behavior)

1. The user should be able to override anything, if we don't provide it here, workarounds will be added anyway, which then turns this discussion pointless (we're trying to come up with a way to avoid workarounds)

2. Any performance overhead is likely not important, it's unlikely we'll have many custom API controllers, and even if we had, our real bottleneck are currently network bottlenecks (e.g. database requests).

[touhidurabir]

The user should be able to override anything, if we don't provide it here, workarounds will be added anyway

Thats true . My concern was the accidental over write of core APIs and prevent it but using the Dispatcher::dispatch hook, that is still possible . So better to allow via this new APIHandler::endpoints::plugin hook also and properly document it . Adding/Overriding API means plugin developer already intervening into the core at very low level and they should understand the risk of mistake or untested behaviour implementation .

The performance overhead is very low and we can just ignore it for now .

[NateWr]

This looks great, thanks @touhidurabir! I think it is fine to have the "easy" custom API approach executed at a lower priority than core API controllers, as long as both options are documented.

I don't have a strong opinion on Events vs Hooks, but it may be easier to generate automated documentation from Event classes.

[touhidurabir]

Removed the following restrictions :

1. Core API routes still has higher priority. So for example, if a plugin's custom API controller define the return value getHandlerPath method as submissions which does match with existing PKPSubmissionController and as a result, we have match of existing API route paths, it will be executed instead of plugins one .
2. Custom API routes will be executed(e.g. the capability will be transferred over to plugins API controller) only when there is no match of current core routing paths .

This means plugin custom API routes will have higher priority . So it will be up to the plugin developer to be careful about the core api routes override .

[jonasraoni]

Regarding hook vs events:

• I'm normally against anything that introduces hardcoded "strings", this could be fixed with a constant holding the hook name, but events provide a namespaced identifier naturally.
• But the most useful is the typing:
  • Implementing a hook will require me to watch the code which calls it, with an event, everything is declared in the class
  • The hooks are more error prone
    • A piece of code can call the hook with X arguments, while another can call with Y arguments, this will not trigger any error...
    • A hook can be removed/renamed or have its arguments changed and implementors won't notice (something to partially address this problem was recently added to the codebase).
  • The event class can also optionally include useful methods to deal with its data

[touhidurabir]

PR has been merged in 3.5/main branch . So this is already available for main branch and will be available from 3.5.0-4 for 3.5 release .

[touhidurabir]

Still need the update of dev doc to point out these details . This one along with combination of #9434 and #11631