Added testing coverage to user API token interfaces

ssddanbrown · ssddanbrown · commit 832fbd65afca · 2019-12-29T19:46:46.000Z
diff --git a/app/Api/ApiToken.php b/app/Api/ApiToken.php
@@ -6,6 +6,6 @@ class ApiToken extends Model
 {
     protected $fillable = ['name', 'expires_at'];
     protected $casts = [
-        'expires_at' => 'datetime:Y-m-d'
+        'expires_at' => 'date:Y-m-d'
     ];
 }
diff --git a/app/Http/Controllers/UserApiTokenController.php b/app/Http/Controllers/UserApiTokenController.php
@@ -17,7 +17,7 @@ public function create(int $userId)
     {
         // Ensure user is has access-api permission and is the current user or has permission to manage the current user.
         $this->checkPermission('access-api');
-        $this->checkPermissionOrCurrentUser('manage-users', $userId);
+        $this->checkPermissionOrCurrentUser('users-manage', $userId);
 
         $user = User::query()->findOrFail($userId);
         return view('users.api-tokens.create', [
@@ -31,7 +31,7 @@ public function create(int $userId)
     public function store(Request $request, int $userId)
     {
         $this->checkPermission('access-api');
-        $this->checkPermissionOrCurrentUser('manage-users', $userId);
+        $this->checkPermissionOrCurrentUser('users-manage', $userId);
 
         $this->validate($request, [
             'name' => 'required|max:250',
@@ -55,8 +55,10 @@ public function store(Request $request, int $userId)
         }
 
         $token->save();
-        // TODO - Notification and activity?
+        $token->refresh();
+
         session()->flash('api-token-secret:' . $token->id, $secret);
+        $this->showSuccessNotification(trans('settings.user_api_token_create_success'));
         return redirect($user->getEditUrl('/api-tokens/' . $token->id));
     }
 
@@ -89,7 +91,7 @@ public function update(Request $request, int $userId, int $tokenId)
         [$user, $token] = $this->checkPermissionAndFetchUserToken($userId, $tokenId);
 
         $token->fill($request->all())->save();
-        // TODO - Notification and activity?
+        $this->showSuccessNotification(trans('settings.user_api_token_update_success'));
         return redirect($user->getEditUrl('/api-tokens/' . $token->id));
     }
 
@@ -113,7 +115,7 @@ public function destroy(int $userId, int $tokenId)
         [$user, $token] = $this->checkPermissionAndFetchUserToken($userId, $tokenId);
         $token->delete();
 
-        // TODO - Notification and activity?, Might have text in translations already (user_api_token_delete_success)
+        $this->showSuccessNotification(trans('settings.user_api_token_delete_success'));
         return redirect($user->getEditUrl('#api_tokens'));
     }
 
@@ -124,8 +126,9 @@ public function destroy(int $userId, int $tokenId)
      */
     protected function checkPermissionAndFetchUserToken(int $userId, int $tokenId): array
     {
-        $this->checkPermission('access-api');
-        $this->checkPermissionOrCurrentUser('manage-users', $userId);
+        $this->checkPermissionOr('users-manage', function () use ($userId) {
+            return $userId === user()->id && userCan('access-api');
+        });
 
         $user = User::query()->findOrFail($userId);
         $token = ApiToken::query()->where('user_id', '=', $user->id)->where('id', '=', $tokenId)->firstOrFail();
diff --git a/database/migrations/2019_12_29_120917_add_api_auth.php b/database/migrations/2019_12_29_120917_add_api_auth.php
@@ -22,7 +22,7 @@ public function up()
             $table->string('client_id')->unique();
             $table->string('client_secret');
             $table->integer('user_id')->unsigned()->index();
-            $table->timestamp('expires_at')->index();
+            $table->date('expires_at')->index();
             $table->nullableTimestamps();
         });
 
diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php
@@ -164,6 +164,8 @@
     'user_api_token_expiry' => 'Expiry Date',
     'user_api_token_expiry_desc' => 'Set a date at which this token expires. After this date, requests made using this token will no longer work. Leaving this field blank will set an expiry 100 years into the future.',
     'user_api_token_create_secret_message' => 'Immediately after creating this token a "client id"" & "client secret" will be generated and displayed. The client secret will only be shown a single time so be sure to copy the value to somewhere safe and secure before proceeding.',
+    'user_api_token_create_success' => 'API token successfully created',
+    'user_api_token_update_success' => 'API token successfully updated',
     'user_api_token' => 'API Token',
     'user_api_token_client_id' => 'Client ID',
     'user_api_token_client_id_desc' => 'This is a non-editable system generated identifier for this token which will need to be provided in API requests.',
diff --git a/resources/views/users/edit.blade.php b/resources/views/users/edit.blade.php
@@ -88,8 +88,7 @@ class="button small outline">{{ trans('settings.users_social_connect') }}</a>
             </section>
         @endif
 
-        {{-- TODO - Review Control--}}
-        @if(($currentUser->id === $user->id && userCan('access-api')) || userCan('manage-users'))
+        @if(($currentUser->id === $user->id && userCan('access-api')) || userCan('users-manage'))
             <section class="card content-wrap auto-height" id="api_tokens">
                 <div class="grid half">
                     <div><h2 class="list-heading">{{ trans('settings.users_api_tokens') }}</h2></div>
diff --git a/tests/User/UserApiTokenTest.php b/tests/User/UserApiTokenTest.php
@@ -0,0 +1,165 @@
+<?php namespace Test;
+
+use BookStack\Api\ApiToken;
+use Carbon\Carbon;
+use Tests\TestCase;
+
+class UserApiTokenTest extends TestCase
+{
+
+    protected $testTokenData = [
+        'name' => 'My test API token',
+        'expires_at' => '2099-04-01',
+    ];
+
+    public function test_tokens_section_not_visible_without_access_api_permission()
+    {
+        $user = $this->getEditor();
+
+        $resp = $this->actingAs($user)->get($user->getEditUrl());
+        $resp->assertDontSeeText('API Tokens');
+
+        $this->giveUserPermissions($user, ['access-api']);
+
+        $resp = $this->actingAs($user)->get($user->getEditUrl());
+        $resp->assertSeeText('API Tokens');
+        $resp->assertSeeText('Create Token');
+    }
+
+    public function test_those_with_manage_users_can_view_other_user_tokens_but_not_create()
+    {
+        $viewer = $this->getViewer();
+        $editor = $this->getEditor();
+        $this->giveUserPermissions($editor, ['users-manage']);
+
+        $resp = $this->actingAs($editor)->get($viewer->getEditUrl());
+        $resp->assertSeeText('API Tokens');
+        $resp->assertDontSeeText('Create Token');
+    }
+
+    public function test_create_api_token()
+    {
+        $editor = $this->getEditor();
+
+        $resp = $this->asAdmin()->get($editor->getEditUrl('/create-api-token'));
+        $resp->assertStatus(200);
+        $resp->assertSee('Create API Token');
+        $resp->assertSee('client secret');
+
+        $resp = $this->post($editor->getEditUrl('/create-api-token'), $this->testTokenData);
+        $token = ApiToken::query()->latest()->first();
+        $resp->assertRedirect($editor->getEditUrl('/api-tokens/' . $token->id));
+        $this->assertDatabaseHas('api_tokens', [
+            'user_id' => $editor->id,
+            'name' => $this->testTokenData['name'],
+            'expires_at' => $this->testTokenData['expires_at'],
+        ]);
+
+        // Check secret token
+        $this->assertSessionHas('api-token-secret:' . $token->id);
+        $secret = session('api-token-secret:' . $token->id);
+        $this->assertDatabaseMissing('api_tokens', [
+            'client_secret' => $secret,
+        ]);
+        $this->assertTrue(\Hash::check($secret, $token->client_secret));
+
+        $this->assertTrue(strlen($token->client_id) === 32);
+        $this->assertTrue(strlen($secret) === 32);
+
+        $this->assertSessionHas('success');
+    }
+
+    public function test_create_with_no_expiry_sets_expiry_hundred_years_away()
+    {
+        $editor = $this->getEditor();
+        $this->asAdmin()->post($editor->getEditUrl('/create-api-token'), ['name' => 'No expiry token']);
+        $token = ApiToken::query()->latest()->first();
+
+        $over = Carbon::now()->addYears(101);
+        $under = Carbon::now()->addYears(99);
+        $this->assertTrue(
+            ($token->expires_at < $over && $token->expires_at > $under),
+            "Token expiry set at 100 years in future"
+        );
+    }
+
+    public function test_created_token_displays_on_profile_page()
+    {
+        $editor = $this->getEditor();
+        $this->asAdmin()->post($editor->getEditUrl('/create-api-token'), $this->testTokenData);
+        $token = ApiToken::query()->latest()->first();
+
+        $resp = $this->get($editor->getEditUrl());
+        $resp->assertElementExists('#api_tokens');
+        $resp->assertElementContains('#api_tokens', $token->name);
+        $resp->assertElementContains('#api_tokens', $token->client_id);
+        $resp->assertElementContains('#api_tokens', $token->expires_at->format('Y-m-d'));
+    }
+
+    public function test_client_secret_shown_once_after_creation()
+    {
+        $editor = $this->getEditor();
+        $resp = $this->asAdmin()->followingRedirects()->post($editor->getEditUrl('/create-api-token'), $this->testTokenData);
+        $resp->assertSeeText('Client Secret');
+
+        $token = ApiToken::query()->latest()->first();
+        $this->assertNull(session('api-token-secret:' . $token->id));
+
+        $resp = $this->get($editor->getEditUrl('/api-tokens/' . $token->id));
+        $resp->assertDontSeeText('Client Secret');
+    }
+
+    public function test_token_update()
+    {
+        $editor = $this->getEditor();
+        $this->asAdmin()->post($editor->getEditUrl('/create-api-token'), $this->testTokenData);
+        $token = ApiToken::query()->latest()->first();
+        $updateData = [
+            'name' => 'My updated token',
+            'expires_at' => '2011-01-01',
+        ];
+
+        $resp = $this->put($editor->getEditUrl('/api-tokens/' . $token->id), $updateData);
+        $resp->assertRedirect($editor->getEditUrl('/api-tokens/' . $token->id));
+
+        $this->assertDatabaseHas('api_tokens', array_merge($updateData, ['id' => $token->id]));
+        $this->assertSessionHas('success');
+    }
+
+    public function test_token_delete()
+    {
+        $editor = $this->getEditor();
+        $this->asAdmin()->post($editor->getEditUrl('/create-api-token'), $this->testTokenData);
+        $token = ApiToken::query()->latest()->first();
+
+        $tokenUrl = $editor->getEditUrl('/api-tokens/' . $token->id);
+
+        $resp = $this->get($tokenUrl . '/delete');
+        $resp->assertSeeText('Delete Token');
+        $resp->assertSeeText($token->name);
+        $resp->assertElementExists('form[action="'.$tokenUrl.'"]');
+
+        $resp = $this->delete($tokenUrl);
+        $resp->assertRedirect($editor->getEditUrl('#api_tokens'));
+        $this->assertDatabaseMissing('api_tokens', ['id' => $token->id]);
+    }
+
+    public function test_user_manage_can_delete_token_without_api_permission_themselves()
+    {
+        $viewer = $this->getViewer();
+        $editor = $this->getEditor();
+        $this->giveUserPermissions($editor, ['users-manage']);
+
+        $this->asAdmin()->post($viewer->getEditUrl('/create-api-token'), $this->testTokenData);
+        $token = ApiToken::query()->latest()->first();
+
+        $resp = $this->actingAs($editor)->get($viewer->getEditUrl('/api-tokens/' . $token->id));
+        $resp->assertStatus(200);
+        $resp->assertSeeText('Delete Token');
+
+        $resp = $this->actingAs($editor)->delete($viewer->getEditUrl('/api-tokens/' . $token->id));
+        $resp->assertRedirect($viewer->getEditUrl('#api_tokens'));
+        $this->assertDatabaseMissing('api_tokens', ['id' => $token->id]);
+    }
+
+}
diff --git a/tests/User/UserPreferencesTest.php b/tests/User/UserPreferencesTest.php
diff --git a/tests/User/UserProfileTest.php b/tests/User/UserProfileTest.php

Original file line number	Diff line number	Diff line change
`@@ -6,6 +6,6 @@ class ApiToken extends Model`
`6`	`6`	`{`
`7`	`7`	`protected $fillable = ['name', 'expires_at'];`
`8`	`8`	`protected $casts = [`
`9`		`- 'expires_at' => 'datetime:Y-m-d'`
	`9`	`+ 'expires_at' => 'date:Y-m-d'`
`10`	`10`	`];`
`11`	`11`	`}`