Repository navigation
fix: evaluate the cross-origin redirect marker before AuthStep's HTTPS guard - #83
Merged
Merged
Conversation
…guard The AUTH stage runs inside the REDIRECT stage, so a redirect re-issue flows back through AuthStep.process. When DefaultRedirectStep follows a cross-origin redirect it strips the Authorization header and stamps the request with the internal cross-origin marker so AuthStep will not re-apply a credential to the foreign host. With allowSchemeDowngrade enabled, that hop can target a plaintext http host. The HTTPS scheme guard ran before the marker check, so a marker-suppressed re-issue to an http host threw IllegalStateException before the suppression branch could run — turning a redirect the redirect step deliberately allowed into a hard failure, and leaving the credential-free marker path dead for any non-https URL. Reorder process so the marker is evaluated first: a marked request is forwarded credential-free over any scheme (no credential is attached, so the plaintext leak the guard protects against cannot occur). The HTTPS guard now applies only on the credential-stamping branch, so a genuine plaintext authenticated request is still rejected. Internal method-body and KDoc change only; no public API change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AuthStep.processapplied its HTTPS-only guard before checking the cross-origin redirect marker. The redirect step deliberately follows some cross-origin hops to a plaintext host — most notably an opted-inallowSchemeDowngradeHTTPS→HTTP downgrade — and marks the re-issued request soAuthStepwill forward it without re-stamping the caller's credential onto a foreign host.Because the HTTPS guard ran first and unconditionally, a marked re-issue whose target was a plaintext host threw
IllegalStateExceptionbefore the marker-suppression branch could run. A redirect the redirect step had intentionally allowed became a hard failure, and the credential-free marker path was effectively dead code for any non-HTTPS URL.Change
The cross-origin marker is now evaluated first. A marked request is forwarded credential-free over any scheme (with the marker stripped so it never reaches the wire): no credential is attached on this path, so the plaintext credential leak the guard exists to prevent cannot occur. The HTTPS guard now applies only on the credential-stamping branch, so a genuine plaintext authenticated request still fails closed.
Tests
httphost (viaallowSchemeDowngrade) is now forwarded without throwing, carries neither theAuthorizationheader nor the internal marker, and reaches the downgraded host.httprequest that would stamp a credential is still rejected by the HTTPS guard before any bytes are written (guarding against opening a real credential-leak path).Internal change only — no public API change.
Closes #9