From 7a3d9d81fe5836894b39d0e218193f7cffd732ff Mon Sep 17 00:00:00 2001 From: Aurelia Date: Fri, 13 Nov 2020 23:28:21 +0100 Subject: basicauth: Minor internal improvements (#3861) * nitpicks and small improvements in basicauth module 1: roll two if statements into one, since err will be nil in the second case anyhow 2: unlock cache mutex after reading the key, as this happens by-value and reduces code complexity 3: switch cache sync.Mutex to sync.RWMutex for better concurrency on cache fast track * allocate the right kind of mutex --- modules/caddyhttp/caddyauth/basicauth.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) (limited to 'modules/caddyhttp/caddyauth') diff --git a/modules/caddyhttp/caddyauth/basicauth.go b/modules/caddyhttp/caddyauth/basicauth.go index d955773..f383199 100644 --- a/modules/caddyhttp/caddyauth/basicauth.go +++ b/modules/caddyhttp/caddyauth/basicauth.go @@ -134,7 +134,7 @@ func (hba *HTTPBasicAuth) Provision(ctx caddy.Context) error { if hba.HashCache != nil { hba.HashCache.cache = make(map[string]bool) - hba.HashCache.mu = new(sync.Mutex) + hba.HashCache.mu = new(sync.RWMutex) } return nil @@ -156,12 +156,9 @@ func (hba HTTPBasicAuth) Authenticate(w http.ResponseWriter, req *http.Request) } same, err := hba.correctPassword(account, []byte(plaintextPasswordStr)) - if err != nil { + if err != nil || !same || !accountExists { return hba.promptForCredentials(w, err) } - if !same || !accountExists { - return hba.promptForCredentials(w, nil) - } return User{ID: username}, true, nil } @@ -180,13 +177,12 @@ func (hba HTTPBasicAuth) correctPassword(account Account, plaintextPassword []by cacheKey := hex.EncodeToString(append(append(account.password, account.salt...), plaintextPassword...)) // fast track: if the result of the input is already cached, use it - hba.HashCache.mu.Lock() + hba.HashCache.mu.RLock() same, ok := hba.HashCache.cache[cacheKey] + hba.HashCache.mu.RUnlock() if ok { - hba.HashCache.mu.Unlock() return same, nil } - hba.HashCache.mu.Unlock() // slow track: do the expensive op, then add it to the cache same, err := compare() @@ -219,7 +215,7 @@ func (hba HTTPBasicAuth) promptForCredentials(w http.ResponseWriter, err error) // helpful for secure password hashes which can be expensive to // compute on every HTTP request. type Cache struct { - mu *sync.Mutex + mu *sync.RWMutex // map of concatenated hashed password + plaintext password + salt, to result cache map[string]bool -- cgit v1.2.3