eduvpn / apple

app for iOS and macOS
Other
62 stars 18 forks source link

app does not properly discover profiles #312

Closed ghost closed 4 years ago

ghost commented 4 years ago

What happens:

  1. Connect to a VPN server that has only one profile (accessible to the user)
  2. It will automatically connect when you select the server (this is correct)
  3. A profile gets added on the server (for the user)
  4. The app keeps connecting to the initial profile that was available without showing the user the new profile

What should happen:

Step 4 above should list the profiles now available instead of (still) connecting to the first one without ever giving the user the option to choose the second profile.

(This can be done by calling /profile_list after selecting the server to make sure the profile ie still there and whether or not there are now > 1 profiles available).

Note: also deal with case of 0 available profiles (now it will just try? to connect to the previously known profile and fail without error)! Note: also deal with a change of profile, e.g. initially the profile has id "internet", now it is "test" Note: when profile access is revoked when the user is on the profile selection screen the app doesn't handle this properly. Admitted: this is a rare case, still a bug. Note: when no profile is available to the user when adding a new server, the app just returns to the "home" screen without informing the user about this... the server should be added, but when choosing it it should show that there are (not yet) any profiles available for the user...

May be relevant on macOS also, my test was done on iOS.

iOS app version: 2.1.5 (735).

ghost commented 4 years ago

Maybe I should have made 4 issues out of this, but as they are all closely related they can be fixed all at once I think by thoroughly going through the code and fixing all edge cases, and maybe some more that I forgot...

roop commented 4 years ago

This is how the redesigned macOS app (and the future redesigned iOS app) handles these scenarios:

The profile list is fetched after the user selects a server in the main screen. If there are multiple profiles, it's shown in a dropdown - the user can select a profile from the dropdown and turn on the VPN switch. When the user turns on the switch: if any pre-fetched profile is available, the selected profile (or if there's just one pre-fetched profile, that profile) is used for connecting; if no pre-fetched profile is available, we fetch the profile list. (@fkooman: Does this sound right?)

Note: also deal with case of 0 available profiles

A "No profiles available" message is shown.

Note: also deal with a change of profile, e.g. initially the profile has id "internet", now it is "test"

If the user goes back to the main screen and selects the server, this is handled without interruptions. If the profile id changes in the server while the user is still on the connection screen, and the user tries to switch on the VPN, this should result in an error being shown, with an option to refresh the profile list.

Note: when profile access is revoked when the user is on the profile selection screen the app doesn't handle this properly. Admitted: this is a rare case, still a bug.

This should result in an error being shown, with an option to refresh the profile list.

Note: when no profile is available to the user when adding a new server, the app just returns to the "home" screen without informing the user about this... the server should be added, but when choosing it it should show that there are (not yet) any profiles available for the user...

When adding a new server, the profiles are not fetched at all, so this issue doesn't arise.

ghost commented 4 years ago

Just make sure before connecting you always fetch the profile config specific for the profile you are trying to connect to. Don't use an old version as the config may have changed on the server... Not sure what you mean with "pre-fetched profile", there is no such thing...

roop commented 4 years ago

Just make sure before connecting you always fetch the profile config specific for the profile you are trying to connect to.

This is being done.

Not sure what you mean with "pre-fetched profile", there is no such thing...

What I mean is: Say the user selects a server from the main screen. The app shows the connection screen and fetches the profile list, and auto-connects if it just one profile, or shows a dropdown if there's >1. If it got >0 profiles in this fetch, that's what I call as "pre-fetched". This profile list remains in memory as long as the user stays in this connection screen. At this point, toggling the VPN switch on or off doesn't cause the profile list to be re-fetched, but does cause the profile config to be fetched afresh.

ghost commented 4 years ago

ah okay, that sounds good!

roop commented 4 years ago

@fkooman Is it okay to close this now, or should we wait until the redesign is published? Do you want to check this in the redesigned app before I close this?

ghost commented 4 years ago

Don't really care :)

roop commented 4 years ago

Okay. Closing then.