From 5cb359d877d00da73f20631d6c78891c5a581a61 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Mon, 30 Nov 2020 17:19:03 +0100 Subject: [PATCH 01/22] WIP --- accounts/pkg/command/server.go | 8 +++++ accounts/pkg/server/http/server.go | 8 +++++ proxy/pkg/command/server.go | 11 +++--- proxy/pkg/middleware/authentication.go | 48 ++++++++++++++++++++++++++ proxy/pkg/middleware/basic_auth.go | 7 ++-- proxy/pkg/middleware/oidc_auth.go | 5 ++- 6 files changed, 78 insertions(+), 9 deletions(-) create mode 100644 proxy/pkg/middleware/authentication.go diff --git a/accounts/pkg/command/server.go b/accounts/pkg/command/server.go index b5edee2db99..e6ce9e76454 100644 --- a/accounts/pkg/command/server.go +++ b/accounts/pkg/command/server.go @@ -2,8 +2,11 @@ package command import ( "context" + gohttp "net/http" + _ "net/http/pprof" "os" "os/signal" + "runtime" "strings" "github.com/micro/cli/v2" @@ -109,6 +112,11 @@ func Server(cfg *config.Config) *cli.Command { cancel() }) } + runtime.SetBlockProfileRate(1) + runtime.SetMutexProfileFraction(1) + go func() { + gohttp.ListenAndServe(":8887", nil) + }() return gr.Run() }, diff --git a/accounts/pkg/server/http/server.go b/accounts/pkg/server/http/server.go index 4514d412ac0..81678a71b11 100644 --- a/accounts/pkg/server/http/server.go +++ b/accounts/pkg/server/http/server.go @@ -1,7 +1,12 @@ package http import ( + _ "net/http/pprof" + + ghttp "net/http" + "github.com/go-chi/chi" + cmw "github.com/go-chi/chi/middleware" "github.com/owncloud/ocis/accounts/pkg/assets" "github.com/owncloud/ocis/accounts/pkg/proto/v0" "github.com/owncloud/ocis/accounts/pkg/version" @@ -27,6 +32,9 @@ func Server(opts ...Option) http.Service { mux := chi.NewMux() + mux.Use(func(next ghttp.Handler) ghttp.Handler { + return cmw.Profiler() + }) mux.Use(middleware.RealIP) mux.Use(middleware.RequestID) mux.Use(middleware.NoCache) diff --git a/proxy/pkg/command/server.go b/proxy/pkg/command/server.go index dad51e24971..2f5cb861987 100644 --- a/proxy/pkg/command/server.go +++ b/proxy/pkg/command/server.go @@ -268,8 +268,8 @@ func loadMiddlewares(ctx context.Context, l log.Logger, cfg *config.Config) alic return alice.New( middleware.HTTPSRedirect, - middleware.OIDCAuth( - middleware.Logger(l), + middleware.Authentication( + // OIDC Options middleware.OIDCProviderFunc(func() (middleware.OIDCProvider, error) { // Initialize a provider by specifying the issuer URL. // it will fetch the keys from the issuer using the .well-known @@ -280,13 +280,12 @@ func loadMiddlewares(ctx context.Context, l log.Logger, cfg *config.Config) alic ) }), middleware.HTTPClient(oidcHTTPClient), - middleware.OIDCIss(cfg.OIDC.Issuer), middleware.TokenCacheSize(cfg.OIDC.UserinfoCache.Size), middleware.TokenCacheTTL(time.Second*time.Duration(cfg.OIDC.UserinfoCache.TTL)), - ), - middleware.BasicAuth( + + // basic Options middleware.Logger(l), - middleware.EnableBasicAuth(cfg.EnableBasicAuth), + middleware.EnableBasicAuth(true), middleware.AccountsClient(accountsClient), middleware.OIDCIss(cfg.OIDC.Issuer), ), diff --git a/proxy/pkg/middleware/authentication.go b/proxy/pkg/middleware/authentication.go new file mode 100644 index 00000000000..00d0efcbec7 --- /dev/null +++ b/proxy/pkg/middleware/authentication.go @@ -0,0 +1,48 @@ +package middleware + +import ( + "fmt" + "net/http" + "time" +) + +// Authentication is a higher level authentication middleware. +func Authentication(opts ...Option) func(next http.Handler) http.Handler { + options := newOptions(opts...) + + oidc := OIDCAuth( + Logger(options.Logger), + OIDCProviderFunc(options.OIDCProviderFunc), + HTTPClient(options.HTTPClient), + OIDCIss(options.OIDCIss), + TokenCacheSize(options.UserinfoCacheSize), + TokenCacheTTL(time.Second*time.Duration(options.UserinfoCacheTTL)), + ) + + basic := BasicAuth( + Logger(options.Logger), + EnableBasicAuth(options.EnableBasicAuth), + AccountsClient(options.AccountsClient), + OIDCIss(options.OIDCIss), + ) + + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // here we multiplex depending on the use agent + userAgent := r.Header.Get("User-Agent") + fmt.Printf("\n\nUser-Agent:\t%s\n\n", userAgent) + switch userAgent { + case "a": + oidc(next).ServeHTTP(w, r) + return + case "b": + basic(next).ServeHTTP(w, r) + return + default: + oidc(next).ServeHTTP(w, r) + basic(next).ServeHTTP(w, r) + return + } + }) + } +} diff --git a/proxy/pkg/middleware/basic_auth.go b/proxy/pkg/middleware/basic_auth.go index f06879b012d..f766dcd5d75 100644 --- a/proxy/pkg/middleware/basic_auth.go +++ b/proxy/pkg/middleware/basic_auth.go @@ -2,11 +2,12 @@ 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/" @@ -38,6 +39,8 @@ func BasicAuth(optionSetters ...Option) func(next http.Handler) http.Handler { account, ok := h.getAccount(req) if !ok { + // TODO need correct hostname + w.Header().Add("WWW-Authenticate", "Basic realm=\"Access to localhost\", charset=\"UTF-8\"") w.WriteHeader(http.StatusUnauthorized) return } diff --git a/proxy/pkg/middleware/oidc_auth.go b/proxy/pkg/middleware/oidc_auth.go index ace5a045b45..1de7ecc5083 100644 --- a/proxy/pkg/middleware/oidc_auth.go +++ b/proxy/pkg/middleware/oidc_auth.go @@ -38,7 +38,10 @@ func OIDCAuth(optionSetters ...Option) func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { if !h.shouldServe(req) { - next.ServeHTTP(w, req) + // TODO need correct hostname + w.Header().Add("WWW-Authenticate", "Bearer realm=\"Access to localhost\", charset=\"UTF-8\"") + //w.WriteHeader(http.StatusUnauthorized) + //next.ServeHTTP(w, req) return } From 348c54f2e78623ce967fb4f5b411180b4cb270a0 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Tue, 1 Dec 2020 16:57:36 +0100 Subject: [PATCH 02/22] write www-authenticate and delegate to reva --- ocis/go.mod | 2 ++ ocis/go.sum | 2 ++ proxy/pkg/command/server.go | 2 +- proxy/pkg/middleware/authentication.go | 49 +++++++++++++++++++------- proxy/pkg/middleware/basic_auth.go | 11 ++++-- proxy/pkg/middleware/oidc_auth.go | 15 +++++--- storage/pkg/command/frontend.go | 5 +++ 7 files changed, 65 insertions(+), 21 deletions(-) diff --git a/ocis/go.mod b/ocis/go.mod index 1f8ad8dcc64..fca8aa036b0 100644 --- a/ocis/go.mod +++ b/ocis/go.mod @@ -7,6 +7,7 @@ require ( contrib.go.opencensus.io/exporter/ocagent v0.7.0 contrib.go.opencensus.io/exporter/zipkin v0.1.2 github.com/UnnoTed/fileb0x v1.1.4 + github.com/cs3org/reva v1.4.1-0.20201201074212-8b4cc174b708 // indirect github.com/go-test/deep v1.0.6 // indirect github.com/gopherjs/gopherjs v0.0.0-20200217142428-fce0ec30dd00 // indirect github.com/micro/cli/v2 v2.1.2 @@ -37,6 +38,7 @@ require ( ) replace ( + github.com/cs3org/reva => ../../../reva github.com/gomodule/redigo => github.com/gomodule/redigo v1.8.2 github.com/owncloud/ocis/accounts => ../accounts github.com/owncloud/ocis/glauth => ../glauth diff --git a/ocis/go.sum b/ocis/go.sum index 8b57477478b..02495531bd5 100644 --- a/ocis/go.sum +++ b/ocis/go.sum @@ -311,6 +311,8 @@ github.com/cs3org/reva v1.4.1-0.20201127111856-e6a6212c1b7b h1:Bypxdf3vXwyEeL86M github.com/cs3org/reva v1.4.1-0.20201127111856-e6a6212c1b7b/go.mod h1:MTBlfobTE8W2hgXQ9+r+75jpJa1TxD04IZm5TpS9H48= github.com/cs3org/reva v1.4.1-0.20201130061320-ac85e68e0600 h1:4CKU+R4UNvILzsPrcAFwEbk/8Hc6vJqwO7SxK0gAm4I= github.com/cs3org/reva v1.4.1-0.20201130061320-ac85e68e0600/go.mod h1:MTBlfobTE8W2hgXQ9+r+75jpJa1TxD04IZm5TpS9H48= +github.com/cs3org/reva v1.4.1-0.20201201074212-8b4cc174b708 h1:uiz1kb5iR6V7GvpX5CW3xeZcbPTvR8P1tlvODTD0zio= +github.com/cs3org/reva v1.4.1-0.20201201074212-8b4cc174b708/go.mod h1:MTBlfobTE8W2hgXQ9+r+75jpJa1TxD04IZm5TpS9H48= github.com/cznic/b v0.0.0-20181122101859-a26611c4d92d h1:SwD98825d6bdB+pEuTxWOXiSjBrHdOl/UVp75eI7JT8= github.com/cznic/b v0.0.0-20181122101859-a26611c4d92d/go.mod h1:URriBxXwVq5ijiJ12C7iIZqlA69nTlI+LgI6/pwftG8= github.com/cznic/mathutil v0.0.0-20181122101859-297441e03548 h1:iwZdTE0PVqJCos1vaoKsclOGD3ADKpshg3SRtYBbwso= diff --git a/proxy/pkg/command/server.go b/proxy/pkg/command/server.go index 2f5cb861987..1d463667999 100644 --- a/proxy/pkg/command/server.go +++ b/proxy/pkg/command/server.go @@ -285,7 +285,7 @@ func loadMiddlewares(ctx context.Context, l log.Logger, cfg *config.Config) alic // basic Options middleware.Logger(l), - middleware.EnableBasicAuth(true), + middleware.EnableBasicAuth(cfg.EnableBasicAuth), middleware.AccountsClient(accountsClient), middleware.OIDCIss(cfg.OIDC.Issuer), ), diff --git a/proxy/pkg/middleware/authentication.go b/proxy/pkg/middleware/authentication.go index 00d0efcbec7..ebc053510ab 100644 --- a/proxy/pkg/middleware/authentication.go +++ b/proxy/pkg/middleware/authentication.go @@ -3,12 +3,32 @@ package middleware import ( "fmt" "net/http" + "strings" "time" ) -// Authentication is a higher level authentication middleware. +var SupportedAuthStrategies []string + +type statusRecorder struct { + http.ResponseWriter + status int +} + +func (rec *statusRecorder) WriteHeader(code int) { + rec.status = code + rec.ResponseWriter.WriteHeader(code) +} + +// Authentication is a higher order authentication middleware. func Authentication(opts ...Option) func(next http.Handler) http.Handler { options := newOptions(opts...) + if options.OIDCIss != "" { + SupportedAuthStrategies = append(SupportedAuthStrategies, "bearer") + } + + if options.EnableBasicAuth { + SupportedAuthStrategies = append(SupportedAuthStrategies, "basic") + } oidc := OIDCAuth( Logger(options.Logger), @@ -28,21 +48,24 @@ func Authentication(opts ...Option) func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // here we multiplex depending on the use agent - userAgent := r.Header.Get("User-Agent") - fmt.Printf("\n\nUser-Agent:\t%s\n\n", userAgent) - switch userAgent { - case "a": - oidc(next).ServeHTTP(w, r) - return - case "b": - basic(next).ServeHTTP(w, r) - return - default: + if options.OIDCIss != "" && options.EnableBasicAuth { + oidc(basic(next)).ServeHTTP(w, r) + } + + if options.OIDCIss != "" && !options.EnableBasicAuth { oidc(next).ServeHTTP(w, r) + } + + if options.OIDCIss == "" && options.EnableBasicAuth { basic(next).ServeHTTP(w, r) - return } + }) } } + +func writeSupportedAuthenticateHeader(w http.ResponseWriter, r *http.Request) { + for i := 0; i < len(SupportedAuthStrategies); i++ { + w.Header().Add("WWW-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(SupportedAuthStrategies[i]), r.Host)) + } +} diff --git a/proxy/pkg/middleware/basic_auth.go b/proxy/pkg/middleware/basic_auth.go index f766dcd5d75..48055f55746 100644 --- a/proxy/pkg/middleware/basic_auth.go +++ b/proxy/pkg/middleware/basic_auth.go @@ -32,15 +32,18 @@ func BasicAuth(optionSetters ...Option) func(next http.Handler) http.Handler { return http.HandlerFunc( func(w http.ResponseWriter, req *http.Request) { if h.isPublicLink(req) || !h.isBasicAuth(req) { + // if we want to prevent duplicated Www-Authenticate headers coming from Reva consider using w.Header().Del("Www-Authenticate") + // but this will require the proxy being aware of endpoints which authentication fallback to Reva. + if !h.isPublicLink(req) { + w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", "Basic", req.Host)) + } next.ServeHTTP(w, req) return } account, ok := h.getAccount(req) - if !ok { - // TODO need correct hostname - w.Header().Add("WWW-Authenticate", "Basic realm=\"Access to localhost\", charset=\"UTF-8\"") + w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", "Basic", req.Host)) w.WriteHeader(http.StatusUnauthorized) return } @@ -50,6 +53,8 @@ func BasicAuth(optionSetters ...Option) func(next http.Handler) http.Handler { Iss: oidcIss, } + fmt.Printf("\n\nHGAHAHAHA\n\n") + next.ServeHTTP(w, req.WithContext(oidc.NewContext(req.Context(), claims))) }, ) diff --git a/proxy/pkg/middleware/oidc_auth.go b/proxy/pkg/middleware/oidc_auth.go index 1de7ecc5083..112a86f581a 100644 --- a/proxy/pkg/middleware/oidc_auth.go +++ b/proxy/pkg/middleware/oidc_auth.go @@ -3,6 +3,7 @@ package middleware import ( "context" "encoding/json" + "fmt" "net/http" "strings" "time" @@ -37,11 +38,17 @@ func OIDCAuth(optionSetters ...Option) func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + // there is no bearer token on the request, if !h.shouldServe(req) { - // TODO need correct hostname - w.Header().Add("WWW-Authenticate", "Bearer realm=\"Access to localhost\", charset=\"UTF-8\"") - //w.WriteHeader(http.StatusUnauthorized) - //next.ServeHTTP(w, req) + // oidc supported but token not present, add header and handover to the next middleware. + + // TODO for this logic to work and we don't return superfluous Www-Authenticate headers we would need to + // add Www-Authenticate only on selected endpoints, because Reva won't cleanup already written headers. + // this means that requests such as: + // curl -v -k -u admin:admin -H "depth: 0" -X PROPFIND https://localhost:9200/remote.php/dav/files | xmllint --format - + // even when succeeding, will contain a Www-Authenticate header. + w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", "Bearer", req.Host)) + next.ServeHTTP(w, req) return } diff --git a/storage/pkg/command/frontend.go b/storage/pkg/command/frontend.go index 7bb726597d9..9dd742a0ccd 100644 --- a/storage/pkg/command/frontend.go +++ b/storage/pkg/command/frontend.go @@ -115,6 +115,11 @@ func Frontend(cfg *config.Config) *cli.Command { "cors": map[string]interface{}{ "allow_credentials": true, }, + "auth": map[string]interface{}{ + "credentials_by_user_agent": map[string]string{ + "mirall": "basic", + }, + }, }, // TODO build services dynamically "services": map[string]interface{}{ From 28e8f75ebd4f3106af2d44aca551dc13b2f94818 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Tue, 1 Dec 2020 17:10:04 +0100 Subject: [PATCH 03/22] whitelist depending on the URI --- proxy/pkg/middleware/authentication.go | 5 +++++ proxy/pkg/middleware/basic_auth.go | 12 ++++++++++-- proxy/pkg/middleware/oidc_auth.go | 7 ++++++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/proxy/pkg/middleware/authentication.go b/proxy/pkg/middleware/authentication.go index ebc053510ab..16a0f5d3c61 100644 --- a/proxy/pkg/middleware/authentication.go +++ b/proxy/pkg/middleware/authentication.go @@ -9,6 +9,11 @@ import ( var SupportedAuthStrategies []string +// ProxyWwwAuthenticate is a list of endpoints that do not rely on reva underlying authentication, such as ocs. +// services that fallback to reva authentication are declared in the "frontend" command on OCIS. +// TODO this should be a regexp, or it can be confused with routes that contain "/ocs" somewhere along the URI +var ProxyWwwAuthenticate = []string{"ocs"} + type statusRecorder struct { http.ResponseWriter status int diff --git a/proxy/pkg/middleware/basic_auth.go b/proxy/pkg/middleware/basic_auth.go index 48055f55746..cd20cf0b67f 100644 --- a/proxy/pkg/middleware/basic_auth.go +++ b/proxy/pkg/middleware/basic_auth.go @@ -35,7 +35,11 @@ func BasicAuth(optionSetters ...Option) func(next http.Handler) http.Handler { // if we want to prevent duplicated Www-Authenticate headers coming from Reva consider using w.Header().Del("Www-Authenticate") // but this will require the proxy being aware of endpoints which authentication fallback to Reva. if !h.isPublicLink(req) { - w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", "Basic", req.Host)) + for i := 0; i < len(ProxyWwwAuthenticate); i++ { + if strings.Contains(req.RequestURI, fmt.Sprintf("/%v/", ProxyWwwAuthenticate[i])) { + w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", "Basic", req.Host)) + } + } } next.ServeHTTP(w, req) return @@ -43,7 +47,11 @@ func BasicAuth(optionSetters ...Option) func(next http.Handler) http.Handler { account, ok := h.getAccount(req) if !ok { - w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", "Basic", req.Host)) + for i := 0; i < len(ProxyWwwAuthenticate); i++ { + if strings.Contains(req.RequestURI, fmt.Sprintf("/%v/", ProxyWwwAuthenticate[i])) { + w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", "Basic", req.Host)) + } + } w.WriteHeader(http.StatusUnauthorized) return } diff --git a/proxy/pkg/middleware/oidc_auth.go b/proxy/pkg/middleware/oidc_auth.go index 112a86f581a..73b46cc0e63 100644 --- a/proxy/pkg/middleware/oidc_auth.go +++ b/proxy/pkg/middleware/oidc_auth.go @@ -47,7 +47,12 @@ func OIDCAuth(optionSetters ...Option) func(next http.Handler) http.Handler { // this means that requests such as: // curl -v -k -u admin:admin -H "depth: 0" -X PROPFIND https://localhost:9200/remote.php/dav/files | xmllint --format - // even when succeeding, will contain a Www-Authenticate header. - w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", "Bearer", req.Host)) + + for i := 0; i < len(ProxyWwwAuthenticate); i++ { + if strings.Contains(req.RequestURI, fmt.Sprintf("/%v/", ProxyWwwAuthenticate[i])) { + w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", "Bearer", req.Host)) + } + } next.ServeHTTP(w, req) return } From 752cd4f62684ce9f0167e992b021eb7c58f4dd15 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Wed, 2 Dec 2020 12:04:09 +0100 Subject: [PATCH 04/22] first draft for configuring user agent multiplex on ocis --- ocis/pkg/command/storagefrontend.go | 4 ++++ proxy/pkg/middleware/authentication.go | 1 - proxy/pkg/middleware/basic_auth.go | 2 -- storage/pkg/command/frontend.go | 16 +++++++++++++--- storage/pkg/config/config.go | 9 +++++++++ storage/pkg/flagset/frontend.go | 8 ++++++++ 6 files changed, 34 insertions(+), 6 deletions(-) diff --git a/ocis/pkg/command/storagefrontend.go b/ocis/pkg/command/storagefrontend.go index 5b7296d2794..8f56ad64c43 100644 --- a/ocis/pkg/command/storagefrontend.go +++ b/ocis/pkg/command/storagefrontend.go @@ -21,6 +21,10 @@ func StorageFrontendCommand(cfg *config.Config) *cli.Command { Action: func(c *cli.Context) error { scfg := configureStorageFrontend(cfg) + if err := command.Frontend(scfg).Before(c); err != nil { + return err + } + return cli.HandleAction( command.Frontend(scfg).Action, c, diff --git a/proxy/pkg/middleware/authentication.go b/proxy/pkg/middleware/authentication.go index 16a0f5d3c61..61d8be138c8 100644 --- a/proxy/pkg/middleware/authentication.go +++ b/proxy/pkg/middleware/authentication.go @@ -64,7 +64,6 @@ func Authentication(opts ...Option) func(next http.Handler) http.Handler { if options.OIDCIss == "" && options.EnableBasicAuth { basic(next).ServeHTTP(w, r) } - }) } } diff --git a/proxy/pkg/middleware/basic_auth.go b/proxy/pkg/middleware/basic_auth.go index cd20cf0b67f..eb4cf7f3d5e 100644 --- a/proxy/pkg/middleware/basic_auth.go +++ b/proxy/pkg/middleware/basic_auth.go @@ -61,8 +61,6 @@ func BasicAuth(optionSetters ...Option) func(next http.Handler) http.Handler { Iss: oidcIss, } - fmt.Printf("\n\nHGAHAHAHA\n\n") - next.ServeHTTP(w, req.WithContext(oidc.NewContext(req.Context(), claims))) }, ) diff --git a/storage/pkg/command/frontend.go b/storage/pkg/command/frontend.go index 9dd742a0ccd..80e1225a619 100644 --- a/storage/pkg/command/frontend.go +++ b/storage/pkg/command/frontend.go @@ -6,6 +6,7 @@ import ( "os" "os/signal" "path" + "strings" "time" "github.com/cs3org/reva/cmd/revad/runtime" @@ -26,6 +27,17 @@ func Frontend(cfg *config.Config) *cli.Command { Before: func(c *cli.Context) error { cfg.Reva.Frontend.Services = c.StringSlice("service") + cfg.Reva.Frontend.Middleware.Auth.CredentialsByUserAgent = make(map[string]string, 0) + uaw := c.StringSlice("user-agent-whitelist") + for _, v := range uaw { + parts := strings.Split(v, ":") + if len(parts) != 2 { + return fmt.Errorf("unexpected config value for user-agent whitelist: %v, expected format is user-agent:challenge", v) // TODO wording + error wrapping? + } + + cfg.Reva.Frontend.Middleware.Auth.CredentialsByUserAgent[parts[0]] = parts[1] + } + return nil }, Action: func(c *cli.Context) error { @@ -116,9 +128,7 @@ func Frontend(cfg *config.Config) *cli.Command { "allow_credentials": true, }, "auth": map[string]interface{}{ - "credentials_by_user_agent": map[string]string{ - "mirall": "basic", - }, + "credentials_by_user_agent": cfg.Reva.Frontend.Middleware.Auth.CredentialsByUserAgent, }, }, // TODO build services dynamically diff --git a/storage/pkg/config/config.go b/storage/pkg/config/config.go index 69c48d83262..33d70c2148f 100644 --- a/storage/pkg/config/config.go +++ b/storage/pkg/config/config.go @@ -83,6 +83,15 @@ type FrontendPort struct { OCDavPrefix string OCSPrefix string PublicURL string + Middleware Middleware +} + +type Middleware struct { + Auth Auth +} + +type Auth struct { + CredentialsByUserAgent map[string]string } // DataGatewayPort has a public url diff --git a/storage/pkg/flagset/frontend.go b/storage/pkg/flagset/frontend.go index dcada4d8bf1..a3267ed736d 100644 --- a/storage/pkg/flagset/frontend.go +++ b/storage/pkg/flagset/frontend.go @@ -133,6 +133,14 @@ func FrontendWithConfig(cfg *config.Config) []cli.Flag { EnvVars: []string{"STORAGE_FRONTEND_UPLOAD_HTTP_METHOD_OVERRIDE"}, Destination: &cfg.Reva.UploadHTTPMethodOverride, }, + + // Middlewares + &cli.StringSliceFlag{ + Name: "user-agent-whitelist", // TODO naming? + Value: cli.NewStringSlice("test"), + Usage: "TODO", + EnvVars: []string{"STORAGE_FRONTEND_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT"}, + }, } flags = append(flags, TracingWithConfig(cfg)...) From 2910e88ba5d8af0586d25c36bd99b3a15860d24a Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Wed, 2 Dec 2020 15:31:17 +0100 Subject: [PATCH 05/22] ugly working draft --- proxy/pkg/command/server.go | 15 ++++++++++-- proxy/pkg/config/config.go | 11 ++++++++- proxy/pkg/flagset/flagset.go | 9 +++++++- proxy/pkg/middleware/authentication.go | 2 ++ proxy/pkg/middleware/basic_auth.go | 32 +++++++++++++++++++++++--- proxy/pkg/middleware/oidc_auth.go | 15 ++++++------ proxy/pkg/middleware/options.go | 9 ++++++++ storage/pkg/command/frontend.go | 2 +- storage/pkg/flagset/frontend.go | 2 +- 9 files changed, 81 insertions(+), 16 deletions(-) diff --git a/proxy/pkg/command/server.go b/proxy/pkg/command/server.go index 1d463667999..5735707688f 100644 --- a/proxy/pkg/command/server.go +++ b/proxy/pkg/command/server.go @@ -3,6 +3,7 @@ package command import ( "context" "crypto/tls" + "fmt" "net/http" "os" "os/signal" @@ -48,8 +49,17 @@ func Server(cfg *config.Config) *cli.Command { } cfg.PreSignedURL.AllowedHTTPMethods = ctx.StringSlice("presignedurl-allow-method") - // When running on single binary mode the before hook from the root command won't get called. We manually - // call this before hook from ocis command, so the configuration can be loaded. + cfg.Reva.Middleware.Auth.CredentialsByUserAgent = make(map[string]string, 0) + uaw := ctx.StringSlice("proxy-user-agent-whitelist") + for _, v := range uaw { + parts := strings.Split(v, ":") + if len(parts) != 2 { + return fmt.Errorf("unexpected config value for user-agent whitelist: %v, expected format is userAgent:challenge", v) + } + + cfg.Reva.Middleware.Auth.CredentialsByUserAgent[parts[0]] = parts[1] + } + return ParseConfig(ctx, cfg) }, Action: func(c *cli.Context) error { @@ -288,6 +298,7 @@ func loadMiddlewares(ctx context.Context, l log.Logger, cfg *config.Config) alic middleware.EnableBasicAuth(cfg.EnableBasicAuth), middleware.AccountsClient(accountsClient), middleware.OIDCIss(cfg.OIDC.Issuer), + middleware.CredentialsByUserAgent(cfg.Reva.Middleware.Auth.CredentialsByUserAgent), ), middleware.SignedURLAuth( middleware.Logger(l), diff --git a/proxy/pkg/config/config.go b/proxy/pkg/config/config.go index 388d8191424..769f597f1fd 100644 --- a/proxy/pkg/config/config.go +++ b/proxy/pkg/config/config.go @@ -80,7 +80,16 @@ var ( // Reva defines all available REVA configuration. type Reva struct { - Address string + Address string + Middleware Middleware +} + +type Middleware struct { + Auth Auth +} + +type Auth struct { + CredentialsByUserAgent map[string]string } // Cache is a TTL cache configuration. diff --git a/proxy/pkg/flagset/flagset.go b/proxy/pkg/flagset/flagset.go index 3a1371a40fa..35a60852b52 100644 --- a/proxy/pkg/flagset/flagset.go +++ b/proxy/pkg/flagset/flagset.go @@ -248,8 +248,15 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { EnvVars: []string{"PROXY_ENABLE_BASIC_AUTH"}, Destination: &cfg.EnableBasicAuth, }, - } + // Reva Middlewares Config + &cli.StringSliceFlag{ + Name: "proxy-user-agent-whitelist", // TODO naming? + Value: cli.NewStringSlice(""), + Usage: "TODO", + EnvVars: []string{"PROXY_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT"}, + }, + } } // ListProxyWithConfig applies the config to the list commands flags. diff --git a/proxy/pkg/middleware/authentication.go b/proxy/pkg/middleware/authentication.go index 61d8be138c8..79a3964d0a8 100644 --- a/proxy/pkg/middleware/authentication.go +++ b/proxy/pkg/middleware/authentication.go @@ -42,6 +42,7 @@ func Authentication(opts ...Option) func(next http.Handler) http.Handler { OIDCIss(options.OIDCIss), TokenCacheSize(options.UserinfoCacheSize), TokenCacheTTL(time.Second*time.Duration(options.UserinfoCacheTTL)), + CredentialsByUserAgent(options.CredentialsByUserAgent), ) basic := BasicAuth( @@ -49,6 +50,7 @@ func Authentication(opts ...Option) func(next http.Handler) http.Handler { EnableBasicAuth(options.EnableBasicAuth), AccountsClient(options.AccountsClient), OIDCIss(options.OIDCIss), + CredentialsByUserAgent(options.CredentialsByUserAgent), ) return func(next http.Handler) http.Handler { diff --git a/proxy/pkg/middleware/basic_auth.go b/proxy/pkg/middleware/basic_auth.go index eb4cf7f3d5e..c0bea0fb7cf 100644 --- a/proxy/pkg/middleware/basic_auth.go +++ b/proxy/pkg/middleware/basic_auth.go @@ -37,21 +37,47 @@ func BasicAuth(optionSetters ...Option) func(next http.Handler) http.Handler { if !h.isPublicLink(req) { for i := 0; i < len(ProxyWwwAuthenticate); i++ { if strings.Contains(req.RequestURI, fmt.Sprintf("/%v/", ProxyWwwAuthenticate[i])) { + for k, v := range options.CredentialsByUserAgent { + if strings.Contains(k, req.UserAgent()) { + w.Header().Del("Www-Authenticate") + w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(v), req.Host)) + goto OUT + } + } w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", "Basic", req.Host)) } } } + OUT: next.ServeHTTP(w, req) return } + w.Header().Del("Www-Authenticate") + account, ok := h.getAccount(req) + + // touch is a user agent locking guard, when touched changes to true it indicates the User-Agent on the + // request is configured to support only one challenge, it it remains untouched, there are no considera- + // tions and we should write all available authentication challenges to the response. + touch := false + if !ok { - for i := 0; i < len(ProxyWwwAuthenticate); i++ { - if strings.Contains(req.RequestURI, fmt.Sprintf("/%v/", ProxyWwwAuthenticate[i])) { - w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", "Basic", req.Host)) + // if the request is bound to a user agent the locked write Www-Authenticate for such user + for k, v := range options.CredentialsByUserAgent { + if strings.Contains(k, req.UserAgent()) { + w.Header().Del("Www-Authenticate") + w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(v), req.Host)) + touch = true + break } } + + // if the request is not bound to any user agent, write all available challenges + if !touch { + writeSupportedAuthenticateHeader(w, req) + } + w.WriteHeader(http.StatusUnauthorized) return } diff --git a/proxy/pkg/middleware/oidc_auth.go b/proxy/pkg/middleware/oidc_auth.go index 73b46cc0e63..b91c3b39556 100644 --- a/proxy/pkg/middleware/oidc_auth.go +++ b/proxy/pkg/middleware/oidc_auth.go @@ -41,18 +41,19 @@ func OIDCAuth(optionSetters ...Option) func(next http.Handler) http.Handler { // there is no bearer token on the request, if !h.shouldServe(req) { // oidc supported but token not present, add header and handover to the next middleware. - - // TODO for this logic to work and we don't return superfluous Www-Authenticate headers we would need to - // add Www-Authenticate only on selected endpoints, because Reva won't cleanup already written headers. - // this means that requests such as: - // curl -v -k -u admin:admin -H "depth: 0" -X PROPFIND https://localhost:9200/remote.php/dav/files | xmllint --format - - // even when succeeding, will contain a Www-Authenticate header. - for i := 0; i < len(ProxyWwwAuthenticate); i++ { if strings.Contains(req.RequestURI, fmt.Sprintf("/%v/", ProxyWwwAuthenticate[i])) { + for k, v := range options.CredentialsByUserAgent { + if strings.Contains(k, req.UserAgent()) { + w.Header().Del("Www-Authenticate") + w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(v), req.Host)) + goto OUT + } + } w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", "Bearer", req.Host)) } } + OUT: next.ServeHTTP(w, req) return } diff --git a/proxy/pkg/middleware/options.go b/proxy/pkg/middleware/options.go index 9d5d0c1ca96..27906f9a5c3 100644 --- a/proxy/pkg/middleware/options.go +++ b/proxy/pkg/middleware/options.go @@ -46,6 +46,8 @@ type Options struct { UserinfoCacheSize int // UserinfoCacheTTL sets the max cache duration for the userinfo cache, intended for the oidc_auth middleware UserinfoCacheTTL time.Duration + // CredentialsByUserAgent sets the auth challenges on a per user-agent basis + CredentialsByUserAgent map[string]string } // newOptions initializes the available default options. @@ -108,6 +110,13 @@ func OIDCIss(iss string) Option { } } +// CredentialsByUserAgent sets UserAgentChallenges. +func CredentialsByUserAgent(v map[string]string) Option { + return func(o *Options) { + o.CredentialsByUserAgent = v + } +} + // RevaGatewayClient provides a function to set the the reva gateway service client option. func RevaGatewayClient(gc gateway.GatewayAPIClient) Option { return func(o *Options) { diff --git a/storage/pkg/command/frontend.go b/storage/pkg/command/frontend.go index 80e1225a619..7b6dac0f1e5 100644 --- a/storage/pkg/command/frontend.go +++ b/storage/pkg/command/frontend.go @@ -32,7 +32,7 @@ func Frontend(cfg *config.Config) *cli.Command { for _, v := range uaw { parts := strings.Split(v, ":") if len(parts) != 2 { - return fmt.Errorf("unexpected config value for user-agent whitelist: %v, expected format is user-agent:challenge", v) // TODO wording + error wrapping? + return fmt.Errorf("unexpected config value for user-agent whitelist: %v, expected format is user-agent:challenge", v) } cfg.Reva.Frontend.Middleware.Auth.CredentialsByUserAgent[parts[0]] = parts[1] diff --git a/storage/pkg/flagset/frontend.go b/storage/pkg/flagset/frontend.go index a3267ed736d..74c60ebdc08 100644 --- a/storage/pkg/flagset/frontend.go +++ b/storage/pkg/flagset/frontend.go @@ -134,7 +134,7 @@ func FrontendWithConfig(cfg *config.Config) []cli.Flag { Destination: &cfg.Reva.UploadHTTPMethodOverride, }, - // Middlewares + // Reva Middlewares Config &cli.StringSliceFlag{ Name: "user-agent-whitelist", // TODO naming? Value: cli.NewStringSlice("test"), From e4974e020d8bd78cd706d4a84bc29bbcf8436bd8 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Wed, 2 Dec 2020 15:51:39 +0100 Subject: [PATCH 06/22] minimal refactor --- proxy/pkg/middleware/authentication.go | 22 ++++++++++++++++++++++ proxy/pkg/middleware/basic_auth.go | 22 +++------------------- proxy/pkg/middleware/oidc_auth.go | 15 +-------------- 3 files changed, 26 insertions(+), 33 deletions(-) diff --git a/proxy/pkg/middleware/authentication.go b/proxy/pkg/middleware/authentication.go index 79a3964d0a8..dccef744fdf 100644 --- a/proxy/pkg/middleware/authentication.go +++ b/proxy/pkg/middleware/authentication.go @@ -75,3 +75,25 @@ func writeSupportedAuthenticateHeader(w http.ResponseWriter, r *http.Request) { w.Header().Add("WWW-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(SupportedAuthStrategies[i]), r.Host)) } } + +func removeSuperfluousAuthenticate(w http.ResponseWriter) { + w.Header().Del("Www-Authenticate") +} + +// userAgentAuthenticateLockIn sets Www-Authenticate according to configured user agents. This is useful for the case of +// legacy clients that do not support protocols like OIDC or OAuth and want to lock a given user agent to a challenge, +// such as basic. More info in https://github.com/cs3org/reva/pull/1350 +func userAgentAuthenticateLockIn(w http.ResponseWriter, req *http.Request, creds map[string]string, fallback string) { + for i := 0; i < len(ProxyWwwAuthenticate); i++ { + if strings.Contains(req.RequestURI, fmt.Sprintf("/%v/", ProxyWwwAuthenticate[i])) { + for k, v := range creds { + if strings.Contains(k, req.UserAgent()) { + removeSuperfluousAuthenticate(w) + w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(v), req.Host)) + return + } + } + w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(fallback), req.Host)) + } + } +} diff --git a/proxy/pkg/middleware/basic_auth.go b/proxy/pkg/middleware/basic_auth.go index c0bea0fb7cf..c16eb8defbc 100644 --- a/proxy/pkg/middleware/basic_auth.go +++ b/proxy/pkg/middleware/basic_auth.go @@ -32,29 +32,14 @@ func BasicAuth(optionSetters ...Option) func(next http.Handler) http.Handler { return http.HandlerFunc( func(w http.ResponseWriter, req *http.Request) { if h.isPublicLink(req) || !h.isBasicAuth(req) { - // if we want to prevent duplicated Www-Authenticate headers coming from Reva consider using w.Header().Del("Www-Authenticate") - // but this will require the proxy being aware of endpoints which authentication fallback to Reva. if !h.isPublicLink(req) { - for i := 0; i < len(ProxyWwwAuthenticate); i++ { - if strings.Contains(req.RequestURI, fmt.Sprintf("/%v/", ProxyWwwAuthenticate[i])) { - for k, v := range options.CredentialsByUserAgent { - if strings.Contains(k, req.UserAgent()) { - w.Header().Del("Www-Authenticate") - w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(v), req.Host)) - goto OUT - } - } - w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", "Basic", req.Host)) - } - } + userAgentAuthenticateLockIn(w, req, options.CredentialsByUserAgent, "basic") } - OUT: next.ServeHTTP(w, req) return } - w.Header().Del("Www-Authenticate") - + removeSuperfluousAuthenticate(w) account, ok := h.getAccount(req) // touch is a user agent locking guard, when touched changes to true it indicates the User-Agent on the @@ -63,10 +48,9 @@ func BasicAuth(optionSetters ...Option) func(next http.Handler) http.Handler { touch := false if !ok { - // if the request is bound to a user agent the locked write Www-Authenticate for such user for k, v := range options.CredentialsByUserAgent { if strings.Contains(k, req.UserAgent()) { - w.Header().Del("Www-Authenticate") + removeSuperfluousAuthenticate(w) w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(v), req.Host)) touch = true break diff --git a/proxy/pkg/middleware/oidc_auth.go b/proxy/pkg/middleware/oidc_auth.go index b91c3b39556..098ff8d079e 100644 --- a/proxy/pkg/middleware/oidc_auth.go +++ b/proxy/pkg/middleware/oidc_auth.go @@ -3,7 +3,6 @@ package middleware import ( "context" "encoding/json" - "fmt" "net/http" "strings" "time" @@ -41,19 +40,7 @@ func OIDCAuth(optionSetters ...Option) func(next http.Handler) http.Handler { // there is no bearer token on the request, if !h.shouldServe(req) { // oidc supported but token not present, add header and handover to the next middleware. - for i := 0; i < len(ProxyWwwAuthenticate); i++ { - if strings.Contains(req.RequestURI, fmt.Sprintf("/%v/", ProxyWwwAuthenticate[i])) { - for k, v := range options.CredentialsByUserAgent { - if strings.Contains(k, req.UserAgent()) { - w.Header().Del("Www-Authenticate") - w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(v), req.Host)) - goto OUT - } - } - w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", "Bearer", req.Host)) - } - } - OUT: + userAgentAuthenticateLockIn(w, req, options.CredentialsByUserAgent, "bearer") next.ServeHTTP(w, req) return } From 6cd2a3fcd66835cc1f5275c6f6fd7eef3f592c6c Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Wed, 2 Dec 2020 16:02:33 +0100 Subject: [PATCH 07/22] use upstream PR provisionally --- ocis/go.mod | 3 +-- ocis/go.sum | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ocis/go.mod b/ocis/go.mod index fca8aa036b0..3ff6df81fbd 100644 --- a/ocis/go.mod +++ b/ocis/go.mod @@ -7,7 +7,6 @@ require ( contrib.go.opencensus.io/exporter/ocagent v0.7.0 contrib.go.opencensus.io/exporter/zipkin v0.1.2 github.com/UnnoTed/fileb0x v1.1.4 - github.com/cs3org/reva v1.4.1-0.20201201074212-8b4cc174b708 // indirect github.com/go-test/deep v1.0.6 // indirect github.com/gopherjs/gopherjs v0.0.0-20200217142428-fce0ec30dd00 // indirect github.com/micro/cli/v2 v2.1.2 @@ -38,7 +37,7 @@ require ( ) replace ( - github.com/cs3org/reva => ../../../reva + github.com/cs3org/reva => github.com/labkode/reva v0.0.0-20201202134237-befa4a5708b6 github.com/gomodule/redigo => github.com/gomodule/redigo v1.8.2 github.com/owncloud/ocis/accounts => ../accounts github.com/owncloud/ocis/glauth => ../glauth diff --git a/ocis/go.sum b/ocis/go.sum index 02495531bd5..3f752494ba1 100644 --- a/ocis/go.sum +++ b/ocis/go.sum @@ -925,6 +925,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/labbsr0x/bindman-dns-webhook v1.0.2/go.mod h1:p6b+VCXIR8NYKpDr8/dg1HKfQoRHCdcsROXKvmoehKA= github.com/labbsr0x/goh v1.0.1/go.mod h1:8K2UhVoaWXcCU7Lxoa2omWnC8gyW8px7/lmO61c027w= +github.com/labkode/reva v0.0.0-20201202134237-befa4a5708b6 h1:uFp/S+omWwuzbI9OW8NbX4bkY4Fm6akaQ9z54s+x2mQ= +github.com/labkode/reva v0.0.0-20201202134237-befa4a5708b6/go.mod h1:MTBlfobTE8W2hgXQ9+r+75jpJa1TxD04IZm5TpS9H48= github.com/labstack/echo v3.2.1+incompatible h1:J2M7YArHx4gi8p/3fDw8tX19SXhBCoRpviyAZSN3I88= github.com/labstack/echo v3.2.1+incompatible/go.mod h1:0INS7j/VjnFxD4E2wkz67b8cVwCLbBmJyDaka6Cmk1s= github.com/labstack/gommon v0.2.7 h1:2qOPq/twXDrQ6ooBGrn3mrmVOC+biLlatwgIu8lbzRM= From c1dcfb655ff19eaa1304261b67da9bd5e503dd13 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Wed, 2 Dec 2020 16:07:04 +0100 Subject: [PATCH 08/22] undo pprof endpoint --- accounts/pkg/command/server.go | 8 -------- accounts/pkg/server/http/server.go | 8 -------- 2 files changed, 16 deletions(-) diff --git a/accounts/pkg/command/server.go b/accounts/pkg/command/server.go index e6ce9e76454..b5edee2db99 100644 --- a/accounts/pkg/command/server.go +++ b/accounts/pkg/command/server.go @@ -2,11 +2,8 @@ package command import ( "context" - gohttp "net/http" - _ "net/http/pprof" "os" "os/signal" - "runtime" "strings" "github.com/micro/cli/v2" @@ -112,11 +109,6 @@ func Server(cfg *config.Config) *cli.Command { cancel() }) } - runtime.SetBlockProfileRate(1) - runtime.SetMutexProfileFraction(1) - go func() { - gohttp.ListenAndServe(":8887", nil) - }() return gr.Run() }, diff --git a/accounts/pkg/server/http/server.go b/accounts/pkg/server/http/server.go index 81678a71b11..4514d412ac0 100644 --- a/accounts/pkg/server/http/server.go +++ b/accounts/pkg/server/http/server.go @@ -1,12 +1,7 @@ package http import ( - _ "net/http/pprof" - - ghttp "net/http" - "github.com/go-chi/chi" - cmw "github.com/go-chi/chi/middleware" "github.com/owncloud/ocis/accounts/pkg/assets" "github.com/owncloud/ocis/accounts/pkg/proto/v0" "github.com/owncloud/ocis/accounts/pkg/version" @@ -32,9 +27,6 @@ func Server(opts ...Option) http.Service { mux := chi.NewMux() - mux.Use(func(next ghttp.Handler) ghttp.Handler { - return cmw.Profiler() - }) mux.Use(middleware.RealIP) mux.Use(middleware.RequestID) mux.Use(middleware.NoCache) From a9922343efb26f1c61d1e331751ca060869b1f39 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Thu, 3 Dec 2020 10:57:32 +0100 Subject: [PATCH 09/22] explain flags --- proxy/pkg/flagset/flagset.go | 7 +++---- storage/pkg/command/frontend.go | 1 + storage/pkg/flagset/frontend.go | 5 ++--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/proxy/pkg/flagset/flagset.go b/proxy/pkg/flagset/flagset.go index 35a60852b52..b8ea4d252a4 100644 --- a/proxy/pkg/flagset/flagset.go +++ b/proxy/pkg/flagset/flagset.go @@ -225,7 +225,7 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { Destination: &cfg.AutoprovisionAccounts, }, - // Presigned URLs + // Pre Signed URLs &cli.StringSliceFlag{ Name: "presignedurl-allow-method", Value: cli.NewStringSlice("GET"), @@ -251,9 +251,8 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { // Reva Middlewares Config &cli.StringSliceFlag{ - Name: "proxy-user-agent-whitelist", // TODO naming? - Value: cli.NewStringSlice(""), - Usage: "TODO", + Name: "proxy-user-agent-whitelist", + Usage: "--user-agent-whitelist=mirall:basic,foo:bearer Given a tuple of [UserAgent:challenge] it locks a given user agent to the authentication challenge. Particularly useful for old clients whose USer-Agent is known and only support one authentication challenge. When this flag is set in the proxy it configures the authentication middlewares.", EnvVars: []string{"PROXY_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT"}, }, } diff --git a/storage/pkg/command/frontend.go b/storage/pkg/command/frontend.go index 7b6dac0f1e5..f3776587750 100644 --- a/storage/pkg/command/frontend.go +++ b/storage/pkg/command/frontend.go @@ -29,6 +29,7 @@ func Frontend(cfg *config.Config) *cli.Command { cfg.Reva.Frontend.Middleware.Auth.CredentialsByUserAgent = make(map[string]string, 0) uaw := c.StringSlice("user-agent-whitelist") + fmt.Printf("\n\n%v\n\n", uaw) for _, v := range uaw { parts := strings.Split(v, ":") if len(parts) != 2 { diff --git a/storage/pkg/flagset/frontend.go b/storage/pkg/flagset/frontend.go index 74c60ebdc08..0a5bdb5651e 100644 --- a/storage/pkg/flagset/frontend.go +++ b/storage/pkg/flagset/frontend.go @@ -136,9 +136,8 @@ func FrontendWithConfig(cfg *config.Config) []cli.Flag { // Reva Middlewares Config &cli.StringSliceFlag{ - Name: "user-agent-whitelist", // TODO naming? - Value: cli.NewStringSlice("test"), - Usage: "TODO", + Name: "user-agent-whitelist", + Usage: "--user-agent-whitelist=mirall:basic,foo:bearer Given a tuple of comma separated [UserAgent:challenge] values, it locks a given user agent to the authentication challenge. Particularly useful for old clients whose USer-Agent is known and only support one authentication challenge. When this flag is set in the storage-frontend it configures Reva.", EnvVars: []string{"STORAGE_FRONTEND_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT"}, }, } From 6753f38bf37466955cb7845188c34f91a775b90d Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Thu, 3 Dec 2020 11:51:25 +0100 Subject: [PATCH 10/22] add utility function Reverse to ocis-pkg --- ocis-pkg/conversions/strings.go | 17 ++++++++++++++++- ocis-pkg/go.sum | 5 +++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/ocis-pkg/conversions/strings.go b/ocis-pkg/conversions/strings.go index a26a5574e50..b712b321be9 100644 --- a/ocis-pkg/conversions/strings.go +++ b/ocis-pkg/conversions/strings.go @@ -1,6 +1,9 @@ package conversions -import "strings" +import ( + "strings" + "unicode/utf8" +) // StringToSliceString splits a string into a slice string according to separator func StringToSliceString(src string, sep string) []string { @@ -12,3 +15,15 @@ func StringToSliceString(src string, sep string) []string { return parts } + +// Reverse reverses a string +func Reverse(s string) string { + size := len(s) + buf := make([]byte, size) + for start := 0; start < size; { + r, n := utf8.DecodeRuneInString(s[start:]) + start += n + utf8.EncodeRune(buf[size-start:], r) + } + return string(buf) +} diff --git a/ocis-pkg/go.sum b/ocis-pkg/go.sum index b3e6cb68966..9c2832d9f26 100644 --- a/ocis-pkg/go.sum +++ b/ocis-pkg/go.sum @@ -118,6 +118,7 @@ github.com/aws/aws-sdk-go v1.34.12 h1:7UbBEYDUa4uW0YmRnOd806MS1yoJMcaodBWDzvBShA github.com/aws/aws-sdk-go v1.34.12/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= github.com/aws/aws-sdk-go v1.35.9 h1:b1HiUpdkFLJyoOQ7zas36YHzjNHH0ivHx/G5lWBeg+U= github.com/aws/aws-sdk-go v1.35.9/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k= +github.com/aws/aws-sdk-go v1.35.23 h1:SCP0d0XvyJTDmfnHEQPvBaYi3kea1VNUo7uQmkVgFts= github.com/aws/aws-sdk-go v1.35.23/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k= github.com/aws/aws-xray-sdk-go v0.9.4/go.mod h1:XtMKdBQfpVut+tJEwI7+dJFRxxRdxHDyVNp2tHXRq04= github.com/baiyubin/aliyun-sts-go-sdk v0.0.0-20180326062324-cfa1a18b161f/go.mod h1:AuiFmCCPBSrqvVMvuqFuk0qogytodnVFVSN5CeJB8Gc= @@ -207,6 +208,7 @@ github.com/cs3org/go-cs3apis v0.0.0-20200810113633-b00aca449666 h1:E7VsSSN/2YZLS github.com/cs3org/go-cs3apis v0.0.0-20200810113633-b00aca449666/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY= github.com/cs3org/go-cs3apis v0.0.0-20201007120910-416ed6cf8b00 h1:LVl25JaflluOchVvaHWtoCynm5OaM+VNai0IYkcCSe0= github.com/cs3org/go-cs3apis v0.0.0-20201007120910-416ed6cf8b00/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY= +github.com/cs3org/go-cs3apis v0.0.0-20201118090759-87929f5bae21 h1:mZpylrgnCgSeaZ5EznvHIPIKuaQHMHZDi2wkJtk4M8Y= github.com/cs3org/go-cs3apis v0.0.0-20201118090759-87929f5bae21/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY= github.com/cs3org/reva v1.1.0 h1:Gih6ECHvMMGSx523SFluFlDmNMuhYelXYShdWvjvW38= github.com/cs3org/reva v1.1.0/go.mod h1:fBzTrNuAKdQ62ybjpdu8nyhBin90/3/3s6DGQDCdBp4= @@ -316,6 +318,7 @@ github.com/go-ozzo/ozzo-validation/v4 v4.2.1 h1:XALUNshPYumA7UShB7iM3ZVlqIBn0jfw github.com/go-ozzo/ozzo-validation/v4 v4.2.1/go.mod h1:2NKgrcHl3z6cJs+3Oo940FPRiTzuqKbvfrL2RxCj6Ew= github.com/go-sql-driver/mysql v1.4.0/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w= github.com/go-sql-driver/mysql v1.4.1/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w= +github.com/go-sql-driver/mysql v1.5.0 h1:ozyZYNQW3x3HtqT1jira07DN2PArx2v7/mN66gGcHOs= github.com/go-sql-driver/mysql v1.5.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= github.com/go-telegram-bot-api/telegram-bot-api v4.6.4+incompatible/go.mod h1:qf9acutJ8cwBUhm1bqgz6Bei9/C/c93FPDljKWwsOgM= @@ -946,6 +949,7 @@ github.com/pkg/term v0.0.0-20200520122047-c3ffed290a03/go.mod h1:Z9+Ul5bCbBKnbCv github.com/pkg/term v1.1.0/go.mod h1:E25nymQcrSllhX42Ok8MRm1+hyBdHY0dCeiKZ9jpNGw= github.com/pkg/xattr v0.4.1 h1:dhclzL6EqOXNaPDWqoeb9tIxATfBSmjqL0b4DpSjwRw= github.com/pkg/xattr v0.4.1/go.mod h1:W2cGD0TBEus7MkUgv0tNZ9JutLtVO3cXu+IBRuHqnFs= +github.com/pkg/xattr v0.4.2 h1:fbVxr9lvkToTGgPljVszvFsOdcbSv5BmGABneyxRgZM= github.com/pkg/xattr v0.4.2/go.mod h1:sBD3RAqlr8Q+RC3FutZcikpT8nyDrIEEBw2J744gVWs= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -1403,6 +1407,7 @@ golang.org/x/sys v0.0.0-20200720211630-cb9d2d5c5666/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200909081042-eff7692f9009/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200918174421-af09f7315aff h1:1CPUrky56AcgSpxz/KfgzQWzfG09u5YOL8MvPYBlrL8= golang.org/x/sys v0.0.0-20200918174421-af09f7315aff/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20201101102859-da207088b7d1 h1:a/mKvvZr9Jcc8oKfcmgzyp7OwF73JPWsQLvH1z2Kxck= golang.org/x/sys v0.0.0-20201101102859-da207088b7d1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= From ef4e573e42f0f9b4a28f1976f3da7c236462ee09 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Thu, 3 Dec 2020 11:53:19 +0100 Subject: [PATCH 11/22] split string by reversing it --- proxy/pkg/command/server.go | 33 +++++++++++++++++++++--------- storage/pkg/command/frontend.go | 36 ++++++++++++++++++++------------- 2 files changed, 46 insertions(+), 23 deletions(-) diff --git a/proxy/pkg/command/server.go b/proxy/pkg/command/server.go index 5735707688f..37808085284 100644 --- a/proxy/pkg/command/server.go +++ b/proxy/pkg/command/server.go @@ -20,6 +20,7 @@ import ( openzipkin "github.com/openzipkin/zipkin-go" zipkinhttp "github.com/openzipkin/zipkin-go/reporter/http" acc "github.com/owncloud/ocis/accounts/pkg/proto/v0" + "github.com/owncloud/ocis/ocis-pkg/conversions" "github.com/owncloud/ocis/ocis-pkg/log" "github.com/owncloud/ocis/ocis-pkg/service/grpc" "github.com/owncloud/ocis/proxy/pkg/config" @@ -49,15 +50,8 @@ func Server(cfg *config.Config) *cli.Command { } cfg.PreSignedURL.AllowedHTTPMethods = ctx.StringSlice("presignedurl-allow-method") - cfg.Reva.Middleware.Auth.CredentialsByUserAgent = make(map[string]string, 0) - uaw := ctx.StringSlice("proxy-user-agent-whitelist") - for _, v := range uaw { - parts := strings.Split(v, ":") - if len(parts) != 2 { - return fmt.Errorf("unexpected config value for user-agent whitelist: %v, expected format is userAgent:challenge", v) - } - - cfg.Reva.Middleware.Auth.CredentialsByUserAgent[parts[0]] = parts[1] + if err := loadUserAgent(ctx, cfg); err != nil { + return err } return ParseConfig(ctx, cfg) @@ -322,3 +316,24 @@ func loadMiddlewares(ctx context.Context, l log.Logger, cfg *config.Config) alic ), ) } + +// loadUserAgent reads the proxy-user-agent-whitelist, since it is a string flag, and attempts to construct a map of +// "user-agent":"challenge" locks in for Reva. +// Modifies cfg. Spaces don't need to be trimmed as urfavecli takes care of it. User agents with spaces are valid. i.e: +// Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:83.0) Gecko/20100101 Firefox/83.0 +func loadUserAgent(c *cli.Context, cfg *config.Config) error { + cfg.Reva.Middleware.Auth.CredentialsByUserAgent = make(map[string]string, 0) + locks := c.StringSlice("proxy-user-agent-whitelist") + + for _, v := range locks { + vv := conversions.Reverse(v) + parts := strings.SplitN(vv, ":", 2) + if len(parts) != 2 { + return fmt.Errorf("unexpected config value for user-agent lock-in: %v, expected format is user-agent:challenge", v) + } + + cfg.Reva.Middleware.Auth.CredentialsByUserAgent[conversions.Reverse(parts[1])] = conversions.Reverse(parts[0]) + } + + return nil +} diff --git a/storage/pkg/command/frontend.go b/storage/pkg/command/frontend.go index f3776587750..06c7fcc6846 100644 --- a/storage/pkg/command/frontend.go +++ b/storage/pkg/command/frontend.go @@ -13,6 +13,7 @@ import ( "github.com/gofrs/uuid" "github.com/micro/cli/v2" "github.com/oklog/run" + "github.com/owncloud/ocis/ocis-pkg/conversions" "github.com/owncloud/ocis/storage/pkg/config" "github.com/owncloud/ocis/storage/pkg/flagset" "github.com/owncloud/ocis/storage/pkg/server/debug" @@ -26,20 +27,7 @@ func Frontend(cfg *config.Config) *cli.Command { Flags: flagset.FrontendWithConfig(cfg), Before: func(c *cli.Context) error { cfg.Reva.Frontend.Services = c.StringSlice("service") - - cfg.Reva.Frontend.Middleware.Auth.CredentialsByUserAgent = make(map[string]string, 0) - uaw := c.StringSlice("user-agent-whitelist") - fmt.Printf("\n\n%v\n\n", uaw) - for _, v := range uaw { - parts := strings.Split(v, ":") - if len(parts) != 2 { - return fmt.Errorf("unexpected config value for user-agent whitelist: %v, expected format is user-agent:challenge", v) - } - - cfg.Reva.Frontend.Middleware.Auth.CredentialsByUserAgent[parts[0]] = parts[1] - } - - return nil + return loadUserAgent(c, cfg) }, Action: func(c *cli.Context) error { logger := NewLogger(cfg) @@ -313,3 +301,23 @@ func Frontend(cfg *config.Config) *cli.Command { }, } } + +// loadUserAgent reads the user-agent-whitelist, since it is a string flag, and attempts to construct a map of +// "user-agent":"challenge" locks in for Reva. +// Modifies cfg. Spaces don't need to be trimmed as urfavecli takes care of it. +func loadUserAgent(c *cli.Context, cfg *config.Config) error { + cfg.Reva.Frontend.Middleware.Auth.CredentialsByUserAgent = make(map[string]string, 0) + locks := c.StringSlice("user-agent-whitelist") + + for _, v := range locks { + vv := conversions.Reverse(v) + parts := strings.SplitN(vv, ":", 2) + if len(parts) != 2 { + return fmt.Errorf("unexpected config value for user-agent lock-in: %v, expected format is user-agent:challenge", v) + } + + cfg.Reva.Frontend.Middleware.Auth.CredentialsByUserAgent[conversions.Reverse(parts[1])] = conversions.Reverse(parts[0]) + } + + return nil +} From 8c83de7db29519cc6256c2bc5d0eca0a18c80db6 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Thu, 3 Dec 2020 12:03:59 +0100 Subject: [PATCH 12/22] document loadUserAgent --- ocis/go.mod | 2 +- ocis/go.sum | 4 ++++ proxy/pkg/command/server.go | 4 ++++ storage/pkg/command/frontend.go | 7 ++++++- 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/ocis/go.mod b/ocis/go.mod index 725a0fa7b21..a49d62b5c89 100644 --- a/ocis/go.mod +++ b/ocis/go.mod @@ -7,6 +7,7 @@ require ( contrib.go.opencensus.io/exporter/ocagent v0.7.0 contrib.go.opencensus.io/exporter/zipkin v0.1.2 github.com/UnnoTed/fileb0x v1.1.4 + github.com/cs3org/reva v1.4.1-0.20201203075131-783e35cbff51 // indirect github.com/go-test/deep v1.0.6 // indirect github.com/gopherjs/gopherjs v0.0.0-20200217142428-fce0ec30dd00 // indirect github.com/micro/cli/v2 v2.1.2 @@ -38,7 +39,6 @@ require ( ) replace ( - github.com/cs3org/reva => github.com/labkode/reva v0.0.0-20201202134237-befa4a5708b6 github.com/gomodule/redigo => github.com/gomodule/redigo v1.8.2 github.com/owncloud/ocis/accounts => ../accounts github.com/owncloud/ocis/glauth => ../glauth diff --git a/ocis/go.sum b/ocis/go.sum index 2ef150d9f1f..e7738329743 100644 --- a/ocis/go.sum +++ b/ocis/go.sum @@ -162,6 +162,8 @@ github.com/aws/aws-sdk-go v1.28.2/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN github.com/aws/aws-sdk-go v1.33.19/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= github.com/aws/aws-sdk-go v1.35.23 h1:SCP0d0XvyJTDmfnHEQPvBaYi3kea1VNUo7uQmkVgFts= github.com/aws/aws-sdk-go v1.35.23/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k= +github.com/aws/aws-sdk-go v1.35.27 h1:F0dUW+kouzchjt4X6kYfYMw1YtQPkA4pihpCDqQMrq8= +github.com/aws/aws-sdk-go v1.35.27/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k= github.com/aws/aws-xray-sdk-go v0.9.4/go.mod h1:XtMKdBQfpVut+tJEwI7+dJFRxxRdxHDyVNp2tHXRq04= github.com/baiyubin/aliyun-sts-go-sdk v0.0.0-20180326062324-cfa1a18b161f/go.mod h1:AuiFmCCPBSrqvVMvuqFuk0qogytodnVFVSN5CeJB8Gc= github.com/beevik/etree v1.1.0 h1:T0xke/WvNtMoCqgzPhkX2r4rjY3GDZFi+FjpRZY2Jbs= @@ -313,6 +315,8 @@ github.com/cs3org/reva v1.4.1-0.20201130061320-ac85e68e0600 h1:4CKU+R4UNvILzsPrc github.com/cs3org/reva v1.4.1-0.20201130061320-ac85e68e0600/go.mod h1:MTBlfobTE8W2hgXQ9+r+75jpJa1TxD04IZm5TpS9H48= github.com/cs3org/reva v1.4.1-0.20201201074212-8b4cc174b708 h1:uiz1kb5iR6V7GvpX5CW3xeZcbPTvR8P1tlvODTD0zio= github.com/cs3org/reva v1.4.1-0.20201201074212-8b4cc174b708/go.mod h1:MTBlfobTE8W2hgXQ9+r+75jpJa1TxD04IZm5TpS9H48= +github.com/cs3org/reva v1.4.1-0.20201203075131-783e35cbff51 h1:WguydLVNj9tDRAw5cz+TnkY3shiTmbpfBPX2NpDHU2c= +github.com/cs3org/reva v1.4.1-0.20201203075131-783e35cbff51/go.mod h1:UUIurLdFYag/W+3YlAqwFuVvfzF3Ps1Elq1NzsE9kvo= github.com/cznic/b v0.0.0-20181122101859-a26611c4d92d h1:SwD98825d6bdB+pEuTxWOXiSjBrHdOl/UVp75eI7JT8= github.com/cznic/b v0.0.0-20181122101859-a26611c4d92d/go.mod h1:URriBxXwVq5ijiJ12C7iIZqlA69nTlI+LgI6/pwftG8= github.com/cznic/mathutil v0.0.0-20181122101859-297441e03548 h1:iwZdTE0PVqJCos1vaoKsclOGD3ADKpshg3SRtYBbwso= diff --git a/proxy/pkg/command/server.go b/proxy/pkg/command/server.go index 37808085284..f4dc963d84d 100644 --- a/proxy/pkg/command/server.go +++ b/proxy/pkg/command/server.go @@ -321,6 +321,10 @@ func loadMiddlewares(ctx context.Context, l log.Logger, cfg *config.Config) alic // "user-agent":"challenge" locks in for Reva. // Modifies cfg. Spaces don't need to be trimmed as urfavecli takes care of it. User agents with spaces are valid. i.e: // Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:83.0) Gecko/20100101 Firefox/83.0 +// This function works by relying in our format of specifying [user-agent:challenge] and the fact that the user agent +// might contain ":" (colon), so the original string is reversed, split in two parts, by the time it is split we +// have the indexes reversed and the tuple is in the format of [challenge:user-agent], then the same process is applied +// in reverse for each individual part func loadUserAgent(c *cli.Context, cfg *config.Config) error { cfg.Reva.Middleware.Auth.CredentialsByUserAgent = make(map[string]string, 0) locks := c.StringSlice("proxy-user-agent-whitelist") diff --git a/storage/pkg/command/frontend.go b/storage/pkg/command/frontend.go index 06c7fcc6846..e67edd44235 100644 --- a/storage/pkg/command/frontend.go +++ b/storage/pkg/command/frontend.go @@ -304,7 +304,12 @@ func Frontend(cfg *config.Config) *cli.Command { // loadUserAgent reads the user-agent-whitelist, since it is a string flag, and attempts to construct a map of // "user-agent":"challenge" locks in for Reva. -// Modifies cfg. Spaces don't need to be trimmed as urfavecli takes care of it. +// Modifies cfg. Spaces don't need to be trimmed as urfavecli takes care of it. User agents with spaces are valid. i.e: +// Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:83.0) Gecko/20100101 Firefox/83.0 +// This function works by relying in our format of specifying [user-agent:challenge] and the fact that the user agent +// might contain ":" (colon), so the original string is reversed, split in two parts, by the time it is split we +// have the indexes reversed and the tuple is in the format of [challenge:user-agent], then the same process is applied +// in reverse for each individual part func loadUserAgent(c *cli.Context, cfg *config.Config) error { cfg.Reva.Frontend.Middleware.Auth.CredentialsByUserAgent = make(map[string]string, 0) locks := c.StringSlice("user-agent-whitelist") From 3870246233076f63bf86d32a3bac0a93b40f8be1 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Thu, 3 Dec 2020 12:09:55 +0100 Subject: [PATCH 13/22] remove unused dep --- accounts/go.mod | 1 - 1 file changed, 1 deletion(-) diff --git a/accounts/go.mod b/accounts/go.mod index 2c38b357b72..8bcd68cbef7 100644 --- a/accounts/go.mod +++ b/accounts/go.mod @@ -31,6 +31,5 @@ require ( replace ( github.com/owncloud/ocis/ocis-pkg => ../ocis-pkg github.com/owncloud/ocis/settings => ../settings - github.com/owncloud/ocis/storage => ../storage google.golang.org/grpc => google.golang.org/grpc v1.26.0 ) From b9df6e417ec7a0088df202fe817477fbb3b5cefc Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Thu, 3 Dec 2020 12:13:52 +0100 Subject: [PATCH 14/22] use more inclusive language --- proxy/pkg/command/server.go | 4 ++-- proxy/pkg/flagset/flagset.go | 4 ++-- storage/pkg/command/frontend.go | 4 ++-- storage/pkg/flagset/frontend.go | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/proxy/pkg/command/server.go b/proxy/pkg/command/server.go index f4dc963d84d..bd196fd7d49 100644 --- a/proxy/pkg/command/server.go +++ b/proxy/pkg/command/server.go @@ -317,7 +317,7 @@ func loadMiddlewares(ctx context.Context, l log.Logger, cfg *config.Config) alic ) } -// loadUserAgent reads the proxy-user-agent-whitelist, since it is a string flag, and attempts to construct a map of +// loadUserAgent reads the proxy-user-agent-lock-in, since it is a string flag, and attempts to construct a map of // "user-agent":"challenge" locks in for Reva. // Modifies cfg. Spaces don't need to be trimmed as urfavecli takes care of it. User agents with spaces are valid. i.e: // Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:83.0) Gecko/20100101 Firefox/83.0 @@ -327,7 +327,7 @@ func loadMiddlewares(ctx context.Context, l log.Logger, cfg *config.Config) alic // in reverse for each individual part func loadUserAgent(c *cli.Context, cfg *config.Config) error { cfg.Reva.Middleware.Auth.CredentialsByUserAgent = make(map[string]string, 0) - locks := c.StringSlice("proxy-user-agent-whitelist") + locks := c.StringSlice("proxy-user-agent-lock-in") for _, v := range locks { vv := conversions.Reverse(v) diff --git a/proxy/pkg/flagset/flagset.go b/proxy/pkg/flagset/flagset.go index b8ea4d252a4..5356b5c6c41 100644 --- a/proxy/pkg/flagset/flagset.go +++ b/proxy/pkg/flagset/flagset.go @@ -251,8 +251,8 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { // Reva Middlewares Config &cli.StringSliceFlag{ - Name: "proxy-user-agent-whitelist", - Usage: "--user-agent-whitelist=mirall:basic,foo:bearer Given a tuple of [UserAgent:challenge] it locks a given user agent to the authentication challenge. Particularly useful for old clients whose USer-Agent is known and only support one authentication challenge. When this flag is set in the proxy it configures the authentication middlewares.", + Name: "proxy-user-agent-lock-in", + Usage: "--user-agent-whitelist-lock-in=mirall:basic,foo:bearer Given a tuple of [UserAgent:challenge] it locks a given user agent to the authentication challenge. Particularly useful for old clients whose USer-Agent is known and only support one authentication challenge. When this flag is set in the proxy it configures the authentication middlewares.", EnvVars: []string{"PROXY_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT"}, }, } diff --git a/storage/pkg/command/frontend.go b/storage/pkg/command/frontend.go index e67edd44235..ee50e1280d4 100644 --- a/storage/pkg/command/frontend.go +++ b/storage/pkg/command/frontend.go @@ -302,7 +302,7 @@ func Frontend(cfg *config.Config) *cli.Command { } } -// loadUserAgent reads the user-agent-whitelist, since it is a string flag, and attempts to construct a map of +// loadUserAgent reads the user-agent-whitelist-lock-in, since it is a string flag, and attempts to construct a map of // "user-agent":"challenge" locks in for Reva. // Modifies cfg. Spaces don't need to be trimmed as urfavecli takes care of it. User agents with spaces are valid. i.e: // Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:83.0) Gecko/20100101 Firefox/83.0 @@ -312,7 +312,7 @@ func Frontend(cfg *config.Config) *cli.Command { // in reverse for each individual part func loadUserAgent(c *cli.Context, cfg *config.Config) error { cfg.Reva.Frontend.Middleware.Auth.CredentialsByUserAgent = make(map[string]string, 0) - locks := c.StringSlice("user-agent-whitelist") + locks := c.StringSlice("user-agent-whitelist-lock-in") for _, v := range locks { vv := conversions.Reverse(v) diff --git a/storage/pkg/flagset/frontend.go b/storage/pkg/flagset/frontend.go index 0a5bdb5651e..a43f89be4f6 100644 --- a/storage/pkg/flagset/frontend.go +++ b/storage/pkg/flagset/frontend.go @@ -136,8 +136,8 @@ func FrontendWithConfig(cfg *config.Config) []cli.Flag { // Reva Middlewares Config &cli.StringSliceFlag{ - Name: "user-agent-whitelist", - Usage: "--user-agent-whitelist=mirall:basic,foo:bearer Given a tuple of comma separated [UserAgent:challenge] values, it locks a given user agent to the authentication challenge. Particularly useful for old clients whose USer-Agent is known and only support one authentication challenge. When this flag is set in the storage-frontend it configures Reva.", + Name: "user-agent-whitelist-lock-in", + Usage: "--user-agent-whitelist-lock-in=mirall:basic,foo:bearer Given a tuple of comma separated [UserAgent:challenge] values, it locks a given user agent to the authentication challenge. Particularly useful for old clients whose USer-Agent is known and only support one authentication challenge. When this flag is set in the storage-frontend it configures Reva.", EnvVars: []string{"STORAGE_FRONTEND_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT"}, }, } From c89ead3fc57e7801c7a3cdb64ae8fd5c02acc294 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Thu, 3 Dec 2020 12:19:49 +0100 Subject: [PATCH 15/22] fix linter --- proxy/go.sum | 1 + proxy/pkg/config/config.go | 2 ++ proxy/pkg/middleware/authentication.go | 1 + storage/go.sum | 1 + storage/pkg/config/config.go | 2 ++ 5 files changed, 7 insertions(+) diff --git a/proxy/go.sum b/proxy/go.sum index 0061092f2b4..6c42c342ad4 100644 --- a/proxy/go.sum +++ b/proxy/go.sum @@ -191,6 +191,7 @@ github.com/cs3org/go-cs3apis v0.0.0-20200810113633-b00aca449666 h1:E7VsSSN/2YZLS github.com/cs3org/go-cs3apis v0.0.0-20200810113633-b00aca449666/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY= github.com/cs3org/go-cs3apis v0.0.0-20201007120910-416ed6cf8b00 h1:LVl25JaflluOchVvaHWtoCynm5OaM+VNai0IYkcCSe0= github.com/cs3org/go-cs3apis v0.0.0-20201007120910-416ed6cf8b00/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY= +github.com/cs3org/go-cs3apis v0.0.0-20201118090759-87929f5bae21 h1:mZpylrgnCgSeaZ5EznvHIPIKuaQHMHZDi2wkJtk4M8Y= github.com/cs3org/go-cs3apis v0.0.0-20201118090759-87929f5bae21/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY= github.com/cs3org/reva v1.2.2-0.20200924071957-e6676516e61e/go.mod h1:DOV5SjpOBKN+aWfOHLdA4KiLQkpyC786PQaXEdRAZ0M= github.com/cs3org/reva v1.4.1-0.20201120104232-f5afafc04c3b h1:bDGaeyhTFrdLF3Pm8XdJ60ADrE4f+f/Mz2hkICvQHJM= diff --git a/proxy/pkg/config/config.go b/proxy/pkg/config/config.go index 769f597f1fd..ff46da12e2f 100644 --- a/proxy/pkg/config/config.go +++ b/proxy/pkg/config/config.go @@ -84,10 +84,12 @@ type Reva struct { Middleware Middleware } +// Middleware configures reva middlewares. type Middleware struct { Auth Auth } +// Auth configures reva http auth middleware. type Auth struct { CredentialsByUserAgent map[string]string } diff --git a/proxy/pkg/middleware/authentication.go b/proxy/pkg/middleware/authentication.go index dccef744fdf..c3be548e97d 100644 --- a/proxy/pkg/middleware/authentication.go +++ b/proxy/pkg/middleware/authentication.go @@ -7,6 +7,7 @@ import ( "time" ) +// SupportedAuthStrategies stores configured challenges. var SupportedAuthStrategies []string // ProxyWwwAuthenticate is a list of endpoints that do not rely on reva underlying authentication, such as ocs. diff --git a/storage/go.sum b/storage/go.sum index adc9014cfd1..d9e2bfa67be 100644 --- a/storage/go.sum +++ b/storage/go.sum @@ -195,6 +195,7 @@ github.com/cs3org/go-cs3apis v0.0.0-20200730121022-c4f3d4f7ddfd/go.mod h1:UXha4T github.com/cs3org/go-cs3apis v0.0.0-20200810113633-b00aca449666/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY= github.com/cs3org/go-cs3apis v0.0.0-20201007120910-416ed6cf8b00 h1:LVl25JaflluOchVvaHWtoCynm5OaM+VNai0IYkcCSe0= github.com/cs3org/go-cs3apis v0.0.0-20201007120910-416ed6cf8b00/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY= +github.com/cs3org/go-cs3apis v0.0.0-20201118090759-87929f5bae21 h1:mZpylrgnCgSeaZ5EznvHIPIKuaQHMHZDi2wkJtk4M8Y= github.com/cs3org/go-cs3apis v0.0.0-20201118090759-87929f5bae21/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY= github.com/cs3org/reva v1.1.0/go.mod h1:fBzTrNuAKdQ62ybjpdu8nyhBin90/3/3s6DGQDCdBp4= github.com/cs3org/reva v1.2.2-0.20200924071957-e6676516e61e/go.mod h1:DOV5SjpOBKN+aWfOHLdA4KiLQkpyC786PQaXEdRAZ0M= diff --git a/storage/pkg/config/config.go b/storage/pkg/config/config.go index 33d70c2148f..84f211dd506 100644 --- a/storage/pkg/config/config.go +++ b/storage/pkg/config/config.go @@ -86,10 +86,12 @@ type FrontendPort struct { Middleware Middleware } +// Middleware configures reva middlewares. type Middleware struct { Auth Auth } +// Auth configures reva http auth middleware. type Auth struct { CredentialsByUserAgent map[string]string } From 9a253370e8355baddfc62ac70faca21f53f38a83 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Thu, 3 Dec 2020 12:22:35 +0100 Subject: [PATCH 16/22] export StatusRecorder --- proxy/pkg/middleware/authentication.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/proxy/pkg/middleware/authentication.go b/proxy/pkg/middleware/authentication.go index c3be548e97d..999952a7964 100644 --- a/proxy/pkg/middleware/authentication.go +++ b/proxy/pkg/middleware/authentication.go @@ -15,12 +15,14 @@ var SupportedAuthStrategies []string // TODO this should be a regexp, or it can be confused with routes that contain "/ocs" somewhere along the URI var ProxyWwwAuthenticate = []string{"ocs"} -type statusRecorder struct { +// StatusRecorder implements the http.ResponseWriter interface to record a response a-posteriori +type StatusRecorder struct { http.ResponseWriter status int } -func (rec *statusRecorder) WriteHeader(code int) { +// WriteHeader implements the http.ResponseWriter interface +func (rec *StatusRecorder) WriteHeader(code int) { rec.status = code rec.ResponseWriter.WriteHeader(code) } From be46bacc5b6fa1480bedad7172cb4aa17f59907c Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Thu, 3 Dec 2020 13:23:14 +0100 Subject: [PATCH 17/22] use make(map[string]string) --- proxy/pkg/command/server.go | 2 +- storage/pkg/command/frontend.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/proxy/pkg/command/server.go b/proxy/pkg/command/server.go index bd196fd7d49..a29de79d742 100644 --- a/proxy/pkg/command/server.go +++ b/proxy/pkg/command/server.go @@ -326,7 +326,7 @@ func loadMiddlewares(ctx context.Context, l log.Logger, cfg *config.Config) alic // have the indexes reversed and the tuple is in the format of [challenge:user-agent], then the same process is applied // in reverse for each individual part func loadUserAgent(c *cli.Context, cfg *config.Config) error { - cfg.Reva.Middleware.Auth.CredentialsByUserAgent = make(map[string]string, 0) + cfg.Reva.Middleware.Auth.CredentialsByUserAgent = make(map[string]string) locks := c.StringSlice("proxy-user-agent-lock-in") for _, v := range locks { diff --git a/storage/pkg/command/frontend.go b/storage/pkg/command/frontend.go index ee50e1280d4..d59a19b5755 100644 --- a/storage/pkg/command/frontend.go +++ b/storage/pkg/command/frontend.go @@ -311,7 +311,7 @@ func Frontend(cfg *config.Config) *cli.Command { // have the indexes reversed and the tuple is in the format of [challenge:user-agent], then the same process is applied // in reverse for each individual part func loadUserAgent(c *cli.Context, cfg *config.Config) error { - cfg.Reva.Frontend.Middleware.Auth.CredentialsByUserAgent = make(map[string]string, 0) + cfg.Reva.Frontend.Middleware.Auth.CredentialsByUserAgent = make(map[string]string) locks := c.StringSlice("user-agent-whitelist-lock-in") for _, v := range locks { From 7d8336ce4b3d795b1a4ac92be95c516824e3bc36 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Fri, 4 Dec 2020 13:17:25 +0100 Subject: [PATCH 18/22] use regexp to assert routes, remove StatusRecorder --- proxy/pkg/middleware/authentication.go | 41 +++++++++++--------------- 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/proxy/pkg/middleware/authentication.go b/proxy/pkg/middleware/authentication.go index 999952a7964..36a0369b1a3 100644 --- a/proxy/pkg/middleware/authentication.go +++ b/proxy/pkg/middleware/authentication.go @@ -3,6 +3,7 @@ package middleware import ( "fmt" "net/http" + "regexp" "strings" "time" ) @@ -11,21 +12,9 @@ import ( var SupportedAuthStrategies []string // ProxyWwwAuthenticate is a list of endpoints that do not rely on reva underlying authentication, such as ocs. -// services that fallback to reva authentication are declared in the "frontend" command on OCIS. -// TODO this should be a regexp, or it can be confused with routes that contain "/ocs" somewhere along the URI -var ProxyWwwAuthenticate = []string{"ocs"} - -// StatusRecorder implements the http.ResponseWriter interface to record a response a-posteriori -type StatusRecorder struct { - http.ResponseWriter - status int -} - -// WriteHeader implements the http.ResponseWriter interface -func (rec *StatusRecorder) WriteHeader(code int) { - rec.status = code - rec.ResponseWriter.WriteHeader(code) -} +// services that fallback to reva authentication are declared in the "frontend" command on OCIS. It is a list of strings +// to be regexp compiled. +var ProxyWwwAuthenticate = []string{"/ocs/v[12].php/cloud/"} // Authentication is a higher order authentication middleware. func Authentication(opts ...Option) func(next http.Handler) http.Handler { @@ -84,19 +73,23 @@ func removeSuperfluousAuthenticate(w http.ResponseWriter) { } // userAgentAuthenticateLockIn sets Www-Authenticate according to configured user agents. This is useful for the case of -// legacy clients that do not support protocols like OIDC or OAuth and want to lock a given user agent to a challenge, -// such as basic. More info in https://github.com/cs3org/reva/pull/1350 +// legacy clients that do not support protocols like OIDC or OAuth and want to lock a given user agent to a challenge +// such as basic. For more context check https://github.com/cs3org/reva/pull/1350 func userAgentAuthenticateLockIn(w http.ResponseWriter, req *http.Request, creds map[string]string, fallback string) { for i := 0; i < len(ProxyWwwAuthenticate); i++ { - if strings.Contains(req.RequestURI, fmt.Sprintf("/%v/", ProxyWwwAuthenticate[i])) { - for k, v := range creds { - if strings.Contains(k, req.UserAgent()) { - removeSuperfluousAuthenticate(w) - w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(v), req.Host)) - return + if r, err := regexp.Compile(ProxyWwwAuthenticate[i]); err == nil { + if r.Match([]byte(req.RequestURI)) { + for k, v := range creds { + if strings.Contains(k, req.UserAgent()) { + removeSuperfluousAuthenticate(w) + w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(v), req.Host)) + return + } } + w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(fallback), req.Host)) } - w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(fallback), req.Host)) + } else { + // deal with err } } } From f1521e4df73f8ad1731c448df3068a79297a1955 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Fri, 4 Dec 2020 13:51:48 +0100 Subject: [PATCH 19/22] refactor authentication.go --- proxy/pkg/middleware/authentication.go | 126 ++++++++++++++++--------- 1 file changed, 81 insertions(+), 45 deletions(-) diff --git a/proxy/pkg/middleware/authentication.go b/proxy/pkg/middleware/authentication.go index 36a0369b1a3..ddbfd0b83b5 100644 --- a/proxy/pkg/middleware/authentication.go +++ b/proxy/pkg/middleware/authentication.go @@ -8,42 +8,35 @@ import ( "time" ) -// SupportedAuthStrategies stores configured challenges. -var SupportedAuthStrategies []string +var ( + // SupportedAuthStrategies stores configured challenges. + SupportedAuthStrategies []string -// ProxyWwwAuthenticate is a list of endpoints that do not rely on reva underlying authentication, such as ocs. -// services that fallback to reva authentication are declared in the "frontend" command on OCIS. It is a list of strings -// to be regexp compiled. -var ProxyWwwAuthenticate = []string{"/ocs/v[12].php/cloud/"} + // ProxyWwwAuthenticate is a list of endpoints that do not rely on reva underlying authentication, such as ocs. + // services that fallback to reva authentication are declared in the "frontend" command on OCIS. It is a list of strings + // to be regexp compiled. + ProxyWwwAuthenticate = []string{"/ocs/v[12].php/cloud/"} + + // WWWAuthenticate captures the Www-Authenticate header string. + WWWAuthenticate = "Www-Authenticate" +) + +// userAgentLocker aids in dependency injection for helper methods. The set of fields is arbitrary and the only relation +// they share is to fulfill their duty and lock a User-Agent to its correct challenge if configured. +type userAgentLocker struct { + w http.ResponseWriter + r *http.Request + locks map[string]string // locks represents a reva user-agent:challenge mapping. + fallback string +} // Authentication is a higher order authentication middleware. func Authentication(opts ...Option) func(next http.Handler) http.Handler { options := newOptions(opts...) - if options.OIDCIss != "" { - SupportedAuthStrategies = append(SupportedAuthStrategies, "bearer") - } - if options.EnableBasicAuth { - SupportedAuthStrategies = append(SupportedAuthStrategies, "basic") - } - - oidc := OIDCAuth( - Logger(options.Logger), - OIDCProviderFunc(options.OIDCProviderFunc), - HTTPClient(options.HTTPClient), - OIDCIss(options.OIDCIss), - TokenCacheSize(options.UserinfoCacheSize), - TokenCacheTTL(time.Second*time.Duration(options.UserinfoCacheTTL)), - CredentialsByUserAgent(options.CredentialsByUserAgent), - ) - - basic := BasicAuth( - Logger(options.Logger), - EnableBasicAuth(options.EnableBasicAuth), - AccountsClient(options.AccountsClient), - OIDCIss(options.OIDCIss), - CredentialsByUserAgent(options.CredentialsByUserAgent), - ) + configureSupportedChallenges(options) + oidc := newOIDCAuth(options) + basic := newBasicAuth(options) return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -62,34 +55,77 @@ func Authentication(opts ...Option) func(next http.Handler) http.Handler { } } +// configureSupportedChallenges adds known authentication challenges to the current session. +func configureSupportedChallenges(options Options) { + if options.OIDCIss != "" { + SupportedAuthStrategies = append(SupportedAuthStrategies, "bearer") + } + + if options.EnableBasicAuth { + SupportedAuthStrategies = append(SupportedAuthStrategies, "basic") + } +} + func writeSupportedAuthenticateHeader(w http.ResponseWriter, r *http.Request) { for i := 0; i < len(SupportedAuthStrategies); i++ { - w.Header().Add("WWW-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(SupportedAuthStrategies[i]), r.Host)) + w.Header().Add(WWWAuthenticate, fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(SupportedAuthStrategies[i]), r.Host)) } } func removeSuperfluousAuthenticate(w http.ResponseWriter) { - w.Header().Del("Www-Authenticate") + w.Header().Del(WWWAuthenticate) } // userAgentAuthenticateLockIn sets Www-Authenticate according to configured user agents. This is useful for the case of // legacy clients that do not support protocols like OIDC or OAuth and want to lock a given user agent to a challenge // such as basic. For more context check https://github.com/cs3org/reva/pull/1350 -func userAgentAuthenticateLockIn(w http.ResponseWriter, req *http.Request, creds map[string]string, fallback string) { +func userAgentAuthenticateLockIn(w http.ResponseWriter, r *http.Request, locks map[string]string, fallback string) { + u := userAgentLocker{ + w: w, + r: r, + locks: locks, + fallback: fallback, + } + for i := 0; i < len(ProxyWwwAuthenticate); i++ { - if r, err := regexp.Compile(ProxyWwwAuthenticate[i]); err == nil { - if r.Match([]byte(req.RequestURI)) { - for k, v := range creds { - if strings.Contains(k, req.UserAgent()) { - removeSuperfluousAuthenticate(w) - w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(v), req.Host)) - return - } - } - w.Header().Add("Www-Authenticate", fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(fallback), req.Host)) + evalRequestURI(&u, i) + } +} + +func evalRequestURI(l *userAgentLocker, i int) { + r := regexp.MustCompile(ProxyWwwAuthenticate[i]) + if r.Match([]byte(l.r.RequestURI)) { + for k, v := range l.locks { + if strings.Contains(k, l.r.UserAgent()) { + removeSuperfluousAuthenticate(l.w) + l.w.Header().Add(WWWAuthenticate, fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(v), l.r.Host)) + return } - } else { - // deal with err } + l.w.Header().Add(WWWAuthenticate, fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(l.fallback), l.r.Host)) } } + +// newOIDCAuth returns a configured oidc middleware +func newOIDCAuth(options Options) func(http.Handler) http.Handler { + return OIDCAuth( + Logger(options.Logger), + OIDCProviderFunc(options.OIDCProviderFunc), + HTTPClient(options.HTTPClient), + OIDCIss(options.OIDCIss), + TokenCacheSize(options.UserinfoCacheSize), + TokenCacheTTL(time.Second*time.Duration(options.UserinfoCacheTTL)), + CredentialsByUserAgent(options.CredentialsByUserAgent), + ) +} + +// newBasicAuth returns a configured oidc middleware +func newBasicAuth(options Options) func(http.Handler) http.Handler { + return BasicAuth( + Logger(options.Logger), + EnableBasicAuth(options.EnableBasicAuth), + AccountsClient(options.AccountsClient), + OIDCIss(options.OIDCIss), + CredentialsByUserAgent(options.CredentialsByUserAgent), + ) +} From 2cddc0a23c382f8c35365cf4bc1c8b28ac5a353a Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Fri, 4 Dec 2020 13:53:34 +0100 Subject: [PATCH 20/22] fix leftover typo --- proxy/pkg/middleware/authentication.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/pkg/middleware/authentication.go b/proxy/pkg/middleware/authentication.go index ddbfd0b83b5..2600ab3c441 100644 --- a/proxy/pkg/middleware/authentication.go +++ b/proxy/pkg/middleware/authentication.go @@ -119,7 +119,7 @@ func newOIDCAuth(options Options) func(http.Handler) http.Handler { ) } -// newBasicAuth returns a configured oidc middleware +// newBasicAuth returns a configured basic middleware func newBasicAuth(options Options) func(http.Handler) http.Handler { return BasicAuth( Logger(options.Logger), From d81e47cc7e0e8c7c109ff88c803fe5ede7deef91 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Fri, 4 Dec 2020 14:09:55 +0100 Subject: [PATCH 21/22] add changelog --- changelog/unreleased/user-agent-lock-in.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/unreleased/user-agent-lock-in.md diff --git a/changelog/unreleased/user-agent-lock-in.md b/changelog/unreleased/user-agent-lock-in.md new file mode 100644 index 00000000000..aec155353eb --- /dev/null +++ b/changelog/unreleased/user-agent-lock-in.md @@ -0,0 +1,5 @@ +Enhancement: Add www-authenticate based on user agent + +We now comply with HTTP spec by adding Www-Authenticate headers on every `401` request. Furthermore we not only take care of such thing at the Proxy but also Reva will take care of it. In addition we now are able to lock-in user-agents to specific challenges. + +https://github.com/owncloud/ocis/pull/1009 From 5b5d3611ad6fa7ed3912a6046472fcc66c04b9dd Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Fri, 4 Dec 2020 14:22:16 +0100 Subject: [PATCH 22/22] add changelog --- .../user-agent-challenge-lock-in.md | 20 +++++++++++++++++++ changelog/unreleased/user-agent-lock-in.md | 5 ----- 2 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 changelog/unreleased/user-agent-challenge-lock-in.md delete mode 100644 changelog/unreleased/user-agent-lock-in.md diff --git a/changelog/unreleased/user-agent-challenge-lock-in.md b/changelog/unreleased/user-agent-challenge-lock-in.md new file mode 100644 index 00000000000..f89ab4eb2a9 --- /dev/null +++ b/changelog/unreleased/user-agent-challenge-lock-in.md @@ -0,0 +1,20 @@ +Enhancement: Add www-authenticate based on user agent + +Tags: reva, proxy + +We now comply with HTTP spec by adding Www-Authenticate headers on every `401` request. Furthermore, we not only take care of such a thing at the Proxy but also Reva will take care of it. In addition, we now are able to lock-in a set of User-Agent to specific challenges. + +Admins can use this feature by configuring OCIS + Reva following this approach: + +``` +STORAGE_FRONTEND_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT="mirall:basic, Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:83.0) Gecko/20100101 Firefox/83.0:bearer" \ +PROXY_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT="mirall:basic, Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:83.0) Gecko/20100101 Firefox/83.0:bearer" \ +PROXY_ENABLE_BASIC_AUTH=true \ +go run cmd/ocis/main.go server +``` + +We introduced two new environment variables: + +`STORAGE_FRONTEND_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT` as well as `PROXY_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT`, The reason they have the same value is not to rely on the os env on a distributed environment, so in redundancy we trust. They both configure the same on the backend storage and OCIS Proxy. + +https://github.com/owncloud/ocis/pull/1009 diff --git a/changelog/unreleased/user-agent-lock-in.md b/changelog/unreleased/user-agent-lock-in.md deleted file mode 100644 index aec155353eb..00000000000 --- a/changelog/unreleased/user-agent-lock-in.md +++ /dev/null @@ -1,5 +0,0 @@ -Enhancement: Add www-authenticate based on user agent - -We now comply with HTTP spec by adding Www-Authenticate headers on every `401` request. Furthermore we not only take care of such thing at the Proxy but also Reva will take care of it. In addition we now are able to lock-in user-agents to specific challenges. - -https://github.com/owncloud/ocis/pull/1009