qzind / qz-print

Archive for legacy qz-print versions (1.8, 1.9). See https://github.com/qzind/tray for modern versions.
Other
141 stars 101 forks source link

Allows wss connection from a non localhost connection #117

Closed klabarge closed 8 years ago

tresf commented 8 years ago

Please squash your commits down to a single commit.

dsanders11 commented 8 years ago

@tresf: Just a heads up, I personally would prefer the binding to be configurable rather than hardcoded to "0.0.0.0". Rationale is that "0.0.0.0" binds to all interfaces on a machine that may have more than one network card. For security reasons it is preferable to allow explicitly locking this to a single interface, even if the default is "0.0.0.0". On a machine with one network interface on an internal network and one exposed to a public interface (like the internet) you'd want to limit the binding to the internal network only.

Configurable also lets it be set to "localhost" to get the original behavior of only allowing localhost connections, which some users may find preferable.

tresf commented 8 years ago

@dsanders11 duly noted. We'll keep 0.0.0.0 by default and allow it to be configurable via the config.

@klabarge if you can move this to the properties file, that would be better.

Relevant lines: (@dsanders11 has already done the necessary work in his original commit)

https://github.com/qzind/qz-print/commit/fc3351241f809284eea70bb25d95552e5645efe2#diff-255661b8647b8b514d9dc2e868744b48R234

https://github.com/qzind/qz-print/commit/fc3351241f809284eea70bb25d95552e5645efe2#diff-f13e5cc159943bec55015c1b9bea4472R85

And then it will need to be added to Linux and Mac installers as well per:

https://github.com/qzind/qz-print/blob/1.9/ant/linux/linux-keygen.sh.in#L79 https://github.com/qzind/qz-print/blob/1.9/ant/apple/apple-keygen.sh.in#L131

We'll still hardcode 0.0.0.0, but will allow the value to be configurable for advanced users per @dsanders11's points.

tresf commented 8 years ago

@klabarge we can also add this documentation about the hostname to the non-localhost binding stuff (the new Print Server page).

tresf commented 8 years ago

@klabarge I won't merge this PR until this has been made configurable. Let me know if you need any help w/ the code.

klabarge commented 8 years ago

Moved to #123