From 4db17ce24a5f8e0662799b227169639ad2fa6238 Mon Sep 17 00:00:00 2001
From: Lukasz Raczylo <lukasz@raczylo.com>
Date: Wed, 12 Nov 2025 11:17:05 +0000
Subject: [PATCH] Remove redundant validation in auth middleware ( issue #89 )

---
 middleware.go                               | 15 +++--------
 middleware/auth_middleware.go               | 15 +++--------
 middleware/middleware_comprehensive_test.go | 29 +++++++++------------
 3 files changed, 21 insertions(+), 38 deletions(-)

diff --git a/middleware.go b/middleware.go
index fbb0737..9d103e4 100644
--- a/middleware.go
+++ b/middleware.go
@@ -138,17 +138,10 @@ func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 
 	if authenticated && !needsRefresh {
 		t.logger.Debug("User authenticated and token valid, proceeding to process authorized request")
-		if accessToken := session.GetAccessToken(); accessToken != "" {
-			if strings.Count(accessToken, ".") == 2 {
-				if err := t.verifyToken(accessToken); err != nil {
-					t.logger.Errorf("Access token validation failed: %v", err)
-					t.handleExpiredToken(rw, req, session, redirectURL)
-					return
-				}
-			} else {
-				t.logger.Debugf("Access token appears opaque, skipping JWT verification for it.")
-			}
-		}
+		// Access token validation is already performed by provider-specific validation
+		// methods (validateAzureTokens/validateStandardTokens) before reaching this point.
+		// Redundant validation here was causing issues with Azure AD tokens that have
+		// JWT format but unverifiable signatures. See issue #89.
 		t.processAuthorizedRequest(rw, req, session, redirectURL)
 		return
 	}
diff --git a/middleware/auth_middleware.go b/middleware/auth_middleware.go
index b365657..ca85f08 100644
--- a/middleware/auth_middleware.go
+++ b/middleware/auth_middleware.go
@@ -261,17 +261,10 @@ func (m *AuthMiddleware) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 
 	if authenticated && !needsRefresh {
 		m.logger.Debug("User authenticated and token valid, proceeding to process authorized request")
-		if accessToken := session.GetAccessToken(); accessToken != "" {
-			if strings.Count(accessToken, ".") == 2 {
-				if err := m.tokenVerifier.VerifyToken(accessToken); err != nil {
-					m.logger.Errorf("Access token validation failed: %v", err)
-					m.handleExpiredToken(rw, req, session, redirectURL)
-					return
-				}
-			} else {
-				m.logger.Debugf("Access token appears opaque, skipping JWT verification for it.")
-			}
-		}
+		// Access token validation is already performed by provider-specific validation
+		// methods (validateAzureTokens/validateStandardTokens) before reaching this point.
+		// Redundant validation here was causing issues with Azure AD tokens that have
+		// JWT format but unverifiable signatures. See issue #89.
 		m.processAuthorizedRequest(rw, req, session, redirectURL)
 		return
 	}
diff --git a/middleware/middleware_comprehensive_test.go b/middleware/middleware_comprehensive_test.go
index 20c846f..d39b2dc 100644
--- a/middleware/middleware_comprehensive_test.go
+++ b/middleware/middleware_comprehensive_test.go
@@ -730,9 +730,9 @@ func TestServeHTTP_ComprehensiveCoverage(t *testing.T) {
 		}
 	})
 
-	t.Run("jwt_token_validation_failure", func(t *testing.T) {
+	t.Run("authenticated_user_proceeds_to_authorized_request", func(t *testing.T) {
 		logger := &mockLogger{}
-		handleExpiredCalled := false
+		nextHandlerCalled := false
 
 		initComplete := make(chan struct{})
 		close(initComplete)
@@ -741,11 +741,14 @@ func TestServeHTTP_ComprehensiveCoverage(t *testing.T) {
 			logger:       logger,
 			issuerURL:    "https://issuer.example.com",
 			initComplete: initComplete,
+			next: http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+				nextHandlerCalled = true
+			}),
 			sessionManager: &mockSessionManager{
 				getSessionFunc: func(req *http.Request) (SessionData, error) {
 					return &mockSessionData{
 						email:       "user@example.com",
-						accessToken: "invalid.jwt.token", // JWT format (has dots)
+						accessToken: "valid.jwt.token", // JWT format (has dots)
 					}, nil
 				},
 				cleanupOldCookiesFunc: func(rw http.ResponseWriter, req *http.Request) {},
@@ -762,34 +765,28 @@ func TestServeHTTP_ComprehensiveCoverage(t *testing.T) {
 				},
 			},
 			isUserAuthenticatedFunc: func(session SessionData) (bool, bool, bool) {
+				// When authenticated=true, it means provider-specific validation already passed
 				return true, false, false // authenticated, no refresh needed
 			},
 			isAllowedDomainFunc: func(email string) bool {
 				return true
 			},
-			tokenVerifier: &mockTokenVerifier{
-				verifyFunc: func(token string) error {
-					return errors.New("token validation failed")
-				},
+			extractClaimsFunc: func(token string) (map[string]interface{}, error) {
+				return map[string]interface{}{"email": "user@example.com"}, nil
 			},
-			authHandler: &mockAuthHandler{
-				initiateAuthFunc: func(rw http.ResponseWriter, req *http.Request, session SessionData, redirectURL string,
-					genNonce, genVerifier, deriveChallenge func() (string, error)) {
-					handleExpiredCalled = true
-				},
+			extractGroupsAndRolesFunc: func(token string) ([]string, []string, error) {
+				return []string{}, []string{}, nil
 			},
 			firstRequestReceived: true,
 		}
 
-		// We'll track this through the authHandler's InitiateAuthentication call
-
 		req := httptest.NewRequest("GET", "/test", nil)
 		rw := httptest.NewRecorder()
 
 		m.ServeHTTP(rw, req)
 
-		if !handleExpiredCalled {
-			t.Error("Expected handleExpiredToken for invalid JWT")
+		if !nextHandlerCalled {
+			t.Error("Expected next handler to be called when user is authenticated")
 		}
 	})