fix: authentication issues caused by update/delete operations (#127)

* fix: auth cache

* fix: remove todo note

Author: bietkul

plugins/auth/auth.go

@@ -36,10 +36,20 @@ var (
 	once      sync.Once
 )
 
-// Auth (TODO - clear cache after fixed entries: LRU?)
+// Cache represents the struct for CredentialCache
+type Cache struct {
+	mu    sync.RWMutex
+	cache map[string]credential.AuthCredential
+}
+
+// CredentialCache represents the cached users/credentials where key is `username`
+var CredentialCache = Cache{
+	mu:    sync.RWMutex{},
+	cache: make(map[string]credential.AuthCredential),
+}
+
 type Auth struct {
 	mu              sync.Mutex
-	credentialCache map[string]credential.AuthCredential
 	jwtRsaPublicKey *rsa.PublicKey
 	jwtRoleKey      string
 	es              authService
@@ -50,9 +60,7 @@ type Auth struct {
 // the instance of the plugin, in order to avoid stateless duplicates.
 func Instance() *Auth {
 	once.Do(func() {
-		singleton = &Auth{
-			credentialCache: make(map[string]credential.AuthCredential),
-		}
+		singleton = &Auth{}
 	})
 	return singleton
 }

plugins/auth/middleware.go

@@ -186,8 +186,8 @@ func (a *Auth) basicAuth(h http.HandlerFunc) http.HandlerFunc {
 				}
 
 				// cache the user
-				if _, ok := a.cachedCredential(username); !ok {
-					a.cacheCredential(username, reqUser)
+				if _, ok := GetCachedCredential(username); !ok {
+					SaveCredentialToCache(username, reqUser)
 				}
 
 				// store request user and credential identifier in the context
@@ -212,8 +212,8 @@ func (a *Auth) basicAuth(h http.HandlerFunc) http.HandlerFunc {
 				}
 
 				// cache the permission
-				if _, ok := a.cachedCredential(username); !ok {
-					a.cacheCredential(username, reqPermission)
+				if _, ok := GetCachedCredential(username); !ok {
+					SaveCredentialToCache(username, reqPermission)
 				}
 
 				// store the request permission and credential identifier in the context
@@ -234,42 +234,45 @@ func (a *Auth) basicAuth(h http.HandlerFunc) http.HandlerFunc {
 		// remove user/permission from cache on write operation
 		if *reqOp == op.Write || *reqOp == op.Delete {
 			username := mux.Vars(req)["username"]
-			a.removeCredentialFromCache(username)
+			RemoveCredentialFromCache(username)
 		}
 
 		h(w, req)
 	}
 }
 
 func (a *Auth) getCredential(ctx context.Context, username string) (credential.AuthCredential, error) {
-	c, ok := a.cachedCredential(username)
+	c, ok := GetCachedCredential(username)
 	if ok {
 		return c, nil
 	}
 	return a.es.getCredential(ctx, username)
 }
 
-func (a *Auth) cachedCredential(username string) (credential.AuthCredential, bool) {
-	a.mu.Lock()
-	defer a.mu.Unlock()
-	if c, ok := a.credentialCache[username]; ok {
+// GetCachedCredential returns the cached credential
+func GetCachedCredential(username string) (credential.AuthCredential, bool) {
+	CredentialCache.mu.Lock()
+	defer CredentialCache.mu.Unlock()
+	if c, ok := CredentialCache.cache[username]; ok {
 		return c, ok
 	}
 	return nil, false
 }
 
-func (a *Auth) removeCredentialFromCache(username string) {
-	a.mu.Lock()
-	defer a.mu.Unlock()
-	delete(a.credentialCache, username)
+// RemoveCredentialFromCache removes the credential from the cache
+func RemoveCredentialFromCache(username string) {
+	CredentialCache.mu.Lock()
+	defer CredentialCache.mu.Unlock()
+	delete(CredentialCache.cache, username)
 }
 
-func (a *Auth) cacheCredential(username string, c credential.AuthCredential) {
+// SaveCredentialToCache saves the credential to the cache
+func SaveCredentialToCache(username string, c credential.AuthCredential) {
 	if c == nil {
 		log.Println(logTag, ": cannot cache 'nil' credential, skipping...")
 		return
 	}
-	a.mu.Lock()
-	defer a.mu.Unlock()
-	a.credentialCache[username] = c
+	CredentialCache.mu.Lock()
+	CredentialCache.cache[username] = c
+	CredentialCache.mu.Unlock()
 }

plugins/permissions/handlers.go

@@ -12,6 +12,7 @@ import (
 	"github.com/appbaseio/arc/model/index"
 	"github.com/appbaseio/arc/model/permission"
 	"github.com/appbaseio/arc/model/user"
+	"github.com/appbaseio/arc/plugins/auth"
 	"github.com/appbaseio/arc/util"
 	"github.com/gorilla/mux"
 )
@@ -255,6 +256,10 @@ func (p *permissions) deletePermission() http.HandlerFunc {
 
 		ok, err := p.es.deletePermission(req.Context(), username)
 		if ok && err == nil {
+			// Clear username record from the cache
+			auth.ClearPassword(username)
+			// Clear user record from the user cache
+			auth.RemoveCredentialFromCache(username)
 			msg := fmt.Sprintf(`permission with "username"="%s" deleted`, username)
 			util.WriteBackMessage(w, msg, http.StatusOK)
 			return

plugins/users/handlers.go

@@ -118,6 +118,7 @@ func (u *Users) postUser() http.HandlerFunc {
 			msg := fmt.Sprintf("an error occurred while hashing password: %v", userBody.Password)
 			log.Errorln(logTag, ":", msg, ":", err)
 			util.WriteBackError(w, msg, http.StatusInternalServerError)
+			return
 		}
 
 		var newUser *user.User
@@ -211,10 +212,24 @@ func (u *Users) patchUser() http.HandlerFunc {
 			}
 		}
 
+		// If user is trying to update the password then store the hashed password
+		if patch["password"] != nil {
+			hashedPassword, err := bcrypt.GenerateFromPassword([]byte(userBody.Password), bcrypt.DefaultCost)
+			if err != nil {
+				msg := fmt.Sprintf("an error occurred while hashing password: %v", userBody.Password)
+				log.Errorln(logTag, ":", msg, ":", err)
+				util.WriteBackError(w, msg, http.StatusInternalServerError)
+				return
+			}
+			patch["password"] = string(hashedPassword)
+		}
+
 		_, err2 := u.es.patchUser(req.Context(), username, patch)
 		if err2 == nil {
 			// Clear username record from the cache
 			auth.ClearPassword(username)
+			// Clear user record from the user cache
+			auth.RemoveCredentialFromCache(username)
 			util.WriteBackMessage(w, "User is updated successfully", http.StatusOK)
 			return
 		}
@@ -284,10 +299,24 @@ func (u *Users) patchUserWithUsername() http.HandlerFunc {
 			}
 		}
 
+		// If user is trying to update the password then store the hashed password
+		if patch["password"] != nil {
+			hashedPassword, err := bcrypt.GenerateFromPassword([]byte(userBody.Password), bcrypt.DefaultCost)
+			if err != nil {
+				msg := fmt.Sprintf("an error occurred while hashing password: %v", userBody.Password)
+				log.Errorln(logTag, ":", msg, ":", err)
+				util.WriteBackError(w, msg, http.StatusInternalServerError)
+				return
+			}
+			patch["password"] = string(hashedPassword)
+		}
+
 		_, err2 := u.es.patchUser(req.Context(), username, patch)
 		if err2 == nil {
 			// Clear username record from the cache
 			auth.ClearPassword(username)
+			// Clear user record from the user cache
+			auth.RemoveCredentialFromCache(username)
 			util.WriteBackMessage(w, "User is updated successfully", http.StatusOK)
 			return
 		}
@@ -306,6 +335,8 @@ func (u *Users) deleteUser() http.HandlerFunc {
 		if ok && err == nil {
 			// Clear username record from the cache
 			auth.ClearPassword(username)
+			// Clear user record from the user cache
+			auth.RemoveCredentialFromCache(username)
 			msg := fmt.Sprintf(`user with "username"="%s" deleted`, username)
 			util.WriteBackMessage(w, msg, http.StatusOK)
 			return
@@ -330,6 +361,8 @@ func (u *Users) deleteUserWithUsername() http.HandlerFunc {
 		if ok && err == nil {
 			// Clear username record from the cache
 			auth.ClearPassword(username)
+			// Clear user record from the user cache
+			auth.RemoveCredentialFromCache(username)
 			msg := fmt.Sprintf(`user with "username"="%s" deleted`, username)
 			util.WriteBackMessage(w, msg, http.StatusOK)
 			return