MDL-78132 badges: move apiBase consumption to backpack

The logic to create the issuer has been moved to the backpack form in order to improve the workflow and update the apiBase with the proper value comming from the badgeconnect.json manifest file. So, as part of this change in the workflow, the following changes has been also implemented (to make the UI easier for users): - The "Open Badges" oAuth issuer button has been removed from the "OAuth Services" admin page. As they are created/updated when a backpack is saved, this button is not required anymore. - The "OAuth2 services" and "Backpack API URL" parameters have been removed from the Manage backpacks form, because they are created on the fly each time the backpack is saved.
dabwao · May 23, 2023 · 03e4afd · 03e4afd
1 parent 2e1c6fd
commit 03e4afd
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 62 deletions.
diff --git a/admin/tool/oauth2/issuers.php b/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];

diff --git a/admin/tool/oauth2/tests/behat/basic_settings.feature b/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"

diff --git a/badges/classes/form/external_backpack.php b/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');

diff --git a/badges/mybackpack.php b/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.

diff --git a/badges/tests/behat/backpack.feature b/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 "[email protected]"
     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"

diff --git a/lib/badgeslib.php b/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;