From 0cdd83aabf28807d26dd1a906f8bee1fbd81c2b6 Mon Sep 17 00:00:00 2001 From: johnson-yi <63399474+johnson-yi@users.noreply.github.com> Date: Sat, 21 Nov 2020 13:54:25 +1100 Subject: [PATCH] Fixes #8584, #8654, #8727 - fixes and improvements for saml (#8795) * Let onelogin/php-saml know to use 'X-Forwarded-*' headers if it is from a trusted proxy * Gracefully handle the case where openssl_csr_new fails when openssl.cnf is invalid/missing * Improve ui of saml sp metadata by displaying it's url and a download button --- app/Http/Controllers/Auth/SamlController.php | 6 ++-- app/Http/Requests/SettingsSamlRequest.php | 33 +++++++++++--------- app/Services/Saml.php | 4 +++ resources/lang/en/admin/settings/general.php | 1 + resources/views/settings/saml.blade.php | 7 ++++- 5 files changed, 34 insertions(+), 17 deletions(-) diff --git a/app/Http/Controllers/Auth/SamlController.php b/app/Http/Controllers/Auth/SamlController.php index b5a63a323..dd48ab169 100644 --- a/app/Http/Controllers/Auth/SamlController.php +++ b/app/Http/Controllers/Auth/SamlController.php @@ -53,8 +53,10 @@ class SamlController extends Controller if (empty($metadata)) { return response()->view('errors.403', [], 403); } - - return response($metadata)->header('Content-Type', 'text/xml'); + + return response()->streamDownload(function () use ($metadata) { + echo $metadata; + }, 'snipe-it-metadata.xml', ['Content-Type' => 'text/xml']); } /** diff --git a/app/Http/Requests/SettingsSamlRequest.php b/app/Http/Requests/SettingsSamlRequest.php index f8629a2b4..ece772c5c 100644 --- a/app/Http/Requests/SettingsSamlRequest.php +++ b/app/Http/Requests/SettingsSamlRequest.php @@ -70,22 +70,27 @@ class SettingsSamlRequest extends FormRequest ]); $csr = openssl_csr_new($dn, $pkey, ['digest_alg' => 'sha256']); - - $x509 = openssl_csr_sign($csr, null, $pkey, 3650, ['digest_alg' => 'sha256']); - openssl_x509_export($x509, $x509cert); - openssl_pkey_export($pkey, $privateKey); + if ($csr) { - $errors = []; - while (($error = openssl_error_string() !== false)) { - $errors[] = $error; - } - - if (!(empty($x509cert) && empty($privateKey))) { - $this->merge([ - 'saml_sp_x509cert' => $x509cert, - 'saml_sp_privatekey' => $privateKey, - ]); + $x509 = openssl_csr_sign($csr, null, $pkey, 3650, ['digest_alg' => 'sha256']); + + openssl_x509_export($x509, $x509cert); + openssl_pkey_export($pkey, $privateKey); + + $errors = []; + while (($error = openssl_error_string() !== false)) { + $errors[] = $error; + } + + if (!(empty($x509cert) && empty($privateKey))) { + $this->merge([ + 'saml_sp_x509cert' => $x509cert, + 'saml_sp_privatekey' => $privateKey, + ]); + } + } else { + $validator->errors()->add('saml_integration', 'openssl.cnf is missing/invalid'); } } diff --git a/app/Services/Saml.php b/app/Services/Saml.php index 0c321f645..bb6c24cff 100644 --- a/app/Services/Saml.php +++ b/app/Services/Saml.php @@ -5,6 +5,7 @@ namespace App\Services; use OneLogin\Saml2\Auth as OneLogin_Saml2_Auth; use OneLogin\Saml2\IdPMetadataParser as OneLogin_Saml2_IdPMetadataParser; use OneLogin\Saml2\Settings as OneLogin_Saml2_Settings; +use OneLogin\Saml2\Utils as OneLogin_Saml2_Utils; use App\Models\Setting; use App\Models\User; use Exception; @@ -153,6 +154,9 @@ class Saml $this->_enabled = $setting->saml_enabled == '1'; if ($this->isEnabled()) { + //Let onelogin/php-saml know to use 'X-Forwarded-*' headers if it is from a trusted proxy + OneLogin_Saml2_Utils::setProxyVars(request()->isFromTrustedProxy()); + data_set($settings, 'sp.entityId', url('/')); data_set($settings, 'sp.assertionConsumerService.url', route('saml.acs')); data_set($settings, 'sp.singleLogoutService.url', route('saml.sls')); diff --git a/resources/lang/en/admin/settings/general.php b/resources/lang/en/admin/settings/general.php index 65b9d8ccc..547ffc769 100644 --- a/resources/lang/en/admin/settings/general.php +++ b/resources/lang/en/admin/settings/general.php @@ -125,6 +125,7 @@ return array( 'saml_sp_acs_url' => 'Assertion Consumer Service (ACS) URL', 'saml_sp_sls_url' => 'Single Logout Service (SLS) URL', 'saml_sp_x509cert' => 'Public Certificate', + 'saml_sp_metadata_url' => 'Metadata URL', 'saml_idp_metadata' => 'SAML IdP Metadata', 'saml_idp_metadata_help' => 'You can specify the IdP metadata using a URL or XML file.', 'saml_attr_mapping_username' => 'Attribute Mapping - Username', diff --git a/resources/views/settings/saml.blade.php b/resources/views/settings/saml.blade.php index f4a53f6cb..adddf700f 100644 --- a/resources/views/settings/saml.blade.php +++ b/resources/views/settings/saml.blade.php @@ -55,6 +55,7 @@ {{ Form::checkbox('saml_enabled', '1', Request::old('saml_enabled', $setting->saml_enabled), [((config('app.lock_passwords')===true)) ? 'disabled ': '', 'class' => 'minimal '. $setting->demoMode, $setting->demoMode]) }} {{ trans('admin/settings/general.saml_enabled') }} + {!! $errors->first('saml_integration', '') !!}
@if (config('app.lock_passwords')===true)

{{ trans('general.feature_disabled') }}

@endif @@ -82,8 +83,12 @@ {{ Form::textarea('saml_sp_x509cert', $setting->saml_sp_x509cert, ['class' => 'form-control', 'wrap' => 'off', 'readonly']) }}
@endif + + {{ Form::label('saml_sp_metadata_url', trans('admin/settings/general.saml_sp_metadata_url')) }} + {{ Form::text('saml_sp_metadata_url', route('saml.metadata'), ['class' => 'form-control', 'readonly']) }} +

- View Metadata + Download Metadata

@endif {!! $errors->first('saml_enabled', '') !!}