Skip to content

Commit

Permalink
MDL-67802 Authentication: Allow disabling email verification for OAuth
Browse files Browse the repository at this point in the history
Allow email account verification to be disabled for any Oauth2 provider.
Also add clear indications to administrators of the danger of doing do,
this is done by an additional form checkbox.
This patch also reverts MDL-66598
  • Loading branch information
Matt Porritt committed Mar 9, 2022
1 parent bb864ee commit 6a4801b
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 39 deletions.
26 changes: 16 additions & 10 deletions admin/tool/oauth2/classes/form/issuer.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ class issuer extends persistent {
/** @var string $type */
protected $type;

/** @var boolean $showrequireconfirm Whether to show the require confirmation email checkbox or not. */
protected $showrequireconfirm;

/**
* Constructor.
*
Expand All @@ -74,7 +71,6 @@ public function __construct($action = null, $customdata = null, $method = 'post'
if (array_key_exists('type', $customdata)) {
$this->type = $customdata['type'];
}
$this->showrequireconfirm = !empty($customdata['showrequireconfirm']);
parent::__construct($action, $customdata, $method, $target, $attributes, $editable, $ajaxformdata);
}

Expand Down Expand Up @@ -169,12 +165,19 @@ public function definition() {
$mform->addHelpButton('alloweddomains', 'issueralloweddomains', 'tool_oauth2');
$mform->hideIf('alloweddomains', 'showonloginpage', 'eq', \core\oauth2\issuer::SERVICEONLY);

if ($this->showrequireconfirm) {
// Require confirmation email for new accounts.
$mform->addElement('advcheckbox', 'requireconfirmation', get_string('issuerrequireconfirmation', 'tool_oauth2'));
$mform->addHelpButton('requireconfirmation', 'issuerrequireconfirmation', 'tool_oauth2');
$mform->hideIf('requireconfirmation', 'showonloginpage', 'eq', \core\oauth2\issuer::SERVICEONLY);
}
// Require confirmation email for new accounts.
$mform->addElement('advcheckbox', 'requireconfirmation',
get_string('issuerrequireconfirmation', 'tool_oauth2'));
$mform->addHelpButton('requireconfirmation', 'issuerrequireconfirmation', 'tool_oauth2');
$mform->hideIf('requireconfirmation', 'showonloginpage',
'eq', \core\oauth2\issuer::SERVICEONLY);

$mform->addElement('checkbox', 'acceptrisk', get_string('acceptrisk', 'tool_oauth2'));
$mform->addHelpButton('acceptrisk', 'acceptrisk', 'tool_oauth2');
$mform->hideIf('acceptrisk', 'showonloginpage',
'eq', \core\oauth2\issuer::SERVICEONLY);
$mform->hideIf('acceptrisk', 'requireconfirmation', 'checked');


if ($this->type == 'imsobv2p1' || $issuer->get('servicetype') == 'imsobv2p1') {
$mform->addRule('baseurl', null, 'required', null, 'client');
Expand Down Expand Up @@ -249,6 +252,9 @@ protected function extra_validation($data, $files, array &$errors) {
if (!strlen(trim($data->loginscopesoffline))) {
$errors['loginscopesoffline'] = get_string('required');
}
if (empty($data->requireconfirmation) && empty($data->acceptrisk)) {
$errors['acceptrisk'] = get_string('required');
}
}
return $errors;
}
Expand Down
34 changes: 5 additions & 29 deletions admin/tool/oauth2/issuers.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,24 +56,7 @@
$PAGE->navbar->add(get_string('createnewservice', 'tool_oauth2') . ' ' . get_string('custom_service', 'tool_oauth2'));
}

$showrequireconfirm = false;
if (!empty($issuerid)) {
// Show the "Require confirmation email" checkbox for trusted issuers like Google, Facebook and Microsoft.
$likefacebook = $DB->sql_like('url', ':facebook');
$likegoogle = $DB->sql_like('url', ':google');
$likemicrosoft = $DB->sql_like('url', ':microsoft');
$params = [
'issuerid' => $issuerid,
'facebook' => '%facebook%',
'google' => '%google%',
'microsoft' => '%microsoft%',
];
$select = "issuerid = :issuerid AND ($likefacebook OR $likegoogle OR $likemicrosoft)";
// We're querying from the oauth2_endpoint table because the base URLs of FB and Microsoft can be empty in the issuer table.
$showrequireconfirm = $DB->record_exists_select('oauth2_endpoint', $select, $params);
}

$mform = new \tool_oauth2\form\issuer(null, ['persistent' => $issuer, 'showrequireconfirm' => $showrequireconfirm]);
$mform = new \tool_oauth2\form\issuer(null, ['persistent' => $issuer]);
}

if ($mform && $mform->is_cancelled()) {
Expand Down Expand Up @@ -128,11 +111,9 @@

$type = required_param('type', PARAM_ALPHANUM);
$docs = required_param('docslink', PARAM_ALPHAEXT);
$showrequireconfirm = optional_param('showrequireconfirm', false, PARAM_BOOL);
require_sesskey();
$issuer = core\oauth2\api::init_standard_issuer($type);
$mform = new \tool_oauth2\form\issuer(null, ['persistent' => $issuer, 'type' => $type,
'showrequireconfirm' => $showrequireconfirm]);
$mform = new \tool_oauth2\form\issuer(null, ['persistent' => $issuer, 'type' => $type]);

$PAGE->navbar->add(get_string('createnewservice', 'tool_oauth2') . ' ' . get_string($type . '_service', 'tool_oauth2'));
echo $OUTPUT->header();
Expand Down Expand Up @@ -207,22 +188,19 @@

// Google template.
$docs = 'admin/tool/oauth2/issuers/google';
$params = ['action' => 'edittemplate', 'type' => 'google', 'sesskey' => sesskey(), 'docslink' => $docs,
'showrequireconfirm' => true];
$params = ['action' => 'edittemplate', 'type' => 'google', 'sesskey' => sesskey(), 'docslink' => $docs];
$addurl = new moodle_url('/admin/tool/oauth2/issuers.php', $params);
echo $renderer->single_button($addurl, get_string('google_service', 'tool_oauth2'));

// Microsoft template.
$docs = 'admin/tool/oauth2/issuers/microsoft';
$params = ['action' => 'edittemplate', 'type' => 'microsoft', 'sesskey' => sesskey(), 'docslink' => $docs,
'showrequireconfirm' => true];
$params = ['action' => 'edittemplate', 'type' => 'microsoft', 'sesskey' => sesskey(), 'docslink' => $docs];
$addurl = new moodle_url('/admin/tool/oauth2/issuers.php', $params);
echo $renderer->single_button($addurl, get_string('microsoft_service', 'tool_oauth2'));

// Facebook template.
$docs = 'admin/tool/oauth2/issuers/facebook';
$params = ['action' => 'edittemplate', 'type' => 'facebook', 'sesskey' => sesskey(), 'docslink' => $docs,
'showrequireconfirm' => true];
$params = ['action' => 'edittemplate', 'type' => 'facebook', 'sesskey' => sesskey(), 'docslink' => $docs];
$addurl = new moodle_url('/admin/tool/oauth2/issuers.php', $params);
echo $renderer->single_button($addurl, get_string('facebook_service', 'tool_oauth2'));

Expand All @@ -249,7 +227,5 @@
echo $renderer->single_button($addurl, get_string('custom_service', 'tool_oauth2'));

echo $renderer->container_end();

echo $OUTPUT->footer();

}
2 changes: 2 additions & 0 deletions admin/tool/oauth2/lang/en/tool_oauth2.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/

$string['acceptrisk'] = 'I understand that disabling email verification can be a security issue';
$string['acceptrisk_help'] = 'Disabling email verification can be a security issue. As it potentially allows users to authenticate as another in the right circumstances.';
$string['authconfirm'] = 'This action will grant permanent API access to Moodle for the authenticated account. This is intended to be used as a system account for managing files owned by Moodle.';
$string['authconnected'] = 'The system account is now connected for offline access';
$string['authnotconnected'] = 'The system account was not connected for offline access';
Expand Down
30 changes: 30 additions & 0 deletions admin/tool/oauth2/tests/behat/email_verification.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
@tool @tool_oauth2 @external @javascript
Feature: OAuth2 email verification
In order to make sure administrators understand the ramifications of email verification
As an administrator
I should see email verifications notifications when configuring an Oauth2 provider.

Background:
Given I log in as "admin"
And I change window size to "large"
And I navigate to "Server > OAuth 2 services" in site administration

Scenario: Create, edit and delete standard service for Google toggling email verification.
Given I press "Google"
And I should see "Create new service: Google"
And I set the following fields to these values:
| Name | Testing service |
| Client ID | thisistheclientid |
| Client secret | supersecret |
Then I should not see "I understand that disabling email verification can be a security issue"
And I click on "Require email verification" "checkbox"
And I should see "I understand that disabling email verification can be a security issue"
And I click on "I understand that disabling email verification can be a security issue" "checkbox"
And I press "Save changes"
And I should see "Changes saved"
And I click on "Edit" "link" in the "Testing service" "table_row"
And I press "Save changes"
And I should see "Required"
And I click on "Require email verification" "checkbox"
And I press "Save changes"
And I should see "Changes saved"

0 comments on commit 6a4801b

Please sign in to comment.