Merge branch 'MDL-78132-master' of https://github.com/sarjona/moodle

admin/tool/oauth2/issuers.php

@@ -210,12 +210,6 @@
     $addurl = new moodle_url('/admin/tool/oauth2/issuers.php', $params);
     echo $renderer->single_button($addurl, get_string('nextcloud_service', 'tool_oauth2'));
 
-    // IMS Open Badges Connect template.
-    $docs = 'admin/tool/oauth2/issuers/imsobv2p1';
-    $params = ['action' => 'edittemplate', 'type' => 'imsobv2p1', 'sesskey' => sesskey(), 'docslink' => $docs];
-    $addurl = new moodle_url('/admin/tool/oauth2/issuers.php', $params);
-    echo $renderer->single_button($addurl, get_string('imsobv2p1_service', 'tool_oauth2'));
-
     // Linkedin template.
     $docs = 'admin/tool/oauth2/issuers/linkedin';
     $params = ['action' => 'edittemplate', 'type' => 'linkedin', 'sesskey' => sesskey(), 'docslink' => $docs];

admin/tool/oauth2/lang/en/deprecated.txt

@@ -0,0 +1 @@
+imsobv2p1_service,tool_oauth2

admin/tool/oauth2/lang/en/tool_oauth2.php

@@ -56,7 +56,6 @@
 $string['endpointurl'] = 'URL';
 $string['facebook_service'] = 'Facebook';
 $string['google_service'] = 'Google';
-$string['imsobv2p1_service'] = 'Open Badges';
 $string['issuersetup'] = 'Detailed instructions on configuring the common OAuth 2 services';
 $string['issuersetuptype'] = 'Detailed instructions on setting up the {$a} OAuth 2 provider';
 $string['issueralloweddomains_help'] = 'If set, this setting is a comma separated list of domains that logins will be restricted to when using this provider.';
@@ -125,3 +124,6 @@
 $string['userfieldmappingdeleted'] = 'User field mapping deleted';
 $string['userfieldmappingsforissuer'] = 'User field mappings for issuer: {$a}';
 $string['privacy:metadata'] = 'The OAuth 2 services plugin does not store any personal data.';
+
+// Deprecated since Moodle 4.3.
+$string['imsobv2p1_service'] = 'Open Badges';

admin/tool/oauth2/tests/behat/basic_settings.feature

@@ -142,40 +142,6 @@ Feature: Basic OAuth2 functionality
     And I should see "Identity issuer deleted"
     And I should not see "Testing service modified"
 
-  Scenario: Create, edit and delete standard service for Open Badges
-    Given I press "Open Badges"
-    And I should see "Create new service: Open Badges"
-    And I set the following fields to these values:
-      | Client ID                  | thisistheclientid                         |
-      | Client secret              | supersecret                               |
-      | Service base URL           | https://dc.imsglobal.org/                 |
-    When I press "Save changes"
-    Then I should see "Changes saved"
-    And I should see "IMS Global Reference Implementation"
-    And I click on "Edit" "link" in the "IMS Global Reference Implementation" "table_row"
-    And I set the following fields to these values:
-      | Name                       | IMS Global                                |
-    And I press "Save changes"
-    And I should see "Changes saved"
-    And I should see "IMS Global"
-    And "Allow services" "icon" should exist in the "IMS Global" "table_row"
-    And "Do not allow login" "icon" should exist in the "IMS Global" "table_row"
-    And "Service discovery successful" "icon" should exist in the "IMS Global" "table_row"
-    And the "src" attribute of "table.admintable th img" "css_element" should contain "IMS-Global-Logo.png"
-    And I click on "Configure endpoints" "link" in the "IMS Global" "table_row"
-    And I should see "https://dc.imsglobal.org/.well-known/badgeconnect.json" in the "discovery_endpoint" "table_row"
-    And I should see "authorization_endpoint"
-    And I navigate to "Server > OAuth 2 services" in site administration
-    And I click on "Configure user field mappings" "link" in the "IMS Global" "table_row"
-    And I should not see "given_name"
-    And I should not see "middle_name"
-    And I navigate to "Server > OAuth 2 services" in site administration
-    And I click on "Delete" "link" in the "IMS Global" "table_row"
-    And I should see "Are you sure you want to delete the identity issuer \"IMS Global\"?"
-    And I press "Continue"
-    And I should see "Identity issuer deleted"
-    And I should not see "IMS Global"
-
   Scenario: Create, edit and delete valid custom OIDC service
     Given I press "Custom"
     And I should see "Create new service: Custom"

badges/classes/form/external_backpack.php

@@ -53,21 +53,20 @@ public function definition() {
         $mform->addElement('hidden', 'action', 'edit');
         $mform->setType('action', PARAM_ALPHA);
 
-        $mform->addElement('text', 'backpackapiurl',  get_string('backpackapiurl', 'core_badges'));
-        $mform->setType('backpackapiurl', PARAM_URL);
-        $mform->addRule('backpackapiurl', null, 'required', null, 'client');
-        $mform->addRule('backpackapiurl', get_string('maximumchars', '', 255), 'maxlength', 255, 'client');
+        $apiversions = badges_get_badge_api_versions();
+        $mform->addElement('select', 'apiversion', get_string('apiversion', 'core_badges'), $apiversions);
+        $mform->setType('apiversion', PARAM_RAW);
+        $mform->setDefault('apiversion', OPEN_BADGES_V2P1);
+        $mform->addRule('apiversion', null, 'required', null, 'client');
 
         $mform->addElement('text', 'backpackweburl', get_string('backpackweburl', 'core_badges'));
         $mform->setType('backpackweburl', PARAM_URL);
         $mform->addRule('backpackweburl', null, 'required', null, 'client');
         $mform->addRule('backpackweburl', get_string('maximumchars', '', 255), 'maxlength', 255, 'client');
 
-        $apiversions = badges_get_badge_api_versions();
-        $mform->addElement('select', 'apiversion', get_string('apiversion', 'core_badges'), $apiversions);
-        $mform->setType('apiversion', PARAM_RAW);
-        $mform->setDefault('apiversion', OPEN_BADGES_V2P1);
-        $mform->addRule('apiversion', null, 'required', null, 'client');
+        $mform->addElement('text', 'backpackapiurl',  get_string('backpackapiurl', 'core_badges'));
+        $mform->setType('backpackapiurl', PARAM_URL);
+        $mform->addRule('backpackapiurl', get_string('maximumchars', '', 255), 'maxlength', 255, 'client');
 
         $mform->addElement('hidden', 'id', ($backpack->id ?? null));
         $mform->setType('id', PARAM_INT);
@@ -86,11 +85,6 @@ public function definition() {
         $issuercontact = $CFG->badges_defaultissuercontact;
         $this->add_auth_fields($issuercontact);
 
-        $oauth2options = badges_get_oauth2_service_options();
-        $mform->addElement('select', 'oauth2_issuerid', get_string('oauth2issuer', 'core_badges'), $oauth2options);
-        $mform->setType('oauth2_issuerid', PARAM_INT);
-        $mform->hideIf('oauth2_issuerid', 'apiversion', 'neq', '2.1');
-
         if ($backpack) {
             $this->set_data($backpack);
         }
@@ -99,7 +93,8 @@ public function definition() {
         $mform->hideIf('backpackemail', 'includeauthdetails');
         $mform->hideIf('backpackemail', 'apiversion', 'in', [OPEN_BADGES_V2P1]);
         $mform->hideIf('password', 'includeauthdetails');
-        $mform->hideIf('password', 'apiversion', 'in', [1, OPEN_BADGES_V2P1]);
+        $mform->hideIf('password', 'apiversion', 'in', [OPEN_BADGES_V1, OPEN_BADGES_V2P1]);
+        $mform->hideIf('backpackapiurl', 'apiversion', 'in', [OPEN_BADGES_V1, OPEN_BADGES_V2P1]);
 
         // Disable short forms.
         $mform->setDisableShortforms();
@@ -117,9 +112,14 @@ public function definition() {
     public function validation($data, $files) {
         $errors = parent::validation($data, $files);
 
-        // Ensure backpackapiurl and  are valid URLs.
-        if (!empty($data['backpackapiurl']) && !preg_match('@^https?://.+@', $data['backpackapiurl'])) {
-            $errors['backpackapiurl'] = get_string('invalidurl', 'badges');
+        // Ensure backpackapiurl and backpackweburl are valid URLs.
+        $isobv21 = isset($data['apiversion']) && $data['apiversion'] == OPEN_BADGES_V2P1;
+        if (!$isobv21) {
+            if (empty($data['backpackapiurl'])) {
+                $errors['backpackapiurl'] = get_string('err_required', 'form');
+            } else if (!preg_match('@^https?://.+@', $data['backpackapiurl'])) {
+                $errors['backpackapiurl'] = get_string('invalidurl', 'badges');
+            }
         }
         if (!empty($data['backpackweburl']) && !preg_match('@^https?://.+@', $data['backpackweburl'])) {
             $errors['backpackweburl'] = get_string('invalidurl', 'badges');

badges/mybackpack.php

@@ -129,7 +129,7 @@
             // User input username/email/password on the backpack site
             // After confirm the scopes.
             redirect(new moodle_url('/badges/backpack-connect.php', ['backpackid' => $data->externalbackpackid]));
-        } else if ($data = $form->get_data()) {
+        } else {
             // The form may have been submitted under one of the following circumstances:
             // 1. After clicking 'Connect to backpack'. We'll have $data->email.
             // 2. After clicking 'Resend verification email'. We'll have $data->email.

badges/tests/behat/backpack.feature

@@ -120,12 +120,15 @@ Feature: Backpack badges
     And I log in as "admin"
     And I navigate to "Badges > Manage backpacks" in site administration
     When I press "Add a new backpack"
-    And I set the field "backpackapiurl" to "http://backpackapiurl.cat"
+    And I set the field "apiversion" to "2"
     And I set the field "backpackweburl" to "aaa"
     And I press "Save changes"
     And I should see "Invalid URL"
     And I set the field "backpackweburl" to "http://backpackweburl.cat"
     And I press "Save changes"
+    And I should see "You must supply a value here"
+    And I set the field "backpackapiurl" to "http://backpackapiurl.cat"
+    And I press "Save changes"
     Then I should see "http://backpackweburl.cat"
     And "Delete" "icon" should exist in the "http://backpackweburl.cat" "table_row"
     And "Edit settings" "icon" should exist in the "http://backpackweburl.cat" "table_row"
@@ -163,9 +166,9 @@ Feature: Backpack badges
     And I log in as "admin"
     And I navigate to "Badges > Manage backpacks" in site administration
     When I press "Add a new backpack"
-    And I set the field "backpackapiurl" to "http://backpackapiurl.cat"
-    And I set the field "backpackweburl" to "http://backpackweburl.cat"
     And I set the field "apiversion" to "2.1"
+    And I set the field "backpackweburl" to "http://backpackweburl.cat"
+    And I should not see "Backpack API URL"
     Then "Include authentication details with the backpack" "checkbox" should not be visible
     And I should not see "Badge issuer email address"
     And I should not see "Badge issuer password"
@@ -180,6 +183,7 @@ Feature: Backpack badges
     And I should see "Badge issuer password"
     And I set the field "backpackemail" to "test@test.com"
     And I set the field "password" to "123456"
+    And I set the field "backpackapiurl" to "http://backpackapiurl.cat"
     And I press "Save changes"
     And I click on "Edit" "link" in the "http://backpackweburl.cat" "table_row"
     And the field "Include authentication details with the backpack" matches value "1"

lib/badgeslib.php

@@ -798,6 +798,40 @@ function badges_delete_site_backpack($id) {
  */
 function badges_save_external_backpack(stdClass $data) {
     global $DB;
+    if ($data->apiversion == OPEN_BADGES_V2P1) {
+        // Check if there is an existing issuer for the given backpackapiurl.
+        foreach (core\oauth2\api::get_all_issuers() as $tmpissuer) {
+            if ($data->backpackweburl == $tmpissuer->get('baseurl')) {
+                $issuer = $tmpissuer;
+                break;
+            }
+        }
+
+        // Create the issuer if it doesn't exist yet.
+        if (empty($issuer)) {
+            $issuer = new \core\oauth2\issuer(0, (object) [
+                'name' => $data->backpackweburl,
+                'baseurl' => $data->backpackweburl,
+                // Note: This is required because the DB schema is broken and does not accept a null value when it should.
+                'image' => '',
+            ]);
+            $issuer->save();
+        }
+
+        // This can't be run from PHPUNIT because testing platforms need real URLs.
+        // In the future, this request can be moved to the moodle-exttests repository.
+        if (!PHPUNIT_TEST) {
+            // Create/update the endpoints for the issuer.
+            \core\oauth2\discovery\imsbadgeconnect::create_endpoints($issuer);
+            $data->oauth2_issuerid = $issuer->get('id');
+
+            $apibase = \core\oauth2\endpoint::get_record([
+                'issuerid' => $data->oauth2_issuerid,
+                'name' => 'apiBase',
+            ]);
+            $data->backpackapiurl = $apibase->get('url');
+        }
+    }
     $backpack = new stdClass();
 
     $backpack->apiversion = $data->apiversion;
@@ -1332,24 +1366,6 @@ function badges_verify_backpack(int $backpackid) {
     return '';
 }
 
-/**
- * Get OAuth2 services for the external backpack.
- *
- * @return array
- * @throws coding_exception
- */
-function badges_get_oauth2_service_options() {
-    global $DB;
-
-    $issuers = core\oauth2\api::get_all_issuers();
-    $options = ['' => 'None'];
-    foreach ($issuers as $issuer) {
-        $options[$issuer->get('id')] = $issuer->get('name');
-    }
-
-    return $options;
-}
-
 /**
  * Generate a public badgr URL that conforms to OBv2. This is done because badgr responses do not currently conform to
  * the spec.

lib/deprecatedlib.php

@@ -3861,3 +3861,26 @@ function cron_setup_user($user = null, $course = null, $leavepagealone = false)
 
     \core\cron::setup_user($user, $course, $leavepagealone);
 }
+
+/**
+ * Get OAuth2 services for the external backpack.
+ *
+ * @return array
+ * @throws coding_exception
+ * @deprecated since 4.3.
+ */
+function badges_get_oauth2_service_options() {
+    debugging(
+        'badges_get_oauth2_service_options() is deprecated. Don\'t use it.',
+        DEBUG_DEVELOPER
+    );
+    global $DB;
+
+    $issuers = core\oauth2\api::get_all_issuers();
+    $options = ['' => 'None'];
+    foreach ($issuers as $issuer) {
+        $options[$issuer->get('id')] = $issuer->get('name');
+    }
+
+    return $options;
+}

lib/upgrade.txt

@@ -5,6 +5,8 @@ information provided here is intended especially for developers.
 
 * Admin settings for passwords (admin_setting_configpasswordunmask) can now be a required field using the following class:
     - admin_setting_requiredpasswordunmask
+* The badges_get_oauth2_service_options() method has been deprecated, because it's not required anymore. It should no longer
+  be used.
 
 === 4.2 ===