-
Notifications
You must be signed in to change notification settings - Fork 28
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
Enable username&password passed in URL #245
Conversation
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.
Unsure what the mailto thing is about, but if that's deliberate this code looks OK and I've seen it working on iOS (under some conditions).
However, the tests appear to be failing. If these are flakey tests, can you comment them out / remove them and create tasks for the owners, please?
@@ -91,28 +91,54 @@ extension URL { | |||
public static let blob = NavigationalScheme(rawValue: "blob") | |||
public static let about = NavigationalScheme(rawValue: "about") | |||
|
|||
public static let mailto = NavigationalScheme(rawValue: "mailto") |
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.
Are these changes related?
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.
mailto uses the same user@domain.com URL structure which was returning wrong URL for punycoded addresses, this path of URLs with username/password is handled separately from the "normal" URLs
the flaky unit tests are fixed in #240 |
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.
Tested macOS and iOS and all seems to work as expected. LGTM
Task/Issue URL: https://app.asana.com/0/414235014887631/1204080079383830/f Tech Design URL: https://app.asana.com/0/481882893211075/1204065038916791/f Security Triage URL: https://app.asana.com/0/1199892415909552/1204067721102886/f BSK PR: duckduckgo/BrowserServicesKit#245 iOS PR: duckduckgo/iOS#1521 CC: @brindy **Description**: - Adds support to pass basic auth username/password in URL **Steps to test this PR**: 1. Validate login/password passed in URL is used for basic authentication and is saved per session 2. Validate login/password passed in URL is not displayed in the UI or saved in browsing history 3. Validate when clicking links containing username/password (user:pass@domain.com), the credentials aren‘t displayed in UI but are used during basic auth 4. Validate if invalid credentials are provided then auth dialog is displayed <!-- Tagging instructions If this PR isn't ready to be merged for whatever reason it should be marked with the `DO NOT MERGE` label (particularly if it's a draft) If it's pending Product Review/PFR, please add the `Pending Product Review` label. If at any point it isn't actively being worked on/ready for review/otherwise moving forward (besides the above PR/PFR exception) strongly consider closing it (or not opening it in the first place). If you decide not to close it, make sure it's labelled to make it clear the PRs state and comment with more information. --> --- ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) **When ready for review, remember to post the PR in MM**
Task/Issue URL: https://app.asana.com/0/414235014887631/1204080079383830/f Tech Design URL: https://app.asana.com/0/481882893211075/1204065038916791/f Security Triage URL: https://app.asana.com/0/1199892415909552/1204067721102886/f BSK PR: duckduckgo/BrowserServicesKit#245 iOS PR: duckduckgo/iOS#1521 CC: @brindy **Description**: - Adds support to pass basic auth username/password in URL **Steps to test this PR**: 1. Validate login/password passed in URL is used for basic authentication and is saved per session 2. Validate login/password passed in URL is not displayed in the UI or saved in browsing history 3. Validate when clicking links containing username/password (user:pass@domain.com), the credentials aren‘t displayed in UI but are used during basic auth 4. Validate if invalid credentials are provided then auth dialog is displayed <!-- Tagging instructions If this PR isn't ready to be merged for whatever reason it should be marked with the `DO NOT MERGE` label (particularly if it's a draft) If it's pending Product Review/PFR, please add the `Pending Product Review` label. If at any point it isn't actively being worked on/ready for review/otherwise moving forward (besides the above PR/PFR exception) strongly consider closing it (or not opening it in the first place). If you decide not to close it, make sure it's labelled to make it clear the PRs state and comment with more information. --> --- ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) **When ready for review, remember to post the PR in MM**
# By Alexey Martemyanov (8) and others # Via GitHub * main: Make FeatureName a struct so it can be extended from client code (#276) add count of all bookmarks in domain to view model (#272) Configuration updates optimizations (#269) Roll-back InteractiveUserScript (#268) Update C-S-S to 4.4.4 (#260) Add invalid payload pixel handler (#264) Fix Error pixel misfiring on valid scenario (#261) add didCancelNavigationAction (#262) Opt-outable private API usage (#254) HTTPS upgrade threading (#256) DDGSync module (#253) fix delayed new window with empty url creation handling (#255) remove major tracker network (#251) Enable username&password passed in URL (#245) fix assertion for popup about:blank navigations without WKNavigation (#247) fix about: scheme fragments dropping (#246) Unify Configuration Management (#241) # Conflicts: # Package.resolved # Package.swift
Please review the release process for BrowserServicesKit here.
Required:
Task/Issue URL: https://app.asana.com/0/414235014887631/1204080079383830/f
Tech Design URL: https://app.asana.com/0/481882893211075/1204065038916791/f
Security Triage URL: https://app.asana.com/0/1199892415909552/1204067721102886/f
iOS PR: duckduckgo/iOS#1521
macOS PR: duckduckgo/macos-browser#1006
What kind of version bump will this require?: Minor
Description:
Steps to test this PR:
OS Testing:
Internal references:
Software Engineering Expectations
Technical Design Template