Bugfix: Oauth2 redirection was broken with base path (#3926)

Added test to ensure base path works correctly with all the authentication mechanisms. Fixes: #3924 --------- Co-authored-by: snyk-bot <[email protected]>
Velocidex · Nov 28, 2024 · c2b8c06 · c2b8c06
1 parent 79219ac
commit c2b8c06
Show file tree

Hide file tree

Showing 45 changed files with 2,642 additions and 2,040 deletions.
diff --git a/api/api.go b/api/api.go
@@ -45,6 +45,7 @@ import (
 	"www.velocidex.com/golang/velociraptor/api/authenticators"
 	api_proto "www.velocidex.com/golang/velociraptor/api/proto"
 	"www.velocidex.com/golang/velociraptor/api/tables"
+	api_utils "www.velocidex.com/golang/velociraptor/api/utils"
 	artifacts_proto "www.velocidex.com/golang/velociraptor/artifacts/proto"
 	config_proto "www.velocidex.com/golang/velociraptor/config/proto"
 	"www.velocidex.com/golang/velociraptor/file_store/api"
@@ -1217,7 +1218,7 @@ func StartMonitoringService(
 		config_obj.Monitoring.BindAddress,
 		config_obj.Monitoring.BindPort)
 
-	mux := http.NewServeMux()
+	mux := api_utils.NewServeMux()
 	mux.Handle("/metrics", promhttp.Handler())
 	server := &http.Server{
 		Addr:     bind_addr,

diff --git a/api/assets.go b/api/assets.go
@@ -32,6 +32,7 @@ import (
 	"github.com/lpar/gzipped"
 	context "golang.org/x/net/context"
 	"www.velocidex.com/golang/velociraptor/api/proto"
+	api_utils "www.velocidex.com/golang/velociraptor/api/utils"
 	utils "www.velocidex.com/golang/velociraptor/api/utils"
 	config_proto "www.velocidex.com/golang/velociraptor/config/proto"
 	"www.velocidex.com/golang/velociraptor/gui/velociraptor"
@@ -42,10 +43,10 @@ import (
 
 func install_static_assets(
 	ctx context.Context,
-	config_obj *config_proto.Config, mux *http.ServeMux) {
+	config_obj *config_proto.Config, mux *api_utils.ServeMux) {
 	base := utils.GetBasePath(config_obj)
 	dir := utils.Join(base, "/app/")
-	mux.Handle(dir, ipFilter(config_obj, http.StripPrefix(
+	mux.Handle(dir, ipFilter(config_obj, api_utils.StripPrefix(
 		dir, fixCSSURLs(config_obj,
 			gzipped.FileServer(NewCachedFilesystem(ctx, gui_assets.NewHTTPFS()))))))
 
@@ -73,35 +74,36 @@ func GetTemplateHandler(
 		return nil, err
 	}
 
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		userinfo := GetUserInfo(r.Context(), config_obj)
+	return api_utils.HandlerFunc(nil,
+		func(w http.ResponseWriter, r *http.Request) {
+			userinfo := GetUserInfo(r.Context(), config_obj)
 
-		// This should never happen!
-		if userinfo.Name == "" {
-			returnError(w, 401, "Unauthenticated access.")
-			return
-		}
+			// This should never happen!
+			if userinfo.Name == "" {
+				returnError(w, 401, "Unauthenticated access.")
+				return
+			}
 
-		users := services.GetUserManager()
-		user_options, err := users.GetUserOptions(r.Context(), userinfo.Name)
-		if err != nil {
-			// Options may not exist yet
-			user_options = &proto.SetGUIOptionsRequest{}
-		}
+			users := services.GetUserManager()
+			user_options, err := users.GetUserOptions(r.Context(), userinfo.Name)
+			if err != nil {
+				// Options may not exist yet
+				user_options = &proto.SetGUIOptionsRequest{}
+			}
 
-		args := velociraptor.HTMLtemplateArgs{
-			Timestamp: time.Now().UTC().UnixNano() / 1000,
-			CsrfToken: csrf.Token(r),
-			BasePath:  utils.GetBasePath(config_obj),
-			Heading:   "Heading",
-			UserTheme: user_options.Theme,
-			OrgId:     user_options.Org,
-		}
-		err = tmpl.Execute(w, args)
-		if err != nil {
-			w.WriteHeader(500)
-		}
-	}), nil
+			args := velociraptor.HTMLtemplateArgs{
+				Timestamp: time.Now().UTC().UnixNano() / 1000,
+				CsrfToken: csrf.Token(r),
+				BasePath:  utils.GetBasePath(config_obj),
+				Heading:   "Heading",
+				UserTheme: user_options.Theme,
+				OrgId:     user_options.Org,
+			}
+			err = tmpl.Execute(w, args)
+			if err != nil {
+				w.WriteHeader(500)
+			}
+		}), nil
 }
 
 // Vite hard compiles the css urls into the bundle so we can not move
@@ -110,18 +112,19 @@ func fixCSSURLs(config_obj *config_proto.Config,
 	parent http.Handler) http.Handler {
 
 	if config_obj.GUI == nil || config_obj.GUI.BasePath == "" {
-		return parent
+		return api_utils.HandlerFunc(parent, parent.ServeHTTP).
+			AddChild("NewInterceptingResponseWriter")
 	}
 
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if !strings.HasSuffix(r.URL.Path, ".css") {
-			parent.ServeHTTP(w, r)
-		} else {
-			parent.ServeHTTP(
-				NewInterceptingResponseWriter(
-					w, r, config_obj.GUI.BasePath), r)
-		}
-	})
+	return api_utils.HandlerFunc(parent,
+		func(w http.ResponseWriter, r *http.Request) {
+			if !strings.HasSuffix(r.URL.Path, ".css") {
+				parent.ServeHTTP(w, r)
+			} else {
+				parent.ServeHTTP(
+					NewInterceptingResponseWriter(config_obj, w, r), r)
+			}
+		}).AddChild("NewInterceptingResponseWriter")
 }
 
 type interceptingResponseWriter struct {
@@ -151,8 +154,8 @@ func (self *interceptingResponseWriter) Write(buf []byte) (int, error) {
 }
 
 func NewInterceptingResponseWriter(
-	w http.ResponseWriter, r *http.Request,
-	base_path string) http.ResponseWriter {
+	config_obj *config_proto.Config,
+	w http.ResponseWriter, r *http.Request) http.ResponseWriter {
 
 	// Try to do brotli compression if it is available.
 	accept_encoding, pres := r.Header["Accept-Encoding"]
@@ -164,8 +167,8 @@ func NewInterceptingResponseWriter(
 			return &interceptingResponseWriter{
 				ResponseWriter: w,
 				from:           "url(/app/assets/",
-				to: fmt.Sprintf("url(/%v/app/assets/",
-					strings.TrimPrefix(base_path, "/")),
+				to: fmt.Sprintf("url(%v/app/assets/",
+					utils.GetBasePath(config_obj)),
 				br_writer: brotli.NewWriter(w),
 			}
 		}
@@ -175,6 +178,7 @@ func NewInterceptingResponseWriter(
 	return &interceptingResponseWriter{
 		ResponseWriter: w,
 		from:           "url(/app/assets/",
-		to:             fmt.Sprintf("url(/%v/app/assets/", base_path),
+		to: fmt.Sprintf("url(%v/app/assets/",
+			utils.GetBasePath(config_obj)),
 	}
 }
diff --git a/api/authenticators/auth.go b/api/authenticators/auth.go
@@ -22,8 +22,8 @@ var (
 
 // All SSO Authenticators implement this interface.
 type Authenticator interface {
-	AddHandlers(mux *http.ServeMux) error
-	AddLogoff(mux *http.ServeMux) error
+	AddHandlers(mux *utils.ServeMux) error
+	AddLogoff(mux *utils.ServeMux) error
 
 	// Make sure the user is authenticated and has at least read
 	// access to the requested org.
@@ -85,8 +85,6 @@ func init() {
 		return &AzureAuthenticator{
 			config_obj:    config_obj,
 			authenticator: auth_config,
-			base:          utils.GetBasePath(config_obj),
-			public_url:    utils.GetPublicURL(config_obj),
 		}, nil
 	})
 
@@ -99,8 +97,6 @@ func init() {
 		return &GitHubAuthenticator{
 			config_obj:    config_obj,
 			authenticator: auth_config,
-			base:          utils.GetBasePath(config_obj),
-			public_url:    utils.GetPublicURL(config_obj),
 		}, nil
 	})
 
@@ -113,8 +109,6 @@ func init() {
 		return &GoogleAuthenticator{
 			config_obj:    config_obj,
 			authenticator: auth_config,
-			base:          utils.GetBasePath(config_obj),
-			public_url:    utils.GetPublicURL(config_obj),
 		}, nil
 	})
 
@@ -127,8 +121,6 @@ func init() {
 		auth_config *config_proto.Authenticator) (Authenticator, error) {
 		return &BasicAuthenticator{
 			config_obj: config_obj,
-			base:       utils.GetBasePath(config_obj),
-			public_url: utils.GetPublicURL(config_obj),
 		}, nil
 	})
 
@@ -140,8 +132,6 @@ func init() {
 
 		result := &CertAuthenticator{
 			config_obj:    config_obj,
-			base:          utils.GetBasePath(config_obj),
-			public_url:    utils.GetPublicURL(config_obj),
 			x509_roots:    x509.NewCertPool(),
 			default_roles: auth_config.DefaultRolesForUnknownUser,
 		}
@@ -162,8 +152,6 @@ func init() {
 		return &OidcAuthenticator{
 			config_obj:    config_obj,
 			authenticator: auth_config,
-			base:          utils.GetBasePath(config_obj),
-			public_url:    utils.GetPublicURL(config_obj),
 		}, nil
 	})