Compare commits

...
Author SHA1 Message Date
KRATOS 0919a848d1 fix(oauth): refuse redirects on credential POSTs (#729)
Token exchange/refresh and the device authorization/poll POSTs validated only
the initial endpoint, then let the http.Client follow a 307/308 that replays the
form body (code, PKCE verifier, refresh_token, client_secret) to an unvalidated
origin.

Add withoutRedirects(): a shallow client copy whose CheckRedirect refuses every
redirect (ErrUnsafeRedirect). Applied at PostToken (covers ExchangeCode, Refresh,
and MCP which delegate to it), RequestDeviceCode, and pollDeviceOnce. The caller's
client is never mutated; legitimate token endpoints that return 200 are unaffected.
2026-07-19 18:16:13 +05:30
5 changed files with 116 additions and 9 deletions

No files matched your search

+6 -6
View File
@@ -49,9 +49,9 @@ func RequestDeviceCode(ctx context.Context, client *http.Client, cfg Config, now
if err := validateTokenEndpoint(endpoint); err != nil {
return DeviceAuth{}, err
}
if client == nil {
client = http.DefaultClient
}
// Refuse redirects so a device-authorization redirect can't replay the
// client_id/client_secret POST body to an unvalidated origin.
client = withoutRedirects(client)
if now == nil {
now = time.Now
}
@@ -157,9 +157,9 @@ func pollDeviceOnce(ctx context.Context, client *http.Client, cfg Config, device
if err := validateTokenEndpoint(cfg.TokenEndpoint); err != nil {
return Token{}, err
}
if client == nil {
client = http.DefaultClient
}
// Refuse redirects so a poll redirect can't replay the device_code/secret
// POST body to an unvalidated origin.
client = withoutRedirects(client)
form := url.Values{}
form.Set("grant_type", deviceGrantType)
form.Set("device_code", deviceCode)
+25
View File
@@ -2,6 +2,7 @@ package oauth
import (
"context"
"errors"
"net/http"
"net/http/httptest"
"strings"
@@ -28,6 +29,30 @@ func TestRequestDeviceCodeSendsClientSecret(t *testing.T) {
}
}
func TestRequestDeviceCodeRefusesRedirect(t *testing.T) {
endpoint, hits := redirectTrap(t, http.StatusTemporaryRedirect) // 307
_, err := RequestDeviceCode(context.Background(), http.DefaultClient,
Config{ClientID: "c", ClientSecret: "shh", DeviceAuthorizationEndpoint: endpoint}, nil)
if !errors.Is(err, ErrUnsafeRedirect) {
t.Fatalf("RequestDeviceCode err = %v, want ErrUnsafeRedirect", err)
}
if n := hits(); n != 0 {
t.Fatalf("attacker received %d request(s) — client_secret replayed", n)
}
}
func TestPollDeviceOnceRefusesRedirect(t *testing.T) {
endpoint, hits := redirectTrap(t, http.StatusPermanentRedirect) // 308
_, err := pollDeviceOnce(context.Background(), http.DefaultClient,
Config{ClientID: "c", ClientSecret: "shh", TokenEndpoint: endpoint}, "device-code", nil)
if !errors.Is(err, ErrUnsafeRedirect) {
t.Fatalf("pollDeviceOnce err = %v, want ErrUnsafeRedirect", err)
}
if n := hits(); n != 0 {
t.Fatalf("attacker received %d request(s) — device_code/secret replayed", n)
}
}
func TestRequestDeviceCode(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
_, _ = w.Write([]byte(`{"device_code":"dc","user_code":"WXYZ-1234","verification_uri":"https://x/dev","verification_uri_complete":"https://x/dev?c=WXYZ","expires_in":600,"interval":3}`))
+20 -3
View File
@@ -116,6 +116,23 @@ func validateTokenEndpoint(endpoint string) error {
return ValidateEndpointURL(endpoint)
}
// withoutRedirects returns a shallow copy of client whose redirect policy
// refuses to follow ANY redirect. A credential-bearing OAuth POST (token
// exchange/refresh, device authorization, device-token poll) must never let a
// 307/308 replay its form body — codes, PKCE verifiers, refresh tokens, client
// secrets — to a target that was never endpoint-validated. The caller's client
// is left unmodified.
func withoutRedirects(client *http.Client) *http.Client {
if client == nil {
client = http.DefaultClient
}
copied := *client
copied.CheckRedirect = func(*http.Request, []*http.Request) error {
return ErrUnsafeRedirect
}
return &copied
}
func isLoopbackHost(host string) bool {
if host == "localhost" {
return true
@@ -176,9 +193,9 @@ func PostToken(ctx context.Context, client *http.Client, tokenEndpoint string, f
if err := validateTokenEndpoint(tokenEndpoint); err != nil {
return Token{}, err
}
if client == nil {
client = http.DefaultClient
}
// Refuse redirects so a 307/308 from the token endpoint can't replay this
// credential-bearing POST body to another (unvalidated) origin.
client = withoutRedirects(client)
if now == nil {
now = time.Now
}
+60
View File
@@ -5,10 +5,12 @@ import (
"crypto/sha256"
"encoding/base64"
"errors"
"io"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"sync/atomic"
"testing"
"time"
)
@@ -126,6 +128,64 @@ func TestBuildAuthorizationURLRejectsInsecureEndpoint(t *testing.T) {
}
}
// redirectTrap starts an "attacker" server that counts any hit, and a credential
// endpoint that redirects to it with the given status. It returns the endpoint
// URL and a func reporting how many times the attacker was contacted — a
// non-zero count means the credential-bearing POST body was replayed (#729).
func redirectTrap(t *testing.T, status int) (endpoint string, attackerHits func() int32) {
t.Helper()
var hits int32
attacker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
atomic.AddInt32(&hits, 1)
_, _ = io.WriteString(w, `{"access_token":"leaked","refresh_token":"leaked"}`)
}))
t.Cleanup(attacker.Close)
endp := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, attacker.URL, status)
}))
t.Cleanup(endp.Close)
return endp.URL, func() int32 { return atomic.LoadInt32(&hits) }
}
func TestPostTokenRefusesRedirect(t *testing.T) {
endpoint, hits := redirectTrap(t, http.StatusTemporaryRedirect) // 307
form := url.Values{}
form.Set("grant_type", "authorization_code")
form.Set("code", "the-code")
form.Set("client_secret", "shh")
_, err := PostToken(context.Background(), http.DefaultClient, endpoint, form, Token{}, nil)
if !errors.Is(err, ErrUnsafeRedirect) {
t.Fatalf("PostToken err = %v, want ErrUnsafeRedirect", err)
}
if n := hits(); n != 0 {
t.Fatalf("attacker received %d request(s) — credential body was replayed", n)
}
}
func TestExchangeCodeRefusesRedirect(t *testing.T) {
endpoint, hits := redirectTrap(t, http.StatusPermanentRedirect) // 308
_, err := ExchangeCode(context.Background(), http.DefaultClient,
Config{ClientID: "c", TokenEndpoint: endpoint}, "the-code", "verifier", "http://127.0.0.1/cb", nil)
if !errors.Is(err, ErrUnsafeRedirect) {
t.Fatalf("ExchangeCode err = %v, want ErrUnsafeRedirect", err)
}
if n := hits(); n != 0 {
t.Fatalf("attacker received %d request(s) — code/verifier replayed", n)
}
}
func TestRefreshRefusesRedirect(t *testing.T) {
endpoint, hits := redirectTrap(t, http.StatusTemporaryRedirect) // 307
_, err := Refresh(context.Background(), http.DefaultClient,
Config{ClientID: "c", TokenEndpoint: endpoint}, Token{RefreshToken: "rt"}, nil)
if !errors.Is(err, ErrUnsafeRedirect) {
t.Fatalf("Refresh err = %v, want ErrUnsafeRedirect", err)
}
if n := hits(); n != 0 {
t.Fatalf("attacker received %d request(s) — refresh_token replayed", n)
}
}
func TestValidateTokenEndpoint(t *testing.T) {
cases := map[string]bool{
"https://auth.example.com/token": true,
+5
View File
@@ -90,6 +90,11 @@ var (
// ErrInsecureTokenEndpoint is returned when a credential would be sent over a
// non-https, non-loopback endpoint.
ErrInsecureTokenEndpoint = errors.New("oauth: refusing to send credential to a non-https token endpoint")
// ErrUnsafeRedirect is returned when a credential-bearing OAuth POST (token
// exchange/refresh, device authorization, or device-token poll) is redirected.
// Following it would replay the form body — codes, PKCE verifiers, refresh
// tokens, client secrets — to a target that never passed endpoint validation.
ErrUnsafeRedirect = errors.New("oauth: refusing to follow a redirect from a credential endpoint")
// ErrNoRefreshToken is returned when a refresh is attempted without one.
ErrNoRefreshToken = errors.New("oauth: no refresh token available")
// ErrAuthorizationPending is the RFC 8628 "keep polling" signal.