Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

External links should request user confirmation #551

Closed
jumde opened this issue Dec 3, 2018 · 13 comments
Closed

External links should request user confirmation #551

jumde opened this issue Dec 3, 2018 · 13 comments

Comments

@jumde
Copy link
Contributor

jumde commented Dec 3, 2018

Description:

Third party apps can be opened from Brave without user confirmation

Steps to Reproduce

  1. Navigate to mailto://[email protected]

Actual result:
Mail opens

Expected result:
Request user confirmation before opening external apps.

Reproduces how often: [Easily reproduced, Intermittent Issue]
Everytime

Brave Version:
1.7 (18.11.30.03)

Device details:
iPhone SE/iOS 12.1

@jumde jumde added the security label Dec 3, 2018
@tildelowengrimm tildelowengrimm added priority/P4 Planned work. We expect to get to it "soon". priority/P3 The next thing for us to work on. and removed priority/P4 Planned work. We expect to get to it "soon". labels Dec 3, 2018
@kylehickinson
Copy link
Collaborator

Should a popup be brought up for any non web URL scheme or just non-first-party Apple app schemes?

@tildelowengrimm
Copy link

Any scheme which will open an app other than Brave.

@kylehickinson
Copy link
Collaborator

Port copy from 1.6, however also need to have this alert come up for all apps not just third-party: https://github.com/brave/browser-ios/blob/development/Client/Frontend/Browser/BrowserViewController/BrowserViewController+WKNavigationDelegate.swift#L191

@jumde
Copy link
Contributor Author

jumde commented Jan 19, 2019

One more test case:

  1. Install appear.in from app store
  2. Navigate to appear.in/brr
  3. Click Open in App - No user confirmation is needed

@kjozwiak
Copy link
Member

Example of the modal notification:

Screen Shot 2019-11-22 at 11 27 01 AM

Verification PASSED on iPhone 6s running iOS 13.2.2 using 1.14 (19.11.22.15)

Verification PASSED on iPhone 6s+ running iOS 12.4.1 using 1.14 (19.11.22.15)

Verification PASSED on iPad Air 3rd Gen running iOS 13.3 using 1.14 (19.11.22.15)

Verification PASSED on iPad Mini 4 running iOS 12.4.1 using 1.14 (19.11.22.15)

@anthonypkeane
Copy link

If the users presses Don't allow, what happens @kylehickinson

Will the user be prompted again next time they visit that site and perform the same action?

@kylehickinson
Copy link
Collaborator

@anthonypkeane They will continue to be prompted each time

@jumde
Copy link
Contributor Author

jumde commented Nov 25, 2019

@kylehickinson - can we use AlertPopupView instead of the regular modal. It seems phishable. I can open a follow up issue for that

@kylehickinson
Copy link
Collaborator

I mean, we could, however webpage alerts title's are always their origins as seen here:

private func titleForJavaScriptPanelInitiatedByFrame(_ frame: WKFrameInfo) -> String {
var title = "\(frame.securityOrigin.`protocol`)://\(frame.securityOrigin.host)"
if frame.securityOrigin.port != 0 {
title += ":\(frame.securityOrigin.port)"
}
return title
}

@srirambv
Copy link
Contributor

@jumde we can add a new issue and mark it for next release unless its a high priority for the current one

@srirambv
Copy link
Contributor

Verification passed on iPhone XR with iOS 13.2 running 1.14(19.11.22.15)

@kjozwiak
Copy link
Member

kjozwiak commented Nov 25, 2019

If the users presses Don't allow, what happens @kylehickinson

As per @kylehickinson via #551 (comment), the modal goes away and the user is prompted again. Also went through those cases via #551 (comment).

@jumde as per #551 (comment), is this something we need to fix in this version or can it moved into the next release? We'll need to know ASAP so someone can start working on it. Can you create a new issue or would you like QA to do it?

@kjozwiak
Copy link
Member

Talked to @jumde who mentioned using AlertPopupView instead of a regular modal isn't urgent and can be addressed in the next version. Created #2039 as a follow up and added it into https://github.com/brave/brave-ios/milestone/33.

iccub added a commit that referenced this issue Oct 13, 2020
Previously we opened external links only when navigation action type
was `linkActivated`. This is not always the case, sometimes tapping
on a link has different navigation type.

This is a followup to #551 ticket.
iccub added a commit that referenced this issue Oct 13, 2020
Previously we opened external links only when navigation action type
was `linkActivated`. This is not always the case, sometimes tapping
on a link has different navigation type.

This is a followup to #551 ticket.
iccub added a commit that referenced this issue Oct 13, 2020
Previously we opened external links only when navigation action type
was `linkActivated`. This is not always the case, sometimes tapping
on a link has different navigation type.

This is a followup to #551 ticket.
iccub added a commit that referenced this issue Oct 13, 2020
…2956)

Previously we opened external links only when navigation action type
was `linkActivated`. This is not always the case, sometimes tapping
on a link has a different navigation type.

This is a followup to #551 ticket.
Brandon-T pushed a commit that referenced this issue Nov 19, 2020
…2956)

Previously we opened external links only when navigation action type
was `linkActivated`. This is not always the case, sometimes tapping
on a link has a different navigation type.

This is a followup to #551 ticket.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.