try to fix two UI tests #15755
try to fix two UI tests #15755
Conversation
diosmosis
added
the
not-in-changelog
diosmosis
changed the title
diosmosis
changed the title
diosmosis
added
Needs Review
|
Note: the one click update changes should be merged into 3.13.5 to make sure as many people as possible have an error free one click update to matomo 4. Though we'll have to try it before it's released... |
|
@diosmosis the changes for updater are exactly the same as in the PR for |
diosmosis
changed the title
|
|
||
| await page.type('#login_form_login', 'superUserLogin'); | ||
| await page.type('#login_form_password', 'superUserPass'); | ||
| await page.click('#login_form_submit'); |
There was a problem hiding this comment.
So automatic login after update won't work anymore? 🤔
There was a problem hiding this comment.
What automatic login are referring to? From my understanding the session is just sent again after the update, but I think it's when updating to 4.x due the app specific token auth change it requires a new login. Otherwise anyone would get logged in even if they aren't authorized to?
There was a problem hiding this comment.
That's what I meant. Guess it's not worth to have a closer look if it's possible to not require a new login...
There was a problem hiding this comment.
@tsteur might know if the session should still work
There was a problem hiding this comment.
if it's ok that the session gets terminated, the changes would be good to merge
There was a problem hiding this comment.
but I think it's when updating to 4.x due the app specific token auth change it requires a new login. Otherwise anyone would get logged in even if they aren't authorized to?
Not sure I fully understand the problem @sgiehl @diosmosis
Is this something that happens for the actual user or is more of a problem only for tests? Generally, when updating, the current token_auth should be automatically be migrated to an app specific token and still work. I suppose the token_auth is not really used for the session though.
What does change is indeed the session cookie maybe as we're removing login and token hash or so from the cookie AFAIK. Not sure if this causes the issue?
If user gets logged out would this cause an issue for https://github.com/matomo-org/matomo/pull/15770/files#diff-adf51e994c74a650d8afec954288d4c3R178 where we require user to be logged in? Not sure if user can be logged out in between (either by this session change or because the cookie expire time which might expire after 30 minutes just in between steps).
Looking at the original PR for app specific tokens, I can only imagine that maybe this session value is missing and causing an issue (random guess) https://github.com/matomo-org/matomo/pull/15410/files#diff-6c322e8c4c05670597358309f152c51aR94
Generally I would say it's fine though if a user gets logged out during the update I would say. As long as it's possible to complete the update fully.
There was a problem hiding this comment.
If user gets logged out would this cause an issue for https://github.com/matomo-org/matomo/pull/15770/files#diff-adf51e994c74a650d8afec954288d4c3R178 where we require user to be logged in?
It might be... though we only make the request internally in Matomo, so maybe we can create a temporary token auth for that request to make sure it works?
There was a problem hiding this comment.
@tsteur made the change after merging in changes from 3.x, can review it again
mattab
added
the
not-in-changelog
Changes: