Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement basic auth cache #958

Merged
merged 5 commits into from
Nov 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions accounts/pkg/service/v0/accounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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?
Expand Down Expand Up @@ -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]
Expand All @@ -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
Expand Down
8 changes: 8 additions & 0 deletions changelog/unreleased/accounts-cache-password-validation.md
Original file line number Diff line number Diff line change
@@ -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
File renamed without changes.
File renamed without changes.
84 changes: 49 additions & 35 deletions proxy/pkg/middleware/basic_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
2 changes: 1 addition & 1 deletion proxy/pkg/middleware/oidc_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down
4 changes: 2 additions & 2 deletions tests/k6/src/test-issue-162.ts
Original file line number Diff line number Diff line change
@@ -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'),
Expand All @@ -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`)
phil-davis marked this conversation as resolved.
Show resolved Hide resolved

check(res, {
'status is 201': () => res.status === 201,
Expand Down