Added comments to code and updated docs

docs/en/developer.md

@@ -21,6 +21,7 @@ We assume ADFS 2.0 or greater is used as an IdP.
   - [Service Provider (SP)](#service-provider-sp)
   - [Identity Provider (IdP)](#identity-provider-idp)
   - [Additional configuration for Azure AD](#additional-configuration-for-azure-ad)
+  - [User groups mapping](#user-groups-mapping)
   - [GUID Transformation](#guid-transformation)
 - [Establish trust](#establish-trust)
 - [Configure SilverStripe Authenticators](#configure-silverstripe-authenticators)
@@ -37,6 +38,7 @@ We assume ADFS 2.0 or greater is used as an IdP.
   - [Adjust the requested AuthN contexts](#adjust-the-requested-authn-contexts)
   - [Create your own SAML configuration for completely custom settings](#create-your-own-saml-configuration-for-completely-custom-settings)
   - [Additional GET Query Params for SAML](#additional-get-query-params-for-saml)
+  - [Automatically redirect after authentication](#automatically-redirect-after-authentication)
 - [Resources](#resources)
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
@@ -153,7 +155,7 @@ SilverStripe\SAML\Extensions\SAMLMemberExtension:
 
 ### User groups mapping
 
-By default, any new users logged in using SSO will not have any groups assigned to them. If you want them to have want to bring over the groups from the Provider via claims field, you could enable it via
+By default, any new users logged in using SSO will not have any groups assigned to them. User groups can be enabled via
 
 ```yml
 SilverStripe\SAML\Services\SAMLConfiguration:
@@ -409,7 +411,7 @@ this configuration allows you to add two GET query parameters to endpoint reques
 `https://your-idp.com/singleSignOnService/saml2?someGetQueryParameter=value&AnotherParameter=differentValue&SAMLRequest=XYZ....`
 
 ### Automatically redirect after authentication
-If the user has CMS permission and you want to redirect to the CMS after successful authentication, you can set the default login destination like this:
+If the user has CMS permission and you want to redirect to the CMS after successful authentication, you can set the default login destination via:
 
 ```yaml
 SilverStripe\Security\Security:

src/Control/SAMLController.php

@@ -203,14 +203,6 @@ public function acs()
             $member->$field = $attributes[$claim][0];
         }
 
-        $mapUserGroup = Config::inst()->get(SAMLConfiguration::class, 'map_user_group');
-        // Map user groups
-        if ($mapUserGroup) {
-            $mapper = SAMLUserGroupMapper::singleton();
-
-            $member = $mapper->map($attributes, $member);
-        }
-
         $member->SAMLSessionIndex = $auth->getSessionIndex();
 
         // This will trigger LDAP update through LDAPMemberExtension::memberLoggedIn, if the LDAP module is installed.
@@ -219,6 +211,14 @@ public function acs()
         // Both SAML and LDAP identify Members by the same GUID field.
         $member->write();
 
+        $mapUserGroup = Config::inst()->get(SAMLConfiguration::class, 'map_user_group');
+        // Map user groups
+        if ($mapUserGroup) {
+            $mapper = SAMLUserGroupMapper::singleton();
+
+            $member = $mapper->map($attributes, $member);
+        }
+
         // Hook for modifying login behaviour
         $this->extend('updateLogin');
 

src/Helpers/SAMLUserGroupMapper.php

@@ -57,11 +57,6 @@ public function map($attributes, $member): Member
                 $group->write();
             }
 
-            // Add group to user and make sure user has been created
-            if (!$member->exists()) {
-                $member->write();
-            }
-
             $member->Groups()->add($group);
         }