Error
- mwversion: 1.44.0-wmf.19
- reqId: 443ead3a-df44-43fa-b101-1087183fa974
- Find reqId in Logstash
[{reqId}] {exception_url} LogicException: CentralAuthReturnRequest not found
Impact
Notes
Details- Request URL
- https://commons.wikimedia.org/wiki/Special:UserLogin
Subject Repo Branch Lines +/- authmanager: Use an URL parameter to keep track of returns Comment ActionsThis seems weird - the URL is /wiki/Special:UserLogin and it must be a POST request since otherwise the code path that leads to continuePrimaryAuthentication() wouldn't be active, but the referrer is https://commons.m.wikimedia.org/wiki/Special:ConfirmEmail/517c09033f0a5ce67a12cdeacc68acc1. How is that even possible?
CentralAuthRedirectingPrimaryAuthenticationProvider::beginPrimaryAuthentication() returns a response with a CentralAuthReturnRequest object in it, in theory that means the following continuePrimaryAuthentication() call should also have that object. But maybe if the corresponding GET/POST parameter is missing, we automatically skip adding the object? I thought that would only happen to AuthenticationRequest objects where $required is set to OPTIONAL, but maybe I'm misremembering.
So I suspect this is some kind of manual request tampering, but in theory the LogicException should not be reachable, and if it is reachable, we need a proper StatusValue-based error instead.
Comment ActionsThis seems really weird, vaguely following the Special:ConfirmEmail code, I see that it sometimes call Special:UserLogin as a GET request. I think maybe this is when we some weird behavior when cookies on shared domain are set but not on local domain (when continueAuthentication is called) - maybe some timeout(?). Then that throws the logic error but I wonder why/when that should ever happen.
But nevertheless, I'll try to reproduce it and see.
Comment ActionsI guess what's happening is:
- login or signup succeeds on the shared domain and redirects the user to Special:UserLogin/return on the local domain
- AuthManagerSpecialPage::handleReturnBeforeExecute() sets the session key for handling this request as a POST, and redirects to Special:UserLogin
- for some reason that redirect doesn't work, or maybe there is just a race between it and the user using another browser tab
- the user clicks on the link in the confirmation email, gets sent to the login page, but since they still have the session cookie and the session has the special key, AuthManagerSpecialPage thinks this is the last step of remote authentication, and should be interpreted as a form submit. But the URL parameter that would be used for CentralAuthReturnRequest is not actually there.
We should probably use a query parameter to ensure that the special /return processing only happens on the same redirect chain, and not in an unrelated browser window.
Comment ActionsChange #1128039 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):
[mediawiki/core@master] authmanager: Use an URL parameter to keep track of returns
Comment ActionsChange #1128039 merged by jenkins-bot:
[mediawiki/core@master] authmanager: Use an URL parameter to keep track of returns
Comment ActionsChange #1130320 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):
[mediawiki/core@wmf/1.44.0-wmf.21] authmanager: Use an URL parameter to keep track of returns
Comment ActionsChange #1130320 merged by jenkins-bot:
[mediawiki/core@wmf/1.44.0-wmf.21] authmanager: Use an URL parameter to keep track of returns
Comment ActionsComment ActionsComment ActionsErrors are gone:
Validation error on return LogicException: CentralAuthReturnRequest not found
Doesn't seem to have been replaced by new errors either.
- Request URL
- https://commons.wikimedia.org/wiki/Special:UserLogin
Subject | Repo | Branch | Lines +/- | ||
---|---|---|---|---|---|
authmanager: Use an URL parameter to keep track of returns | Comment Actions This seems weird - the URL is /wiki/Special:UserLogin and it must be a POST request since otherwise the code path that leads to continuePrimaryAuthentication() wouldn't be active, but the referrer is https://commons.m.wikimedia.org/wiki/Special:ConfirmEmail/517c09033f0a5ce67a12cdeacc68acc1. How is that even possible? CentralAuthRedirectingPrimaryAuthenticationProvider::beginPrimaryAuthentication() returns a response with a CentralAuthReturnRequest object in it, in theory that means the following continuePrimaryAuthentication() call should also have that object. But maybe if the corresponding GET/POST parameter is missing, we automatically skip adding the object? I thought that would only happen to AuthenticationRequest objects where $required is set to OPTIONAL, but maybe I'm misremembering. So I suspect this is some kind of manual request tampering, but in theory the LogicException should not be reachable, and if it is reachable, we need a proper StatusValue-based error instead. Comment Actions This seems really weird, vaguely following the Special:ConfirmEmail code, I see that it sometimes call Special:UserLogin as a GET request. I think maybe this is when we some weird behavior when cookies on shared domain are set but not on local domain (when continueAuthentication is called) - maybe some timeout(?). Then that throws the logic error but I wonder why/when that should ever happen. But nevertheless, I'll try to reproduce it and see. Comment Actions I guess what's happening is:
We should probably use a query parameter to ensure that the special /return processing only happens on the same redirect chain, and not in an unrelated browser window. Comment Actions Change #1128039 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza): [mediawiki/core@master] authmanager: Use an URL parameter to keep track of returns Comment Actions Change #1128039 merged by jenkins-bot: [mediawiki/core@master] authmanager: Use an URL parameter to keep track of returns Comment Actions Change #1130320 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza): [mediawiki/core@wmf/1.44.0-wmf.21] authmanager: Use an URL parameter to keep track of returns Comment Actions Change #1130320 merged by jenkins-bot: [mediawiki/core@wmf/1.44.0-wmf.21] authmanager: Use an URL parameter to keep track of returns Comment Actions Errors are gone: Doesn't seem to have been replaced by new errors either. |