-
Notifications
You must be signed in to change notification settings - Fork 1
Add some ssl support #1
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
base: master
Are you sure you want to change the base?
Conversation
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.
@phpmaps Sorry for sooo long response, for some reason I didn't get any notifications about your participation. If this PR is still valid for you I like your idea, just couple things should be changed and we are good to go.
| .option('ssl', { | ||
| alias: 's', | ||
| describe: 'Suppress ssl issues', | ||
| type: 'boolean', | ||
| default: false | ||
| }) |
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.
let's name this option strict-ssl-verification and it's default should be true
| const httpsAgent = new https.Agent({ rejectUnauthorized: false }); | ||
| const opts = argv.ssl ? { body: fileContents, agent: httpsAgent } : { body: fileContents }; |
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.
| const httpsAgent = new https.Agent({ rejectUnauthorized: false }); | |
| const opts = argv.ssl ? { body: fileContents, agent: httpsAgent } : { body: fileContents }; | |
| const agent = new https.Agent({ rejectUnauthorized: !argv.strictSslVerification }); | |
| const opts = Object.assign({body: fileContents}, argv.strictSslVerification ? {} : { agent }); |
| "inquirer": "^5.1.0", | ||
| "https": "^1.0.0" |
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.
since we are in nodejs environment, we don't need this dependency
| "inquirer": "^5.1.0", | |
| "https": "^1.0.0" | |
| "inquirer": "^5.1.0" |
This PR adds some SSL support to make the CLI a little more secure :)
With Basic Authentication set on an HTTPS WebDav server like:
https://bla:bla@webdav.geo.com/bucketthe following exception would be raised by thenode-fetchmodule during file transfer.Error
This PR includes new proposed usage:
Commands
webdav-watch watch --folder ./music --s false --remote https://bla:bla@webdav.geo.com/bucketOR
webdav-watch watch --folder ./music --ssl false --remote https://bla:bla@webdav.geo.com/bucketAND
webdav-watch watch --folder ./music --s true --remote https://bla:bla@webdav.geo.com/bucketOR
webdav-watch watch --folder ./music --ssl true --remote https://bla:bla@webdav.geo.com/bucketCommand using config
Example Configs