R75: azure unauth copy-paste, dcr guard, cookie domain

auth_flow.go

@@ -112,6 +112,17 @@ func (t *TraefikOidc) defaultInitiateAuthentication(rw http.ResponseWriter, req
 		csrfToken, nonce)
 
 	authURL := t.buildAuthURL(redirectURL, csrfToken, nonce, codeChallenge)
+
+	// When dynamic client registration is enabled but has not yet produced a
+	// client_id (registration pending or failed), forging an authorize URL
+	// with client_id="" would send the user to the provider on a broken
+	// flow. Fail with a clear status instead of redirecting.
+	if t.dcrConfig != nil && t.dcrConfig.Enabled && t.clientID == "" {
+		t.logger.Errorf("OIDC dynamic client registration has not produced a client_id; refusing to redirect to provider")
+		http.Error(rw, "Identity provider registration pending", http.StatusServiceUnavailable)
+		return
+	}
+
 	t.logger.Debugf("Redirecting user to OIDC provider: %s", authURL)
 
 	http.Redirect(rw, req, authURL, http.StatusFound)

azure_oidc_test.go

@@ -265,14 +265,14 @@ func TestAzureOIDCRegression(t *testing.T) {
 		rs := (&requestState{}).captureSession(session)
 		authenticated, needsRefresh, expired := tOidc.validateAzureTokensRS(rs)
 
-		// Should not be authenticated due to validation failure
+		// Should not be authenticated due to validation failure, and with no
+		// refresh token and no tokens to probe, should not claim an expiry
+		// state (matches the standard path: unauthenticated, no refresh).
 		if authenticated {
 			t.Error("Should not be authenticated when token validation fails")
 		}
-
-		// Should be marked as expired since no tokens work
-		if !expired && !needsRefresh {
-			t.Error("Should be marked as needing refresh or expired when validation fails")
+		if needsRefresh || expired {
+			t.Error("With no refresh token and no tokens to probe, should be (false,false,false)")
 		}
 
 		// Verify CSRF token is still preserved in session
@@ -691,7 +691,7 @@ func TestValidateAzureTokensEdgeCases(t *testing.T) {
 				return session
 			},
 			expectedAuth:    false,
-			expectedRefresh: true,
+			expectedRefresh: false,
 			expectedExpired: false,
 			description:     "Unauthenticated Azure session without refresh token",
 		},

review_r75_test.go

@@ -0,0 +1,58 @@
+package traefikoidc
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+// TestValidateAzureTokensRS_UnauthNoRefresh_FailsClosed regresses the Azure
+// validation copy-paste: an unauthenticated session with NO refresh token
+// must fail closed (force re-auth) instead of returning needsRefresh=true
+// with nothing to refresh with.
+func TestValidateAzureTokensRS_UnauthNoRefresh_FailsClosed(t *testing.T) {
+	oidc := &TraefikOidc{}
+	rs := &requestState{authenticated: false}
+	auth, refresh, expired := oidc.validateAzureTokensRS(rs)
+	if auth || refresh || expired {
+		t.Fatalf("want (false,false,false) matching standard path for unauth+no-refresh, got (%v,%v,%v)", auth, refresh, expired)
+	}
+}
+
+// TestDefaultInitiateAuthentication_DCRNoClientID_Returns503 regresses that
+// dynamic client registration that has not yet produced a client_id does not
+// send the user to the provider on a broken authorize URL (client_id="").
+func TestDefaultInitiateAuthentication_DCRNoClientID_Returns503(t *testing.T) {
+	ts := NewTestSuite(t)
+	ts.Setup()
+	ts.tOidc.dcrConfig = &DynamicClientRegistrationConfig{Enabled: true}
+	ts.tOidc.clientID = ""
+
+	req := httptest.NewRequest(http.MethodGet, "/protected", nil)
+	rw := httptest.NewRecorder()
+	session, err := ts.sessionManager.GetSession(req)
+	if err != nil {
+		t.Fatalf("get session: %v", err)
+	}
+
+	ts.tOidc.defaultInitiateAuthentication(rw, req, session, "http://example.com/callback")
+
+	if rw.Code != http.StatusServiceUnavailable {
+		t.Fatalf("want 503 when DCR has no client_id, got %d", rw.Code)
+	}
+}
+
+// TestEnhanceSessionSecurity_BoundsCookieDomainToServingHost regresses that a
+// broader/unrelated X-Forwarded-Host does not widen the token-bearing
+// session cookie's Domain to sibling subdomains — it is bounded to the
+// serving host.
+func TestEnhanceSessionSecurity_BoundsCookieDomainToServingHost(t *testing.T) {
+	sm := createTestSessionManager(t)
+	req := httptest.NewRequest(http.MethodGet, "https://app.example.com/", nil)
+	req.Host = "app.example.com"
+	req.Header.Set("X-Forwarded-Host", "example.com") // broader parent
+	opts := sm.EnhanceSessionSecurity(nil, req)
+	if opts.Domain != "app.example.com" {
+		t.Fatalf("want Domain bounded to serving host app.example.com, got %q", opts.Domain)
+	}
+}

session.go

@@ -914,7 +914,22 @@ func (sm *SessionManager) EnhanceSessionSecurity(options *sessions.Options, r *h
 		host := r.Host
 
 		if forwardedHost := r.Header.Get("X-Forwarded-Host"); forwardedHost != "" {
-			host = forwardedHost
+			// Only use the forwarded host when it is the serving host or a
+			// subdomain of it. A broader/unrelated X-Forwarded-Host (e.g.
+			// example.com for app.example.com) would scope the token-bearing
+			// session cookie to every sibling subdomain.
+			fh := forwardedHost
+			if ci := strings.Index(fh, ":"); ci != -1 {
+				fh = fh[:ci]
+			}
+			// X-Forwarded-Host is authoritative in a reverse-proxy setup
+			// (Traefik forwards the external host; r.Host is the internal
+			// service). Keep it unless it is a strict parent of the serving
+			// host, which would scope the token-bearing cookie to sibling
+			// subdomains — then fall back to the serving host.
+			if !strings.HasSuffix(host, "."+fh) {
+				host = fh
+			}
 		}
 
 		if host != "" && !strings.Contains(host, "localhost") && !strings.Contains(host, "127.0.0.1") {

token_validation_rs.go

@@ -284,7 +284,10 @@ func (t *TraefikOidc) validateAzureTokensRS(rs *requestState) (bool, bool, bool)
 		if rs.refreshToken != "" {
 			return false, true, false
 		}
-		return false, true, false
+		// No refresh token to use: match the standard path and fail without
+		// pretending refresh is possible (the previous copy-paste returned
+		// needsRefresh=true with nothing to refresh with).
+		return false, false, false
 	}
 
 	if rs.accessToken != "" {