@tsteur opened this Pull Request on January 17th 2020 Member

fix #6559

  • We now store token_auth no longer in the user table but hashed in a new table user_token_auth
  • To not needing to rewrite the entire app re piwik.token_auth basically now storing in the session an extra token auth which is randomly generated when a user logs in. This random token is basically just like an nonce and because we need to store it in the session, it is stored in plain text in the database (base64 encoded in the session table). The plain text storage is not an issue though since it is not really a token auth but more an nonce and it only works if you also have the sessionID. And if you have the sessionId, you are authenticated anyway
  • Also created https://github.com/matomo-org/matomo/pull/15390 so sessionIDs are hashed as well.
  • Also removes login and the weakly hashed token_auth from the cookie.
  • To not break BC for existing token_auths we migrate them automatically.
  • Token auths still have same permission and access as the actual user. Created https://github.com/matomo-org/matomo/issues/15368 which may be interesting in the future.
  • We differentiate between system tokens and regular user tokens.
    • User auth token is a token created by the user and a user can delete them etc.
    • A system auth token is created by the system and not visible to the user. It's used in the background when needed. This tokens have an expiry date and are invalidated/deleted typically within a few hours or days. They were needed because eg when we archive using http (when CliMulti cannot be used), then we need to issue an HTTP request from CoreArchive and this request has to be authenticated. In this case CoreArchive generates a token auth that is valid for say 2 days and it can use this generated token to authenticate the archiving requests.
  • We store the tokens hashed as sha512. Was going to use sha3-512 but so far sha512 seems more "proven". There is a column in the table storing the algorithm for future migrations.
  • Introduces a new page "Security" where these tokens can be managed plus the password and 2FA.

Todo before release

  • We'll need to adjust the mobile app before the release to support this new flow of accessing tokens and also to let them enter a token directly.
  • We'll need to adjust some docs/faqs as we now only show the token after creation only.
@tsteur commented on January 17th 2020 Member

Ready for a first review...

Will in the mean time need to check why some screenshots like https://builds-artifacts.matomo.org/matomo-org/matomo/4.x-dev/37868/UIIntegrationTest_admin_home.png
and some Dashboard screenshots seem to be failing now.

@tsteur commented on January 26th 2020 Member

The tests should now pass.

@diosmosis commented on February 7th 2020 Member

Left some comments, have a pending review w/ some TODOs for myself when reviewing. Will test tomorrow and submit the review.

@diosmosis commented on February 11th 2020 Member

Noticed an issue when trying to test, the code seems to be trying to use matomo_user_token_auth before the update can run. So it fails with:

image
@diosmosis commented on February 14th 2020 Member

Noticed something else: removing the token_auth column makes downgrading impossible. Not sure if it's an issue for users, but for development it makes it impossible to go back to 3.x-dev and test w/o adding the column back and then setting a token auth manually.

@tsteur commented on February 20th 2020 Member

Noticed something else: removing the token_auth column makes downgrading impossible. Not sure if it's an issue for users, but for development it makes it impossible to go back to 3.x-dev and test w/o adding the column back and then setting a token auth manually.

I could maybe instead of removing the column generate some random token_auth value and keep the existing column?

I can't keep the original token_auth value since it be a security issue as the token would then still be stored in plain text. The only way I can think of be to keep the column but set a random value for each token which is basically not in use as part of Matomo 4. Could be bit confusing but would allow to downgrade.

@diosmosis commented on February 20th 2020 Member

I could maybe instead of removing the column generate some random token_auth value and keep the existing column?

That sounds like it could work 👍

Powered by GitHub Issue Mirror