From d1c0acdd55c16cf7f6bfd9c2e642cc93a55b9c46 Mon Sep 17 00:00:00 2001
From: Peter Dias <peter@moodle.com>
Date: Thu, 27 May 2021 15:07:23 +0800
Subject: [PATCH] MDL-65637 core_oauth2: Create new oauth2 mapping function

Separated userinfo mapping into a function and new unit test
---
 lib/classes/oauth2/client.php |  29 +++++++-
 lib/tests/client_test.php     | 125 ++++++++++++++++++++++++++++++++++
 2 files changed, 152 insertions(+), 2 deletions(-)
 create mode 100644 lib/tests/client_test.php

diff --git a/lib/classes/oauth2/client.php b/lib/classes/oauth2/client.php
index 6c49aaaa67c..85f0c79c148 100644
--- a/lib/classes/oauth2/client.php
+++ b/lib/classes/oauth2/client.php
@@ -500,6 +500,16 @@ class client extends \oauth2_client {
             return false;
         }
 
+        return $this->map_userinfo_to_fields($userinfo);
+    }
+
+    /**
+     * Maps the oauth2 response to userfields.
+     *
+     * @param stdClass $userinfo
+     * @return array
+     */
+    protected function map_userinfo_to_fields(stdClass $userinfo): array {
         $map = $this->get_userinfo_mapping();
 
         $user = new stdClass();
@@ -507,10 +517,25 @@ class client extends \oauth2_client {
             // We support nested objects via a-b-c syntax.
             $getfunc = function($obj, $prop) use (&$getfunc) {
                 $proplist = explode('-', $prop, 2);
-                if (empty($proplist[0]) || empty($obj->{$proplist[0]})) {
+
+                // The value of proplist[0] can be falsey, so just check if not set.
+                if (empty($obj) || !isset($proplist[0])) {
+                    return false;
+                }
+
+                if (preg_match('/^(.*)\[([0-9]*)\]$/', $proplist[0], $matches)
+                        && count($matches) == 3) {
+                    $property = $matches[1];
+                    $index = $matches[2];
+                    $obj = $obj->{$property}[$index] ?? null;
+                } else if (!empty($obj->{$proplist[0]})) {
+                    $obj = $obj->{$proplist[0]};
+                } else if (is_array($obj) && !empty($obj[$proplist[0]])) {
+                    $obj = $obj[$proplist[0]];
+                } else {
+                    // Nothing found after checking all possible valid combinations, return false.
                     return false;
                 }
-                $obj = $obj->{$proplist[0]};
 
                 if (count($proplist) > 1) {
                     return $getfunc($obj, $proplist[1]);
diff --git a/lib/tests/client_test.php b/lib/tests/client_test.php
new file mode 100644
index 00000000000..2f6a7183847
--- /dev/null
+++ b/lib/tests/client_test.php
@@ -0,0 +1,125 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Unit test client_test.
+ *
+ * Unit test for testable functions in core/oauth2/client.php
+ *
+ * @copyright  2021 Peter Dias
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ * @package    core
+ */
+class client_test extends advanced_testcase {
+    /**
+     * Uses the static dataset as feed-in
+     *
+     * @return array
+     */
+    public function map_response_provider(): array {
+        return [
+            "Nested objects syntax a-b-c syntax " => [
+                [
+                    "name-firstname" => "firstname",
+                    "contact-phone-home" => "homenumber",
+                ], [
+                    "firstname" => "John",
+                    "homenumber" => "020000000",
+                ]
+            ],
+            "Nested objects syntax with array support a-b[0]-c syntax " => [
+                [
+                    "name-firstname" => "firstname",
+                    "contact-phone-home" => "homenumber",
+                    "picture[0]-url" => "urltest",
+                ], [
+                    "firstname" => "John",
+                    "homenumber" => "020000000",
+                    "urltest" => "www.google.com",
+                ]
+            ],
+            "Nested objects syntax with array support a-b-0-c syntax " => [
+                [
+                    "name-firstname" => "firstname",
+                    "contact-phone-home" => "homenumber",
+                    "picture-0-url" => "urltest",
+                ], [
+                    "firstname" => "John",
+                    "homenumber" => "020000000",
+                    "urltest" => "www.google.com",
+                ]
+            ],
+            "Nested objects syntax with array support a-b-0-c syntax with non-existent nodes" => [
+                [
+                    "name-firstname" => "firstname",
+                    "contact-phone-home" => "homenumber",
+                    "picture-0-url-url" => "urltest",
+                ], [
+                    "firstname" => "John",
+                    "homenumber" => "020000000",
+                ]
+            ],
+        ];
+    }
+
+    /**
+     * Test the map_userinfo_to_fields function
+     *
+     * @dataProvider map_response_provider
+     * @param array $mapping
+     * @param array $expected
+     * @throws ReflectionException
+     */
+    public function test_map_userinfo_to_fields(array $mapping, array $expected) {
+        $dataset = [
+            "name" => (object) [
+                "firstname" => "John",
+                "lastname" => "Doe",
+            ],
+            "contact" => (object) [
+                "email" => "john@example.com",
+                "phone" => (object) [
+                    "mobile" => "010000000",
+                    "home" => "020000000"
+                ],
+            ],
+            "picture" => [
+                [
+                    "url" => "www.google.com",
+                    "description" => "This is a URL",
+                ],
+                [
+                    "url" => "www.facebook.com",
+                    "description" => "This is another URL",
+                ]
+            ]
+        ];
+
+        $method = new ReflectionMethod("core\oauth2\client", "map_userinfo_to_fields");
+        $method->setAccessible(true);
+
+        $issuer = new \core\oauth2\issuer(0);
+        $mockbuilder = $this->getMockBuilder('core\oauth2\client');
+        $mockbuilder->onlyMethods(['get_userinfo_mapping']);
+        $mockbuilder->setConstructorArgs([$issuer, "", ""]);
+
+        $mock = $mockbuilder->getMock();
+        $mock->expects($this->once())
+            ->method('get_userinfo_mapping')
+            ->will($this->returnValue($mapping));
+        $this->assertSame($expected, $method->invoke($mock, (object) $dataset));
+    }
+}