Reviewed and updated SAML2 authncontext option

Added tests to cover.
Changed default to align with existing default.
Added env option parsing.
For #1998

diff --git a/.env.example.complete b/.env.example.complete
index a3b0702d5b1385b9492d01d8ffe2c0a32ea4da3d..71fe66bca93c7d5fddfbe334afbbddf14488db5e 100644 (file)
--- a/.env.example.complete
+++ b/.env.example.complete
@@ -222,12 +222,7 @@ SAML2_IDP_x509=null
 SAML2_ONELOGIN_OVERRIDES=null
 SAML2_DUMP_USER_DETAILS=false
 SAML2_AUTOLOAD_METADATA=false
-
-# SAML Authentication context.
-# Set to false and no AuthContext will be sent in the AuthNRequest,
-# Set true and you will get an AuthContext 'exact' 'urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport'
-# Set an array with the possible auth context values: array ('urn:oasis:names:tc:SAML:2.0:ac:classes:Password', 'urn:oasis:names:tc:SAML:2.0:ac:classes:X509'),
-SAML2_IDP_AUTHNCONTEXT=false
+SAML2_IDP_AUTHNCONTEXT=true
 
 # SAML group sync configuration
 # Refer to https://www.bookstackapp.com/docs/admin/saml2-auth/

diff --git a/app/Config/saml2.php b/app/Config/saml2.php
index 0e186c26937e0b97282d6e46abd5692f8b6666f3..8ba96954905f029c5883c3b4bd5616945d9cfe5a 100644 (file)
--- a/app/Config/saml2.php
+++ b/app/Config/saml2.php
@@ -1,5 +1,7 @@
 <?php
 
+$SAML2_IDP_AUTHNCONTEXT = env('SAML2_IDP_AUTHNCONTEXT', true);
+
 return [
 
     // Display name, shown to users, for SAML2 option
@@ -140,10 +142,12 @@ return [
             // ),
         ],
         'security' => [
-            // Specifies Authentication context
-            // false means that IDP choose authentication method
-            // null force Form based authentication or is possible set via array supported methods. See to onelogin/php-sampl/advance_settings
-            'requestedAuthnContext' => env('SAML2_IDP_AUTHNCONTEXT',false), 
+            // SAML2 Authn context
+            // When set to false no AuthContext will be sent in the AuthNRequest,
+            // When set to true (Default) you will get an AuthContext 'exact' 'urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport'.
+            // Multiple forced values can be passed via a space separated array, For example:
+            // SAML2_IDP_AUTHNCONTEXT="urn:federation:authentication:windows urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport"
+            'requestedAuthnContext' => is_string($SAML2_IDP_AUTHNCONTEXT) ? explode(' ', $SAML2_IDP_AUTHNCONTEXT) : $SAML2_IDP_AUTHNCONTEXT,
         ],
     ],
 

diff --git a/tests/Auth/Saml2Test.php b/tests/Auth/Saml2Test.php
index 58c02b47188a625b83a5aeb3d0ad80c5f4f81de0..b6b02e2f76d8aef7a65493bfda7449b8ca5271dd 100644 (file)
--- a/tests/Auth/Saml2Test.php
+++ b/tests/Auth/Saml2Test.php
@@ -28,6 +28,7 @@ class Saml2Test extends TestCase
             'saml2.autoload_from_metadata' => false,
             'saml2.onelogin.idp.x509cert' => $this->testCert,
             'saml2.onelogin.debug' => false,
+            'saml2.onelogin.security.requestedAuthnContext' => true,
         ]);
     }
 
@@ -328,6 +329,40 @@ class Saml2Test extends TestCase
         });
     }
 
+    public function test_login_request_contains_expected_default_authncontext()
+    {
+        $authReq = $this->getAuthnRequest();
+        $this->assertStringContainsString('samlp:RequestedAuthnContext Comparison="exact"', $authReq);
+        $this->assertStringContainsString('<saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport</saml:AuthnContextClassRef>', $authReq);
+    }
+
+    public function test_false_idp_authncontext_option_does_not_pass_authncontext_in_saml_request()
+    {
+        config()->set(['saml2.onelogin.security.requestedAuthnContext' => false]);
+        $authReq = $this->getAuthnRequest();
+        $this->assertStringNotContainsString('samlp:RequestedAuthnContext', $authReq);
+        $this->assertStringNotContainsString('<saml:AuthnContextClassRef>', $authReq);
+    }
+
+    public function test_array_idp_authncontext_option_passes_value_as_authncontextclassref_in_request()
+    {
+        config()->set(['saml2.onelogin.security.requestedAuthnContext' => ['urn:federation:authentication:windows', 'urn:federation:authentication:linux']]);
+        $authReq = $this->getAuthnRequest();
+        $this->assertStringContainsString('samlp:RequestedAuthnContext', $authReq);
+        $this->assertStringContainsString('<saml:AuthnContextClassRef>urn:federation:authentication:windows</saml:AuthnContextClassRef>', $authReq);
+        $this->assertStringContainsString('<saml:AuthnContextClassRef>urn:federation:authentication:linux</saml:AuthnContextClassRef>', $authReq);
+    }
+
+    protected function getAuthnRequest(): string
+    {
+        $req = $this->post('/saml2/login');
+        $location = $req->headers->get('Location');
+        $query = explode('?', $location)[1];
+        $params = [];
+        parse_str($query, $params);
+        return gzinflate(base64_decode($params['SAMLRequest']));
+    }
+
     protected function withGet(array $options, callable $callback)
     {
         return $this->withGlobal($_GET, $options, $callback);

diff --git a/tests/Unit/ConfigTest.php b/tests/Unit/ConfigTest.php
index 1d4decc2b330036feed751d396e52215770128ef..bb475c3545c19d68ec1a409ea71d5448a5efdf65 100644 (file)
--- a/tests/Unit/ConfigTest.php
+++ b/tests/Unit/ConfigTest.php
@@ -67,12 +67,22 @@ class ConfigTest extends TestCase
         $this->checkEnvConfigResult('APP_URL', '', 'session.path', '/');
     }
 
+    public function test_saml2_idp_authn_context_string_parsed_as_space_separated_array()
+    {
+        $this->checkEnvConfigResult(
+            'SAML2_IDP_AUTHNCONTEXT',
+            'urn:federation:authentication:windows urn:federation:authentication:linux',
+            'saml2.onelogin.security.requestedAuthnContext',
+            ['urn:federation:authentication:windows', 'urn:federation:authentication:linux']
+        );
+    }
+
     /**
      * Set an environment variable of the given name and value
      * then check the given config key to see if it matches the given result.
      * Providing a null $envVal clears the variable.
      */
-    protected function checkEnvConfigResult(string $envName, ?string $envVal, string $configKey, string $expectedResult)
+    protected function checkEnvConfigResult(string $envName, ?string $envVal, string $configKey, mixed $expectedResult)
     {
         $this->runWithEnv($envName, $envVal, function() use ($configKey, $expectedResult) {
             $this->assertEquals($expectedResult, config($configKey));