diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 26b24f54..3b02033a 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -32,7 +32,8 @@ class Handler extends ExceptionHandler $this->renderable(function (\Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException $exception, $request) { return response()->json([ - 'message' => 'unauthorized', + 'message' => 'forbidden', + 'reason' => $exception->getMessage(), ], 403); }); diff --git a/app/Http/Controllers/Auth/PersonalAccessTokenController.php b/app/Http/Controllers/Auth/PersonalAccessTokenController.php new file mode 100644 index 00000000..2057dc55 --- /dev/null +++ b/app/Http/Controllers/Auth/PersonalAccessTokenController.php @@ -0,0 +1,59 @@ +|\Illuminate\Http\JsonResponse + */ + public function forUser(Request $request) + { + if (Gate::denies('manage-pat')) { + throw new AccessDeniedHttpException(__('errors.unsupported_with_sso_only')); + } + + return parent::forUser($request); + } + + /** + * Create a new personal access token for the user. + * + * @param \Illuminate\Http\Request $request + * @return \Laravel\Passport\PersonalAccessTokenResult|\Illuminate\Http\JsonResponse + */ + public function store(Request $request) + { + if (Gate::denies('manage-pat')) { + throw new AccessDeniedHttpException(__('errors.unsupported_with_sso_only')); + } + + return parent::store($request); + } + + /** + * Delete the given token. + * + * @param \Illuminate\Http\Request $request + * @param string $tokenId + * @return \Illuminate\Http\Response|\Illuminate\Http\JsonResponse + */ + public function destroy(Request $request, $tokenId) + { + if (Gate::denies('manage-pat')) { + throw new AccessDeniedHttpException(__('errors.unsupported_with_sso_only')); + } + + return parent::destroy($request, $tokenId); + } + + +} \ No newline at end of file diff --git a/app/Http/Controllers/Auth/WebAuthnManageController.php b/app/Http/Controllers/Auth/WebAuthnManageController.php index 27180b11..90bfce93 100644 --- a/app/Http/Controllers/Auth/WebAuthnManageController.php +++ b/app/Http/Controllers/Auth/WebAuthnManageController.php @@ -5,7 +5,9 @@ namespace App\Http\Controllers\Auth; use App\Http\Controllers\Controller; use App\Http\Requests\WebauthnRenameRequest; use Illuminate\Http\Request; +use Illuminate\Support\Facades\Gate; use Illuminate\Support\Facades\Log; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; class WebAuthnManageController extends Controller { @@ -16,6 +18,10 @@ class WebAuthnManageController extends Controller */ public function index(Request $request) { + if (Gate::denies('manage-webauthn-credentials')) { + throw new AccessDeniedHttpException(__('errors.unsupported_with_sso_only')); + } + $allUserCredentials = $request->user()->webAuthnCredentials()->WhereEnabled()->get(); return response()->json($allUserCredentials, 200); @@ -46,6 +52,10 @@ class WebAuthnManageController extends Controller public function delete(Request $request, $credential) { Log::info('Deletion of security device requested'); + + if (Gate::denies('manage-webauthn-credentials')) { + throw new AccessDeniedHttpException(__('errors.unsupported_with_sso_only')); + } $user = $request->user(); $user->flushCredential($credential); diff --git a/app/Http/Controllers/Auth/WebAuthnRegisterController.php b/app/Http/Controllers/Auth/WebAuthnRegisterController.php index 3ab1fe6d..78b52d59 100644 --- a/app/Http/Controllers/Auth/WebAuthnRegisterController.php +++ b/app/Http/Controllers/Auth/WebAuthnRegisterController.php @@ -3,19 +3,19 @@ namespace App\Http\Controllers\Auth; use App\Http\Controllers\Controller; +use App\Http\Requests\WebauthnAttestationRequest; +use App\Http\Requests\WebauthnAttestedRequest; use Illuminate\Contracts\Support\Responsable; use Illuminate\Http\Response; use Illuminate\Support\Facades\Log; use Laragear\WebAuthn\Enums\UserVerification; -use Laragear\WebAuthn\Http\Requests\AttestationRequest; -use Laragear\WebAuthn\Http\Requests\AttestedRequest; class WebAuthnRegisterController extends Controller { /** * Returns a challenge to be verified by the user device. */ - public function options(AttestationRequest $request) : Responsable + public function options(WebauthnAttestationRequest $request) : Responsable { switch (config('webauthn.user_verification')) { case UserVerification::DISCOURAGED: @@ -35,7 +35,7 @@ class WebAuthnRegisterController extends Controller /** * Registers a device for further WebAuthn authentication. */ - public function register(AttestedRequest $request) : Response + public function register(WebauthnAttestedRequest $request) : Response { $request->save(); diff --git a/app/Http/Controllers/SinglePageController.php b/app/Http/Controllers/SinglePageController.php index deaf3dde..d8d26a50 100644 --- a/app/Http/Controllers/SinglePageController.php +++ b/app/Http/Controllers/SinglePageController.php @@ -29,6 +29,7 @@ class SinglePageController extends Controller 'disableRegistration', 'enableSso', 'useSsoOnly', + 'allowPatWhileSsoOnly', ]); $settings = $appSettings->map(function (mixed $item, string $key) { return null; diff --git a/app/Http/Requests/WebauthnAttestationRequest.php b/app/Http/Requests/WebauthnAttestationRequest.php new file mode 100644 index 00000000..f193a643 --- /dev/null +++ b/app/Http/Requests/WebauthnAttestationRequest.php @@ -0,0 +1,31 @@ +isAdministrator()) { + return true; + } + }); + + Gate::define('manage-pat', function (User $user) { + $useSsoOnly = Settings::get('useSsoOnly'); + + return ($useSsoOnly && Settings::get('allowPatWhileSsoOnly')) || $useSsoOnly !== true; + }); + + Gate::define('manage-webauthn-credentials', function (User $user) { + return ! Settings::get('useSsoOnly'); + }); } } diff --git a/config/2fauth.php b/config/2fauth.php index 2aa02ccc..8c949c40 100644 --- a/config/2fauth.php +++ b/config/2fauth.php @@ -139,6 +139,7 @@ return [ 'disableRegistration' => false, 'enableSso' => true, 'useSsoOnly' => false, + 'allowPatWhileSsoOnly' => false, 'restrictRegistration' => false, 'restrictList' => '', 'restrictRule' => '', diff --git a/resources/js/views/admin/Auth.vue b/resources/js/views/admin/Auth.vue index 8d35a76a..428194af 100644 --- a/resources/js/views/admin/Auth.vue +++ b/resources/js/views/admin/Auth.vue @@ -80,6 +80,8 @@ + +

{{ $t('admin.registrations') }}

diff --git a/resources/js/views/settings/OAuth.vue b/resources/js/views/settings/OAuth.vue index 79d37423..1139b2cd 100644 --- a/resources/js/views/settings/OAuth.vue +++ b/resources/js/views/settings/OAuth.vue @@ -2,11 +2,13 @@ import Form from '@/components/formElements/Form' import userService from '@/services/userService' import SettingTabs from '@/layouts/SettingTabs.vue' + import { useAppSettingsStore } from '@/stores/appSettings' import { useNotifyStore } from '@/stores/notify' import { UseColorMode } from '@vueuse/components' import { useUserStore } from '@/stores/user' import Spinner from '@/components/Spinner.vue' + const appSettings = useAppSettingsStore() const $2fauth = inject('2fauth') const notify = useNotifyStore() const user = useUserStore() @@ -20,7 +22,7 @@ const visibleTokenId = ref(null) const isDisabled = computed(() => { - return (appSettings.enableSso && appSettings.useSsoOnly) || user.authenticated_by_proxy + return (appSettings.enableSso && appSettings.useSsoOnly && ! appSettings.allowPatWhileSsoOnly) || user.authenticated_by_proxy }) onMounted(() => { @@ -51,8 +53,8 @@ }) }) .catch(error => { - if( error.response.status === 405 ) { - // The backend returns a 405 response if the user is authenticated by a reverse proxy + if( error.response.status === 403 ) { + // The backend returns a 403 response if the user is authenticated by a reverse proxy // or if SSO only is enabled. // The form is already disabled (see isDisabled) so we do nothing more here } diff --git a/resources/js/views/settings/WebAuthn.vue b/resources/js/views/settings/WebAuthn.vue index 3bf47b71..eabd8156 100644 --- a/resources/js/views/settings/WebAuthn.vue +++ b/resources/js/views/settings/WebAuthn.vue @@ -98,8 +98,8 @@ credentials.value = response.data }) .catch(error => { - if( error.response.status === 405 ) { - // The backend returns a 405 response if the user is authenticated by a reverse proxy + if( error.response.status === 403 ) { + // The backend returns a 403 response if the user is authenticated by a reverse proxy // or if SSO only is enabled. // The form is already disabled (see isDisabled) so we do nothing more here } diff --git a/resources/lang/en/admin.php b/resources/lang/en/admin.php index 584e5cbb..2f7e93b7 100644 --- a/resources/lang/en/admin.php +++ b/resources/lang/en/admin.php @@ -118,6 +118,10 @@ return [ 'label' => 'Use SSO only', 'help' => 'Make SSO the only available method to log in to 2FAuth. Password login and Webauthn are then disabled for regular users. Administrators are not affected by this restriction.', ], + 'allow_pat_in_sso_only' => [ + 'label' => 'Allow PAT usage', + 'help' => 'Let users create personal access tokens and use them to authenticate with third party application like the 2FAuth web extension', + ], 'keep_sso_registration_enabled' => [ 'label' => 'Keep SSO registration enabled', 'help' => 'Allow new users to sign in for the first time via SSO whereas registration is disabled', diff --git a/routes/web.php b/routes/web.php index 44878b49..614a5620 100644 --- a/routes/web.php +++ b/routes/web.php @@ -3,6 +3,7 @@ use App\Http\Controllers\Auth\ForgotPasswordController; use App\Http\Controllers\Auth\LoginController; use App\Http\Controllers\Auth\PasswordController; +use App\Http\Controllers\Auth\PersonalAccessTokenController; use App\Http\Controllers\Auth\RegisterController; use App\Http\Controllers\Auth\ResetPasswordController; use App\Http\Controllers\Auth\SocialiteController; @@ -23,7 +24,6 @@ use Illuminate\Session\Middleware\StartSession; // use Illuminate\Foundation\Events\DiagnosingHealth; // use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Route; -use Laravel\Passport\Http\Controllers\PersonalAccessTokenController; // use App\Models\User; // use App\Notifications\SignedInWithNewDeviceNotification; @@ -67,7 +67,7 @@ Route::group(['middleware' => ['forceLogout', 'throttle:10,1']], function () { /** * Routes protected by an authentication guard but rejected when the reverse-proxy - * guard is enabled or SSO only is enabled + * guard is enabled */ Route::group(['middleware' => ['behind-auth', 'rejectIfReverseProxy']], function () { Route::put('user', [UserController::class, 'update'])->name('user.update'); @@ -75,15 +75,16 @@ Route::group(['middleware' => ['behind-auth', 'rejectIfReverseProxy']], function Route::get('user/logout', [LoginController::class, 'logout'])->name('user.logout'); Route::delete('user', [UserController::class, 'delete'])->name('user.delete')->middleware('rejectIfDemoMode'); - Route::get('oauth/personal-access-tokens', [PersonalAccessTokenController::class, 'forUser'])->name('passport.personal.tokens.index')->middleware('RejectIfSsoOnlyAndNotForAdmin'); - Route::post('oauth/personal-access-tokens', [PersonalAccessTokenController::class, 'store'])->name('passport.personal.tokens.store')->middleware('RejectIfSsoOnlyAndNotForAdmin'); - Route::delete('oauth/personal-access-tokens/{token_id}', [PersonalAccessTokenController::class, 'destroy'])->name('passport.personal.tokens.destroy')->middleware('RejectIfSsoOnlyAndNotForAdmin'); + // Following routes are also forbidden to regular users when "SSO only" is enabled, but using Authorization gates + Route::get('oauth/personal-access-tokens', [PersonalAccessTokenController::class, 'forUser'])->name('passport.personal.tokens.index'); + Route::post('oauth/personal-access-tokens', [PersonalAccessTokenController::class, 'store'])->name('passport.personal.tokens.store'); + Route::delete('oauth/personal-access-tokens/{token_id}', [PersonalAccessTokenController::class, 'destroy'])->name('passport.personal.tokens.destroy'); - Route::post('webauthn/register/options', [WebAuthnRegisterController::class, 'options'])->name('webauthn.register.options')->middleware('RejectIfSsoOnlyAndNotForAdmin'); - Route::post('webauthn/register', [WebAuthnRegisterController::class, 'register'])->name('webauthn.register')->middleware('RejectIfSsoOnlyAndNotForAdmin'); - Route::get('webauthn/credentials', [WebAuthnManageController::class, 'index'])->name('webauthn.credentials.index')->middleware('RejectIfSsoOnlyAndNotForAdmin'); - Route::patch('webauthn/credentials/{credential}/name', [WebAuthnManageController::class, 'rename'])->name('webauthn.credentials.rename')->middleware('RejectIfSsoOnlyAndNotForAdmin'); - Route::delete('webauthn/credentials/{credential}', [WebAuthnManageController::class, 'delete'])->name('webauthn.credentials.delete')->middleware('RejectIfSsoOnlyAndNotForAdmin'); + Route::post('webauthn/register/options', [WebAuthnRegisterController::class, 'options'])->name('webauthn.register.options'); + Route::post('webauthn/register', [WebAuthnRegisterController::class, 'register'])->name('webauthn.register'); + Route::get('webauthn/credentials', [WebAuthnManageController::class, 'index'])->name('webauthn.credentials.index'); + Route::patch('webauthn/credentials/{credential}/name', [WebAuthnManageController::class, 'rename'])->name('webauthn.credentials.rename'); + Route::delete('webauthn/credentials/{credential}', [WebAuthnManageController::class, 'delete'])->name('webauthn.credentials.delete'); }); /** diff --git a/tests/Feature/Http/Middlewares/RejectIfSsoOnlyAndNotForAdminMiddlewareTest.php b/tests/Feature/Http/Middlewares/RejectIfSsoOnlyAndNotForAdminMiddlewareTest.php index 310008f0..bb868fea 100644 --- a/tests/Feature/Http/Middlewares/RejectIfSsoOnlyAndNotForAdminMiddlewareTest.php +++ b/tests/Feature/Http/Middlewares/RejectIfSsoOnlyAndNotForAdminMiddlewareTest.php @@ -133,74 +133,4 @@ class RejectIfSsoOnlyAndNotForAdminMiddlewareTest extends FeatureTestCase ], ]; } - - #[Test] - #[DataProvider('provideProtectedEndPoints')] - public function test_protected_endpoint_are_allowed_if_requested_by_an_admin(string $method, string $url) - { - $expectedResponseCodes = [ - Response::HTTP_OK, - Response::HTTP_UNPROCESSABLE_ENTITY, - Response::HTTP_NOT_FOUND, - Response::HTTP_CREATED, - Response::HTTP_NO_CONTENT, - ]; - - $response = $this->actingAs($this->admin, 'web-guard') - ->json($method, $url, [ - 'email' => $this->admin->email, - ]); - - $this->assertContains($response->getStatusCode(), $expectedResponseCodes); - } - - #[Test] - #[DataProvider('provideProtectedEndPoints')] - public function test_protected_endpoint_returns_NOT_ALLOWED_if_requested_by_regular_user(string $method, string $url) - { - $this->actingAs($this->user, 'web-guard') - ->json($method, $url) - ->assertMethodNotAllowed(); - } - - /** - * Provide Valid data for validation test - */ - public static function provideProtectedEndPoints() : array - { - return [ - 'WEBAUTHN_REGISTER' => [ - 'method' => 'POST', - 'url' => '/webauthn/register', - ], - 'WEBAUTHN_REGISTER_OPTIONS' => [ - 'method' => 'POST', - 'url' => '/webauthn/register/options', - ], - 'WEBAUTHN_CREDENTIALS_ALL' => [ - 'method' => 'GET', - 'url' => '/webauthn/credentials', - ], - 'WEBAUTHN_CREDENTIALS_PATCH' => [ - 'method' => 'PATCH', - 'url' => '/webauthn/credentials/FAKE_CREDENTIAL_ID/name', - ], - 'WEBAUTHN_CREDENTIALS_DELETE' => [ - 'method' => 'DELETE', - 'url' => '/webauthn/credentials/FAKE_CREDENTIAL_ID', - ], - 'OAUTH_PAT_ALL' => [ - 'method' => 'GET', - 'url' => '/oauth/personal-access-tokens', - ], - 'OAUTH_PAT_STORE' => [ - 'method' => 'POST', - 'url' => '/oauth/personal-access-tokens', - ], - 'OAUTH_PAT_DELETE' => [ - 'method' => 'DELETE', - 'url' => '/oauth/personal-access-tokens/FAKE_TOKEN_ID', - ], - ]; - } } diff --git a/tests/Feature/Permissions/ManagePatPermissionsTest.php b/tests/Feature/Permissions/ManagePatPermissionsTest.php new file mode 100644 index 00000000..7612b5ce --- /dev/null +++ b/tests/Feature/Permissions/ManagePatPermissionsTest.php @@ -0,0 +1,142 @@ +user = User::factory()->create(); + $this->admin = User::factory()->administrator()->create([ + 'password' => self::PASSWORD, + ]); + } + + #[Test] + #[DataProvider('providePatManagementEndPoints')] + public function test_pat_management_endpoint_is_permitted_to_regular_user_without_useSsoOnly(string $method, string $url) + { + Settings::set('useSsoOnly', false); + Settings::set('allowPatWhileSsoOnly', false); + + $response = $this->actingAs($this->user, 'web-guard') + ->json($method, $url); + + $this->assertNotEquals($response->getStatusCode(), Response::HTTP_FORBIDDEN); + } + + #[Test] + #[DataProvider('providePatManagementEndPoints')] + public function test_pat_management_endpoint_is_forbidden_to_regular_user_with_useSsoOnly(string $method, string $url) + { + Settings::set('useSsoOnly', true); + Settings::set('allowPatWhileSsoOnly', false); + + $this->actingAs($this->user, 'web-guard') + ->json($method, $url) + ->assertForbidden(); + } + + #[Test] + #[DataProvider('providePatManagementEndPoints')] + public function test_pat_management_endpoint_is_permitted_to_regular_user_with_useSsoOnly_bypassed(string $method, string $url) + { + Settings::set('useSsoOnly', true); + Settings::set('allowPatWhileSsoOnly', true); + + $response = $this->actingAs($this->user, 'web-guard') + ->json($method, $url); + + $this->assertNotEquals($response->getStatusCode(), Response::HTTP_FORBIDDEN); + } + + #[Test] + #[DataProvider('providePatManagementEndPoints')] + public function test_pat_management_endpoint_is_permitted_to_admin_without_useSsoOnly(string $method, string $url) + { + Settings::set('useSsoOnly', false); + Settings::set('allowPatWhileSsoOnly', false); + + $response = $this->actingAs($this->admin, 'web-guard') + ->json($method, $url); + + $this->assertNotEquals($response->getStatusCode(), Response::HTTP_FORBIDDEN); + } + + #[Test] + #[DataProvider('providePatManagementEndPoints')] + public function test_pat_management_endpoint_is_permitted_to_admin_with_useSsoOnly(string $method, string $url) + { + Settings::set('useSsoOnly', true); + Settings::set('allowPatWhileSsoOnly', false); + + $response = $this->actingAs($this->admin, 'web-guard') + ->json($method, $url); + + $this->assertNotEquals($response->getStatusCode(), Response::HTTP_FORBIDDEN); + } + + #[Test] + #[DataProvider('providePatManagementEndPoints')] + public function test_pat_management_endpoint_is_permitted_to_admin_with_useSsoOnly_bypassed(string $method, string $url) + { + Settings::set('useSsoOnly', true); + Settings::set('allowPatWhileSsoOnly', true); + + $response = $this->actingAs($this->admin, 'web-guard') + ->json($method, $url); + + $this->assertNotEquals($response->getStatusCode(), Response::HTTP_FORBIDDEN); + } + + /** + * Provide Valid data for validation test + */ + public static function providePatManagementEndPoints() : array + { + return [ + 'OAUTH_PAT_ALL' => [ + 'method' => 'GET', + 'url' => '/oauth/personal-access-tokens', + ], + 'OAUTH_PAT_STORE' => [ + 'method' => 'POST', + 'url' => '/oauth/personal-access-tokens', + ], + 'OAUTH_PAT_DELETE' => [ + 'method' => 'DELETE', + 'url' => '/oauth/personal-access-tokens/FAKE_TOKEN_ID', + ], + ]; + } +} diff --git a/tests/Feature/Permissions/ManageWebauthnPermissionsTest.php b/tests/Feature/Permissions/ManageWebauthnPermissionsTest.php new file mode 100644 index 00000000..884b7264 --- /dev/null +++ b/tests/Feature/Permissions/ManageWebauthnPermissionsTest.php @@ -0,0 +1,122 @@ +user = User::factory()->create(); + $this->admin = User::factory()->administrator()->create([ + 'password' => self::PASSWORD, + ]); + } + + #[Test] + #[DataProvider('provideWebauthnManagementEndPoints')] + public function test_webauthn_management_endpoint_is_permitted_to_regular_user_without_useSsoOnly(string $method, string $url) + { + Settings::set('useSsoOnly', false); + + $response = $this->actingAs($this->user, 'web-guard') + ->json($method, $url); + + $this->assertNotEquals($response->getStatusCode(), Response::HTTP_FORBIDDEN); + } + + #[Test] + #[DataProvider('provideWebauthnManagementEndPoints')] + public function test_webauthn_management_endpoint_is_forbidden_to_regular_user_with_useSsoOnly(string $method, string $url) + { + Settings::set('useSsoOnly', true); + + $this->actingAs($this->user, 'web-guard') + ->json($method, $url) + ->assertForbidden(); + } + + #[Test] + #[DataProvider('provideWebauthnManagementEndPoints')] + public function test_webauthn_management_endpoint_is_permitted_to_admin_without_useSsoOnly(string $method, string $url) + { + Settings::set('useSsoOnly', false); + + $response = $this->actingAs($this->admin, 'web-guard') + ->json($method, $url); + + $this->assertNotEquals($response->getStatusCode(), Response::HTTP_FORBIDDEN); + } + + #[Test] + #[DataProvider('provideWebauthnManagementEndPoints')] + public function test_webauthn_management_endpoint_is_permitted_to_admin_with_useSsoOnly(string $method, string $url) + { + Settings::set('useSsoOnly', true); + + $response = $this->actingAs($this->admin, 'web-guard') + ->json($method, $url); + + $this->assertNotEquals($response->getStatusCode(), Response::HTTP_FORBIDDEN); + } + + /** + * Provide Valid data for validation test + */ + public static function provideWebauthnManagementEndPoints() : array + { + return [ + 'WEBAUTHN_REGISTER_OPTIONS' => [ + 'method' => 'POST', + 'url' => '/webauthn/register/options', + ], + 'WEBAUTHN_REGISTER' => [ + 'method' => 'POST', + 'url' => '/webauthn/register', + ], + 'WEBAUTHN_CREDENTIALS_ALL' => [ + 'method' => 'GET', + 'url' => '/webauthn/credentials', + ], + 'WEBAUTHN_CREDENTIALS_PATCH' => [ + 'method' => 'PATCH', + 'url' => '/webauthn/credentials/FAKE_CREDENTIAL_ID/name', + ], + 'WEBAUTHN_CREDENTIALS_DELETE' => [ + 'method' => 'DELETE', + 'url' => '/webauthn/credentials/FAKE_CREDENTIAL_ID', + ], + ]; + } +}