add unprotected flag to the proxy routes

I added an unprotected flag to the proxy routes which is evaluated by
the authentication middleware. This way we won't have to maintain a
hardcoded list of unprotected paths and path prefixes and we will
hopefully reduce the times we encounter the basic auth prompt by web
browsers.

services/proxy/pkg/config/config.go

@@ -54,6 +54,7 @@ type Route struct {
 	// Service name to look up in the registry
 	Service     string `yaml:"service,omitempty"`
 	ApacheVHost bool   `yaml:"apache_vhost,omitempty"`
+	Unprotected bool   `yaml:"unprotected,omitempty"`
 }
 
 // RouteType defines the type of a route

services/proxy/pkg/config/defaults/defaultconfig.go

@@ -71,20 +71,24 @@ func DefaultPolicies() []config.Policy {
 			Name: "ocis",
 			Routes: []config.Route{
 				{
-					Endpoint: "/",
-					Backend:  "http://localhost:9100",
+					Endpoint:    "/",
+					Backend:     "http://localhost:9100",
+					Unprotected: true,
 				},
 				{
-					Endpoint: "/.well-known/",
-					Backend:  "http://localhost:9130",
+					Endpoint:    "/.well-known/",
+					Backend:     "http://localhost:9130",
+					Unprotected: true,
 				},
 				{
-					Endpoint: "/konnect/",
-					Backend:  "http://localhost:9130",
+					Endpoint:    "/konnect/",
+					Backend:     "http://localhost:9130",
+					Unprotected: true,
 				},
 				{
-					Endpoint: "/signin/",
-					Backend:  "http://localhost:9130",
+					Endpoint:    "/signin/",
+					Backend:     "http://localhost:9130",
+					Unprotected: true,
 				},
 				{
 					Endpoint: "/archiver",
@@ -161,24 +165,27 @@ func DefaultPolicies() []config.Policy {
 					Backend:  "http://localhost:9140",
 				},
 				{
-					Endpoint: "/app/", // /app or /apps? ocdav only handles /apps
-					Backend:  "http://localhost:9140",
+					Endpoint:    "/app/", // /app or /apps? ocdav only handles /apps
+					Backend:     "http://localhost:9140",
+					Unprotected: true, // TODO check if this is safe
 				},
 				{
 					Endpoint: "/graph/",
 					Backend:  "http://localhost:9120",
 				},
 				{
-					Endpoint: "/graph-explorer",
-					Backend:  "http://localhost:9135",
+					Endpoint:    "/graph-explorer",
+					Backend:     "http://localhost:9135",
+					Unprotected: true,
 				},
 				{
 					Endpoint: "/api/v0/settings",
 					Backend:  "http://localhost:9190",
 				},
 				{
-					Endpoint: "/settings.js",
-					Backend:  "http://localhost:9190",
+					Endpoint:    "/settings.js",
+					Backend:     "http://localhost:9190",
+					Unprotected: true,
 				},
 			},
 		},

services/proxy/pkg/middleware/authentication.go

@@ -6,6 +6,7 @@ import (
 	"regexp"
 	"strings"
 
+	"github.com/owncloud/ocis/v2/services/proxy/pkg/router"
 	"github.com/owncloud/ocis/v2/services/proxy/pkg/webdav"
 	"golang.org/x/text/cases"
 	"golang.org/x/text/language"
@@ -26,49 +27,6 @@ var (
 		"/remote.php/ocs/apps/files_sharing/api/v1/tokeninfo/unprotected",
 		"/ocs/v1.php/cloud/capabilities",
 	}
-	// _unprotectedPaths contains paths which don't need to be authenticated.
-	_unprotectedPaths = map[string]struct{}{
-		"/":                    {},
-		"/login":               {},
-		"/settings":            {},
-		"/app/list":            {},
-		"/config.json":         {},
-		"/manifest.json":       {},
-		"/index.html":          {},
-		"/oidc-callback.html":  {},
-		"/oidc-callback":       {},
-		"/settings.js":         {},
-		"/data":                {},
-		"/konnect/v1/userinfo": {},
-		"/status.php":          {},
-		"/favicon.ico":         {},
-		"/ocs/v1.php/config":   {},
-		"/ocs/v2.php/config":   {},
-	}
-	// _unprotectedPathPrefixes contains paths which don't need to be authenticated.
-	_unprotectedPathPrefixes = [...]string{
-		"/files/",
-		"/data/",
-		"/account/",
-		"/s/",
-		"/external",
-		"/apps/openidconnect/redirect",
-		"/settings/",
-		"/user-management/",
-		"/.well-known/",
-		"/js/",
-		"/css/",
-		"/fonts/",
-		"/icons/",
-		"/themes/",
-		"/signin/",
-		"/konnect/",
-		"/text-editor/",
-		"/preview/",
-		"/pdf-viewer/",
-		"/draw-io/",
-		"/index.html#/",
-	}
 )
 
 const (
@@ -91,7 +49,8 @@ func Authentication(auths []Authenticator, opts ...Option) func(next http.Handle
 
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			if isOIDCTokenAuth(r) || isUnprotectedPath(r) {
+			ri := router.ContextRoutingInfo(r.Context())
+			if isOIDCTokenAuth(r) || ri.IsRouteUnprotected() {
 				// The authentication for this request is handled by the IdP.
 				next.ServeHTTP(w, r)
 				return
@@ -153,18 +112,6 @@ func isOIDCTokenAuth(req *http.Request) bool {
 	return req.URL.Path == "/konnect/v1/token"
 }
 
-func isUnprotectedPath(r *http.Request) bool {
-	if _, ok := _unprotectedPaths[r.URL.Path]; ok {
-		return true
-	}
-	for _, p := range _unprotectedPathPrefixes {
-		if strings.HasPrefix(r.URL.Path, p) {
-			return true
-		}
-	}
-	return false
-}
-
 func isPublicPath(p string) bool {
 	for _, pp := range _publicPaths {
 		if strings.HasPrefix(p, pp) {

services/proxy/pkg/proxy/proxy.go

@@ -43,8 +43,8 @@ func NewMultiHostReverseProxy(opts ...Option) *MultiHostReverseProxy {
 	}
 
 	rp.Director = func(r *http.Request) {
-		fn := router.DirectorFunc(r.Context())
-		fn(r)
+		ri := router.ContextRoutingInfo(r.Context())
+		ri.Director()(r)
 	}
 
 	// equals http.DefaultTransport except TLSClientConfig

services/proxy/pkg/router/router.go

@@ -14,23 +14,29 @@ import (
 	"go-micro.dev/v4/selector"
 )
 
-const directorCtxKey string = "director"
+const routingInfoCtxKey string = "director"
+
+var noInfo = RoutingInfo{}
 
 func Middleware(policySelector *config.PolicySelector, policies []config.Policy, logger log.Logger) func(http.Handler) http.Handler {
 	router := New(policySelector, policies, logger)
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			fn := router.Route(r)
-			next.ServeHTTP(w, r.WithContext(SetDirectorFunc(r.Context(), fn)))
+			ri, ok := router.Route(r)
+			if !ok {
+				w.WriteHeader(http.StatusInternalServerError)
+				return
+			}
+			next.ServeHTTP(w, r.WithContext(SetRoutingInfo(r.Context(), ri)))
 		})
 	}
 }
 
-func (rt Router) Route(r *http.Request) func(*http.Request) {
+func (rt Router) Route(r *http.Request) (RoutingInfo, bool) {
 	pol, err := rt.policySelector(r)
 	if err != nil {
 		rt.logger.Error().Err(err).Msg("Error while selecting pol")
-		return nil
+		return noInfo, false
 	}
 
 	if _, ok := rt.directors[pol]; !ok {
@@ -38,7 +44,7 @@ func (rt Router) Route(r *http.Request) func(*http.Request) {
 			Error().
 			Str("policy", pol).
 			Msg("policy is not configured")
-		return nil
+		return noInfo, false
 	}
 
 	method := ""
@@ -70,19 +76,16 @@ func (rt Router) Route(r *http.Request) func(*http.Request) {
 					Str("routeType", string(rtype)).
 					Msg("director found")
 
-				return rt.directors[pol][rtype][method][endpoint]
+				return rt.directors[pol][rtype][method][endpoint], true
 			}
 		}
 	}
 
 	// override default director with root. If any
-	switch {
-	case rt.directors[pol][config.PrefixRoute][method]["/"] != nil:
-		// try specific method
-		return rt.directors[pol][config.PrefixRoute][method]["/"]
-	case rt.directors[pol][config.PrefixRoute][""]["/"] != nil:
-		// fallback to unspecific method
-		return rt.directors[pol][config.PrefixRoute][""]["/"]
+	if ri, ok := rt.directors[pol][config.PrefixRoute][method]["/"]; ok { // try specific method
+		return ri, true
+	} else if ri, ok := rt.directors[pol][config.PrefixRoute][""]["/"]; ok { // fallback to unspecific method
+		return ri, true
 	}
 
 	rt.logger.
@@ -90,7 +93,7 @@ func (rt Router) Route(r *http.Request) func(*http.Request) {
 		Str("policy", pol).
 		Str("path", r.URL.Path).
 		Msg("no director found")
-	return nil
+	return noInfo, false
 }
 
 func New(policySelector *config.PolicySelector, policies []config.Policy, logger log.Logger) Router {
@@ -114,7 +117,7 @@ func New(policySelector *config.PolicySelector, policies []config.Policy, logger
 	}
 
 	r := Router{
-		directors:      make(map[string]map[config.RouteType]map[string]map[string]func(req *http.Request)),
+		directors:      make(map[string]map[config.RouteType]map[string]map[string]RoutingInfo),
 		policySelector: selector,
 	}
 	for _, pol := range policies {
@@ -140,72 +143,88 @@ func New(policySelector *config.PolicySelector, policies []config.Policy, logger
 	return r
 }
 
+type RoutingInfo struct {
+	director    func(*http.Request)
+	unprotected bool
+}
+
+func (r RoutingInfo) Director() func(*http.Request) {
+	return r.director
+}
+
+func (r RoutingInfo) IsRouteUnprotected() bool {
+	return r.unprotected
+}
+
 type Router struct {
 	logger         log.Logger
-	directors      map[string]map[config.RouteType]map[string]map[string]func(req *http.Request)
+	directors      map[string]map[config.RouteType]map[string]map[string]RoutingInfo
 	policySelector policy.Selector
 }
 
 func (rt Router) addHost(policy string, target *url.URL, route config.Route) {
 	targetQuery := target.RawQuery
 	if rt.directors[policy] == nil {
-		rt.directors[policy] = make(map[config.RouteType]map[string]map[string]func(req *http.Request))
+		rt.directors[policy] = make(map[config.RouteType]map[string]map[string]RoutingInfo)
 	}
 	routeType := config.DefaultRouteType
 	if route.Type != "" {
 		routeType = route.Type
 	}
 	if rt.directors[policy][routeType] == nil {
-		rt.directors[policy][routeType] = make(map[string]map[string]func(req *http.Request))
+		rt.directors[policy][routeType] = make(map[string]map[string]RoutingInfo)
 	}
 	if rt.directors[policy][routeType][route.Method] == nil {
-		rt.directors[policy][routeType][route.Method] = make(map[string]func(req *http.Request))
+		rt.directors[policy][routeType][route.Method] = make(map[string]RoutingInfo)
 	}
 
 	reg := registry.GetRegistry()
 	sel := selector.NewSelector(selector.Registry(reg))
 
-	rt.directors[policy][routeType][route.Method][route.Endpoint] = func(req *http.Request) {
-		if route.Service != "" {
-			// select next node
-			next, err := sel.Select(route.Service)
-			if err != nil {
-				rt.logger.Error().Err(err).
-					Str("service", route.Service).
-					Msg("could not select service from the registry")
-				return // TODO error? fallback to target.Host & Scheme?
+	rt.directors[policy][routeType][route.Method][route.Endpoint] = RoutingInfo{
+		unprotected: route.Unprotected,
+		director: func(req *http.Request) {
+			if route.Service != "" {
+				// select next node
+				next, err := sel.Select(route.Service)
+				if err != nil {
+					rt.logger.Error().Err(err).
+						Str("service", route.Service).
+						Msg("could not select service from the registry")
+					return // TODO error? fallback to target.Host & Scheme?
+				}
+				node, err := next()
+				if err != nil {
+					rt.logger.Error().Err(err).
+						Str("service", route.Service).
+						Msg("could not select next node")
+					return // TODO error? fallback to target.Host & Scheme?
+				}
+				req.URL.Host = node.Address
+				req.URL.Scheme = node.Metadata["protocol"] // TODO check property exists?
+
+			} else {
+				req.URL.Host = target.Host
+				req.URL.Scheme = target.Scheme
 			}
-			node, err := next()
-			if err != nil {
-				rt.logger.Error().Err(err).
-					Str("service", route.Service).
-					Msg("could not select next node")
-				return // TODO error? fallback to target.Host & Scheme?
+
+			// Apache deployments host addresses need to match on req.Host and req.URL.Host
+			// see https://stackoverflow.com/questions/34745654/golang-reverseproxy-with-apache2-sni-hostname-error
+			if route.ApacheVHost {
+				req.Host = target.Host
 			}
-			req.URL.Host = node.Address
-			req.URL.Scheme = node.Metadata["protocol"] // TODO check property exists?
 
-		} else {
-			req.URL.Host = target.Host
-			req.URL.Scheme = target.Scheme
-		}
-
-		// Apache deployments host addresses need to match on req.Host and req.URL.Host
-		// see https://stackoverflow.com/questions/34745654/golang-reverseproxy-with-apache2-sni-hostname-error
-		if route.ApacheVHost {
-			req.Host = target.Host
-		}
-
-		req.URL.Path = singleJoiningSlash(target.Path, req.URL.Path)
-		if targetQuery == "" || req.URL.RawQuery == "" {
-			req.URL.RawQuery = targetQuery + req.URL.RawQuery
-		} else {
-			req.URL.RawQuery = targetQuery + "&" + req.URL.RawQuery
-		}
-		if _, ok := req.Header["User-Agent"]; !ok {
-			// explicitly disable User-Agent so it's not set to default value
-			req.Header.Set("User-Agent", "")
-		}
+			req.URL.Path = singleJoiningSlash(target.Path, req.URL.Path)
+			if targetQuery == "" || req.URL.RawQuery == "" {
+				req.URL.RawQuery = targetQuery + req.URL.RawQuery
+			} else {
+				req.URL.RawQuery = targetQuery + "&" + req.URL.RawQuery
+			}
+			if _, ok := req.Header["User-Agent"]; !ok {
+				// explicitly disable User-Agent so it's not set to default value
+				req.Header.Set("User-Agent", "")
+			}
+		},
 	}
 }
 func singleJoiningSlash(a, b string) string {
@@ -250,12 +269,12 @@ func prefixRouteMatcher(prefix string, target url.URL) bool {
 	return strings.HasPrefix(target.Path, prefix) && prefix != "/"
 }
 
-func SetDirectorFunc(parent context.Context, fn func(*http.Request)) context.Context {
-	return context.WithValue(parent, directorCtxKey, fn)
+func SetRoutingInfo(parent context.Context, ri RoutingInfo) context.Context {
+	return context.WithValue(parent, routingInfoCtxKey, ri)
 }
 
-// DirectorFunc gets the director function from the context.
-func DirectorFunc(ctx context.Context) func(*http.Request) {
-	val := ctx.Value(directorCtxKey)
-	return val.(func(*http.Request))
+// ContextRoutingInfo gets the routing information from the context.
+func ContextRoutingInfo(ctx context.Context) RoutingInfo {
+	val := ctx.Value(routingInfoCtxKey)
+	return val.(RoutingInfo)
 }