Skip to content

Conversation

@susnux
Copy link
Contributor

@susnux susnux commented Jun 4, 2025

Summary

Originally this should have been only a refactoring to migrate the code to Typescript and add some more tests for it.
But doing so I discovered some issues within the logic that revealed to cause the linked issue (or to cause at least the frontend part of it).

  1. If the request fails we should NOT update the token (do not emit an event!)
  2. if a invalid token is about to be set do not update it
  3. some issues noticed where we use the wrong types (e.g. local storage only works with strings).

The issue itself if likely to be orginally caused by some server issue (e.g. overloaded) which causes a 500 return, then we have an "undefined" token (as the old code did not check for the validity) and tried to update it.
This causes all following requests to use an invalid token.

Checklist

@susnux susnux added this to the Nextcloud 32 milestone Jun 4, 2025
@susnux susnux requested review from a team as code owners June 4, 2025 17:55
@susnux susnux added bug 3. to review Waiting for reviews labels Jun 4, 2025
@susnux susnux requested review from artonge, skjnldsv and sorbaugh and removed request for a team June 4, 2025 17:55
@susnux
Copy link
Contributor Author

susnux commented Jun 4, 2025

Best to review commit by commit - most changes are tests only.

@susnux
Copy link
Contributor Author

susnux commented Jun 4, 2025

/backport to stable31

@susnux
Copy link
Contributor Author

susnux commented Jun 4, 2025

/backport to stable30

@susnux susnux requested review from leftybournes and removed request for a team June 4, 2025 17:56
@susnux susnux force-pushed the fix/requesttoken branch from 0e5b17c to 094a48b Compare June 4, 2025 18:00
@susnux susnux force-pushed the fix/requesttoken branch 3 times, most recently from 76ab7d3 to 8193a42 Compare June 11, 2025 13:00
susnux added 4 commits June 16, 2025 15:54
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
…n reading order

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
susnux added 2 commits June 16, 2025 15:55
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the fix/requesttoken branch from a77f2b2 to 5f00649 Compare June 16, 2025 13:55
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the fix/requesttoken branch from a7940d8 to 5d0d490 Compare June 16, 2025 13:56
@susnux
Copy link
Contributor Author

susnux commented Jun 16, 2025

/compile

Copy link
Member

@AndyScherzinger AndyScherzinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐘

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@susnux susnux merged commit eb15061 into master Jun 16, 2025
202 of 210 checks passed
@susnux susnux deleted the fix/requesttoken branch June 16, 2025 15:55
@michnovka
Copy link

Thank you for this! The linked bug has been present for years, I really hope this is the end of it.

* Get interval in seconds
*/
function getInterval(): number {
const interval = seesionLifetime
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oups :octocat:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seeeesion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: CSRF check failed

8 participants