mirror of
https://github.com/lukaszraczylo/traefikoidc.git
synced 2026-10-19 04:47:48 +00:00
Remove redundant validation in auth middleware ( issue #89 )
This commit is contained in:
+4
-11
@@ -138,17 +138,10 @@ func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
|
|||||||
|
|
||||||
if authenticated && !needsRefresh {
|
if authenticated && !needsRefresh {
|
||||||
t.logger.Debug("User authenticated and token valid, proceeding to process authorized request")
|
t.logger.Debug("User authenticated and token valid, proceeding to process authorized request")
|
||||||
if accessToken := session.GetAccessToken(); accessToken != "" {
|
// Access token validation is already performed by provider-specific validation
|
||||||
if strings.Count(accessToken, ".") == 2 {
|
// methods (validateAzureTokens/validateStandardTokens) before reaching this point.
|
||||||
if err := t.verifyToken(accessToken); err != nil {
|
// Redundant validation here was causing issues with Azure AD tokens that have
|
||||||
t.logger.Errorf("Access token validation failed: %v", err)
|
// JWT format but unverifiable signatures. See issue #89.
|
||||||
t.handleExpiredToken(rw, req, session, redirectURL)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
t.logger.Debugf("Access token appears opaque, skipping JWT verification for it.")
|
|
||||||
}
|
|
||||||
}
|
|
||||||
t.processAuthorizedRequest(rw, req, session, redirectURL)
|
t.processAuthorizedRequest(rw, req, session, redirectURL)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -261,17 +261,10 @@ func (m *AuthMiddleware) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
|
|||||||
|
|
||||||
if authenticated && !needsRefresh {
|
if authenticated && !needsRefresh {
|
||||||
m.logger.Debug("User authenticated and token valid, proceeding to process authorized request")
|
m.logger.Debug("User authenticated and token valid, proceeding to process authorized request")
|
||||||
if accessToken := session.GetAccessToken(); accessToken != "" {
|
// Access token validation is already performed by provider-specific validation
|
||||||
if strings.Count(accessToken, ".") == 2 {
|
// methods (validateAzureTokens/validateStandardTokens) before reaching this point.
|
||||||
if err := m.tokenVerifier.VerifyToken(accessToken); err != nil {
|
// Redundant validation here was causing issues with Azure AD tokens that have
|
||||||
m.logger.Errorf("Access token validation failed: %v", err)
|
// JWT format but unverifiable signatures. See issue #89.
|
||||||
m.handleExpiredToken(rw, req, session, redirectURL)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
m.logger.Debugf("Access token appears opaque, skipping JWT verification for it.")
|
|
||||||
}
|
|
||||||
}
|
|
||||||
m.processAuthorizedRequest(rw, req, session, redirectURL)
|
m.processAuthorizedRequest(rw, req, session, redirectURL)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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{}
|
logger := &mockLogger{}
|
||||||
handleExpiredCalled := false
|
nextHandlerCalled := false
|
||||||
|
|
||||||
initComplete := make(chan struct{})
|
initComplete := make(chan struct{})
|
||||||
close(initComplete)
|
close(initComplete)
|
||||||
@@ -741,11 +741,14 @@ func TestServeHTTP_ComprehensiveCoverage(t *testing.T) {
|
|||||||
logger: logger,
|
logger: logger,
|
||||||
issuerURL: "https://issuer.example.com",
|
issuerURL: "https://issuer.example.com",
|
||||||
initComplete: initComplete,
|
initComplete: initComplete,
|
||||||
|
next: http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
|
||||||
|
nextHandlerCalled = true
|
||||||
|
}),
|
||||||
sessionManager: &mockSessionManager{
|
sessionManager: &mockSessionManager{
|
||||||
getSessionFunc: func(req *http.Request) (SessionData, error) {
|
getSessionFunc: func(req *http.Request) (SessionData, error) {
|
||||||
return &mockSessionData{
|
return &mockSessionData{
|
||||||
email: "[email protected]",
|
email: "[email protected]",
|
||||||
accessToken: "invalid.jwt.token", // JWT format (has dots)
|
accessToken: "valid.jwt.token", // JWT format (has dots)
|
||||||
}, nil
|
}, nil
|
||||||
},
|
},
|
||||||
cleanupOldCookiesFunc: func(rw http.ResponseWriter, req *http.Request) {},
|
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) {
|
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
|
return true, false, false // authenticated, no refresh needed
|
||||||
},
|
},
|
||||||
isAllowedDomainFunc: func(email string) bool {
|
isAllowedDomainFunc: func(email string) bool {
|
||||||
return true
|
return true
|
||||||
},
|
},
|
||||||
tokenVerifier: &mockTokenVerifier{
|
extractClaimsFunc: func(token string) (map[string]interface{}, error) {
|
||||||
verifyFunc: func(token string) error {
|
return map[string]interface{}{"email": "[email protected]"}, nil
|
||||||
return errors.New("token validation failed")
|
|
||||||
},
|
|
||||||
},
|
},
|
||||||
authHandler: &mockAuthHandler{
|
extractGroupsAndRolesFunc: func(token string) ([]string, []string, error) {
|
||||||
initiateAuthFunc: func(rw http.ResponseWriter, req *http.Request, session SessionData, redirectURL string,
|
return []string{}, []string{}, nil
|
||||||
genNonce, genVerifier, deriveChallenge func() (string, error)) {
|
|
||||||
handleExpiredCalled = true
|
|
||||||
},
|
|
||||||
},
|
},
|
||||||
firstRequestReceived: true,
|
firstRequestReceived: true,
|
||||||
}
|
}
|
||||||
|
|
||||||
// We'll track this through the authHandler's InitiateAuthentication call
|
|
||||||
|
|
||||||
req := httptest.NewRequest("GET", "/test", nil)
|
req := httptest.NewRequest("GET", "/test", nil)
|
||||||
rw := httptest.NewRecorder()
|
rw := httptest.NewRecorder()
|
||||||
|
|
||||||
m.ServeHTTP(rw, req)
|
m.ServeHTTP(rw, req)
|
||||||
|
|
||||||
if !handleExpiredCalled {
|
if !nextHandlerCalled {
|
||||||
t.Error("Expected handleExpiredToken for invalid JWT")
|
t.Error("Expected next handler to be called when user is authenticated")
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user