Skip to content

fix(oauth2): adjust db schemas when migrating from owncloud #50193

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.

Sign up for GitHub

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

Jump to bottom
Merged
AndyScherzinger merged 1 commit into from
Feb 5, 2025
Merged

fix(oauth2): adjust db schemas when migrating from owncloud #50193

AndyScherzinger merged 1 commit into from
Feb 5, 2025

Conversation

@st3iny
Copy link
Member

@st3iny st3iny commented Jan 15, 2025 •
edited
Loading

Summary

See also #49075

This PR fixes two issues:

  1. Not being able to create or auth using OAuth2 clients after migrating from ownCloud.
  2. Not being able to refresh legacy tokens from ownCloud after migrating to Nextcloud.

It does not aim to migrate legacy access tokens from ownCloud as this incurs a lot of work and tokens are only valid for an hour. So, it does not make sense as the migration window set by admins is probably longer than their lifetime anyway.

What I tested

  1. Install ownCloud (latest) and install/enable the oauth2 app.
  2. Add an oauth2 client, use it to generate an access token and refresh it at least once.
  3. Dump related tables: mysqldump owncloud oc_oauth2_access_tokens oc_oauth2_refresh_tokens oc_oauth2_clients
  4. Import dumped tables into Nextcloud.
  5. Reset all oauth2 migrations and drop oauth2 tables (oc_oauth2_access_tokens, oc_oauth2_refresh_tokens and oc_oauth2_clients).
  6. Import dumped tables.
  7. Migrate oauth2 and run repair steps: occ migrations:migrate oauth2 && occ maintenance:repair
  8. Refresh the token generated in step 2 using your oauth2 client (might be necessary to adjust the token URL if Nextcloud is on another domain).
  9. Observe that the refreshed access token can still be used to log into Nextcloud.

TODO

  • Needs to be tested with a live ownCloud instance (see above)

Checklist

@st3iny st3iny self-assigned this Jan 15, 2025
st3iny
st3iny commented Jan 15, 2025
View reviewed changes
@st3iny st3iny marked this pull request as ready for review January 16, 2025 10:51
@st3iny st3iny added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 16, 2025
nickvergessen
nickvergessen reviewed Jan 16, 2025
View reviewed changes
ChristophWurst
ChristophWurst reviewed Jan 20, 2025
View reviewed changes
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.

Looks good so far

@st3iny
Copy link
Member Author

st3iny commented Jan 20, 2025

/backport! to stable29

@backportbot backportbot bot mentioned this pull request Jan 20, 2025
Closed
2 tasks
@AndyScherzinger AndyScherzinger force-pushed the fix/oauth2/owncloud-migration branch from 91c93e1 to 0fc1f08 Compare February 5, 2025 17:34
@AndyScherzinger
Copy link
Member

AndyScherzinger commented Feb 5, 2025

/backport to stable30

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Feb 5, 2025

/backport to stable31

@st3iny st3iny force-pushed the fix/oauth2/owncloud-migration branch from 0fc1f08 to 754a047 Compare February 5, 2025 20:56
@st3iny
Copy link
Member Author

st3iny commented Feb 5, 2025

/backport to stable29

Let's recreate the backport as it was incomplete.

@st3iny st3iny added this to the Nextcloud 32 milestone Feb 5, 2025
@st3iny st3iny added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 5, 2025
@AndyScherzinger AndyScherzinger merged commit c45736c into master Feb 5, 2025
189 checks passed
@AndyScherzinger AndyScherzinger deleted the fix/oauth2/owncloud-migration branch February 5, 2025 21:15
This was referenced Feb 5, 2025
Merged
Merged
Merged
skjnldsv
skjnldsv reviewed Feb 6, 2025
View reviewed changes
lib/private/Repair.php
\OC::$server->getConfig()
),
new MigrateOauthTables(\OC::$server->get(Connection::class)),
\OC::$server->get(MigrateOauthTables::class),
Copy link
Member

@skjnldsv skjnldsv Feb 6, 2025

Choose a reason for hiding this comment

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

Suggested change
\OC::$server->get(MigrateOauthTables::class),
\OCP\Server::get(MigrateOauthTables::class),

Next time 😉

@joshtrichards joshtrichards mentioned this pull request Feb 14, 2025
Merged
4 tasks
@susnux susnux mentioned this pull request Feb 23, 2025
Closed
8 tasks
@joshtrichards joshtrichards mentioned this pull request Feb 26, 2025
Closed
8 tasks
@nextcloud-bot nextcloud-bot mentioned this pull request Aug 19, 2025
Merged
@skjnldsv skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@skjnldsv skjnldsv skjnldsv left review comments

@nickvergessen nickvergessen nickvergessen approved these changes

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@julien-nc julien-nc Awaiting requested review from julien-nc

Assignees

@st3iny st3iny

Labels

4. to release Ready to be released and/or waiting for tests to finish bug feature: authentication feature: owncloud migration

Projects

💌 📅 👥 Groupware team
Status: ☑️ Done

Milestone

Nextcloud 32

Development

Successfully merging this pull request may close these issues.

[Bug]: The schema of oc_oauth2_access_tokens is not adjusted properly and its data is not migrated correctly when migrating from ownCloud

6 participants

@st3iny @AndyScherzinger @nickvergessen @ChristophWurst @skjnldsv