From 9bba84684fa53a6c4f38c4fd7cea08552208b228 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sat, 16 Nov 2019 15:24:09 +0000
Subject: [PATCH] Appeased codeclimate by extracting out external_auth_id group
 matching

---
 app/Auth/Access/ExternalAuthService.php | 29 +++++++++++++++----------
 app/Auth/Access/Saml2Service.php        |  3 +--
 app/Auth/Role.php                       |  7 ++++++
 3 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/app/Auth/Access/ExternalAuthService.php b/app/Auth/Access/ExternalAuthService.php
index 77c7d1351..4bd8f8680 100644
--- a/app/Auth/Access/ExternalAuthService.php
+++ b/app/Auth/Access/ExternalAuthService.php
@@ -9,26 +9,33 @@ class ExternalAuthService
     /**
      * Check a role against an array of group names to see if it matches.
      * Checked against role 'external_auth_id' if set otherwise the name of the role.
-     * @param \BookStack\Auth\Role $role
-     * @param array $groupNames
-     * @return bool
      */
-    protected function roleMatchesGroupNames(Role $role, array $groupNames)
+    protected function roleMatchesGroupNames(Role $role, array $groupNames): bool
     {
         if ($role->external_auth_id) {
-            $externalAuthIds = explode(',', strtolower($role->external_auth_id));
-            foreach ($externalAuthIds as $externalAuthId) {
-                if (in_array(trim($externalAuthId), $groupNames)) {
-                    return true;
-                }
-            }
-            return false;
+            return $this->externalIdMatchesGroupNames($role->external_auth_id, $groupNames);
         }
 
         $roleName = str_replace(' ', '-', trim(strtolower($role->display_name)));
         return in_array($roleName, $groupNames);
     }
 
+    /**
+     * Check if the given external auth ID string matches one of the given group names.
+     */
+    protected function externalIdMatchesGroupNames(string $externalId, array $groupNames): bool
+    {
+        $externalAuthIds = explode(',', strtolower($externalId));
+
+        foreach ($externalAuthIds as $externalAuthId) {
+            if (in_array(trim($externalAuthId), $groupNames)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     /**
      * Match an array of group names to BookStack system roles.
      * Formats group names to be lower-case and hyphenated.
diff --git a/app/Auth/Access/Saml2Service.php b/app/Auth/Access/Saml2Service.php
index bb57ceb73..57db3ce65 100644
--- a/app/Auth/Access/Saml2Service.php
+++ b/app/Auth/Access/Saml2Service.php
@@ -147,10 +147,9 @@ class Saml2Service extends ExternalAuthService
     protected function registerUser(array $userDetails): User
     {
         // Create an array of the user data to create a new user instance
-
         $userData = [
             'name' => $userDetails['name'],
-            'email' => $userDetails['email'] ?? '',
+            'email' => $userDetails['email'],
             'password' => Str::random(32),
             'external_auth_id' => $userDetails['external_id'],
             'email_confirmed' => true,
diff --git a/app/Auth/Role.php b/app/Auth/Role.php
index 712f5299b..3342ef5a8 100644
--- a/app/Auth/Role.php
+++ b/app/Auth/Role.php
@@ -4,6 +4,13 @@ use BookStack\Auth\Permissions\JointPermission;
 use BookStack\Auth\Permissions\RolePermission;
 use BookStack\Model;
 
+/**
+ * Class Role
+ * @property string $display_name
+ * @property string $description
+ * @property string $external_auth_id
+ * @package BookStack\Auth
+ */
 class Role extends Model
 {