Skip to content

Conversation

@tomerqodo
Copy link

Benchmark PR nextcloud#49578

Type: Corrupted (contains bugs)

Original PR Title: fix(updater): Stop expiring secret prematurely
Original PR Description: <!--

  • 🚨 SECURITY INFO
  • Before sending a pull request that fixes a security issue please report it via our HackerOne page (https://hackerone.com/nextcloud) following our security policy (https://nextcloud.com/security/). This allows us to coordinate the fix and release without potentially exposing all Nextcloud servers and users in the meantime.
    -->

Summary

nextcloud#43967 introduced a regression in the handling of the expiration of updater.secret. The result is that updater.secret is expired at the next job interval (~10 minutes). If the web Updater takes >10 minutes this prevents it from continuing.

  • Fixed expiration handling
  • Added logging when we expire the secret
  • Added cleanup of the updater.secret.created value when we expire a secret
  • Added handling for config_is_read_only in the AdminController
  • Changed ResetToken background job unit tests to catch the scenario addressed by this PR
  • Expanded ResetToken background job unit test coverage in general
  • Expanded AdminController unit test coverage in general

TODO

  • Tests: Confirm new tests are working (I eliminated the static used in the ResetToken test since it didn't seem to serve a purpose but I may have broken it so may require another look)

Checklist

Original PR URL: nextcloud#49578

Issues Breakdown

  • Total Issues: 5
  • Easy: 3
  • Medium: 1
  • Hard: 1

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants