From e91258baf502577eb12aba4851dbde5e85267bd5 Mon Sep 17 00:00:00 2001
From: Yannick Warnier <ywarnier@beeznest.org>
Date: Thu, 25 Mar 2021 11:26:29 +0100
Subject: [PATCH] Plugin: Azure Active Directory: Improve error reporting

---
 .../azure_active_directory/src/callback.php   | 39 +++++++++----------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/plugin/azure_active_directory/src/callback.php b/plugin/azure_active_directory/src/callback.php
index d2e4d979c4..481ffe1d6a 100644
--- a/plugin/azure_active_directory/src/callback.php
+++ b/plugin/azure_active_directory/src/callback.php
@@ -1,6 +1,9 @@
 <?php
 /* For license terms, see /license.txt */
-
+/**
+ * Callback script for Azure. The URL of this file is sent to Azure as a
+ * point of contact to send particular signals.
+ */
 require __DIR__.'/../../../main/inc/global.inc.php';
 
 $plugin = AzureActiveDirectory::create();
@@ -8,7 +11,8 @@ $plugin = AzureActiveDirectory::create();
 $provider = $plugin->getProvider();
 
 if (!isset($_GET['code'])) {
-    // If we don't have an authorization code then get one
+    // If we don't have an authorization code then get one by redirecting
+    // users to Azure (with the callback URL information)
     $authUrl = $provider->getAuthorizationUrl();
 
     ChamiloSession::write('oauth2state', $provider->getState());
@@ -39,6 +43,8 @@ try {
         throw new Exception('Token not found.');
     }
 
+    // We use the e-mail to authenticate the user, so check that at least one
+    // e-mail source exists
     if (empty($me['mail']) || empty($me['mailNickname'])) {
         throw new Exception('Mail empty');
     }
@@ -54,28 +60,24 @@ try {
     );
 
     $userId = null;
-    // Check EXTRA_FIELD_ORGANISATION_EMAIL
+    // Get the user ID (if any) from the EXTRA_FIELD_ORGANISATION_EMAIL extra
+    // field
     if (!empty($organisationValue) && isset($organisationValue['item_id'])) {
         $userId = $organisationValue['item_id'];
     }
 
     if (empty($userId)) {
-        // Check EXTRA_FIELD_AZURE_ID
+        // If the previous step didn't work, get the user ID from
+        // EXTRA_FIELD_AZURE_ID
         if (!empty($azureValue) && isset($azureValue['item_id'])) {
             $userId = $azureValue['item_id'];
         }
     }
 
-    /*$emptyValues = empty($organisationValue['item_id']) || empty($azureValue['item_id']);
-    $differentValues = !$emptyValues && $organisationValue['item_id'] != $azureValue['item_id'];
-
-    if ($emptyValues || $differentValues) {
-        throw new Exception('Empty values');
-    }*/
-
     if (empty($userId)) {
+        // If we didn't find the user
         if ($plugin->get(AzureActiveDirectory::SETTING_PROVISION_USERS) === 'true') {
-            // Create user
+            // If the option is set to create users, create it
             $userId = UserManager::create_user(
                 $me['givenName'],
                 $me['surname'],
@@ -100,26 +102,21 @@ try {
                 throw new Exception(get_lang('UserNotAdded').' '.$me['mailNickname']);
             }
         } else {
-            throw new Exception('User not found when checking the extra fields.');
+            throw new Exception('User not found when checking the extra fields from '.$me['mail'].' or '.$me['mailNickname'].'.');
         }
     }
 
     $userInfo = api_get_user_info($userId);
 
     if (empty($userInfo)) {
-        throw new Exception('User not found');
+        throw new Exception('User '.$userId.' not found.');
     }
 
     if ($userInfo['active'] != '1') {
-        throw new Exception('account_inactive');
+        throw new Exception(get_lang('AccountInactive'));
     }
 } catch (Exception $exception) {
-    $message = Display::return_message($plugin->get_lang('InvalidId'), 'error');
-
-    if ($exception->getMessage() === 'account_inactive') {
-        $message = Display::return_message(get_lang('AccountInactive'), 'error');
-    }
-
+    $message = Display::return_message($exception->getMessage(), 'error');
     Display::addFlash($message);
     header('Location: '.api_get_path(WEB_PATH));
     exit;