Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: drop support for php 8.0 and nextcloud <= 29 #575

Merged
merged 4 commits into from
May 7, 2024
Merged

Conversation

st3iny
Copy link
Member

@st3iny st3iny commented Apr 16, 2024

Required for #574 and #573

Fix #573

This has to be done because we updated web-auth/webauthn-lib in server and we have to update it here too. It only supports PHP >= 8.1.

Ref nextcloud/server#44761

@st3iny st3iny added the 3. to review Waiting for reviews label Apr 16, 2024
@st3iny st3iny self-assigned this Apr 16, 2024
@st3iny st3iny changed the title feat!: drop support for PHP 8.0 feat!: drop support for php 8.0 and nextcloud <= 29 Apr 16, 2024
@st3iny
Copy link
Member Author

st3iny commented Apr 16, 2024

Oh god, what have we done?! Tests fail because the already updated version of the webauthn dependency from server is loaded.

So we need to drop PHP, update the dependency, fix deprecations and fix tests all at the same time.

@st3iny st3iny added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Apr 16, 2024
@st3iny st3iny marked this pull request as draft April 16, 2024 18:58
@st3iny st3iny added the enhancement New feature or request label Apr 16, 2024
@st3iny st3iny added this to the 2.0.0 milestone Apr 16, 2024
Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
@st3iny st3iny force-pushed the feat/drop-php-8.0 branch 3 times, most recently from 339a6a7 to f3d6509 Compare April 19, 2024 16:41
@st3iny st3iny added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 19, 2024
@st3iny st3iny marked this pull request as ready for review April 19, 2024 16:53
Adapt WebAuthn code in the backend. Replace custom (non-compliant)
WebAuthn code in the frontend with @simplewebauthn/browser analogous to
the changes in the server repository.

Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
Copy link
Member Author

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Things I tested

U2F migrator

  1. Set up a fresh Nc 23 instance.
  2. Install (deprecated) twfactor_u2f on Nc 23.
  3. Switch to a Nc master instance.
  4. Copy U2F registration table to my Nc master instance.
  5. Import real U2F data provided by Christoph.
  6. Run the migrator
  7. ✅ No errors and entries were migrated.

DB migrations

  1. Set up a fresh Nc 23 instance.
  2. Install the most recent version of twofactor_webauthn before we took over: https://github.com/nextcloud/twofactor_webauthn/releases/tag/v0.2.15
  3. Enable the app and add 2 keys.
  4. Switch to a Nc master instance.
  5. Install this branch and run all migrations.
  6. Delete all twofactor_webauthn migrations from the DB and remove the registration table.
  7. Import all twofactor_webauthn migrations and the registration table from the Nc 23 instance.
  8. Run all migrations on the master instance: occ migrations:migrate twofactor_webauthn
  9. ✅ All imported registrations from the Nc 23 instance were migrated without errors.
  10. ✅ Logging in with the imported devices still works.

Summary

The switch from Ramsey\Uuid\Uuid to Symfony\Component\Uid\Uuid did not break anything despite having to modify old database migrations.

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

:shipit:

@st3iny st3iny merged commit 9387075 into main May 7, 2024
27 checks passed
@st3iny st3iny deleted the feat/drop-php-8.0 branch May 7, 2024 12:53
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 enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

Bump web-auth/webauthn-lib to 4.8.5
2 participants