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

Never send token_auth as GET parameter, but send it as POST instead #7349

Closed
mattab opened this issue Mar 4, 2015 · 13 comments
Closed

Never send token_auth as GET parameter, but send it as POST instead #7349

mattab opened this issue Mar 4, 2015 · 13 comments
Labels
c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Milestone

Comments

@mattab
Copy link
Member

mattab commented Mar 4, 2015

The goal of this issue is to ensure that in Piwik core, including the core:archive cron task and other logic, we will not send the token_auth as a GET parameter. Instead we should send with POST the token_auth so that it does not show up in logs and whenever the GET URL is output

This follows up #5277 and #7301

Also related to #4171

@mattab mattab added Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. labels Mar 4, 2015
@mattab mattab added this to the Mid term milestone Mar 4, 2015
@mattab
Copy link
Member Author

mattab commented Mar 4, 2015

Note: this is already done for Ajax requests done in the UI, was done in #3359

@mnapoli
Copy link
Contributor

mnapoli commented Mar 4, 2015

Note that we can also use HTTP headers to pass the token. I don't know if there's any reason to prefer one method to the other though.

@mnapoli
Copy link
Contributor

mnapoli commented Mar 4, 2015

After having a look, we have for example Amazon and GitHub that use the official Authorization HTTP header. That's probably a good example to follow.

One advantage I see with this is that you are not forced to use POST requests: you can keep using GET, but also any other HTTP method. So this would be necessary if we ever want to do REST, so I think using headers is better because it would be forward compatible.

@mattab
Copy link
Member Author

mattab commented Mar 4, 2015

One advantage I see with this is that you are not forced to use POST requests: you can keep using GET

quick note: the change in this issue would not force to use POST or GET, as already Piwik API will work when called on POST or GET (either will work). currently ajax requests in the UI use POST (to hide the token_auth) and core:archive command use GET.

@mnapoli
Copy link
Contributor

mnapoli commented Mar 4, 2015

It would "force" to use POST if you want to keep the token secure though.

@mattab
Copy link
Member Author

mattab commented Mar 4, 2015

To clarify: this issue is only about controlling within the Piwik
platform itself that we don't use token_auth as GET.

In general, we cannot force to use POST since already thousands of users
use GET when talking to their Piwik API and we could not break it for
them - even for 3.0.0 or other major versions

@mnapoli
Copy link
Contributor

mnapoli commented Mar 4, 2015

Yes the GET method with URL parameter would still work in any case.

To clarify: this issue is only about controlling within the Piwik platform itself that we don't use token_auth as GET.

I understand but I don't see how it affects POST vs headers. We could change all requests in Piwik itself to use POST, but we could also change them to use headers for the token (and add support for that in the API). I believe going the "headers" way is better because it would allow us to move more easily towards rest.

If we just do POST instead of GET everywhere inside Piwik, that's going the opposite way of rest. E.g. using POST to get information doesn't make sense in HTTP.

@mattab
Copy link
Member Author

mattab commented Mar 4, 2015

If we just do POST instead of GET everywhere inside Piwik, that's going the opposite way of rest. E.g. using POST to get information doesn't make sense in HTTP.

Ok I now understand your point, that we should not use POST because it's not the way of rest. +1 for that and for Authorization header then 👍

@MagicFab
Copy link

@mattab could you clarify if this affects using widgets? It seems only anonymous access would keep widgets useful as otherwise widgets would require including the token in the URL exposing it (in code, logs, etc.). Correct me if I am wrong.

@mattab mattab modified the milestones: Long term, Mid term Dec 5, 2016
@c-prompt
Copy link

c-prompt commented Apr 1, 2019

Looking through my access_log recently reminded me of a comment @tsteur made in #14099:

BTW: You want to send those requests through POST request otherwise you may have eg the token_auth from the request in web server log files

I just noticed the token_auth is shown in access_logs anytime the PHP Tracking Web API client (Method 2: HTTP Request) is used. For example, I see a bunch of these in my access_log:

12.345.67.89 - - [31/Mar/2019:12:13:29 -0500] "GET /matomo/piwik.php?idsite=1&rec=1&apiv=1&r=963767&cip=987.65.43.210&token_auth=my_admin_token_auth&_idts=1234094409&_idvc=0&_id=4a29e8bd0d739ec2&url=https%3A%2F%2Fwebsitename.com&urlref=&pv_id=714aa3&action_name=API+was+used HTTP/1.1" 200 3312 "-" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/70.0.3538.77 HeadlessChrome/70.0.3538.77 Safari/537.36"

According to Tracking HTTP API:

To track page views, events, visits, you have to send a HTTP request (GET or POST) to your Tracking HTTP API endpoint, for example, http://your-piwik-domain.example/piwik.php with the correct query parameters set.

However, it's not clear how to send these via POST when using PiwikTracker (instead of CURL). I haven't dug into Matomo's code but shouldn't the PiwikTracker be using POST so the token_auth doesn't show up? Is there a way to force PiwikTracker to use POST? Here's some basic scrubbed code I'm using:

<?php
require_once('/matomo/libs/PiwikTracker/PiwikTracker.php');

\PiwikTracker::$URL = 'https:' . BASE_URL . 'matomo';
$piwikTracker = new \PiwikTracker(1, 'https:' . BASE_URL . 'matomo');

$token = $this->getMatomoTokenAuth();

if (isset($matomo['force_new_visit']) && $matomo['force_new_visit'])
{ $piwikTracker->setForceNewVisit(); }

if (isset($matomo['visitorId']) && $matomo['visitorId'])
{ $piwikTracker->setVisitorId($matomo['visitorId']); }

if (isset($matomo['user-agent']) && $matomo['user-agent'])
{ $piwikTracker->setUserAgent($matomo['user-agent']); }

$piwikTracker->setTokenAuth($token);
$piwikTracker->setUrl($matomo['url']);
$piwikTracker->setUrlReferer($global['HTTP_REFERER']);
$piwikTracker->setIp($global['ip_address']);

if ($username) { $piwikTracker->setUserId($username); }

$piwikTracker->doTrackPageView(urldecode($matomo['page_title']));
$visitorID = $piwikTracker->getVisitorId();

@tsteur
Copy link
Member

tsteur commented Apr 1, 2019

I suggest you create an issue in the Matomo PHP Tracker for this: https://github.com/matomo-org/matomo-php-tracker/
For the archiving there is #14190

@tsteur
Copy link
Member

tsteur commented Dec 7, 2023

This one might be done?

@sgiehl
Copy link
Member

sgiehl commented Dec 7, 2023

Yes. The UI shouldn't be sending the token_auth as GET anywhere anymore.

@sgiehl sgiehl closed this as completed Dec 7, 2023
@sgiehl sgiehl added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. and removed not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. labels Dec 7, 2023
@sgiehl sgiehl modified the milestones: Backlog (Help wanted), 4.14.0 Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Projects
None yet
Development

No branches or pull requests

9 participants