feat(htpasswd): move htpasswd processing to a helper struct and add r…

…eload Signed-off-by: Vladimir Ermakov <[email protected]>
project-zot · Feb 5, 2025 · 1f35ccb · 1f35ccb
1 parent 05823cd
commit 1f35ccb
Show file tree

Hide file tree

Showing 6 changed files with 156 additions and 48 deletions.
diff --git a/pkg/api/authn.go b/pkg/api/authn.go
@@ -1,7 +1,6 @@
 package api
 
 import (
-	"bufio"
 	"context"
 	"crypto/sha256"
 	"crypto/x509"
@@ -27,7 +26,6 @@ import (
 	"github.com/zitadel/oidc/v3/pkg/client/rp"
 	httphelper "github.com/zitadel/oidc/v3/pkg/http"
 	"github.com/zitadel/oidc/v3/pkg/oidc"
-	"golang.org/x/crypto/bcrypt"
 	"golang.org/x/oauth2"
 	githubOAuth "golang.org/x/oauth2/github"
 
@@ -47,13 +45,16 @@ const (
 )
 
 type AuthnMiddleware struct {
-	credMap    map[string]string
+	htpasswd   *HTPasswd
 	ldapClient *LDAPClient
 	log        log.Logger
 }
 
 func AuthHandler(ctlr *Controller) mux.MiddlewareFunc {
-	authnMiddleware := &AuthnMiddleware{log: ctlr.Log}
+	authnMiddleware := &AuthnMiddleware{
+		htpasswd: ctlr.HTPasswd,
+		log:      ctlr.Log,
+	}
 
 	if ctlr.Config.IsBearerAuthEnabled() {
 		return bearerAuthHandler(ctlr)
@@ -110,40 +111,38 @@ func (amw *AuthnMiddleware) basicAuthn(ctlr *Controller, userAc *reqCtx.UserAcce
 		return false, nil
 	}
 
-	passphraseHash, ok := amw.credMap[identity]
-	if ok {
-		// first, HTTPPassword authN (which is local)
-		if err := bcrypt.CompareHashAndPassword([]byte(passphraseHash), []byte(passphrase)); err == nil {
-			// Process request
-			var groups []string
-
-			if ctlr.Config.HTTP.AccessControl != nil {
-				ac := NewAccessController(ctlr.Config)
-				groups = ac.getUserGroups(identity)
-			}
-
-			userAc.SetUsername(identity)
-			userAc.AddGroups(groups)
-			userAc.SaveOnRequest(request)
+	// first, HTTPPassword authN (which is local)
+	htOk, _ := amw.htpasswd.Authenticate(identity, passphrase)
+	if htOk {
+		// Process request
+		var groups []string
 
-			// saved logged session only if the request comes from web (has UI session header value)
-			if hasSessionHeader(request) {
-				if err := saveUserLoggedSession(cookieStore, response, request, identity, ctlr.Log); err != nil {
-					return false, err
-				}
-			}
+		if ctlr.Config.HTTP.AccessControl != nil {
+			ac := NewAccessController(ctlr.Config)
+			groups = ac.getUserGroups(identity)
+		}
 
-			// we have already populated the request context with userAc
-			if err := ctlr.MetaDB.SetUserGroups(request.Context(), groups); err != nil {
-				ctlr.Log.Error().Err(err).Str("identity", identity).Msg("failed to update user profile")
+		userAc.SetUsername(identity)
+		userAc.AddGroups(groups)
+		userAc.SaveOnRequest(request)
 
+		// saved logged session only if the request comes from web (has UI session header value)
+		if hasSessionHeader(request) {
+			if err := saveUserLoggedSession(cookieStore, response, request, identity, ctlr.Log); err != nil {
 				return false, err
 			}
+		}
 
-			ctlr.Log.Info().Str("identity", identity).Msgf("user profile successfully set")
+		// we have already populated the request context with userAc
+		if err := ctlr.MetaDB.SetUserGroups(request.Context(), groups); err != nil {
+			ctlr.Log.Error().Err(err).Str("identity", identity).Msg("failed to update user profile")
 
-			return true, nil
+			return false, err
 		}
+
+		ctlr.Log.Info().Str("identity", identity).Msgf("user profile successfully set")
+
+		return true, nil
 	}
 
 	// next, LDAP if configured (network-based which can lose connectivity)
@@ -255,8 +254,6 @@ func (amw *AuthnMiddleware) tryAuthnHandlers(ctlr *Controller) mux.MiddlewareFun
 		return noPasswdAuth(ctlr)
 	}
 
-	amw.credMap = make(map[string]string)
-
 	delay := ctlr.Config.HTTP.Auth.FailDelay
 
 	// ldap and htpasswd based authN
@@ -309,22 +306,11 @@ func (amw *AuthnMiddleware) tryAuthnHandlers(ctlr *Controller) mux.MiddlewareFun
 	}
 
 	if ctlr.Config.IsHtpasswdAuthEnabled() {
-		credsFile, err := os.Open(ctlr.Config.HTTP.Auth.HTPasswd.Path)
+		err := amw.htpasswd.Reload(ctlr.Config.HTTP.Auth.HTPasswd.Path)
 		if err != nil {
 			amw.log.Panic().Err(err).Str("credsFile", ctlr.Config.HTTP.Auth.HTPasswd.Path).
 				Msg("failed to open creds-file")
 		}
-		defer credsFile.Close()
-
-		scanner := bufio.NewScanner(credsFile)
-
-		for scanner.Scan() {
-			line := scanner.Text()
-			if strings.Contains(line, ":") {
-				tokens := strings.Split(scanner.Text(), ":")
-				amw.credMap[tokens[0]] = tokens[1]
-			}
-		}
 	}
 
 	// openid based authN

diff --git a/pkg/api/controller.go b/pkg/api/controller.go
@@ -50,6 +50,7 @@ type Controller struct {
 	SyncOnDemand    SyncOnDemand
 	RelyingParties  map[string]rp.RelyingParty
 	CookieStore     *CookieStore
+	HTPasswd        *HTPasswd
 	LDAPClient      *LDAPClient
 	taskScheduler   *scheduler.Scheduler
 	// runtime params
@@ -100,6 +101,7 @@ func NewController(appConfig *config.Config) *Controller {
 
 	controller.Config = appConfig
 	controller.Log = logger
+	controller.HTPasswd = NewHTPasswd(logger)
 
 	if appConfig.Log.Audit != "" {
 		audit := log.NewAuditLogger(appConfig.Log.Level, appConfig.Log.Audit)
@@ -362,8 +364,15 @@ func (c *Controller) LoadNewConfig(newConfig *config.Config) {
 	c.Config.HTTP.AccessControl = newConfig.HTTP.AccessControl
 
 	if c.Config.HTTP.Auth != nil {
+		c.Config.HTTP.Auth.HTPasswd = newConfig.HTTP.Auth.HTPasswd
 		c.Config.HTTP.Auth.LDAP = newConfig.HTTP.Auth.LDAP
 
+		if c.Config.HTTP.Auth.HTPasswd.Path == "" {
+			c.HTPasswd.Clear()
+		} else {
+			_ = c.HTPasswd.Reload(c.Config.HTTP.Auth.HTPasswd.Path)
+		}
+
 		if c.LDAPClient != nil {
 			c.LDAPClient.lock.Lock()
 			c.LDAPClient.BindDN = newConfig.HTTP.Auth.LDAP.BindDN()

diff --git a/pkg/api/htpasswd.go b/pkg/api/htpasswd.go
@@ -0,0 +1,85 @@
+package api
+
+import (
+	"bufio"
+	"os"
+	"strings"
+	"sync"
+
+	"golang.org/x/crypto/bcrypt"
+
+	"zotregistry.dev/zot/pkg/log"
+)
+
+type HTPasswd struct {
+	mu      sync.RWMutex
+	credMap map[string]string
+	log     log.Logger
+}
+
+func NewHTPasswd(log log.Logger) *HTPasswd {
+	return &HTPasswd{
+		credMap: make(map[string]string),
+		log:     log,
+	}
+}
+
+func (s *HTPasswd) Reload(filePath string) error {
+	credMap := make(map[string]string)
+
+	credsFile, err := os.Open(filePath)
+	if err != nil {
+		s.log.Error().Err(err).Str("credsFile", filePath).Msg("failed to reload htpasswd")
+
+		return err
+	}
+	defer credsFile.Close()
+
+	scanner := bufio.NewScanner(credsFile)
+
+	for scanner.Scan() {
+		line := scanner.Text()
+		if strings.Contains(line, ":") {
+			tokens := strings.Split(scanner.Text(), ":")
+			credMap[tokens[0]] = tokens[1]
+		}
+	}
+
+	if len(credMap) == 0 {
+		s.log.Warn().Str("credsFile", filePath).Msg("loaded htpasswd file appears to have zero users")
+	}
+
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.credMap = credMap
+
+	return nil
+}
+
+func (s *HTPasswd) Get(username string) (passphraseHash string, present bool) { //nolint: nonamedreturns
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+
+	passphraseHash, present = s.credMap[username]
+
+	return
+}
+
+func (s *HTPasswd) Clear() {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	s.credMap = make(map[string]string)
+}
+
+func (s *HTPasswd) Authenticate(username, passphrase string) (ok, present bool) { //nolint: nonamedreturns
+	passphraseHash, present := s.Get(username)
+	if !present {
+		return false, false
+	}
+
+	err := bcrypt.CompareHashAndPassword([]byte(passphraseHash), []byte(passphrase))
+	ok = err == nil
+
+	return
+}
diff --git a/pkg/cli/server/config_reloader.go b/pkg/cli/server/config_reloader.go
@@ -16,11 +16,12 @@ import (
 type HotReloader struct {
 	watcher             *fsnotify.Watcher
 	configPath          string
+	htpasswdPath        string
 	ldapCredentialsPath string
 	ctlr                *api.Controller
 }
 
-func NewHotReloader(ctlr *api.Controller, filePath, ldapCredentialsPath string) (*HotReloader, error) {
+func NewHotReloader(ctlr *api.Controller, filePath, htpasswdPath, ldapCredentialsPath string) (*HotReloader, error) {
 	// creates a new file watcher
 	watcher, err := fsnotify.NewWatcher()
 	if err != nil {
@@ -30,6 +31,7 @@ func NewHotReloader(ctlr *api.Controller, filePath, ldapCredentialsPath string)
 	hotReloader := &HotReloader{
 		watcher:             watcher,
 		configPath:          filePath,
+		htpasswdPath:        htpasswdPath,
 		ldapCredentialsPath: ldapCredentialsPath,
 		ctlr:                ctlr,
 	}
@@ -83,6 +85,20 @@ func (hr *HotReloader) Start() {
 							continue
 						}
 
+						if hr.ctlr.Config.HTTP.Auth != nil &&
+							hr.ctlr.Config.HTTP.Auth.HTPasswd.Path != newConfig.HTTP.Auth.HTPasswd.Path {
+							err = hr.watcher.Remove(hr.ctlr.Config.HTTP.Auth.HTPasswd.Path)
+							if err != nil && !errors.Is(err, fsnotify.ErrNonExistentWatch) {
+								log.Error().Err(err).Msg("failed to remove old watch for the htpasswd file")
+							}
+
+							err = hr.watcher.Add(newConfig.HTTP.Auth.HTPasswd.Path)
+							if err != nil {
+								log.Panic().Err(err).Str("htpasswd-file", newConfig.HTTP.Auth.HTPasswd.Path).
+									Msg("failed to watch htpasswd file")
+							}
+						}
+
 						if hr.ctlr.Config.HTTP.Auth != nil && hr.ctlr.Config.HTTP.Auth.LDAP != nil &&
 							hr.ctlr.Config.HTTP.Auth.LDAP.CredentialsFile != newConfig.HTTP.Auth.LDAP.CredentialsFile {
 							err = hr.watcher.Remove(hr.ctlr.Config.HTTP.Auth.LDAP.CredentialsFile)
@@ -117,6 +133,13 @@ func (hr *HotReloader) Start() {
 			log.Panic().Err(err).Str("config", hr.configPath).Msg("failed to add config file to fsnotity watcher")
 		}
 
+		if hr.htpasswdPath != "" {
+			if err := hr.watcher.Add(hr.htpasswdPath); err != nil {
+				log.Panic().Err(err).Str("htpasswd-file", hr.htpasswdPath).
+					Msg("failed to add htpasswd to fsnotity watcher")
+			}
+		}
+
 		if hr.ldapCredentialsPath != "" {
 			if err := hr.watcher.Add(hr.ldapCredentialsPath); err != nil {
 				log.Panic().Err(err).Str("ldap-credentials", hr.ldapCredentialsPath).

diff --git a/pkg/cli/server/root.go b/pkg/cli/server/root.go
@@ -57,13 +57,18 @@ func newServeCmd(conf *config.Config) *cobra.Command {
 
 			ctlr := api.NewController(conf)
 
+			htpasswdPath := ""
 			ldapCredentials := ""
 
+			if conf.HTTP.Auth != nil {
+				htpasswdPath = conf.HTTP.Auth.HTPasswd.Path
+			}
+
 			if conf.HTTP.Auth != nil && conf.HTTP.Auth.LDAP != nil {
 				ldapCredentials = conf.HTTP.Auth.LDAP.CredentialsFile
 			}
 			// config reloader
-			hotReloader, err := NewHotReloader(ctlr, args[0], ldapCredentials)
+			hotReloader, err := NewHotReloader(ctlr, args[0], htpasswdPath, ldapCredentials)
 			if err != nil {
 				ctlr.Log.Error().Err(err).Msg("failed to create a new hot reloader")
 

diff --git a/pkg/extensions/sync/sync_test.go b/pkg/extensions/sync/sync_test.go
@@ -2069,7 +2069,7 @@ func TestConfigReloader(t *testing.T) {
 			_, err = cfgfile.WriteString(content)
 			So(err, ShouldBeNil)
 
-			hotReloader, err := cli.NewHotReloader(dctlr, cfgfile.Name(), "")
+			hotReloader, err := cli.NewHotReloader(dctlr, cfgfile.Name(), "", "")
 			So(err, ShouldBeNil)
 
 			hotReloader.Start()
@@ -2219,7 +2219,7 @@ func TestConfigReloader(t *testing.T) {
 			_, err = cfgfile.WriteString(content)
 			So(err, ShouldBeNil)
 
-			hotReloader, err := cli.NewHotReloader(dctlr, cfgfile.Name(), "")
+			hotReloader, err := cli.NewHotReloader(dctlr, cfgfile.Name(), "", "")
 			So(err, ShouldBeNil)
 
 			hotReloader.Start()