phpbb/phpbb — pull request #3100 by galaxyAbstractor (2014-11-01 15:22:11+00:00)
Title: [ticket/11595] REST API - Auth System

In the [previous pull request](https://github.com/phpbb/phpbb/pull/1490) it was decided to split it into smaller pull requests for easier merging.

## Authentication
This pull request contains API authentication. It has changed a small bit since the last pull request, I'll come to that shortly.

This allows an application to request authorization to access a phpBB board as an user. After an application has been authorized by the user, it can proceed to access parts of a board that requires login, currently the only endpoint (other than requesting authorization) is to verify that the application can create requests on behalf of the user.

## Authorization flow
For an application to use features of a board, it first needs to ask for permission from the user. This starts by accessing the `/api/auth/generate_keys` endpoint. This generates three 16-character long keys (using `unique_id()`), an exchange key, an auth key and a sign key. The keys are saved in the database, but only the exchange key is sent to the application.

The application would then tell the user to visit `/api/auth/authorize/{exchange_key}`, where the user proceeds to give the application a name and gives the application permission.

When that's done and the application is authorized, the application retrieves the auth key and sign key by calling the `/api/auth/exchange_key/{exchange_key}` endpoint. This moves the keys from the temporary storage in the api_exchange_keys table to the api_keys table and returns the auth and sign key to the application.

To verify that the application has access, the application would call the `/api/auth/verify` endpoint. The authentication info has been split away from the "route" as GET attributes to allow for optional parameters in the route.

To make an authenticated request, you have to hash the request first. This is done by taking the endpoint, and adding `auth_key={auth_key}&serial={serial}` to the end (order matters, and the ? is left out intentionally). An example request might look like: `/api/auth/verifyauth_key=jhrtbgfe45th789k&serial=1`. Then you use `hash_hmac` to hash it using your sign key, like so: `$hash = hash_hmac('sha256', $request, $sign_key);`. When that's done, it's time to make the request, which is done like so: `/api/auth/verify?auth_key=jhrtbgfe45th789k&serial=1&hash=$hash`.

The serial is used to protect against someone resending an already sent request. It should be increased with each request you make.

## Changes from previous PR
Previously, the application could chose it's own auth and sign key as long as they were 16 chars long. This made the process of authorization a bit easier, but at the same time made the API unsecure. An application developer could possibly hardcode auth and sign keys in their applications, and could in that way visit phpBB boards with the hardcoded keys until an account is found.

## Feedback
I'm unsure about a couple of things that I would like to know if it's a good or bad idea or if I should do something else instead.

### Exceptions
I have used a lot of exceptions in my code to catch errors. There's a couple of ways of using exceptions, and I'm unsure if I should do it like I did (since I can't see phpBB use exceptions very often). The way I've done it is that I've created a handful exception classes where the classname describes the exception. This way you can catch different exceptions in different catch-clauses, but leads to a lot of empty classes and files, which might not be desired? The other way is to have a single exception with different status codes and messages, and you switch over that in the catch-clause. However, the way I currently catch exceptions I could have gone for the second approach anyway.

### Language
I probably have to fix the language to be more clear than it is right now, but then there is another thing: localization of error messages returned by the API. These messages are not intended for the board user to see, but give feedback to the developer and application about what went wrong. If these would be translated, applications could break that catches these error messages and compares against them. I've tried to stick to HTTP error codes for the API error codes, which makes some errors distinct only by the message. Maybe have an unique error code for each error instead?

### Length of the test case
I'm using datasets and arrays, and when done in as shown in the [wiki](https://wiki.phpbb.com/Unit_Tests), with one-line arrays, it was quite hard for me to follow when trying to debug and read my unit tests. But I found that the tests have become obnoxiously long the way I formatted it, do you want me to minify it to one-line arrays in the wiki to have shorter test files?

## TODO
There's some things left to do which may and will affect the auth system.

### Rate limiting
I think it would be a nice idea to have some kind of rate limiting on the API to stop abuse (posting spam fast, generating a lot of keys, etc). I'm a bit unsure about how to implement it, so I will look into it, but suggestions are welcome.

### Purge the api_exchange_keys table occasionally
If an application requests an exchange key which is later not used, it will need to be automatically removed after a while (1 hour? 1 day? ...?) so it wont clutter the database with a lot of unused keys. Does phpBB have some mechanism to automatically do some cleaning periodically?