Merge pull request #958 from owncloud/basic-auth-cache

implement basic auth cache

accounts/pkg/service/v0/accounts.go

@@ -2,7 +2,9 @@ package service
 
 import (
 	"context"
+	"crypto/sha256"
 	"fmt"
+	"github.com/owncloud/ocis/ocis-pkg/cache"
 	"golang.org/x/crypto/bcrypt"
 	"path"
 	"regexp"
@@ -32,6 +34,12 @@ import (
 // accLock mutually exclude readers from writers on account files
 var accLock sync.Mutex
 
+// passwordValidCache caches basic auth password validations
+var passwordValidCache = cache.NewCache(cache.Size(1024))
+
+// passwordValidCacheExpiration defines the entry lifetime
+const passwordValidCacheExpiration = 10 * time.Minute
+
 // an auth request is currently hardcoded and has to match this regex
 // login eq \"teddy\" and password eq \"F&1!b90t111!\"
 var authQuery = regexp.MustCompile(`^login eq '(.*)' and password eq '(.*)'$`) // TODO how is ' escaped in the password?
@@ -128,7 +136,6 @@ func (s Service) ListAccounts(ctx context.Context, in *proto.ListAccountsRequest
 
 	teardownServiceUser := s.serviceUserToIndex()
 	defer teardownServiceUser()
-
 	match, authRequest := getAuthQueryMatch(in.Query)
 	if authRequest {
 		password := match[2]
@@ -152,9 +159,22 @@ func (s Service) ListAccounts(ctx context.Context, in *proto.ListAccountsRequest
 		if err != nil || a.PasswordProfile == nil || len(a.PasswordProfile.Password) == 0 {
 			return merrors.Unauthorized(s.id, "account not found or invalid credentials")
 		}
-		if !isPasswordValid(s.log, a.PasswordProfile.Password, password) {
-			return merrors.Unauthorized(s.id, "account not found or invalid credentials")
+
+		h := sha256.New()
+		h.Write([]byte(a.PasswordProfile.Password))
+
+		k := fmt.Sprintf("%x", h.Sum([]byte(password)))
+
+		if hit := passwordValidCache.Get(k); hit == nil || !hit.V.(bool) {
+			var ok bool
+
+			if ok = isPasswordValid(s.log, a.PasswordProfile.Password, password); !ok {
+				return merrors.Unauthorized(s.id, "account not found or invalid credentials")
+			}
+
+			passwordValidCache.Set(k, ok, time.Now().Add(passwordValidCacheExpiration))
 		}
+
 		a.PasswordProfile.Password = ""
 		out.Accounts = []*proto.Account{a}
 		return nil

changelog/unreleased/accounts-cache-password-validation.md

@@ -0,0 +1,8 @@
+Change: Cache password validation
+
+Tags: accounts
+
+The password validity check for requests like `login eq '%s' and password eq '%s'` is now cached for 10 minutes.
+This improves the performance for basic auth requests. 
+
+https://github.com/owncloud/ocis/pull/958

proxy/pkg/middleware/basic_auth.go

@@ -2,63 +2,61 @@ package middleware
 
 import (
 	"fmt"
-	"net/http"
-	"strings"
-
 	accounts "github.com/owncloud/ocis/accounts/pkg/proto/v0"
 	"github.com/owncloud/ocis/ocis-pkg/log"
 	"github.com/owncloud/ocis/ocis-pkg/oidc"
+	"net/http"
+	"strings"
 )
 
 const publicFilesEndpoint = "/remote.php/dav/public-files/"
 
 // BasicAuth provides a middleware to check if BasicAuth is provided
 func BasicAuth(optionSetters ...Option) func(next http.Handler) http.Handler {
 	options := newOptions(optionSetters...)
+	logger := options.Logger
+	oidcIss := options.OIDCIss
+
 	if options.EnableBasicAuth {
 		options.Logger.Warn().Msg("basic auth enabled, use only for testing or development")
 	}
 
+	h := basicAuth{
+		logger:         logger,
+		enabled:        options.EnableBasicAuth,
+		accountsClient: options.AccountsClient,
+	}
+
 	return func(next http.Handler) http.Handler {
-		return &basicAuth{
-			next:           next,
-			logger:         options.Logger,
-			enabled:        options.EnableBasicAuth,
-			accountsClient: options.AccountsClient,
-			oidcIss:        options.OIDCIss,
-		}
+		return http.HandlerFunc(
+			func(w http.ResponseWriter, req *http.Request) {
+				if h.isPublicLink(req) || !h.isBasicAuth(req) {
+					next.ServeHTTP(w, req)
+					return
+				}
+
+				account, ok := h.getAccount(req)
+
+				if !ok {
+					w.WriteHeader(http.StatusUnauthorized)
+					return
+				}
+
+				claims := &oidc.StandardClaims{
+					OcisID: account.Id,
+					Iss:    oidcIss,
+				}
+
+				next.ServeHTTP(w, req.WithContext(oidc.NewContext(req.Context(), claims)))
+			},
+		)
 	}
 }
 
 type basicAuth struct {
-	next           http.Handler
 	logger         log.Logger
 	enabled        bool
 	accountsClient accounts.AccountsService
-	oidcIss        string
-}
-
-func (m basicAuth) ServeHTTP(w http.ResponseWriter, req *http.Request) {
-	if m.isPublicLink(req) || !m.isBasicAuth(req) {
-		m.next.ServeHTTP(w, req)
-		return
-	}
-
-	login, password, _ := req.BasicAuth()
-
-	account, status := getAccount(m.logger, m.accountsClient, fmt.Sprintf("login eq '%s' and password eq '%s'", strings.ReplaceAll(login, "'", "''"), strings.ReplaceAll(password, "'", "''")))
-
-	if status != 0 {
-		w.WriteHeader(http.StatusUnauthorized)
-		return
-	}
-
-	claims := &oidc.StandardClaims{
-		OcisID: account.Id,
-		Iss:    m.oidcIss,
-	}
-
-	m.next.ServeHTTP(w, req.WithContext(oidc.NewContext(req.Context(), claims)))
 }
 
 func (m basicAuth) isPublicLink(req *http.Request) bool {
@@ -72,3 +70,19 @@ func (m basicAuth) isBasicAuth(req *http.Request) bool {
 
 	return m.enabled && ok && login != "" && password != ""
 }
+
+func (m basicAuth) getAccount(req *http.Request) (*accounts.Account, bool) {
+	login, password, _ := req.BasicAuth()
+
+	account, status := getAccount(
+		m.logger,
+		m.accountsClient,
+		fmt.Sprintf(
+			"login eq '%s' and password eq '%s'",
+			strings.ReplaceAll(login, "'", "''"),
+			strings.ReplaceAll(password, "'", "''"),
+		),
+	)
+
+	return account, status == 0
+}

proxy/pkg/middleware/oidc_auth.go

@@ -10,9 +10,9 @@ import (
 	"github.com/dgrijalva/jwt-go"
 
 	gOidc "github.com/coreos/go-oidc"
+	"github.com/owncloud/ocis/ocis-pkg/cache"
 	"github.com/owncloud/ocis/ocis-pkg/log"
 	"github.com/owncloud/ocis/ocis-pkg/oidc"
-	"github.com/owncloud/ocis/proxy/pkg/cache"
 	"golang.org/x/oauth2"
 )
 

tests/k6/src/test-issue-162.ts

@@ -1,6 +1,6 @@
 import {sleep, check} from 'k6';
 import {Options} from "k6/options";
-import {defaults, api, utils} from "./lib";
+import {defaults, api} from "./lib";
 
 const files = {
     'kb_50.jpg': open('./_files/kb_50.jpg', 'b'),
@@ -13,7 +13,7 @@ export let options: Options = {
 };
 
 export default () => {
-    const res = api.uploadFile(defaults.accounts.einstein, files['kb_50.jpg'], `kb_50-${utils.randomString()}.jpg`)
+    const res = api.uploadFile(defaults.accounts.einstein, files['kb_50.jpg'], `kb_50-${__VU}-${__ITER}.jpg`)
 
     check(res, {
         'status is 201': () => res.status === 201,