From 8482210209d55bd16ce1a31454e9ca3c46e57848 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Mon, 5 Feb 2024 13:19:52 +0800 Subject: [PATCH] MDL-80836 auth_lti: take user through login instead of sesspiggyback Browsers are phasing out 3rd party cookies. Those which can be set are partitioned to the top level embedding site, so piggybacking is prevented. This will break the account linking process. This fix swaps the piggyback for a login round trip, as originally intended, which resolves the issue. --- auth/lti/classes/output/renderer.php | 11 +++++------ auth/lti/lang/en/auth_lti.php | 2 +- .../local/ltiadvantage/login.mustache | 18 ++++-------------- 3 files changed, 10 insertions(+), 21 deletions(-) diff --git a/auth/lti/classes/output/renderer.php b/auth/lti/classes/output/renderer.php index 4bae8a0b24fbe..9ebe104d58714 100644 --- a/auth/lti/classes/output/renderer.php +++ b/auth/lti/classes/output/renderer.php @@ -33,33 +33,32 @@ class renderer extends \plugin_renderer_base { * @return string the html. */ public function render_account_binding_options_page(int $provisioningmode): string { + $formaction = new \moodle_url('/auth/lti/login.php'); $notification = new notification(get_string('firstlaunchnotice', 'auth_lti'), \core\notification::INFO, false); - $noauthnotice = new notification(get_string('firstlaunchnoauthnotice', 'auth_lti', get_docs_url('Publish_as_LTI_tool')), - \core\notification::WARNING, false); $cancreateaccounts = !get_config('moodle', 'authpreventaccountcreation'); if ($provisioningmode == \auth_plugin_lti::PROVISIONING_MODE_PROMPT_EXISTING_ONLY) { $cancreateaccounts = false; } - $accountinfo = ['isloggedin' => isloggedin()]; + $accountinfo = []; if (isloggedin()) { global $USER; - $accountinfo = array_merge($accountinfo, [ + $accountinfo = [ 'firstname' => $USER->firstname, 'lastname' => $USER->lastname, 'email' => $USER->email, 'picturehtml' => $this->output->user_picture($USER, ['size' => 35, 'class' => 'round']), - ]); + ]; } $context = [ + 'isloggedin' => isloggedin(), 'info' => $notification->export_for_template($this), 'formaction' => $formaction->out(), 'sesskey' => sesskey(), 'accountinfo' => $accountinfo, 'cancreateaccounts' => $cancreateaccounts, - 'noauthnotice' => $noauthnotice->export_for_template($this) ]; return parent::render_from_template('auth_lti/local/ltiadvantage/login', $context); } diff --git a/auth/lti/lang/en/auth_lti.php b/auth/lti/lang/en/auth_lti.php index 05c29d5f3c5f9..abfc35cfdaca5 100644 --- a/auth/lti/lang/en/auth_lti.php +++ b/auth/lti/lang/en/auth_lti.php @@ -35,7 +35,7 @@ $string['getstartedwithnewaccount'] = 'Get started with a new account'; $string['haveexistingaccount'] = 'I have an existing account'; $string['linkthisaccount'] = 'Link this account'; -$string['mustbeloggedin'] = 'You need to be logged in to your existing account'; +$string['mustbeloggedin'] = 'Sign in to link your existing account'; $string['pluginname'] = 'LTI'; $string['privacy:metadata:auth_lti'] = 'LTI authentication'; $string['privacy:metadata:auth_lti:authsubsystem'] = 'This plugin is connected to the authentication subsystem.'; diff --git a/auth/lti/templates/local/ltiadvantage/login.mustache b/auth/lti/templates/local/ltiadvantage/login.mustache index 1bdd6eabcce4e..e6ed8e2a8cfa6 100644 --- a/auth/lti/templates/local/ltiadvantage/login.mustache +++ b/auth/lti/templates/local/ltiadvantage/login.mustache @@ -32,7 +32,6 @@ * info - a notification describing the first launch options * cancreateaccounts - whether or not the user is allowed to create auth_lti accounts * accountinfo - information about the user, importantly whether they are logged in or not. - * noauthnotice - a notification telling the user they must be authenticated to link accounts. Only relevant when not logged in. Example context (json): { @@ -46,19 +45,12 @@ "issuccess": true }, "cancreateaccounts": true, + "isloggedin": true, "accountinfo": { - "isloggedin": true, "firstname": "John", "lastname": "Smith", "email": "john@example.com", "picturehtml": "\"\"" - }, - "noauthnotice": { - "message": "To link your existing account you must be logged in to the site...", - "extraclasses": "", - "announce": false, - "closebutton": false, - "iswarning": true } } }} @@ -79,8 +71,8 @@

{{#str}} useexistingaccount, auth_lti {{/str}}

- {{#accountinfo}} {{#isloggedin}} + {{#accountinfo}}

{{#str}} currentlyloggedinas, auth_lti {{/str}} @@ -90,14 +82,12 @@ {{firstname}} {{lastname}} ({{email}})

+ {{/accountinfo}} {{/isloggedin}} {{^isloggedin}}

{{#str}} mustbeloggedin, auth_lti {{/str}}

- {{#noauthnotice}} - {{> core/notification}} - {{/noauthnotice}} + {{/isloggedin}} - {{/accountinfo}}