Improve the flows for manually added bridges and their validation

added 13.0 stable Backlog Desktop Feature UX Team Sponsor 96 UX labels

assigned to @donuts

marked this issue as related to #40552 (closed)

changed title from Update the placeholder text and formatting of the "Add a bridge manually" text field to Update the placeholder text and formatting of the "Add a bridge manually" text box

changed the description

unassigned @donuts

removed 13.0 stable label

added 13.5 stable label

added Roadmap::Future label and removed Backlog label

@donuts is this required for Sponsor 96

Nope, but I would like to fit it into 13.5 still if possible (pending new designs).

assigned to @donuts

removed Sponsor 96 label

marked this issue as related to #42036 (closed)

changed title from Update the placeholder text and formatting of the "Add a bridge manually" text box to Add validation and improve the formatting of manually added bridge lines

marked this issue as related to #42299 (closed)

mentioned in issue #41482

removed Roadmap::Future label

added Backlog label

assigned to @jag and unassigned @donuts

@jag There's only one task remaining here before we can hand this ticket off to dev for implementation, and that's to consider error validation.

Could you add a style based on Fx's design system to the designs, and work with the devs to figure out what the error messages would be in practice? I suspect we'll need separate error messages for normal vs lox bridges.

Looping in @gus, who also mentioned this feature to me a few weeks ago. I think it would be good to get his input on the designs before handoff.

@donuts, what is the expected behavior for "try again"? I wonder if we should instead suggest them to try a different bridge distributor, because "try again" in Moat will give the same bridges for x hours (I don't remember exactly the amount of time that you get the same bridges using the same IP).

@gus these are for bridge addresses that were directly added by the user. I guess the idea is to allow the user to edit the bridges and try and reconnect.

However, I'm not sure we'll be implementing the "try again" design in the mockups any time soon. Right now, the "Connect" button takes the user to the "about:torconnect" page. To have something like "try again" work, I think we would need to stay within the "about:preferences" and try the connection there, but that would take some redesign.

removed Desktop label

added All Platforms label

added Doing label and removed Backlog label

Thanks @donuts will do!

mentioned in issue #41187

@donuts just wanted to verify: does this validation include a mix of regular bridges and lox invites? that is, should i be accounting for these scenarios in the same input box?

Bridge line Lox invite

otherwise i can work on errors for bridge lines and lox invites separately.

Yep, exactly! Since lox and regular bridges can't be configured at the same time, we'll need to return an error message in that scenario too.

Something extra to check would be for duplicate fingerprints. See #40242 (closed)

Thank you @henry I'll take note of that.

@meskio, @cohosh we need your AC Team eyes here for bridge format validation as sometimes we ask users to copy and paste some uncommon lines, for example, Snowflake with different stun servers, Conjure, and other PTs.

I read the issue diagonaly but I'm not sure how I can help here. Is there a specific I can answer? Or you are just poking so I keep an eye oh what is happening here so I'm aware?

Anyway, I'll keep an eye, thank you.

At the moment we're doing minimal validations on the bridge lines, because the specs themselves aren't very precise.

So we check only whether they have an IP address (more or less, the regex isn't very strict and as a result something like abc.123.456.789:99999 will pass).

We try to check if a line has a fingerprint, but if the validation fails the line will be still accepted, because it could be an argument for the PT.

Before an IP address there can be only a word without spaces, so something like obfs4 snowflake 192.168.1.2:1234 would not pass. However, we don't enforce the PT name to be for example ascii only. So obfs4️⃣ 192.168.1.2:1234 will pass.

What we can do is:

make the IP check much stricter (separate the IPv4 vs IPv6 case, check that octets in IPv4 are 255 max, and check the port value)
make the vanilla bridge validation very strict (if we don't see a PT, we don't accept additional arguments, only IP address and optionally a fingerprint)
for some known PTs, validate also the additional arguments
for some known PTs, such as obfs4, make the fingerprint compulsory
enforce the PT name to be ASCII (not sure about what the specs say)

I think the fingerprint should be compulsory for all PTs the same for the PT type. It's technically possible to pass bridgelines without fingerprint or PT type to c-tor and they will work, but we never distribute those and I don't think we should support them.

We could give an error if the format doesn't match what we expect but let users submit the bridge if they are sure on what they are doing.

So a bridgeline should have the following format:

type ip:port fingperint args

type. It should be empty or in the list of known types: obfs4, snowflake, webtunnel, conjure or meek_lite
ip & port. you have good proposals on how to improve it
fingerprint. It should be a hexadecimal or missing
args. Can be empty or a list of 'key=value' separated by spaces. Depending on the PT we can require some values:
- obfs4. it must have cert and iat-mode keys
- snowflake. it must have url, fingerprint and ice
- webtunnel. it must have url
- meek. it must have url and front
- conjure. no specific parameters for now

type. It should only be in the list of known types: obfs4, snowflake, webtunnel, conjure or meek_lite

I wouldn't make this a fatal error though, to simplify the procedure of adding new PTs. But I think it's fine to check the arguments.

but we never distribute those and I don't think we should support them.

IIRC either we had Conjure or Webtunnel lines without a fingerprint, I don't know if that changed.

@meskio and @pierov something to keep in mind:

Existing users may have already manually added bridges before we introduce these stricter changes. If they go to edit these bridges through the UI (to add or remove some bridges) then I'm not sure we should block them from saving their existing bridges.

E.g. it could be frustrating for a user if they had previously added a bridge without a fingerprint (that still worked before), and then we require them to include a fingerprint when they just want to add or remove one bridge, especially since they are likely not to have an easy means of finding the fingerprint.

But if we can tell they have lines that could never work, then we could try and get them to clean those lines out.

@meskio, there is the "vanilla" case, ie, bridges without any PT. They are very useful in Russia.

IIRC either we had Conjure or Webtunnel lines without a fingerprint, I don't know if that changed.

Ohh, you are right, I see the conjure bridge didn't have a fingerprint. I hope @cohosh can give some light here.

I'm not sure we should block them from saving their existing bridges.

I don't think we should block users from submitting bridges if we apply the strickter rules I was proposing. Just give a warning.

there is the "vanilla" case, ie, bridges without any PT. They are very useful in Russia.

mmm, ok, I thought vanilla bridges are not in use except for vary small corner cases.

I updated my rules above with the comments you made.

The bridge format might change to include ed25519 fingerprint: tpo/core/torspec#257

@donuts During the sponsor 96 meeting, we talked about addressing our immediate needs in the design first so here are some (currently incomplete at the moment) validation errors and a wizard-like interface which changes depending on whether the input is a Lox invite or a bridge line.

Do you think this is headed in the right direction?

Click to see screenshot

@jag I like the design overall!

I think the last step might need the most redesign:

I don't think we have an easy means of verifying the "Connected" or "Can't connect" states before we actually save them, which would be after the dialog closes.
We likely do not want the "..." options menu within the dialog.
A user never having added bridges before might not understand the content of what they are seeing here. Unlike in the main "about:preferences#connection" page, we do not have the surrounding context to guide them.

After some thought, I'm not sure what we want the user to gain from this last step. We didn't have it before and have only added it to accommodate Lox.

When the user enters the lines directly, since they could verify the full address directly in the previous step, I'm not sure there is much point in showing it again, but truncated. I think maybe being able to check the bridge emojis might be useful, so we could focus the design on that.
When a user uses a Lox invite, I'm not sure what they would make of this final step. What are they expected to do with this information? Are they meant to make some decision based on it? In this case, we might want to just summarise with "Invite successful". Maybe with some introductory text to Lox, like "Over time you might gain new or better bridges, plus invites for your trusted contacts".
In both cases, I think we want to avoid giving the impression that the user should be self-verifying. I.e. it is one thing to use "bridge emojis" to feel sure that you copy and pasted correctly, but I think we want to avoid the impression that this is a safety step. Unless malicious bridges is a thing.

Thanks for the feedback! Good to know about the technical limitations of that dialog I'll make some adjustments.

The thinking behind it is that we could not only check for formatting errors (first screen), but also check for connectivity errors (second screen). But if checking for connectivity is difficult to implement, I'll adjust the mockups!

OpenPGP_0x2F17E7825E755F08.asc

...

-- Jag Talon

Designer for the Tor Project

@henry would you know what kind of errors the Lox Authority can return? I imagine there are errors for:

Invite that has already been used
Cannot contact LA (perhaps the LA is censored)

Are there others?

mentioned in issue tpo/anti-censorship/team#116 (closed)

@henry @donuts @onyinyang Hello! I simplified the error reporting to show errors inline instead of putting it in separate pages. The hope is that this makes it easier for users (the issue is shown directly below the input box) so we're able to contextualize the error) and for development too since there's only one page to implement.

I retained the "Bridges added" dialog not for users to validate but more like to communicate that the bridges have been locked in and added and for users to click on an explicit "Connect" button. From what I understand sometimes users get confused when they're thrown back to the Settings page and there's no explicit connect button.

mentioned in merge request !890 (merged)

mentioned in merge request !901 (merged)

mentioned in merge request !903 (merged)

Removing UX Team since it's being implemented in !903 (merged) cc @donuts

I'm assuming we're only validating Lox invites so far, and we still need to validate and return errors for normal bridges? Is that correct?

we still need to validate and return errors for normal bridges?

I added something basic validation, using an existing method we had in the backend for parsing the output from the tor process. So it should pass for valid bridges, and will catch some incorrect formats, but there are also some holes. E.g. it will treat "0:0" as a valid bridge.

Awesome, that sounds like a good starting point—thanks @henry!

@jag Since this is still assigned to you (presumably to figure out the next steps for this issue?) I've re-added the UX Team label so we don't lose track of this on the team kanban.

@donuts makes sense! would it be more appropriate to move it to Backlog since it's now less of a priority that an initial implementation is available?

Yeah that sounds good. I'm not sure where we left Android design-wise too.

removed UX Team label

added Needs Review label and removed Doing label

added UX Team label

We've an initial implementation here from the linked MRs but lets leave this open until we have it working everywhere.

added Backlog label and removed Needs Review label

added Next label and removed Backlog label

added Backlog label and removed Next label

@henry This ticket got a little side-tracked by Lox, which needed a basic style for validation and errors, but the original intention was always for generic improvements to the (now named) "Add new bridges" dialog.

Part of those changes included improving the formatting of bridge lines when entered into the dialog, i.e. making it so that one bridge line = one text line, providing an in-line button to remove the whole bridge, and adding a counter below the text box.

However I remember in an earlier meeting you were skeptical of how doable some or all of these changes would be. Can you take another look please, and let me know what:

Has been done as part of the lox work
Can be done in future work
Can't be done (or can't be done in an accessible way)

Plus any other ideas you may have for ways in which we can improve the entry of bridge addresses here.

To be clear, no need to "do" anything atm, I'm just trying to figure out how we can best update Figma to be more in line with reality—before we hand this issue back.

@donuts here's my initial take on each feature.

one bridge line = one text line

This can be done easily right now with white-space: pre or similar. All the lines would share one horizontal scrollbar though.

providing an in-line button to remove the whole bridge

Without making each line a separate input, I don't think there is a good way to expose this to the accessibility tree or to fit the "cross" buttons into the Tab cycle.

What is the intended use case for this anyway? If you have existing bridges and want to remove one, then we have an option in the "..." menu for that, so it would likely only be needed for adding new bridges, but why would you need a quick way to remove a bridge you just entered?

Plus "Triple-click and backspace" or "Home, Shift+End, backspace" will already remove the line of text.

Not sure how we would want this to work with the lox invite.

adding a counter below the text box

That should be possible, although I guess with lox we would also want "bridge pass invite".

Not sure what to do in the case where we get junk on a line, perhaps just show nothing?

Note that currently we only verify the entry and show an error when the user presses "Next" so that we don't hit them with error messages whilst they are still typing in an address manually. In a related way, in the case where a user is entering a bridge manually, a truncation of the full bridge address will still look like a valid bridge (e.g. just before the user has entered the fingerprint), so I'm not sure we want to give an indication of "you have entered a bridge address" when they are only half way through.

grey bars at the start of each line

That was not actually a quote, but I noticed it in the mockup. This would require some painting on top, plus updating the positions and clipping whenever the input is scrolled. So I think it would be do-able with some magic, but I guess we would have to figure out if it is worth the hassle.

Overall, looking at the features you want, we might want to consider having independent inputs.

Initially:

<input type="text" title="bridge addresses or invite" placeholder="Paste bridge addresses or invite" />

If the user enters a lox invite, we do not generate any more inputs.

Otherwise, if they enter a bridge address:

<input type="text" title="bridge address 1" value="<first bridge address>" />
<button title="Remove"></button>
<input type="text" title="bridge address 2" value="<second bridge address>" />
<button title="Remove"></button>
<!-- And so on for each line that was pasted. Plus a final empty entry, to accept more input. -->
<input type="text" title="bridge address N" />

We might need to do some trickery to intercept paste events to split the newlines across multiple inputs.

This would mean loosing the multi-line editing capabilities, such as being able to traverse lines using up and down arrows. Plus each input would scroll independently.

Moreover, I don't think it would be a good idea to visually pretend that this is a multi-line input. In particular, I would not have a single blue focus ring around everything, but instead give each input individual focus styling.

This is excellent feedback, thank you very much :)

mentioned in issue #41383

unassigned @jag

mentioned in issue tpo/core/torspec#257

added 14.5 stable label and removed 13.5 stable label

added Roadmap::Future label and removed Backlog label

Improve the flows for manually added bridges and their validation

Design estimate

User story

Opportunity statement

Scope

Out of scope

References

Previous designs

Jag's experimentations

Mockups on Figma

Acceptance criteria

Scenario 1: Add new bridges

Scenario 2: Replace bridges

Scenario 3: Invalid bridge lines

Designs

Child items ...

Activity