Page MenuHomePhabricator

First Save bug (2018)
Closed, ResolvedPublic

Description

For each new browser session, all user's first attempts to save an edit on my site fails due to "loss of session" error. Once the first error occurs, the user's can edit without error from then on. My system is:

RHEL7 with CA Policy Agent to ensure authenticated sessions via remote enterprise identity provider.
Mediawiki - 1.30.0 (830bb58)
Auth_remoteuser - 2.0.1 (0af2823)16:22, 24 April 2018

In LocalSettings.php my "Auth_remoteuser" config is:

if(isset($_SERVER['HTTP_AGENCYUID'] )) { $HTTP_AGENCYUID = $_SERVER['HTTP_AGENCYUID'];} 
else                                   { $HTTP_AGENCYUID = null; }
$wgGroupPermissions['*']['autocreateaccount'] = true;
wfLoadExtension( 'Auth_remoteuser' );  
$wgAuthRemoteuserUserName = $HTTP_AGENCYUID;
$wgAuthRemoteuserUserPrefsForced = [
    'email'    => $HTTP_AGENCYEMAIL,
    'realname' => $HTTP_DISPLAYNAME
  ];

and an analysis of my session header is shown here:

Event Timeline

Revansx created this task.Jul 5 2018, 11:12 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 5 2018, 11:12 PM
Revansx assigned this task to Tgr.Jul 5 2018, 11:13 PM
Revansx updated the task description. (Show Details)Jul 5 2018, 11:18 PM
Revansx updated the task description. (Show Details)
Revansx updated the task description. (Show Details)Jul 5 2018, 11:20 PM
Revansx updated the task description. (Show Details)
Tgr updated the task description. (Show Details)Jul 6 2018, 8:43 AM
Tgr updated the task description. (Show Details)
Tgr added a comment.Jul 6 2018, 8:46 AM

Trying to save an edit (SubmitAction::show(), specifically) persists the session just before processing the edit. Normally this should not change the session; either the user is logged in via a persistent session, in which case persisting it again should be a no-op; or the user is logged in via an immutable session in which case persisting should in general be a no-op; or the user is not logged in, in which case an anonymous session is created (so that does change the session, technically, but anonymous edit tokens do not rely on the session so that does not cause any problem). Auth_remoteuser must be getting some of that wrong, and creating a non-anonymous session when Session::persist() is called, so an edit token is suddenly required; but since it wasn't when the edit page was displayed, it wasn't submitted. Since at this point the session exists, the page with the error message does contain the token and the next submit works.

So it seems that Auth_remoteuser does not start a session when $wgAuthRemoteuserUserName is set, only when MediaWiki calls Session::persist() (which is the wrong behavior for an immutable session provider, it should always create the session, and completely ignore persist calls. Alternatively, if it sets up its own persistent session (which seems to be the case, what with the <wikiId>RemoteToken cookie), it should stick to that - maybe it's using the wrong priority.

@Revansx can you confirm whether the user seems logged in before starting to edit?

I confirm that the user certainly "seems" logged-in before starting to edit.

Tgr added a comment.Jul 6 2018, 2:16 PM

Do you set callUserLoggedInHook? Does unsetting it help?

callUserLoggedInHook``` is a completely new term to me. What do you advise?
Tgr added a comment.Jul 6 2018, 3:33 PM

Nevermind, I misread the code. Can you verify that the username cookie (Jsmith in the example) and the userid cookie (2) match (so the user Jsmith indeed has an ID of 2)? You can Use Special:ApiSandbox (and then action = query, list = users) to check user IDs.

Tgr added a subscriber: Hexmode.Jul 6 2018, 3:34 PM

[Username and UserID] -- Yes. They match

If there are changes made to the code of this extension that resolve this issue, how would I be notified? here?

I've the same issue, and I think the problem is a partial session information, the user is logged in but some session cookies are missing.

what I noticed is that:

  1. when a user is created the following cookies are set: token, _session, UserID, RemoteToken, UserName
  2. when the user logs in again from only, RemoteToken and UserName are set.

Look at this log https://pastebin.com/43TBnvsa.
What happen is that the extension detects first missing the session and set all cookies (line 20 of this log ) and then detects the session id as dirty again and removes all cookies and set ups only few (from line 74 of this log) deleting the other

When saving a new content the editor tries ti validate the session with one of the missing cookies. This trigger a re-creation of the complete session.

These are the settings I used to test on a brand new MW 1.30 + authremote_user 2.01:

wfLoadExtension( 'Auth_remoteuser' );
$wgGroupPermissions['*']['createaccount'] = false;
$wgGroupPermissions['*']['autocreateaccount'] = true;
$wgAuthRemoteuserUserName = 'User005';

Revansx added a comment.EditedJul 11 2018, 12:08 AM

So, to be clear, you are saying that you are having this issue with the configuration you listed? Not that you solved it. Correct?

Because, our configurations are functionally identical

Aklapper added a comment.EditedJul 11 2018, 9:53 AM

[offtopic] @Revansx: In the upper corner this task says that it is "open". That means it is not solved. If there are changes to this task, then they are listed in this task.
If anyone writes a patch, please share it in Gerrit so it can get reviewed and merged. See https://www.mediawiki.org/wiki/Gerrit/Tutorial

roger that. thanks!

@Revansx , yes I am having the same issues, and I have not solved it. I added some more information

I'm not sure if this is the problem, but would you be willing to try moving the wfLoadExtension call to the end. In @Ninopul's example, this would look more like:

$wgGroupPermissions['*']['createaccount'] = false;
$wgGroupPermissions['*']['autocreateaccount'] = true;
$wgAuthRemoteuserUserName = 'User005';
wfLoadExtension( 'Auth_remoteuser' );

It might not be this way anymore, but in the original version the configuration needed to be specified before the extension is loaded.

Change 445775 had a related patch set uploaded (by Enst80; owner: Enst80):
[mediawiki/extensions/Auth_remoteuser@master] Trust generated session id on new request

https://gerrit.wikimedia.org/r/445775

Enst80 edited subscribers, added: Enst80; removed: gerritbot.Jul 14 2018, 1:35 PM

Thanks @Revansx for your detailed description and thanks @Tgr for the hints in the right direction. Yes, Auth_remoteuser doesn't handle the creation of new empty sessions correctly.

Hopefully this simple patch will address most (if not all) of the occuring problems. At least the debug log now don't show session id resets anymore.

Revansx added a comment.EditedJul 16 2018, 1:40 PM

The patch definitely seems to help with the hugely problematic First Save issue, however, after exercising the system some today, I did experience a few:

  1. loss of session errors upon a random save as well as
  2. some odd attempts to visit a wiki article named API that I was not expecting to see (using Extension:Wiretap)

I need to test more and gather my facts. Please stay open to the idea that there is still an issue with Auth_RU.

I think the problem is solved. Thank you.

I have to say.. I can't believe that the solution to all my system usability issues, the reason my roll-out was delayed for months, could have been handled a year ago by commenting out 1 line. I am gratefully in awe.

That's great to hear! Thank you for testing the change.

@Enst80 Given the simplicity of this fix, would it make sense to pull this change into the older branches?

Yes, indeed! I'll upload the backported patches.

Change 446728 had a related patch set uploaded (by Enst80; owner: Enst80):
[mediawiki/extensions/Auth_remoteuser@REL1_27] Trust generated session id on new request

https://gerrit.wikimedia.org/r/446728

Change 446729 had a related patch set uploaded (by Enst80; owner: Enst80):
[mediawiki/extensions/Auth_remoteuser@REL1_28] Trust generated session id on new request

https://gerrit.wikimedia.org/r/446729

Change 446730 had a related patch set uploaded (by Enst80; owner: Enst80):
[mediawiki/extensions/Auth_remoteuser@REL1_29] Trust generated session id on new request

https://gerrit.wikimedia.org/r/446730

Change 446731 had a related patch set uploaded (by Enst80; owner: Enst80):
[mediawiki/extensions/Auth_remoteuser@REL1_30] Trust generated session id on new request

https://gerrit.wikimedia.org/r/446731

Change 446732 had a related patch set uploaded (by Enst80; owner: Enst80):
[mediawiki/extensions/Auth_remoteuser@REL1_31] Trust generated session id on new request

https://gerrit.wikimedia.org/r/446732

Change 445775 merged by jenkins-bot:
[mediawiki/extensions/Auth_remoteuser@master] Trust generated session id on new request

https://gerrit.wikimedia.org/r/445775

Change 446732 merged by jenkins-bot:
[mediawiki/extensions/Auth_remoteuser@REL1_31] Trust generated session id on new request

https://gerrit.wikimedia.org/r/446732

Change 446731 merged by jenkins-bot:
[mediawiki/extensions/Auth_remoteuser@REL1_30] Trust generated session id on new request

https://gerrit.wikimedia.org/r/446731

Change 446730 merged by jenkins-bot:
[mediawiki/extensions/Auth_remoteuser@REL1_29] Trust generated session id on new request

https://gerrit.wikimedia.org/r/446730

Change 446728 merged by jenkins-bot:
[mediawiki/extensions/Auth_remoteuser@REL1_27] Trust generated session id on new request

https://gerrit.wikimedia.org/r/446728

Change 446729 merged by Enst80:
[mediawiki/extensions/Auth_remoteuser@REL1_28] Trust generated session id on new request

https://gerrit.wikimedia.org/r/446729

Enst80 closed this task as Resolved.Jul 20 2018, 6:21 PM