Skip to content

fix: use requesting frame origin in permission helper and device choosers#50052

Merged
VerteDinde merged 2 commits intomainfrom
wc-perm-helper-origin
Mar 9, 2026
Merged

fix: use requesting frame origin in permission helper and device choosers#50052
VerteDinde merged 2 commits intomainfrom
wc-perm-helper-origin

Conversation

@codebytere
Copy link
Copy Markdown
Member

@codebytere codebytere commented Mar 3, 2026

Description of Change

WebContentsPermissionHelper::RequestPermission was passing web_contents_->GetLastCommittedURL() as the requesting origin instead of the actual requesting frame's origin. When a subframe initiates a permission request, this meant the top-level page's URL was forwarded rather than the subframe's. The HID, USB, and Serial device choosers had the same pattern — they keyed grants to the primary main frame's origin instead of the requesting frame's.

This PR switches all of these paths to use requesting_frame->GetLastCommittedOrigin() so the origin matches the frame that actually initiated the request.

No API changes — details.requestingUrl is unchanged (it was already derived from the requesting RFH in ElectronPermissionManager).

Checklist

Release Notes

Notes: Fixed an issue where permission and device-chooser handlers received the top-level page origin instead of the requesting subframe's origin.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened recently label Mar 3, 2026
…ions

`WebContentsPermissionHelper::RequestPermission` passes
`web_contents_->GetLastCommittedURL()` as the origin to the permission
manager instead of the actual requesting frame's origin. This enables
origin confusion when granting permissions to embedded third-party iframes,
since app permission handlers see the top-level origin instead of the
iframe's. The same pattern exists in the HID, USB, and Serial device
choosers, where grants are keyed to the primary main frame's origin rather
than the requesting frame's.

Fix this by using `requesting_frame->GetLastCommittedOrigin()` in all
affected code paths, renaming `details.requestingUrl` to
`details.requestingOrigin`, and populating it with the serialized
origin only.
@codebytere codebytere force-pushed the wc-perm-helper-origin branch from ee110a5 to b287b8e Compare March 4, 2026 08:55
@codebytere codebytere changed the title fix: use requesting frame origin instead of top-level URL for permissions refactor: use requesting frame origin instead of top-level URL for permissions Mar 4, 2026
@codebytere codebytere added semver/minor backwards-compatible functionality semver/major incompatible API changes no-backport and removed semver/minor backwards-compatible functionality labels Mar 4, 2026
@codebytere codebytere marked this pull request as ready for review March 4, 2026 11:19
Copy link
Copy Markdown
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍

Possible to have a deprecation warning for requestingUrl ?

@ckerr ckerr self-requested a review March 6, 2026 18:05
The previous commit changed the details.requestingUrl field to
details.requestingOrigin in permission request/check handlers. That
field was already populated from the requesting frame's RFH, so the
rename was unnecessary and would break apps that read the existing
property. Revert to requestingUrl to preserve the existing API shape.

The functional changes to use the requesting frame in
WebContentsPermissionHelper and the HID/USB/Serial choosers remain.
@MarshallOfSound MarshallOfSound changed the title refactor: use requesting frame origin instead of top-level URL for permissions fix: use requesting frame origin in permission helper and device choosers Mar 7, 2026
@MarshallOfSound MarshallOfSound added semver/patch backwards-compatible bug fixes target/39-x-y PR should also be added to the "39-x-y" branch. target/40-x-y PR should also be added to the "40-x-y" branch. target/41-x-y PR should also be added to the "41-x-y" branch. and removed semver/major incompatible API changes no-backport labels Mar 7, 2026
@electron-cation electron-cation bot added new-pr 🌱 PR opened recently and removed api-review/requested 🗳 new-pr 🌱 PR opened recently labels Mar 7, 2026
@VerteDinde VerteDinde merged commit 2c6dd11 into main Mar 9, 2026
126 of 129 checks passed
@VerteDinde VerteDinde deleted the wc-perm-helper-origin branch March 9, 2026 16:20
@release-clerk
Copy link
Copy Markdown

release-clerk bot commented Mar 9, 2026

Release Notes Persisted

Fixed an issue where permission and device-chooser handlers received the top-level page origin instead of the requesting subframe's origin.

@trop
Copy link
Copy Markdown
Contributor

trop bot commented Mar 9, 2026

I have automatically backported this PR to "39-x-y", please check out #50147

@trop trop bot added the in-flight/39-x-y label Mar 9, 2026
@trop trop bot removed the target/39-x-y PR should also be added to the "39-x-y" branch. label Mar 9, 2026
@trop
Copy link
Copy Markdown
Contributor

trop bot commented Mar 9, 2026

I have automatically backported this PR to "41-x-y", please check out #50148

@trop
Copy link
Copy Markdown
Contributor

trop bot commented Mar 9, 2026

I have automatically backported this PR to "40-x-y", please check out #50149

@trop trop bot added in-flight/41-x-y in-flight/40-x-y and removed target/41-x-y PR should also be added to the "41-x-y" branch. target/40-x-y PR should also be added to the "40-x-y" branch. labels Mar 9, 2026
@MarshallOfSound
Copy link
Copy Markdown
Member

/trop run backport-to 38-x-y

@trop
Copy link
Copy Markdown
Contributor

trop bot commented Mar 9, 2026

The backport process for this PR has been manually initiated - sending your PR to 38-x-y!

@trop
Copy link
Copy Markdown
Contributor

trop bot commented Mar 9, 2026

I have automatically backported this PR to "38-x-y", please check out #50151

@trop trop bot added in-flight/38-x-y merged/38-x-y PR was merged to the "38-x-y" branch. merged/41-x-y PR was merged to the "41-x-y" branch. merged/40-x-y PR was merged to the "40-x-y" branch. merged/39-x-y PR was merged to the "39-x-y" branch. and removed in-flight/38-x-y in-flight/41-x-y in-flight/40-x-y in-flight/39-x-y labels Mar 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged/38-x-y PR was merged to the "38-x-y" branch. merged/39-x-y PR was merged to the "39-x-y" branch. merged/40-x-y PR was merged to the "40-x-y" branch. merged/41-x-y PR was merged to the "41-x-y" branch. semver/patch backwards-compatible bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants