webui: Implement support for reverse proxies #2343

psyke83 · 2024-04-01T03:06:50Z

Description

Add "web_ui_address" option to UI Network tab.
Add tray icon link handling, with proxy connectivity check to decide which link to open.
Add reverse proxy suggestion to Setup document as a hint to circumvent self-signed SSL warnings.

Screenshot

Issues Fixed or Closed

Resolves #1034

N.B.: this PR doesn't implement the stated proposal in title of #1034 (insecure http support in Sunshine), but instead enhances the url handler to use a proxied address if set by the user, and add some basic documentation on reverse proxying. A reverse proxy should typically proxy the secure https://<host>:47990 address, and self-signed certs shouldn't cause issues (if using Nginx Proxy Manager, at the very least).

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Dependency update (updates to dependencies)
Documentation update (changes to documentation)
Repository update (changes to repository files, e.g. .github/...)

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added or updated the in code docstring/documentation-blocks for new or existing methods/components

Branch Updates

LizardByte requires that branches be up-to-date before merging. This means that after any PR is merged, this branch
must be updated before it can be merged. You must also
Allow edits from maintainers.

I want maintainers to keep my branch updated

codecov · 2024-04-01T03:21:26Z

Codecov Report

Attention: Patch coverage is 3.84615% with 25 lines in your changes are missing coverage. Please review.

Project coverage is 6.50%. Comparing base (2da6fb0) to head (4c6f5b8).

Additional details and impacted files

@@            Coverage Diff             @@
##           nightly   #2343      +/-   ##
==========================================
- Coverage     6.50%   6.50%   -0.01%     
==========================================
  Files           85      85              
  Lines        18382   18406      +24     
  Branches      8348    8355       +7     
==========================================
+ Hits          1196    1197       +1     
+ Misses       16136   16081      -55     
- Partials      1050    1128      +78

Flag	Coverage Δ
Linux	`4.41% <0.00%> (-0.01%)`	⬇️
Windows	`2.00% <0.00%> (-0.01%)`	⬇️
macOS-12	`8.72% <4.16%> (-0.02%)`	⬇️
macOS-13	`8.06% <4.00%> (-0.01%)`	⬇️
macOS-14	`8.38% <4.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
src/config.h	`0.00% <ø> (ø)`
src/config.cpp	`5.33% <50.00%> (-0.02%)`	⬇️
src/entry_handler.cpp	`5.36% <0.00%> (+0.54%)`	⬆️
src/httpcommon.cpp	`4.06% <0.00%> (-0.48%)`	⬇️

... and 15 files with indirect coverage changes

docs/source/about/advanced_usage.rst

ReenigneArcher · 2024-04-01T13:04:38Z

src/confighttp.cpp

+          if (!config::nvhttp.web_ui_address.empty()) {
+            BOOST_LOG(info) << "Configuration UI available at ["sv << config::nvhttp.web_ui_address << "]"sv;
+          }
+          BOOST_LOG(info) << "Configuration UI available at [https://localhost:"sv << port << "]"sv;


Maybe make an array of the addresses it's available at, and only have one log message?

Related to this, I have some reservations about the PR related to privacy: users may not want their reverse proxy address to be exposed when sending logs.

In my case, I use a reverse proxy for all my home services with the intention not to expose anything outside of my LAN, but I'm forced to use a DNS AAAA record, as my ISP uses DNS spoofing to block A records that refer to a private network space. I use the reverse proxy for multiple services, but none are exposed to the external network, and I'd prefer not to be leaking the address online unnecessarily.

Good point... I think it's safe to only log the localhost address.

Backed out the logging, but keep in mind that the proxy address will be exposed by open_url() & resolve_command_str() invocations.

ReenigneArcher · 2024-04-01T13:06:58Z

src/entry_handler.cpp

+
+  if (!config::nvhttp.web_ui_address.empty()) {
+    // Don't append internal TCP port as port is mapped by reverse proxy
+    url = config::nvhttp.web_ui_address;


Would we want to use the reverse proxy address from the server itself?

You mean via the link handler as used by the tray icon on the host? At least in my case, yes. The main benefit of using the reverse proxy for Sunshine in particular is so that I can use my own valid SSL cert that's managed on a different machine than the Sunshine host. Having the tray icon open the self-signed site means I need to deal with the SSL warnings each time, which cancels out most of the benefit of using the proxy.

Do you think this should be optional? In the event of an internet outage, people won't be able to open ui via the tray icon.

Or fallback to the localhost address if the proxy address is unreachable?

I've implemented the latter suggestion as it seems to make the best sense and keep things simpler.

src_assets/common/assets/web/public/assets/locale/en.json

psyke83 · 2024-04-03T04:00:58Z

src/httpcommon.cpp

+      curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0);  // ignore self-signed cert
+      curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 0);  // ignore self-signed cert


These lines will let the function detect a proxied or other address that is still using a self-signed cert. For example, you can test functionality without a reverse proxy by setting the url to https://sunshinehost:47990.

It might be OK to leave this here, but if we want to enforce valid SSL checking, I can remove the lines and perhaps add a note that a valid SSL cert will be required to the docs.

psyke83 · 2024-04-03T04:42:33Z

src_assets/common/assets/web/config.html

+        <!-- Web UI Address -->
+        <div class="mb-3">
+          <label for="web_ui_address" class="form-label">{{ $t('config.web_ui_address') }}</label>
+          <input type="text" class="form-control" id="web_ui_address" placeholder="https://localhost:47990" v-model="config.web_ui_address" />


Should I change the placeholder to a reverse proxy example? If yes, is https://example.com:80/sunshine appropriate here (and elsewhere)? I'm asking because the External IP entry is using an example IP range, so I assume users won't get confused that the placeholder is only an example and not the actual default when unset.

* Add "web_ui_address" option to UI Network tab. * Add tray icon link handling, with proxy connectivity check to decide which link to open. * Add reverse proxy suggestion to Setup document as a hint to circumvent self-signed SSL warnings.

psyke83 force-pushed the reverse_proxy_support branch from 67c5a54 to c6d52da Compare April 1, 2024 03:08

psyke83 force-pushed the reverse_proxy_support branch 2 times, most recently from 7e2b222 to eaca72d Compare April 1, 2024 04:15

ReenigneArcher reviewed Apr 1, 2024

View reviewed changes

ReenigneArcher added the pr:planned label Apr 1, 2024

psyke83 force-pushed the reverse_proxy_support branch 3 times, most recently from 2a70c10 to bce31e3 Compare April 3, 2024 03:49

psyke83 commented Apr 3, 2024

View reviewed changes

psyke83 force-pushed the reverse_proxy_support branch 2 times, most recently from 23758c0 to 3a49fef Compare April 3, 2024 04:32

psyke83 commented Apr 3, 2024

View reviewed changes

webui: Implement support for reverse proxies

4c6f5b8

* Add "web_ui_address" option to UI Network tab. * Add tray icon link handling, with proxy connectivity check to decide which link to open. * Add reverse proxy suggestion to Setup document as a hint to circumvent self-signed SSL warnings.

psyke83 force-pushed the reverse_proxy_support branch from 3a49fef to 4c6f5b8 Compare April 3, 2024 04:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

webui: Implement support for reverse proxies #2343

webui: Implement support for reverse proxies #2343

psyke83 commented Apr 1, 2024 •

edited

codecov bot commented Apr 1, 2024 •

edited

ReenigneArcher Apr 1, 2024

psyke83 Apr 2, 2024

ReenigneArcher Apr 2, 2024

psyke83 Apr 3, 2024

ReenigneArcher Apr 1, 2024

psyke83 Apr 2, 2024 •

edited

ReenigneArcher Apr 2, 2024

psyke83 Apr 3, 2024

psyke83 Apr 3, 2024

psyke83 Apr 3, 2024

		curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0); // ignore self-signed cert
		curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 0); // ignore self-signed cert

webui: Implement support for reverse proxies #2343

Are you sure you want to change the base?

webui: Implement support for reverse proxies #2343

Conversation

psyke83 commented Apr 1, 2024 • edited

Description

Screenshot

Issues Fixed or Closed

Type of Change

Checklist

Branch Updates

codecov bot commented Apr 1, 2024 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

psyke83 Apr 2, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

psyke83 commented Apr 1, 2024 •

edited

codecov bot commented Apr 1, 2024 •

edited

psyke83 Apr 2, 2024 •

edited