lib/model, lib/config: Apply sensible defaults for auto-accepted encrypted folder (fixes #8296) #8427
Conversation
This file contains 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
Encrypted folders should not have the fs watcher enabled and rarely benefit from a scheduled rescan. The GUI adjusts the suggested settings (watcher disabled, one day rescan interval) when accepting a receive-encrypted folder. Mirror that behavior to the auto-accept case where the GUI is not involved.
Versioning does not work well for encrypted folders. The GUI adjusts the suggested settings accordingly (no versioning) when accepting a receive-encrypted folder. Add it to the hard requirements enforced in FolderConfiguration.prepare() in the backend as well, so it also applies to the auto-accept case where the GUI is not involved.
|
Lets maybe do the versioning in the model/auto-accept as well. While it's cumbersome and unlikely to be useful in the real world, I do think versioning is theoretically usable (one could manually get versions and decrypt them). If it were to be disabled in the folder config, we also need to disable it in the web UI (otherwise a user will set folder versioning, and we'll silently remove it -> surprises are always bad). |
|
Sure, that's a valid point. I suppose the same could be said about "Ignore Permissions", which will get disabled silently if set in the GUI for a receive-encrypted folder. Should we avoid that in addition? Did anyone actually test what happens with versioning enabled on a receive-encrypted folder? |
|
Ignore permission controls is disabled (as in you can't uncheck it) when type is receive-encrypted. And no I haven't tested that. |
imsodin
approved these changes
Jul 14, 2022
calmh
added a commit
to calmh/syncthing
that referenced
this pull request
Jul 26, 2022
* main: lib/model, lib/config: Apply sensible defaults for auto-accepted encrypted folder (fixes syncthing#8296) (syncthing#8427) gui: Move filesystem watcher explanation from tooltip to help block (syncthing#8432) gui: Use discovered IDs from cache when adding a new remote device (syncthing#8382) build: Update goleveldb (syncthing#8440)
calmh
added a commit
to calmh/syncthing
that referenced
this pull request
Jul 28, 2022
* main: cmd/syncthing, lib/config: Remove restartOnWakeup option & functionality (fixes syncthing#8448) (syncthing#8449) gui: Remove blank meta tags (syncthing#8362) gui: Add device sync status (fixes syncthing#7981) (syncthing#8401) gui: Fix detailed staggered versioning information in folder info (ref syncthing#8348) (syncthing#8433) all: Support syncing ownership (fixes syncthing#1329) (syncthing#8434) gui, man, authors: Update docs, translations, and contributors lib/model, lib/config: Apply sensible defaults for auto-accepted encrypted folder (fixes syncthing#8296) (syncthing#8427) gui: Move filesystem watcher explanation from tooltip to help block (syncthing#8432) gui: Use discovered IDs from cache when adding a new remote device (syncthing#8382) build: Update goleveldb (syncthing#8440) gui, man, authors: Update docs, translations, and contributors cmd/syncthing/cli: Add show discovery command (fixes syncthing#8007) (syncthing#8378) gui, man, authors: Update docs, translations, and contributors lib/osutil: Only announce address of interfaces which are up (fixes syncthing#7458) (syncthing#8422) gui: Fix missing span end tag and missing nbsp semicolon in HTML (syncthing#8419)
st-review
added
the
frozen-due-to-age
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Purpose
Encrypted folders should not have the fs watcher enabled and rarely benefit from a scheduled rescan. Versioning does not work well for them either. The GUI adjusts the suggested settings accordingly (watcher disabled, one day rescan interval, no versioning) when accepting a receive-encrypted folder. Mirror that behavior to the auto-accept case where the GUI is not involved.
For watcher and rescan, the defaults are overridden especially in
handleAutoAccepts(), while the versioning is filtered out completely in the backend'sFolderConfiguration.prepare().Fixes #8296.
Testing
Manual testing with auto-accept enabled or disabled from a certain remote device.
Documentation
Is it worth mentioning this automatic override in the docs? The only place I've found is at https://docs.syncthing.net/users/config.html#config-option-device.autoacceptfolders, which is pretty terse in general.