Skip to content

Commit

Permalink
MDL-80836 auth_lti: take user through login instead of sesspiggyback
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
snake committed Feb 15, 2024
1 parent 8a38a37 commit 8482210
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 21 deletions.
11 changes: 5 additions & 6 deletions auth/lti/classes/output/renderer.php
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion auth/lti/lang/en/auth_lti.php
Expand Up @@ -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.';
Expand Down
18 changes: 4 additions & 14 deletions auth/lti/templates/local/ltiadvantage/login.mustache
Expand Up @@ -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):
{
Expand All @@ -46,19 +45,12 @@
"issuccess": true
},
"cancreateaccounts": true,
"isloggedin": true,
"accountinfo": {
"isloggedin": true,
"firstname": "John",
"lastname": "Smith",
"email": "john@example.com",
"picturehtml": "<img src=\"http://site.example.com/pluginfile.php/5/user/icon/boost/f2?rev=99\" class=\"round\" alt=\"\" width=\"35\" height=\"35\">"
},
"noauthnotice": {
"message": "To link your existing account you must be logged in to the site...",
"extraclasses": "",
"announce": false,
"closebutton": false,
"iswarning": true
}
}
}}
Expand All @@ -79,8 +71,8 @@
<div class="card-body text-center d-flex flex-column">
<i class="fa fa-user-circle-o fa-2x link"></i>
<h4 class="card-title">{{#str}} useexistingaccount, auth_lti {{/str}}</h4>
{{#accountinfo}}
{{#isloggedin}}
{{#accountinfo}}
<p class="card-text mt-2">
<span class="text-muted">
{{#str}} currentlyloggedinas, auth_lti {{/str}}
Expand All @@ -90,14 +82,12 @@
{{firstname}} {{lastname}} ({{email}})
</p>
<input type="submit" class="btn btn-primary mt-auto" name="existing_account" value="{{#str}} linkthisaccount, auth_lti {{/str}}">
{{/accountinfo}}
{{/isloggedin}}
{{^isloggedin}}
<p class="card-text text-muted">{{#str}} mustbeloggedin, auth_lti {{/str}}</p>
{{#noauthnotice}}
{{> core/notification}}
{{/noauthnotice}}
<input type="submit" class="btn btn-primary mt-auto" name="existing_account" value="{{#str}} login, moodle {{/str}}">
{{/isloggedin}}
{{/accountinfo}}
</div>
</div>
</div>
Expand Down

0 comments on commit 8482210

Please sign in to comment.