-
Notifications
You must be signed in to change notification settings - Fork 41
Feature: Add section about Cryptomator desktop network settings #66
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
Conversation
infeo
commented
Jan 28, 2025
•
edited
Loading
edited
WalkthroughThe pull request introduces comprehensive documentation updates for Cryptomator, focusing on network-related configurations and the introduction of Cryptomator Hub. A new section titled "Network Settings" is added to the Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
source/desktop/network.rst (3)
8-8
: Consider adding security context for TLS version.The TLS requirement is correctly specified. Consider adding a brief note explaining why TLS 1.2 is the minimum requirement (e.g., "TLS 1.2 is required as earlier versions are considered insecure").
50-55
: Enhance proxy configuration documentation.Consider these improvements:
- Add example values (e.g.,
[1]
could beproxy.example.com
)- Add a warning about security implications of proxy configuration
- Recommend backing up
Cryptomator.cfg
before editingExample enhancement:
java-options=-Dhttp.proxyHost=[1] java-options=-Dhttp.proxyPort=[2] java-options=-Dhttps.proxyHost=[1] java-options=-Dhttps.proxyPort=[2] java-options=-Dhttp.nonProxyHosts=localhost|127.0.0.1|cryptomator-vault|[3] + +Example: +java-options=-Dhttp.proxyHost=proxy.example.com +java-options=-Dhttp.proxyPort=8080 +java-options=-Dhttps.proxyHost=proxy.example.com +java-options=-Dhttps.proxyPort=8080 +java-options=-Dhttp.nonProxyHosts=localhost|127.0.0.1|cryptomator-vault|internal.example.com + +Note: Before editing the configuration file, create a backup copy. Incorrect proxy settings +might prevent Cryptomator from accessing the network.
56-56
: Consider adding a troubleshooting section.It would be helpful to add a section about common network-related issues and their solutions, such as:
- Connection timeout errors
- Certificate validation failures
- Proxy authentication issues
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
source/desktop/network.rst
(1 hunks)source/hub/introduction.rst
(1 hunks)source/index.rst
(1 hunks)
🔇 Additional comments (3)
source/hub/introduction.rst (1)
8-9
: LGTM! Well-structured anchor reference.The anchor reference is correctly formatted and placed, enabling proper cross-referencing throughout the documentation.
source/index.rst (1)
32-32
: LGTM! Proper toctree entry.The new network settings documentation is correctly integrated into the Desktop section's table of contents.
source/desktop/network.rst (1)
17-18
: Clarify internal CA list details.Consider specifying:
- Which common CAs are included in the internal list
- How users can view this list
- How the list is updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just a few minor things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
source/desktop/network.rst (4)
10-10
: Add details about TLS version support.Consider adding information about the maximum supported TLS version and any deprecated versions for security-conscious users.
-All network connections to the internet are using HTTPS with at least TLS 1.2. +All network connections to the internet are using HTTPS with TLS 1.2 or higher (up to TLS 1.3).
8-10
: Clarify localhost connection behavior.Based on previous discussions, it would be helpful to explicitly mention the behavior with localhost connections, especially for developers or users setting up local instances.
If the network connection is present, it is used for optional features, i.e. update checks and searching the error database for solutions. The only exception is when unlocking :ref:`Cryptomator Hub <hub/introduction>` vaults, then a network connection to the hub server is required. -All network connections to the internet are using HTTPS with at least TLS 1.2. +All network connections to the internet are using HTTPS with at least TLS 1.2. Note that localhost connections (127.0.0.1) use HTTP, as HTTPS is not required for local connections.
33-33
: Enhance the JEP reference with context.The JEP 319 reference could be more helpful with a brief description of its relevance.
-.. [1] For more information about the location and contained certificates, see `JEP 319 <https://openjdk.org/jeps/319>`_. +.. [1] For more information about the default trust store location and included root certificates in OpenJDK, see `JEP 319: Root Certificates <https://openjdk.org/jeps/319>`_.
62-66
: Improve placeholder documentation and add validation steps.The placeholders could be more descriptive, and it would be helpful to include steps for validating the proxy configuration.
- java-options=-Dhttp.proxyHost=[1] - java-options=-Dhttp.proxyPort=[2] - java-options=-Dhttps.proxyHost=[1] - java-options=-Dhttps.proxyPort=[2] - java-options=-Dhttp.nonProxyHosts=localhost|127.0.0.1|cryptomator-vault|[3] + java-options=-Dhttp.proxyHost=<proxy-host> # Example: proxy.example.com + java-options=-Dhttp.proxyPort=<proxy-port> # Example: 8080 + java-options=-Dhttps.proxyHost=<proxy-host> # Same as http.proxyHost + java-options=-Dhttps.proxyPort=<proxy-port> # Same as http.proxyPort + java-options=-Dhttp.nonProxyHosts=localhost|127.0.0.1|cryptomator-vault|<additional-hosts> # Example: *.internal.netAfter configuration, restart Cryptomator and verify the proxy settings by checking the connection to the update server or Hub.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
source/desktop/network.rst
(1 hunks)
🔇 Additional comments (1)
source/desktop/network.rst (1)
22-23
: Add information about default trust store certificates.As mentioned in past reviews, it would be helpful to explain the source of certificates in the default trust store.
| Linux | | PKCS#12 file ``/etc/cryptomator/certs.p12``; If the file does not exist, the JDK default | -| | | trust store is used. [1]_ | +| | | trust store is used, which contains a curated list of common Certificate Authorities. [1]_ |