ivpn / ios-app

Official IVPN iOS app
https://www.ivpn.net/apps-ios
GNU General Public License v3.0
439 stars 92 forks source link

Add support for Device Management #416

Closed jurajhilje closed 7 months ago

jurajhilje commented 8 months ago

Description

There are few updates needed to support the Device Management feature:

gorkapernas commented 8 months ago

@jurajhilje we need to make some small text changes. This has been discussed with Viktor already.

Changes: "Log out all devices" change to "Log out from all devices" "Retry (I have removed the device)" change to "Retry"

Also, the following requirement hasn't been implemented:

If the feature is Enabled, when I remove a device in the dashboard, I expect that the app on that device will switch to a “logged out” state and I receive a warning about this when I open the app prompting me to log in again to use the service

We already have a warning on the desktop apps when force logout happens, but I'm not sure if we need to modify the text to meet the device management criteria Cc @johnnyburnaway. See screenshot below for further details. Image 2024-01-16 at 4 01 50 PM

jurajhilje commented 8 months ago

@gorkapernas I will implement the same alert on iOS and Android apps (before displaying the login screen), so we have the same UX across all platforms. cc: @johnnyburnaway

jurajhilje commented 8 months ago

@gorkapernas Update is available in TestFlight 2.12.0 (8)

gorkapernas commented 8 months ago

Verified on 2.12.0 (8), the changes suggested have been implemented correctly. However, we still have some issues.

  1. The force logout warning is displayed in the main screen (map), however the text mentions that the user has been redirected to the login screen. Therefore, we need to either change the text or redirect the user to the login form and then display the warning. Cc @johnnyburnaway

  2. When reaching the device limit with a legacy account, we also display the Device Management options. Please note that Device management is not supposed to work for legacy accounts, so any mentions to Device Management should be avoided in this case.

  3. There is a UI issue in iPads with the device limit dialog, the issue happens when changing from portrait to landscape while the dialog is visible on the screen. See screenshot below for further details.

  4. When reaching the device limit, It is suggested to also add the option "Retry" when Device Management is disabled, just like we have right now. Existing users are already familiar with session management, so they know that they can logout from a device and retry in the new device. So I think it would be useful to provide this option also when Device Management is disabled.

gorkapernas commented 8 months ago

@jurajhilje there is an additional issue, the link for the action button "Upgrade for 7 devices" redirects to /account/payment, the user cannot upgrade by making a new payment, since the new funds will be added to the current plan. We should redirect the user to /account/change-product instead.

Furthermore, should we change the text "Upgrade for 7 devices" to "Switch to IVPN Pro"? I think it would more accurate since there isn't really an upgrade, but a plan change. But on the other hand, maybe it's better to mention "7 devices". WDYT? @jurajhilje @johnnyburnaway

jurajhilje commented 8 months ago

@gorkapernas "Upgrade" link is not hardcoded in the apps, but instead is received from the API response as upgrade_to_url (part of the session limit error response). Can you please open an issue in the backend project?

jurajhilje commented 8 months ago

@gorkapernas New 2.12.0 (9) build is available. I updated all points from your previous comment, except the iPad UI issue. It's not reproducible on iPadOS 17.1 and 17.2, and since we use system dialogs I'm guessing that it was an iOS bug fixed in the recent updates. We can leave the ticket open, as we'll probably still do some minor text changes.

gorkapernas commented 8 months ago

@jurajhilje FYI, the iPad issue was reproduced on an iPad 10 iPadOS 17.1.1. I'll test the other fixes and let you know if there are any problems.

Also waiting for your feedback about this question. / Cc @johnnyburnaway

Furthermore, should we change the text "Upgrade for 7 devices" to "Switch to IVPN Pro"? I think it would more accurate since there isn't really an upgrade, but a plan change. But on the other hand, maybe it's better to mention "7 devices". WDYT? @jurajhilje @johnnyburnaway

jurajhilje commented 8 months ago

@gorkapernas You were right about the iPadOS issue, it's now improved by forcing the position below the input field, even when changing orientation. New build: 2.12.0 (10)

gorkapernas commented 8 months ago

I was able to verify fixed all issues, except from issue # 2, currently it isn't possible to create sessions will legacy accounts on staging due to a backend issue. I'll update the ticket again once I'm able to test sessions management with legacy accounts.

gorkapernas commented 8 months ago

Verified fixed the issue with legacy accounts, however, we still need to make a small change in the dialog for these accounts. When logging in with a legacy standard account, we don't provide the option to upgrade for 7 devices. This option was available in old IVPN versions as "Switch to IVPN Pro", so we should also include it in the new device limit dialog for legacy standard accounts.

gorkapernas commented 8 months ago

@jurajhilje we need to change "Upgrade for 7 devices" to "Switch to IVPN Pro". Requirements in confluence have also been updated.

jurajhilje commented 8 months ago

@gorkapernas New 2.12.0 (12) build is available:

jurajhilje commented 8 months ago

@gorkapernas New 2.12.0 (19) build is available:

gorkapernas commented 8 months ago

@jurajhilje it seems like the buttons "Visit Device Management" and "Enable Device Management" in the device limit dialog are missing for new PRO accounts. This issue does not happen on the Android app.

jurajhilje commented 8 months ago

@gorkapernas New 2.12.0 (20) is available.

gorkapernas commented 8 months ago

Verified on 2.12.0 (20), iPhone XR iOS 17.4 and iPad 10 iPadOS 17.1.1, all the issues reported have been fixed. Tested with legacy and new account IDs. This is good to go.