From 33b1897e55124e6d42dd28e36dbaf62228852991 Mon Sep 17 00:00:00 2001 From: Yannick Forget Date: Mon, 3 Oct 2022 08:40:35 +0200 Subject: [PATCH 1/3] MDL-61880 core: Retain OAuth 2.0 user field mappings after issuer update --- lib/classes/oauth2/api.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/classes/oauth2/api.php b/lib/classes/oauth2/api.php index 4eb1eb8513b..c6aece1b0e8 100644 --- a/lib/classes/oauth2/api.php +++ b/lib/classes/oauth2/api.php @@ -332,15 +332,15 @@ class api { // Will throw exceptions on validation failures. if ($create) { $issuer->create(); + + // Perform service discovery. + $classname = self::get_service_classname($issuer->get('servicetype')); + $classname::discover_endpoints($issuer); + self::guess_image($issuer); } else { $issuer->update(); } - // Perform service discovery. - $classname = self::get_service_classname($issuer->get('servicetype')); - $classname::discover_endpoints($issuer); - self::guess_image($issuer); - return $issuer; } From 177caaa3b5ceff54e5b7be77226fce801e1d3fbf Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Thu, 20 Oct 2022 11:08:39 +0800 Subject: [PATCH 2/3] MDL-61880 core: don't clear issuer fields not present in form data In cases where we have an id, load the persistent and then set new data before saving. This ensures we don't delete fields which are missing in the form data. --- lib/classes/oauth2/api.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/classes/oauth2/api.php b/lib/classes/oauth2/api.php index c6aece1b0e8..5aeac855a57 100644 --- a/lib/classes/oauth2/api.php +++ b/lib/classes/oauth2/api.php @@ -327,7 +327,10 @@ class api { */ protected static function create_or_update_issuer($data, bool $create): issuer { require_capability('moodle/site:config', context_system::instance()); - $issuer = new issuer(0, $data); + $issuer = new issuer($data->id ?? 0, $data); + if (!empty($data->id)) { + $issuer->set_many((array)$data); + } // Will throw exceptions on validation failures. if ($create) { From 93318586a5d0dd619c9ddde052b681ca0845fdde Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Mon, 31 Oct 2022 17:31:59 +0800 Subject: [PATCH 3/3] MDL-61880 tool_oauth2: don't rely on discovery when editing issuers Adjusts the testing scenarios so that we don't rely on endpoint discovery being run during an issuer edit, which is no longer the case. For scenarios that need a userinfo endpoint, set this manually. --- .../tool/oauth2/tests/behat/basic_settings.feature | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/admin/tool/oauth2/tests/behat/basic_settings.feature b/admin/tool/oauth2/tests/behat/basic_settings.feature index 311c84a3e86..6a190be0c3f 100644 --- a/admin/tool/oauth2/tests/behat/basic_settings.feature +++ b/admin/tool/oauth2/tests/behat/basic_settings.feature @@ -135,7 +135,6 @@ Feature: Basic OAuth2 functionality And I set the following fields to these values: | Name | Testing service modified | And I press "Save changes" - And I should see "Could not discover service endpoints" And I should see "Testing service modified" And I click on "Delete" "link" in the "Testing service modified" "table_row" And I should see "Are you sure you want to delete the identity issuer \"Testing service modified\"?" @@ -240,13 +239,12 @@ Feature: Basic OAuth2 functionality And I press "Save changes" And "Do not allow login" "icon" should exist in the "Valid custom" "table_row" And "Allow services" "icon" should exist in the "Valid custom" "table_row" - And "Service discovery successful" "icon" should exist in the "Valid custom" "table_row" + And I should see "-" in the "Valid custom" "table_row" And I click on "Edit" "link" in the "Valid custom service" "table_row" And I set the following fields to these values: | Name | Invalid custom service | | Service base URL | https://dc.imsglobal.org/ | And I press "Save changes" - Then I should see "Could not discover end points for identity issuer: Invalid custom service" And I should see "-" in the "Invalid custom service" "table_row" And I click on "Delete" "link" in the "Invalid custom service" "table_row" And I should see "Are you sure you want to delete the identity issuer \"Invalid custom service\"?" @@ -282,13 +280,12 @@ Feature: Basic OAuth2 functionality And I press "Save changes" And "Do not allow login" "icon" should exist in the "Valid custom" "table_row" And "Allow services" "icon" should exist in the "Valid custom" "table_row" - And "Service discovery successful" "icon" should exist in the "Valid custom" "table_row" + And I should see "-" in the "Valid custom" "table_row" And I click on "Edit" "link" in the "Valid custom service" "table_row" And I set the following fields to these values: | Name | Invalid custom service | | Service base URL | https://dc.imsglobal.org/ | And I press "Save changes" - Then I should see "Could not discover end points for identity issuer: Invalid custom service" And I should see "-" in the "Invalid custom service" "table_row" And I click on "Edit" "link" in the "Invalid custom service" "table_row" And I set the following fields to these values: @@ -352,9 +349,12 @@ Feature: Basic OAuth2 functionality And I should see "Custom display name" in the "Empty custom service" "table_row" And "Not configured" "icon" should exist in the "Empty custom service" "table_row" And "Do not allow services" "icon" should exist in the "Empty custom service" "table_row" - And I click on "Edit" "link" in the "Empty custom service" "table_row" + And I click on "Configure endpoints" "link" in the "Empty custom service" "table_row" + And I press "Create new endpoint for issuer \"Empty custom service\"" And I set the following fields to these values: - | Service base URL | https://accounts.google.com | + | Name | userinfo_endpoint | + | URL | https://example.com/userinfo | And I press "Save changes" + And I navigate to "Server > OAuth 2 services" in site administration And "Allow login" "icon" should exist in the "Empty custom service" "table_row" And "Do not allow services" "icon" should exist in the "Empty custom service" "table_row"