Page MenuHomePhabricator

user authentication with tokens broken (API)
Closed, DeclinedPublic

Description

Author: srbauer

Description:
using the login tokens retrieved with action=login does not work for me - neither in get or post mode.

I get the following error message with the tokens retrieved seconds before for an allpages query (values high to make sure I get an error):
"error":
"code": "apaplimit"
"info": "aplimit may not be over 500 (set to 50000) for users"

BTW: The code seems wrong too.


Version: unspecified
Severity: normal

Details

Reference
bz10308

Event Timeline

bzimport raised the priority of this task from to Lowest.Nov 21 2014, 9:54 PM
bzimport set Reference to bz10308.

srbauer wrote:

Sorry, forgot to add: I'm using a bot account.

Bot accounts are allowed a *five* thousand limit, not *fifty* thousand. I agree that the error should be "may not be over 5000" and the code should be "aplimit", I'll look into that later today. But however hard you try, 50,000 is never gonna work.

srbauer wrote:

To value 50000 is an arbitrary value because I just wanted to the error message in this case to check my login status. I know the 500/5000 limit.

Created attachment 3801
Fixes double parameter prefix in error message

The attached patch fixes the "apaplimit" issue (which, BTW, was present with ALL modules: setting rclimit too high resulted in an rcrclimit error).

I'm still investigating the token issue. The token doesn't work for me either, but action=login also attaches a cookie to its response, which does work.

attachment ApiBase-doubleparam.patch ignored as obsolete

Created attachment 3802
Handle lgtoken and friends in ApiMain::__construct()

I've attached a (working) patch that fixes this bug, but the implementation is kind of hacky. ApiMain::__construct() checks whether lgtoken and friends are set, and re-inits $wgUser from the lg* data. This is done by copying the lg* data to the $_SESSION array, where User::loadFromSession() will eventually find it.

Attached:

Fixed codes in r23358. Roan thanks for the patch but I had to modify it a bit to avoid introducing a new parameter in dieUsage(). Hope i didn't mess it up much :)

The login patch needs to be reworked -- all parameters need to go through the parameter parsing system - declared in getAllowedParams(), described in getParamDescription(). The question still remains why user cannot login using action=login. Also, if we decide to add the ability to login during query, the login process (parameter validation, creating session, etc) must go through the same code steps as action=login. For example, it could perform a nested api call just like feedwatchlist.

(In reply to comment #7)

The login patch needs to be reworked -- all parameters need to go through the
parameter parsing system - declared in getAllowedParams(), described in
getParamDescription(). The question still remains why user cannot login using
action=login. Also, if we decide to add the ability to login during query, the
login process (parameter validation, creating session, etc) must go through the
same code steps as action=login. For example, it could perform a nested api
call just like feedwatchlist.

getAllowedParams() will prepend a prefix (e.g. 'ap' for list=allpages) to the parameters, and we'll end up with aplgtoken, something I don't think we want. A user *can* login through action=login, that's not the problem. The problem is that passing lgusername, lguserid and lgtoken with every request after the login should allow the user to remain logged in, something that hadn't been implemented yet.

To illustrate:

  1. I request api.php?action=login&lgname=Catrope&lgpassword=PASSWORD
  2. I get back something like <login lgusername="Catrope" lguserid="1" lgtoken="123ABC">
  3. I request api.php?action=delete&title=User:Catrope&lgusername=Catrope&lguserid=1&lgtoken=123ABC

This should allow me to successfully delete User:Catrope even if I don't pass a cookie on the second request.

I still fail to see what the bug is about. Upon successful login, session cookie is set by the server:

enwikiToken=11111; enwikiUserName=Yurik; enwikiUserID=11111; enwiki_session=11111

Also, api returns 3 additional values that are not exactly needed because they duplicate the above (I am contemplating removing them). Note that session id is not returned.
$result['lguserid'] = $_SESSION['wsUserID'];
$result['lgusername'] = $_SESSION['wsUserName'];
$result['lgtoken'] = $_SESSION['wsToken'];

When the next request is made, user's session is loaded from the sesion cookie, not from the parameters, and should work. If it doesn't, we have a bug in the code, which we should fix, but we should not introduce more code to work around it, unless there is a good reason.

Lastly, in case we decide to implement an ability to supply login/domain/password with each request, we can do it by manually calling the login module from the ApiMain in a "transparent" mode -- ApiLogin will analyze the parameters, decide if it needs to attempt a login, and return success/failure to ApiMain. ApiMain may then either continue with the requested module or stop.

I agree that sessions are probably the way to go. WONTFIX?

I will fontfix it for now. If the actual bug appears, feel free to reopen.