diff --git a/accounts/pkg/service/v0/accounts.go b/accounts/pkg/service/v0/accounts.go index 2d88c313f..6627e3e35 100644 --- a/accounts/pkg/service/v0/accounts.go +++ b/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 diff --git a/changelog/unreleased/accounts-cache-password-validation.md b/changelog/unreleased/accounts-cache-password-validation.md new file mode 100644 index 000000000..4e3363b9c --- /dev/null +++ b/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 \ No newline at end of file diff --git a/proxy/pkg/cache/cache.go b/ocis-pkg/cache/cache.go similarity index 100% rename from proxy/pkg/cache/cache.go rename to ocis-pkg/cache/cache.go diff --git a/proxy/pkg/cache/option.go b/ocis-pkg/cache/option.go similarity index 100% rename from proxy/pkg/cache/option.go rename to ocis-pkg/cache/option.go diff --git a/proxy/pkg/middleware/basic_auth.go b/proxy/pkg/middleware/basic_auth.go index 58e0d7a08..f06879b01 100644 --- a/proxy/pkg/middleware/basic_auth.go +++ b/proxy/pkg/middleware/basic_auth.go @@ -2,12 +2,11 @@ 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/" @@ -15,50 +14,49 @@ 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 +} diff --git a/proxy/pkg/middleware/oidc_auth.go b/proxy/pkg/middleware/oidc_auth.go index f7aaaacdc..ace5a045b 100644 --- a/proxy/pkg/middleware/oidc_auth.go +++ b/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" ) diff --git a/tests/k6/src/test-issue-162.ts b/tests/k6/src/test-issue-162.ts index 11bd86263..c2e57cf5c 100644 --- a/tests/k6/src/test-issue-162.ts +++ b/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,