Fixes internal console-auth implementation (#1833)

When console-auth is set to internal, load user credentials at startup
and validate HTTP basic auth requests using the preloaded values.
Consequently, adding or rotating user credentials now requires the
flow-collector container to be restarted.

cmd/flow-collector/handlers.go

@@ -1,10 +1,14 @@
 package main
 
 import (
+	"bytes"
 	"fmt"
 	"io"
 	"log"
 	"net/http"
+	"os"
+	"path/filepath"
+	"regexp"
 
 	"github.com/skupperproject/skupper/pkg/flow"
 )
@@ -226,3 +230,70 @@ func (c *Controller) promqueryrangeHandler(w http.ResponseWriter, r *http.Reques
 		log.Printf("COLLECTOR: rangequery proxy response write error: %s", err.Error())
 	}
 }
+
+func noAuth(h http.HandlerFunc) http.HandlerFunc {
+	return h
+}
+
+// basicAuthHandler handles basic auth for multiple users.
+type basicAuthHandler map[string]string
+
+func newBasicAuthHandler(root string) (basicAuthHandler, error) {
+	basicUsers := make(basicAuthHandler)
+
+	// Restrict usernames to files begining with an alphanumeric character
+	// Omits hidden files
+	fileRexp := regexp.MustCompile(`^[a-zA-Z0-9].*$`)
+
+	entries, err := os.ReadDir(root)
+	if err != nil {
+		return basicUsers, err
+	}
+	var buf bytes.Buffer
+	for _, entry := range entries {
+		if entry.IsDir() {
+			continue
+		}
+		username := entry.Name()
+
+		if !fileRexp.MatchString(username) {
+			continue
+		}
+		path := filepath.Join(root, username)
+		f, err := os.Open(path)
+		if err != nil {
+			log.Printf("COLLECTOR: basic auth file open %q error: %s", path, err.Error())
+			continue
+		}
+		defer f.Close()
+
+		buf.Reset()
+		if _, err := io.Copy(&buf, f); err != nil {
+			log.Printf("COLLECTOR: basic auth file read %q error: %s", path, err.Error())
+			continue
+		}
+
+		basicUsers[username] = buf.String()
+	}
+	return basicUsers, nil
+}
+
+func (h basicAuthHandler) HandlerFunc(next http.HandlerFunc) http.HandlerFunc {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		user, password, ok := r.BasicAuth()
+
+		if ok && h.check(user, password) {
+			next.ServeHTTP(w, r)
+			return
+		}
+		w.Header().Set("WWW-Authenticate", "Basic realm=skupper")
+		http.Error(w, "Unauthorized", http.StatusUnauthorized)
+	})
+}
+
+func (h basicAuthHandler) check(user, given string) bool {
+	if required, ok := h[user]; ok {
+		return given == required
+	}
+	return false
+}

cmd/flow-collector/handlers_test.go

@@ -0,0 +1,96 @@
+package main
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestBasic(t *testing.T) {
+	configuredUsers := map[string]string{
+		"test-user": "plaintext-password",
+		"admin":     "p@ssword!",
+	}
+	tmpDir := t.TempDir()
+	writeUser := func(usr, pwd string) {
+		userFile, err := os.Create(filepath.Join(tmpDir, usr))
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer userFile.Close()
+		userFile.Write([]byte(pwd))
+	}
+	for usr, pwd := range configuredUsers {
+		writeUser(usr, pwd)
+	}
+	writeUser("unreadable", "test") // ensure unreadable files are gracefully skipped
+	os.Chmod(filepath.Join(tmpDir, "unreadable"), 0220)
+
+	BasicAuth, err := newBasicAuthHandler(tmpDir)
+	if err != nil {
+		t.Fatal("unexpected error", err)
+	}
+
+	tstSrv := httptest.NewTLSServer(BasicAuth.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+		rw.Write([]byte("OK"))
+	}))
+	defer tstSrv.Close()
+	client := tstSrv.Client()
+	assertStatusCode := func(expected int, req *http.Request) {
+		t.Helper()
+		resp, err := client.Do(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if resp.StatusCode != expected {
+			t.Fatalf("expected http %d: got %d", expected, resp.StatusCode)
+		}
+	}
+	unauthenticated, _ := http.NewRequest(http.MethodGet, tstSrv.URL, nil)
+	assertStatusCode(401, unauthenticated)
+
+	incorrectPass, _ := http.NewRequest(http.MethodGet, tstSrv.URL, nil)
+	incorrectPass.SetBasicAuth("test-user", "X"+configuredUsers["test-user"])
+	assertStatusCode(401, incorrectPass)
+
+	incorrectUser, _ := http.NewRequest(http.MethodGet, tstSrv.URL, nil)
+	incorrectUser.SetBasicAuth("test-user-x", configuredUsers["test-user"])
+	assertStatusCode(401, incorrectPass)
+
+	unreadableUser, _ := http.NewRequest(http.MethodGet, tstSrv.URL, nil)
+	unreadableUser.SetBasicAuth("unreadable", "test")
+	assertStatusCode(401, unreadableUser)
+
+	mixedUserPass, _ := http.NewRequest(http.MethodGet, tstSrv.URL, nil)
+	mixedUserPass.SetBasicAuth("admin", configuredUsers["test-user"])
+	assertStatusCode(401, mixedUserPass)
+
+	for usr, pwd := range configuredUsers {
+		req, _ := http.NewRequest(http.MethodGet, tstSrv.URL, nil)
+		req.SetBasicAuth(usr, pwd)
+		assertStatusCode(200, req)
+	}
+}
+
+func FuzzBasic(f *testing.F) {
+	const (
+		tUser     = "skupper"
+		tPassword = "P@ssword!"
+	)
+	basic := basicAuthHandler{
+		tUser: tPassword,
+	}
+	f.Add(tUser, tPassword)
+	f.Add(tPassword, tUser)
+	f.Add(tUser, "")
+	f.Add("", tPassword)
+	f.Fuzz(func(t *testing.T, user, password string) {
+		expected := user == tUser && password == tPassword
+		out := basic.check(user, password)
+		if expected != out {
+			t.Errorf("%q:%q does not match %q:%q", user, password, tUser, tPassword)
+		}
+	})
+}

cmd/flow-collector/main.go

@@ -4,16 +4,13 @@ import (
 	"context"
 	"encoding/base64"
 	"encoding/json"
-	"errors"
 	"flag"
 	"fmt"
-	"io"
 	"log"
 	"net/http"
 	_ "net/http/pprof"
 	"os"
 	"os/signal"
-	"path"
 	"strconv"
 	"syscall"
 	"time"
@@ -55,8 +52,13 @@ type UserResponse struct {
 	AuthMode string `json:"authType"`
 }
 
-var onlyOneSignalHandler = make(chan struct{})
-var shutdownSignals = []os.Signal{os.Interrupt, syscall.SIGTERM}
+var (
+	// authenticated made variable to be swapped out at startup
+	authenticated func(next http.HandlerFunc) http.HandlerFunc = noAuth
+
+	onlyOneSignalHandler = make(chan struct{})
+	shutdownSignals      = []os.Signal{os.Interrupt, syscall.SIGTERM}
+)
 
 func getConnectInfo(file string) (connectJson, error) {
 	cj := connectJson{}
@@ -102,46 +104,6 @@ func cors(next http.Handler) http.Handler {
 	})
 }
 
-func authenticate(dir string, user string, password string) bool {
-	filename := path.Join(dir, user)
-	file, err := os.Open(filename)
-	if err != nil {
-		if errors.Is(err, os.ErrNotExist) {
-			log.Printf("COLLECTOR: Failed to authenticate %s, no such user exists", user)
-		} else {
-			log.Printf("COLLECTOR: Failed to authenticate %s: %s", user, err)
-		}
-		return false
-	}
-	defer file.Close()
-
-	bytes, err := io.ReadAll(file)
-	if err != nil {
-		log.Printf("COLLECTOR: Failed to authenticate %s: %s", user, err)
-		return false
-	}
-	return string(bytes) == password
-}
-
-func authenticated(h http.HandlerFunc) http.HandlerFunc {
-	dir := os.Getenv("FLOW_USERS")
-
-	if dir != "" {
-		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			user, password, ok := r.BasicAuth()
-
-			if ok && authenticate(dir, user, password) {
-				h.ServeHTTP(w, r)
-			} else {
-				w.Header().Set("WWW-Authenticate", "Basic realm=skupper")
-				http.Error(w, "Unauthorized", http.StatusUnauthorized)
-			}
-		})
-	} else {
-		return h
-	}
-}
-
 func getOpenshiftUser(r *http.Request) UserResponse {
 	userResponse := UserResponse{
 		Username: "",
@@ -208,6 +170,20 @@ func internalLogout(w http.ResponseWriter, r *http.Request, validNonces map[stri
 	http.Error(w, "Unauthorized", http.StatusUnauthorized)
 }
 
+func configureAuth() error {
+	root := os.Getenv("FLOW_USERS")
+	if root == "" {
+		return nil
+	}
+	log.Printf("COLLECTOR: Configuring basic auth handler from %q \n", root)
+	basic, err := newBasicAuthHandler(root)
+	if err != nil {
+		return err
+	}
+	authenticated = basic.HandlerFunc
+	return nil
+}
+
 func main() {
 	flags := flag.NewFlagSet(os.Args[0], flag.ExitOnError)
 	// if -version used, report and exit
@@ -219,7 +195,6 @@ func main() {
 		fmt.Println(version.Version)
 		os.Exit(0)
 	}
-
 	// Startup message
 	log.Printf("COLLECTOR: Starting Skupper Flow collector controller version %s \n", version.Version)
 
@@ -297,6 +272,10 @@ func main() {
 		}
 	}
 
+	if err := configureAuth(); err != nil {
+		log.Fatalf("unrecoverable error setting up authentication: %s", err)
+	}
+
 	tlsConfig := certs.GetTlsConfigRetriever(true, types.ControllerConfigPath+"tls.crt", types.ControllerConfigPath+"tls.key", types.ControllerConfigPath+"ca.crt")
 
 	conn, err := getConnectInfo(types.ControllerConfigPath + "connect.json")