feat(apple): UI notification for reauth #3684

roop · 2024-02-19T09:06:04Z

This PR shows an alert (macOS) / local notification (iOS) when the user is signed out because of a tunnel error.

Fixes the apple part of #3329.

vercel · 2024-02-19T09:06:11Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
firezone	⬜️ Ignored (Inspect)	Visit Preview		Mar 6, 2024 5:40am

github-actions · 2024-02-19T09:09:59Z

Performance Test Results

TCP

Test Name	Received/s	Sent/s	Retransmits
direct-tcp-client2server	66.1 MiB (+1%)	66.7 MiB (+1%)	57 (+30%)
direct-tcp-server2client	54.6 MiB (-1%)	54.8 MiB (-1%)	34 (-19%)
relayed-tcp-client2server	29.6 MiB (+6%)	31.1 MiB (+1%)	113 (+565%)
relayed-tcp-server2client	28.5 MiB (-3%)	31.5 MiB (-2%)	481 (+3%)

UDP

Test Name	Total/s	Jitter	Lost
direct-udp-client2server	50.0 MiB (-0%)	0.18ms (+1%)	0.00% (NaN%)
direct-udp-server2client	50.0 MiB (-0%)	0.24ms (+17%)	0.02% (-96%)
relayed-udp-client2server	50.0 MiB (+0%)	0.56ms (+21%)	19.70% (-6%)
relayed-udp-server2client	50.0 MiB (+0%)	0.50ms (-6%)	20.25% (-13%)

github-actions · 2024-02-19T09:10:27Z

Terraform Cloud Plan Output

Plan: 8 to add, 7 to change, 8 to destroy.

Terraform Cloud Plan

github-actions · 2024-02-19T09:10:29Z

Performance Test Results: direct-perf

TCP

Direction	Received/s	Sent/s	Retransmits
Client to Server	142.1 MiB (+0%)	141.3 MiB (-0%)	147 (-7%)
Server to Client	90.1 MiB (+0%)	90.9 MiB (-0%)	143 (-19%)

UDP

Direction	Total/s	Jitter	Lost
Client to Server	250.0 MiB (-0%)	0.03ms (-35%)	78.37% (+0%)
Server to Client	250.0 MiB (+0%)	0.69ms (+164%)	45.51% (+1%)

roop · 2024-02-19T10:46:29Z

macOS:

iOS:

roop · 2024-02-19T16:35:06Z

In iOS:

We ask for permissions at app launch, but whether the user allowed notifications or not, we'll allow the user to sign in
The notification disappears automatically after a few seconds. We're supposed to be able to make it persistent, but setting the style to "Persistent" (in iOS Settings > Notifications > Firezone > Banner Style) seems to have no effect for me, which is weird. I'll investigate further and report.
- Update: A restart of the phone fixed that. The banner stays on screen till the user presses the home screen.

jamilbk

LGTM, just a couple questions regarding the macOS notification!

jamilbk · 2024-02-21T00:43:52Z

swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/AuthStore.swift

+      alert.messageText = "Your Firezone session has ended"
+      alert.informativeText = "Please sign in again to reconnect"
+      NSApp.activate(ignoringOtherApps: true)
+      alert.runModal()


Is it possible to change the buttons on this alert to:

Dismiss | Sign in so that the user can just click Sign in to fix it without having to click in the menubar?

Also, are the macOS notifications running in Notification center or are they separate? Do they auto-dismiss or stay visible until user action?

Dismiss | Sign in so that the user can just click Sign in to fix it without having to click in the menubar?

Makes sense, will do that. Update: It's possible to add buttons to the notification in iOS, so will do it in iOS as well.

Also, are the macOS notifications running in Notification center or are they separate? Do they auto-dismiss or stay visible until user action?

iOS-like notifications are available in macOS as well, they behave similar to iOS (by default they auto-disappear but the user can make them stay in OS Settings). However, they are easy to miss (because they are small and in a corner), so we're using an in-app alert instead.

Gotcha. I'm wondering if it makes sense to use the Notification Center notification for macOS as well so that user's can turn it off if needed and so that it respects the Do Not Disturb settings by the user.

Otherwise it might be annoying to have that alert pop up in the middle of a presentation or something with no way to deactivate or see how much longer you have left in your session.

If they miss and notice the resources can't be accessed they'll probably figure out to click on the Firezone in menubar.

What do you think?

github-actions · 2024-02-21T00:52:59Z

Performance Test Results: direct-tcp-client2server

Received/s	Sent/s	Retransmits
60.0 MiB (+2%)	60.3 MiB (+2%)	0 (NaN%)

github-actions · 2024-02-21T00:53:03Z

Performance Test Results: relayed-tcp-server2client

Received/s	Sent/s	Retransmits
45.7 MiB (+6%)	45.9 MiB (+6%)	0 (NaN%)

github-actions · 2024-02-21T00:53:05Z

Performance Test Results: direct-udp-server2client

Total/s	Jitter	Lost
736.5 MiB (-10%)	0.31ms (-2%)	91.81% (-1%)

github-actions · 2024-02-21T00:53:13Z

Performance Test Results: relayed-udp-client2server

Total/s	Jitter	Lost
1.0 GiB (+1%)	0.33ms (+0%)	96.57% (-0%)

github-actions · 2024-02-21T00:53:13Z

Performance Test Results: relayed-tcp-client2server

Received/s	Sent/s	Retransmits
44.1 MiB (+12%)	44.2 MiB (+12%)	0 (NaN%)

github-actions · 2024-02-21T00:53:20Z

Performance Test Results: direct-tcp-server2client

Received/s	Sent/s	Retransmits
49.4 MiB (+1%)	49.5 MiB (+1%)	0 (NaN%)

github-actions · 2024-02-21T00:53:22Z

Performance Test Results: direct-udp-client2server

Total/s	Jitter	Lost
999.9 MiB (-0%)	0.88ms (+301%)	93.79% (-0%)

github-actions · 2024-02-21T00:53:29Z

Performance Test Results: relayed-udp-server2client

Total/s	Jitter	Lost
676.9 MiB (+16%)	0.45ms (-36%)	26.66% (-71%)

jamilbk

Requesting changes as discussed in today's call

jamilbk · 2024-03-04T12:51:12Z

@roop Is this still a WIP or should I close it?

roop · 2024-03-04T13:37:58Z

The changes requested were:

Adding 'Sign In' and 'Dismiss' buttons in the notification, so that the user can trigger the login from the notification itself.
Use User Notifications in macOS as well (instead of an alert shown from the app)

The problem is that in macOS, for a user notification posted from the tunnel, the launching-the-app from the notification is not working correctly. The notification is shown, but on clicking on the notification, or on a button in the notification doesn't launch/activate the main app. (Found a relevant forum discussion on this too.) This works correctly in iOS.

@jamilbk So ,shall we stick to using user notifications in macOS, without the 'Sign In' button in the notifications? Or should we go back to alerts?

roop · 2024-03-04T13:39:38Z

@roop Is this still a WIP or should I close it?

It's still a work-in-progress.

jamilbk · 2024-03-04T13:44:28Z

The problem is that in macOS, for a user notification posted from the tunnel, the launching-the-app from the notification is not working correctly.

Ah, I see, thanks for the clarification. Yeah, I thought we may run into issues with that.

Let's keep the existing alert style on macOS in that case then and just have the 'Sign in' and 'Dismiss' options if that's still feasible.

You'll need to rebase as I think there's been some CI changes since this was opened.

- Add padding to info text - Add an image to increase visual differentiation between the VPN and notification screens

roop · 2024-03-05T15:06:04Z

macOS alert with 'Sign In' button:

iOS notification after a long-press (which is how we access actions in a notification):

jamilbk

LGTM, just some code organization / cleanup

swift/apple/FirezoneKit/Sources/FirezoneKit/Features/AskPermissionView.swift

jamilbk · 2024-03-05T16:46:01Z

swift/apple/FirezoneKit/Sources/FirezoneKit/Features/AskPermissionView.swift

+            Spacer()
+              .frame(maxHeight: 20)
+            Text(
+              "After tapping on the above button, tap on 'Allow' when prompted."


Suggested change

"After tapping on the above button, tap on 'Allow' when prompted."

"After tapping the above button, tap 'Allow' when prompted."

Fixed in e83bbab

swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/AuthStore.swift

swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift

…ificationHelper

As per review comments

roop · 2024-03-06T05:12:09Z

@jamilbk:

Same here -- you could extract this into a single SessionNotification.showSignedOutNotification() function and in the SessionNotification class have the macOS / iOS conditional.

I couldn't fit both iOS and macOS code into a single function because they need different inputs, because they get called from different places (iOS: from the tunnel, macOS: from the app). They also do different things (iOS: notification, macOS: alert).

So, there are two different functions for iOS and macOS, and both are in SessionNotificationHelper:

public static func showSignedOutNotificationiOS(logger: AppLogger)
static func showSignedOutAlertmacOS(logger: AppLogger, authStore: AuthStore)

jamilbk · 2024-03-06T06:12:44Z

swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/SessionNotificationHelper.swift

This is much easier to understand, thanks!

roop requested a review from jamilbk February 19, 2024 09:06

jamilbk approved these changes Feb 21, 2024

View reviewed changes

jamilbk enabled auto-merge February 21, 2024 02:20

jamilbk disabled auto-merge February 21, 2024 15:18

jamilbk requested changes Feb 21, 2024

View reviewed changes

roop force-pushed the roop/notify_for_reauth_2 branch from 2482897 to e6f9892 Compare March 5, 2024 14:12

roop added 6 commits March 5, 2024 20:32

iOS: Get notification permission on app start

682b09e

TunnelShutdownEvent: Make 'action' available from 'reason'

e15148e

iOS: Show user notification from inside the tunnel

9382acb

macOS: Show alert on signing out from the main app due to tunnel exiting

71c24d5

iOS: Make the Ask Permissions view look better

3944aed

- Add padding to info text - Add an image to increase visual differentiation between the VPN and notification screens

iOS: Add 'Sign In' and 'Dismiss' buttons to notification

882af1f

macOS: Add 'Sign In' and 'Dismiss' buttons to alert

1b2ffa4

roop force-pushed the roop/notify_for_reauth_2 branch from e6f9892 to 1b2ffa4 Compare March 5, 2024 15:03

roop requested a review from jamilbk March 5, 2024 15:06

jamilbk requested changes Mar 5, 2024

View reviewed changes

roop added 6 commits March 6, 2024 10:05

Rename NotificationDecisionHelper to SessionNotificationHelper

d33ce45

Move showSignedOutNotification() / showSignedOutAlert() to SessionNot…

534c0dc

…ificationHelper

iOS: Fix notification permission view text

e83bbab

As per review comments

SessionNotificationHelper: Fix comment

01447d1

SessionNotificationHelper: Fix variable names

fadf3db

SessionNotificationHelper: Fix log messages

4bff345

roop requested a review from jamilbk March 6, 2024 05:13

macOS: Don't show alert on user-initiated sign-out

c149e33

jamilbk reviewed Mar 6, 2024

View reviewed changes

swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/SessionNotificationHelper.swift

Copy link

Member

jamilbk Mar 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is much easier to understand, thanks!

jamilbk approved these changes Mar 6, 2024

View reviewed changes

jamilbk added this pull request to the merge queue Mar 6, 2024

Merged via the queue into main with commit 2003e3d Mar 6, 2024
66 checks passed

jamilbk deleted the roop/notify_for_reauth_2 branch March 6, 2024 06:29

jamilbk mentioned this pull request Mar 6, 2024

UI notification for reauth #3329

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(apple): UI notification for reauth #3684

feat(apple): UI notification for reauth #3684

roop commented Feb 19, 2024

vercel bot commented Feb 19, 2024 •

edited

Loading

github-actions bot commented Feb 19, 2024 •

edited

Loading

github-actions bot commented Feb 19, 2024 •

edited

Loading

github-actions bot commented Feb 19, 2024

roop commented Feb 19, 2024

roop commented Feb 19, 2024 •

edited

Loading

jamilbk left a comment

jamilbk Feb 21, 2024

roop Feb 21, 2024 •

edited

Loading

jamilbk Feb 21, 2024

github-actions bot commented Feb 21, 2024

github-actions bot commented Feb 21, 2024

github-actions bot commented Feb 21, 2024

github-actions bot commented Feb 21, 2024

github-actions bot commented Feb 21, 2024

github-actions bot commented Feb 21, 2024

github-actions bot commented Feb 21, 2024

github-actions bot commented Feb 21, 2024

jamilbk left a comment

jamilbk commented Mar 4, 2024

roop commented Mar 4, 2024 •

edited

Loading

roop commented Mar 4, 2024

jamilbk commented Mar 4, 2024

roop commented Mar 5, 2024

jamilbk left a comment

jamilbk Mar 5, 2024

roop Mar 6, 2024

roop commented Mar 6, 2024

jamilbk Mar 6, 2024

	"After tapping on the above button, tap on 'Allow' when prompted."
	"After tapping the above button, tap 'Allow' when prompted."

feat(apple): UI notification for reauth #3684

feat(apple): UI notification for reauth #3684

Conversation

roop commented Feb 19, 2024

vercel bot commented Feb 19, 2024 • edited Loading

github-actions bot commented Feb 19, 2024 • edited Loading

Performance Test Results

TCP

UDP

github-actions bot commented Feb 19, 2024 • edited Loading

Terraform Cloud Plan Output

github-actions bot commented Feb 19, 2024

Performance Test Results: direct-perf

TCP

UDP

roop commented Feb 19, 2024

roop commented Feb 19, 2024 • edited Loading

jamilbk left a comment

Choose a reason for hiding this comment

jamilbk Feb 21, 2024

Choose a reason for hiding this comment

roop Feb 21, 2024 • edited Loading

Choose a reason for hiding this comment

jamilbk Feb 21, 2024

Choose a reason for hiding this comment

github-actions bot commented Feb 21, 2024

Performance Test Results: direct-tcp-client2server

github-actions bot commented Feb 21, 2024

Performance Test Results: relayed-tcp-server2client

github-actions bot commented Feb 21, 2024

Performance Test Results: direct-udp-server2client

github-actions bot commented Feb 21, 2024

Performance Test Results: relayed-udp-client2server

github-actions bot commented Feb 21, 2024

Performance Test Results: relayed-tcp-client2server

github-actions bot commented Feb 21, 2024

Performance Test Results: direct-tcp-server2client

github-actions bot commented Feb 21, 2024

Performance Test Results: direct-udp-client2server

github-actions bot commented Feb 21, 2024

Performance Test Results: relayed-udp-server2client

jamilbk left a comment

Choose a reason for hiding this comment

jamilbk commented Mar 4, 2024

roop commented Mar 4, 2024 • edited Loading

roop commented Mar 4, 2024

jamilbk commented Mar 4, 2024

roop commented Mar 5, 2024

jamilbk left a comment

Choose a reason for hiding this comment

jamilbk Mar 5, 2024

Choose a reason for hiding this comment

roop Mar 6, 2024

Choose a reason for hiding this comment

roop commented Mar 6, 2024

jamilbk Mar 6, 2024

Choose a reason for hiding this comment

vercel bot commented Feb 19, 2024 •

edited

Loading

github-actions bot commented Feb 19, 2024 •

edited

Loading

github-actions bot commented Feb 19, 2024 •

edited

Loading

roop commented Feb 19, 2024 •

edited

Loading

roop Feb 21, 2024 •

edited

Loading

roop commented Mar 4, 2024 •

edited

Loading