Merge pull request #15153 from openshift-cherrypick-robot/cherry-pick-15127-to-release-4.18

[release-4.18] OCPBUGS-57181: Fix '/metrics' endpoint token review to handle interna…

Author: openshift-merge-bot[bot]

pkg/auth/oauth2/auth.go

@@ -304,12 +304,7 @@ func (a *OAuth2Authenticator) LoginFunc(w http.ResponseWriter, r *http.Request)
 	http.Redirect(w, r, a.oauth2Config().AuthCodeURL(state), http.StatusSeeOther)
 }
 
-func (a *OAuth2Authenticator) ReviewToken(r *http.Request) error {
-	token, err := sessions.GetSessionTokenFromCookie(r)
-	if err != nil {
-		return err
-	}
-
+func (a *OAuth2Authenticator) ReviewToken(ctx context.Context, token string) error {
 	tokenReview := &authv1.TokenReview{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "authentication.k8s.io/v1",
@@ -324,7 +319,7 @@ func (a *OAuth2Authenticator) ReviewToken(r *http.Request) error {
 		internalK8sClientset.
 		AuthenticationV1().
 		TokenReviews().
-		Create(r.Context(), tokenReview, metav1.CreateOptions{})
+		Create(ctx, tokenReview, metav1.CreateOptions{})
 
 	if err != nil {
 		return fmt.Errorf("failed to create TokenReview, %v", err)

pkg/auth/static/auth.go

@@ -1,6 +1,7 @@
 package static
 
 import (
+	"context"
 	"net/http"
 
 	"github.com/openshift/console/pkg/auth"
@@ -22,7 +23,7 @@ func (s *StaticAuthenticator) Authenticate(w http.ResponseWriter, req *http.Requ
 	return &userCopy, nil
 }
 
-func (s *StaticAuthenticator) ReviewToken(r *http.Request) error {
+func (s *StaticAuthenticator) ReviewToken(ctx context.Context, token string) error {
 	return nil
 }
 

pkg/auth/types.go

@@ -1,14 +1,15 @@
 package auth
 
 import (
+	"context"
 	"net/http"
 
 	"github.com/openshift/console/pkg/auth/sessions"
 )
 
 type Authenticator interface {
 	Authenticate(w http.ResponseWriter, req *http.Request) (*User, error)
-	ReviewToken(r *http.Request) error
+	ReviewToken(context context.Context, token string) error
 
 	LoginFunc(w http.ResponseWriter, req *http.Request)
 	LogoutFunc(w http.ResponseWriter, req *http.Request)

pkg/metrics/handler.go

@@ -44,11 +44,51 @@ func authMiddlewareWithUser(authenticator auth.Authenticator, csrfVerifier *csrf
 }
 
 func withTokenReview(authenticator auth.Authenticator, h http.HandlerFunc) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		user, err := authenticator.Authenticate(w, r)
+		if err != nil {
+			klog.V(4).Infof("TOKEN_REVIEW: '%s %s' unauthorized, unable to authenticate, %v", r.Method, r.URL.Path, err)
+			w.WriteHeader(http.StatusUnauthorized)
+			return
+		}
+
+		err = authenticator.ReviewToken(r.Context(), user.Token)
+		if err != nil {
+			klog.V(4).Infof("TOKEN_REVIEW: '%s %s' unauthorized, invalid user token, %v", r.Method, r.URL.Path, err)
+			w.WriteHeader(http.StatusUnauthorized)
+			return
+		}
+		klog.V(4).Infof("TOKEN_REVIEW: '%s %s' user token successfully validated", r.Method, r.URL.Path)
+		h(w, r)
+	}
+}
+
+func withBearerTokenReview(authenticator auth.Authenticator, h http.HandlerFunc) http.HandlerFunc {
 	return http.HandlerFunc(
 		func(w http.ResponseWriter, r *http.Request) {
-			err := authenticator.ReviewToken(r)
+			authorizationHeader := r.Header.Get("Authorization")
+			if authorizationHeader == "" {
+				klog.V(4).Infof("TOKEN_REVIEW: '%s %s' unauthorized, missing Authorization header", r.Method, r.URL.Path)
+				w.WriteHeader(http.StatusUnauthorized)
+				return
+			}
+
+			if !strings.HasPrefix(authorizationHeader, "Bearer ") {
+				klog.V(4).Infof("TOKEN_REVIEW: '%s %s' unauthorized, 'Bearer' type Authorization header required", r.Method, r.URL.Path)
+				w.WriteHeader(http.StatusUnauthorized)
+				return
+			}
+
+			bearerToken := strings.TrimPrefix(authorizationHeader, "Bearer ")
+			if bearerToken == "" {
+				klog.V(4).Infof("TOKEN_REVIEW: '%s %s' unauthorized, empty or missing Bearer token", r.Method, r.URL.Path)
+				w.WriteHeader(http.StatusUnauthorized)
+				return
+			}
+
+			err := authenticator.ReviewToken(r.Context(), bearerToken)
 			if err != nil {
-				klog.Errorf("TOKEN_REVIEW: '%s %s' unauthorized, invalid user token, %v", r.Method, r.URL.Path, err)
+				klog.V(4).Infof("TOKEN_REVIEW: '%s %s' unauthorized, invalid user token, %v", r.Method, r.URL.Path, err)
 				w.WriteHeader(http.StatusUnauthorized)
 				return
 			}

pkg/server/middleware.go

@@ -31,7 +31,6 @@ import (
 	"github.com/openshift/console/pkg/graphql/resolver"
 	helmhandlerspkg "github.com/openshift/console/pkg/helm/handlers"
 	"github.com/openshift/console/pkg/knative"
-	"github.com/openshift/console/pkg/metrics"
 	"github.com/openshift/console/pkg/olm"
 	"github.com/openshift/console/pkg/plugins"
 	"github.com/openshift/console/pkg/proxy"
@@ -292,8 +291,13 @@ func (s *Server) HTTPHandler() (http.Handler, error) {
 	}
 
 	tokenReviewHandler := func(h http.HandlerFunc) http.HandlerFunc {
-		return authHandler(withTokenReview(authenticator, h))
+		return s.CSRFVerifier.WithCSRFVerification(withTokenReview(authenticator, h))
 	}
+
+	bearerTokenReviewHandler := func(h http.HandlerFunc) http.HandlerFunc {
+		return withBearerTokenReview(authenticator, h)
+	}
+
 	handleFunc(authLoginEndpoint, s.Authenticator.LoginFunc)
 	handleFunc(authLogoutEndpoint, allowMethod(http.MethodPost, s.handleLogout))
 	handleFunc(AuthLoginCallbackEndpoint, s.Authenticator.CallbackFunc(fn))
@@ -603,11 +607,10 @@ func (s *Server) HTTPHandler() (http.Handler, error) {
 	prometheus.MustRegister(usageMetrics.GetCollectors()...)
 	prometheus.MustRegister(s.AuthMetrics.GetCollectors()...)
 
-	handle("/metrics", metrics.AddHeaderAsCookieMiddleware(
-		tokenReviewHandler(func(w http.ResponseWriter, r *http.Request) {
-			promhttp.Handler().ServeHTTP(w, r)
-		}),
-	))
+	handle("/metrics", bearerTokenReviewHandler(func(w http.ResponseWriter, r *http.Request) {
+		promhttp.Handler().ServeHTTP(w, r)
+	}))
+
 	handleFunc("/metrics/usage", tokenReviewHandler(func(w http.ResponseWriter, r *http.Request) {
 		usage.Handle(usageMetrics, w, r)
 	}))