From a85088c7a4100b9c6ea7c4fa11a751d43fa309b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Jaussoin?= Date: Tue, 13 Sep 2022 15:21:19 +0200 Subject: [PATCH] Fix #40 Drop the From requirement in the API Key authentication mechanism Small fixes and adjustements in tests --- .../Http/Controllers/Api/ApiKeyController.php | 1 - .../Middleware/AuthenticateDigestOrKey.php | 40 +++++++++---------- .../api/documentation_markdown.blade.php | 9 +++-- flexiapi/tests/Feature/AccountApiKeyTest.php | 11 ++++- .../tests/Feature/AccountProvisioningTest.php | 2 +- flexiapi/tests/TestCase.php | 1 - flexisip-account-manager.spec | 2 +- 7 files changed, 37 insertions(+), 29 deletions(-) diff --git a/flexiapi/app/Http/Controllers/Api/ApiKeyController.php b/flexiapi/app/Http/Controllers/Api/ApiKeyController.php index 7279f76..324f5a3 100644 --- a/flexiapi/app/Http/Controllers/Api/ApiKeyController.php +++ b/flexiapi/app/Http/Controllers/Api/ApiKeyController.php @@ -5,7 +5,6 @@ namespace App\Http\Controllers\Api; use App\AuthToken; use App\Http\Controllers\Controller; use Illuminate\Http\Request; -use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Cookie; class ApiKeyController extends Controller diff --git a/flexiapi/app/Http/Middleware/AuthenticateDigestOrKey.php b/flexiapi/app/Http/Middleware/AuthenticateDigestOrKey.php index 7944c2c..caffe29 100644 --- a/flexiapi/app/Http/Middleware/AuthenticateDigestOrKey.php +++ b/flexiapi/app/Http/Middleware/AuthenticateDigestOrKey.php @@ -20,6 +20,7 @@ namespace App\Http\Middleware; use App\Account; +use App\ApiKey; use Carbon\Carbon; use Illuminate\Validation\Rule; use Illuminate\Support\Facades\Auth; @@ -43,6 +44,25 @@ class AuthenticateDigestOrKey */ public function handle($request, Closure $next) { + // Key authentication + + if ($request->header('x-api-key') || $request->cookie('x-api-key')) { + $apiKey = ApiKey::where('key', $request->header('x-api-key') ?? $request->cookie('x-api-key')) + ->first(); + + if ($apiKey) { + $apiKey->last_used_at = Carbon::now(); + $apiKey->save(); + + Auth::login($apiKey->account); + $response = $next($request); + + return $response; + } + + return $this->generateUnauthorizedResponse($apiKey->account); + } + Validator::make(['from' => $request->header('From')], [ 'from' => 'required', ])->validate(); @@ -57,26 +77,6 @@ class AuthenticateDigestOrKey $resolvedRealm = config('app.realm') ?? $domain; - // Key authentication - - if ($request->header('x-api-key') || $request->cookie('x-api-key')) { - if ($account->apiKey - && ($account->apiKey->key == $request->header('x-api-key') - || $account->apiKey->key == $request->cookie('x-api-key') - )) { - // Refresh the API Key - $account->apiKey->last_used_at = Carbon::now(); - $account->apiKey->save(); - - Auth::login($account); - $response = $next($request); - - return $response; - } - - return $this->generateUnauthorizedResponse($account); - } - // DIGEST authentication if ($request->header('Authorization')) { diff --git a/flexiapi/resources/views/api/documentation_markdown.blade.php b/flexiapi/resources/views/api/documentation_markdown.blade.php index 9376c93..50d8a89 100644 --- a/flexiapi/resources/views/api/documentation_markdown.blade.php +++ b/flexiapi/resources/views/api/documentation_markdown.blade.php @@ -4,7 +4,7 @@ An API to deal with the Flexisip server. The API is available under `/api` -A `from` (consisting of the user SIP address, prefixed with `sip:`), `content-type` and `accept` HTTP headers are REQUIRED to use the API properly +A `content-type` and `accept` HTTP headers are REQUIRED to use the API properly ``` > GET /api/{endpoint} @@ -34,7 +34,6 @@ You can then use your freshly generated key by adding a new `x-api-key` header t ``` > GET /api/{endpoint} -> from: sip:foobar@sip.example.org > x-api-key: {your-api-key} > … ``` @@ -43,7 +42,6 @@ Or using a cookie: ``` > GET /api/{endpoint} -> from: sip:foobar@sip.example.org > Cookie: x-api-key={your-api-key} > … ``` @@ -51,10 +49,13 @@ Or using a cookie: #### Using DIGEST To discover the available hashing algorythm you MUST send an unauthenticated request to one of the restricted endpoints.
-For the moment only DIGEST-MD5 and DIGEST-SHA-256 are supported through the authentication layer. +Only DIGEST-MD5 and DIGEST-SHA-256 are supported through the authentication layer. + +A `from` (consisting of the user SIP address, prefixed with `sip:`) header is required to initiate the DIGEST flow. ``` > GET /api/{restricted-endpoint} +> from: sip:foobar@sip.example.org > … diff --git a/flexiapi/tests/Feature/AccountApiKeyTest.php b/flexiapi/tests/Feature/AccountApiKeyTest.php index a03600f..a7bc6a5 100644 --- a/flexiapi/tests/Feature/AccountApiKeyTest.php +++ b/flexiapi/tests/Feature/AccountApiKeyTest.php @@ -68,6 +68,10 @@ class AccountApiKeyTest extends TestCase $authToken = json_decode($response)->token; + // Try to retrieve an API key from the un-attached auth_token + $response = $this->json($this->method, $this->route . '/' . $authToken) + ->assertStatus(404); + // Attach the auth_token to the account $password = Password::factory()->create(); $password->account->generateApiKey(); @@ -100,10 +104,15 @@ class AccountApiKeyTest extends TestCase ->assertStatus(404); // Check the if the API key can be used for the account + $response = $this->withHeaders(['x-api-key' => $apiKey]) + ->json($this->method, '/api/accounts/me') + ->assertStatus(200) + ->content(); + // Try with a wrong From $response = $this->withHeaders([ - 'From' => 'sip:'.$password->account->identifier, 'x-api-key' => $apiKey, + 'From' => 'sip:baduser@server.tld' ]) ->json($this->method, '/api/accounts/me') ->assertStatus(200) diff --git a/flexiapi/tests/Feature/AccountProvisioningTest.php b/flexiapi/tests/Feature/AccountProvisioningTest.php index ce22708..6598e1d 100644 --- a/flexiapi/tests/Feature/AccountProvisioningTest.php +++ b/flexiapi/tests/Feature/AccountProvisioningTest.php @@ -80,7 +80,7 @@ class AccountProvisioningTest extends TestCase // Regenerate a new provisioning token from the authenticated account $this->keyAuthenticated($password->account) - ->json($this->method, '/api/accounts/me/provision') + ->get('/api/accounts/me/provision') ->assertStatus(200) ->assertSee('provisioning_token') ->assertDontSee($provisioningToken); diff --git a/flexiapi/tests/TestCase.php b/flexiapi/tests/TestCase.php index 7449ca0..029edee 100644 --- a/flexiapi/tests/TestCase.php +++ b/flexiapi/tests/TestCase.php @@ -33,7 +33,6 @@ abstract class TestCase extends BaseTestCase protected function keyAuthenticated(Account $account) { return $this->withHeaders([ - 'From' => 'sip:'.$account->identifier, 'x-api-key' => $account->apiKey->key, ]); } diff --git a/flexisip-account-manager.spec b/flexisip-account-manager.spec index f03f3b6..f143f35 100644 --- a/flexisip-account-manager.spec +++ b/flexisip-account-manager.spec @@ -8,7 +8,7 @@ #%define _datadir %{_datarootdir} #%define _docdir %{_datadir}/doc -%define build_number 150 +%define build_number 151 %define var_dir /var/opt/belledonne-communications %define opt_dir /opt/belledonne-communications/share/flexisip-account-manager