From d5ce6b680cbf50ddfa5ede2c680f8546498052ac Mon Sep 17 00:00:00 2001 From: Robert Meredith Date: Mon, 2 May 2022 20:35:11 +1000 Subject: [PATCH 01/67] Skip intermediate login page with single provider --- app/Config/auth.php | 5 ++++ app/Http/Controllers/Auth/LoginController.php | 27 ++++++++++++++++--- resources/views/auth/login-redirect.blade.php | 16 +++++++++++ tests/Auth/OidcTest.php | 14 ++++++++++ 4 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 resources/views/auth/login-redirect.blade.php diff --git a/app/Config/auth.php b/app/Config/auth.php index 1e1a9d350..ec4389a6c 100644 --- a/app/Config/auth.php +++ b/app/Config/auth.php @@ -13,6 +13,11 @@ return [ // Options: standard, ldap, saml2, oidc 'method' => env('AUTH_METHOD', 'standard'), + // Automatically redirect to external login provider if only one provider is being used + // instead of displaying a single-button login page and requiring users to click through + // Supported methods: saml2, oidc + 'auto_redirect' => env('AUTH_AUTO_REDIRECT', false), + // Authentication Defaults // This option controls the default authentication "guard" and password // reset options for your application. diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 742e10472..695bfa28d 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -25,14 +25,14 @@ class LoginController extends Controller | */ - use AuthenticatesUsers; + use AuthenticatesUsers { logout as traitLogout; } /** * Redirection paths. */ protected $redirectTo = '/'; protected $redirectPath = '/'; - protected $redirectAfterLogout = '/login'; + protected $redirectAfterLogout = '/'; protected $socialAuthService; protected $loginService; @@ -50,7 +50,7 @@ class LoginController extends Controller $this->loginService = $loginService; $this->redirectPath = url('/'); - $this->redirectAfterLogout = url('/login'); + $this->redirectAfterLogout = url(config('auth.auto_redirect') ? '/login?logout=1' : '/'); } public function username() @@ -73,6 +73,7 @@ class LoginController extends Controller { $socialDrivers = $this->socialAuthService->getActiveDrivers(); $authMethod = config('auth.method'); + $autoRedirect = config('auth.auto_redirect'); if ($request->has('email')) { session()->flashInput([ @@ -84,6 +85,12 @@ class LoginController extends Controller // Store the previous location for redirect after login $this->updateIntendedFromPrevious(); + if ($autoRedirect && !($request->has('logout') && $request->get('logout') == '1') && count($socialDrivers) == 0 && in_array($authMethod, ['oidc', 'saml2'])) { + return view('auth.login-redirect', [ + 'authMethod' => $authMethod, + ]); + } + return view('auth.login', [ 'socialDrivers' => $socialDrivers, 'authMethod' => $authMethod, @@ -251,4 +258,18 @@ class LoginController extends Controller redirect()->setIntendedUrl($previous); } + + /** + * Logout user and perform subsequent redirect. + * + * @param \Illuminate\Http\Request $request + * + * @return mixed + */ + public function logout(Request $request) + { + $this->traitLogout($request); + + return redirect($this->redirectAfterLogout); + } } diff --git a/resources/views/auth/login-redirect.blade.php b/resources/views/auth/login-redirect.blade.php new file mode 100644 index 000000000..08501c6b3 --- /dev/null +++ b/resources/views/auth/login-redirect.blade.php @@ -0,0 +1,16 @@ + + + + + + + + + + + diff --git a/tests/Auth/OidcTest.php b/tests/Auth/OidcTest.php index 9aebb4d04..34fd70115 100644 --- a/tests/Auth/OidcTest.php +++ b/tests/Auth/OidcTest.php @@ -26,6 +26,7 @@ class OidcTest extends TestCase config()->set([ 'auth.method' => 'oidc', + 'auth.auto_redirect' => false, 'auth.defaults.guard' => 'oidc', 'oidc.name' => 'SingleSignOn-Testing', 'oidc.display_name_claims' => ['name'], @@ -111,6 +112,19 @@ class OidcTest extends TestCase $this->assertPermissionError($resp); } + public function test_automatic_redirect_on_login() + { + config()->set([ + 'auth.auto_redirect' => true, + 'services.google.client_id' => false, + 'services.github.client_id' => false, + ]); + $req = $this->get('/login'); + $req->assertSeeText('SingleSignOn-Testing'); + $req->assertElementExists('form[action$="/oidc/login"][method=POST] button'); + $req->assertElementExists('div#loginredirect-wrapper'); + } + public function test_login() { $req = $this->post('/oidc/login'); From d795af04dfa69fe1fc95f13aa0b0d43f3bdac268 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 4 May 2022 21:03:13 +0100 Subject: [PATCH 02/67] Added ability to escape role "External Auth ID" commas - Using a backslash in this field before a comma. - Could potentially (Although unlikely) be a breaking change. For #3405 --- app/Auth/Access/GroupSyncService.php | 18 ++++++-- database/factories/Auth/RoleFactory.php | 1 + tests/Auth/GroupSyncServiceTest.php | 59 +++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 4 deletions(-) create mode 100644 tests/Auth/GroupSyncServiceTest.php diff --git a/app/Auth/Access/GroupSyncService.php b/app/Auth/Access/GroupSyncService.php index db19b007a..74f0539d8 100644 --- a/app/Auth/Access/GroupSyncService.php +++ b/app/Auth/Access/GroupSyncService.php @@ -28,10 +28,8 @@ class GroupSyncService */ protected function externalIdMatchesGroupNames(string $externalId, array $groupNames): bool { - $externalAuthIds = explode(',', strtolower($externalId)); - - foreach ($externalAuthIds as $externalAuthId) { - if (in_array(trim($externalAuthId), $groupNames)) { + foreach ($this->parseRoleExternalAuthId($externalId) as $externalAuthId) { + if (in_array($externalAuthId, $groupNames)) { return true; } } @@ -39,6 +37,18 @@ class GroupSyncService return false; } + protected function parseRoleExternalAuthId(string $externalId): array + { + $inputIds = preg_split('/(? $this->faker->sentence(3), 'description' => $this->faker->sentence(10), + 'external_auth_id' => '', ]; } } diff --git a/tests/Auth/GroupSyncServiceTest.php b/tests/Auth/GroupSyncServiceTest.php new file mode 100644 index 000000000..ee8dee008 --- /dev/null +++ b/tests/Auth/GroupSyncServiceTest.php @@ -0,0 +1,59 @@ +getViewer(); + + $roleA = Role::factory()->create(['display_name' => 'Wizards']); + $roleB = Role::factory()->create(['display_name' => 'Gremlins']); + $roleC = Role::factory()->create(['display_name' => 'ABC123', 'external_auth_id' => 'sales']); + $roleD = Role::factory()->create(['display_name' => 'DEF456', 'external_auth_id' => 'admin-team']); + + foreach([$roleA, $roleB, $roleC, $roleD] as $role) { + $this->assertFalse($user->hasRole($role->id)); + } + + (new GroupSyncService())->syncUserWithFoundGroups($user, ['Wizards', 'Gremlinz', 'Sales', 'Admin Team'], false); + + $user = User::query()->find($user->id); + $this->assertTrue($user->hasRole($roleA->id)); + $this->assertFalse($user->hasRole($roleB->id)); + $this->assertTrue($user->hasRole($roleC->id)); + $this->assertTrue($user->hasRole($roleD->id)); + } + + public function test_multiple_values_in_role_external_auth_id_handled() + { + $user = $this->getViewer(); + $role = Role::factory()->create(['display_name' => 'ABC123', 'external_auth_id' => 'sales, engineering, developers, marketers']); + $this->assertFalse($user->hasRole($role->id)); + + (new GroupSyncService())->syncUserWithFoundGroups($user, ['Developers'], false); + + $user = User::query()->find($user->id); + $this->assertTrue($user->hasRole($role->id)); + } + + public function test_commas_can_be_used_in_external_auth_id_if_escaped() + { + $user = $this->getViewer(); + $role = Role::factory()->create(['display_name' => 'ABC123', 'external_auth_id' => 'sales\,-developers, marketers']); + $this->assertFalse($user->hasRole($role->id)); + + (new GroupSyncService())->syncUserWithFoundGroups($user, ['Sales, Developers'], false); + + $user = User::query()->find($user->id); + $this->assertTrue($user->hasRole($role->id)); + } + +} \ No newline at end of file From f0218232876247e9101abb3ca4591bd0ae4b7a7a Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 11 May 2022 16:46:59 +0100 Subject: [PATCH 03/67] Updated default value for secure session detection Updated default value for APP_URL so that the startsWith call is not passed null, since that causes deprecation notice in PHP8.1. Would show when APP_URL was not set, adding extra confusiion. --- app/Config/session.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Config/session.php b/app/Config/session.php index 4bbb78901..a00d75807 100644 --- a/app/Config/session.php +++ b/app/Config/session.php @@ -72,7 +72,7 @@ return [ // to the server if the browser has a HTTPS connection. This will keep // the cookie from being sent to you if it can not be done securely. 'secure' => env('SESSION_SECURE_COOKIE', null) - ?? Str::startsWith(env('APP_URL'), 'https:'), + ?? Str::startsWith(env('APP_URL', ''), 'https:'), // HTTP Access Only // Setting this value to true will prevent JavaScript from accessing the From bef2045df1c62ca9b2622e924cd8757050acd96f Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 12 May 2022 17:27:29 +0100 Subject: [PATCH 04/67] Embedded css sources for easier firefox dev work --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index b49a2a07f..7a85af47a 100644 --- a/package.json +++ b/package.json @@ -1,8 +1,8 @@ { "private": true, "scripts": { - "build:css:dev": "sass ./resources/sass:./public/dist", - "build:css:watch": "sass ./resources/sass:./public/dist --watch", + "build:css:dev": "sass ./resources/sass:./public/dist --embed-sources", + "build:css:watch": "sass ./resources/sass:./public/dist --watch --embed-sources", "build:css:production": "sass ./resources/sass:./public/dist -s compressed", "build:js:dev": "node dev/build/esbuild.js", "build:js:watch": "chokidar --initial \"./resources/**/*.js\" -c \"npm run build:js:dev\"", From 221d910ff299fec49f343d3d27f66fa03f3f60aa Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 12 May 2022 17:27:57 +0100 Subject: [PATCH 05/67] Reduced excess margin in chapter contents lists --- resources/sass/_lists.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/resources/sass/_lists.scss b/resources/sass/_lists.scss index 26d12a25d..dfde5a282 100644 --- a/resources/sass/_lists.scss +++ b/resources/sass/_lists.scss @@ -235,6 +235,7 @@ display: none; padding-inline-start: 0; position: relative; + margin-bottom: 0; } [chapter-toggle].open + .sub-menu { display: block; @@ -485,6 +486,9 @@ ul.pagination { display: block; white-space: nowrap; } + > .entity-list > .entity-list-item:last-child { + margin-bottom: -$-xs; + } } .entity-list-item-image { From a0fe6147d8a658c21271c54110aec2251f4ed08a Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 13 May 2022 17:12:45 +0100 Subject: [PATCH 06/67] Improved the display of dropdown menus - Tweaked styling to add a little extra shadow and be more rounded to match other UI areas. - Added slight horizontal inset when in right sidebar to prevent shadow being cut-off in most cases. - Added logic to "drop upwards" if dropping down would take the menu offscreen. --- resources/js/components/dropdown.js | 30 ++++++++++++++----- resources/sass/_lists.scss | 10 +++++-- .../views/entities/export-menu.blade.php | 7 ++++- .../pages/parts/editor-toolbar.blade.php | 4 ++- 4 files changed, 40 insertions(+), 11 deletions(-) diff --git a/resources/js/components/dropdown.js b/resources/js/components/dropdown.js index f761ecf01..e84076502 100644 --- a/resources/js/components/dropdown.js +++ b/resources/js/components/dropdown.js @@ -28,18 +28,31 @@ class DropDown { this.menu.classList.add('anim', 'menuIn'); this.toggle.setAttribute('aria-expanded', 'true'); + const menuOriginalRect = this.menu.getBoundingClientRect(); + let heightOffset = 0; + const toggleHeight = this.toggle.getBoundingClientRect().height; + const dropUpwards = menuOriginalRect.bottom > window.innerHeight; + + // If enabled, Move to body to prevent being trapped within scrollable sections if (this.moveMenu) { - // Move to body to prevent being trapped within scrollable sections - this.rect = this.menu.getBoundingClientRect(); this.body.appendChild(this.menu); this.menu.style.position = 'fixed'; if (this.direction === 'right') { - this.menu.style.right = `${(this.rect.right - this.rect.width)}px`; + this.menu.style.right = `${(menuOriginalRect.right - menuOriginalRect.width)}px`; } else { - this.menu.style.left = `${this.rect.left}px`; + this.menu.style.left = `${menuOriginalRect.left}px`; } - this.menu.style.top = `${this.rect.top}px`; - this.menu.style.width = `${this.rect.width}px`; + this.menu.style.width = `${menuOriginalRect.width}px`; + heightOffset = dropUpwards ? (window.innerHeight - menuOriginalRect.top - toggleHeight / 2) : menuOriginalRect.top; + } + + // Adjust menu to display upwards if near the bottom of the screen + if (dropUpwards) { + this.menu.style.top = 'initial'; + this.menu.style.bottom = `${heightOffset}px`; + } else { + this.menu.style.top = `${heightOffset}px`; + this.menu.style.bottom = 'initial'; } // Set listener to hide on mouse leave or window click @@ -74,13 +87,16 @@ class DropDown { this.menu.style.display = 'none'; this.menu.classList.remove('anim', 'menuIn'); this.toggle.setAttribute('aria-expanded', 'false'); + this.menu.style.top = ''; + this.menu.style.bottom = ''; + if (this.moveMenu) { this.menu.style.position = ''; this.menu.style[this.direction] = ''; - this.menu.style.top = ''; this.menu.style.width = ''; this.container.appendChild(this.menu); } + this.showing = false; } diff --git a/resources/sass/_lists.scss b/resources/sass/_lists.scss index dfde5a282..3d36fd7bd 100644 --- a/resources/sass/_lists.scss +++ b/resources/sass/_lists.scss @@ -578,8 +578,8 @@ ul.pagination { right: 0; margin: $-m 0; @include lightDark(background-color, #fff, #333); - box-shadow: 0 2px 4px rgba(0, 0, 0, 0.18); - border-radius: 1px; + box-shadow: 0 1px 6px 0 rgba(0, 0, 0, 0.18); + border-radius: 3px; min-width: 180px; padding: $-xs 0; @include lightDark(color, #555, #eee); @@ -656,6 +656,12 @@ ul.pagination { } } +// Shift in right-sidebar dropdown menus to prevent shadows +// being cut by scrollable container. +.tri-layout-right .dropdown-menu { + right: $-xs; +} + // Books grid view .featured-image-container { position: relative; diff --git a/resources/views/entities/export-menu.blade.php b/resources/views/entities/export-menu.blade.php index dd7231095..bac240b1e 100644 --- a/resources/views/entities/export-menu.blade.php +++ b/resources/views/entities/export-menu.blade.php @@ -1,13 +1,18 @@ -
-