Performed review of "public intended" functionality provided in #1817

- Updated logic to take url from referrer rather than pass as a query parameter.
- Added tests to cover functionality.
- Updated 404 page with login action button if not signed in.
- Updated 404 page with text to indicate permissions may be affecting visibility.

Related to #1817 and #1706

diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php
index 4c396d3cd3f756af684f0e9c5af579bc3ed9d766..4660c16d5e9b47899595d47cf35b48cc51cabc47 100644 (file)
--- a/app/Http/Controllers/Auth/LoginController.php
+++ b/app/Http/Controllers/Auth/LoginController.php
@@ -76,8 +76,9 @@ class LoginController extends Controller
             ]);
         }
 
             ]);
         }
 

-        if ($request->has('intended')) {
-            redirect()->setIntendedUrl($request->get('intended'));
+        $previous = url()->previous('');
+        if (setting('app-public') && $previous && $previous !== url('/login')) {
+            redirect()->setIntendedUrl($previous);

         }
 
         return view('auth.login', [
         }
 
         return view('auth.login', [

diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php
index 38f1ce28a9de0cd3d1b10f7975a7be11bd468279..06a5285f56fc4ce11e6642549a1002b1bacae698 100644 (file)
--- a/resources/lang/en/errors.php
+++ b/resources/lang/en/errors.php
@@ -83,6 +83,7 @@ return [
     // Error pages
     '404_page_not_found' => 'Page Not Found',
     'sorry_page_not_found' => 'Sorry, The page you were looking for could not be found.',
     // Error pages
     '404_page_not_found' => 'Page Not Found',
     'sorry_page_not_found' => 'Sorry, The page you were looking for could not be found.',

+    'sorry_page_not_found_permission_warning' => 'If you expected this page to exist, you might not have permission to view it.',

     'return_home' => 'Return to home',
     'error_occurred' => 'An Error Occurred',
     'app_down' => ':appName is down right now',
     'return_home' => 'Return to home',
     'error_occurred' => 'An Error Occurred',
     'app_down' => ':appName is down right now',

diff --git a/resources/views/common/header.blade.php b/resources/views/common/header.blade.php
index 3004acf3c0a2f8f157caad1229b7b6c75cfb61cc..ec90739ee497f8dfd41155ab90cae76f547f153a 100644 (file)
--- a/resources/views/common/header.blade.php
+++ b/resources/views/common/header.blade.php
@@ -45,7 +45,7 @@
                         @if(setting('registration-enabled') && config('auth.method') === 'standard')
                             <a href="{{ url('/register') }}">@icon('new-user'){{ trans('auth.sign_up') }}</a>
                         @endif
                         @if(setting('registration-enabled') && config('auth.method') === 'standard')
                             <a href="{{ url('/register') }}">@icon('new-user'){{ trans('auth.sign_up') }}</a>
                         @endif

-                        <a href="{{ action('Auth\LoginController@getLogin', ['intended' => url()->current()]) }}">@icon('login'){{ trans('auth.log_in') }}</a>
+                        <a href="{{ url('/login')  }}">@icon('login'){{ trans('auth.log_in') }}</a>

                     @endif
                 </div>
                 @if(signedInUser())
                     @endif
                 </div>
                 @if(signedInUser())

diff --git a/resources/views/errors/404.blade.php b/resources/views/errors/404.blade.php
index 9c599307ed0279c22de797ea8790717aa91cadf8..02f97fc546fcbc195ec04421731fba44b15cfb1d 100644 (file)
--- a/resources/views/errors/404.blade.php
+++ b/resources/views/errors/404.blade.php
@@ -3,13 +3,17 @@
 @section('content')
 <div class="container mt-l">
 
 @section('content')
 <div class="container mt-l">
 

-    <div class="card mb-xl px-l pb-xl pt-l">
+    <div class="card mb-xl px-l pb-l pt-l">

         <div class="grid half v-center">
             <div>
                 <h1 class="list-heading">{{ $message ?? trans('errors.404_page_not_found') }}</h1>
                 <h5>{{ trans('errors.sorry_page_not_found') }}</h5>
         <div class="grid half v-center">
             <div>
                 <h1 class="list-heading">{{ $message ?? trans('errors.404_page_not_found') }}</h1>
                 <h5>{{ trans('errors.sorry_page_not_found') }}</h5>

+                <p>{{ trans('errors.sorry_page_not_found_permission_warning') }}</p>

             </div>
             <div class="text-right">
             </div>
             <div class="text-right">

+                @if(!signedInUser())
+                    <a href="{{ url('/login') }}" class="button outline">{{ trans('auth.log_in') }}</a>
+                @endif

                 <a href="{{ url('/') }}" class="button outline">{{ trans('errors.return_home') }}</a>
             </div>
         </div>
                 <a href="{{ url('/') }}" class="button outline">{{ trans('errors.return_home') }}</a>
             </div>
         </div>

diff --git a/tests/PublicActionTest.php b/tests/PublicActionTest.php
index 27b4822fa2bce01f5379b50e3558333c01846003..3670df87d39df58e28ea7383955e3f9db473ab7f 100644 (file)
--- a/tests/PublicActionTest.php
+++ b/tests/PublicActionTest.php
@@ -1,16 +1,25 @@
 <?php namespace Tests;
 
 <?php namespace Tests;
 

+use Auth;
+use BookStack\Auth\Permissions\PermissionService;
+use BookStack\Auth\Permissions\RolePermission;
+use BookStack\Auth\Role;
+use BookStack\Auth\User;
+use BookStack\Entities\Book;
+use BookStack\Entities\Chapter;
+use BookStack\Entities\Page;
+

 class PublicActionTest extends BrowserKitTest
 {
 
     public function test_app_not_public()
     {
         $this->setSettings(['app-public' => 'false']);
 class PublicActionTest extends BrowserKitTest
 {
 
     public function test_app_not_public()
     {
         $this->setSettings(['app-public' => 'false']);

-        $book = \BookStack\Entities\Book::orderBy('name', 'asc')->first();
+        $book = Book::orderBy('name', 'asc')->first();

         $this->visit('/books')->seePageIs('/login');
         $this->visit($book->getUrl())->seePageIs('/login');
 
         $this->visit('/books')->seePageIs('/login');
         $this->visit($book->getUrl())->seePageIs('/login');
 

-        $page = \BookStack\Entities\Page::first();
+        $page = Page::first();

         $this->visit($page->getUrl())->seePageIs('/login');
     }
 
         $this->visit($page->getUrl())->seePageIs('/login');
     }
 


@@ -35,7 +44,7 @@ class PublicActionTest extends BrowserKitTest
     public function test_books_viewable()
     {
         $this->setSettings(['app-public' => 'true']);
     public function test_books_viewable()
     {
         $this->setSettings(['app-public' => 'true']);

-        $books = \BookStack\Entities\Book::orderBy('name', 'asc')->take(10)->get();
+        $books = Book::orderBy('name', 'asc')->take(10)->get();

         $bookToVisit = $books[1];
 
         // Check books index page is showing
         $bookToVisit = $books[1];
 
         // Check books index page is showing


@@ -52,7 +61,7 @@ class PublicActionTest extends BrowserKitTest
     public function test_chapters_viewable()
     {
         $this->setSettings(['app-public' => 'true']);
     public function test_chapters_viewable()
     {
         $this->setSettings(['app-public' => 'true']);

-        $chapterToVisit = \BookStack\Entities\Chapter::first();
+        $chapterToVisit = Chapter::first();

         $pageToVisit = $chapterToVisit->pages()->first();
 
         // Check chapters index page is showing
         $pageToVisit = $chapterToVisit->pages()->first();
 
         // Check chapters index page is showing


@@ -70,15 +79,15 @@ class PublicActionTest extends BrowserKitTest
     public function test_public_page_creation()
     {
         $this->setSettings(['app-public' => 'true']);
     public function test_public_page_creation()
     {
         $this->setSettings(['app-public' => 'true']);

-        $publicRole = \BookStack\Auth\Role::getSystemRole('public');
+        $publicRole = Role::getSystemRole('public');

         // Grant all permissions to public
         $publicRole->permissions()->detach();
         // Grant all permissions to public
         $publicRole->permissions()->detach();

-        foreach (\BookStack\Auth\Permissions\RolePermission::all() as $perm) {
+        foreach (RolePermission::all() as $perm) {

             $publicRole->attachPermission($perm);
         }
             $publicRole->attachPermission($perm);
         }

-        $this->app[\BookStack\Auth\Permissions\PermissionService::class]->buildJointPermissionForRole($publicRole);
+        $this->app[PermissionService::class]->buildJointPermissionForRole($publicRole);

 
 

-        $chapter = \BookStack\Entities\Chapter::first();
+        $chapter = Chapter::first();

         $this->visit($chapter->book->getUrl());
         $this->visit($chapter->getUrl())
             ->click('New Page')
         $this->visit($chapter->book->getUrl());
         $this->visit($chapter->getUrl())
             ->click('New Page')


@@ -89,7 +98,7 @@ class PublicActionTest extends BrowserKitTest
             'name' => 'My guest page'
         ])->seePageIs($chapter->book->getUrl('/page/my-guest-page/edit'));
 
             'name' => 'My guest page'
         ])->seePageIs($chapter->book->getUrl('/page/my-guest-page/edit'));
 

-        $user = \BookStack\Auth\User::getDefault();
+        $user = User::getDefault();

         $this->seeInDatabase('pages', [
             'name' => 'My guest page',
             'chapter_id' => $chapter->id,
         $this->seeInDatabase('pages', [
             'name' => 'My guest page',
             'chapter_id' => $chapter->id,


@@ -100,9 +109,9 @@ class PublicActionTest extends BrowserKitTest
 
     public function test_content_not_listed_on_404_for_public_users()
     {
 
     public function test_content_not_listed_on_404_for_public_users()
     {

-        $page = \BookStack\Entities\Page::first();
+        $page = Page::first();

         $this->asAdmin()->visit($page->getUrl());
         $this->asAdmin()->visit($page->getUrl());

-        \Auth::logout();
+        Auth::logout();

         view()->share('pageTitle', '');
         $this->forceVisit('/cats/dogs/hippos');
         $this->dontSee($page->name);
         view()->share('pageTitle', '');
         $this->forceVisit('/cats/dogs/hippos');
         $this->dontSee($page->name);


@@ -139,4 +148,36 @@ class PublicActionTest extends BrowserKitTest
         $this->seeText("User-agent: *\nDisallow: /");
     }
 
         $this->seeText("User-agent: *\nDisallow: /");
     }
 

+    public function test_public_view_then_login_redirects_to_previous_content()
+    {
+        $this->setSettings(['app-public' => 'true']);
+        $book = Book::query()->first();
+        $this->visit($book->getUrl())
+            ->see($book->name)
+            ->visit('/login')
+            ->type('[email protected]', '#email')
+            ->type('password', '#password')
+            ->press('Log In')
+            ->seePageUrlIs($book->getUrl());
+    }
+
+    public function test_access_hidden_content_then_login_redirects_to_intended_content()
+    {
+        $this->setSettings(['app-public' => 'true']);
+        $book = Book::query()->first();
+        $this->setEntityRestrictions($book);
+
+        try {
+            $this->visit($book->getUrl());
+        } catch (\Exception $exception) {}
+
+        $this->see('Book not found')
+            ->dontSee($book->name)
+            ->visit('/login')
+            ->type('[email protected]', '#email')
+            ->type('password', '#password')
+            ->press('Log In')
+            ->seePageUrlIs($book->getUrl())
+            ->see($book->name);
+    }

 }
\ No newline at end of file
 }
\ No newline at end of file