From ddfc01bff9242db5042f751ee91ef41d8cbfb5ad Mon Sep 17 00:00:00 2001
From: David Christofas <dchristofas@owncloud.com>
Date: Mon, 8 Aug 2022 16:59:05 +0200
Subject: [PATCH] refactor unprotected paths check

---
 .../proxy/pkg/middleware/authentication.go    | 51 ++++++++++++++-----
 services/proxy/pkg/middleware/oidc_auth.go    | 13 -----
 2 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/services/proxy/pkg/middleware/authentication.go b/services/proxy/pkg/middleware/authentication.go
index 6c961657d..887e116be 100644
--- a/services/proxy/pkg/middleware/authentication.go
+++ b/services/proxy/pkg/middleware/authentication.go
@@ -23,7 +23,31 @@ var (
 		"/remote.php/dav/public-files/",
 		"/remote.php/ocs/apps/files_sharing/api/v1/tokeninfo/unprotected",
 		"/ocs/v1.php/cloud/capabilities",
-		"/data",
+	}
+	// _unprotectedPaths contains paths which don't need to be authenticated.
+	_unprotectedPaths = map[string]struct{}{
+		"/":                    {},
+		"/login":               {},
+		"/app/list":            {},
+		"/config.json":         {},
+		"/oidc-callback.html":  {},
+		"/oidc-callback":       {},
+		"/settings.js":         {},
+		"/data":                {},
+		"/konnect/v1/userinfo": {},
+		"/status.php":          {},
+	}
+	// _unprotectedPathPrefixes contains paths which don't need to be authenticated.
+	_unprotectedPathPrefixes = [...]string{
+		"/files",
+		"/settings",
+		"/user-management",
+		"/.well-known",
+		"/js",
+		"/icons",
+		"/themes",
+		"/signin",
+		"/konnect",
 	}
 )
 
@@ -46,18 +70,7 @@ 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) ||
-				r.URL.Path == "/" ||
-				strings.HasPrefix(r.URL.Path, "/.well-known") ||
-				r.URL.Path == "/login" ||
-				strings.HasPrefix(r.URL.Path, "/js") ||
-				strings.HasPrefix(r.URL.Path, "/themes") ||
-				strings.HasPrefix(r.URL.Path, "/signin") ||
-				strings.HasPrefix(r.URL.Path, "/konnect") ||
-				r.URL.Path == "/config.json" ||
-				r.URL.Path == "/oidc-callback.html" ||
-				r.URL.Path == "/oidc-callback" ||
-				r.URL.Path == "/settings.js" {
+			if isOIDCTokenAuth(r) || isUnprotectedPath(r) {
 				// The authentication for this request is handled by the IdP.
 				next.ServeHTTP(w, r)
 				return
@@ -96,6 +109,18 @@ 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) {
diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go
index 5d740843d..a6c1b5519 100644
--- a/services/proxy/pkg/middleware/oidc_auth.go
+++ b/services/proxy/pkg/middleware/oidc_auth.go
@@ -25,11 +25,6 @@ const (
 	_bearerPrefix        = "Bearer "
 )
 
-var (
-	// _unauthenticatePaths contains paths which don't need to be authenticated.
-	_unauthenticatePaths = [...]string{"/konnect/v1/userinfo", "/status.php"}
-)
-
 // OIDCProvider used to mock the oidc provider during tests
 type OIDCProvider interface {
 	UserInfo(ctx context.Context, ts oauth2.TokenSource) (*gOidc.UserInfo, error)
@@ -146,14 +141,6 @@ func (m OIDCAuthenticator) shouldServe(req *http.Request) bool {
 		return false
 	}
 
-	// todo: looks dirty, check later
-	// TODO: make a PR to coreos/go-oidc for exposing userinfo endpoint on provider, see https://github.com/coreos/go-oidc/issues/248
-	for _, ignoringPath := range _unauthenticatePaths {
-		if req.URL.Path == ignoringPath {
-			return false
-		}
-	}
-
 	header := req.Header.Get(_headerAuthorization)
 	return strings.HasPrefix(header, _bearerPrefix)
 }