From 7a3d9d81fe5836894b39d0e218193f7cffd732ff Mon Sep 17 00:00:00 2001
From: Aurelia <aurelia@schittler.dev>
Date: Fri, 13 Nov 2020 23:28:21 +0100
Subject: [PATCH] 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(-)
diff --git a/modules/caddyhttp/caddyauth/basicauth.go b/modules/caddyhttp/caddyauth/basicauth.go
index d95577305..f383199e8 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