WIP

pkg/authorization/rules.go

@@ -4,32 +4,24 @@ import (
 	"net"
 	"net/http"
 
+	middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
 )
 
-type AuthorizationPolicy int
-
-const (
-	NonePolicy AuthorizationPolicy = iota
-	AllowPolicy
-	DelegatePolicy
-	DenyPolicy
-)
-
 type RuleSet interface {
-	MatchesRequest(req *http.Request) AuthorizationPolicy
+	MatchesRequest(req *http.Request) middlewareapi.AuthorizationPolicy
 }
 
 type rule struct {
 	conditions []condition
-	policy     AuthorizationPolicy
+	policy     middlewareapi.AuthorizationPolicy
 }
 
-func (r rule) matches(req *http.Request) AuthorizationPolicy {
+func (r rule) matches(req *http.Request) middlewareapi.AuthorizationPolicy {
 	for _, condition := range r.conditions {
 		if !condition.matches(req) {
 			// One of the conditions didn't match so this rule does not apply
-			return NonePolicy
+			return middlewareapi.OmittedPolicy
 		}
 	}
 	// If all conditions match, return the configured rule policy
@@ -60,17 +52,17 @@ func newRule(authRule options.AuthorizationRule, getClientIPFunc func(*http.Requ
 		conditions = append(conditions, condition)
 	}
 
-	var policy AuthorizationPolicy
+	var policy middlewareapi.AuthorizationPolicy
 	switch authRule.Policy {
 	case options.AllowPolicy:
-		policy = AllowPolicy
+		policy = middlewareapi.AllowPolicy
 	case options.DelegatePolicy:
-		policy = DelegatePolicy
+		policy = middlewareapi.DelegatePolicy
 	case options.DenyPolicy:
-		policy = DenyPolicy
+		policy = middlewareapi.DenyPolicy
 	default:
 		// This shouldn't be the case and should be prevented by validation
-		policy = NonePolicy
+		policy = middlewareapi.OmittedPolicy
 	}
 
 	return rule{
@@ -83,15 +75,15 @@ type ruleSet struct {
 	rules []rule
 }
 
-func (r ruleSet) MatchesRequest(req *http.Request) AuthorizationPolicy {
+func (r ruleSet) MatchesRequest(req *http.Request) middlewareapi.AuthorizationPolicy {
 	for _, rule := range r.rules {
-		if policy := rule.matches(req); policy != NonePolicy {
+		if policy := rule.matches(req); policy != middlewareapi.OmittedPolicy {
 			// The rule applies to this request, return its policy
 			return policy
 		}
 	}
 	// No rules matched
-	return NonePolicy
+	return middlewareapi.OmittedPolicy
 }
 
 func NewRuleSet(requestRules []options.AuthorizationRule, getClientIPFunc func(*http.Request) net.IP) (RuleSet, error) {

pkg/authorization/rules_test.go

@@ -6,10 +6,11 @@ import (
 	"net/http/httptest"
 	"testing"
 
+	middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
 )
 
-var result AuthorizationPolicy
+var result middlewareapi.AuthorizationPolicy
 
 func benchmarkRuleSetMatches(ruleCount int, b *testing.B) {
 	rule1 := options.AuthorizationRule{
@@ -53,10 +54,10 @@ func benchmarkRuleSetMatches(ruleCount int, b *testing.B) {
 
 	req := httptest.NewRequest("GET", "/foo/bar/baz", nil)
 
-	var r AuthorizationPolicy
+	var r middlewareapi.AuthorizationPolicy
 	for n := 0; n < b.N; n++ {
 		r = ruleSet.MatchesRequest(req)
-		if r != NonePolicy {
+		if r != middlewareapi.OmittedPolicy {
 			b.Fatal("expected policy not to match")
 		}
 	}

pkg/middleware/request_authorization.go

@@ -0,0 +1,61 @@
+package middleware
+
+import (
+	"fmt"
+	"net"
+	"net/http"
+
+	"github.com/justinas/alice"
+	middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/app/pagewriter"
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/authorization"
+)
+
+func NewRequestAuthorization(writer pagewriter.Writer, requestRules []options.AuthorizationRule, getClientIPFunc func(*http.Request) net.IP) (alice.Constructor, error) {
+	ruleset, err := authorization.NewRuleSet(requestRules, getClientIPFunc)
+	if err != nil {
+		return nil, fmt.Errorf("could not initialise ruleset: %w", err)
+	}
+
+	ra := &requestAuthorizer{
+		ruleset: ruleset,
+		writer:  writer,
+	}
+
+	return ra.checkRequestAuthorization, nil
+}
+
+type requestAuthorizer struct {
+	ruleset authorization.RuleSet
+	writer  pagewriter.Writer
+}
+
+func (r *requestAuthorizer) checkRequestAuthorization(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+		scope := middlewareapi.GetRequestScope(req)
+		// If scope is nil, this will panic.
+		// A scope should always be injected before this handler is called.
+		if scope.Authorization.Policy != middlewareapi.OmittedPolicy {
+			// The request was already authorized, pass to the next handler
+			next.ServeHTTP(rw, req)
+			return
+		}
+
+		policy := r.ruleset.MatchesRequest(req)
+		switch policy {
+		case middlewareapi.AllowPolicy, middlewareapi.DelegatePolicy:
+			scope.Authorization.Type = middlewareapi.RequestAuthorization
+			scope.Authorization.Policy = policy
+		case middlewareapi.DenyPolicy:
+			r.writer.WriteErrorPage(rw, pagewriter.ErrorPageOpts{
+				Status:    http.StatusForbidden,
+				RequestID: scope.RequestID,
+				AppError:  "Request denied by authorization policy",
+				Messages:  []interface{}{"Request denied by authorization policy"},
+			})
+		}
+
+		next.ServeHTTP(rw, req)
+	})
+}

pkg/validation/allowlist.go

@@ -1,70 +0,0 @@
-package validation
-
-import (
-	"fmt"
-	"os"
-	"regexp"
-	"strings"
-
-	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
-	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/ip"
-)
-
-func validateAllowlists(o *options.Options) []string {
-	msgs := []string{}
-
-	msgs = append(msgs, validateRoutes(o)...)
-	msgs = append(msgs, validateRegexes(o)...)
-	msgs = append(msgs, validateTrustedIPs(o)...)
-
-	if len(o.TrustedIPs) > 0 && o.ReverseProxy {
-		_, err := fmt.Fprintln(os.Stderr, "WARNING: mixing --trusted-ip with --reverse-proxy is a potential security vulnerability. An attacker can inject a trusted IP into an X-Real-IP or X-Forwarded-For header if they aren't properly protected outside of oauth2-proxy")
-		if err != nil {
-			panic(err)
-		}
-	}
-
-	return msgs
-}
-
-// validateRoutes validates method=path routes passed with options.SkipAuthRoutes
-func validateRoutes(o *options.Options) []string {
-	msgs := []string{}
-	for _, route := range o.SkipAuthRoutes {
-		var regex string
-		parts := strings.SplitN(route, "=", 2)
-		if len(parts) == 1 {
-			regex = parts[0]
-		} else {
-			regex = parts[1]
-		}
-		_, err := regexp.Compile(regex)
-		if err != nil {
-			msgs = append(msgs, fmt.Sprintf("error compiling regex /%s/: %v", regex, err))
-		}
-	}
-	return msgs
-}
-
-// validateRegex validates regex paths passed with options.SkipAuthRegex
-func validateRegexes(o *options.Options) []string {
-	msgs := []string{}
-	for _, regex := range o.SkipAuthRegex {
-		_, err := regexp.Compile(regex)
-		if err != nil {
-			msgs = append(msgs, fmt.Sprintf("error compiling regex /%s/: %v", regex, err))
-		}
-	}
-	return msgs
-}
-
-// validateTrustedIPs validates IP/CIDRs for IP based allowlists
-func validateTrustedIPs(o *options.Options) []string {
-	msgs := []string{}
-	for i, ipStr := range o.TrustedIPs {
-		if nil == ip.ParseIPNet(ipStr) {
-			msgs = append(msgs, fmt.Sprintf("trusted_ips[%d] (%s) could not be recognized", i, ipStr))
-		}
-	}
-	return msgs
-}

pkg/validation/allowlist_test.go

@@ -1,125 +1,125 @@
 package validation
 
-import (
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/ginkgo/extensions/table"
-	. "github.com/onsi/gomega"
-
-	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
-)
-
-var _ = Describe("Allowlist", func() {
-	type validateRoutesTableInput struct {
-		routes     []string
-		errStrings []string
-	}
-
-	type validateRegexesTableInput struct {
-		regexes    []string
-		errStrings []string
-	}
-
-	type validateTrustedIPsTableInput struct {
-		trustedIPs []string
-		errStrings []string
-	}
-
-	DescribeTable("validateRoutes",
-		func(r *validateRoutesTableInput) {
-			opts := &options.Options{
-				SkipAuthRoutes: r.routes,
-			}
-			Expect(validateRoutes(opts)).To(ConsistOf(r.errStrings))
-		},
-		Entry("Valid regex routes", &validateRoutesTableInput{
-			routes: []string{
-				"/foo",
-				"POST=/foo/bar",
-				"PUT=^/foo/bar$",
-				"DELETE=/crazy/(?:regex)?/[^/]+/stuff$",
-			},
-			errStrings: []string{},
-		}),
-		Entry("Bad regexes do not compile", &validateRoutesTableInput{
-			routes: []string{
-				"POST=/(foo",
-				"OPTIONS=/foo/bar)",
-				"GET=^]/foo/bar[$",
-				"GET=^]/foo/bar[$",
-			},
-			errStrings: []string{
-				"error compiling regex //(foo/: error parsing regexp: missing closing ): `/(foo`",
-				"error compiling regex //foo/bar)/: error parsing regexp: unexpected ): `/foo/bar)`",
-				"error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",
-				"error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",
-			},
-		}),
-	)
-
-	DescribeTable("validateRegexes",
-		func(r *validateRegexesTableInput) {
-			opts := &options.Options{
-				SkipAuthRegex: r.regexes,
-			}
-			Expect(validateRegexes(opts)).To(ConsistOf(r.errStrings))
-		},
-		Entry("Valid regex routes", &validateRegexesTableInput{
-			regexes: []string{
-				"/foo",
-				"/foo/bar",
-				"^/foo/bar$",
-				"/crazy/(?:regex)?/[^/]+/stuff$",
-			},
-			errStrings: []string{},
-		}),
-		Entry("Bad regexes do not compile", &validateRegexesTableInput{
-			regexes: []string{
-				"/(foo",
-				"/foo/bar)",
-				"^]/foo/bar[$",
-				"^]/foo/bar[$",
-			},
-			errStrings: []string{
-				"error compiling regex //(foo/: error parsing regexp: missing closing ): `/(foo`",
-				"error compiling regex //foo/bar)/: error parsing regexp: unexpected ): `/foo/bar)`",
-				"error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",
-				"error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",
-			},
-		}),
-	)
-
-	DescribeTable("validateTrustedIPs",
-		func(t *validateTrustedIPsTableInput) {
-			opts := &options.Options{
-				TrustedIPs: t.trustedIPs,
-			}
-			Expect(validateTrustedIPs(opts)).To(ConsistOf(t.errStrings))
-		},
-		Entry("Non-overlapping valid IPs", &validateTrustedIPsTableInput{
-			trustedIPs: []string{
-				"127.0.0.1",
-				"10.32.0.1/32",
-				"43.36.201.0/24",
-				"::1",
-				"2a12:105:ee7:9234:0:0:0:0/64",
-			},
-			errStrings: []string{},
-		}),
-		Entry("Overlapping valid IPs", &validateTrustedIPsTableInput{
-			trustedIPs: []string{
-				"135.180.78.199",
-				"135.180.78.199/32",
-				"d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4",
-				"d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4/128",
-			},
-			errStrings: []string{},
-		}),
-		Entry("Invalid IPs", &validateTrustedIPsTableInput{
-			trustedIPs: []string{"[::1]", "alkwlkbn/32"},
-			errStrings: []string{
-				"trusted_ips[0] ([::1]) could not be recognized",
-				"trusted_ips[1] (alkwlkbn/32) could not be recognized",
-			},
-		}),
-	)
-})
+// import (
+// 	. "github.com/onsi/ginkgo"
+// 	. "github.com/onsi/ginkgo/extensions/table"
+// 	. "github.com/onsi/gomega"
+//
+// 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
+// )
+//
+// var _ = Describe("Allowlist", func() {
+// 	type validateRoutesTableInput struct {
+// 		routes     []string
+// 		errStrings []string
+// 	}
+//
+// 	type validateRegexesTableInput struct {
+// 		regexes    []string
+// 		errStrings []string
+// 	}
+//
+// 	type validateTrustedIPsTableInput struct {
+// 		trustedIPs []string
+// 		errStrings []string
+// 	}
+//
+// 	DescribeTable("validateRoutes",
+// 		func(r *validateRoutesTableInput) {
+// 			opts := &options.Options{
+// 				SkipAuthRoutes: r.routes,
+// 			}
+// 			Expect(validateRoutes(opts)).To(ConsistOf(r.errStrings))
+// 		},
+// 		Entry("Valid regex routes", &validateRoutesTableInput{
+// 			routes: []string{
+// 				"/foo",
+// 				"POST=/foo/bar",
+// 				"PUT=^/foo/bar$",
+// 				"DELETE=/crazy/(?:regex)?/[^/]+/stuff$",
+// 			},
+// 			errStrings: []string{},
+// 		}),
+// 		Entry("Bad regexes do not compile", &validateRoutesTableInput{
+// 			routes: []string{
+// 				"POST=/(foo",
+// 				"OPTIONS=/foo/bar)",
+// 				"GET=^]/foo/bar[$",
+// 				"GET=^]/foo/bar[$",
+// 			},
+// 			errStrings: []string{
+// 				"error compiling regex //(foo/: error parsing regexp: missing closing ): `/(foo`",
+// 				"error compiling regex //foo/bar)/: error parsing regexp: unexpected ): `/foo/bar)`",
+// 				"error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",
+// 				"error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",
+// 			},
+// 		}),
+// 	)
+//
+// 	DescribeTable("validateRegexes",
+// 		func(r *validateRegexesTableInput) {
+// 			opts := &options.Options{
+// 				SkipAuthRegex: r.regexes,
+// 			}
+// 			Expect(validateRegexes(opts)).To(ConsistOf(r.errStrings))
+// 		},
+// 		Entry("Valid regex routes", &validateRegexesTableInput{
+// 			regexes: []string{
+// 				"/foo",
+// 				"/foo/bar",
+// 				"^/foo/bar$",
+// 				"/crazy/(?:regex)?/[^/]+/stuff$",
+// 			},
+// 			errStrings: []string{},
+// 		}),
+// 		Entry("Bad regexes do not compile", &validateRegexesTableInput{
+// 			regexes: []string{
+// 				"/(foo",
+// 				"/foo/bar)",
+// 				"^]/foo/bar[$",
+// 				"^]/foo/bar[$",
+// 			},
+// 			errStrings: []string{
+// 				"error compiling regex //(foo/: error parsing regexp: missing closing ): `/(foo`",
+// 				"error compiling regex //foo/bar)/: error parsing regexp: unexpected ): `/foo/bar)`",
+// 				"error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",
+// 				"error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",
+// 			},
+// 		}),
+// 	)
+//
+// 	DescribeTable("validateTrustedIPs",
+// 		func(t *validateTrustedIPsTableInput) {
+// 			opts := &options.Options{
+// 				TrustedIPs: t.trustedIPs,
+// 			}
+// 			Expect(validateTrustedIPs(opts)).To(ConsistOf(t.errStrings))
+// 		},
+// 		Entry("Non-overlapping valid IPs", &validateTrustedIPsTableInput{
+// 			trustedIPs: []string{
+// 				"127.0.0.1",
+// 				"10.32.0.1/32",
+// 				"43.36.201.0/24",
+// 				"::1",
+// 				"2a12:105:ee7:9234:0:0:0:0/64",
+// 			},
+// 			errStrings: []string{},
+// 		}),
+// 		Entry("Overlapping valid IPs", &validateTrustedIPsTableInput{
+// 			trustedIPs: []string{
+// 				"135.180.78.199",
+// 				"135.180.78.199/32",
+// 				"d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4",
+// 				"d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4/128",
+// 			},
+// 			errStrings: []string{},
+// 		}),
+// 		Entry("Invalid IPs", &validateTrustedIPsTableInput{
+// 			trustedIPs: []string{"[::1]", "alkwlkbn/32"},
+// 			errStrings: []string{
+// 				"trusted_ips[0] ([::1]) could not be recognized",
+// 				"trusted_ips[1] (alkwlkbn/32) could not be recognized",
+// 			},
+// 		}),
+// 	)
+// })

pkg/validation/authorization.go

@@ -0,0 +1,94 @@
+package validation
+
+import (
+	"fmt"
+	"os"
+	"regexp"
+
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/ip"
+)
+
+func validateAuthorization(authorization options.Authorization, reverseProxy bool) []string {
+	msgs := []string{}
+
+	msgs = append(msgs, validateRequestRules(authorization.RequestRules, reverseProxy)...)
+
+	return msgs
+}
+
+func validateRequestRules(rules []options.AuthorizationRule, reverseProxy bool) []string {
+	msgs := []string{}
+
+	ids := make(map[string]struct{})
+
+	for _, rule := range rules {
+		msgs = append(msgs, validateRequestRule(ids, rule, reverseProxy)...)
+	}
+
+	return msgs
+}
+
+func validateRequestRule(ids map[string]struct{}, rule options.AuthorizationRule, reverseProxy bool) []string {
+	msgs := []string{}
+
+	if rule.ID == "" {
+		msgs = append(msgs, "request rule has empty ID: IDs are required for all request rules")
+	}
+
+	if _, ok := ids[rule.ID]; ok {
+		msgs = append(msgs, fmt.Sprintf("multiple request rules found with ID %q: request rule IDs must be unique", rule.ID))
+	}
+	ids[rule.ID] = struct{}{}
+
+	msgs = append(msgs, validateRequestRulePolicy(rule.ID, rule.Policy)...)
+	msgs = append(msgs, validateRequestRulePath(rule.ID, rule.Path)...)
+	msgs = append(msgs, validateRequestRuleIPs(rule.ID, rule.IPs, reverseProxy)...)
+
+	return msgs
+}
+
+func validateRequestRulePolicy(ruleID string, policy options.AuthorizationPolicy) []string {
+	msgs := []string{}
+
+	switch policy {
+	case options.AllowPolicy, options.DenyPolicy, options.DelegatePolicy:
+		// Do nothing for valid options
+	default:
+		msgs = append(msgs, fmt.Sprintf("request rule %q has invalid policy (%s): policy must be one of %s, %s or %s", ruleID, policy, options.AllowPolicy, options.DenyPolicy, options.DelegatePolicy))
+	}
+
+	return msgs
+}
+
+// validateRequestRulePath validates paths for path/regex based conditions
+func validateRequestRulePath(ruleID string, path string) []string {
+	msgs := []string{}
+
+	_, err := regexp.Compile(path)
+	if err != nil {
+		msgs = append(msgs, fmt.Sprintf("error compiling path regex (%s) for rule %q: %v", path, ruleID, err))
+	}
+
+	return msgs
+}
+
+// validateRequestRuleIPs validates IP/CIDRs for IP based conditions.
+func validateRequestRuleIPs(ruleID string, ips []string, reverseProxy bool) []string {
+	msgs := []string{}
+
+	if len(ips) > 0 && reverseProxy {
+		_, err := fmt.Fprintln(os.Stderr, "WARNING: mixing IP authorization with --reverse-proxy is a potential security vulnerability. An attacker can inject a trusted IP into an X-Real-IP or X-Forwarded-For header if they aren't properly protected outside of oauth2-proxy")
+		if err != nil {
+			panic(err)
+		}
+	}
+
+	for i, ipStr := range ips {
+		if nil == ip.ParseIPNet(ipStr) {
+			msgs = append(msgs, fmt.Sprintf("rule %q IP [%d] (%s) could not be recognized", ruleID, i, ipStr))
+		}
+	}
+
+	return msgs
+}

pkg/validation/options.go

@@ -20,6 +20,7 @@ import (
 // are of the correct format
 func Validate(o *options.Options) error {
 	msgs := validateCookie(o.Cookie)
+	msgs = append(msgs, validateAuthorization(o.Authorization, o.ReverseProxy)...)
 	msgs = append(msgs, validateSessionCookieMinimal(o)...)
 	msgs = append(msgs, validateRedisSessionStore(o)...)
 	msgs = append(msgs, prefixValues("injectRequestHeaders: ", validateHeaders(o.InjectRequestHeaders)...)...)
@@ -96,9 +97,6 @@ func Validate(o *options.Options) error {
 		})
 	}
 
-	// Do this after ReverseProxy validation for TrustedIP coordinated checks
-	msgs = append(msgs, validateAllowlists(o)...)
-
 	if len(msgs) != 0 {
 		return fmt.Errorf("invalid configuration:\n  %s",
 			strings.Join(msgs, "\n  "))