diff --git a/app.go b/app.go index fb90943d5b7e9ab968263a2bf997a45eccc01d3d..8613a847334212e58999f0a625285a0ea03a3923 100644 --- a/app.go +++ b/app.go @@ -188,7 +188,7 @@ func (a *theApp) buildHandlerPipeline() (http.Handler, error) { handler = urilimiter.NewMiddleware(handler, a.config.General.MaxURILength) if a.config.General.NamespaceInPath { - handler = namespaceinpath.NewMiddleware(handler, a.config.General.Domain, a.config.Authentication.RedirectURI) + handler = namespaceinpath.NewMiddleware(handler, a.config.General.Domain, a.config.Authentication.RedirectURI, a.config.General.StatusPath) } handler = rejectmethods.NewMiddleware(handler) diff --git a/internal/namespaceinpath/middleware.go b/internal/namespaceinpath/middleware.go index cc706949efb813da63b47948c9de0a5c8146c493..99a5d9d49d89ca1a0b44712c728103071e9829d5 100644 --- a/internal/namespaceinpath/middleware.go +++ b/internal/namespaceinpath/middleware.go @@ -20,14 +20,16 @@ type middleware struct { pagesDomain string authRedirectURI string skipAuthRewrite bool + statusPath string } // NewMiddleware creates a new middleware -func NewMiddleware(h http.Handler, pagesDomain string, authRedirectURI string) http.Handler { +func NewMiddleware(h http.Handler, pagesDomain string, authRedirectURI string, statusPath string) http.Handler { return &middleware{ next: h, pagesDomain: pagesDomain, authRedirectURI: authRedirectURI, + statusPath: statusPath, // Based on the documentation: https://gitlab.com/gitlab-org/gitlab-pages#gitlab-access-control // When auth redirect URI is without reserved namespace, we skip url rewrite. skipAuthRewrite: isAuthRedirectURLWithoutNamespace(authRedirectURI), @@ -54,9 +56,20 @@ func (m *middleware) ServeHTTP(w http.ResponseWriter, r *http.Request) { m.next.ServeHTTP(crw, r) } +// isHealthCheckPath checks if the request path is a health check path +// Health check paths should match the configured StatusPath and should not be processed as namespace paths +func (m *middleware) isHealthCheckPath(path string) bool { + return m.statusPath != "" && path == m.statusPath +} + // extractNamespaceFromPath extracts the namespace from the request URL path // and updates the request object accordingly. func (m *middleware) extractNamespaceFromPath(r *http.Request) { + // Skip processing for health check URLs + if m.isHealthCheckPath(r.URL.Path) { + return + } + if m.skipAuthRewrite && isAuthRedirectURL(r.URL, m.authRedirectURI) { return } diff --git a/internal/namespaceinpath/middleware_test.go b/internal/namespaceinpath/middleware_test.go index 816a6dd799ed92eb8534116824056dc545ccf123..60c8abd5b855fcf5812e1d99b1e549a19c0044e5 100644 --- a/internal/namespaceinpath/middleware_test.go +++ b/internal/namespaceinpath/middleware_test.go @@ -11,6 +11,7 @@ import ( func TestMiddleware_ServeHTTP(t *testing.T) { pagesDomain := "example.com" + statusPath := "/-/readiness" tests := []struct { name string @@ -90,6 +91,30 @@ func TestMiddleware_ServeHTTP(t *testing.T) { expectedHost: "group.example.com", expectedPath: "/path/to/file/", }, + { + name: "Health check URL should not be modified", + requestURL: "http://example.com/-/readiness", + authRedirectURI: "http://example.com/projects/auth", + expectedStatus: http.StatusOK, + expectedHost: "", + expectedPath: "/-/readiness", + }, + { + name: "Health check URL with different path should be processed as namespace", + requestURL: "http://example.com/-/healthcheck", + authRedirectURI: "http://example.com/projects/auth", + expectedStatus: http.StatusOK, + expectedHost: "-.example.com", + expectedPath: "/healthcheck", + }, + { + name: "Health check URL with query parameters should not be modified", + requestURL: "http://example.com/-/readiness?param=value", + authRedirectURI: "http://example.com/projects/auth", + expectedStatus: http.StatusOK, + expectedHost: "", + expectedPath: "/-/readiness", + }, } for _, tt := range tests { @@ -98,7 +123,7 @@ func TestMiddleware_ServeHTTP(t *testing.T) { w.WriteHeader(http.StatusOK) }) - nipMiddleware := NewMiddleware(nextHandler, pagesDomain, tt.authRedirectURI) + nipMiddleware := NewMiddleware(nextHandler, pagesDomain, tt.authRedirectURI, statusPath) req := httptest.NewRequest("GET", tt.requestURL, nil) rec := httptest.NewRecorder() @@ -121,6 +146,8 @@ func TestMiddleware_ServeHTTP(t *testing.T) { } func TestMiddleware_ExtractNamespaceFromPath(t *testing.T) { + statusPath := "/-/readiness" + tests := []struct { name string url string @@ -161,6 +188,22 @@ func TestMiddleware_ExtractNamespaceFromPath(t *testing.T) { expectedPath: "/a-path", expectedNamespace: "", }, + { + name: "Health check URL should not be modified", + url: "http://example.com/-/readiness", + authRedirectURI: "http://example.com/projects/auth", + expectedHost: "example.com", + expectedPath: "/-/readiness", + expectedNamespace: "", + }, + { + name: "Health check URL with different path should be processed as namespace", + url: "http://example.com/-/healthcheck", + authRedirectURI: "http://example.com/projects/auth", + expectedHost: "-.example.com", + expectedPath: "/healthcheck", + expectedNamespace: "-", + }, } for _, test := range tests { @@ -170,6 +213,7 @@ func TestMiddleware_ExtractNamespaceFromPath(t *testing.T) { nipMiddleware := &middleware{ pagesDomain: "example.com", authRedirectURI: test.authRedirectURI, + statusPath: statusPath, skipAuthRewrite: isAuthRedirectURLWithoutNamespace(test.authRedirectURI), } @@ -308,3 +352,65 @@ func TestIsAuthRedirectURLWithoutNamespace(t *testing.T) { }) } } + +func TestIsHealthCheckPath(t *testing.T) { + statusPath := "/-/readiness" + + middleware := &middleware{ + statusPath: statusPath, + } + + tests := []struct { + name string + path string + expected bool + }{ + { + name: "Health check path with readiness", + path: "/-/readiness", + expected: true, + }, + { + name: "Health check path with healthcheck", + path: "/-/healthcheck", + expected: false, + }, + { + name: "Health check path with liveness", + path: "/-/liveness", + expected: false, + }, + { + name: "Health check path with status", + path: "/-/status", + expected: false, + }, + { + name: "Regular path should not be health check", + path: "/api/v1/health", + expected: false, + }, + { + name: "Namespace path should not be health check", + path: "/namespace/project", + expected: false, + }, + { + name: "Root path should not be health check", + path: "/", + expected: false, + }, + { + name: "Empty path should not be health check", + path: "", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := middleware.isHealthCheckPath(tt.path) + require.Equal(t, tt.expected, result) + }) + } +}