Use proxy requests #175
Use proxy requests #175
Conversation
ruihildt
force-pushed
the
use-proxy-requests
branch
2 times, most recently
from
d9e888c to
438b356
Compare
ruihildt
force-pushed
the
use-proxy-requests
branch
3 times, most recently
from
00ffe9a to
26c9aef
Compare
ruihildt
force-pushed
the
use-proxy-requests
branch
from
b14fe9b to
bf0e3f1
Compare
ruihildt
force-pushed
the
use-proxy-requests
branch
3 times, most recently
from
72c50f2 to
372518d
Compare
|
@hankolsen There are still some little ajustements to be made, but the bulk of the feature change (allow setting proxy per site) is implemented. I realize this is is a bigger change than usual, so if I can help in any way, please let me know. |
ruihildt
force-pushed
the
use-proxy-requests
branch
2 times, most recently
from
f467664 to
e7f46dc
Compare
|
One thing I noticed while test-driving a build of this branch in Mullvad Browser after temporarily disabling extension signing so it would persist across browser restarts is that whenever the extension is opened on a new website, it saves a "socks disabled" config entry for that URL into local storage which persists across browser restarts. While Mullvad Browser no doubt targets a lower threat model than Tor Browser, I'd say this could still be a concern for users who are under the impression that their history is completely wiped each time the browser is closed. While it's pretty obvious that a website URL must be saved in order to make a specific proxy configuration for it, it's less obvious that this is happening even when the extension is simply opened but no configuration change is made. I'd propose only storing the website when the user makes a change to its configuration, and possibly wiping a website from storage when its proxy setting is disabled, at least once the current session ends so there's no cross-session history being saved if there doesn't have to be. |
|
Historically, we created this config automatically so that you could connect to the current server proxy directly (which was the only thing that you could do with OpenVPN). But since we don't support OpenVPN anymore, it indeed make sense to get rid of that behavior.
This is at odds with the current Frequent/Recent feature. I imagine that having a custom list similarly as the Mullvad VPN app does would partially alleviate that concern, since having a proxy listed doesn't mean that you have connected at a specific point in time. It would probably have the same benefits. |
|
@BionicBison05 I realize I have misunderstood the issue you noted. I confused the proxy history entries with the host proxy entry, which reveals the visited websites. Working on a fix. |
ruihildt
force-pushed
the
use-proxy-requests
branch
from
dc83b5c to
ad0419d
Compare
|
Fix appears to be working on my end regarding autogeneration of host-based configs. I'm still a little concerned about the lack of ability to wipe a host-based config once it is created, short of reinstalling the extension or wiping local browser data the manual way. Maybe some sort of delete button could be implemented to give users the option to manage this themselves, sort of like this rough mockup:
Ultimately up to you how far this risk should be mitigated, though. |
|
In a future version, it is planned to add a page to deal with all domain specific proxies, and a button to delete them will be added. |
src/composables/useSocksProxy.ts
Outdated
| const currentHostProxyDNSEnabled = computed(() => currentHostProxyDetails.value?.proxyDNS ?? false); | ||
|
|
||
| const setProxyDetails = (proxyDetails: Ref<ProxyDetails>, enabled: boolean) => { | ||
| proxyDetails.value = { |
There was a problem hiding this comment.
Can't you just do proxyDetails.value.socksEnabled = enabled instead of spreading the current value?
There was a problem hiding this comment.
Probably old React/Redux immutability remnants from the past resurfacing.^^"
There was a problem hiding this comment.
It seems I can get rid of this setProxyDetails function entirely actually.
src/composables/useSocksProxy.ts
Outdated
| setProxyDetails(globalProxyDetails, !globalProxyDetails.value.socksEnabled); | ||
| }; | ||
| const toggleCurrentHostProxy = () => { | ||
| hostProxiesDetails.value[activeTabHost.value] = { |
There was a problem hiding this comment.
Can't you just do hostProxiesDetails.value[activeTabHost.value].socksEnabled = !currentHostProxyEnabled.value instead of spreading the current value?
src/composables/useSocksProxy.ts
Outdated
|
|
||
| await updateConnection(); | ||
| globalProxyDetails.value = { ...globalProxyDetails.value, proxyDNS: updatedGlobalProxyDNS }; |
There was a problem hiding this comment.
Again, do you need to spread here?
src/composables/useSocksProxy.ts
Outdated
|
|
||
| await updateConnection(); | ||
| globalProxyDetails.value = { ...globalProxyDetails.value, proxyDNS: updatedGlobalProxyDNS }; | ||
| globalProxy.value = { ...globalProxy.value, proxyDNS: updatedGlobalProxyDNS }; |
There was a problem hiding this comment.
Again, do you need to spread here?
src/composables/useSocksProxy.ts
Outdated
| socksDetails.value = { | ||
| socksEnabled: false, | ||
| hostProxiesDetails.value[activeTabHost.value] = { | ||
| ...hostProxiesDetails.value[activeTabHost.value], |
There was a problem hiding this comment.
Again, do you need to spread here?
src/composables/useSocksProxy.ts
Outdated
| proxyDNS: updatedCurrentHostProxyDNS, | ||
| }; | ||
| hostProxies.value[activeTabHost.value] = { | ||
| ...hostProxies.value[activeTabHost.value], |
There was a problem hiding this comment.
Again, do you need to spread here?
src/helpers/socksProxy.ts
Outdated
| proxiedHosts.includes(currentHost) && | ||
| hostProxiesDetailsParsed[currentHost].socksEnabled | ||
| ) { | ||
| // console.log(`host proxy "${currentHost}" used for :`, details.url); |
There was a problem hiding this comment.
Remove stuff that is not used?
This is a major refactor of the way proxy work in the extension. Instead of a proxy config being used for the whole browser, each requests are intercepted and can use a specific proxy config based on arbitrary rules. The use of the pageAction is removed, because it is non-standard and will eventually be removed by Firefox. Finally, the interface has been adapted and improved.
ruihildt
force-pushed
the
use-proxy-requests
branch
from
52143d3 to
de538a7
Compare
This PR replace the custom proxy implementation which is browser wide with a targeted approach, where each requests is intercepted and proxied according to the user choice.
Fixes #8 #25 #188 #183 #179