Fix initializing anonymous session #17084
Conversation
sgiehl
added
not-in-changelog
|
Looking at the test results indicates, that we actually never returned a success state for anonymous before. The tests for that didn't change since 2014. |
plugins/Login/Auth.php
Outdated
| @@ -114,6 +114,10 @@ private function authenticateWithLoginAndToken($token, $login) | |||
| { | |||
| $user = $this->userModel->getUserByTokenAuth($token); | |||
|
|
|||
| if (empty($user) && $login === 'anonymous') { | |||
There was a problem hiding this comment.
that would have regressed with the App specific token auths and I suspect it works on demo because it was installed pre Matomo 4 where we migrated existing token auths. So it would have been broken for new Matomo 4 installs. In tests I think we automatically create such a token maybe. So the alternative could be creating an anonymous token auth but not sure if that would have any side effects etc (eg if user deletes that token or user or so)... Does it also need to be added to authenticateWithToken (maybe needed for when calling the API)?
There was a problem hiding this comment.
or maybe a different fix be to add token_auth for anonymous user on install
There was a problem hiding this comment.
But my local install was a pre Matomo 4 as well. And there is a migrated token_auth for anonymous user in the database.
There was a problem hiding this comment.
yes sorry. I was meaning it wouldn't be there maybe for new installs?
There was a problem hiding this comment.
the original user said it happened after an update I don't think it is due to being a new install?
I debugged a bit locally and noticed in Login/Auth.php, $token is set to anonymous. Since this is not a token that is in user_token_auth of course it wouldn't find the user. Can you confirm if this is the case for you @sgiehl ?
There was a problem hiding this comment.
I see. It's actually here for new installs: https://github.com/matomo-org/matomo/blob/4.1.1-b2/core/Db/Schema/Mysql.php#L579 so maybe there was just a migration issue? I've likely deleted my token auths table a few times which explains why I ran into the issue.
There was a problem hiding this comment.
Ok. That makes sense. Guess I deleted my token table a few times as well. So what would be the preferred solution?
Readding the token with an update script? Or should we try doing that on each update, to ensure it doesn't get lost somehow? Guess we could also try adding it when someone changes the settings for anonymous user access maybe.
There was a problem hiding this comment.
Is there an update file that has the addition of the anonymous user_token_auth entry? I can't seem to find it (by grepping for anonymous in core/Updates).
There was a problem hiding this comment.
it was done here: https://github.com/matomo-org/matomo/blob/4.0.0-rc4/core/Updates/4.0.0-b1.php#L76 and AFAIK an anonymous user should usually always exist.
No preference on how to fix it. There are a lot of ways... could be also a mix of different ways... easiest be though to ensure maybe in a regular task maybe and when enabling/disabling anonymous that the token exists or not exists etc.
There was a problem hiding this comment.
I've slightly modified the PR so getUserByTokenAuth for the token anonymous will always return the anonymous user. This should fix the problem without the requirement to have an anonymous token at all. And it should also be possible to authenticate in API with token anonymous
sgiehl
force-pushed
the
fixanonymousaccess
branch
from
dfd5f8d
to
ca1d919
Compare
sgiehl
force-pushed
the
fixanonymousaccess
branch
from
ca1d919
to
0d6855f
Compare
|
code looks ok to me, can't think of a way this could be abused. not sure if @tsteur wants to take a look before merging? |
|
Looks good @diosmosis 👍 |
Description:
I've debug for quite a while now, but actually couldn't find the change that actually caused the problem.
Even going a few hundreds commits back in history didn't fix the issue locally, so it seems to be there for a while.
Which actually makes it even more strange that it still works on demo.
Nevertheless, for some reason the Authentication does never return a success state for
anonymoususer. So the Auth object holds no login, and the list of available sites stays emptyfixes #17077
Review