fix(session): Do not update authtoken last_check for passwordless#44670
Merged
ChristophWurst merged 1 commit intomasterfrom Apr 26, 2024
Merged
fix(session): Do not update authtoken last_check for passwordless#44670ChristophWurst merged 1 commit intomasterfrom
ChristophWurst merged 1 commit intomasterfrom
Conversation
2 tasks
juliusknorr
approved these changes
Apr 8, 2024
Member
Author
|
/backport to stable28 |
Member
Author
|
/backport to stable27 |
Member
|
Does this reintroduce #29678 ? |
Member
Author
Good catch but I don't think so. The old issue was that we missed to updated last_check when we actually did check the password. Here we skip the update intentionally because no checks are performed. For the path of password checks the token update is still in place: server/lib/private/User/Session.php Line 787 in cc42b2e The unit test added covers an assertion for the token update. |
Altahrim
approved these changes
Apr 26, 2024
Member
Author
Tested by setting the last_activity to 0 and the timestamp is updated. |
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
cc42b2e to
21ee7f5
Compare
This was referenced Apr 26, 2024
Contributor
|
/backport to stable26 |
2 tasks
Contributor
|
/backport to stable29 |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The column oc_authtoken.last_check is used to debounce the password checks of the login password encoded in app passwords. If instances have set
auth.storeCryptedPasswordtofalse, the password column stays NULL and there is no password check. In that case we keep the last_check column updated without any actual check performed. This is superfluous.How to test
auth.storeCryptedPassword => false,in config/config.phpcurl -u "USER:APPPASSWORD" https://localhost/apps/filesMaster: last_check is set to the current timestamp. This indicates that there was an UPDATE query.
Here: last_check remains 0.
Note: in reality last_check will stay at the time of token creation. You can skip setting last_check to 0 but then you have to wait more than five minutes between creating and using the token.
Checklist