-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow user group mapping #51
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,65 @@ | ||||||
<?php | ||||||
|
||||||
namespace SilverStripe\SAML\Helpers; | ||||||
|
||||||
use SilverStripe\Core\Config\Configurable; | ||||||
use SilverStripe\Core\Injector\Injectable; | ||||||
use SilverStripe\ORM\DataObject; | ||||||
use SilverStripe\SAML\Services\SAMLConfiguration; | ||||||
use SilverStripe\Security\Group; | ||||||
use SilverStripe\Security\Member; | ||||||
|
||||||
class SAMLUserGroupMapper | ||||||
{ | ||||||
use Injectable; | ||||||
use Configurable; | ||||||
|
||||||
/** | ||||||
* @var string | ||||||
*/ | ||||||
private static $group_claims_field; | ||||||
|
||||||
/** | ||||||
* @var array | ||||||
*/ | ||||||
private static $dependencies = [ | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
'SAMLConfService' => '%$' . SAMLConfiguration::class, | ||||||
]; | ||||||
|
||||||
/** | ||||||
* Check if group claims field is set and assigns member to group | ||||||
* | ||||||
* @param [] $attributes | ||||||
* @param Member $member | ||||||
* @return Member | ||||||
*/ | ||||||
public function map($attributes, $member): Member | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
{ | ||||||
$groups = $this->config()->get('group_claims_field'); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not a big deal but this style is valid and in our docs :) https://docs.silverstripe.org/en/5/developer_guides/configuration/configuration/#configuration-masks |
||||||
|
||||||
if (!isset($attributes[$groups])) { | ||||||
return $member; | ||||||
} | ||||||
|
||||||
// Get groups from saml response | ||||||
$groupTitles = $attributes[$groups]; | ||||||
|
||||||
foreach ($groupTitles as $groupTitle) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move the query outside the loop, so we are executing one query that fetches everything into Arraylist. then you can look at the titles and check if we have the group or if we need a new one. |
||||||
// Get Group object by Title | ||||||
$group = DataObject::get_one(Group::class, [ | ||||||
'"Group"."Title"' => $groupTitle | ||||||
]); | ||||||
|
||||||
// Create group if it doesn't exist yet | ||||||
if (!$group) { | ||||||
$group = new Group(); | ||||||
$group->Title = $groupTitle; | ||||||
$group->write(); | ||||||
} | ||||||
|
||||||
$member->Groups()->add($group); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we also want to remove users from groups they are no longer part of - this probably requires marking groups as 'synced' in some way so we know which ones to remove them from. Its quite likely that a site would have some groups synced with saml but others that are not (we've got at least one project with this requirement) - we wouldn't want to remove users from these 'manual' groups. |
||||||
} | ||||||
|
||||||
return $member; | ||||||
} | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.