Skip to content
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

Command-line client. Do not trust SSL certificates by default, unless '--trust' option is set. #5022

Merged
merged 2 commits into from
Oct 11, 2022
Merged

Command-line client. Do not trust SSL certificates by default, unless '--trust' option is set. #5022

merged 2 commits into from
Oct 11, 2022

Conversation

allexzander
Copy link
Contributor

@allexzander allexzander commented Oct 7, 2022 •
edited
Loading

Signed-off-by: allexzander blackslayer4@gmail.com

@allexzander allexzander changed the title Command-line client. Do not trust SSL certificates by default, unlss … Command-line client. Do not trust SSL certificates by default, unless '--trust' option is set. Oct 7, 2022
@allexzander allexzander force-pushed the bugfix/cmdclient-do-not-trust-certs-by-default branch from f453542 to da89694 Compare October 7, 2022 15:13
github-actions[bot]
github-actions bot requested changes Oct 7, 2022
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ clang-tidy found issue(s) with the introduced code (1/1)

src/libsync/account.h Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Oct 7, 2022 •
edited
Loading

Codecov Report

Merging #5022 (e721f93) into master (256aa52) will decrease coverage by 0.12%.
The diff coverage is 14.28%.

❗ Current head e721f93 differs from pull request most recent head 564a3ad. Consider uploading reports for the commit 564a3ad to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #5022      +/-   ##
==========================================
- Coverage   57.28%   57.16%   -0.13%     
==========================================
  Files         138      138              
  Lines       17394    17401       +7     
==========================================
- Hits         9965     9947      -18     
- Misses       7429     7454      +25
Impacted Files Coverage Δ
src/libsync/account.cpp 38.00% <0.00%> (-0.47%) ⬇️
src/libsync/account.h 33.33% <100.00%> (+3.92%) ⬆️
src/libsync/vfs/cfapi/hydrationjob.cpp 52.38% <0.00%> (-3.71%) ⬇️
src/libsync/filesystem.cpp 73.11% <0.00%> (-3.23%) ⬇️
src/libsync/vfs/cfapi/vfs_cfapi.cpp 83.39% <0.00%> (-2.38%) ⬇️
src/libsync/vfs/cfapi/cfapiwrapper.cpp 72.50% <0.00%> (-1.82%) ⬇️
src/libsync/syncengine.cpp 82.88% <0.00%> (-0.46%) ⬇️
src/libsync/propagatedownload.cpp 64.61% <0.00%> (+1.17%) ⬆️

@allexzander allexzander force-pushed the bugfix/cmdclient-do-not-trust-certs-by-default branch from da89694 to e721f93 Compare October 7, 2022 16:13
@mgallien mgallien requested review from github-actions[bot] and removed request for github-actions[bot] October 10, 2022 06:51
claucambra
claucambra approved these changes Oct 10, 2022
View reviewed changes
Copy link
Collaborator

@claucambra claucambra left a comment

Choose a reason for hiding this comment

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

Just a tiny nitpick about method name but good besides that

src/libsync/account.cpp Outdated Show resolved Hide resolved
src/libsync/account.h Outdated Show resolved Hide resolved
src/cmd/simplesslerrorhandler.cpp Outdated Show resolved Hide resolved
@allexzander allexzander force-pushed the bugfix/cmdclient-do-not-trust-certs-by-default branch from e721f93 to afa55e0 Compare October 10, 2022 13:54
@allexzander
Copy link
Contributor Author

@claucambra I had to fix a small logic error and re-request a review. It does not really affect much but it's best to keep it right.

@allexzander allexzander force-pushed the bugfix/cmdclient-do-not-trust-certs-by-default branch from 58e4efd to af05de6 Compare October 11, 2022 06:43
@allexzander
Command-line client. Do not trust SSL certificates by default, unlss …
4930531 
…'--trust' option is set.

Signed-off-by: allexzander <blackslayer4@gmail.com>
@allexzander
Fix error in logic.
564a3ad 
Signed-off-by: alex-z <blackslayer4@gmail.com>
@allexzander allexzander force-pushed the bugfix/cmdclient-do-not-trust-certs-by-default branch from af05de6 to 564a3ad Compare October 11, 2022 06:43
@nextcloud-desktop-bot
Copy link

AppImage file: nextcloud-PR-5022-564a3ad987a40135c9d6dedd5a1238caf0ce2d52-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@sonarcloud
Copy link

sonarcloud bot commented Oct 11, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@allexzander allexzander merged commit 42f6a63 into master Oct 11, 2022
@allexzander allexzander deleted the bugfix/cmdclient-do-not-trust-certs-by-default branch October 11, 2022 11:15
@allexzander
Copy link
Contributor Author

/backport to stable-3.6

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Oct 11, 2022
Merged
@mgallien mgallien added this to the 3.7.0 milestone Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] requested changes

@claucambra claucambra claucambra approved these changes

@mgallien mgallien Awaiting requested review from mgallien

@camilasan camilasan Awaiting requested review from camilasan

Assignees
No one assigned
Labels
backport-request
Projects
None yet
Milestone
3.7.0
Development

Successfully merging this pull request may close these issues.
4 participants
@allexzander @nextcloud-desktop-bot @claucambra @mgallien