authorize: handle user-unauthenticated response for deny blocks (#3559)

* authorize: handle user-unauthenticated response for deny blocks

* fix test

authorize/check_response.go

@@ -27,6 +27,41 @@ import (
 	"github.com/pomerium/pomerium/pkg/webauthnutil"
 )
 
+func (a *Authorize) handleResult(
+	ctx context.Context,
+	in *envoy_service_auth_v3.CheckRequest,
+	request *evaluator.Request,
+	result *evaluator.Result,
+	isForwardAuthVerify bool,
+) (*envoy_service_auth_v3.CheckResponse, error) {
+	// when the user is unauthenticated it means they haven't
+	// logged in yet, so redirect to authenticate
+	if result.Allow.Reasons.Has(criteria.ReasonUserUnauthenticated) ||
+		result.Deny.Reasons.Has(criteria.ReasonUserUnauthenticated) {
+		return a.requireLoginResponse(ctx, in, request, isForwardAuthVerify)
+	}
+
+	// when the user's device is unauthenticated it means they haven't
+	// registered a webauthn device yet, so redirect to the webauthn flow
+	if result.Allow.Reasons.Has(criteria.ReasonDeviceUnauthenticated) ||
+		result.Deny.Reasons.Has(criteria.ReasonDeviceUnauthenticated) {
+		return a.requireWebAuthnResponse(ctx, in, request, result, isForwardAuthVerify)
+	}
+
+	// if there's a deny, the result is denied using the deny reasons.
+	if result.Deny.Value {
+		return a.handleResultDenied(ctx, in, request, result, isForwardAuthVerify, result.Deny.Reasons)
+	}
+
+	// if there's an allow, the result is allowed.
+	if result.Allow.Value {
+		return a.handleResultAllowed(ctx, in, result)
+	}
+
+	// otherwise, the result is denied using the allow reasons.
+	return a.handleResultDenied(ctx, in, request, result, isForwardAuthVerify, result.Allow.Reasons)
+}
+
 func (a *Authorize) handleResultAllowed(
 	ctx context.Context,
 	in *envoy_service_auth_v3.CheckRequest,
@@ -47,13 +82,7 @@ func (a *Authorize) handleResultDenied(
 	denyStatusText := http.StatusText(http.StatusForbidden)
 
 	switch {
-	case reasons.Has(criteria.ReasonUserUnauthenticated):
-		// when the user is unauthenticated it means they haven't
-		// logged in yet, so redirect to authenticate
-		return a.requireLoginResponse(ctx, in, request, isForwardAuthVerify)
 	case reasons.Has(criteria.ReasonDeviceUnauthenticated):
-		// when the user's device is unauthenticated it means they haven't
-		// registered a webauthn device yet, so redirect to the webauthn flow
 		return a.requireWebAuthnResponse(ctx, in, request, result, isForwardAuthVerify)
 	case reasons.Has(criteria.ReasonDeviceUnauthorized):
 		denyStatusCode = httputil.StatusDeviceUnauthorized

authorize/check_response_test.go

@@ -21,8 +21,40 @@ import (
 	"github.com/pomerium/pomerium/internal/atomicutil"
 	"github.com/pomerium/pomerium/internal/encoding/jws"
 	"github.com/pomerium/pomerium/internal/testutil"
+	"github.com/pomerium/pomerium/pkg/policy/criteria"
 )
 
+func TestAuthorize_handleResult(t *testing.T) {
+	opt := config.NewDefaultOptions()
+	opt.AuthenticateURLString = "https://authenticate.example.com"
+	opt.DataBrokerURLString = "https://databroker.example.com"
+	opt.SharedKey = "E8wWIMnihUx+AUfRegAQDNs8eRb3UrB5G3zlJW9XJDM="
+	a, err := New(&config.Config{Options: opt})
+	require.NoError(t, err)
+
+	t.Run("user-unauthenticated", func(t *testing.T) {
+		res, err := a.handleResult(context.Background(),
+			&envoy_service_auth_v3.CheckRequest{},
+			&evaluator.Request{},
+			&evaluator.Result{
+				Allow: evaluator.NewRuleResult(false, criteria.ReasonUserUnauthenticated),
+			},
+			false)
+		assert.NoError(t, err)
+		assert.Equal(t, 302, int(res.GetDeniedResponse().GetStatus().GetCode()))
+
+		res, err = a.handleResult(context.Background(),
+			&envoy_service_auth_v3.CheckRequest{},
+			&evaluator.Request{},
+			&evaluator.Result{
+				Deny: evaluator.NewRuleResult(false, criteria.ReasonUserUnauthenticated),
+			},
+			false)
+		assert.NoError(t, err)
+		assert.Equal(t, 302, int(res.GetDeniedResponse().GetStatus().GetCode()))
+	})
+}
+
 func TestAuthorize_okResponse(t *testing.T) {
 	opt := &config.Options{
 		AuthenticateURLString: "https://authenticate.example.com",

authorize/grpc.go

@@ -95,19 +95,7 @@ func (a *Authorize) Check(ctx context.Context, in *envoy_service_auth_v3.CheckRe
 	}
 
 	isForwardAuthVerify := isForwardAuth && hreq.URL.Path == "/verify"
-
+	return a.handleResult(ctx, in, req, res, isForwardAuthVerify)
-	// if there's a deny, the result is denied using the deny reasons.
-	if res.Deny.Value {
-		return a.handleResultDenied(ctx, in, req, res, isForwardAuthVerify, res.Deny.Reasons)
-	}
-
-	// if there's an allow, the result is allowed.
-	if res.Allow.Value {
-		return a.handleResultAllowed(ctx, in, res)
-	}
-
-	// otherwise, the result is denied using the allow reasons.
-	return a.handleResultDenied(ctx, in, req, res, isForwardAuthVerify, res.Allow.Reasons)
 }
 
 // isForwardAuth returns if the current request is a forward auth route.