Merge pull request #87 from italia/min-spid-level

Improve support for minimum SPID level
italia · Jun 28, 2022 · 8c2e3f6 · 8c2e3f6
2 parents 6cb78f7 + 15b53b2
commit 8c2e3f6
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 30 deletions.
diff --git a/src/Console/stubs/example/views/layouts/app.stub b/src/Console/stubs/example/views/layouts/app.stub
@@ -14,7 +14,7 @@
 
     <!-- Styles -->
     <link href="{{ asset('css/app.css') }}" rel="stylesheet">
-    <link type="text/css" rel="stylesheet" href="{{ asset('/vendor/spid-auth/css/agid-spid-enter.min.1.0.0.css') }}">
+    <link type="text/css" rel="stylesheet" href="{{ asset('/vendor/spid-auth/css/spid-sp-access-button.min.css') }}">
 </head>
 <body>
     <div id="app" class="flex-center position-ref full-height">

diff --git a/src/SPIDAuth.php b/src/SPIDAuth.php
@@ -161,11 +161,10 @@ public function acs(): RedirectResponse
 
         $SPIDUser = new SPIDUser($attributes);
         $idpEntityName = $this->getIdpEntityName($lastResponseXML);
-        $spidSessionIndex = $this->getSAML($idp)->getSessionIndex() ?? $this->getRandomString();
 
         session(['spid_idp' => $idp]);
         session(['spid_idpEntityName' => $idpEntityName]);
-        session(['spid_sessionIndex' => $spidSessionIndex]);
+        session(['spid_sessionId' => $this->getSAML($idp)->getLastMessageId()]);
         session(['spid_nameId' => $this->getSAML($idp)->getNameId()]);
         session(['spid_user' => $SPIDUser]);
 
@@ -191,7 +190,7 @@ public function acs(): RedirectResponse
     public function logout(): RedirectResponse
     {
         if ($this->isAuthenticated()) {
-            $sessionIndex = session()->pull('spid_sessionIndex');
+            $sessionId = session()->pull('spid_sessionId');
             $nameId = session()->pull('spid_nameId');
             $returnTo = url(config('spid-auth.after_logout_url'));
             $idp = session()->get('spid_idp');
@@ -211,7 +210,7 @@ public function logout(): RedirectResponse
             }
 
             try {
-                return $this->getSAML($idp)->logout($returnTo, [], $nameId, $sessionIndex, false, SAMLConstants::NAMEID_TRANSIENT, $idpEntityId);
+                return $this->getSAML($idp)->logout($returnTo, [], $nameId, $sessionId, false, SAMLConstants::NAMEID_TRANSIENT, $idpEntityId);
             } catch (SAMLError $e) {
                 throw new SPIDLogoutException($e->getMessage(), SPIDLogoutException::SAML_LOGOUT_ERROR, $e);
             }
@@ -250,7 +249,7 @@ public function logout(): RedirectResponse
      */
     public function isAuthenticated(): bool
     {
-        return session()->has('spid_sessionIndex');
+        return session()->has('spid_sessionId');
     }
 
     /**
@@ -482,8 +481,13 @@ protected function validateLoginResponse(string $lastResponseXML, string $lastRe
             throw new SPIDLoginException('SAML response validation error: empty or missing AudienceRestriction element', SPIDLoginException::SAML_VALIDATION_ERROR);
         }
 
-        if (0 === preg_match('/https:\/\/www\.spid\.gov\.it\/SpidL[123]/', $authContextClassRef->textContent)) {
+        $matchedSPIDLevel = [];
+        $configuredSpidLevel = preg_match('/https:\/\/www\.spid\.gov\.it\/SpidL([123])/', config('spid-auth.sp_spid_level'), $matchedSPIDLevel) ? (int) $matchedSPIDLevel[1] : null;
+
+        if (0 === preg_match('/https:\/\/www\.spid\.gov\.it\/SpidL([123])/', $authContextClassRef->textContent, $matchedSPIDLevel)) {
             throw new SPIDLoginException('SAML response validation error: wrong AuthContextClassRef element', SPIDLoginException::SAML_VALIDATION_ERROR);
+        } elseif ((int) $matchedSPIDLevel[1] < $configuredSpidLevel) {
+            throw new SPIDLoginException('SAML response validation error: minimum SPID Level not enforced', SPIDLoginException::SAML_VALIDATION_ERROR);
         }
 
         try {

diff --git a/tests/MiddlewareTest.php b/tests/MiddlewareTest.php
@@ -36,7 +36,7 @@ public function testAuthenticated()
         Route::get('/', function () {
             return 'home';
         })->middleware('spid.auth');
-        $response = $this->withSession(['spid_sessionIndex' => 'sessionIndex'])->get('/');
+        $response = $this->withSession(['spid_sessionId' => 'sessionId'])->get('/');
         $response->assertSuccessful();
     }
 

diff --git a/tests/ResponseValidationTest.php b/tests/ResponseValidationTest.php
@@ -195,6 +195,17 @@ public function testWrongAuthContextClassRef()
         ]);
     }
 
+    public function testWrongSPIDLevel()
+    {
+        $this->runValidationTest([
+            'responseXmlFile' => 'valid_level1.xml',
+            'config' => [
+                'spid-auth.sp_spid_level' => 'https://www.spid.gov.it/SpidL2',
+            ],
+            'exceptionMessage' => 'SAML response validation error: minimum SPID Level not enforced',
+        ]);
+    }
+
     public function testInvalidResponseIssueInstant()
     {
         $this->runValidationTest([
@@ -272,6 +283,9 @@ protected function runValidationTest(array $testSettings)
         $compiledResponseXML = str_replace('{{IssueInstant}}', SAMLUtils::parseTime2SAML(time()), $responseXML);
         $compiledResponseXML = str_replace('{{ResponseIssueInstant}}', $testSettings['responseIssueInstant'] ?? SAMLUtils::parseTime2SAML(time()), $compiledResponseXML);
         $compiledResponseXML = str_replace('{{AssertionIssueInstant}}', $testSettings['assertionIssueInstant'] ?? SAMLUtils::parseTime2SAML(time()), $compiledResponseXML);
+        foreach ($testSettings['config'] ?? [] as $key => $value) {
+            $this->app['config']->set($key, $value);
+        }
         $this->setSPIDAuthMock()->withLastResponseXML($compiledResponseXML);
         $this->withoutExceptionHandling();
         $this->expectException(SPIDLoginException::class);

diff --git a/tests/SPIDAuthBaseTestCase.php b/tests/SPIDAuthBaseTestCase.php
@@ -47,10 +47,11 @@ protected function getPackageProviders($app)
         return ['Italia\SPIDAuth\ServiceProvider'];
     }
 
-    protected function setSPIDAuthMock($spidLevel = 1)
+    protected function setSPIDAuthMock(?array $testSettings = [])
     {
         $testRedirectURL = $this->app['config']->get('spid-idps.test.singleSignOnService.url');
-        $responseXML = file_get_contents(__DIR__ . "/responses/valid_level{$spidLevel}.xml");
+        $responseXMLFile = ($testSettings['responseXmlFile'] ?? false) ? $testSettings['responseXmlFile'] : 'valid_level1.xml';
+        $responseXML = file_get_contents(__DIR__ . '/responses/' . $responseXMLFile);
         $compiledResponseXML = str_replace('{{IssueInstant}}', SAMLUtils::parseTime2SAML(time()), $responseXML);
         $compiledResponseXML = str_replace('{{ResponseIssueInstant}}', SAMLUtils::parseTime2SAML(time()), $compiledResponseXML);
         $compiledResponseXML = str_replace('{{AssertionIssueInstant}}', SAMLUtils::parseTime2SAML(time()), $compiledResponseXML);
@@ -79,19 +80,17 @@ protected function setSPIDAuthMock($spidLevel = 1)
             <samlp:AuthnRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" IssueInstant="' . SAMLUtils::parseTime2SAML(time()) . '" />'
         );
         $SAMLAuth->shouldReceive('getLastResponseXML')->andReturn($compiledResponseXML)->byDefault();
-        $SAMLAuth->shouldReceive('getSessionIndex')->andReturnUsing(function () use ($spidLevel) {
-            return $spidLevel > 1 ? null : 'sessionIndex';
-        });
+        $SAMLAuth->shouldReceive('getLastMessageId')->andReturn('sessionId');
         $SAMLAuth->shouldReceive('getNameId')->andReturn('nameId');
-        $SAMLAuth->shouldReceive('logout')->with(URL::to($this->afterLogoutURL), [], 'nameId', 'sessionIndex', false, SAMLConstants::NAMEID_TRANSIENT, 'spid-testenv')->andReturn(
+        $SAMLAuth->shouldReceive('logout')->with(URL::to($this->afterLogoutURL), [], 'nameId', 'sessionId', false, SAMLConstants::NAMEID_TRANSIENT, 'spid-testenv')->andReturn(
             Response::redirectTo($this->logoutURL)
         )->byDefault();
         $SAMLAuth->shouldReceive('getErrors')->andReturn(false)->byDefault();
         $SAMLAuth->shouldReceive('getSPMetadata')->andReturn()->byDefault();
         $SAMLAuth->shouldReceive('getSettings')->andReturn($SAMLAuth);
 
         $SAMLAuth->shouldReceive('withErrors')->andReturnUsing(function () {
-            return m::self()->shouldReceive('logout')->with(URL::to($this->afterLogoutURL), [], 'nameId', 'sessionIndex', false, SAMLConstants::NAMEID_TRANSIENT, 'spid-testenv')->andThrow(
+            return m::self()->shouldReceive('logout')->with(URL::to($this->afterLogoutURL), [], 'nameId', 'sessionId', false, SAMLConstants::NAMEID_TRANSIENT, 'spid-testenv')->andThrow(
                 new SAMLError(
                     'The IdP does not support Single Log Out',
                     SAMLError::SAML_SINGLE_LOGOUT_NOT_SUPPORTED

diff --git a/tests/SPIDAuthTest.php b/tests/SPIDAuthTest.php
@@ -25,7 +25,7 @@ public function testLogin()
     public function testLoginIfAuthenticated()
     {
         $response = $this->withSession([
-            'spid_sessionIndex' => 'sessionIndex',
+            'spid_sessionId' => 'sessionId',
         ])->get($this->loginURL);
 
         $response->assertRedirect($this->afterLoginURL);
@@ -34,7 +34,7 @@ public function testLoginIfAuthenticated()
     public function testLoginIfAuthenticatedWithIntendedURL()
     {
         $response = $this->withSession([
-            'spid_sessionIndex' => 'sessionIndex',
+            'spid_sessionId' => 'sessionId',
             'url.intended' => 'intendedURL',
         ])->get($this->loginURL);
 
@@ -44,7 +44,7 @@ public function testLoginIfAuthenticatedWithIntendedURL()
     public function testDoLoginIfAuthenticated()
     {
         $response = $this->withSession([
-            'spid_sessionIndex' => 'sessionIndex',
+            'spid_sessionId' => 'sessionId',
         ])->post($this->doLoginURL);
 
         $response->assertRedirect($this->afterLoginURL);
@@ -53,7 +53,7 @@ public function testDoLoginIfAuthenticated()
     public function testDoLoginIfAuthenticatedWithIntendedURL()
     {
         $response = $this->withSession([
-            'spid_sessionIndex' => 'sessionIndex',
+            'spid_sessionId' => 'sessionId',
             'url.intended' => 'intendedURL',
         ])->post($this->doLoginURL);
 
@@ -114,11 +114,10 @@ public function testDoLogin()
         $response->assertRedirect();
     }
 
-    public function testAcs($spidLevel = 1)
+    public function testAcs()
     {
         Event::fake();
-        $this->setSPIDAuthMock($spidLevel);
-        $expectedSessionIndex = $spidLevel > 1 ? 'RANDOM_STRING' : 'sessionIndex';
+        $this->setSPIDAuthMock();
 
         $response = $this->withCookies([
             'spid_lastRequestId' => 'UNIQUE_ID',
@@ -127,7 +126,7 @@ public function testAcs($spidLevel = 1)
         ])->post($this->acsURL);
 
         $response->assertSessionHas('spid_idpEntityName', 'Test IdP');
-        $response->assertSessionHas('spid_sessionIndex', $expectedSessionIndex);
+        $response->assertSessionHas('spid_sessionId', 'sessionId');
         $response->assertSessionHas('spid_nameId', 'nameId');
         $response->assertSessionHas('spid_user');
         $response->assertRedirect($this->afterLoginURL);
@@ -165,7 +164,7 @@ public function testAcsWithIntendedURL()
 
         $this->assertFalse(cache()->has('RANDOM_STRING'));
         $response->assertSessionHas('spid_idpEntityName', 'Test IdP');
-        $response->assertSessionHas('spid_sessionIndex', 'sessionIndex');
+        $response->assertSessionHas('spid_sessionId', 'sessionId');
         $response->assertSessionHas('spid_nameId', 'nameId');
         $response->assertSessionHas('spid_user');
         $response->assertRedirect('intendedURL');
@@ -260,14 +259,34 @@ public function testAcsWithReplayAttack()
         $response->assertStatus(500);
     }
 
-    public function testAcsWithSpidLevel2()
+    public function testAcsWithSPIDLevel2()
     {
-        $this->testAcs(2);
+        $this->setSPIDAuthMock([
+            'responseXmlFile' => 'valid_level2.xml',
+        ]);
+
+        $response = $this->withCookies([
+            'spid_lastRequestId' => 'UNIQUE_ID',
+            'spid_lastRequestIssueInstant' => SAMLUtils::parseTime2SAML(time()),
+            'spid_idp' => 'test',
+        ])->post($this->acsURL);
+
+        $response->assertRedirect();
     }
 
-    public function testAcsWithSpidLevel3()
+    public function testAcsWithSPIDLevel3()
     {
-        $this->testAcs(3);
+        $this->setSPIDAuthMock([
+            'responseXmlFile' => 'valid_level3.xml',
+        ]);
+
+        $response = $this->withCookies([
+            'spid_lastRequestId' => 'UNIQUE_ID',
+            'spid_lastRequestIssueInstant' => SAMLUtils::parseTime2SAML(time()),
+            'spid_idp' => 'test',
+        ])->post($this->acsURL);
+
+        $response->assertRedirect();
     }
 
     public function testLogout()
@@ -276,7 +295,7 @@ public function testLogout()
 
         $response = $this->get($this->logoutURL);
 
-        $response->assertSessionMissing('spid_sessionIndex');
+        $response->assertSessionMissing('spid_sessionId');
         $response->assertSessionMissing('spid_nameId');
         $response->assertRedirect($this->logoutURL);
     }
@@ -357,7 +376,7 @@ public function testLogoutSpOnly()
 
         $response = $this->get($this->logoutURL);
 
-        $response->assertSessionMissing('spid_sessionIndex');
+        $response->assertSessionMissing('spid_sessionId');
         $response->assertSessionMissing('spid_nameId');
         $response->assertRedirect($this->afterLogoutURL);
         Event::assertDispatched(LogoutEvent::class, function ($e) {