Transport Tester GUI App: add support for generic transport, send report, UI polish #170

amircybersec · 2024-01-22T07:00:29Z

This PR replaces PR #137; since I had to rebase and refactor the PR #137 that it got a bit out of hand and decided to cherry pick commits into a clean branch off main. Here's is the summary of changes made to the Transport Tester GUI App:

CHANGELOG:

Add a new optional report collector URL field to the GUI (shared front-end)
Submit reports to a remote destination using report package (shared backend). One simple approach to set up a remote collector using Google Spreadsheet is discussed here.
Add support for existing SDK transports and wrapping (e.g. split, socks5, tls, ss, etc)
Redact UserInfo from transports in reports, e.g.: ss://REDACTED@104.238.100.16:65496
Include transport specific setup errors (e.g. split not supported over udp) in response error field
Display error message on the GUI (front-end)
Renamed app to Transport Tester
Add Powered By Outline logo
Add popover to show information for each field

This reverts commit e793764.

(cherry picked from commit 909904e)

(cherry picked from commit 2d7a657)

(cherry picked from commit 87da88a)

(cherry picked from commit 40f4ef1)

(cherry picked from commit 651c520)

(cherry picked from commit 5854d2c)

(cherry picked from commit 2feeaea)

(cherry picked from commit 87868ea)

(cherry picked from commit 79f7fb8)

(cherry picked from commit ed94c54)

(cherry picked from commit 4cc5676)

(cherry picked from commit 4e465d7)

amircybersec · 2024-01-22T22:04:50Z

@daniellacosse could you please review this PR? I applied the comments on the previous PR which is now closed over to this one; I had to refactor and rebase the previous PR and it got a bit messy and I decided to start off on a new branch and cherrypick the changes over to this one.

BTW, I tested the app on MacOS, iOS and Android and everything checked out in the overall functionality.

Popover on Android emulator breaks before WenView is upgraded which should not be a problem on physical devices that have already logged into Google Play services. I guess Google is going to include a more updated version of WebView as default soon which automatically fixes this issue on Android emulator.

daniellacosse

we should be using slots instead of unsafeHTML https://lit.dev/docs/frameworks/react/#using-slots

daniellacosse · 2024-01-23T15:12:46Z

x/examples/outline-connectivity-app/README.md

+
+One simple approach to set up a remote collector using Google Spreadsheet is discussed [here](https://github.com/amircybersec/report-collector).
+
+The following are app screenshots on Android and iOS. The app can also run on Linux, MacOs and Windows.


nit: MacOS

daniellacosse · 2024-01-23T15:14:41Z

x/examples/outline-connectivity-app/README.md

@@ -4,6 +4,22 @@

 This is a simple cross-platform app to test connectivity to Outline servers, using the Outline SDK. It is built with [Wails](https://wails.app/) and [Capacitor](https://capacitorjs.com/).

+## Usage
+
+This is an example application that accepts a transport configuration (for example `shadowsocks`) as defined [here](https://pkg.go.dev/github.com/Jigsaw-Code/outline-sdk/x/config) and performs DNS resolution over the specified transport using provided list of resolvers and domain name.


This line slightly conflicts with the line above. I might merge the below line with the above and rename this section.

daniellacosse · 2024-01-23T15:15:45Z

x/examples/outline-connectivity-app/shared_backend/main.go

 type ConnectivityTestError struct {
 	// TODO: add Shadowsocks/Transport error
-	Op string `json:"operation"`
+	Op string `json:"op,omitempty"`


I'm personally not a fan of abbreviations? Maybe we change Op to Operation instead

x/examples/outline-connectivity-app/shared_backend/main.go

daniellacosse · 2024-01-23T15:56:23Z

x/examples/outline-connectivity-app/shared_backend/main.go

+			}
+			retryCollector := &report.RetryCollector{
+				Collector:    remoteCollector,
+				MaxRetry:     3,


Can we pass in a config object with these values?

x/examples/outline-connectivity-app/shared_frontend/pages/connectivity_test/index.ts

daniellacosse · 2024-01-23T15:58:42Z

x/examples/outline-connectivity-app/shared_frontend/pages/connectivity_test/outline_logo.ts

@@ -0,0 +1,50 @@
+import { css, html, LitElement, nothing } from "lit";


I feel that the use of separate component here is a bit overkill. Why not <img src='./outline_logo.svg' /> and style that?

@daniellacosse that was the first approach I tried but I could not figure out a way to serve static content as the app could not find ./outline_logo.svg file location. Do you have any tips on serving static content in Index.ts?

It may require some doing. This is how to serve static assets in Wails: https://wails.io/docs/guides/dynamic-assets/ And I think Capacitor mounts a webdir: https://capacitorjs.com/docs/config

The reason I opted for creating a custom Lit element for the logo was to avoid framework specific setup. The current approach only works for SVG or text-based assets though. It seems like we don't need to do that for CSS assets here since they are encapsulated in custom Lite elements in the code.

For PNG/JPG/etc images, we need to offer a way to serve static assets though.

If you agree, I can add this as TODO item as technical debt / improvement to README and do it via a separate PR.

Yeah, okay with a TODO.

daniellacosse · 2024-01-23T16:00:00Z

x/examples/outline-connectivity-app/shared_frontend/pages/connectivity_test/popover_info.ts

+    <div @click="${this.showPopover}" popovertarget="my-popover" popovertargetaction="show">ℹ️</div>
+    <div popover="auto" class="popup" id="my-popover">
+      <span @click="${this.closePopover}" class="close-button" popovertargetaction="hide">&times;</span>
+      ${unsafeHTML(this.popupText)}


you should definitely be using slots instead of unsafeHTML https://lit.dev/docs/frameworks/react/#using-slots

I was able to use slots to show the popover message but I am not able to display HTML formatted text (HTML tags) to display correctly using slots. For example, currently I am using  and <a> url tags in one of the popovers:

<info-popup popupText="Options include ss://, split://, tls://, socks5://, and/or a combination of them. For examples and more information check out the documentation <a href='https://pkg.go.dev/github.com/Jigsaw-Code/outline-sdk/x/config'> here</a>."> </info-popup>

which looks like this:

If I use slot, the message would look like this:

<info-popup> Options include ss://, split://, tls://, socks5://, and/or a combination of them. For examples and more information check out the documentation <a href='https://pkg.go.dev/github.com/Jigsaw-Code/outline-sdk/x/config'> here</a>. </info-popup>

The main reason I used unsafeHTML was to be able to render HTML formatted text. Even though I am rendering static text and unsafeHTML does not pose a security issue, I agree it is not a clean solution here.

I can ditch HTML formatting altogether here but if you have any suggestions to support markup or HTML tags in the popover text, please do let me know.

Can you show me how you implemented the slot? Here's an example I did recently that works: Jigsaw-Code/outline-server#1482

The slot works but the HTML formatting of the text is ignored...

This is the change in popover_info.ts file:

render() { return html` <div @click="${this.showPopover}" popovertarget="my-popover" popovertargetaction="show">ℹ️</div> <div popover="auto" class="popup" id="my-popover"> × <slot></slot> </div> `; }

I think maybe the CSS reset is taking effect when you use the <slot></slot>, which is intentional. There's no telling how different browsers will style things, so to ensure consistency we strip the default styles and re-implement them.

daniellacosse · 2024-01-23T16:01:37Z

x/examples/outline-connectivity-app/shared_frontend/pages/connectivity_test/popover_info.ts

+
+  static styles = css`
+  .popup {
+    top: -50%; /* Position above the element */


note that I'm saving most of my HTML/CSS comments for last

amircybersec and others added 22 commits January 15, 2024 23:21

add dynamic config support

e793764

Merge branch 'Jigsaw-Code:main' into main

bbd9f82

Revert "add dynamic config support"

ba98871

This reverts commit e793764.

feature: add report field to the frontend

3810c98

(cherry picked from commit 909904e)

feat: collect reports in backend +retry/sampling

508bcdf

(cherry picked from commit 2d7a657)

UI: remove prefix, transport optional, add error

e913ea8

(cherry picked from commit 87da88a)

Backend: support for any transport, sanitize ss://

56c7945

(cherry picked from commit 40f4ef1)

collect report on on stdout

ad1b432

(cherry picked from commit 651c520)

add logo element

400cd46

(cherry picked from commit 5854d2c)

refactor and improve popover element

3a4465f

(cherry picked from commit 2feeaea)

add logo and popover to the app

2656406

(cherry picked from commit 87868ea)

made IsSuccess more readable

c663172

(cherry picked from commit 79f7fb8)

rename accessKey to transport

0617fa5

(cherry picked from commit ed94c54)

rename accessKey to transport

a70eb60

(cherry picked from commit 4cc5676)

refactor: new dns pkg & config sanitizer

226bb4d

(cherry picked from commit 4e465d7)

tidy packages

950e7af

don't show Err field if empty

28ba1cc

handle connectivity.ConnectivityError nil value

f3a273f

upgrade gradle version

dc7cf51

upgrade gradle

48b1ffd

revert accidental changes

dd28010

update readme

27a5bfe

amircybersec mentioned this pull request Jan 22, 2024

Add support for various transports, and mechanism to collect connectivity report #137

Closed

readme edits

07255d1

amircybersec marked this pull request as ready for review January 22, 2024 22:02

amircybersec requested a review from daniellacosse January 22, 2024 22:02

daniellacosse requested changes Jan 23, 2024

View reviewed changes

amircybersec added 2 commits February 14, 2024 15:32

use slot instead of unsafeHTML

a2d0c91

using slot instead of popover-text

aec542a

updated readme

8c7a12b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Transport Tester GUI App: add support for generic transport, send report, UI polish #170

Transport Tester GUI App: add support for generic transport, send report, UI polish #170

amircybersec commented Jan 22, 2024 •

edited

Loading

amircybersec commented Jan 22, 2024 •

edited

Loading

daniellacosse left a comment

daniellacosse Jan 23, 2024

daniellacosse Jan 23, 2024

daniellacosse Jan 23, 2024

daniellacosse Jan 23, 2024

daniellacosse Jan 23, 2024

amircybersec Jan 23, 2024

daniellacosse Jan 23, 2024

amircybersec Jan 25, 2024

daniellacosse Jan 25, 2024

daniellacosse Jan 23, 2024

amircybersec Jan 25, 2024

daniellacosse Jan 25, 2024

amircybersec Jan 25, 2024

daniellacosse Jan 25, 2024 •

edited

Loading

daniellacosse Jan 23, 2024 •

edited

Loading


		One simple approach to set up a remote collector using Google Spreadsheet is discussed [here](https://github.com/amircybersec/report-collector).

		The following are app screenshots on Android and iOS. The app can also run on Linux, MacOs and Windows.

		@@ -0,0 +1,50 @@
		import { css, html, LitElement, nothing } from "lit";

Transport Tester GUI App: add support for generic transport, send report, UI polish #170

Are you sure you want to change the base?

Transport Tester GUI App: add support for generic transport, send report, UI polish #170

Conversation

amircybersec commented Jan 22, 2024 • edited Loading

amircybersec commented Jan 22, 2024 • edited Loading

daniellacosse left a 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

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

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

daniellacosse Jan 25, 2024 • edited Loading

Choose a reason for hiding this comment

daniellacosse Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

amircybersec commented Jan 22, 2024 •

edited

Loading

amircybersec commented Jan 22, 2024 •

edited

Loading

daniellacosse Jan 25, 2024 •

edited

Loading

daniellacosse Jan 23, 2024 •

edited

Loading