Skip to content

Add command for setting proxy #2660

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
Merged
merged 1 commit into from
Mar 28, 2017
Merged

Add command for setting proxy #2660

merged 1 commit into from
Mar 28, 2017

Conversation

rosen-vladimirov
Copy link
Contributor

At the moment users can set proxy by manually editing the file <cli installation dir>/config/config.json.
However this has two issues:

  • the proxy does not support authentication
  • if you install newer version of CLI, the file is overwritten, so you have to set it again.

Fix both issues by:

  • creating command that sets the proxy.
  • (currently) for Windows introduce support for authenticated proxy - username and password are stored via Windows Credential Manager API functions.

Expected workflow:
$ tns <command that makes http request>
Your proxy requires a username and password. You can run
tns proxy set
In order to supply NativeScript with the credentials needed.

$ tns proxy set 127.0.0.1 8888 user pass
Sucessfully setup proxy.

$ tns proxy get
Hostname: 127.0.0.1
Port: 8888
Proxy is Enabled

$ tns proxy clear
Sucessfully cleared proxy.

$ tns proxy get
No proxy set

@rosen-vladimirov rosen-vladimirov added this to the 3.0.0-RC milestone Mar 27, 2017
@rosen-vladimirov rosen-vladimirov self-assigned this Mar 27, 2017
yyosifov
yyosifov reviewed Mar 27, 2017
View reviewed changes
docs/man_pages/general/autocomplete-disable.md
@@ -20,4 +20,7 @@ Command | Description
[error-reporting](error-reporting.html) | Configures anonymous error reporting for the NativeScript CLI.
[help](help.html) | Lists the available commands or shows information about the selected command.
[doctor](doctor.html) | Checks your system for configuration problems which might prevent the NativeScript CLI from working properly.
[proxy](proxy.html) | Displays proxy settings.
[proxy clear](proxy-clear.html) | Clears proxy settings.
[proxy set](proxy-set.html) | Sets proxy settings.
Copy link
Contributor

@yyosifov yyosifov Mar 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

the proxy get or proxy list command is not documented anywhere?

I guess it's the "tns proxy" but in this way, how will the user view the documentation of the proxy command?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there's no proxy list command - the default command is proxy get , so in case you write just tns proxy it will execute proxy get command.

yyosifov
yyosifov reviewed Mar 27, 2017
View reviewed changes
docs/man_pages/general/autocomplete-disable.md
@@ -20,4 +20,7 @@ Command | Description
[error-reporting](error-reporting.html) | Configures anonymous error reporting for the NativeScript CLI.
[help](help.html) | Lists the available commands or shows information about the selected command.
[doctor](doctor.html) | Checks your system for configuration problems which might prevent the NativeScript CLI from working properly.
[proxy](proxy.html) | Displays proxy settings.
[proxy clear](proxy-clear.html) | Clears proxy settings.
[proxy set](proxy-set.html) | Sets proxy settings.
Copy link
Contributor

Choose a reason for hiding this comment

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

How do I update a proxy to be enabled/disabled? Can I do it via command or by manually editing the config.json?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You either set it or clear it.

Plamen5kov
Plamen5kov reviewed Mar 27, 2017
View reviewed changes
lib/config.ts
@@ -6,7 +6,6 @@ export class Configuration extends ConfigBase implements IConfiguration { // Use
CI_LOGGER = false;
DEBUG = false;
TYPESCRIPT_COMPILER_OPTIONS = {};
USE_PROXY = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

do we consider having a way to keep this functionality of enable/disable proxy?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe a short article describing what is using a proxy good for and how to set it up, would be helpful for the users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The functionality is introduced as command, so there's no need to manually edit any file. When you use the command, it will create required file (not the config.json ), so there's no need for the user to do anything else - just use the commands.
Where do you think is the appropriate place for such an article?

Copy link
Contributor

@Plamen5kov Plamen5kov Mar 28, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Better ask @PetyaSotirova

Copy link
Contributor

@Plamen5kov Plamen5kov Mar 28, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The functionality is introduced as command, so there's no need to manually edit any file.

I ment will we still have a way to enable/disable the proxy in any way through the newly created command or the only way would be to clear it and loosing all previous settings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no way to disable it - set it or clear it. The problem with disabling is that the credentials will be kept when they are not used, which is somehow a security risk. That's why we've decided to skip the disabling - set or clear only.

@rosen-vladimirov
Add command for setting proxy
ae061dd 
At the moment users can set proxy by manually editing the file `<cli installation dir>/config/config.json`.
However this has two issues:
 - the proxy does not support authentication
 - if you install newer version of CLI, the file is overwritten, so you have to set it again.

Fix both issues by:
 - creating command that sets the proxy.
 - (currently) for Windows introduce support for authenticated proxy - username and password are stored via Windows Credential Manager API functions.

Expected workflow:
`$ tns <command that makes http request>`
Your proxy requires a username and password. You can run
        tns proxy set <hostname> <port> <username> <password>
In order to supply NativeScript with the credentials needed.

`$ tns proxy set 127.0.0.1 8888 user pass`
Sucessfully setup proxy.

`$ tns proxy get`
Hostname: 127.0.0.1
Port: 8888
Proxy is Enabled

`$ tns proxy clear`
Sucessfully cleared proxy.

`$ tns proxy get`
No proxy set
@rosen-vladimirov rosen-vladimirov merged commit 3ca6474 into master Mar 28, 2017
@rosen-vladimirov rosen-vladimirov deleted the vladimirov/auth-proxy branch March 28, 2017 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yyosifov yyosifov yyosifov left review comments

@Plamen5kov Plamen5kov Plamen5kov left review comments

@TsvetanMilanov TsvetanMilanov TsvetanMilanov approved these changes

Assignees

@rosen-vladimirov rosen-vladimirov

Labels
feature os: android
Projects
None yet
Milestone
3.0.0-RC
Development

Successfully merging this pull request may close these issues.
4 participants
@rosen-vladimirov @yyosifov @Plamen5kov @TsvetanMilanov