From 262a86bdbd466f2e9b40d83c95ab0be7117488ec Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Thu, 14 Nov 2024 17:31:36 +0100 Subject: [PATCH] cleanup --- .gitignore | 3 +- README.md | 33 +++++++++++------ systemapi-config.toml | 5 ++- systemapi/config.go | 9 +++-- systemapi/server.go | 79 ++++++++++++++++++++++++++++------------ systemapi/server_test.go | 29 +++++++++++---- 6 files changed, 108 insertions(+), 50 deletions(-) diff --git a/.gitignore b/.gitignore index d19c6ea..508c1a8 100644 --- a/.gitignore +++ b/.gitignore @@ -21,4 +21,5 @@ /build /cert.pem /key.pem -/pipe.fifo \ No newline at end of file +/pipe.fifo +/basic-auth-secret.txt \ No newline at end of file diff --git a/README.md b/README.md index 3dfb73c..3d64de1 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,7 @@ It currently does the following things: - **Actions**: Ability to execute shell commands via API - **Configuration** through file uploads - **HTTP Basic Auth** for API endpoints +- All actions show up in the event log --- @@ -101,24 +102,32 @@ hash is be stored in a file (specified in the config file) to enable basic auth across restarts. The config file ([systemapi-config.toml](./systemapi-config.toml)) includes a `basic_auth_secret_path`. -- If this file is specified but doesn't exist, system-api will not start and log an error. -- If the file exists and is empty, then the APIs are unauthenticated until a secret is configured. - If the file exists and is not empty, then the APIs are authenticated for passwords that match the hash in this file. +- If the file exists and is empty, then the APIs are unauthenticated until a secret is configured. +- If this file is specified but doesn't exist, system-api will create it (empty). ```bash -# Set `basic_auth_secret_path` in the config file and create it empty -touch .basic-auth-secret -vi systemapi-config.toml +# The included systemapi-config.toml uses basic-auth-secret.txt for basic_auth_secret_path +cat systemapi-config.toml -# Start the server, -$ go run cmd/system-api/main.go --config systemapi-config.toml +# Start the server +go run cmd/system-api/main.go --config systemapi-config.toml # Initially, requests are unauthenticated -$ curl localhost:3535/livez +curl -v localhost:3535/livez + +# Set the basic auth secret. From here on, authentication is required for all API requests. +curl -d "foobar" localhost:3535/api/v1/set-basic-auth + +# Check that hash was written to the file +cat basic-auth-secret.txt + +# API calls with no basic auth credentials are provided fail now, with '401 Unauthorized' because +curl -v localhost:3535/livez -# Set the basic auth secret -$ curl -d "foobar" localhost:3535/api/v1/set-basic-auth +# API calls work if correct basic auth credentials are provided +curl -v -u admin:foobar localhost:3535/livez -# Now requests are authenticated -$ curl -u admin:foobar -v localhost:3535/livez +# The update also shows up in the logs +curl -u admin:foobar localhost:3535/logs ``` diff --git a/systemapi-config.toml b/systemapi-config.toml index 7ba7d00..612dcaa 100644 --- a/systemapi-config.toml +++ b/systemapi-config.toml @@ -1,11 +1,12 @@ [general] listen_addr = "0.0.0.0:3535" pipe_file = "pipe.fifo" +pprof = true log_json = false log_debug = true -# The path to the secret file used for basic auth -basic_auth_secret_path = "/tmp/basic_auth_secret" +# Enable HTTP Basic auth by setting a file for the hashed secret +basic_auth_secret_path = "basic-auth-secret.txt" [actions] # reboot = "reboot" diff --git a/systemapi/config.go b/systemapi/config.go index 1a1f351..ff02b41 100644 --- a/systemapi/config.go +++ b/systemapi/config.go @@ -7,10 +7,11 @@ import ( ) type systemAPIConfigGeneral struct { - ListenAddr string `toml:"listen_addr"` - PipeFile string `toml:"pipe_file"` - LogJSON bool `toml:"log_json"` - LogDebug bool `toml:"log_debug"` + ListenAddr string `toml:"listen_addr"` + PipeFile string `toml:"pipe_file"` + LogJSON bool `toml:"log_json"` + LogDebug bool `toml:"log_debug"` + EnablePprof bool `toml:"pprof"` // Enables pprof endpoints BasicAuthSecretPath string `toml:"basic_auth_secret_path"` } diff --git a/systemapi/server.go b/systemapi/server.go index 560443f..efe3a79 100644 --- a/systemapi/server.go +++ b/systemapi/server.go @@ -24,9 +24,8 @@ import ( ) type HTTPServerConfig struct { - Config *SystemAPIConfig - Log *httplog.Logger - EnablePprof bool + Config *SystemAPIConfig + Log *httplog.Logger DrainDuration time.Duration GracefulShutdownDuration time.Duration @@ -59,26 +58,13 @@ func NewServer(cfg *HTTPServerConfig) (srv *Server, err error) { events: make([]Event, 0), } - if cfg.Config.General.BasicAuthSecretPath != "" { - // Abort if the file does not exist - if _, err := os.Stat(cfg.Config.General.BasicAuthSecretPath); os.IsNotExist(err) { - return nil, fmt.Errorf("basic auth secret file does not exist: %s", cfg.Config.General.BasicAuthSecretPath) - } - - // Read the secret from the file - secret, err := os.ReadFile(cfg.Config.General.BasicAuthSecretPath) - if err != nil { - return nil, fmt.Errorf("failed to read basic auth secret file: %w", err) - } - - if len(secret) == 0 { - cfg.Log.Info("Basic auth file without secret loaded, auth disabled until secret is configured", "file", cfg.Config.General.BasicAuthSecretPath) - } else { - cfg.Log.Info("Basic auth enabled", "file", cfg.Config.General.BasicAuthSecretPath) - } - srv.basicAuthHash = string(secret) + // Load (or create) file with basic auth secret hash + err = srv.loadBasicAuthSecretFromFile() + if err != nil { + return nil, err } + // Setup the pipe file if cfg.Config.General.PipeFile != "" { os.Remove(cfg.Config.General.PipeFile) err := syscall.Mknod(cfg.Config.General.PipeFile, syscall.S_IFIFO|0o666, 0) @@ -89,6 +75,7 @@ func NewServer(cfg *HTTPServerConfig) (srv *Server, err error) { go srv.readPipeInBackground() } + // Create the HTTP server srv.srv = &http.Server{ Addr: cfg.Config.General.ListenAddr, Handler: srv.getRouter(), @@ -99,27 +86,68 @@ func NewServer(cfg *HTTPServerConfig) (srv *Server, err error) { return srv, nil } +func (s *Server) loadBasicAuthSecretFromFile() error { + if s.cfg.Config.General.BasicAuthSecretPath == "" { + return nil + } + + // Create if the file does not exist + if _, err := os.Stat(s.cfg.Config.General.BasicAuthSecretPath); os.IsNotExist(err) { + err = os.WriteFile(s.cfg.Config.General.BasicAuthSecretPath, []byte{}, 0o600) + if err != nil { + return fmt.Errorf("failed to create basic auth secret file: %w", err) + } + s.cfg.Log.Info("Basic auth file created, auth disabled until secret is configured", "file", s.cfg.Config.General.BasicAuthSecretPath) + s.basicAuthHash = "" + return nil + } + + // Read the secret from the file + secret, err := os.ReadFile(s.cfg.Config.General.BasicAuthSecretPath) + if err != nil { + return fmt.Errorf("failed to read basic auth secret file: %w", err) + } + + if len(secret) == 0 { + s.cfg.Log.Info("Basic auth file without secret loaded, auth disabled until secret is configured", "file", s.cfg.Config.General.BasicAuthSecretPath) + } else { + s.cfg.Log.Info("Basic auth enabled", "file", s.cfg.Config.General.BasicAuthSecretPath) + } + s.basicAuthHash = string(secret) + return nil +} + func (s *Server) getRouter() http.Handler { mux := chi.NewRouter() mux.Use(httplog.RequestLogger(s.log)) mux.Use(middleware.Recoverer) + + // Enable a custom HTTP Basic Auth middleware mux.Use(BasicAuth("system-api", s.getBasicAuthHashedCredentials)) + // Common APIs mux.Get("/", s.handleLivenessCheck) mux.Get("/livez", s.handleLivenessCheck) + + // Event (log) APIs mux.Get("/api/v1/new_event", s.handleNewEvent) mux.Get("/api/v1/events", s.handleGetEvents) mux.Get("/logs", s.handleGetLogs) + // API to set the basic auth secret mux.Post("/api/v1/set-basic-auth", s.handleSetBasicAuthCreds) + // API to trigger an action mux.Get("/api/v1/actions/{action}", s.handleAction) + + // API to upload a file mux.Post("/api/v1/file-upload/{file}", s.handleFileUpload) - if s.cfg.EnablePprof { - s.log.Info("pprof API enabled") + // Optionally, pprof + if s.cfg.Config.General.EnablePprof { mux.Mount("/debug", middleware.Profiler()) + s.log.Info("pprof API enabled: /debug/pprof/") } return mux @@ -213,6 +241,7 @@ func (s *Server) writeEventsAsText(w http.ResponseWriter) { return } } + w.WriteHeader(http.StatusOK) } func (s *Server) handleGetEvents(w http.ResponseWriter, r *http.Request) { @@ -223,7 +252,7 @@ func (s *Server) handleGetEvents(w http.ResponseWriter, r *http.Request) { return } - // write events as JSON response + // Send events as JSON response s.eventsLock.RLock() defer s.eventsLock.RUnlock() w.Header().Set("Content-Type", "application/json") @@ -233,6 +262,7 @@ func (s *Server) handleGetEvents(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) return } + w.WriteHeader(http.StatusOK) } func (s *Server) handleGetLogs(w http.ResponseWriter, r *http.Request) { @@ -353,5 +383,6 @@ func (s *Server) handleSetBasicAuthCreds(w http.ResponseWriter, r *http.Request) s.basicAuthHash = secretHash s.log.Info("Basic auth secret updated") + s.addInternalEvent("basic auth secret updated. new hash: " + secretHash) w.WriteHeader(http.StatusOK) } diff --git a/systemapi/server_test.go b/systemapi/server_test.go index 287afbe..1dd37dd 100644 --- a/systemapi/server_test.go +++ b/systemapi/server_test.go @@ -51,8 +51,8 @@ func execRequest(t *testing.T, router http.Handler, method, url string, requestB return execRequestAuth(t, router, method, url, requestBody, "", "") } -// Helper to create prepared executors for specific API endpoints -func makeRequestExecutor(t *testing.T, router http.Handler, method, url string) func(basicAuthUser, basicAuthPass string, requestBody io.Reader) (statusCode int, responsePayload []byte) { +// Helper to create prepared test runners for specific API endpoints +func createRequestRunner(t *testing.T, router http.Handler, method, url string) func(basicAuthUser, basicAuthPass string, requestBody io.Reader) (statusCode int, responsePayload []byte) { t.Helper() return func(basicAuthUser, basicAuthPass string, requestBody io.Reader) (statusCode int, responsePayload []byte) { return execRequestAuth(t, router, method, url, requestBody, basicAuthUser, basicAuthPass) @@ -69,7 +69,7 @@ func TestGeneralHandlers(t *testing.T) { code, _ := execRequest(t, router, http.MethodGet, "/livez", nil) require.Equal(t, http.StatusOK, code) - // Test /api/v1/events + // /api/v1/events is initially empty code, respBody := execRequest(t, router, http.MethodGet, "/api/v1/events", nil) require.Equal(t, http.StatusOK, code) require.Equal(t, "[]\n", string(respBody)) @@ -78,6 +78,17 @@ func TestGeneralHandlers(t *testing.T) { code, _ = execRequest(t, router, http.MethodGet, "/api/v1/new_event?message=foo", nil) require.Equal(t, http.StatusOK, code) require.Len(t, srv.events, 1) + require.Equal(t, "foo", srv.events[0].Message) + + // /api/v1/events now has an entry + code, respBody = execRequest(t, router, http.MethodGet, "/api/v1/events", nil) + require.Equal(t, http.StatusOK, code) + require.Contains(t, string(respBody), "foo") + + // /logs should also work + code, respBody = execRequest(t, router, http.MethodGet, "/logs", nil) + require.Equal(t, http.StatusOK, code) + require.Contains(t, string(respBody), "foo\n") } func TestBasicAuth(t *testing.T) { @@ -93,9 +104,13 @@ func TestBasicAuth(t *testing.T) { cfg := getTestConfig() cfg.Config.General.BasicAuthSecretPath = tempDir + "/basic_auth_secret" - // Server should fail to start if the basic auth secret file does not exist + // Create the server instance _, err := NewServer(cfg) - require.Error(t, err) + require.NoError(t, err) + + // Ensure the basic auth secret file was created + _, err = os.Stat(cfg.Config.General.BasicAuthSecretPath) + require.NoError(t, err) // Create the temporary file to store the basic auth secret err = os.WriteFile(cfg.Config.General.BasicAuthSecretPath, []byte{}, 0o600) @@ -107,8 +122,8 @@ func TestBasicAuth(t *testing.T) { router := srv.getRouter() // Prepare request helpers - reqGetLiveZ := makeRequestExecutor(t, router, http.MethodGet, "/livez") - reqSetBasicAuthSecret := makeRequestExecutor(t, router, http.MethodPost, "/api/v1/set-basic-auth") + reqGetLiveZ := createRequestRunner(t, router, http.MethodGet, "/livez") + reqSetBasicAuthSecret := createRequestRunner(t, router, http.MethodPost, "/api/v1/set-basic-auth") // Initially, /livez should work without basic auth code, _ := reqGetLiveZ("", "", nil)