VPN-5992 Allow extensions to request Access to the Proxy Network #9639
Conversation
|
Since @lesleyjanenorton has more context, I'm going to let her review this one. If you want a review from me as well, let me know - I'll take the time to read the docs for this project so I can do this PR justice. |
| @@ -19,7 +19,8 @@ MZButtonBase { | |||
|
|
|||
| function handleClick() { | |||
| toolTip.close(); | |||
| if (VPNController.state !== VPNController.StateOff) { | |||
| if (VPNController.state !== VPNController.StateOff && | |||
| VPNController.state !== VPNController.StateOnPartial) { | |||
There was a problem hiding this comment.
One scenario I think we'll need to resolve but not in this PR...
- We connect via the extension, we're in StateOnPartial...
- We connect via the client, now we're in StateOn...
- We deactivate from the client... we're in StateOff... but then that means Firefox is now also unprotected(?)
This is how it is supposed to work from the PRD but I'm guessing we'll get foxfooder feedback saying this is unexpected behavior so we might think about how to persist the extension vpn state through a deactivation in the client.
There was a problem hiding this comment.
Given that the proxy api works on request basis, and we stream the state of the vpn we can simply request to re-enable the vpn to partial with the next request. Given we set a proxy in that case, the request would have a small delay.
Or we check before disconnecting if there is an active web-extension connection. however then we rely on the port continuesly beeing open.
| @@ -122,6 +137,10 @@ QJsonObject WebExtensionAdapter::serializeStatus() { | |||
|
|
|||
| { | |||
| Controller::State state = vpn->controller()->state(); | |||
| if (state == Controller::StateOnPartial) { | |||
| state = Controller::StateOn; // Old extensions like MAC dont know and | |||
| // dont need to know about this state. | |||
There was a problem hiding this comment.
We can always update MAC to handle this if we for some reason need to
There was a problem hiding this comment.
Let's do that, we probably should start thinking about versioning the api too.
| sock.destroy(); | ||
| }); | ||
|
|
||
| it('A Webextension can NOT deactivate the VPN if it was activated in Client', |
There was a problem hiding this comment.
Related to other comment: Eventually we will allow deactivating the vpn for Firefox in this situation right? This may be a terrible idea but I'm almost wondering if we'll want another state machine for the extension to make keeping track of situations like client is on but Firefox VPN is off a bit easier. Otherwise if we do allow the user to remove Firefox traffic from the tunnel when the vpn was activated from the client, but tell the extension that the VPN is 'on', how will the extension know that Firefox is excluded/"off"?
There was a problem hiding this comment.
The Webextension state machine is going to be diffrent from the client.
If full device protection is enabled, the extension can use the localproxy to "exlude firefox"
If it's in partialOn we can just not use a proxy.
Thinking about this more, i think we might need to tell the extension actually if it is partially or full-device on.
But in no situation we need the extension to downgrade from full device protection :)
| logger.debug() << "Deactivation" << m_state; | ||
| if (m_initiator > user) { |
There was a problem hiding this comment.
Eventually we want to embellish this to allow deactivating the VPN for Firefox from the extension in this situation though right?
There was a problem hiding this comment.
probably not, given:
We cannot know that whatever is talking using the web-extension api really is firefox. could be a malicous actor.
We don't need to disable the vpn for firefox to not use the vpn connection see #9676
Co-authored-by: Lesley Norton <lesley@mozilla.com>
Co-authored-by: Lesley Norton <lesley@mozilla.com>
This pull request introduces support for a new StateOnPartial state in the controller, enhancing the VPN's state management. It also adds an ActivationPrincipal enum to track whether a connection was initiated by a user or an extension, along with corresponding permission checks for deactivation. Additionally, this update includes several new methods and refines existing ones to handle the new state and activation principal, ensuring improved VPN control and security.