core/authorize: result denied improvements (#4952)

* core/authorize: result denied improvements

* add authenticate robots.txt

* fix tests

authenticate/handlers.go

@@ -63,6 +63,7 @@ func (a *Authenticate) Mount(r *mux.Router) {
 	r.Path("/").Handler(http.RedirectHandler("/.pomerium/", http.StatusFound))
 
 	r.Path("/robots.txt").HandlerFunc(a.RobotsTxt).Methods(http.MethodGet)
+
 	// Identity Provider (IdP) endpoints
 	r.Path("/oauth2/callback").Handler(httputil.HandlerFunc(a.OAuthCallback)).Methods(http.MethodGet, http.MethodPost)
 

authorize/check_response.go

@@ -136,6 +136,11 @@ func (a *Authorize) deniedResponse(
 
 	var respBody []byte
 	switch {
+	case getCheckRequestURL(in).Path == "/robots.txt":
+		code = 200
+		respBody = []byte("User-agent: *\nDisallow: /")
+		respHeader = append(respHeader,
+			mkHeader("Content-Type", "text/plain"))
 	case isJSONWebRequest(in):
 		respBody, _ = json.Marshal(map[string]any{
 			"error":      reason,
@@ -369,7 +374,11 @@ func isGRPCWebRequest(in *envoy_service_auth_v3.CheckRequest) bool {
 		return false
 	}
 
-	return accept.Acceptable("application/grpc-web-text")
+	mediaType, _ := accept.MostAcceptable([]string{
+		"text/html",
+		"application/grpc-web-text",
+	})
+	return mediaType == "application/grpc-web-text"
 }
 
 func isJSONWebRequest(in *envoy_service_auth_v3.CheckRequest) bool {
@@ -388,7 +397,11 @@ func isJSONWebRequest(in *envoy_service_auth_v3.CheckRequest) bool {
 		return false
 	}
 
-	return accept.Acceptable("application/json")
+	mediaType, _ := accept.MostAcceptable([]string{
+		"text/html",
+		"application/json",
+	})
+	return mediaType == "application/json"
 }
 
 func getHeader(hdrs map[string]string, key string) string {

config/envoyconfig/route_configurations_test.go

@@ -47,7 +47,6 @@ func TestBuilder_buildMainRouteConfiguration(t *testing.T) {
 					`+protojson.Format(b.buildControlPlanePrefixRoute(cfg.Options, "/.pomerium/"))+`,
 					`+protojson.Format(b.buildControlPlanePathRoute(cfg.Options, "/.well-known/pomerium"))+`,
 					`+protojson.Format(b.buildControlPlanePrefixRoute(cfg.Options, "/.well-known/pomerium/"))+`,
-					`+protojson.Format(b.buildControlPlanePathRoute(cfg.Options, "/robots.txt"))+`,
 					{
 						"name": "policy-0",
 						"match": {

config/envoyconfig/routes.go

@@ -68,10 +68,6 @@ func (b *Builder) buildPomeriumHTTPRoutes(
 			b.buildControlPlanePathRoute(options, "/.well-known/pomerium"),
 			b.buildControlPlanePrefixRoute(options, "/.well-known/pomerium/"),
 		)
-		// per #837, only add robots.txt if there are no unauthenticated routes
-		if !hasPublicPolicyMatchingURL(options, url.URL{Scheme: "https", Host: host, Path: "/robots.txt"}) {
-			routes = append(routes, b.buildControlPlanePathRoute(options, "/robots.txt"))
-		}
 	}
 
 	authRoutes, err := b.buildPomeriumAuthenticateHTTPRoutes(options, host)
@@ -102,6 +98,7 @@ func (b *Builder) buildPomeriumAuthenticateHTTPRoutes(
 			return []*envoy_config_route_v3.Route{
 				b.buildControlPlanePathRoute(options, options.AuthenticateCallbackPath),
 				b.buildControlPlanePathRoute(options, "/"),
+				b.buildControlPlanePathRoute(options, "/robots.txt"),
 			}, nil
 		}
 	}
@@ -609,15 +606,6 @@ func setHostRewriteOptions(policy *config.Policy, action *envoy_config_route_v3.
 	}
 }
 
-func hasPublicPolicyMatchingURL(options *config.Options, requestURL url.URL) bool {
-	for _, policy := range options.GetAllPolicies() {
-		if policy.AllowPublicUnauthenticatedAccess && policy.Matches(requestURL) {
-			return true
-		}
-	}
-	return false
-}
-
 func isProxyFrontingAuthenticate(options *config.Options, host string) (bool, error) {
 	authenticateURL, err := options.GetAuthenticateURL()
 	if err != nil {

config/envoyconfig/routes_test.go

@@ -110,9 +110,9 @@ func Test_buildPomeriumHTTPRoutes(t *testing.T) {
 			`+routeString("prefix", "/.pomerium/")+`,
 			`+routeString("path", "/.well-known/pomerium")+`,
 			`+routeString("prefix", "/.well-known/pomerium/")+`,
-			`+routeString("path", "/robots.txt")+`,
 			`+routeString("path", "/oauth2/callback")+`,
-			`+routeString("path", "/")+`
+			`+routeString("path", "/")+`,
+			`+routeString("path", "/robots.txt")+`
 		]`, routes)
 	})
 	t.Run("proxy fronting authenticate", func(t *testing.T) {
@@ -125,56 +125,6 @@ func Test_buildPomeriumHTTPRoutes(t *testing.T) {
 		require.NoError(t, err)
 		testutil.AssertProtoJSONEqual(t, "null", routes)
 	})
-
-	t.Run("with robots", func(t *testing.T) {
-		options := &config.Options{
-			Services:                 "all",
-			AuthenticateURLString:    "https://authenticate.example.com",
-			AuthenticateCallbackPath: "/oauth2/callback",
-			Policies: []config.Policy{{
-				From: "https://from.example.com",
-				To:   mustParseWeightedURLs(t, "https://to.example.com"),
-			}},
-		}
-		_ = options.Policies[0].Validate()
-		routes, err := b.buildPomeriumHTTPRoutes(options, "from.example.com")
-		require.NoError(t, err)
-
-		testutil.AssertProtoJSONEqual(t, `[
-			`+routeString("path", "/ping")+`,
-			`+routeString("path", "/healthz")+`,
-			`+routeString("path", "/.pomerium")+`,
-			`+routeString("prefix", "/.pomerium/")+`,
-			`+routeString("path", "/.well-known/pomerium")+`,
-			`+routeString("prefix", "/.well-known/pomerium/")+`,
-			`+routeString("path", "/robots.txt")+`
-		]`, routes)
-	})
-
-	t.Run("without robots", func(t *testing.T) {
-		options := &config.Options{
-			Services:                 "all",
-			AuthenticateURLString:    "https://authenticate.example.com",
-			AuthenticateCallbackPath: "/oauth2/callback",
-			Policies: []config.Policy{{
-				From:                             "https://from.example.com",
-				To:                               mustParseWeightedURLs(t, "https://to.example.com"),
-				AllowPublicUnauthenticatedAccess: true,
-			}},
-		}
-		_ = options.Policies[0].Validate()
-		routes, err := b.buildPomeriumHTTPRoutes(options, "from.example.com")
-		require.NoError(t, err)
-
-		testutil.AssertProtoJSONEqual(t, `[
-			`+routeString("path", "/ping")+`,
-			`+routeString("path", "/healthz")+`,
-			`+routeString("path", "/.pomerium")+`,
-			`+routeString("prefix", "/.pomerium/")+`,
-			`+routeString("path", "/.well-known/pomerium")+`,
-			`+routeString("prefix", "/.well-known/pomerium/")+`
-		]`, routes)
-	})
 }
 
 func Test_buildControlPlanePathRoute(t *testing.T) {

proxy/handlers.go

@@ -2,7 +2,6 @@ package proxy
 
 import (
 	"errors"
-	"fmt"
 	"io"
 	"net/http"
 	"net/url"
@@ -43,13 +42,6 @@ func (p *Proxy) registerDashboardHandlers(r *mux.Router) *mux.Router {
 	return r
 }
 
-// RobotsTxt sets the User-Agent header in the response to be "Disallow"
-func (p *Proxy) RobotsTxt(w http.ResponseWriter, _ *http.Request) {
-	w.Header().Set("Content-Type", "text/plain; charset=utf-8")
-	w.WriteHeader(http.StatusOK)
-	fmt.Fprintf(w, "User-agent: *\nDisallow: /")
-}
-
 // SignOut clears the local session and redirects the request to the sign out url.
 // It's the responsibility of the authenticate service to revoke the remote session and clear
 // the authenticate service's session state.

proxy/handlers_test.go

@@ -16,20 +16,6 @@ import (
 	"github.com/pomerium/pomerium/internal/urlutil"
 )
 
-func TestProxy_RobotsTxt(t *testing.T) {
-	proxy := Proxy{}
-	req := httptest.NewRequest(http.MethodGet, "/robots.txt", nil)
-	rr := httptest.NewRecorder()
-	proxy.RobotsTxt(rr, req)
-	if status := rr.Code; status != http.StatusOK {
-		t.Errorf("handler returned wrong status code: got %v want %v", status, http.StatusOK)
-	}
-	expected := "User-agent: *\nDisallow: /"
-	if rr.Body.String() != expected {
-		t.Errorf("handler returned wrong body: got %v want %v", rr.Body.String(), expected)
-	}
-}
-
 func TestProxy_SignOut(t *testing.T) {
 	t.Parallel()
 	tests := []struct {

proxy/proxy.go

@@ -112,7 +112,6 @@ func (p *Proxy) setHandlers(opts *config.Options) error {
 	})
 	r.SkipClean(true)
 	r.StrictSlash(true)
-	r.HandleFunc("/robots.txt", p.RobotsTxt).Methods(http.MethodGet)
 	// dashboard handlers are registered to all routes
 	r = p.registerDashboardHandlers(r)