Repair account creation via CLI - #10726
Open
i2h3 wants to merge 1 commit into
Open
Conversation
Setting up an account with --userid/--apppassword/--serverurl never succeeded on macOS. Four independent defects were involved: - Options were only accepted as "--option value". Argument lists are now normalised with Utility::expandCommandLineOptionValues(), so the "--option=value" spelling works for every option in both the client and nextcloudcmd. The latter has to normalise before it checks for --userid, which is what selects provisioning mode. - --localdirpath is documented as optional but an empty one was rejected outright, although the setup already had a branch for accounts without a folder. It now falls back to the folder the account wizard would suggest. - FolderMan::addFolder() refuses to create a classic sync folder while the app-level File Provider mode is enabled, yet the setup always tried to create one, so it always failed. localSyncFolderRequired() now mirrors that guard and the account is set up without a folder in that mode. - 09f4692 added the account and its sync folder before the credentials were checked. Setting up the folder ends by scheduling qApp->quit(), so the credential check, the account save and the keychain write never ran. The account is now only added once the credentials validate, which also means a failed setup no longer leaves a half-written account behind. Two related fixes: a failed folder setup called deleteAccount(), revoking the app password that was passed on the command line, and the client overwrote the configured VFS setting on every start because the account setup parser instance is never null. nextcloudcmd now runs the event loop so the asynchronous setup can finish. Its keychain jobs never signal completion, which the previous immediate return hid; the wait for the credentials is bounded so provisioning ends with a warning instead of hanging. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
i2h3
requested review from
camilasan,
claucambra,
mgallien and
nilsding
as code owners
September 2, 2026 17:14
github-project-automation
Bot
moved this to 🧭 Planning evaluation (don't pick)
in 💻 Desktop Clients team
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Provisioning still has malformed-option, File Provider initialization, credential persistence, and test-coverage issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Repairs command-line account provisioning and asynchronous credential persistence.
Changes:
- Supports
--option=valuesyntax. - Defers account creation until authentication succeeds.
- Handles default folders, File Provider mode, cleanup, and keychain timeouts.
File summaries
| File | Description |
|---|---|
src/gui/application.cpp |
Normalizes options and protects VFS settings. |
src/gui/accountsetupfromcommandlinejob.h |
Declares folder-selection helpers. |
src/gui/accountsetupfromcommandlinejob.cpp |
Reworks provisioning lifecycle and persistence. |
src/common/utility.h |
Declares argument normalization utility. |
src/common/utility.cpp |
Implements inline-option expansion. |
src/cmd/cmd.cpp |
Adds normalization and asynchronous event loop. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 6
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| auto args(app_args); | ||
| // Accept both "--option value" and "--option=value" for every option below. | ||
| auto args = Utility::expandCommandLineOptionValues(app_args); |
Comment on lines
+57
to
+61
| #ifdef BUILD_FILE_PROVIDER_MODULE | ||
| // Mirrors the guard in FolderMan::addFolder(): while the app-level File Provider mode | ||
| // is enabled, a classic sync folder cannot be created at all. The account is synced | ||
| // through the File Provider domain that is set up for it instead. | ||
| return !ConfigFile().macFileProviderModeEnabled(); |
|
|
||
| credentialsPersistTimer->start(credentialsPersistTimeout); | ||
|
|
||
| accountManager->save(); |
| return noLeadingSlashPath(noTrailingSlashPath(relativePathToRemoteSyncRoot)); | ||
| } | ||
|
|
||
| QStringList Utility::expandCommandLineOptionValues(const QStringList &arguments) |
| #include "theme.h" | ||
|
|
||
| #include <chrono> | ||
| #include <iostream> |
| finishAccountSetup(); | ||
| }); | ||
|
|
||
| credentialsPersistTimer->start(credentialsPersistTimeout); |
Contributor
|
Artifact containing the AppImage: nextcloud-appimage-pr-10726.zip Digest: To test this change/fix you can download the above artifact file, unzip it, and run it. Please make sure to quit your existing Nextcloud app and backup your data. |
|
i2h3
moved this from 🧭 Planning evaluation (don't pick)
to 📄 To do
in 💻 Desktop Clients team
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Nextcloud --userid=admin --apppassword=admin --serverurl=http://…printedUnrecognized option '--userid=admin'. Fixing that spelling only exposed the next failure, and so on — account setup from the command line could not succeed on macOS at all.Defects fixed
--option valuewas parsed, never--option=value--localdirpathdocumented optional, but an empty one was rejectedFolderMan::addFolder()refuses classic folders in File Provider mode; setup always created one09f46929e3added the account and folder before authenticating, and quit before the credential check, save and keychain write could runAlso: the failure path called
deleteAccount(), revoking the app password passed on the command line, andcfg.setVfsEnabled()ran on every start, clobbering the user's setting.Notes
nextcloudcmdneeds an event loop for the now fully asynchronous setup. Its QtKeychain jobs never emitfinished— pre-existing, hidden by the previous immediate return. The wait is bounded at 30 s so provisioning terminates with a warning rather than hanging; whether the signed release binary is affected is still open.Testing
Against a local server, with the built client:
--userid=…inline form accepted; account created,dav_userset, password in keychain--apppassword: account created--localdirpath: default folder derived and created--userid=: reportsuserid not specifiedtestnextcloudcmdprovisioningbehave as the test expects