-
Notifications
You must be signed in to change notification settings - Fork 37.4k
Do not allow localhost redirect in favor of loopback IP redirect #267546
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
Do not allow localhost redirect in favor of loopback IP redirect #267546
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.
Pull Request Overview
This PR removes localhost redirect URLs from OAuth configuration in favor of loopback IP (127.0.0.1) redirect URLs only. This change improves security by preventing potential localhost-based attacks while maintaining the same functionality through the more secure loopback IP addresses.
Key Changes
- Removes
http://localhost/andhttp://localhost:${DEFAULT_AUTH_FLOW_PORT}/from OAuth redirect URI configurations - Retains
http://127.0.0.1/andhttp://127.0.0.1:${DEFAULT_AUTH_FLOW_PORT}/for local development authentication flows - Updates corresponding test assertions to match the new configuration
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/vs/base/common/oauth.ts | Removes localhost redirect URIs from dynamic registration, keeping only loopback IP addresses |
| src/vs/base/test/common/oauth.test.ts | Updates test expectations to match the removal of localhost redirect URIs |
|
Addresses #267545 |
@microsoft-github-policy-service agree |
TylerLeonhardt
left a comment
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.
LGTM. We only use 127.0.0.1 when we spin up the local server so this makes sense.
Fixes #267545