diff --git a/admin/tool/policy/classes/output/page_agreedocs.php b/admin/tool/policy/classes/output/page_agreedocs.php index 5ede1610a2b..af6ebd61947 100644 --- a/admin/tool/policy/classes/output/page_agreedocs.php +++ b/admin/tool/policy/classes/output/page_agreedocs.php @@ -69,6 +69,9 @@ class page_agreedocs implements renderable, templatable { /** @var array Info or error messages to show. */ protected $messages = []; + /** @var bool This is an existing user (rather than non-loggedin/guest). */ + protected $isexistinguser; + /** * Prepare the page for rendering. * @@ -87,6 +90,7 @@ class page_agreedocs implements renderable, templatable { $this->action = $action; + $this->isexistinguser = isloggedin() && !isguestuser(); $behalfid = $behalfid ?: $USER->id; if ($realuser->id != $behalfid) { $this->behalfuser = core_user::get_user($behalfid, '*', MUST_EXIST); @@ -112,7 +116,7 @@ class page_agreedocs implements renderable, templatable { protected function accept_and_revoke_policies() { global $USER; - if (!empty($USER->id)) { + if ($this->isexistinguser) { // Existing user. if (!empty($this->action) && confirm_sesskey()) { // The form has been sent. Update policies acceptances according to $this->agreedocs. @@ -182,15 +186,13 @@ class page_agreedocs implements renderable, templatable { * Before display the consent page, the user has to view all the still-non-accepted policy docs. * This function checks if the non-accepted policy docs have been shown and redirect to them. * - * @param array $userid User identifier who wants to access to the consent page. - * @param url $returnurl URL to return after shown the policy docs. + * @param int $userid User identifier who wants to access to the consent page. + * @param moodle_url $returnurl URL to return after shown the policy docs. */ protected function redirect_to_policies($userid, $returnurl = null) { - global $USER; - - $acceptances = api::get_user_acceptances($userid); $allpolicies = $this->policies; - if (!empty($userid)) { + if ($this->isexistinguser) { + $acceptances = api::get_user_acceptances($userid); foreach ($allpolicies as $policy) { if (api::is_user_version_accepted($userid, $policy->id, $acceptances)) { // If this version is accepted by the user, remove from the pending policies list. @@ -234,16 +236,30 @@ class page_agreedocs implements renderable, templatable { } /** - * Redirect to $SESSION->wantsurl if defined or to $CFG->wwwroot if not. + * Redirect to signup page if defined or to $CFG->wwwroot if not. */ protected function redirect_to_previous_url() { global $SESSION; - if (!empty($SESSION->wantsurl)) { - $returnurl = $SESSION->wantsurl; - unset($SESSION->wantsurl); + if ($this->isexistinguser) { + // Existing user. + if (!empty($SESSION->wantsurl)) { + $returnurl = $SESSION->wantsurl; + unset($SESSION->wantsurl); + } else { + $returnurl = new moodle_url('/admin/tool/policy/user.php'); + } } else { - $returnurl = (new moodle_url('/admin/tool/policy/user.php'))->out(); + // Non-authenticated user. + $issignup = \cache::make('core', 'presignup')->get('tool_policy_issignup'); + if ($issignup) { + // User came here from signup page - redirect back there. + $returnurl = new moodle_url('/login/signup.php'); + \cache::make('core', 'presignup')->set('tool_policy_issignup', false); + } else { + // Guests should not be on this page unless it's part of signup - redirect home. + $returnurl = new moodle_url('/'); + } } redirect($returnurl); @@ -255,35 +271,35 @@ class page_agreedocs implements renderable, templatable { * @param int $userid */ protected function prepare_global_page_access($userid) { - global $PAGE, $SESSION, $SITE, $USER; + global $PAGE, $SITE, $USER; // Guest users or not logged users (but the users during the signup process) are not allowed to access to this page. - $newsignupuser = !empty($SESSION->wantsurl) && strpos($SESSION->wantsurl, 'login/signup.php') !== false; - if (isguestuser() || (empty($USER->id) && !$newsignupuser)) { + $newsignupuser = \cache::make('core', 'presignup')->get('tool_policy_issignup'); + if (!$this->isexistinguser && !$newsignupuser) { $this->redirect_to_previous_url(); } // Check for correct user capabilities. - if (!empty($USER->id)) { + if ($this->isexistinguser) { // For existing users, it's needed to check if they have the capability for accepting policies. api::can_accept_policies($this->behalfid, true); } else { // For new users, the behalfid parameter is ignored. - if ($this->behalfid != $USER->id) { + if ($this->behalfid) { redirect(new moodle_url('/admin/tool/policy/index.php')); } } // If the current user has the $USER->policyagreed = 1 or $userpolicyagreed = 1 - // and $SESSION->wantsurl is defined, redirect to the return page. - $hasagreedsignupuser = empty($USER->id) && $this->signupuserpolicyagreed; + // redirect to the return page. + $hasagreedsignupuser = !$this->isexistinguser && $this->signupuserpolicyagreed; $hasagreedloggeduser = $USER->id == $userid && !empty($USER->policyagreed); if (!is_siteadmin() && ($hasagreedsignupuser || $hasagreedloggeduser)) { $this->redirect_to_previous_url(); } $myparams = []; - if (!empty($USER->id) && !empty($this->behalfid) && $this->behalfid != $USER->id) { + if ($this->isexistinguser && !empty($this->behalfid) && $this->behalfid != $USER->id) { $myparams['userid'] = $this->behalfid; } $myurl = new moodle_url('/admin/tool/policy/index.php', $myparams); @@ -308,7 +324,6 @@ class page_agreedocs implements renderable, templatable { global $USER; // Get all the policy version acceptances for this user. - $acceptances = api::get_user_acceptances($userid); $lang = current_language(); foreach ($this->policies as $policy) { // Get a link to display the full policy document. @@ -320,9 +335,10 @@ class page_agreedocs implements renderable, templatable { $policymodal = html_writer::link($policy->url, $policy->name, $policyattributes); // Check if this policy version has been agreed or not. - if (!empty($userid)) { + if ($this->isexistinguser) { // Existing user. $versionagreed = false; + $acceptances = api::get_user_acceptances($userid); $policy->versionacceptance = api::get_user_version_acceptance($userid, $policy->id, $acceptances); if (!empty($policy->versionacceptance)) { // The policy version has ever been agreed. Check if status = 1 to know if still is accepted. @@ -352,13 +368,13 @@ class page_agreedocs implements renderable, templatable { * Export the page data for the mustache template. * * @param renderer_base $output renderer to be used to render the page elements. - * @return stdClass + * @return \stdClass */ public function export_for_template(renderer_base $output) { global $USER; $myparams = []; - if (!empty($USER->id) && !empty($this->behalfid) && $this->behalfid != $USER->id) { + if ($this->isexistinguser && !empty($this->behalfid) && $this->behalfid != $USER->id) { $myparams['userid'] = $this->behalfid; } $data = (object) [ diff --git a/admin/tool/policy/index.php b/admin/tool/policy/index.php index 4be85548368..067d8fabc34 100644 --- a/admin/tool/policy/index.php +++ b/admin/tool/policy/index.php @@ -45,7 +45,7 @@ $PAGE->set_pagelayout('standard'); $PAGE->set_url('/admin/tool/policy/index.php'); $PAGE->set_popup_notification_allowed(false); -if (!empty($USER->id)) { +if (isloggedin() && !isguestuser()) { // Existing user. $haspermissionagreedocs = api::can_accept_policies($behalfid); } else { diff --git a/admin/tool/policy/lib.php b/admin/tool/policy/lib.php index 6c9afa90e0c..3073212169b 100644 --- a/admin/tool/policy/lib.php +++ b/admin/tool/policy/lib.php @@ -116,7 +116,7 @@ function tool_policy_standard_footer_html() { * Hooks redirection to policy acceptance pages before sign up. */ function tool_policy_pre_signup_requests() { - global $CFG, $SESSION; + global $CFG; // Do nothing if we are not set as the site policies handler. if (empty($CFG->sitepolicyhandler) || $CFG->sitepolicyhandler !== 'tool_policy') { @@ -127,7 +127,7 @@ function tool_policy_pre_signup_requests() { $userpolicyagreed = cache::make('core', 'presignup')->get('tool_policy_userpolicyagreed'); if (!empty($policies) && !$userpolicyagreed) { // Redirect to "Policy" pages for consenting before creating the user. - $SESSION->wantsurl = (new \moodle_url('/login/signup.php'))->out(); + cache::make('core', 'presignup')->set('tool_policy_issignup', 1); redirect(new \moodle_url('/admin/tool/policy/index.php')); } } diff --git a/admin/tool/policy/tests/behat/consent.feature b/admin/tool/policy/tests/behat/consent.feature index ec9aae6c9cd..fde43ccc0cb 100644 --- a/admin/tool/policy/tests/behat/consent.feature +++ b/admin/tool/policy/tests/behat/consent.feature @@ -611,3 +611,55 @@ Feature: User must accept policy managed by this plugin when logging in and sign And I should see "Policies and agreements" And I should see "No permission to agree to the policies on behalf of this user" And I should see "Sorry, you do not have the required permission to agree to the following policies on behalf of User 1" + + Scenario: Accept policy on sign up as a guest, one policy + Given the following config values are set as admin: + | registerauth | email | + | passwordpolicy | 0 | + | sitepolicyhandler | tool_policy | + Given the following policies exist: + | Policy | Name | Revision | Content | Summary | Status | + | P1 | This site policy | | full text1 | short text1 | archived | + | P1 | This site policy | | full text2 | short text2 | active | + | P1 | This site policy | | full text3 | short text3 | draft | + And I am on site homepage + And I follow "Log in" + # First log in as a guest + And I press "Log in as a guest" + # Now sign up + And I follow "Log in" + When I press "Create new account" + Then I should see "This site policy" + And I should see "short text2" + And I should see "full text2" + And I press "Next" + And I should see "Please agree to the following policies" + And I should see "This site policy" + And I should see "short text2" + And I should not see "full text2" + And I set the field "I agree to the This site policy" to "1" + And I press "Next" + And I should not see "I understand and agree" + And I set the following fields to these values: + | Username | user1 | + | Password | user1 | + | Email address | user1@address.invalid | + | Email (again) | user1@address.invalid | + | First name | User1 | + | Surname | L1 | + And I press "Create my new account" + And I should see "Confirm your account" + And I should see "An email should have been sent to your address at user1@address.invalid" + And I confirm email for "user1" + And I should see "Thanks, User1 L1" + And I should see "Your registration has been confirmed" + And I open my profile in edit mode + And the field "First name" matches value "User1" + And I log out + # Confirm that user can login and browse the site. + And I log in as "user1" + And I follow "Profile" in the user menu + # User can see his own agreements in the profile. + And I follow "Policies and agreements" + And "Agreed" "icon" should exist in the "This site policy" "table_row" + And I log out