Skip to content

Conversation

@st3iny
Copy link
Member

@st3iny st3iny commented Feb 17, 2025

  • Resolves: None

Summary

This brings back support for legacy ownCloud clients in Nextcloud. They will continue to work seamlessly even when the OAuth authentication mode (default) is used.

How to test?

  1. Set up an ownCloud instance.
  2. Connect an ownCloud desktop client to it.
  3. Export all oauth2 tables oc_oauth2_access_tokens, oc_oauth2_refresh_tokens and oc_oauth2_clients to a Nextcloud instance.
  4. Reset all oauth2 migrations on Nextcloud.
  5. Trigger all oauth2 migrations and repair the instance: occ migrations:migrate oauth2 && occ maintenance:repair
  6. Shut down the ownCloud desktop client.
  7. Point the domain of ownCloud to the Nextcloud instance.
  8. Start the ownCloud desktop client.

Case 1 (best): Refresh token is retained (default)

Observe that the client will continue to work seamlessly. No prompt to login again.

Case 2 (worst): Refresh token is lost

(This case can be forced manually by removing relevant access token from the oc_oauth2_access_tokens table. Alternatively, you can just log out and in again in the ownCloud desktop client manually.)

  1. Follow the login flow to re-authenticate the ownCloud desktop client.

Observe that the the flow succeeds and the client is able to sync again.

TODO

  • Extract $enableOcClientSupport into a system/app config (currently hard-coded)
  • Think about importing legacy clients on instances which were already migrated (occ command makes sense) -> they were deleted by default during the migration
  • Add hint to occ command that it is only required on servers that were migrated without the patch and the system config

Checklist

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.

Makes sense! I did not test the full migration

Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

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

LGTM.

Just to be sure i get it:
occ oauth2:import-legacy-oc-client CLIENT_ID CLIENT_SECRET creates a client. We know we can do this after the Owncloud repair step has been executed because it has deleted the clients that have oc://* as redirect uri. Right?

I guess my question is: Is it necessary to run the occ oauth2:import-legacy-oc-client in all scenarios or only in specific ones? In which case it would be good to add details in the command description.

@st3iny
Copy link
Member Author

st3iny commented Feb 20, 2025

I guess my question is: Is it necessary to run the occ oauth2:import-legacy-oc-client in all scenarios or only in specific ones? In which case it would be good to add details in the command description.

Only if the server has been migrated before this patch was applied or if the system config was not enabled before the migration. In those scenarios, the client was deleted from the table during the migration and has to be added manually via the new occ command. Otherwise, if the config was set and the patch was applied before the migration, the client will be retained and the command is not required to be run.

Good point, I'll add this as a hint to the command's description.

@susnux susnux added this to the Nextcloud 32 milestone Mar 2, 2025
@st3iny st3iny force-pushed the fix/oauth2/retain-legacy-oc-client-support branch from 17517ea to 5b1fc2c Compare April 1, 2025 09:17
Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
@st3iny st3iny force-pushed the fix/oauth2/retain-legacy-oc-client-support branch from 5b1fc2c to 246da73 Compare April 1, 2025 09:37
@st3iny st3iny added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 1, 2025
@st3iny st3iny marked this pull request as ready for review April 1, 2025 09:37
@st3iny st3iny requested review from a team and provokateurin as code owners April 1, 2025 09:37
@st3iny st3iny requested review from artonge and yemkareems and removed request for a team April 1, 2025 09:37
@st3iny st3iny added 4. to release Ready to be released and/or waiting for tests to finish bug and removed 3. to review Waiting for reviews labels Apr 1, 2025
@st3iny st3iny merged commit 15d04b1 into master Apr 1, 2025
203 of 216 checks passed
@st3iny st3iny deleted the fix/oauth2/retain-legacy-oc-client-support branch April 1, 2025 14:36
@github-project-automation github-project-automation bot moved this from 🏗️ In progress to ☑️ Done in 💌 📅 👥 Groupware team Apr 1, 2025
@st3iny
Copy link
Member Author

st3iny commented Apr 1, 2025

/backport to stable29

@st3iny
Copy link
Member Author

st3iny commented Apr 1, 2025

/backport to stable30

@st3iny
Copy link
Member Author

st3iny commented Apr 1, 2025

/backport to stable31

@backportbot
Copy link

backportbot bot commented Apr 1, 2025

The backport to stable29 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable29
git pull origin stable29

# Create the new backport branch
git checkout -b backport/50858/stable29

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 246da73a

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/50858/stable29

Error: Failed to check for changes with origin/stable29: No changes found in backport branch


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@backportbot
Copy link

backportbot bot commented Apr 1, 2025

The backport to stable30 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable30
git pull origin stable30

# Create the new backport branch
git checkout -b backport/50858/stable30

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 246da73a

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/50858/stable30

Error: Failed to check for changes with origin/stable30: No changes found in backport branch


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@backportbot
Copy link

backportbot bot commented Apr 1, 2025

The backport to stable31 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable31
git pull origin stable31

# Create the new backport branch
git checkout -b backport/50858/stable31

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 246da73a

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/50858/stable31

Error: Failed to check for changes with origin/stable31: No changes found in backport branch


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

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

Projects

Status: ☑️ Done

Development

Successfully merging this pull request may close these issues.

7 participants