diff --git a/README.md b/README.md index 3ebff4c..2cb2c9a 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,21 @@ [![Scrutinizer Code Quality](https://scrutinizer-ci.com/g/silverstripe/silverstripe-saml/badges/quality-score.png)](https://scrutinizer-ci.com/g/silverstripe/silverstripe-saml/) [![codecov](https://codecov.io/gh/silverstripe/silverstripe-saml/branch/master/graph/badge.svg)](https://codecov.io/gh/silverstripe/silverstripe-saml) +## Table of Contents + + + + + +- [Introduction](#introduction) +- [Requirements](#requirements) +- [Overview](#overview) +- [Security](#security) +- [In-depth guides](#in-depth-guides) +- [Changelog](#changelog) + + + ## Introduction This SilverStripe module provides single sign-on authentication integration with a SAML provider. diff --git a/_config/saml.yml b/_config/saml.yml index 4b519d5..c4e0e72 100644 --- a/_config/saml.yml +++ b/_config/saml.yml @@ -19,10 +19,12 @@ SilverStripe\SAML\Authenticators\SAMLAuthenticator: SilverStripe\SAML\Services\SAMLConfiguration: strict: true debug: false + expect_binary_nameid: true + allow_insecure_email_linking: false Security: # Algorithm that the toolkit will use on signing process. Options: # - 'http://www.w3.org/2000/09/xmldsig#rsa-sha1' # - 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256' # - 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha384' # - 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha512' - signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256" \ No newline at end of file + signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256" diff --git a/docs/en/azure-ad.md b/docs/en/azure-ad.md new file mode 100644 index 0000000..744ab08 --- /dev/null +++ b/docs/en/azure-ad.md @@ -0,0 +1,53 @@ + + + + +- [Azure AD administrator guide](#azure-ad-administrator-guide) + - [Table of contents](#table-of-contents) + - [Overview](#overview) + - [Creating a new Enterprise Application](#creating-a-new-enterprise-application) + - [FAQs](#faqs) + - [Why do we require using `objectId` instead of `userprincipalname`?](#why-do-we-require-using-objectid-instead-of-userprincipalname) + + + +# Azure AD administrator guide + +This guide will step you through configuring a new SAML integration in Azure AD, such that Azure AD can act as an Identity Provider (IdP) for a SilverStripe site. + +## Table of contents + +## Overview + +This is not an exhaustive guide, and it only covers Azure AD. Other SAML IdPs should have similar configuration, and the module should be able to work with them provided they can give the same guarantees about the attributes being passed through in the SAML assertion. + +As an implementor of the IdP, you need to ensure the following have been set up: + +* A bi-directional trust between SP and IdP (via metadata files) +* Ensuring that `user.objectId` is used as the User Identifier (aka SAML Name ID) rather than `user.userprincipalname` + +## Creating a new Enterprise Application + +1. Log in as an Azure portal administrator. +2. In the Azure portal, go to the ['Azure Active Directory' service](https://portal.azure.com/#blade/Microsoft_AAD_IAM/ActiveDirectoryMenuBlade/Overview) +3. Click on 'Enterprise Applications', then 'New application' +4. Select 'Non-gallery application' and enter a descriptive name (e.g. 'Intranet'), then click Add +5. Once the application is created, select 'Single sign-on' from the left-menu, then select 'SAML' +6. Either upload the metadata file provided by the site developers, or enter the Entity ID and Reply URLs provided by the developers. These should always have the same base hostname, with the Reply URL including '/saml/acs' at the end +7. Under 'User Attributes', select `user.objectId` as the User Identifier, instead of the default (`user.userprincipalname`) +8. Provide the 'App Federation Metadata Url' and the base64 copy of the certificate under section 4 to the site developers + +Once the site developers have deployed the certificates and login URLs, the configuration should be set up and ready to be tested. You will need to configure users or groups that have access to this application before Azure AD will pass them through to the website, you can do this from the 'Users and groups' sidebar menu. + +## FAQs + +### Why do we require using `objectId` instead of `userprincipalname`? + +This is because `user.userprincipalname` (UPN) is not guaranteed to be unique *forever*, it is only guaranteed to be unique for the lifetime of a user's account. This is problematic in some situations. For example: + +1. John Smith joins the organisation, and given a UPN of john.smith@contoso.com +2. John Smith is added to the list of users that can access the website, and is given special administrator privileges over the website +3. John then leaves the organisation and his account is deleted. The website is not aware that John has been deleted because SAML provides no mechanism for this, so the website still has an administrator account configured for John, but nobody can login to it right now. +4. A different John Smith starts at the organisation in a different role (e.g. not an administrator). They are given access to the website, and when they login they see they have administrative privileges. + +This is because the UPN is only unique at any given time, not forever. To ensure that we don't run into security issues with the above, we require that Azure AD integrations use the `objectId` of the user instead. This means that the second John Smith in the above example will have a new account created for him, as his `objectId` will not match the `objectId` of the former John Smith. diff --git a/docs/en/build-toc.sh b/docs/en/build-toc.sh index 8d81858..d15086d 100755 --- a/docs/en/build-toc.sh +++ b/docs/en/build-toc.sh @@ -6,6 +6,6 @@ if [ "$?" -ne "0" ]; then exit 2 fi -for file in adfs.md developer.md troubleshooting.md; do +for file in adfs.md azure-ad.md contributing.md developer.md tech_notes.md troubleshooting.md ../../README.md; do doctoc $file && sed -i "" "/Table of Contents.*generated with/d" $file done diff --git a/docs/en/contributing.md b/docs/en/contributing.md new file mode 100644 index 0000000..2f19c4a --- /dev/null +++ b/docs/en/contributing.md @@ -0,0 +1,35 @@ +# Contribution guidelines +This document describes additional contribution guidelines that apply to this module only. + +## Table of Contents + + + + + +- [Documentation](#documentation) +- [Adding new functionality](#adding-new-functionality) +- [Adding support for new identity providers (IdPs)](#adding-support-for-new-identity-providers-idps) + + + +## Documentation +All changes should be documented to a similar (or better) level as the existing code. + +If you add or change headings in any of the existing documentation files, or create a new markdown file, make sure you re-run `./build-toc`: + +- `npm install -g doctoc` +- `cd /path/to/module/docs/en` +- `./build-toc` + +If the above doesn't work because build-toc fails or similar, feel free to submit your PR without updating the table of contents and we can do this upon merge. + +## Adding new functionality +This module follows semantic versioning, therefore id you change anything that breaks backwards compatibility that will require a new major release. Please be clear about this when creating new pull requests. Alternatively, you can get you feature merged faster if you include YML configuration to disable your feature, and make it optional to enable. + +If you add additional functionality, make sure to document it both in the codebase as well as the appropriate file in the docs directory. + +## Adding support for new identity providers (IdPs) +Adding support for new IdPs generally means you also need to add documentation that describes how that IdP should be configured to work with this module. + +For an example of the level of detail to include, see the existing documentation for [Azure AD](azure-ad.md) and [ADFS](adfs.md). Generally, this should describe at a high level exactly what you expect from the administrator of the IdP, and should be a document that you are comfortable sending to your IdP sysadmin to implement. diff --git a/docs/en/developer.md b/docs/en/developer.md index a53e197..457824f 100644 --- a/docs/en/developer.md +++ b/docs/en/developer.md @@ -20,10 +20,11 @@ We assume ADFS 2.0 or greater is used as an IdP. - [A note on signature algorithm config](#a-note-on-signature-algorithm-config) - [Service Provider (SP)](#service-provider-sp) - [Identity Provider (IdP)](#identity-provider-idp) + - [Additional configuration for Azure AD](#additional-configuration-for-azure-ad) - [Establish trust](#establish-trust) - [Configure SilverStripe Authenticators](#configure-silverstripe-authenticators) - [Show the SAML Login button on login form](#show-the-saml-login-button-on-login-form) - - [Bypass auto login](#bypass-auto-login) + - [Automatically require SAML login for every request](#automatically-require-saml-login-for-every-request) - [Test the connection](#test-the-connection) - [Configure LDAP synchronisation](#configure-ldap-synchronisation) - [Connect with LDAP](#connect-with-ldap) @@ -31,6 +32,9 @@ We assume ADFS 2.0 or greater is used as an IdP. - [Debugging](#debugging) - [SAML debugging](#saml-debugging) - [Advanced SAML configuration](#advanced-saml-configuration) + - [Allow insecure linking-by-email](#allow-insecure-linking-by-email) + - [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) - [Resources](#resources) @@ -65,7 +69,10 @@ Contact your system administrator if you are not sure how to install these. You also need to make the certificate for your ADFS endpoint available to the SilverStripe site. Talk with your ADFS administrator to find out how to obtain this. -If you are managing ADFS yourself, consult the [ADFS administrator guide](adfs.md). +* In you are integrating with ADFS, direct the ADFS administrator to the [ADFS administrator guide](adfs.md). +* If you are integrating with Azure AD, direct the Azure AD administrator to the [Azure AD administrator guide](azure-ad.md). + +Note: For Azure AD, you will first need to decide on the Entity ID (see next step) so that you can provide this to the Azure AD administrator - they can't provide you the certificates until you provide them the Entity ID and Reply URL values. You may also be able to extract the certificate yourself from the IdP endpoint if it has already been configured: `https:///FederationMetadata/2007-06/FederationMetadata.xml`. @@ -73,11 +80,15 @@ You may also be able to extract the certificate yourself from the IdP endpoint i Now we need to make the *silverstripe-saml* module aware of where the certificates can be found. -Add the following configuration to `mysite/_config/saml.yml` (make sure to replace paths to the certificates and keys): +**Note:** If you are configuring this application for integration with Azure AD, a couple of extra keys need to be set. See the '[Additional configuration for Azure AD](#additional-configuration-for-azure-ad)' section below. + +Add the following configuration to `app/_config/saml.yml` (make sure to replace paths to the certificates and keys): ```yaml + --- Name: mysamlsettings +After: '#samlsettings' --- SilverStripe\SAML\Services\SAMLConfiguration: strict: true @@ -92,6 +103,7 @@ SilverStripe\SAML\Services\SAMLConfiguration: singleSignOnService: "https:///adfs/ls/" Security: signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256" + ``` If you don't use absolute paths, the certificate paths will be relative to the site web root. @@ -112,7 +124,7 @@ SilverStripe\SAML\Services\SAMLConfiguration: ### Service Provider (SP) - - `entityId`: This should be the base URL with https for the SP + - `entityId`: This should be the base URL with https for the SP (e.g. https://example.com) - `privateKey`: The private key used for signing SAML request - `x509cert`: The public key that the IdP is using for verifying a signed request @@ -122,13 +134,36 @@ SilverStripe\SAML\Services\SAMLConfiguration: - `x509cert`: The token-signing certificate from ADFS (base 64 encoded) - `singleSignOnService`: The endpoint on ADFS for where to send the SAML login request +### Additional configuration for Azure AD + +When configuring the module to support Azure AD, a couple of additional configuration values need to be set to work with Azure AD out of the box. The below configuration should be merged into the YML configuration you have added above. + +```yaml +SilverStripe\SAML\Services\SAMLConfiguration: + expect_binary_nameid: false + SP: + nameIdFormat: 'urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified' + +SilverStripe\SAML\Extensions\SAMLMemberExtension: + claims_field_mappings: + - 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name': 'Email' +``` + ## Establish trust -At this stage the SilverStripe site trusts the ADFS, but the ADFS does not have any way to establish the identity of the SilverStripe site. +At this stage the SilverStripe site trusts the IdP, but the IdP does not have any way to establish the identity of the SilverStripe site. + +The IdP should now be configured to extract the SP certificate from SilverStripe's SP endpoint. Once this is completed, bi-directional trust has been established and the authentication should be possible. -ADFS should now be configured to extract the SP certificate from SilverStripe's SP endpoint. Once this is completed, bi-directional trust has been established and the authentication should be possible. +*silverstripe-saml* has some specific requirements on how ADFS, Azure AD and other IdPs are configured. Consult one of the following guides depending on the IdP you are integrating with. -*silverstripe-saml* has some specific requirements on how ADFS is configured. If you are managing ADFS yourself, or you are assisting an ADFS administrator, consult the [ADFS administrator guide](adfs.md). +* [ADFS administrator guide](adfs.md) +* [Azure AD administrator guide](azure-ad.md) + +In particular, most IdPs will require that you provide them with the entity ID and reply URLs (sometimes called the Assertion Consumer Service URL or ACS URL). These can be found by going to https:///saml/metadata once the above YML configuration is in place. + +* The Entity ID is the URL exactly as you have entered it in the YML above, which should be the URL to the root of your website (e.g. https://example.com) +* The Reply URL is the Entity ID, with the suffix '/saml/acs' added to the end (e.g. https://example.com/saml/acs) ## Configure SilverStripe Authenticators @@ -243,6 +278,36 @@ Also ensure that all protocols are matching. SAML is very sensitive to differenc ## Advanced SAML configuration +### Allow insecure linking-by-email + +Normally the SAML module looks for a `Member` record based only on the GUID returned by the IdP. However, this can break in some situations, particularly when you are retrofitting single-sign-on into an existing system that already has member records in the database. A common use-case is that the website is setup, with centralised SSO being added later. At that point, you already have members that are setup with standard email/password logins, and those email addresses are the same as the user's primary email in the IdP. When the SAML module searches for a user when they login via SSO for the first time, it won't find them based on GUID, and will throw an error because it will attempt to create a new member with the same email as an existing user. + +For this reason, the `allow_insecure_email_linking` YML config variable exists. During the transition period, you can enable this option so that if the lookup-by-GUID fails to find a valid member, the module will then attempt to lookup via the provided email address before falling back to creating a new member record. + +**Note: This is not recommended in production.** If this setting is enabled, then we fall back to relying on the non-unique email address to log in to an existing member's account. For example, consider the situation where John Smith previously had an email/password login to your website. They leave the company, and a new John Smith (unrelated to the website) inherits the email address when they join the company. If this option is enabled and the new John Smith attempts to login, despite them not being allowed access, we will set the user up and link them to the old accounts (and whatever permissions the old user had). + +We strongly recommend that you perform a full review of all users and permission levels for all members in the CMS **before you enable this setting** to ensure you will only create accounts for people that currently exist at the IdP. + +You can enable this setting with the following YML config: + +```yaml + +--- +Name: mysamlsettings +After: '#samlsettings' +--- +SilverStripe\SAML\Services\SAMLConfiguration: + allow_insecure_email_linking: true +``` + +**Note**: You will also need to specify a `SAMLMemberExtension.claims_field_mappings` claim map that sets a value for 'Email', so that the IdP provides a value to include in the email field, for example: + +```yaml +SilverStripe\SAML\Extensions\SAMLMemberExtension: + claims_field_mappings: + - 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress': 'Email' +``` + ### Adjust the requested AuthN contexts By default, this module requests the following contexts (aka. 'ways users can login'): diff --git a/docs/en/tech_notes.md b/docs/en/tech_notes.md index ce5ff92..4d14a3b 100644 --- a/docs/en/tech_notes.md +++ b/docs/en/tech_notes.md @@ -1,3 +1,14 @@ + + + + +- [Technical notes](#technical-notes) + - [Interface between SAML and LDAP](#interface-between-saml-and-ldap) + - [SAML+LDAP sequence](#samlldap-sequence) + - [Member record manipulation](#member-record-manipulation) + + + # Technical notes ## Interface between SAML and LDAP diff --git a/src/Control/SAMLController.php b/src/Control/SAMLController.php index 97473af..8a6e732 100644 --- a/src/Control/SAMLController.php +++ b/src/Control/SAMLController.php @@ -3,10 +3,13 @@ namespace SilverStripe\SAML\Control; use Exception; +use function gmmktime; use OneLogin\Saml2\Auth; +use OneLogin\Saml2\Constants; use OneLogin\Saml2\Utils; use OneLogin\Saml2\Error; use Psr\Log\LoggerInterface; +use SilverStripe\Core\Config\Config; use SilverStripe\ORM\ValidationResult; use SilverStripe\SAML\Authenticators\SAMLAuthenticator; use SilverStripe\SAML\Authenticators\SAMLLoginForm; @@ -15,6 +18,8 @@ use SilverStripe\Control\Director; use SilverStripe\Control\HTTPResponse; use SilverStripe\Core\Injector\Injector; +use SilverStripe\SAML\Model\SAMLResponse; +use SilverStripe\SAML\Services\SAMLConfiguration; use SilverStripe\Security\IdentityStore; use SilverStripe\Security\Member; use SilverStripe\Security\Security; @@ -61,6 +66,9 @@ public function acs() $auth = Injector::inst()->get(SAMLHelper::class)->getSAMLAuth(); $caughtException = null; + // Log both errors (reported by php-saml and thrown as exception) with a common ID for later tracking + $uniqueErrorId = uniqid('SAML-'); + // Force php-saml module to use the current absolute base URL (e.g. https://www.example.com/saml). This avoids // errors that we otherwise get when having a multi-directory ACS URL like /saml/acs). // See https://github.com/onelogin/php-saml/issues/249 @@ -77,15 +85,13 @@ public function acs() } // If there was an issue with the SAML response, if it was missing or if the SAML response indicates that they - // aren't authorised, then log the issue and provide a traceable error back to the user via the LoginForm - if ($caughtException || !empty($error) || !$auth->isAuthenticated()) { - // Log both errors (reported by php-saml and thrown as exception) with a common ID for later tracking - $id = uniqid('SAML-'); - + // aren't authorised, then log the issue and provide a traceable error back to the user via the login form + $hasError = $caughtException || !empty($error); + if ($hasError || !$auth->isAuthenticated() || $this->checkForReplayAttack($auth, $uniqueErrorId)) { if ($caughtException instanceof Exception) { $this->getLogger()->error(sprintf( '[%s] [code: %s] %s (%s:%s)', - $id, + $uniqueErrorId, $e->getCode(), $e->getMessage(), $e->getFile(), @@ -94,7 +100,7 @@ public function acs() } if (!empty($error)) { - $this->getLogger()->error(sprintf('[%s] %s', $id, $error)); + $this->getLogger()->error(sprintf('[%s] %s', $uniqueErrorId, $error)); } $this->getForm()->sessionMessage( @@ -102,7 +108,7 @@ public function acs() 'SilverStripe\\SAML\\Control\\SAMLController.ERR_SAML_ACS_FAILURE', 'Unfortunately we couldn\'t log you in. If this continues, please contact your I.T. department' . ' with the following reference: {ref}', - ['ref' => $id] + ['ref' => $uniqueErrorId] ), ValidationResult::TYPE_ERROR ); @@ -112,43 +118,62 @@ public function acs() return $this->redirect('Security/login'); } - // If processing reaches here, then the user is authenticated - the rest of this method is just processing their - // legitimate information and configuring their account. + /** + * If processing reaches here, then the user is authenticated - the rest of this method is just processing their + * legitimate information and configuring their account. + */ - // Check that the NameID is a binary string (which signals that it is a guid - $decodedNameId = base64_decode($auth->getNameId()); - if (ctype_print($decodedNameId)) { - $this->getForm()->sessionMessage('NameID from IdP is not a binary GUID.', ValidationResult::TYPE_ERROR); - $this->getRequest()->getSession()->save($this->getRequest()); - return $this->getRedirect(); - } + // If we expect the NameID to be a binary version of the GUID (ADFS), check that it actually is + // If we are configured not to expect a binary NameID, then we assume it is a direct GUID (Azure AD) + if (Config::inst()->get(SAMLConfiguration::class, 'expect_binary_nameid')) { + $decodedNameId = base64_decode($auth->getNameId()); + if (ctype_print($decodedNameId)) { + $this->getForm()->sessionMessage('NameID from IdP is not a binary GUID.', ValidationResult::TYPE_ERROR); + $this->getRequest()->getSession()->save($this->getRequest()); + return $this->getRedirect(); + } - // transform the NameId to guid - $helper = SAMLHelper::singleton(); - $guid = $helper->binToStrGuid($decodedNameId); - if (!$helper->validGuid($guid)) { - $errorMessage = "Not a valid GUID '{$guid}' recieved from server."; - $this->getLogger()->error($errorMessage); - $this->getForm()->sessionMessage($errorMessage, ValidationResult::TYPE_ERROR); - $this->getRequest()->getSession()->save($this->getRequest()); - return $this->getRedirect(); + // transform the NameId to guid + $helper = SAMLHelper::singleton(); + $guid = $helper->binToStrGuid($decodedNameId); + if (!$helper->validGuid($guid)) { + $errorMessage = "Not a valid GUID '{$guid}' recieved from server."; + $this->getLogger()->error($errorMessage); + $this->getForm()->sessionMessage($errorMessage, ValidationResult::TYPE_ERROR); + $this->getRequest()->getSession()->save($this->getRequest()); + return $this->getRedirect(); + } + } else { + $guid = $auth->getNameId(); } + $attributes = $auth->getAttributes(); + + $fieldToClaimMap = array_flip(Member::config()->claims_field_mappings); + // Write a rudimentary member with basic fields on every login, so that we at least have something - // if LDAP synchronisation fails. + // if there is no further sync (e.g. via LDAP) $member = Member::get()->filter('GUID', $guid)->limit(1)->first(); - if (!($member && $member->exists())) { + if (!($member && $member->exists()) && Config::inst()->get(SAMLConfiguration::class, 'allow_insecure_email_linking') && isset($fieldToClaimMap['Email'])) { + // If there is no member found via GUID and we allow linking via email, search by email + $member = Member::get()->filter('Email', $attributes[$fieldToClaimMap['Email']])->limit(1)->first(); + + if (!($member && $member->exists())) { + $member = new Member(); + } + + $member->GUID = $guid; + } elseif (!($member && $member->exists())) { + // If the member doesn't exist and we don't allow linking via email, then create a new member $member = new Member(); $member->GUID = $guid; } - $attributes = $auth->getAttributes(); - foreach ($member->config()->claims_field_mappings as $claim => $field) { if (!isset($attributes[$claim][0])) { $this->getLogger()->warning( sprintf( - 'Claim rule \'%s\' configured in LDAPMember.claims_field_mappings, ' . + 'Claim rule \'%s\' configured in SAMLMemberExtension.claims_field_mappings, ' . 'but wasn\'t passed through. Please check IdP claim rules.', $claim ) @@ -162,15 +187,15 @@ public function acs() $member->SAMLSessionIndex = $auth->getSessionIndex(); - // This will trigger LDAP update through LDAPMemberExtension::memberLoggedIn. The LDAP update will also write - // the Member record a second time, but the member must be written before IdentityStore->logIn() is called. - // Both SAML and LDAP identify Members by the GUID field. + // This will trigger LDAP update through LDAPMemberExtension::memberLoggedIn, if the LDAP module is installed. + // The LDAP update will also write the Member record a second time, but the member *must* be written before + // IdentityStore->logIn() is called, otherwise the identity store throws an exception. + // Both SAML and LDAP identify Members by the same GUID field. $member->write(); /** @var IdentityStore $identityStore */ $identityStore = Injector::inst()->get(IdentityStore::class); - $persistent = Security::config()->get('autologin_enabled'); - $identityStore->logIn($member, $persistent, $this->getRequest()); + $identityStore->logIn($member, false, $this->getRequest()); return $this->getRedirect(); } @@ -229,6 +254,52 @@ protected function getRedirect() return $this->redirect(Director::absoluteBaseURL()); } + /** + * If processing reaches here, then the user is authenticated but potentially not valid. We first need to confirm + * that they are not an attacker performing a SAML replay attack (capturing the raw traffic from a compromised + * device and then re-submitting the same SAML response). + * + * To combat this, we store SAML response IDs for the amount of time they're valid for (plus a configurable offset + * to account for potential time skew), and if the ID has been seen before we log an error message and return true + * (which indicates that this specific request is a replay attack). + * + * If no replay attack is detected, then the SAML response is logged so that future requests can be blocked. + * + * @param Auth $auth The Auth object that includes the processed response + * @param string $uniqueErrorId The error code to use when logging error messages for this given error + * @return bool true if this response is a replay attack, false if it's the first time we've seen the ID + */ + protected function checkForReplayAttack(Auth $auth, $uniqueErrorId = '') + { + $responseId = $auth->getLastMessageId(); + $expiry = $auth->getLastAssertionNotOnOrAfter(); // Note: Expiry will always be stored and returned in UTC + + // Search for any SAMLResponse objects where the response ID is the same and the expiry is within the range + $count = SAMLResponse::get()->filter(['ResponseID' => $responseId])->count(); + + if ($count > 0) { + // Response found, therefore this is a replay attack - log the error and return false so the user is denied + $this->getLogger()->error(sprintf( + '[%s] SAML replay attack detected! Response ID "%s", expires "%s", client IP "%s"', + $uniqueErrorId, + $responseId, + $expiry, + $this->getRequest()->getIP() + )); + + return true; + } else { + // No attack detected, log the SAML response + $response = new SAMLResponse([ + 'ResponseID' => $responseId, + 'Expiry' => $expiry + ]); + + $response->write(); + return false; + } + } + /** * Get a logger * diff --git a/src/Model/SAMLResponse.php b/src/Model/SAMLResponse.php new file mode 100644 index 0000000..dfb0464 --- /dev/null +++ b/src/Model/SAMLResponse.php @@ -0,0 +1,15 @@ + 'Varchar(255)', + 'Expiry' => 'Varchar(12)' // Returned by php-saml as a UTC datetime in unix epoch format + ]; +} diff --git a/src/Services/SAMLConfiguration.php b/src/Services/SAMLConfiguration.php index eb02639..c144b8f 100644 --- a/src/Services/SAMLConfiguration.php +++ b/src/Services/SAMLConfiguration.php @@ -52,6 +52,25 @@ class SAMLConfiguration */ private static $authn_contexts; + /** + * @config + * @var bool Whether or not we expect to receive a binary NameID from the IdP. We expect to receive a binary NameID + * from ADFS, but don't expect it from Azure AD or most other SAML implementations that provide GUIDs. + * + * Defaults to true to preserve backwards compatibility (ADFS). + */ + private static $expect_binary_nameid = true; + + /** + * @config + * @var bool Whether or not we allow searching for existing members in the SilverStripe database based on their + * email address. Marked as insecure because if warnings in developer documentation are not read and understood, + * this can provide access to the website to people who should not otherwise have access. + * + * Defaults to false to prevent looking up members based on email address. + */ + private static $allow_insecure_email_linking = false; + /** * @return array */