Skip to content

Commit

Permalink
basicauth: Minor internal improvements (#3861)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
acuteaura authored Nov 13, 2020
1 parent 95af426 commit 7a3d9d8
Showing 1 changed file with 5 additions and 9 deletions.
14 changes: 5 additions & 9 deletions modules/caddyhttp/caddyauth/basicauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 7a3d9d8

Please sign in to comment.