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 tokens / "users" should be separate from regular users #14353

Closed
Amunak opened this issue Apr 17, 2019 · 5 comments
Closed

API tokens / "users" should be separate from regular users #14353

Amunak opened this issue Apr 17, 2019 · 5 comments
Labels
duplicate For issues that already existed in our issue tracker and were reported previously.

Comments

@Amunak
Copy link

Amunak commented Apr 17, 2019 •
edited

The current way of setting up API keys with security in mind is really cumbersome.

Ideally every API user should have their own API token (what you now call token_auth). The reasons are as follow:

  • It should be possible to give specific rights to each API client even within a single website - some may need just "read" privileges, while others may need "write" privilege.
  • It should be possible to revoke single keys without affecting others in case a single API client becomes compromised.
  • Administrators should be able to manage API keys for the websites they have access to.
  • Regular users should not be able to see or manage API keys. It is an advanced use case, and giving users easy access to their own secret API key could compromise security if they don't understand how important that key is.
  • API users should not count towards "user count" in terms of licensing and such.

Currently, the only way to manage API keys with security in mind as an administrator is to:

  1. create a new user and give them at least "write" privileges and a strong password
  2. login as that user and write down their token_auth
  3. log back in as an administrator and give the user final / correct privileges (which may be read only in some use cases)
  4. repeat steps 1-3 for every single API token you need

Finally, when one of the tokens is compromised, you need to reset the password of the user (unless you saved it somewhere), log back in as them, regenerate the key and change it in your API client.

Alternatively you could use only a single "API user" or even your own auth token. But that has huge security issues. For example, when you need a token for tracking, using your own superadmin token would mean that if the API client gets compromised, your whole Matomo instance would be compromised until you noticed something.

In the same vein, even using a single user for all API clients isn't ideal. When one API client becomes compromised, it has access to other websites and more privileges. Moreover resetting the token means you now have to change it in multiple places, making it, again, pretty cumbersome.

If you do it the "secure way" and create a user for each API token you need those users (I assume) count towards the license limit(s). This puts people between a decision of better security versus a more expensiver license, which is terrible. I really hope this is not a deliberate choice to push people towards more expensive license tiers

Therefore I propose this:

Add a settings page for managing (generating and revoking) API keys. They would behave similar to users (in the way privileges are assigned), but they don't have a username, password or an email. For ease of implementation you could even use the same model(s) you use now, just leave the password blank, disallow login and use some generated username.

Furthermore if you wish to keep compatibility with the current state I'd advise making a toggle to disable token_auth for users. Ideally there would be a separate permission to allow or deny each user to see/generate their own token auth for use in e.g. alternative clients for Matomo.

This would greatly improve Matomo API security, limit user error leading to compromised instances and improve ease of use.
@Amunak Amunak changed the title API tokens / "ussers" should be separate from regular users API tokens / "users" should be separate from regular users Apr 17, 2019
@Findus23
Copy link
Member

I fully agree with you and think that this is one of the major things that need to be changed in Matomo. The issue is just that this is a large fundamental change that would break a lot of things (both plugins and integrations).
I think there is also already a github issue about this somewhere, but I can't find it at the moment.
Backwards compatibility could be improved by migrating token_auth to a new api key that has all rights and can be deleted after every integration is migrated.

Only thing I disagree with is Regular users should not be able to see or manage API keys:
I think the fact that all data can be also read in JSON/XML/etc. form is one of the most useful features in Matomo and should therefore be available to all users.
But of course it should be made sure that users understand the implications of creating an admin API key.

@Amunak
Copy link
Author

Amunak commented Apr 17, 2019 •
edited

I fully agree with you and think that this is one of the major things that need to be changed in Matomo. The issue is just that this is a large fundamental change that would break a lot of things (both plugins and integrations).
While it's true that requiring plugins and such to use the new system would break things, there is no reason to abandon the old token_auth system just yet. Especially if you want to keep users' access to API keys.

You can just keep the old system intact, and build up the new one. New plugins and integrations will want to use that, but nothing will stop old ones from working. Eventually the migration to just API keys could be considered as well, but that could be a secondary concern.

I think the fact that all data can be also read in JSON/XML/etc. form is one of the most useful features in Matomo and should therefore be available to all users.

Would removing auth_token break that? I didn't realize that. But if it is made so that users with only "view" permissions can create only "view" api keys and such, this shouldn't be a big issue. And while we wouldn't want admins to see the actual tokens (as per #10938) they must be able to see who has created what tokens, what permissions they have, when they were last used, etc. and they need to be able to delete those tokens.

But of course it should be made sure that users understand the implications of creating an admin API key.

I think I've already seen some warning somewhere, but even that didn't seem enough. I'd just hide the token by default (or not generated it at all if possible without breaking functionality) and then when the user clicks "show" they'd first be presented with a huge pop-up warning before being allowed to see or copy the string. This should be implemented with #13710.

I think there is also already a github issue about this somewhere, but I can't find it at the moment.

I went over like a year of closed and open issues with "API" and "token" in them and didn't find anything...

And now I found it I think: #6559

I was even thinking about maybe helping out but looking around the Matomo codebase it'd probably take a while for me to dig in it properly. I couldn't even find the model where users are stored.

@tsteur
Copy link
Member

tsteur commented Apr 17, 2019

I think this is pretty much a duplicate of #6559 ? And getting rid of token_auth should be happening in Matomo 4. At the same time we will very very likely need to implement #6559 .

In general we're totally aware of the changes that need to happen, it's just a massive undertaking and we need to plan this for a major release as there will be heaps of breaking things and lots of work.

Can we maybe close this as a duplicate?

@tsteur
Copy link
Member

tsteur commented Apr 17, 2019

There is also #9457 but it shouldn't be needed as we will very very likely remove token_auth altogether in favour of some other modern authentication methods.

@tsteur
Copy link
Member

tsteur commented Jan 8, 2020

Closing this for now as a duplicate

@tsteur tsteur closed this as completed Jan 8, 2020
@tsteur tsteur added the duplicate For issues that already existed in our issue tracker and were reported previously. label Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
duplicate For issues that already existed in our issue tracker and were reported previously.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@tsteur @Amunak @Findus23