Skip to content
This repository was archived by the owner on Feb 24, 2025. It is now read-only.

Conversation

@diegoreymendez
Copy link
Contributor

@diegoreymendez diegoreymendez commented Aug 18, 2023

Task/Issue URL: https://app.asana.com/0/0/1205275221447702/f

Description:

This PR offers a solution to the System Extension activation issues we currently have.
It also modifies the startup flow: now when the user enables the system extension they'll still need to start the tunnel manually.

Steps to test this PR:

  1. Launch the app and reset the Network Protection Status
  2. Start the onboarding, see that the system-extension-blocked alert is shown.
  3. Ignore the request, tap on the button to start the onboarding again.
  4. Make sure system settings is opened.

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

@diegoreymendez diegoreymendez changed the title Resolves sysex activation issues Implements solution for System Extension activation issues Aug 18, 2023
@diegoreymendez diegoreymendez marked this pull request as ready for review August 18, 2023 08:48
//
if onboardingStatusRawValue == OnboardingStatus.isOnboarding(step: .userNeedsToAllowExtension).rawValue {
onboardingStatusRawValue = OnboardingStatus.completed.rawValue
return
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This modifies the flow - if we're enabling the system extension, we'll have the user start the tunnel manually again.

As requested here: https://app.asana.com/0/0/1205275221447701/1205278158970389/f

/// see the alert every single time, they'll see both the alert and system settings being opened automatically.
///
if hasPendingActivationRequests() {
openSystemSettingsSecurity()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there are pending activation requests we'll open system settings directly.

Comment on lines +82 to +95
let task = Process()
let pipe = Pipe()

task.standardOutput = pipe
task.launchPath = "/bin/bash" // Specify the shell to use
task.arguments = ["-c", "$(which systemextensionsctl) list | $(which egrep) -c '(?:\(bundleID)).+(?:activated waiting for user)+'"]

task.launch()
task.waitUntilExit()

let data = pipe.fileHandleForReading.readDataToEndOfFile()
let output = String(data: data, encoding: .utf8)?.trimmingCharacters(in: .whitespacesAndNewlines)

return (Int(output ?? "0") ?? 0) > 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While we can request the system extension properties using a specific type of request, doing so requires more complicated modifications to the code.

This gives us quickly something that works immediately for macOS 11+.

Sending a properties request is much more complicated because it requires me to modify SystemExtensionManager to support different delegates and running multiple requests.

This decision was a tradeoff.


let bundleID: String
let manager: OSSystemExtensionManager
private static let systemSettingsSecurityURL = "x-apple.systempreferences:com.apple.preference.security?Security"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice, I hadn't seen this URL format before

Copy link
Collaborator

@samsymons samsymons left a comment

Choose a reason for hiding this comment

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

For me this works perfectly, nice solution. Actually I like your solution better than Apple's since it takes the user right to the section they need to click anyway.

@diegoreymendez diegoreymendez merged commit 23825e7 into diego/feature-network-protection-onboarding Aug 19, 2023
@diegoreymendez diegoreymendez deleted the diego/netp-new-onboarding-6 branch August 19, 2023 11:46
@diegoreymendez
Copy link
Contributor Author

Thank you for the review @samsymons !

diegoreymendez added a commit that referenced this pull request Aug 23, 2023
Task/Issue URL: https://app.asana.com/0/0/1205301030177476/f

## Description:

Implements the new onboarding for Network Protection.

## Incremental PR reviews

This is a list of the individual PRs (and reviews) that make up this
feature:
- #1464
- #1466
- #1474
- #1484
- #1502
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants