Skip to content

Commit

Permalink
N°7807 - Add support for organization selection during autoprovision…
Browse files Browse the repository at this point in the history 
…ing Combodo#4 - propose few enhancements + tests
  • Loading branch information
@odain-cbd
odain-cbd committed Dec 24, 2024
1 parent 64d7ebe commit ef5f5a3
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 18 deletions.
4 changes: 2 additions & 2 deletions src/Config.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand All @@ -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;
}
}
Expand Down
18 changes: 17 additions & 1 deletion src/HybridAuthLoginExtension.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
[
Expand All @@ -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();
Expand Down
88 changes: 78 additions & 10 deletions tests/php-unit-tests/HybridAuthLoginExtensionTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand Down Expand Up @@ -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");
Expand All @@ -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);
Expand All @@ -249,25 +285,57 @@ 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);
$this->assertFalse(strpos($sOutput, "An error occurred"), "An error occurred should NOT appear in output: " . $this->sEmail . " . should appear in the welcome page :" . $sOutput);
}

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);
$this->assertTrue(false !== strpos($sOutput, "No login_mode specified by service provider"), "An error occurred should appear in output: " . $this->sEmail . " . should appear in the welcome page :" . $sOutput);
}

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);
}
}
14 changes: 9 additions & 5 deletions tests/php-unit-tests/Provider/ServiceProviderMock.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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);
}
}

Expand Down

0 comments on commit ef5f5a3

Please sign in to comment.