fix(redirect): Allow credentials for HTTPS redirects in different domains
- Dominant language
- C++
- Stars
- 3.9k
- Forks
- 1k
- Avg merge
- 1d 17h
- Merged PRs (30d)
- 123
Description
## Solution
This patch improves the redirect credential handling to support legitimate SSO scenarios while maintaining security.
### Changes
Replace the overly strict host/port matching with a smarter check:
1. **Allow HTTPS-to-HTTPS redirects** regardless of domain (they're encrypted end-to-end)
2. **Only strip credentials for HTTP redirects** to prevent leaking auth tokens in plaintext
3. **Still prevent HTTPS→HTTP downgrades** (this was already checked at line 251)
### Proposed Fix
```cpp
// Line 279-288: Replace with:
bool isSafeRedirect = (redirectUrl.scheme() == QLatin1String("https"))
&& (requestedUrl.host() == redirectUrl.host()
&& requestedUrl.port() == redirectUrl.port());
if (!isSafeRedirect) {
qCWarning(lcNetworkJob).nospace() << "redirect target mismatches origin or uses HTTP, removing credentials"
<< " origin=" << requestedUrl.scheme() << "://" << requestedUrl.host() << ":" << requestedUrl.port()
<< " target=" << redirectUrl.scheme() << "://" << redirectUrl.host() << ":" << redirectUrl.port();
auto headers = request.headers();
headers.removeAll(QHttpHeaders::WellKnownHeader::Authorization);
request.setHeaders(headers);
request.setAttribute(AbstractCredentials::DontAddCredentialsAttribute, true);
}
```
### Why This Works
- **HTTPS redirects to different domains**: Safe for SSO (transport-layer encryption protects credentials)
- **HTTP redirects**: Credentials removed (plaintext exposure prevented)
- **HTTPS→HTTP downgrades**: Caught by existing check at line 251, request won't reach this point
- **Same-origin redirects**: Keep credentials as before
### Testing
Should test with:
1. OAuth2 redirect to different HTTPS domain ✅ (now works)
2. HTTPS→HTTP redirect ✅ (still blocked)
3. Same-origin redirect ✅ (still works)
4. HTTP→HTTP cross-domain ✅ (credentials removed as before)
Fixes: Regression from commit 585b6ca32dc97c51ede2dad1c002b8a2625e12fc
Contributor guide
Research direction
Start at the network job's redirect credential handling around lines 251 and 279-288. Reconcile the stated HTTPS cross-domain requirement with the proposed host-matching condition, then exercise the four listed redirect scenarios; done means HTTPS SSO redirects retain credentials while HTTP and downgrade cases do not.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- desktop, networking, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 58/100