mirror of
https://github.com/lukaszraczylo/traefikoidc.git
synced 2026-10-14 04:07:46 +00:00
Fix bearer Authorization template strip order + nonce constant-time compare (round 45)
This commit is contained in:
+1
-1
@@ -233,7 +233,7 @@ func (t *TraefikOidc) handleCallback(rw http.ResponseWriter, req *http.Request,
|
||||
return
|
||||
}
|
||||
|
||||
if nonceClaim != sessionNonce {
|
||||
if !constantTimeStringCompare(nonceClaim, sessionNonce) {
|
||||
t.logger.Error("Nonce claim does not match session nonce during callback")
|
||||
t.sendErrorResponse(rw, req, "Authentication failed: Nonce mismatch", http.StatusInternalServerError)
|
||||
return
|
||||
|
||||
+10
-7
@@ -788,6 +788,16 @@ func (t *TraefikOidc) forwardAuthorized(rw http.ResponseWriter, req *http.Reques
|
||||
}
|
||||
}
|
||||
|
||||
// Bearer source: strip the raw inbound Authorization header to keep the
|
||||
// token out of downstream service logs (off-by-config for operators who
|
||||
// chain services that each re-verify the bearer). Stripped here, before
|
||||
// header-template application, so an operator-supplied Authorization
|
||||
// template (e.g. "Authorization: Bearer {{.AccessToken}}") can re-supply
|
||||
// the downstream value rather than being immediately deleted.
|
||||
if p.Source == sourceBearer && t.stripAuthorizationHeader {
|
||||
req.Header.Del("Authorization")
|
||||
}
|
||||
|
||||
if len(t.headerTemplates) > 0 {
|
||||
// p.Claims may be nil (e.g. session without an ID token). Templates
|
||||
// referencing .Claims.* will simply produce empty values — matches
|
||||
@@ -853,13 +863,6 @@ func (t *TraefikOidc) forwardAuthorized(rw http.ResponseWriter, req *http.Reques
|
||||
}
|
||||
}
|
||||
|
||||
// Bearer source: strip the Authorization header to keep the raw token
|
||||
// out of downstream service logs. Off-by-config for operators who chain
|
||||
// services that each re-verify the bearer.
|
||||
if p.Source == sourceBearer && t.stripAuthorizationHeader {
|
||||
req.Header.Del("Authorization")
|
||||
}
|
||||
|
||||
t.logger.Debugf("Request authorized for user %s (source=%d), forwarding to next handler", p.Identifier, p.Source)
|
||||
|
||||
t.next.ServeHTTP(rw, req)
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
package traefikoidc
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
"text/template"
|
||||
)
|
||||
|
||||
// TestForwardAuthorized_BearerStripPreservesTemplatedAuthorization is a
|
||||
// regression test for the order in forwardAuthorized between the bearer
|
||||
// Authorization strip and header-template application. Previously the strip
|
||||
// (when source==sourceBearer && StripAuthorizationHeader, the default) ran
|
||||
// AFTER template application, deleting an operator-supplied Authorization
|
||||
// template (e.g. "Authorization: Bearer {{.AccessToken}}") and defeating
|
||||
// its purpose. The strip must run first so an explicit template survives.
|
||||
func TestForwardAuthorized_BearerStripPreservesTemplatedAuthorization(t *testing.T) {
|
||||
var forwarded *http.Request
|
||||
next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
forwarded = r.Clone(r.Context())
|
||||
w.WriteHeader(http.StatusOK)
|
||||
})
|
||||
|
||||
authTpl := template.Must(template.New("auth").Parse("Bearer {{.AccessToken}}"))
|
||||
oidc := &TraefikOidc{
|
||||
logger: NewLogger("debug"),
|
||||
next: next,
|
||||
stripAuthorizationHeader: true,
|
||||
headerTemplates: map[string]*template.Template{
|
||||
"Authorization": authTpl,
|
||||
},
|
||||
}
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
|
||||
req.Header.Set("Authorization", "Bearer raw-inbound-token")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
oidc.forwardAuthorized(rec, req, &principal{
|
||||
Source: sourceBearer,
|
||||
AccessToken: "access-token-x",
|
||||
})
|
||||
|
||||
d := forwarded.Header.Get("Authorization")
|
||||
// The operator's templated value must survive, replacing the raw inbound
|
||||
// token that would otherwise leak into downstream logs.
|
||||
if d != "Bearer access-token-x" {
|
||||
t.Fatalf("Authorization = %q, want %q", d, "Bearer access-token-x")
|
||||
}
|
||||
}
|
||||
|
||||
// TestForwardAuthorized_BearerStripDefault is the regression-guard for the
|
||||
// default config (strip on, no Authorization template): the raw inbound
|
||||
// bearer header must still be removed before forwarding.
|
||||
func TestForwardAuthorized_BearerStripDefault(t *testing.T) {
|
||||
var forwarded *http.Request
|
||||
next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
forwarded = r.Clone(r.Context())
|
||||
w.WriteHeader(http.StatusOK)
|
||||
})
|
||||
|
||||
oidc := &TraefikOidc{
|
||||
logger: NewLogger("debug"),
|
||||
next: next,
|
||||
stripAuthorizationHeader: true,
|
||||
}
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
|
||||
req.Header.Set("Authorization", "Bearer raw-inbound-token")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
oidc.forwardAuthorized(rec, req, &principal{Source: sourceBearer})
|
||||
|
||||
if v := forwarded.Header.Get("Authorization"); v != "" {
|
||||
t.Fatalf("Authorization = %q, want empty (stripped)", v)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user