-
Notifications
You must be signed in to change notification settings - Fork 369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Block OpenVPN relays from being selected on android #5574
Block OpenVPN relays from being selected on android #5574
Conversation
f20cb3b
to
b85daaa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 5 of 6 files at r1, 1 of 2 files at r2, all commit messages.
Reviewable status: 6 of 7 files reviewed, 1 unresolved discussion (waiting on @Pururun)
android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadDaemon.kt
line 67 at r2 (raw file):
// Fix tunnel protocol being set to any getSettings()?.relaySettings?.let { relaySettings -> if (relaySettings is RelaySettings.Normal) {
We should scope it as limited as possible I feel like::
relaySettings is RelaySettings.Normal && relaySettings.tunnelType != Constraint.Only(TunnelType.Wireguard)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 6 of 7 files reviewed, 1 unresolved discussion (waiting on @Pururun)
android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadDaemon.kt
line 67 at r2 (raw file):
Previously, Rawa (David Göransson) wrote…
We should scope it as limited as possible I feel like::
relaySettings is RelaySettings.Normal && relaySettings.tunnelType != Constraint.Only(TunnelType.Wireguard)
Also if this is approved we should reference some issue that has a due date to remove this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 2 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Pururun)
ed528ab
to
9c6b196
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 7 of 8 files at r3, 1 of 1 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Pururun)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Pururun)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! all files reviewed, all discussions resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
bbecfef
to
5bc23d3
Compare
This change is