Merge pull request #819 from jcmoraisjr/jm-auth-deny-on-failure

Always deny requests of failed auth configurations
jcmoraisjr · Jul 10, 2021 · ba84b76 · ba84b76
2 parents 47b1d14 + 4752c27
commit ba84b76
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 8 deletions.
diff --git a/pkg/converters/ingress/annotations/backend.go b/pkg/converters/ingress/annotations/backend.go
@@ -113,10 +113,15 @@ func (c *updater) buildBackendAuthExternal(d *backData) {
 		if url.Source == nil || url.Value == "" {
 			continue
 		}
+
+		// starting here the auth backend should be configured or requests should be denied
+		// AlwaysDeny will be changed to false if the configuration succeeed
+		path.AuthExternal.AlwaysDeny = true
+
 		external := c.haproxy.Global().External
 		if external.IsExternal() && !external.HasLua {
 			c.logger.Warn("external authentication on %v needs Lua json module, install lua-json4 and enable 'external-has-lua' global config", url.Source)
-			return
+			continue
 		}
 
 		urlProto, urlHost, urlPort, urlPath, err := ingutils.ParseURL(url.Value)
@@ -221,6 +226,7 @@ func (c *updater) buildBackendAuthExternal(d *backData) {
 			urlPath = "/"
 		}
 
+		path.AuthExternal.AlwaysDeny = false
 		path.AuthExternal.AuthBackendName = authBackendName
 		path.AuthExternal.AuthPath = urlPath
 		path.AuthExternal.Method = method
@@ -613,17 +619,23 @@ func (c *updater) buildBackendOAuth(d *backData) {
 		if oauth.Source == nil {
 			continue
 		}
+
+		// starting here the auth backend should be configured or requests should be denied
+		// AlwaysDeny will be changed to false if the configuration succeeed
+		path.AuthExternal.AlwaysDeny = true
+
 		if oauth.Value != "oauth2_proxy" && oauth.Value != "oauth2-proxy" {
 			c.logger.Warn("ignoring invalid oauth implementation '%s' on %v", oauth, oauth.Source)
 			continue
 		}
 		external := c.haproxy.Global().External
 		if external.IsExternal() && !external.HasLua {
 			c.logger.Warn("oauth2_proxy on %v needs Lua json module, install lua-json4 and enable 'external-has-lua' global config", oauth.Source)
-			return
+			continue
 		}
 		if authURL := d.mapper.Get(ingtypes.BackAuthURL); authURL.Value != "" {
 			c.logger.Warn("ignoring oauth configuration on %v: auth-url was configured and has precedence", authURL.Source)
+			path.AuthExternal.AlwaysDeny = false
 			continue
 		}
 		uriPrefix := "/oauth2"
@@ -652,6 +664,7 @@ func (c *updater) buildBackendOAuth(d *backData) {
 			headersMap[h[0]] = buildAuthRequestVarName(h[len(h)-1])
 		}
 
+		path.AuthExternal.AlwaysDeny = false
 		path.AuthExternal.AuthBackendName = backend.ID
 		path.AuthExternal.AllowedPath = uriPrefix + "/"
 		path.AuthExternal.AuthPath = uriPrefix + "/auth"

diff --git a/pkg/converters/ingress/annotations/backend_test.go b/pkg/converters/ingress/annotations/backend_test.go
@@ -193,17 +193,20 @@ func TestAuthExternal(t *testing.T) {
 		// 0
 		{
 			url:     "10.0.0.1:8080/app",
+			expBack: hatypes.AuthExternal{AlwaysDeny: true},
 			logging: `WARN ignoring URL on ingress 'default/ing1': invalid URL syntax: 10.0.0.1:8080/app`,
 		},
 		// 1
 		{
 			url:     "fail://app1.local",
+			expBack: hatypes.AuthExternal{AlwaysDeny: true},
 			logging: `WARN ignoring auth URL with an invalid protocol on ingress 'default/ing1': fail`,
 		},
 		// 2
 		{
 			url:        "http://app1.local",
 			isExternal: true,
+			expBack:    hatypes.AuthExternal{AlwaysDeny: true},
 			logging:    `WARN external authentication on ingress 'default/ing1' needs Lua json module, install lua-json4 and enable 'external-has-lua' global config`,
 		},
 		// 3
@@ -238,6 +241,7 @@ func TestAuthExternal(t *testing.T) {
 		// 6
 		{
 			url:     "http://appfail.local/app",
+			expBack: hatypes.AuthExternal{AlwaysDeny: true},
 			logging: `WARN ignoring auth URL with an invalid domain on ingress 'default/ing1': host not found: appfail.local`,
 		},
 		// 7
@@ -307,16 +311,19 @@ func TestAuthExternal(t *testing.T) {
 		// 13
 		{
 			url:     "svc://noservice",
+			expBack: hatypes.AuthExternal{AlwaysDeny: true},
 			logging: `WARN skipping auth-url on ingress 'default/ing1': missing service port: svc://noservice`,
 		},
 		// 14
 		{
-			url: "svc://noservice:80",
+			url:     "svc://noservice:80",
+			expBack: hatypes.AuthExternal{AlwaysDeny: true},
 			// svc not found, warn is issued in the ingress parsing
 		},
 		// 15
 		{
 			url:     "svc://authservice",
+			expBack: hatypes.AuthExternal{AlwaysDeny: true},
 			logging: `WARN skipping auth-url on ingress 'default/ing1': missing service port: svc://authservice`,
 		},
 		// 16
@@ -1487,7 +1494,7 @@ func TestOAuth(t *testing.T) {
 				},
 			},
 			authExp: map[string]hatypes.AuthExternal{
-				"/": {},
+				"/": {AlwaysDeny: true},
 			},
 			logging: "WARN ignoring invalid oauth implementation 'none' on ingress 'default/ing1'",
 		},
@@ -1499,7 +1506,7 @@ func TestOAuth(t *testing.T) {
 				},
 			},
 			authExp: map[string]hatypes.AuthExternal{
-				"/": {},
+				"/": {AlwaysDeny: true},
 			},
 			logging: "ERROR path '/oauth2' was not found on namespace 'default'",
 		},
@@ -1655,10 +1662,13 @@ func TestOAuth(t *testing.T) {
 			external: true,
 			backend:  "default:back:/oauth2",
 			authExp: map[string]hatypes.AuthExternal{
-				"/":    {},
-				"/app": {},
+				"/":    {AlwaysDeny: true},
+				"/app": {AlwaysDeny: true},
 			},
-			logging: "WARN oauth2_proxy on ingress 'default/ing1' needs Lua json module, install lua-json4 and enable 'external-has-lua' global config",
+			logging: `
+WARN oauth2_proxy on ingress 'default/ing1' needs Lua json module, install lua-json4 and enable 'external-has-lua' global config
+WARN oauth2_proxy on ingress 'default/ing1' needs Lua json module, install lua-json4 and enable 'external-has-lua' global config
+`,
 		},
 		// 11
 		{

diff --git a/pkg/haproxy/instance_test.go b/pkg/haproxy/instance_test.go
@@ -652,6 +652,18 @@ d1.local#/ path01`,
     http-request lua.auth-intercept _auth_4001 /oauth2/auth HEAD '*' '-' '-' if { var(txn.pathID) path01 }
     http-request redirect location http://auth.local/login if !{ var(txn.auth_response_successful) -m bool } { var(txn.pathID) path01 }
     http-request set-header X-Auth-Request-Email %[var(req.auth_response_header.x_auth_request_email)] if { var(req.auth_response_header.x_auth_request_email) -m found } { var(txn.pathID) path01 }`,
+		},
+		{
+			doconfig: func(g *hatypes.Global, h *hatypes.Host, b *hatypes.Backend) {
+				auth := &b.FindBackendPath(h.FindPath("/app1")[0].Link).AuthExternal
+				auth.AlwaysDeny = true
+			},
+			path: []string{"/app1", "/app2"},
+			expected: `
+    # path01 = d1.local/app1
+    # path02 = d1.local/app2
+    http-request set-var(txn.pathID) var(req.base),lower,map_beg(/etc/haproxy/maps/_back_d1_app_8080_idpath__begin.map)
+    http-request deny { var(txn.pathID) path01 }`,
 		},
 		{
 			doconfig: func(g *hatypes.Global, h *hatypes.Host, b *hatypes.Backend) {

diff --git a/pkg/haproxy/types/types.go b/pkg/haproxy/types/types.go
@@ -757,6 +757,7 @@ type Cookie struct {
 // AuthExternal ...
 type AuthExternal struct {
 	AllowedPath     string
+	AlwaysDeny      bool
 	AuthBackendName string
 	AuthPath        string
 	HeadersFail     []string

diff --git a/rootfs/etc/templates/haproxy/haproxy.tmpl b/rootfs/etc/templates/haproxy/haproxy.tmpl
@@ -629,6 +629,10 @@ backend {{ $backend.ID }}
 {{- $authCfg := $backend.PathConfig "AuthExternal" }}
 {{- range $i, $auth := $authCfg.Items }}
 {{- range $pathIDs := $authCfg.PathIDs $i }}
+{{- if $auth.AlwaysDeny }}
+    http-request deny
+        {{- if $pathIDs }} { var(txn.pathID) {{ $pathIDs }} }{{ end }}
+{{- else }}
 {{- if $auth.AuthBackendName }}
     http-request set-header X-Real-IP %[src]
         {{- if $pathIDs }} if { var(txn.pathID) {{ $pathIDs }} }{{ end }}
@@ -653,6 +657,7 @@ backend {{ $backend.ID }}
 {{- end }}
 {{- end }}
 {{- end }}
+{{- end }}
 
 {{- /*------------------------------------*/}}
 {{- if $backend.Cookie.Name }}