From 194baa743614bf1fce6665fab8f65a98db6bb664 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 f7e1d305d52e8..4fb2b84f5eeea 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}}