From ef5f5a3d5e76d111b0cc4ec68786630a16e8afff Mon Sep 17 00:00:00 2001 From: odain Date: Tue, 24 Dec 2024 16:07:52 +0100 Subject: [PATCH] =?UTF-8?q?=20N=C2=B07807=20-=20Add=20support=20for=20orga?= =?UTF-8?q?nization=20selection=20during=20autoprovisioning=20#4=20-=20pro?= =?UTF-8?q?pose=20few=20enhancements=20+=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Config.php | 4 +- src/HybridAuthLoginExtension.php | 18 +++- .../HybridAuthLoginExtensionTest.php | 88 ++++++++++++++++--- .../Provider/ServiceProviderMock.php | 14 +-- 4 files changed, 106 insertions(+), 18 deletions(-) diff --git a/src/Config.php b/src/Config.php index 7c5c7ea..871a60e 100644 --- a/src/Config.php +++ b/src/Config.php @@ -156,7 +156,7 @@ public static function GetSynchroProfile(string $sLoginMode) : string { $aCurrentProviderConf = self::GetProviderConf($sLoginMode); if (null !== $aCurrentProviderConf){ $sDefautProfile = $aCurrentProviderConf['default_profile'] ?? null; - if (null !== $sDefautProfile){ + if (utils::IsNotNullOrEmptyString($sDefautProfile)){ return $sDefautProfile; } } @@ -181,7 +181,7 @@ public static function GetDefaultOrg(string $sLoginMode) { $aCurrentProviderConf = self::GetProviderConf($sLoginMode); if (null !== $aCurrentProviderConf){ $sDefaultOrg = $aCurrentProviderConf['default_organization'] ?? null; - if (null !== $sDefaultOrg){ + if (utils::IsNotNullOrEmptyString($sDefaultOrg)){ return $sDefaultOrg; } } diff --git a/src/HybridAuthLoginExtension.php b/src/HybridAuthLoginExtension.php index bbde458..e2d4014 100644 --- a/src/HybridAuthLoginExtension.php +++ b/src/HybridAuthLoginExtension.php @@ -337,7 +337,7 @@ private function DoUserProvisioning(string $sLoginMode) // Create the person $sFirstName = $oUserProfile->firstName; $sLastName = $oUserProfile->lastName; - $sOrganization = $oUserProfile->data["organization"] ?? Config::GetDefaultOrg($sLoginMode); + $sOrganization = $this->GetOrganizationForProvisioning($sLoginMode, $oUserProfile->data["organization"] ?? null); $aAdditionalParams = array('phone' => $oUserProfile->phone); IssueLog::Info("OpenID Person provisioning", HybridAuthLoginExtension::LOG_CHANNEL, [ @@ -361,6 +361,22 @@ private function DoUserProvisioning(string $sLoginMode) } } + private function GetOrganizationForProvisioning(string $sLoginMode, ?string $sIdPOrgName) : string + { + if (is_null($sIdPOrgName)){ + return Config::GetDefaultOrg($sLoginMode); + } + + $oOrg = MetaModel::GetObjectByName('Organization', $sIdPOrgName, false); + if (! is_null($oOrg)) + { + return $sIdPOrgName; + } + + IssueLog::Error(Dict::S('UI:Login:Error:WrongOrganizationName', null, ['idp_organization' => $sIdPOrgName])); + return Config::GetDefaultOrg($sLoginMode); + } + public function GetTwigContext() { $oLoginContext = new LoginTwigContext(); diff --git a/tests/php-unit-tests/HybridAuthLoginExtensionTest.php b/tests/php-unit-tests/HybridAuthLoginExtensionTest.php index 12d5482..2cc2605 100644 --- a/tests/php-unit-tests/HybridAuthLoginExtensionTest.php +++ b/tests/php-unit-tests/HybridAuthLoginExtensionTest.php @@ -3,6 +3,7 @@ namespace Combodo\iTop\HybridAuth\Test; use Combodo\iTop\HybridAuth\Config; +use Combodo\iTop\HybridAuth\HybridAuthLoginExtension; use Combodo\iTop\HybridAuth\Test\Provider\ServiceProviderMock; use Combodo\iTop\Test\UnitTest\ItopDataTestCase; use MetaModel; @@ -175,7 +176,7 @@ protected function CallItopUrl($sUri, $bXDebugEnabled=false, ?array $aPostFields } public function test_SSOConnectedAlready_NoiTopUserProvisioning_OK() { - $aData = ['profile_email' => $this->sEmail]; + $aData = ['email' => $this->sEmail]; file_put_contents(ServiceProviderMock::GetFileConfPath(), json_encode($aData)); $sOutput = $this->CallItopUrl("/pages/UI.php"); @@ -184,7 +185,7 @@ public function test_SSOConnectedAlready_NoiTopUserProvisioning_OK() { } public function test_SSOConnectedAlready_NoiTopUserProvisioning_UnknownUser() { - $aData = ['profile_email' => 'unknown_' . $this->sUniqId . '@titi.fr']; + $aData = ['email' => 'unknown_' . $this->sUniqId . '@titi.fr']; file_put_contents(ServiceProviderMock::GetFileConfPath(), json_encode($aData)); $sOutput = $this->CallItopUrl("/pages/UI.php"); @@ -215,10 +216,10 @@ public function test_SSOConnectedAlready_WithiTopUserProvisioning_OK($sProfile, $sLatName = $this->sUniqId . "_lastName"; $sPhone = "123456789"; $aData = [ - 'profile_email' => $this->sProvisionedUserPersonEmail, - 'profile_firstName' => $sFirstName, - 'profile_lastName' => $sLatName, - 'profile_phone' => $sPhone, + 'email' => $this->sProvisionedUserPersonEmail, + 'firstName' => $sFirstName, + 'lastName' => $sLatName, + 'phone' => $sPhone, ]; file_put_contents(ServiceProviderMock::GetFileConfPath(), json_encode($aData)); $sOutput = $this->CallItopUrl("/pages/UI.php"); @@ -231,12 +232,47 @@ public function test_SSOConnectedAlready_WithiTopUserProvisioning_OK($sProfile, "user logged in => his lastname . ".$sLatName." . should appear in the welcome page :".$sOutput); } + $this->VerifyProvisioningIsOk($sFirstName, $sPhone, $sLatName, $sProfile, $this->oOrg->GetKey()); + } + + public function test_SSOConnectedAlready_WithiTopUserProvisioning_UseIdPOrg_OK() { + $sProfile = "Configuration Manager"; + $this->oiTopConfig->SetModuleSetting('combodo-hybridauth', 'synchronize_user', true); + $this->oiTopConfig->SetModuleSetting('combodo-hybridauth', 'synchronize_contact', true); + $this->oiTopConfig->SetModuleSetting('combodo-hybridauth', 'default_organization', $this->oOrg->Get('name')); + $this->oiTopConfig->SetModuleSetting('combodo-hybridauth', 'default_profile', $sProfile); + + $this->SaveItopConfFile(); + + $this->sProvisionedUserPersonEmail = 'usercontacttoprovision_' .$this->sUniqId. '@test.fr'; + $sFirstName = $this->sUniqId . "_firstName"; + $sLatName = $this->sUniqId . "_lastName"; + $sPhone = "123456789"; + + $sIdPOrgName = "IdP_".$this->sUniqId; + $oIdPOrg = $this->CreateOrganization($sIdPOrgName); + $aData = [ + 'email' => $this->sProvisionedUserPersonEmail, + 'firstName' => $sFirstName, + 'lastName' => $sLatName, + 'phone' => $sPhone, + 'organization' => $sIdPOrgName, + ]; + file_put_contents(ServiceProviderMock::GetFileConfPath(), json_encode($aData)); + $sOutput = $this->CallItopUrl("/pages/UI.php"); + + $this->assertFalse(strpos($sOutput, "login-body"), "user logged in => no login page:".$sOutput); + $this->VerifyProvisioningIsOk($sFirstName, $sPhone, $sLatName, $sProfile, $oIdPOrg->GetKey()); + } + + private function VerifyProvisioningIsOk(string $sFirstName, string $sPhone, string $sLatName, string $sProfile, string $sOrgId) : void + { $oExpectedPerson = MetaModel::GetObjectByColumn("Person", "email", $this->sProvisionedUserPersonEmail); $this->assertNotNull($oExpectedPerson); $this->assertEquals($sFirstName, $oExpectedPerson->Get('first_name')); $this->assertEquals($sPhone, $oExpectedPerson->Get('phone')); $this->assertEquals($sLatName, $oExpectedPerson->Get('name')); - $this->assertEquals($this->oOrg->GetKey(), $oExpectedPerson->Get('org_id')); + $this->assertEquals($sOrgId, $oExpectedPerson->Get('org_id')); $oExpectedUser = MetaModel::GetObjectByColumn("UserExternal", "login", $this->sProvisionedUserPersonEmail); $this->assertNotNull($oExpectedUser); @@ -249,7 +285,7 @@ public function test_SSOConnectedAlready_WithiTopUserProvisioning_OK($sProfile, } public function testLandingPage(){ - $aData = ['profile_email' => $this->sEmail]; + $aData = ['email' => $this->sEmail]; file_put_contents(ServiceProviderMock::GetFileConfPath(), json_encode($aData)); $sOutput = $this->CallItopUrl("/env-" . $this->GetTestEnvironment() . "/combodo-hybridauth/landing.php?login_mode=" . $this->sLoginMode, false, []); $this->assertFalse(strpos($sOutput, "login-body"), "user logged in => no login page:" . $sOutput); @@ -257,7 +293,7 @@ public function testLandingPage(){ } public function testLandingPageFailureNoLoginModeProvided(){ - $aData = ['profile_email' => $this->sEmail]; + $aData = ['email' => $this->sEmail]; file_put_contents(ServiceProviderMock::GetFileConfPath(), json_encode($aData)); $sOutput = $this->CallItopUrl("/env-" . $this->GetTestEnvironment() . "/combodo-hybridauth/landing.php", false, []); $this->assertTrue(false !== strpos($sOutput, "login-body"), "user logged in => login page:" . $sOutput); @@ -265,9 +301,41 @@ public function testLandingPageFailureNoLoginModeProvided(){ } public function testLandingPageFailureInvalidSSOLoginMode(){ - $aData = ['profile_email' => $this->sEmail]; + $aData = ['email' => $this->sEmail]; file_put_contents(ServiceProviderMock::GetFileConfPath(), json_encode($aData)); $sOutput = $this->CallItopUrl("/env-" . $this->GetTestEnvironment() . "/combodo-hybridauth/landing.php?login_mode=hybridauth-badlyconfigured", false, []); $this->assertTrue(false !== strpos($sOutput, "login-body"), "user logged in => login page:" . $sOutput); } + + public function GetOrganizationForProvisioningProvider() + { + $sDefaultOrgName = 'IDP_ORG1'.uniqid(); + $sOrgName2 = 'IDP_ORG2'.uniqid(); + return [ + 'no org returned by IdP' => [ $sDefaultOrgName, $sOrgName2, null, $sDefaultOrgName ], + 'unknown org returned by IdP' => [ $sDefaultOrgName, $sOrgName2, "unknown_IDP_Org", $sDefaultOrgName ], + 'use IdP org name' => [ $sDefaultOrgName, $sOrgName2, $sOrgName2, $sOrgName2 ], + ]; + } + + /** + * @dataProvider GetOrganizationForProvisioningProvider + */ + public function testGetOrganizationForProvisioning(string $sDefaultOrgName, string $sOrgName2, ?string $sIdpOrgName, string $sExpectedOrgReturned) + { + $this->oiTopConfig->SetModuleSetting('combodo-hybridauth', 'synchronize_user', true); + $this->oiTopConfig->SetModuleSetting('combodo-hybridauth', 'synchronize_contact', true); + MetaModel::GetConfig()->SetModuleSetting('combodo-hybridauth', 'default_organization', $sDefaultOrgName); + + $this->oiTopConfig->SetModuleSetting('combodo-hybridauth', 'default_profile', "Configuration Manager"); + + $this->SaveItopConfFile(); + $this->CreateOrganization($sDefaultOrgName); + $this->CreateOrganization($sOrgName2); + + $oHybridAuthLoginExtension = new HybridAuthLoginExtension(); + + $sOrgName = $this->InvokeNonPublicMethod(HybridAuthLoginExtension::class, 'GetOrganizationForProvisioning', $oHybridAuthLoginExtension, [ $this->sLoginMode, $sIdpOrgName]); + $this->assertEquals($sExpectedOrgReturned, $sOrgName); + } } diff --git a/tests/php-unit-tests/Provider/ServiceProviderMock.php b/tests/php-unit-tests/Provider/ServiceProviderMock.php index 8fc440f..be869f9 100644 --- a/tests/php-unit-tests/Provider/ServiceProviderMock.php +++ b/tests/php-unit-tests/Provider/ServiceProviderMock.php @@ -23,7 +23,7 @@ public function authenticate() { $aData = $this->GetData(); \IssueLog::Info("ServiceProvider->authenticate data to pass to OpenID:", null, $aData); - $sEmail = $aData['profile_email'] ?? null; + $sEmail = $aData['email'] ?? null; if (! is_null($sEmail)) { Session::Set('auth_user', $sEmail); Session::Unset('login_will_redirect'); @@ -41,13 +41,17 @@ public function getUserProfile() : Profile { $oProfile = new Profile(); $sProfileFields = ['firstName', 'lastName', 'email', 'phone']; - foreach ($sProfileFields as $sField) { - $sSessionFieldKey = 'profile_'.$sField; - $sValue = $aData[$sSessionFieldKey] ?? null; - if (!is_null($sValue)) { + foreach ($aData as $sField => $sValue) { + if (in_array($sField, $sProfileFields)) { $property = $class->getProperty($sField); $property->setAccessible(true); $property->setValue($oProfile, $sValue); + } else { + $property = $class->getProperty('data'); + $property->setAccessible(true); + $aProfileData = $property->GetValue($oProfile); + $aProfileData[$sField] = $sValue; + $property->setValue($oProfile, $aProfileData); } }