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

Tracking API behaviour on incorrect token_auth should be improved #10685

Closed
dropadrop opened this issue Oct 5, 2016 · 2 comments
Closed

Tracking API behaviour on incorrect token_auth should be improved #10685

dropadrop opened this issue Oct 5, 2016 · 2 comments
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. c: Usability For issues that let users achieve a defined goal more effectively or efficiently. Help wanted Beginner friendly issues or issues where we'd highly appreciate community's help and involvement.

Comments

@dropadrop
Copy link

I'm importing historical data to piwik through the tracking API, and after resetting to notice the auth_token changed on user password reset I spent quite a while wondering where my events were going.

With each event I also set the original time it was received, ip-address and other location related data. Setting these require also providing the auth_token.

Current behaviour:
It seems that if the auth_token is provided but incorrect then the data is stored without the values requiring the auth token. No warnings are logged, calls receive status 200, so no visible indication that things are going sour.

Expected behaviour:
A clear indication that things are going wrong, IMO the events should not even be stored as providing a token_auth indicates that the party sending the event wants to be authenticated so you are either looking at storing wrong / partial data for a valid user or storing bogus data for an "attacker".

I'd say there are 3 possible solutions (or combination):

  1. Return http status different from 200. This would also have side effects as it could be used to try to figure out the auth_token (btw, better would be to use auth_token as a shared secret and use it as salt to hash a set of required parameters, then include the hash as a parameter)
  2. Behave like now but log a warning
  3. Log a warning and don't store the data

I'd think 3 would be the correct option. Generally when I'm calling the API I'm importing hundreds of thousands of events so cleaning out bad data becomes challenging, especially if I already have lots of valid data for the same site.

@mattab
Copy link
Member

mattab commented Nov 12, 2016

Thanks @dropadrop for the report. In Tracking API when token_auth is invalid it would make sense to log a warning. Not sure about not storing the data at all... but it might be the more correct thing to do indeed. When storing partial data, the warning messages and data inacurracies could be easily ignored... while no data tracked makes it easier to find the problem. If we don't track any data at all then we can also return http status 401 Unauthorized.

@mattab mattab added this to the 3.0.0 milestone Nov 12, 2016
@mattab mattab added c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. c: Usability For issues that let users achieve a defined goal more effectively or efficiently. labels Nov 12, 2016
@mattab mattab added the Help wanted Beginner friendly issues or issues where we'd highly appreciate community's help and involvement. label Oct 6, 2017
@tsteur
Copy link
Member

tsteur commented Dec 13, 2018

I think this was fixed in #13675

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. c: Usability For issues that let users achieve a defined goal more effectively or efficiently. Help wanted Beginner friendly issues or issues where we'd highly appreciate community's help and involvement.
Projects
None yet
Development

No branches or pull requests

3 participants