Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

API keys are completely broken #674

Closed
anonymous-matomo-user opened this issue Apr 22, 2009 · 6 comments
Closed

API keys are completely broken #674

anonymous-matomo-user opened this issue Apr 22, 2009 · 6 comments
Labels
Bug For errors / faults / flaws / inconsistencies etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. worksforme The issue cannot be reproduced and things work as intended.
Milestone
RobotRock

Comments

@anonymous-matomo-user
Copy link

I posted a blog comment about this but I think someone deleted my comment. All the api token authentication is completely busted. I had posted the fix in the comment, but now i can't find it. I don't see any mention in the changelog so I'm assuming it's not fixed.
@anonymous-matomo-user
Copy link
Author

in core/API/Request.php parse_str is incorrectly used and overwrites all contents of $requestArray, thus erasing any supplied token_auth parameter. A new array should be passed to parse_str and the contents then merged into $requestArray.

            if(!is_null($request))
            {
                    $request = trim($request);
                    $request = str_replace(array("\n","\t"),'', $request);

$newAr = array();
parse_str($request, $newAr);
$requestArray = $requestArray + $newAr;

@anonymous-matomo-user
Copy link
Author

http://www.php.net/manual/en/function.parse-str.php#84645

comments on the parse_str page which verify that parse_str does erase the contents of the second parameter before putting the results into it.

@robocoder
Copy link
Contributor

Refactored in [1021]

@anonymous-matomo-user
Copy link
Author

changeset 1021 does not change the affected code. I checked SVN before I submitted this bug, it is still present.

https://github.com/piwik/piwik/blob/master/core/API/Request.php

    function __construct($request = null)
    {
        $defaultRequest = $_GET + $_POST;
        $requestArray = $defaultRequest;

        if(!is_null($request))
        {
            $request = trim($request);
            $request = str_replace(array("\n","\t"),'', $request);
            parse_str($request, $requestArray);

            // if a token_auth is specified in the API request, we load the right permissions
            if(isset($requestArray['token_auth']))
            {
                Piwik_PostEvent('API.Request.authenticate', $requestArray['token_auth']);
                Zend_Registry::get('access')->reloadAccess();
            }

            $requestArray = $requestArray + $defaultRequest;
        }

parse_str($request, $requestArray) <-- $requestArray is now an empty array. Next line is

if isset($requestArray['token_auth']))

^ always false (unless token_auth is present in $request)

But, we can see that token_auth is not present in $request

plugins/VisitsSummary/Controller.php getVisits()

            $requestString =        "method=VisitsSummary.getVisits".                                                        "&format=original".                                                        "&disable_generic_filters=true";
            $request = new Piwik_API_Request($requestString);
            return $request->process();

@anonymous-matomo-user
Copy link
Author

You can also test this bug by requesting any API plugin with an auth token and seeing that it does not work.

$host='example.com';
$path='/piwik/';
$token='apitokenhere';
$siteid=1;
$today   = date('Y-m-d');
$startDay = date('Y-m-d', (time() - 3600*24*12));

      $dataUrl =  'http://'.$host.$path .'index.php';
      $dataUrl .= '?module=VisitsSummary';
      $dataUrl .= '&action=getLastVisitsGraph';
      $dataUrl .= '&idSite='.$siteid;
      $dataUrl .= '&period=day';
      $dataUrl .= '&date='. $startDay;
      $dataUrl .= '%2C'. $today;
      $dataUrl .= '&viewDataTable=generateDataChartEvolution';
      $dataUrl .= '&disableLink=1';
      $dataUrl .= '&token_auth='.$token;
      $data =  file_get_contents($dataUrl);
      var_dump($data);

w/o patch you get a login HTML page.
w/patch you get the data.

@robocoder
Copy link
Contributor

Sorry, but I think you misread the code and PHP docs.

Yes, parse_str() stores its results in the second parameter (in this case, $requestArray). While it is true that we overwrite the previous contents of $requestArray where the code was earlier initialize, $requestArray is not an empty array.

Without your mods and no token, I get:

<?xml version="1.0" encoding="utf-8" ?>
<result>
    <error message="You can't access this resource as it requires a 'view' access for the website id = 1." />
</result>

If I use a valid token, I get the expected results for VisitsSummary.getVisits.

(BTW your last example is not a valid API call.)

@anonymous-matomo-user anonymous-matomo-user added this to the RobotRock milestone Jul 8, 2014
Closed
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug For errors / faults / flaws / inconsistencies etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. worksforme The issue cannot be reproduced and things work as intended.
Projects
None yet
Milestone
RobotRock
Development

No branches or pull requests
2 participants
@robocoder @anonymous-matomo-user