Skip to content

Commit

Permalink
Saml orgid usercreation (#5048)
Browse files Browse the repository at this point in the history
* set orgid on user registration

* fix indentation and usergroup type hint

* add getOrgid; use named arguments

* add test for orgid is array in saml response
  • Loading branch information
bloo-dye committed May 10, 2024
1 parent dffd142 commit b70f80d
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 10 deletions.
26 changes: 19 additions & 7 deletions src/Auth/Saml.php
Original file line number Diff line number Diff line change
Expand Up @@ -155,15 +155,19 @@ public function assertIdpResponse(): AuthResponse
// GET EMAIL
$email = $this->extractAttribute($this->settings['idp']['emailAttr']);

// GET ORGID
$orgid = $this->getOrgid();

// GET POPULATED USERS OBJECT
$Users = $this->getUsers($email);
$Users = $this->getUsers($email, $orgid);
if (!$Users instanceof Users) {
$this->AuthResponse->userid = 0;
$this->AuthResponse->initTeamRequired = true;
$this->AuthResponse->initTeamUserInfo = array(
'email' => $email,
'firstname' => $this->getName(),
'lastname' => $this->getName(true),
'orgid' => $orgid,
);
return $this->AuthResponse;
}
Expand Down Expand Up @@ -211,6 +215,15 @@ private function extractAttribute(string $attribute): string
return $attr;
}

private function getOrgid(): ?string
{
$orgid = $this->samlUserdata[$this->settings['idp']['orgidAttr'] ?? 'Unknown'] ?? null;
if (is_array($orgid)) {
return $orgid[0];
}
return $orgid;
}

/**
* Get firstname or lastname from idp
**/
Expand Down Expand Up @@ -277,16 +290,15 @@ private function getTeams(): array | int
throw new ImproperActionException('Could not find team ID to assign user!');
}

private function getExistingUser(string $email): Users | false
private function getExistingUser(string $email, ?string $orgid = null): Users | false
{
try {
// we first try to match a local user with the email
return ExistingUser::fromEmail($email);
} catch (ResourceNotFoundException) {
// try finding the user with the orgid because email didn't work
// but only if we explicitly want to
if ($this->configArr['saml_fallback_orgid'] === '1' && !empty($this->settings['idp']['orgidAttr'])) {
$orgid = $this->extractAttribute($this->settings['idp']['orgidAttr']);
if ($this->configArr['saml_fallback_orgid'] === '1' && $orgid) {
try {
$Users = ExistingUser::fromOrgid($orgid);
// ok we found our user thanks to the orgid, maybe we want to update our stored email?
Expand All @@ -302,9 +314,9 @@ private function getExistingUser(string $email): Users | false
}
}

private function getUsers(string $email): Users | int
private function getUsers(string $email, ?string $orgid = null): Users | int
{
$Users = $this->getExistingUser($email);
$Users = $this->getExistingUser($email, $orgid);
if ($Users === false) {
// the user doesn't exist yet in the db
// what do we do? Lookup the config setting for that case
Expand All @@ -325,7 +337,7 @@ private function getUsers(string $email): Users | int
}

// CREATE USER (and force validation of user, with user permissions)
$Users = ValidatedUser::fromExternal($email, $teams, $this->getName(), $this->getName(true));
$Users = ValidatedUser::fromExternal($email, $teams, $this->getName(), $this->getName(true), orgid: $orgid);
}
return $Users;
}
Expand Down
2 changes: 2 additions & 0 deletions src/controllers/LoginController.php
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ public function getResponse(): Response
$this->App->Session->set('teaminit_email', $AuthResponse->initTeamUserInfo['email']);
$this->App->Session->set('teaminit_firstname', $AuthResponse->initTeamUserInfo['firstname']);
$this->App->Session->set('teaminit_lastname', $AuthResponse->initTeamUserInfo['lastname']);
$this->App->Session->set('teaminit_orgid', $AuthResponse->initTeamUserInfo['orgid']);
return new RedirectResponse('/login.php');
}

Expand Down Expand Up @@ -342,6 +343,7 @@ private function initTeamSelection(): void
array($this->App->Request->request->getInt('team_id')),
$this->App->Request->request->getString('teaminit_firstname'),
$this->App->Request->request->getString('teaminit_lastname'),
orgid: $this->App->Session->get('teaminit_orgid'),
);
$this->App->Session->set('teaminit_done', true);
// will display the appropriate message to user
Expand Down
3 changes: 2 additions & 1 deletion src/models/ExistingUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,10 @@ public static function fromScratch(
?Usergroup $usergroup = null,
bool $automaticValidationEnabled = false,
bool $alertAdmin = true,
?string $orgid = null,
): Users {
$Users = new self();
$userid = $Users->createOne($email, $teams, $firstname, $lastname, '', $usergroup, $automaticValidationEnabled, $alertAdmin);
$userid = $Users->createOne($email, $teams, $firstname, $lastname, '', $usergroup, $automaticValidationEnabled, $alertAdmin, orgid: $orgid);
$fresh = new self($userid);
// we need to report the needValidation flag into the new object
$fresh->needValidation = $Users->needValidation;
Expand Down
4 changes: 2 additions & 2 deletions src/models/ValidatedUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ public static function fromOrgid(string $orgid): Users
return self::search('orgid', $orgid, true);
}

public static function fromExternal(string $email, array $teams, string $firstname, string $lastname): Users
public static function fromExternal(string $email, array $teams, string $firstname, string $lastname, ?string $orgid = null): Users
{
return parent::fromScratch($email, $teams, $firstname, $lastname, null, true);
return parent::fromScratch($email, $teams, $firstname, $lastname, automaticValidationEnabled: true, orgid: $orgid);
}

public static function fromAdmin(string $email, array $teams, string $firstname, string $lastname, Usergroup $usergroup): Users
Expand Down
12 changes: 12 additions & 0 deletions tests/unit/Auth/SamlTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,18 @@ public function testAssertIdpResponseEmailArrayResponse(): void
$this->assertEquals(1, $authResponse->selectedTeam);
}

/**
* Idp sends an array of orgIDs
*/
public function testAssertIdpResponseOrgidArrayResponse(): void
{
$samlUserdata = $this->samlUserdata;
$samlUserdata['internal_id'] = array('internal_id_1');

$authResponse = $this->getAuthResponse($samlUserdata);
$this->assertEquals(1, $authResponse->selectedTeam);
}

/**
* Idp doesn't send back an email
*/
Expand Down
1 change: 1 addition & 0 deletions web/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
$App->Session->set('teaminit_email', $AuthResponse->initTeamUserInfo['email']);
$App->Session->set('teaminit_firstname', $AuthResponse->initTeamUserInfo['firstname']);
$App->Session->set('teaminit_lastname', $AuthResponse->initTeamUserInfo['lastname']);
$App->Session->set('teaminit_orgid', $AuthResponse->initTeamUserInfo['orgid']);
$location = '/login.php';

// if the user is in several teams, we need to redirect to the team selection
Expand Down

0 comments on commit b70f80d

Please sign in to comment.