New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0] Web Services #23424

Open
wants to merge 1 commit into
base: 4.0-dev
from

Conversation

@wilsonge
Copy link
Contributor

wilsonge commented Jan 1, 2019

This is a base for a web services implementation for Joomla 4

What this has got in

Core

  • Content Negotiation (core provides a basic api integration using the https://github.com/tobscure/json-api library)
  • Plugin based routes to allow custom endpoints to be added
  • Authentication on endpoints using users basic auth (through plugins to allow future oAuth integrations)
  • Exceptions added to allow quality error handling

Integrations/Testing

  • Sample com_content integration
  • Basic system tests

What does this not have in

  • oAuth authentication
  • Ability to turn off web services (this will probably need to happen before this ships in J4)
  • Some sort of fundamental change to our error handling (the way we set up errors in JTable and JModel mean we'll likely never be able to have nice error handling with the current model system. In many places we're likely going to need to add 500's just because we won't be able to detect the right exceptions. In the future we should consider a new mvc layer based on the new entity layer https://github.com/joomla-framework/entities but this will take longer and be much harder to implement. The fundamental base application would not be expected to change but extension MVC's will have to to have a nice working integration.

Thanks

  • @mbabker for large amounts of consulting and framework contributions especially around the router as well as for the code for api part of the ComponentInstaller Adapter
  • @muhakh and @cokencorn for their work on their GSOC project which formed a large amount of the base application
  • @isacandrei for work bug testing and working on various next gen parts

Before moaning about a lack of feature X

Please read the specification https://joomla-projects.github.io/gsoc18_webservices/ - this has long been agreed on and is my no means perfect. Think about if it's absolutely essential for a day 1 integration or not. What we have in this PR is already significantly more than wordpress has and is probably equal to what's available in the other top level CMS'

@dgrammatiko

This comment has been minimized.

Copy link
Contributor

dgrammatiko commented Jan 1, 2019

Merge it!

defined('JPATH_PLATFORM') or die;
/**
* Exception class defining a authentication failed event

This comment has been minimized.

@brianteeman

brianteeman Jan 1, 2019

Contributor

==> an not a

This comment has been minimized.

@wilsonge

wilsonge Jan 1, 2019

Author Contributor

Thanks :) Fixed

/**
* Exception class defining a authentication failed event
*
* @since 3.6.3

This comment has been minimized.

@brianteeman

brianteeman Jan 1, 2019

Contributor

3.6.3 ??

This comment has been minimized.

@wilsonge

wilsonge Jan 1, 2019

Author Contributor

Thanks :) Fixed

@brianteeman

This comment has been minimized.

Copy link
Contributor

brianteeman commented Jan 1, 2019

Sorry to be pedantic but the correct term according to the w3c it is "Web Services" not "webservices"

As this is new to Joomla its best to get it right from the beginning

@wilsonge wilsonge force-pushed the wilsonge:webservices-version2 branch from e3a131f to 9cbc242 Jan 1, 2019

@wilsonge wilsonge changed the title [4.0] Webservices [4.0] Web Services Jan 1, 2019

@wilsonge wilsonge force-pushed the wilsonge:webservices-version2 branch from 9cbc242 to 302291d Jan 1, 2019

@wilsonge

This comment has been minimized.

Copy link
Contributor Author

wilsonge commented Jan 1, 2019

@brianteeman I've amended the language strings and the doc blocks where we refer to web services as a noun. I've left the plugin type as it is - because obviously it can't have spaces and i've found from practical experience with editors-xtd that most people struggle with the concept of removing the - from the plugin class names - so it's more practical to keep the group as a single word I think. Happy to be challenged on that if that's the majority tho

@wilsonge wilsonge force-pushed the wilsonge:webservices-version2 branch from 302291d to 4a44683 Jan 1, 2019

Show resolved Hide resolved api/index.php Outdated
Show resolved Hide resolved api/includes/framework.php Outdated
Show resolved Hide resolved api/includes/framework.php Outdated
@mbabker

This comment has been minimized.

Copy link
Member

mbabker commented Jan 2, 2019

App needs to be registered in Joomla\CMS\Application\ApplicationHelper::getClientInfo()

(Side note, this method should be redone to use the event system instead of hardcoding a list...)

* @var string
* @since __DEPLOY_VERSION__
*/
protected $fieldsToRender = ['id', 'typeAlias', 'asset_id', 'title', 'introtext', 'state', 'catid', 'created'];

This comment has been minimized.

@bembelimen

bembelimen Jan 5, 2019

Contributor

Don't trust state :)

This comment has been minimized.

@wilsonge

wilsonge Jan 5, 2019

Author Contributor

Why?

@wilsonge wilsonge force-pushed the wilsonge:webservices-version2 branch 5 times, most recently from c493b01 to 33e9957 Jan 5, 2019

@Hackwar

This comment has been minimized.

Copy link
Member

Hackwar commented Jan 7, 2019

I like this, but we would indeed need at least a way to switch it off. Maybe simply adding a switch in configuration.php?

@wilsonge wilsonge force-pushed the wilsonge:webservices-version2 branch from 33e9957 to 58e6c44 Jan 7, 2019

@wilsonge

This comment has been minimized.

Copy link
Contributor Author

wilsonge commented Jan 7, 2019

@Hackwar addressed all your comments. So I deliberately didn't put in the switch yet because I want it in a separate PR. Reason being - there is an argument that inter-component webservices should always be available. The switch should be a limit for the public facing webservices (I honestly can see this both ways). As a result I want to deliberately NOT have that discussion in this PR and leave it for later and instead introduce the base application here and let that discussion be done in a separate follow up PR (which I commit to doing in order to allow that conversation to take place)

@wilsonge wilsonge force-pushed the wilsonge:webservices-version2 branch from 58e6c44 to 7d6b0a5 Jan 8, 2019

@manojLondhe

This comment has been minimized.

Copy link
Contributor

manojLondhe commented Jan 17, 2019

I tried testing this

This worked

  • Get articles list
    • GET {{host}}/api/index.php/article

This did not work

  • Get single article

    • GET {{host}}/api/index.php/article/1
    • {"errors":{"code":500,"title":"Internal server error","detail":"Array"}}
  • Create article

    • POST {{host}}/api/index.php/article
    • {"errors":[{"code":500,"title":"Internal server error","detail":"RuntimeException: Table articles not supported. File not found. in /libraries/src/MVC/Controller/ApiController.php:295\nStack trace:\n#0 /libraries/src/MVC/Controller/BaseController.php(735): Joomla\CMS\MVC\Controller\ApiController->add()\n#1 /libraries/src/Dispatcher/ComponentDispatcher.php(146): Joomla\CMS\MVC\Controller\BaseController->execute('add')\n#2 /components/com_content/Dispatcher/Dispatcher.php(57): Joomla\CMS\Dispatcher\ComponentDispatcher->dispatch()\n#3 /libraries/src/Component/ComponentHelper.php(382): Joomla\Component\Content\Site\Dispatcher\Dispatcher->dispatch()\n#4 /libraries/src/Application/ApiApplication.php(331): Joomla\CMS\Component\ComponentHelper::renderComponent('com_content')\n#5 /libraries/src/Application/ApiApplication.php(108): Joomla\CMS\Application\ApiApplication->dispatch()\n#6 /libraries/src/Application/CMSApplication.php(240): Joomla\CMS\Application\ApiApplication->doExecute()\n#7 /api/includes/app.php(54): Joomla\CMS\Application\CMSApplication->execute()\n#8 /api/index.php(35): require_once('/home/manoj/GIT...')\n#9 {main}"}]}
  • Update article

    • POST {{host}}/api/index.php/article/1
    • {"errors":[{"title":"Resource not found","code":404}]}
  • Delete article

    • DELETE {{host}}/api/index.php/article/1
    • {"errors":{"code":500,"title":"Internal server error","detail":"Array"}}

Here is a postman collection I created and used
(import below url in postman, create postman env, set up host, username, password and use this)
https://www.getpostman.com/collections/cf166ea0211f65ff23dc

@dgrammatiko

This comment has been minimized.

Copy link
Contributor

dgrammatiko commented Jan 17, 2019

@wilsonge quick question: does web services require the joomla instance to be in the root folder or will it work on a subfolder?

@mbabker

This comment has been minimized.

Copy link
Member

mbabker commented Jan 17, 2019

The API app should behave the same as the admin app as far as folder structure goes (since the requests are going to /api/index.php and not through the root /index.php).

@sanderpotjer

This comment has been minimized.

Copy link
Member

sanderpotjer commented Jan 24, 2019

First off all: thank you @wilsonge and all others worked on this Web Services PR, great work!

We have been looking into this PR this afternoon. In the past we have developed a custom REST API solution to serve Joomla articles for our customers. So as a test-case we are now doing this again, but by using this new core feature (and possibly a custom web services plugin).

A couple comments so far:

1. Documentation
You asked us to read https://joomla-projects.github.io/gsoc18_webservices/, which kind of pushed us into the incorrect direction for testing this PR ;-) The example URLs on https://joomla-projects.github.io/gsoc18_webservices/?specification/chapters/urls-and-routing.md are no longer valid. It might be good to update that page, or have some up to date documentation for people that want to test this PR. And mention that the authentication works via the Joomla user accounts.

2. Requests not working
We confirm the "not working" items mentioned by @manojLondhe

3. SEF support
The GET articles list is working, but it seems that as soon as we turn "Use URL Rewriting" on, or completely disable the "Search Engine Friendly URLs" the API will return a "Resource not found" error response.

4. ACL
Currently, the only authentication to access the API is done by checking the password of a user. We understand that additional API authentication plugins can be created, but would it not be good to add an ACL check as well? We do see a reference to an action "core.login.api" in https://github.com/joomla/joomla-cms/pull/23424/files#diff-45ae31282d7eb57ef5d8dc26df256d9fR286 but that is not implemented further in the web services / Joomla permission interface. We do think it would be a good idea to add that "core.login.api" action and check for the permission. In this way you don't grant anyone with an account on the website access to the API by default. There are situations that only a specific "API users" should be able to access the API. The "core.login.api" is also in line with the offline, admin and site login actions. As a bonus: this might be the solution as well to "turn off web services" by simply not allowing this action for the Public user group.

These are some first comments based on our first impressions. Its great to see Web Services are on the horizon of Joomla 4 and will try to test/contribute as much as possible to make this happen for Joomla 4 :)

@wilsonge wilsonge force-pushed the wilsonge:webservices-version2 branch 2 times, most recently from 3029b4a to 9a9f7cc Jan 24, 2019

@wilsonge

This comment has been minimized.

Copy link
Contributor Author

wilsonge commented Jan 24, 2019

First of all thanks so much for the detailed feedback and testing!

Documentation

Sorry forgot to update that page. This had the 'correct' URLs on https://github.com/joomla-projects/gsoc18_webservices/blob/master/manual/en-US/about-joomla/installing-the-repo.md but either way I've normalised everything (I've now also expanded the API Auth section). Note that since your test I've added in the version prefix as per the original plan. Hopefully this now reflects what's in the product (once github propagates the commits into the docs)

There's a possibility I might swap the Basic Auth out for a token based system before this goes stable. But for now at least it will be basic auth.

Requests not working

Did you guys clear the libraries/autoload_psr4.php file to autoload the components api sections? Because if you didn't that would definitely cause the issue you're seeing. I have also fixed an issue with the edit and post commands. Looks like I still have a bug in the delete one which I'll look into tomorrow.

SEF support

I thought I'd covered this with https://github.com/wilsonge/joomla-cms/blob/webservices-version2/libraries/src/Router/ApiRouter.php#L117-L128 - might need some help to test this if it's not working as I'm mostly working local and haven't set up the whole htaccess stuff.

ACL

Oops section missing from com_config now added. Hopefully that's the permission you were looking for (note by default I'd picked admin users only but again that can all be negotiated)

@wilsonge wilsonge force-pushed the wilsonge:webservices-version2 branch from 9a9f7cc to db1a3b2 Jan 26, 2019

@wilsonge

This comment has been minimized.

Copy link
Contributor Author

wilsonge commented Jan 26, 2019

Delete button fixed. That should be all the endpoints working again

@wilsonge wilsonge force-pushed the wilsonge:webservices-version2 branch from db1a3b2 to 998e631 Jan 26, 2019

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