Implement XMLRPC like retrocompatibility endpoints
Validate phone-info endpoint phone format
Improve OVHSMS error handling and log errors
Complete tests
Fix #34 return a generic message for 404 errors
Fix #38 simplify the domain resolving parameter and remove the domain parameter in the token based account creation endpoint
This commit is contained in:
Timothée Jaussoin 2022-08-10 12:02:05 +02:00
parent f080003516
commit b1d58d83c9
12 changed files with 632 additions and 430 deletions

View file

@ -14,6 +14,7 @@ APP_API_KEY_EXPIRATION_MINUTES=60 # Number of minutes the generated API Keys are
# Risky toggles
APP_EVERYONE_IS_ADMIN=false # Allow any accounts to request the API as an administrator
APP_ADMINS_MANAGE_MULTI_DOMAINS=false # Allow admins to handle all the accounts in the database
APP_DANGEROUS_ENDPOINTS=false # Enable some dangerous endpoints used for XMLRPC like fallback usage
# SIP server parameters
ACCOUNT_PROXY_REGISTRAR_ADDRESS=sip.example.com # Proxy registrar address, can be different than the SIP domain

View file

@ -3,6 +3,7 @@
namespace App\Exceptions;
use Throwable;
use Illuminate\Database\Eloquent\ModelNotFoundException;
use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler;
class Handler extends ExceptionHandler
@ -23,6 +24,10 @@ class Handler extends ExceptionHandler
public function render($request, Throwable $exception)
{
if ($exception instanceof ModelNotFoundException && $request->wantsJson()) {
return response()->json(['message' => 'Missing elements to perform the request'], 404);
}
return parent::render($request, $exception);
}
}

View file

@ -71,9 +71,7 @@ class AccountController extends Controller
$account->username = $request->get('username');
$account->email = $request->get('email');
$account->display_name = $request->get('display_name');
$account->domain = $request->has('domain') && config('app.admins_manage_multi_domains')
? $request->get('domain')
: config('app.sip_domain');
$account->domain = $this->resolveDomain($request);
$account->ip_address = $request->ip();
$account->creation_time = Carbon::now();
$account->user_agent = config('app.name');

View file

@ -23,15 +23,20 @@ use Illuminate\Http\Request;
use Illuminate\Validation\Rule;
use Illuminate\Support\Str;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Facades\Mail;
use App\Http\Controllers\Controller;
use Carbon\Carbon;
use App\Account;
use App\AccountTombstone;
use App\AccountCreationToken;
use App\Alias;
use App\Http\Controllers\Account\AuthenticateController as WebAuthenticateController;
use App\Libraries\OvhSMS;
use App\Mail\RegisterConfirmation;
use App\Rules\IsNotPhoneNumber;
use App\Rules\NoUppercase;
use App\Rules\WithoutSpaces;
class AccountController extends Controller
{
@ -48,6 +53,164 @@ class AccountController extends Controller
]);
}
/**
* /!\ Dangerous endpoint, disabled by default
*/
public function phoneInfo(Request $request, string $phone)
{
if (!config('app.dangerous_endpoints')) return abort(404);
$request->merge(['phone' => $phone]);
$request->validate([
'phone' => [ 'required', new WithoutSpaces, 'starts_with:+']
]);
$alias = Alias::where('alias', $phone)->first();
$account = $alias
? $alias->account
: Account::sip($phone)->firstOrFail();
return \response()->json([
'activated' => $account->activated,
'realm' => $account->realm
]);
}
/**
* /!\ Dangerous endpoint, disabled by default
* Store directly the account and alias in the DB and send a SMS or email for the validation
*/
public function storePublic(Request $request)
{
if (!config('app.dangerous_endpoints')) return abort(404);
$request->validate([
'username' => [
'prohibits:phone',
new NoUppercase,
new IsNotPhoneNumber,
Rule::unique('accounts', 'username')->where(function ($query) use ($request) {
$query->where('domain', $request->has('domain') ? $request->get('domain') : config('app.sip_domain'));
}),
Rule::unique('accounts_tombstones', 'username')->where(function ($query) use ($request) {
$query->where('domain', $request->has('domain') ? $request->get('domain') : config('app.sip_domain'));
}),
'filled',
],
'algorithm' => 'required|in:SHA-256,MD5',
'password' => 'required|filled',
'domain' => 'min:3',
'email' => 'required_without:phone|email',
'phone' => [
'required_without:email',
'prohibits:username',
'unique:aliases,alias',
'unique:accounts,username',
new WithoutSpaces, 'starts_with:+'
]
]);
$account = new Account;
$account->username = !empty($request->get('username'))
? $request->get('username')
: $request->get('phone');
$account->email = $request->get('email');
$account->activated = false;
$account->domain = $request->has('domain')
? $request->get('domain')
: config('app.sip_domain');
$account->ip_address = $request->ip();
$account->creation_time = Carbon::now();
$account->user_agent = config('app.name');
$account->provisioning_token = Str::random(WebAuthenticateController::$emailCodeSize);
$account->save();
$account->updatePassword($request->get('password'), $request->get('algorithm'));
Log::channel('events')->info('API: Account created using the public endpoint', ['id' => $account->identifier]);
// Send validation by phone
if ($request->has('phone')) {
$alias = new Alias;
$alias->alias = $request->get('phone');
$alias->domain = config('app.sip_domain');
$alias->account_id = $account->id;
$alias->save();
$account->confirmation_key = generatePin();
$account->save();
Log::channel('events')->info('API: Account created using the public endpoint by phone', ['id' => $account->identifier]);
$ovhSMS = new OvhSMS;
$ovhSMS->send($request->get('phone'), 'Your ' . config('app.name') . ' recovery code is ' . $account->confirmation_key);
}
// Send validation by email
elseif ($request->has('email')) {
$account->confirmation_key = Str::random(WebAuthenticateController::$emailCodeSize);
$account->save();
Log::channel('events')->info('API: Account created using the public endpoint by email', ['id' => $account->identifier]);
Mail::to($account)->send(new RegisterConfirmation($account));
}
// Full reload
return Account::withoutGlobalScopes()->find($account->id);
}
/**
* /!\ Dangerous endpoint, disabled by default
*/
public function recoverByPhone(Request $request)
{
if (!config('app.dangerous_endpoints')) return abort(404);
$request->validate([
'phone' => [
'required', new WithoutSpaces, 'starts_with:+'
]
]);
$alias = Alias::where('alias', $request->get('phone'))->first();
$account = $alias
? $alias->account
: Account::sip($request->get('phone'))->firstOrFail();
$account->confirmation_key = generatePin();
$account->save();
Log::channel('events')->info('API: Account recovery by phone', ['id' => $account->identifier]);
$ovhSMS = new OvhSMS;
$ovhSMS->send($request->get('phone'), 'Your ' . config('app.name') . ' recovery code is ' . $account->confirmation_key);
}
/**
* /!\ Dangerous endpoint, disabled by default
*/
public function recoverUsingKey(string $sip, string $recoveryKey)
{
if (!config('app.dangerous_endpoints')) return abort(404);
$account = Account::sip($sip)
->where('confirmation_key', $recoveryKey)
->firstOrFail();
if ($account->activationExpired()) abort(403, 'Activation expired');
$account->activated = true;
$account->confirmation_key = null;
$account->save();
$account->passwords->each(function ($i, $k) {
$i->makeVisible(['password']);
});
return $account;
}
public function store(Request $request)
{
$request->validate([
@ -56,21 +219,16 @@ class AccountController extends Controller
new NoUppercase,
new IsNotPhoneNumber,
Rule::unique('accounts', 'username')->where(function ($query) use ($request) {
$query->where('domain', $request->has('domain') && config('app.everyone_is_admin') && config('app.admins_manage_multi_domains')
? $request->get('domain')
: config('app.sip_domain'));
$query->where('domain', config('app.sip_domain'));
}),
Rule::unique('accounts_tombstones', 'username')->where(function ($query) use ($request) {
$query->where('domain', $request->has('domain') && config('app.everyone_is_admin') && config('app.admins_manage_multi_domains')
? $request->get('domain')
: config('app.sip_domain'));
$query->where('domain', config('app.sip_domain'));
}),
'filled',
],
'algorithm' => 'required|in:SHA-256,MD5',
'password' => 'required|filled',
'dtmf_protocol' => 'nullable|in:' . Account::dtmfProtocolsRule(),
'domain' => 'min:3',
'account_creation_token' => [
'required_without:token',
Rule::exists('account_creation_tokens', 'token')->where(function ($query) {
@ -96,9 +254,7 @@ class AccountController extends Controller
$account->username = $request->get('username');
$account->email = $request->get('email');
$account->activated = false;
$account->domain = ($request->has('domain') && config('app.everyone_is_admin') && config('app.admins_manage_multi_domains'))
? $request->get('domain')
: config('app.sip_domain');
$account->domain = config('app.sip_domain');
$account->ip_address = $request->ip();
$account->creation_time = Carbon::now();
$account->user_agent = config('app.name');

View file

@ -111,10 +111,7 @@ class AccountController extends Controller
new NoUppercase,
new IsNotPhoneNumber,
Rule::unique('accounts', 'username')->where(function ($query) use ($request) {
$query->where('domain', $request->has('domain') && config('app.admins_manage_multi_domains')
? $request->get('domain')
: config('app.sip_domain')
);
$query->where('domain', $this->resolveDomain($request));
}),
'filled',
],
@ -139,15 +136,11 @@ class AccountController extends Controller
$account->username = $request->get('username');
$account->email = $request->get('email');
$account->display_name = $request->get('display_name');
$account->activated = $request->has('activated')
? (bool)$request->get('activated')
: false;
$account->activated = $request->has('activated') ? (bool)$request->get('activated') : false;
$account->ip_address = $request->ip();
$account->dtmf_protocol = $request->get('dtmf_protocol');
$account->creation_time = Carbon::now();
$account->domain = $request->has('domain') && config('app.admins_manage_multi_domains')
? $request->get('domain')
: config('app.sip_domain');
$account->domain = $this->resolveDomain($request);
$account->user_agent = $request->header('User-Agent') ?? config('app.name');
if (!$request->has('activated') || !(bool)$request->get('activated')) {

View file

@ -5,9 +5,20 @@ namespace App\Http\Controllers;
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
use Illuminate\Foundation\Bus\DispatchesJobs;
use Illuminate\Foundation\Validation\ValidatesRequests;
use Illuminate\Http\Request;
use Illuminate\Routing\Controller as BaseController;
class Controller extends BaseController
{
use AuthorizesRequests, DispatchesJobs, ValidatesRequests;
protected function resolveDomain(Request $request): string
{
return $request->has('domain')
&& $request->user()
&& $request->user()->isAdmin()
&& config('app.admins_manage_multi_domains')
? $request->get('domain')
: config('app.sip_domain');
}
}

View file

@ -20,6 +20,7 @@
namespace App\Libraries;
use Ovh\Api;
use Illuminate\Support\Facades\Log;
class OvhSMS
{
@ -28,6 +29,10 @@ class OvhSMS
public function __construct()
{
if (empty(config('ovh.app_key'))) {
Log::error('OVH SMS API not configured');
}
$this->_api = new Api(
config('ovh.app_key'),
config('ovh.app_secret'),
@ -35,12 +40,24 @@ class OvhSMS
config('ovh.app_consumer_key')
);
try {
$smsServices = $this->_api->get('/sms/');
if (!empty($smsServices)) $this->_smsService = $smsServices[0];
if (!empty($smsServices)) {
$this->_smsService = $smsServices[0];
}
} catch (\GuzzleHttp\Exception\ClientException $e) {
Log::error('OVH SMS API not reachable: ' . $e->getMessage());
}
}
public function send(string $to, string $message)
{
if (!$this->_smsService) {
Log::error('OVH SMS API not configured');
return;
}
$content = (object) [
'charset' => 'UTF-8',
'class' => 'phoneDisplay',
@ -54,9 +71,13 @@ class OvhSMS
'validityPeriod' => 2880
];
try {
$this->_api->post('/sms/'. $this->_smsService . '/jobs', $content);
// One credit removed
$this->_api->get('/sms/'. $this->_smsService . '/jobs');
} catch (\GuzzleHttp\Exception\ClientException $e) {
Log::error('OVH SMS not sent: ' . $e->getMessage());
}
}
}

538
flexiapi/composer.lock generated

File diff suppressed because it is too large Load diff

View file

@ -67,6 +67,11 @@ return [
*/
'admins_manage_multi_domains' => env('APP_ADMINS_MANAGE_MULTI_DOMAINS', false),
/**
* /!\ Enable dangerous endpoints required for fallback
*/
'dangerous_endpoints' => env('APP_DANGEROUS_ENDPOINTS', false),
/*
|--------------------------------------------------------------------------
| Application Environment

View file

@ -112,6 +112,26 @@ Return `404` if the token is non existing or invalid.
## Accounts
### `POST /accounts/public`
@if(config('app.dangerous_endpoints'))**Enabled on this instance**@else**Not enabled on this instance**@endif
<span class="badge badge-success">Public</span>
<span class="badge badge-warning">Unsecure endpoint</span>
Create an account.
Return `422` if the parameters are invalid.
Send an email with the activation key if `email` is set, send an SMS otherwise.
JSON parameters:
* `username` required if `phone` not set, unique username, minimum 6 characters
* `password` required minimum 6 characters
* `algorithm` required, values can be `SHA-256` or `MD5`
* `domain` if not set the value is enforced to the default registration domain set in the global configuration
* `email` optional if `phone` set, an email, set an email to the account
* `phone` required if `username` not set, optional if `email` set, a phone number, set a phone number to the account
### `POST /accounts/with-account-creation-token`
<span class="badge badge-success">Public</span>
Create an account using an `account_creation_token`.
@ -122,7 +142,6 @@ JSON parameters:
* `username` unique username, minimum 6 characters
* `password` required minimum 6 characters
* `algorithm` required, values can be `SHA-256` or `MD5`
* `domain` **not configurable except during test deployments** the value is enforced to the default registration domain set in the global configuration
* `account_creation_token` the unique `account_creation_token`
* `dtmf_protocol` optional, values must be `sipinfo` or `rfc2833`
@ -131,6 +150,39 @@ JSON parameters:
Retrieve public information about the account.
Return `404` if the account doesn't exists.
### `GET /accounts/{phone}/info-by-phone`
@if(config('app.dangerous_endpoints'))**Enabled on this instance**@else**Not enabled on this instance**@endif
<span class="badge badge-success">Public</span>
<span class="badge badge-warning">Unsecure endpoint</span>
Retrieve public information about the account.
Return `404` if the account doesn't exists.
### `POST /accounts/recover-by-phone`
@if(config('app.dangerous_endpoints'))**Enabled on this instance**@else**Not enabled on this instance**@endif
<span class="badge badge-success">Public</span>
<span class="badge badge-warning">Unsecure endpoint</span>
Send a SMS with a recovery PIN code to the `phone` number provided.
Return `404` if the account doesn't exists.
JSON parameters:
* `phone` required the phone number to send the SMS to
### `GET /accounts/{sip}/recover/{recover_key}`
@if(config('app.dangerous_endpoints'))**Enabled on this instance**@else**Not enabled on this instance**@endif
<span class="badge badge-success">Public</span>
<span class="badge badge-warning">Unsecure endpoint</span>
Activate the account if the correct `recover_key` is provided.
Return the account information (including the hashed password) if valid.
Return `404` if the account doesn't exists.
### `POST /accounts/{sip}/activate/email`
<span class="badge badge-success">Public</span>
Activate an account using a secret code received by email.
@ -194,8 +246,7 @@ JSON parameters:
* `username` unique username, minimum 6 characters
* `password` required minimum 6 characters
* `algorithm` required, values can be `SHA-256` or `MD5`
* `domain` **not configurable by default. The value is enforced to the default domain set in the global configuration (`app.sip_domain`)**
The `domain` field is taken into account ONLY when `app.admins_manage_multi_domains` is set to `true` in the global configuration
* `domain` **not configurable by default**. Only configurable if `APP_ADMINS_MANAGE_MULTI_DOMAINS` is set to `true` in the global configuration. Otherwise `APP_SIP_DOMAIN` is used.
* `activated` optional, a boolean, set to `false` by default
* `display_name` optional, string
* `admin` optional, a boolean, set to `false` by default, create an admin account

View file

@ -29,13 +29,22 @@ Route::get('ping', 'Api\PingController@ping');
Route::post('account_creation_tokens/send-by-push', 'Api\AccountCreationTokenController@sendByPush');
// Old URL, for retro-compatibility
Route::post('tokens', 'Api\AccountCreationTokenController@sendByPush');
Route::get('accounts/{sip}/info', 'Api\AccountController@info');
Route::post('accounts/with-account-creation-token', 'Api\AccountController@store');
// Old URL, for retro-compatibility
Route::post('accounts/with-token', 'Api\AccountController@store');
Route::post('accounts/{sip}/activate/email', 'Api\AccountController@activateEmail');
Route::post('accounts/{sip}/activate/phone', 'Api\AccountController@activatePhone');
// /!\ Dangerous endpoints
Route::post('accounts/public', 'Api\AccountController@storePublic');
Route::get('accounts/{sip}/recover/{recovery_key}', 'Api\AccountController@recoverUsingKey');
Route::post('accounts/recover-by-phone', 'Api\AccountController@recoverByPhone');
Route::get('accounts/{phone}/info-by-phone', 'Api\AccountController@phoneInfo');
Route::post('accounts/auth_token', 'Api\AuthTokenController@store');
Route::get('accounts/me/api_key/{auth_token}', 'Api\ApiKeyController@generateFromToken')->middleware('cookie', 'cookie.encrypt');

View file

@ -24,7 +24,7 @@ use App\Account;
use App\AccountTombstone;
use App\ActivationExpiration;
use App\Admin;
use App\Alias as AppAlias;
use Carbon\Carbon;
use Illuminate\Foundation\Testing\RefreshDatabase;
@ -480,6 +480,194 @@ class AccountApiTest extends TestCase
]);
}
/**
* /!\ Dangerous endpoints
*/
public function testRecover()
{
$confirmationKey = '0123';
$password = Password::factory()->create();
$password->account->generateApiKey();
$password->account->confirmation_key = $confirmationKey;
$password->account->activated = false;
$password->account->save();
config()->set('app.dangerous_endpoints', true);
$this->assertDatabaseHas('accounts', [
'username' => $password->account->username,
'domain' => $password->account->domain,
'activated' => false
]);
$this->get($this->route.'/'.$password->account->identifier.'/recover/'.$confirmationKey)
->assertJson(['passwords' => [[
'password' => $password->password,
'algorithm' => $password->algorithm
]]])
->assertStatus(200);
$this->json('GET', $this->route.'/'.$password->account->identifier.'/recover/'.$confirmationKey)
->assertStatus(404);
$this->assertDatabaseHas('accounts', [
'username' => $password->account->username,
'domain' => $password->account->domain,
'activated' => true
]);
}
/**
* /!\ Dangerous endpoints
*/
public function testRecoverPhone()
{
$phone = '+3361234';
$password = Password::factory()->create();
$password->account->generateApiKey();
$password->account->activated = false;
$password->account->save();
config()->set('app.dangerous_endpoints', true);
$alias = new AppAlias;
$alias->alias = $phone;
$alias->domain = $password->account->domain;
$alias->account_id = $password->account->id;
$alias->save();
$this->json($this->method, $this->route.'/recover-by-phone', [
'phone' => $phone
])
->assertStatus(200);
$password->account->refresh();
$this->get($this->route.'/'.$password->account->identifier.'/recover/'.$password->account->confirmation_key)
->assertStatus(200)
->assertJson([
'activated' => true
]);
$this->get($this->route.'/'.$phone.'/info-by-phone')
->assertStatus(200)
->assertJson([
'activated' => true
]);
$this->get($this->route.'/+1234/info-by-phone')
->assertStatus(404);
$this->json('GET', $this->route.'/'.$password->account->identifier.'/info-by-phone')
->assertStatus(422)
->assertJsonValidationErrors(['phone']);
}
/**
* /!\ Dangerous endpoints
*/
public function testCreatePublic()
{
$username = 'publicuser';
config()->set('app.dangerous_endpoints', true);
// Missing email
$this->json($this->method, $this->route.'/public', [
'username' => $username,
'algorithm' => 'SHA-256',
'password' => '2',
])
->assertStatus(422)
->assertJsonValidationErrors(['email']);
$this->json($this->method, $this->route.'/public', [
'username' => $username,
'algorithm' => 'SHA-256',
'password' => '2',
'email' => 'john@doe.tld',
])
->assertStatus(200)
->assertJson([
'activated' => false
]);
// Already created
$this->json($this->method, $this->route.'/public', [
'username' => $username,
'algorithm' => 'SHA-256',
'password' => '2',
'email' => 'john@doe.tld',
])
->assertStatus(422)
->assertJsonValidationErrors(['username']);
$this->assertDatabaseHas('accounts', [
'username' => $username,
'domain' => config('app.sip_domain')
]);
}
public function testCreatePublicPhone()
{
$phone = '+12345';
config()->set('app.dangerous_endpoints', true);
// Username and phone
$this->json($this->method, $this->route.'/public', [
'username' => 'myusername',
'phone' => $phone,
'algorithm' => 'SHA-256',
'password' => '2',
'email' => 'john@doe.tld',
])
->assertStatus(422)
->assertJsonValidationErrors(['phone', 'username']);
// Bad phone format
$this->json($this->method, $this->route.'/public', [
'phone' => 'username',
'algorithm' => 'SHA-256',
'password' => '2',
'email' => 'john@doe.tld',
])
->assertStatus(422)
->assertJsonValidationErrors(['phone']);
$this->json($this->method, $this->route.'/public', [
'phone' => $phone,
'algorithm' => 'SHA-256',
'password' => '2',
'email' => 'john@doe.tld',
])
->assertStatus(200)
->assertJson([
'activated' => false
]);
// Already exists
$this->json($this->method, $this->route.'/public', [
'phone' => $phone,
'algorithm' => 'SHA-256',
'password' => '2',
'email' => 'john@doe.tld',
])
->assertStatus(422)
->assertJsonValidationErrors(['phone']);
$this->assertDatabaseHas('accounts', [
'username' => $phone,
'domain' => config('app.sip_domain')
]);
$this->assertDatabaseHas('aliases', [
'alias' => $phone,
'domain' => config('app.sip_domain')
]);
}
public function testActivatePhone()
{
$confirmationKey = '0123';
@ -515,9 +703,9 @@ class AccountApiTest extends TestCase
])
->assertStatus(200);
$this->get($this->route.'/'.$password->account->identifier.'/info')
->assertStatus(200)
->assertJson([
$this->assertDatabaseHas('accounts', [
'username' => $password->account->username,
'domain' => $password->account->domain,
'activated' => true
]);
}
@ -576,9 +764,7 @@ class AccountApiTest extends TestCase
'password' => $password
])
->assertStatus(422)
->assertJson([
'errors' => ['algorithm' => true]
]);
->assertJsonValidationErrors(['algorithm']);
// Fresh password without an old one
$this->keyAuthenticated($account)
@ -606,9 +792,7 @@ class AccountApiTest extends TestCase
'password' => $newPassword
])
->assertStatus(422)
->assertJson([
'errors' => ['old_password' => true]
]);
->assertJsonValidationErrors(['old_password']);
// Set the new password with incorrect old password
$this->keyAuthenticated($account)
@ -617,9 +801,7 @@ class AccountApiTest extends TestCase
'old_password' => 'blabla',
'password' => $newPassword
])
->assertJson([
'errors' => ['old_password' => true]
])
->assertJsonValidationErrors(['old_password'])
->assertStatus(422);
// Set the new password