diff --git a/admin/tool/oauth2/issuers.php b/admin/tool/oauth2/issuers.php index 79d04c87f98..35156a77bcc 100644 --- a/admin/tool/oauth2/issuers.php +++ b/admin/tool/oauth2/issuers.php @@ -210,12 +210,6 @@ if ($mform && $mform->is_cancelled()) { $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 index 565cc356d16..f45a2c21019 100644 --- 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 index 3d632760df8..06ea4a49135 100644 --- a/badges/classes/form/external_backpack.php +++ b/badges/classes/form/external_backpack.php @@ -53,21 +53,20 @@ class external_backpack extends \moodleform { $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 @@ class external_backpack extends \moodleform { $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 @@ class external_backpack extends \moodleform { $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 @@ class external_backpack extends \moodleform { 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 index 4220f5eaf9a..dfae36b4dca 100644 --- a/badges/mybackpack.php +++ b/badges/mybackpack.php @@ -129,7 +129,7 @@ if ($backpack) { // 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 index 3f584b6d1f0..15e4f673faa 100644 --- 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 "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" diff --git a/lib/badgeslib.php b/lib/badgeslib.php index d1f4d5ff5f0..f7225ec1b3f 100644 --- 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;