Skip to content

Commit 6d5ad4e

Browse files
fix(service/auth): emit WWW-Authenticate DPoP on all proof rejections (DSPX-3397)
Server-side DPoP validation only attached a WWW-Authenticate header for DPoPNonceError (use_dpop_nonce). Every other proof rejection (tampered htu, replayed jti, malformed nonce, bad ath, wrong htm) returned a bare 401 with no challenge header, so RFC 9449 §7.1-compliant clients/tests could not tell a DPoP failure from an unrelated 401. This made the opentdf/tests xtest negative cases unreliable: tampered_htu failed always (htu is checked before nonce), replayed_jti was flaky (gated on whether the cached nonce had rotated, deciding whether the nonce or jti check fired first), and tampered_nonce failed whenever a non-nonce check tripped first. Add a DPoPProofError marker type (delegating Error, Unwrap), wrap all non-nonce validateDPoP errors with it in checkToken, and have both MuxHandler and ConnectAuthNInterceptor emit `WWW-Authenticate: DPoP error="invalid_dpop_proof"` (plus a fresh DPoP-Nonce when require_nonce is on). The use_dpop_nonce path is unchanged. Add unit tests covering the error type, the malformed-nonce wrap contract, and the challenge headers for both handlers across nonce-on/off. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
1 parent 2a7095a commit 6d5ad4e

3 files changed

Lines changed: 177 additions & 4 deletions

File tree

service/internal/auth/authn.go

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,17 @@ func (e *DPoPNonceMalformedError) Error() string {
377377
return e.Message
378378
}
379379

380+
// DPoPProofError marks a non-retryable DPoP proof rejection (tampered htu/htm,
381+
// bad ath, replayed jti, malformed nonce). Handlers translate it into a
382+
// WWW-Authenticate: DPoP error="invalid_dpop_proof" challenge per RFC 9449 §7.1.
383+
type DPoPProofError struct {
384+
err error
385+
}
386+
387+
func (e *DPoPProofError) Error() string { return e.err.Error() }
388+
389+
func (e *DPoPProofError) Unwrap() error { return e.err }
390+
380391
func normalizeURL(o string, u *url.URL) string {
381392
// Currently this does not do a full normatlization
382393
ou, err := url.Parse(o)
@@ -471,6 +482,22 @@ func (a Authentication) MuxHandler(handler http.Handler) http.Handler {
471482
http.Error(w, "unauthenticated", http.StatusUnauthorized)
472483
return
473484
}
485+
// Other DPoP proof failures get an invalid_dpop_proof challenge (RFC 9449 §7.1).
486+
var proofErr *DPoPProofError
487+
if errors.As(err, &proofErr) {
488+
if a.dpopNonceManager.requireNonce {
489+
w.Header().Set("DPoP-Nonce", a.dpopNonceManager.getCurrentNonce())
490+
}
491+
w.Header().Set("WWW-Authenticate", `DPoP error="invalid_dpop_proof"`)
492+
log.WarnContext(
493+
ctxWithAuthX,
494+
"unauthenticated",
495+
slog.Any("error", err),
496+
slog.Any("dpop", dp),
497+
)
498+
http.Error(w, "unauthenticated", http.StatusUnauthorized)
499+
return
500+
}
474501
log.WarnContext(
475502
ctxWithAuthX,
476503
"unauthenticated",
@@ -616,6 +643,16 @@ func (a Authentication) ConnectAuthNInterceptor() connect.UnaryInterceptorFunc {
616643
connectErr.Meta().Set("WWW-Authenticate", `DPoP error="use_dpop_nonce"`)
617644
return nil, connectErr
618645
}
646+
// Other DPoP proof failures get an invalid_dpop_proof challenge (RFC 9449 §7.1).
647+
var proofErr *DPoPProofError
648+
if errors.As(err, &proofErr) {
649+
connectErr := connect.NewError(connect.CodeUnauthenticated, errors.New("unauthenticated"))
650+
if a.dpopNonceManager.requireNonce {
651+
connectErr.Meta().Set("DPoP-Nonce", a.dpopNonceManager.getCurrentNonce())
652+
}
653+
connectErr.Meta().Set("WWW-Authenticate", `DPoP error="invalid_dpop_proof"`)
654+
return nil, connectErr
655+
}
619656
return nil, connect.NewError(connect.CodeUnauthenticated, errors.New("unauthenticated"))
620657
}
621658

@@ -1061,11 +1098,14 @@ func (a *Authentication) checkToken(ctx context.Context, authHeader []string, dp
10611098
if err != nil {
10621099
var nonceErr *DPoPNonceError
10631100
if errors.As(err, &nonceErr) {
1101+
// Retryable nonce challenge: returned unwrapped so handlers issue use_dpop_nonce.
10641102
a.logger.DebugContext(ctx, "dpop nonce challenge issued", slog.String("reason", nonceErr.Message))
1065-
} else {
1066-
a.logger.WarnContext(ctx, "failed to validate dpop", slog.Any("err", err))
1103+
return nil, nil, err
10671104
}
1068-
return nil, nil, err
1105+
// Any other DPoP proof failure (tampered htu/htm, bad ath, replayed jti,
1106+
// malformed nonce) becomes an invalid_dpop_proof challenge.
1107+
a.logger.WarnContext(ctx, "failed to validate dpop", slog.Any("err", err))
1108+
return nil, nil, &DPoPProofError{err: err}
10691109
}
10701110
ctx = ctxAuth.ContextWithAuthNInfo(ctx, dpopKey, accessToken, tokenRaw)
10711111
return accessToken, ctx, nil

service/internal/auth/authn_test.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -844,6 +844,61 @@ func (s *AuthSuite) Test_ConnectAuthNInterceptor_RequiresHeaderWithExistingConte
844844
s.Require().ErrorAs(err, &connectErr)
845845
}
846846

847+
func (s *AuthSuite) Test_MuxHandler_DPoPProofError_IssuesInvalidProofChallenge() {
848+
auth := s.newAuthDPoP(false)
849+
rec := s.muxAuthErrorRecorder(auth, &DPoPProofError{err: errors.New("incorrect `htu` claim in DPoP JWT")})
850+
851+
s.Equal(http.StatusUnauthorized, rec.Code)
852+
s.Equal(`DPoP error="invalid_dpop_proof"`, rec.Header().Get("WWW-Authenticate"))
853+
// Without RequireNonce there is nothing to retry against, so no nonce is issued.
854+
s.Empty(rec.Header().Get("DPoP-Nonce"))
855+
}
856+
857+
func (s *AuthSuite) Test_MuxHandler_DPoPProofError_IncludesNonceWhenRequired() {
858+
auth := s.newAuthDPoP(true)
859+
rec := s.muxAuthErrorRecorder(auth, &DPoPProofError{err: errors.New("DPoP proof replay detected")})
860+
861+
s.Equal(http.StatusUnauthorized, rec.Code)
862+
s.Equal(`DPoP error="invalid_dpop_proof"`, rec.Header().Get("WWW-Authenticate"))
863+
s.NotEmpty(rec.Header().Get("DPoP-Nonce"), "a fresh nonce aids the client's retry")
864+
}
865+
866+
func (s *AuthSuite) Test_MuxHandler_DPoPNonceError_IssuesUseNonceChallenge() {
867+
auth := s.newAuthDPoP(true)
868+
rec := s.muxAuthErrorRecorder(auth, &DPoPNonceError{Message: "nonce required for retry"})
869+
870+
s.Equal(http.StatusUnauthorized, rec.Code)
871+
s.Equal(`DPoP error="use_dpop_nonce"`, rec.Header().Get("WWW-Authenticate"))
872+
s.NotEmpty(rec.Header().Get("DPoP-Nonce"))
873+
}
874+
875+
func (s *AuthSuite) Test_ConnectAuthNInterceptor_DPoPProofError_IssuesInvalidProofChallenge() {
876+
auth := s.newAuthDPoP(false)
877+
connectErr := s.connectAuthError(auth, &DPoPProofError{err: errors.New("incorrect `htu` claim in DPoP JWT")})
878+
879+
s.Equal(connect.CodeUnauthenticated, connectErr.Code())
880+
s.Equal(`DPoP error="invalid_dpop_proof"`, connectErr.Meta().Get("WWW-Authenticate"))
881+
s.Empty(connectErr.Meta().Get("DPoP-Nonce"))
882+
}
883+
884+
func (s *AuthSuite) Test_ConnectAuthNInterceptor_DPoPProofError_IncludesNonceWhenRequired() {
885+
auth := s.newAuthDPoP(true)
886+
connectErr := s.connectAuthError(auth, &DPoPProofError{err: errors.New("DPoP proof replay detected")})
887+
888+
s.Equal(connect.CodeUnauthenticated, connectErr.Code())
889+
s.Equal(`DPoP error="invalid_dpop_proof"`, connectErr.Meta().Get("WWW-Authenticate"))
890+
s.NotEmpty(connectErr.Meta().Get("DPoP-Nonce"), "a fresh nonce aids the client's retry")
891+
}
892+
893+
func (s *AuthSuite) Test_ConnectAuthNInterceptor_DPoPNonceError_IssuesUseNonceChallenge() {
894+
auth := s.newAuthDPoP(true)
895+
connectErr := s.connectAuthError(auth, &DPoPNonceError{Message: "nonce required for retry"})
896+
897+
s.Equal(connect.CodeUnauthenticated, connectErr.Code())
898+
s.Equal(`DPoP error="use_dpop_nonce"`, connectErr.Meta().Get("WWW-Authenticate"))
899+
s.NotEmpty(connectErr.Meta().Get("DPoP-Nonce"))
900+
}
901+
847902
func (s *AuthSuite) Test_CheckToken_When_Authorization_Header_Invalid_Expect_Error() {
848903
_, _, err := s.auth.checkToken(context.Background(), []string{"BPOP "}, receiverInfo{}, nil)
849904
s.Require().Error(err)
@@ -1441,6 +1496,48 @@ func (s *AuthSuite) Test_RoleRequestForConnectProcedure() {
14411496
}
14421497
}
14431498

1499+
// dpopChallengeRoute is a non-public route used by the DPoP challenge handler tests.
1500+
// checkToken is stubbed in those tests, so the exact procedure value is irrelevant.
1501+
const dpopChallengeRoute = "/dpop.test/Challenge"
1502+
1503+
// muxAuthErrorRecorder drives MuxHandler with checkToken stubbed to return retErr and
1504+
// returns the recorded response. The request carries DPoP Authorization + proof headers.
1505+
func (s *AuthSuite) muxAuthErrorRecorder(auth *Authentication, retErr error) *httptest.ResponseRecorder {
1506+
auth._testCheckTokenFunc = func(context.Context, []string, receiverInfo, []string) (jwt.Token, context.Context, error) {
1507+
return nil, nil, retErr
1508+
}
1509+
handler := auth.MuxHandler(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}))
1510+
rec := httptest.NewRecorder()
1511+
req := httptest.NewRequest(http.MethodPost, dpopChallengeRoute, nil)
1512+
req.Header.Set("Authorization", "DPoP token")
1513+
req.Header.Set("DPoP", "proof")
1514+
handler.ServeHTTP(rec, req)
1515+
return rec
1516+
}
1517+
1518+
// connectAuthError drives ConnectAuthNInterceptor with checkToken stubbed to return retErr
1519+
// and returns the resulting *connect.Error.
1520+
func (s *AuthSuite) connectAuthError(auth *Authentication, retErr error) *connect.Error {
1521+
auth._testCheckTokenFunc = func(context.Context, []string, receiverInfo, []string) (jwt.Token, context.Context, error) {
1522+
return nil, nil, retErr
1523+
}
1524+
interceptor := auth.ConnectAuthNInterceptor()
1525+
next := func(context.Context, connect.AnyRequest) (connect.AnyResponse, error) {
1526+
return connect.NewResponse(&kas.RewrapResponse{}), nil
1527+
}
1528+
req := &authnTestRequest{
1529+
Request: connect.NewRequest(&kas.RewrapRequest{}),
1530+
procedure: dpopChallengeRoute,
1531+
}
1532+
req.Header().Set("Authorization", "DPoP token")
1533+
req.Header().Set("DPoP", "proof")
1534+
_, err := interceptor(next)(s.T().Context(), req)
1535+
s.Require().Error(err)
1536+
var connectErr *connect.Error
1537+
s.Require().ErrorAs(err, &connectErr)
1538+
return connectErr
1539+
}
1540+
14441541
func Test_GetClientIDFromToken(t *testing.T) {
14451542
tests := []struct {
14461543
name string

service/internal/auth/dpop_nonce_test.go

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"crypto/rsa"
88
"crypto/sha256"
99
"encoding/base64"
10+
"errors"
1011
"net/http"
1112
"testing"
1213
"time"
@@ -103,6 +104,31 @@ func TestDPoPNonceError(t *testing.T) {
103104
})
104105
}
105106

107+
func TestDPoPProofError(t *testing.T) {
108+
inner := errors.New("incorrect `htu` claim in DPoP JWT")
109+
var err error = &DPoPProofError{err: inner}
110+
111+
t.Run("error message delegates to inner", func(t *testing.T) {
112+
// Handlers/tests rely on substring matching of the underlying error.
113+
assert.Equal(t, inner.Error(), err.Error())
114+
})
115+
116+
t.Run("unwraps to inner error", func(t *testing.T) {
117+
assert.ErrorIs(t, err, inner)
118+
})
119+
120+
t.Run("detected via errors.As", func(t *testing.T) {
121+
var proofErr *DPoPProofError
122+
require.ErrorAs(t, err, &proofErr)
123+
})
124+
125+
t.Run("does not match DPoPNonceError", func(t *testing.T) {
126+
// A wrapped non-nonce failure must not be mistaken for a retryable challenge.
127+
var nonceErr *DPoPNonceError
128+
assert.NotErrorAs(t, err, &nonceErr)
129+
})
130+
}
131+
106132
func TestDPoPAlgorithmRestrictions(t *testing.T) {
107133
testCases := []struct {
108134
alg jwa.SignatureAlgorithm
@@ -131,6 +157,12 @@ func TestDPoPAlgorithmRestrictions(t *testing.T) {
131157

132158
// newAuthWithNonce creates an Authentication using the suite's OIDC server with RequireNonce=true.
133159
func (s *AuthSuite) newAuthWithNonce() *Authentication {
160+
return s.newAuthDPoP(true)
161+
}
162+
163+
// newAuthDPoP creates a DPoP-enforcing Authentication backed by the suite's OIDC server,
164+
// with the nonce challenge toggled by requireNonce.
165+
func (s *AuthSuite) newAuthDPoP(requireNonce bool) *Authentication {
134166
auth, err := NewAuthenticator(
135167
context.Background(),
136168
Config{
@@ -141,7 +173,7 @@ func (s *AuthSuite) newAuthWithNonce() *Authentication {
141173
DPoPSkew: time.Hour,
142174
TokenSkew: time.Minute,
143175
DPoP: DPoPConfig{
144-
RequireNonce: true,
176+
RequireNonce: requireNonce,
145177
NonceExpiration: 5 * time.Minute,
146178
StrictHTU: false,
147179
},
@@ -285,6 +317,10 @@ func (s *AuthSuite) TestDPoP_MalformedNonce_Returns_DPoPNonceMalformedError() {
285317
// Confirm it does NOT match DPoPNonceError, so handlers hard-reject rather than issue a challenge.
286318
var nonceErr *DPoPNonceError
287319
s.Require().NotErrorAs(err, &nonceErr)
320+
321+
// checkToken wraps it in DPoPProofError so handlers issue an invalid_dpop_proof challenge.
322+
var proofErr *DPoPProofError
323+
s.Require().ErrorAs(err, &proofErr)
288324
}
289325

290326
func (s *AuthSuite) TestDPoP_WrongNonce_Returns_DPoPNonceError() {

0 commit comments

Comments
 (0)