Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

deepak1556
Copy link
Member

Description of Change

With net module support to utility process added via #40017, it is currently not straightforward to handle basic auth. Apps would need to have some sort of boilerplate code from utility process to allow the main process trigger an interactive UI for obtaining the credentials. However, we already have logic obtained from Chromium to support basic auth requests from renderers or service workers via the app.on('login', ...) event. It would be simpler if apps can opt-in to the same event for requests from utility process as well, this is now possible via a new property respondToAuthRequestsFromMainProcess on the utilityProcess.fork api.

Refs microsoft/vscode#212949

Release Notes

Notes: Add support for responding to auth requests initiated from utility process via app#login event

@deepak1556 deepak1556 added semver/minor backwards-compatible functionality target/30-x-y PR should also be added to the "30-x-y" branch. target/31-x-y PR should also be added to the "31-x-y" branch. target/32-x-y PR should also be added to the "32-x-y" branch. labels Jun 24, 2024
@deepak1556 deepak1556 force-pushed the robo/add_app_login_utility_process branch 2 times, most recently from d84a9ce to c6ab261 Compare June 24, 2024 13:59
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened recently label Jul 1, 2024
Copy link
Member

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

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

API LGTM

@erickzhao erickzhao self-assigned this Jul 22, 2024
docs/api/app.md Outdated Show resolved Hide resolved
shell/browser/login_handler.cc Outdated Show resolved Hide resolved
@erickzhao erickzhao removed their assignment Aug 9, 2024
@deepak1556 deepak1556 force-pushed the robo/add_app_login_utility_process branch from c6ab261 to 143b6d8 Compare August 12, 2024 11:39
@deepak1556 deepak1556 requested a review from a team as a code owner August 12, 2024 11:39
Copy link
Member

@itsananderson itsananderson left a comment

Choose a reason for hiding this comment

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

API LGTM

@jkleinsc jkleinsc force-pushed the robo/add_app_login_utility_process branch from 449c1d8 to 26eb0ab Compare August 13, 2024 19:27
@deepak1556
Copy link
Member Author

Failing test is unrelated, merging.

@deepak1556 deepak1556 merged commit 9b166b3 into main Aug 14, 2024
@deepak1556 deepak1556 deleted the robo/add_app_login_utility_process branch August 14, 2024 02:36
@release-clerk
Copy link

release-clerk bot commented Aug 14, 2024

Release Notes Persisted

Add support for responding to auth requests initiated from utility process via app#login event

@trop
Copy link
Contributor

trop bot commented Aug 14, 2024

I was unable to backport this PR to "30-x-y" cleanly;
you will need to perform this backport manually.

@trop trop bot removed the target/30-x-y PR should also be added to the "30-x-y" branch. label Aug 14, 2024
@trop
Copy link
Contributor

trop bot commented Aug 14, 2024

I was unable to backport this PR to "31-x-y" cleanly;
you will need to perform this backport manually.

@trop trop bot added needs-manual-bp/30-x-y and removed target/31-x-y PR should also be added to the "31-x-y" branch. labels Aug 14, 2024
@trop
Copy link
Contributor

trop bot commented Aug 14, 2024

I was unable to backport this PR to "32-x-y" cleanly;
you will need to perform this backport manually.

@trop trop bot added needs-manual-bp/31-x-y and removed target/32-x-y PR should also be added to the "32-x-y" branch. labels Aug 14, 2024
@deepak1556
Copy link
Member Author

Thanks @jkleinsc for rebasing the PR

@trop
Copy link
Contributor

trop bot commented Aug 14, 2024

@deepak1556 has manually backported this PR to "32-x-y", please check out #43317

@trop trop bot added in-flight/32-x-y merged/32-x-y PR was merged to the "32-x-y" branch. and removed needs-manual-bp/32-x-y in-flight/32-x-y labels Aug 14, 2024
LeUser111 pushed a commit to LeUser111/electron that referenced this pull request Feb 11, 2025
…ron#42631)

* feat: support app#login event for utility process net requests

* chore: address review feedback

* GlobalRequestID: Avoid unwanted inlining and narrowing int conversions

Refs https://chromium-review.googlesource.com/c/chromium/src/+/5702737
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-review/approved ✅ merged/32-x-y PR was merged to the "32-x-y" branch. semver/minor backwards-compatible functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.