Page MenuHomePhabricator

Allow oauthclient-php to forge requests containing files
Closed, ResolvedPublic

Description

Currently it is not possible to upload files using the Client class from the oauthclient-php library. The target wiki through an "Invalid signature" error when a post parameter contains a CurlFile.

Event Timeline

0x010C created this task.Feb 7 2018, 6:56 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 7 2018, 6:56 PM
Tgr added a subscriber: Tgr.Feb 7 2018, 7:04 PM

Can you provide an example request?

Change 408853 had a related patch set uploaded (by 0x010C; owner: 0x010C):
[mediawiki/oauthclient-php@master] Allow file upload through the Client

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

0x010C added a comment.EditedFeb 7 2018, 7:19 PM

Yes sure:

use MediaWiki\OAuthClient\Client;

$client = new Client(...);

[...]

$apiParams = array(
    'format' => 'json',
    'action' => 'upload',
    'filename' => 'placeholder.jpg',
    'text' => 'foo bar',
    'file' => new \CurlFile( 'path/to/my/file.jpg' ),
    'token' => $csrfToken
);

$result = $client->makeOAuthCall(
        $accessToken,
        'https://localhost/w/api.php',
        true,
        $apiParams,
        true //$hasFile, parameter I've added to manage OAuth calls containing files
    ) );

[...]

By default, Is it possible to upload file using oauthclient-php library? I am stuck here. Unable to upload any file.
Here is the request and response I am getting.

Request

$apiParams = array(
    'filename' => 'test-upload1232.jpg',
    'action' => 'upload',
    'file' => file_get_contents($path),
    'comment'=> 'Testing Upload',
    'text' => 'testing upload',
    'token' => $editToken,
    'format' => 'json',
);

$client->setExtraParams( $apiParams );

echo $client->makeOAuthCall(
    $accessToken,
    'https://test.wikipedia.org/w/api.php',
    true,
    $apiParams
);

Response

{"error":{"code":"badupload_file","info":"File upload parameter \"file\" is not a file upload; be sure to use \"multipart/form-data\" for your POST and include a filename in the \"Content-Disposition\" header.","*":"See https://test.wikipedia.org/w/api.php for API usage. Subscribe to the mediawiki-api-announce mailing list at <https://lists.wikimedia.org/mailman/listinfo/mediawiki-api-announce> for notice of API deprecations and breaking changes."},"servedby":"mw1343"}

@Tgr Please shed some light.

@0x010C Have you manged to upload file using this library?

@0x010C
I have tried your patch.

First error after adding your patch is

http_build_query(): Parameter 1 expected to be Array or Object. Incorrect value given

It is produced because of line number 269-271 from here https://gerrit.wikimedia.org/r/#/c/mediawiki/oauthclient-php/+/408853/2/src/Client.php

After disabling that I am getting an authorization error

{"error":{"code":"mwoauth-invalid-authorization","info":"The authorization headers in your request are not valid: Invalid signature","*":"See https://test.wikipedia.org/w/api.php for API usage. Subscribe to the mediawiki-api-announce mailing list at <https://lists.wikimedia.org/mailman/listinfo/mediawiki-api-announce> for notice of API deprecations and breaking changes."},"servedby":"mw1343"}

Because intially we were sending $this->config from the makeOAuthCall() now sending nothing.

:(

I have checked again and see that if I am changing the file parameter from

from

'file' => file_get_contents($path),

to

'file' => new \CurlFile( $path ),

then only I am getting this authorization header error.

@Jnanaranjan_sahu The $this->config parameter send to makeCurlCall in 5th position on line 253 in the initial version was useless, this method was only taking 4 parameters. In fact, makeCurlCall can access by itself to $this->config because it shares the same instance of the Client class with makeOAuthCall (as you can see on the lines 276/279 of the initial version).

In my first patch, you had to set to true a new $hasFile parameter when calling makeOAuthCall, so I guess you get the mwoauth-invalid-authorization error because you missed it. But as suggested by @Tgr, I've just removed it in a new patch by figuring out automatically if there is a file in postFields or not, so you can continue to use makeOAuthCall as you did before.

@0x010C Thank You Antoine for your respond. I am really stuck and thinking why I used this library as I am in the last stage of the developement and I got to know that this doesn't support file upload by default. Ahhh!

I understood your point.

Actually I have done it that way before. It checks if input has file parameter has value then $hasfile is true. But your method is right also.

The actual error now is
when I use

'file' => file_get_contents($path)

I get the this erro

{"code":"badupload_file","info":"File upload parameter \"file\" is not a file upload; be sure to use \"multipart/form-data\" for your POST and include a filename in the \"Content-Disposition\" header

But when I use your specified curl method

'file' => new \CurlFile( $path ),

I get the error as

"code":"mwoauth-invalid-authorization","info":"The authorization headers in your request are not valid: Invalid signature

Note: I am curious to know that if we are not posting the post variable(nor including that if hasfile is true) then how the data will be posted.

Thank You again. Hope I can solved this with your help.

I still had no luck regarding this.
I am moving the app from my local to toolforge.
Will I be able to download to toolforge repository and upload to commons via URL from there?

Change 408853 merged by jenkins-bot:
[mediawiki/oauthclient-php@master] Allow file upload through the Client

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

Tgr closed this task as Resolved.Dec 8 2018, 7:57 PM

Verified that uploading works now. Thanks!

@Jnanaranjan_sahu at a glance the problem is that you are using setExtraParams. File uploading means your request format will be multipart/form-data, not application/x-www-form-urlencoded as it usually is, and the OAuth specification says you need to sign an application/x-www-form-urlencoded body but not a multipart/form-data body.

0x010C added a comment.Dec 8 2018, 9:03 PM

@Tgr thanks for the review and the merge!

Jnanaranjan_sahu added a comment.EditedDec 9 2018, 6:13 AM

@Tgr Thanks for explaining.

That solved the issue. (That was too simple but I have been struggling. My bad)

Can upload using the new patch.

Can't we make it simple and use file_gets_content() instead if Curlfile class? Just asking

Tgr added a comment.Dec 9 2018, 7:40 AM

You can, but that will result in an urlencoded POST body. Maybe that works too, I don't know what are the exact requirements for the upload API.

As far as I can tell setExtraParams is not really useful (it's needed for the OAuth handshake, but that's abstracted away from the client; it's not needed for API calls). I removed the reference from the README.

Jnanaranjan_sahu added a comment.EditedDec 9 2018, 7:55 AM

Yes you are right.
If I use file_gets_content(), I am getting authorization error.

The authorization headers in your request are not valid: Invalid signature