Add enable-default-deny-strict (#180)

config.go

@@ -340,6 +340,7 @@ func (r *Config) isReverseProxySettingsValid() error {
 	if !r.EnableForwarding {
 		validationRegistry := []func() error{
 			r.isUpstreamValid,
+			r.isDefaultDenyValid,
 			r.isEnableUmaValid,
 			r.isTokenVerificationSettingsValid,
 			r.isResourceValid,
@@ -570,6 +571,15 @@ func (r *Config) isEnableUmaValid() error {
 	return nil
 }
 
+func (r *Config) isDefaultDenyValid() error {
+	if r.EnableDefaultDeny && r.EnableDefaultDenyStrict {
+		return errors.New(
+			"only one of enable-default-deny/enable-default-deny-strict can be true",
+		)
+	}
+	return nil
+}
+
 func (r *Config) updateDiscoveryURI() error {
 	// step: fix up the url if required, the underlining lib will add
 	// the .well-known/openid-configuration to the discovery url for us.

config_test.go

@@ -1937,3 +1937,59 @@ func TestUpdateRealm(t *testing.T) {
 		)
 	}
 }
+
+func TestDefaultDenyValid(t *testing.T) {
+	testCases := []struct {
+		Name   string
+		Config *Config
+		Valid  bool
+	}{
+		{
+			Name: "ValidDefaultDeny",
+			Config: &Config{
+				EnableDefaultDeny: true,
+				ClientID:          "test",
+				ClientSecret:      "test",
+				NoRedirects:       true,
+			},
+			Valid: true,
+		},
+		{
+			Name: "ValidDefaultDenyStrict",
+			Config: &Config{
+				EnableDefaultDenyStrict: true,
+				ClientID:                "test",
+				ClientSecret:            "test",
+				NoRedirects:             true,
+			},
+			Valid: true,
+		},
+		{
+			Name: "InvalidDefaultDeny",
+			Config: &Config{
+				EnableDefaultDenyStrict: true,
+				EnableDefaultDeny:       true,
+				ClientID:                "test",
+				ClientSecret:            "test",
+			},
+			Valid: false,
+		},
+	}
+
+	for _, testCase := range testCases {
+		testCase := testCase
+		t.Run(
+			testCase.Name,
+			func(t *testing.T) {
+				err := testCase.Config.isDefaultDenyValid()
+				if err != nil && testCase.Valid {
+					t.Fatalf("Expected test not to fail")
+				}
+
+				if err == nil && !testCase.Valid {
+					t.Fatalf("Expected test to fail")
+				}
+			},
+		)
+	}
+}

doc.go

@@ -207,8 +207,10 @@ type Config struct {
 	EnableRequestID bool `json:"enable-request-id" yaml:"enable-request-id" usage:"indicates we should add a request id if none found" env:"ENABLE_REQUEST_ID"`
 	// EnableLogoutRedirect indicates we should redirect to the identity provider for logging out
 	EnableLogoutRedirect bool `json:"enable-logout-redirect" yaml:"enable-logout-redirect" usage:"indicates we should redirect to the identity provider for logging out" env:"ENABLE_LOGOUT_REDIRECT"`
-	// EnableDefaultDeny indicates we should deny by default all requests
-	EnableDefaultDeny bool `json:"enable-default-deny" yaml:"enable-default-deny" usage:"enables a default denial on all requests, you have to explicitly say what is permitted (recommended)" env:"ENABLE_DEFAULT_DENY"`
+	// EnableDefaultDeny indicates we should deny by default all unauthenticated requests
+	EnableDefaultDeny bool `json:"enable-default-deny" yaml:"enable-default-deny" usage:"enables a default denial on all unauthenticated requests, you have to explicitly say what is permitted, although be aware that it allows any valid token" env:"ENABLE_DEFAULT_DENY"`
+	// EnableDefaultDenyStrict indicates we should deny by default all requests
+	EnableDefaultDenyStrict bool `json:"enable-default-deny-strict" yaml:"enable-default-deny-strict" usage:"enables a default denial on all requests, even valid token is denied unless you create some resources" env:"ENABLE_DEFAULT_DENY_STRICT"`
 	// EnableEncryptedToken indicates the access token should be encoded
 	EnableEncryptedToken bool `json:"enable-encrypted-token" yaml:"enable-encrypted-token" usage:"enable encryption for the access tokens" env:"ENABLE_ENCRYPTED_TOKEN"`
 	// ForceEncryptedCookie indicates that the access token in the cookie should be encoded, regardless what EnableEncryptedToken says. This way, Louketo Proxy may receive tokens in header in the clear, whereas tokens in cookies remain encrypted

handlers_test.go

@@ -911,6 +911,20 @@ func TestDiscoveryURL(t *testing.T) {
 				},
 			},
 		},
+		{
+			Name: "TestWithDefaultDenyStrictDiscoveryOK",
+			ProxySettings: func(c *Config) {
+				c.EnableDefaultDenyStrict = true
+			},
+			ExecutionSettings: []fakeRequest{
+				{
+					URI:                     "/oauth/discovery",
+					ExpectedProxy:           false,
+					ExpectedCode:            http.StatusOK,
+					ExpectedContentContains: "login_endpoint",
+				},
+			},
+		},
 		{
 			Name: "TestEndpointPathCorrectWithDefaultDenyDiscoveryOK",
 			ProxySettings: func(c *Config) {

middleware.go

@@ -813,3 +813,13 @@ func proxyDenyMiddleware(next http.Handler) http.Handler {
 		next.ServeHTTP(w, req.WithContext(ctx))
 	})
 }
+
+// deny middleware
+func (r *oauthProxy) denyMiddleware(next http.Handler) http.Handler {
+	r.log.Info("enabling the deny middleware")
+
+	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		w.WriteHeader(http.StatusUnauthorized)
+		next.ServeHTTP(w, req)
+	})
+}

server.go

@@ -85,6 +85,8 @@ func init() {
 	prometheus.MustRegister(statusMetric)
 }
 
+const allPath = "/*"
+
 // newProxy create's a new proxy from configuration
 func newProxy(config *Config) (*oauthProxy, error) {
 	// create the service logger
@@ -209,7 +211,7 @@ func createLogger(config *Config) (*zap.Logger, error) {
 func (r *oauthProxy) useDefaultStack(engine chi.Router) {
 	engine.NotFound(emptyHandler)
 
-	if r.config.EnableDefaultDeny {
+	if r.config.EnableDefaultDeny || r.config.EnableDefaultDenyStrict {
 		engine.Use(r.methodCheckMiddleware)
 	} else {
 		engine.MethodNotAllowed(emptyHandler)
@@ -372,6 +374,7 @@ func (r *oauthProxy) createReverseProxy() error {
 
 	// step: provision in the protected resources
 	enableDefaultDeny := r.config.EnableDefaultDeny
+	enableDefaultDenyStrict := r.config.EnableDefaultDenyStrict
 
 	for _, x := range r.config.Resources {
 		if x.URL[len(x.URL)-1:] == "/" {
@@ -381,22 +384,23 @@ func (r *oauthProxy) createReverseProxy() error {
 				zap.String("amended", strings.TrimRight(x.URL, "/")))
 		}
 
-		if x.URL == "/*" && r.config.EnableDefaultDeny {
+		if x.URL == allPath && (r.config.EnableDefaultDeny || r.config.EnableDefaultDenyStrict) {
 			switch x.WhiteListed {
 			case true:
 				return errors.New("you've asked for a default denial but whitelisted everything")
 			default:
 				enableDefaultDeny = false
+				enableDefaultDenyStrict = false
 			}
 		}
 	}
 
-	if enableDefaultDeny {
+	if enableDefaultDeny || enableDefaultDenyStrict {
 		r.log.Info("adding a default denial into the protected resources")
 
 		r.config.Resources = append(
 			r.config.Resources,
-			&Resource{URL: "/*", Methods: allHTTPMethods},
+			&Resource{URL: allPath, Methods: allHTTPMethods},
 		)
 	}
 
@@ -412,6 +416,13 @@ func (r *oauthProxy) createReverseProxy() error {
 			r.identityHeadersMiddleware(r.config.AddClaims),
 		}
 
+		if x.URL == allPath && !x.WhiteListed && enableDefaultDenyStrict {
+			middlewares = []func(http.Handler) http.Handler{
+				r.denyMiddleware,
+				proxyDenyMiddleware,
+			}
+		}
+
 		if r.config.EnableUma {
 			middlewares = []func(http.Handler) http.Handler{
 				r.authenticationMiddleware(),

server_test.go

@@ -756,6 +756,118 @@ func TestDefaultDenial(t *testing.T) {
 				assert.Equal(t, "", body)
 			},
 		},
+		{
+			Method:        "whAS9023",
+			URI:           "/permited_with_valid_token",
+			HasToken:      true,
+			ProxyRequest:  true,
+			ExpectedProxy: false,
+			Redirects:     false,
+			ExpectedCode:  http.StatusNotImplemented,
+			ExpectedContent: func(body string, testNum int) {
+				assert.Equal(t, "", body)
+			},
+		},
+		{
+			Method:        "GET",
+			URI:           "/permited_with_valid_token",
+			HasToken:      true,
+			ProxyRequest:  true,
+			ExpectedProxy: true,
+			Redirects:     false,
+			ExpectedCode:  http.StatusOK,
+			ExpectedContent: func(body string, testNum int) {
+				assert.Contains(t, body, "gzip")
+			},
+		},
+	}
+	newFakeProxy(config, &fakeAuthConfig{}).RunTests(t, requests)
+}
+
+func TestDefaultDenialStrict(t *testing.T) {
+	config := newFakeKeycloakConfig()
+	config.EnableDefaultDenyStrict = true
+	config.Resources = []*Resource{
+		{
+			URL:         "/public/*",
+			Methods:     allHTTPMethods,
+			WhiteListed: true,
+		},
+		{
+			URL:     "/private",
+			Methods: []string{"GET"},
+		},
+	}
+	requests := []fakeRequest{
+		{
+			URI:                     "/public/allowed",
+			ExpectedProxy:           true,
+			ExpectedCode:            http.StatusOK,
+			ExpectedContentContains: "gzip",
+		},
+		{
+			URI:       "/not_permited",
+			Redirects: false,
+			ExpectedContent: func(body string, testNum int) {
+				assert.Equal(t, body, "")
+			},
+		},
+		// lowercase methods should not be valid
+		{
+			Method:       "get",
+			URI:          "/not_permited",
+			Redirects:    false,
+			ExpectedCode: http.StatusNotImplemented,
+			ExpectedContent: func(body string, testNum int) {
+				assert.Equal(t, "", body)
+			},
+		},
+		// any "crap" methods should not be valid
+		{
+			Method:       "whAS9023",
+			URI:          "/not_permited",
+			Redirects:    false,
+			ExpectedCode: http.StatusNotImplemented,
+			ExpectedContent: func(body string, testNum int) {
+				assert.Equal(t, "", body)
+			},
+		},
+		{
+			Method:        "GET",
+			URI:           "/not_permited_with_valid_token",
+			HasToken:      true,
+			ProxyRequest:  true,
+			ExpectedProxy: false,
+			Redirects:     false,
+			ExpectedCode:  http.StatusUnauthorized,
+			ExpectedContent: func(body string, testNum int) {
+				assert.Equal(t, "", body)
+			},
+		},
+		{
+			Method:        "GET",
+			URI:           "/private",
+			HasToken:      true,
+			ProxyRequest:  true,
+			ExpectedProxy: true,
+			Redirects:     false,
+			ExpectedCode:  http.StatusOK,
+			ExpectedContent: func(body string, testNum int) {
+				assert.Contains(t, body, "gzip")
+			},
+		},
+		{
+			Method:        "POST",
+			URI:           "/private",
+			HasToken:      true,
+			ProxyRequest:  true,
+			ExpectedProxy: false,
+			Redirects:     false,
+			ExpectedCode:  http.StatusUnauthorized,
+			ExpectedContent: func(body string, testNum int) {
+				assert.Equal(t, "", body)
+			},
+		},
 	}
 	newFakeProxy(config, &fakeAuthConfig{}).RunTests(t, requests)
 }