feat(htpasswd): add autoreload for htpasswd #2933
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,198 @@ | ||
| package api | ||
|
|
||
| import ( | ||
| "bufio" | ||
| "context" | ||
| "errors" | ||
| "os" | ||
| "os/signal" | ||
| "strings" | ||
| "sync" | ||
| "syscall" | ||
|
|
||
| "github.com/fsnotify/fsnotify" | ||
| "golang.org/x/crypto/bcrypt" | ||
|
|
||
| "zotregistry.dev/zot/pkg/log" | ||
| ) | ||
|
|
||
| // HTPasswd user auth store | ||
| // | ||
| // Currently supports only bcrypt hashes. | ||
| 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("htpasswd-file", filePath).Msg("failed to reload htpasswd") | ||
|
|
||
| return err | ||
| } | ||
| defer credsFile.Close() | ||
|
|
||
| scanner := bufio.NewScanner(credsFile) | ||
|
|
||
| for scanner.Scan() { | ||
| user, hash, ok := strings.Cut(scanner.Text(), ":") | ||
| if ok { | ||
| credMap[user] = hash | ||
| } | ||
| } | ||
|
|
||
| if len(credMap) == 0 { | ||
| s.log.Warn().Str("htpasswd-file", filePath).Msg("loaded htpasswd file appears to have zero users") | ||
| } else { | ||
| s.log.Info().Str("htpasswd-file", filePath).Int("users", len(credMap)).Msg("loaded htpasswd file") | ||
| } | ||
|
|
||
| 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 | ||
|
|
||
| if err != nil && !errors.Is(err, bcrypt.ErrMismatchedHashAndPassword) { | ||
| // Log that user's hash has unsupported format. Better than silently return 401. | ||
| s.log.Warn().Err(err).Str("username", username).Msg("htpasswd bcrypt compare failed") | ||
| } | ||
|
|
||
| return | ||
| } | ||
|
|
||
| // HTPasswdWatcher helper which triggers htpasswd reload on file change event. | ||
| // | ||
| // Cannot be restarted. | ||
| type HTPasswdWatcher struct { | ||
| htp *HTPasswd | ||
| filePath string | ||
| watcher *fsnotify.Watcher | ||
| cancel context.CancelFunc | ||
| log log.Logger | ||
| } | ||
|
|
||
| // NewHTPasswdWatcher create and start watcher. | ||
| func NewHTPasswdWatcher(htp *HTPasswd, filePath string) (*HTPasswdWatcher, error) { | ||
| watcher, err := fsnotify.NewWatcher() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if filePath != "" { | ||
| err = watcher.Add(filePath) | ||
| if err != nil { | ||
| return nil, errors.Join(err, watcher.Close()) | ||
| } | ||
| } | ||
|
|
||
| // background event processor job context | ||
| ctx, cancel := signal.NotifyContext(context.Background(), syscall.SIGTERM, syscall.SIGINT) | ||
|
|
||
| ret := &HTPasswdWatcher{ | ||
| htp: htp, | ||
| filePath: filePath, | ||
| watcher: watcher, | ||
| cancel: cancel, | ||
| log: htp.log, | ||
| } | ||
|
|
||
| go func() { | ||
| defer ret.watcher.Close() //nolint: errcheck | ||
|
|
||
| for { | ||
| select { | ||
| case ev := <-ret.watcher.Events: | ||
| if ev.Op != fsnotify.Write { | ||
| continue | ||
| } | ||
|
|
||
| ret.log.Info().Str("htpasswd-file", ret.filePath).Msg("htpasswd file changed, trying to reload config") | ||
|
|
||
| err := ret.htp.Reload(ret.filePath) | ||
| if err != nil { | ||
| ret.log.Warn().Err(err).Str("htpasswd-file", ret.filePath).Msg("failed to reload file") | ||
| } | ||
|
|
||
| case err := <-ret.watcher.Errors: | ||
| ret.log.Panic().Err(err).Str("htpasswd-file", ret.filePath).Msg("fsnotfy error while watching config") | ||
|
|
||
| case <-ctx.Done(): | ||
| ret.log.Debug().Msg("htpasswd watcher terminating...") | ||
|
|
||
| return | ||
| } | ||
| } | ||
| }() | ||
|
|
||
| return ret, nil | ||
| } | ||
|
|
||
| // ChangeFile changes monitored file. Empty string clears store. | ||
| func (s *HTPasswdWatcher) ChangeFile(filePath string) error { | ||
| if s.filePath != "" { | ||
| err := s.watcher.Remove(s.filePath) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| if filePath == "" { | ||
| s.filePath = filePath | ||
| s.htp.Clear() | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| err := s.watcher.Add(filePath) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| s.filePath = filePath | ||
|
|
||
| return s.htp.Reload(filePath) | ||
| } | ||
|
|
||
| func (s *HTPasswdWatcher) Close() error { | ||
| s.cancel() | ||
|
|
||
| return nil | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't panic a bit harsh? Do we want to stop the server in case of an error watching the credentials file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, i copied that behaviour from config watcher. Unsure if i ever seen that error, so probably better to die, than left unpredictable state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A change in the config file not being applied is something impacting the entire service, while a change in htpasswd not being applied impacts just specific users who were added/removed/had the passwords changed.
I don't think we should stop the service in this specific case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that's more like entire system error, than this file error. Or i'm wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well yes, but do we want to interrupt service for all users, or just the ones which don't have access anyway? Or would it help the admin with troubleshooting the issue if the entire service stops?