From 364a7591d640298b478511c7d4589bfc8ccd7622 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Thu, 7 Sep 2023 15:25:39 +0300 Subject: [PATCH] feat: issuer from Forwarded header (#443) --- go.mod | 2 +- go.sum | 6 +-- pkg/op/config.go | 34 ++++++++++++++ pkg/op/config_test.go | 106 +++++++++++++++++++++++++++++++++++++++++- pkg/op/op.go | 7 +++ 5 files changed, 149 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index a8cd590..250edd5 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/gorilla/securecookie v1.1.1 github.com/jeremija/gosubmit v0.2.7 github.com/muhlemmer/gu v0.3.1 + github.com/muhlemmer/httpforwarded v0.1.0 github.com/rs/cors v1.10.0 github.com/sirupsen/logrus v1.9.3 github.com/stretchr/testify v1.8.4 @@ -34,6 +35,5 @@ require ( golang.org/x/sys v0.12.0 // indirect google.golang.org/appengine v1.6.7 // indirect google.golang.org/protobuf v1.31.0 // indirect - gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index e5102a8..7cd9691 100644 --- a/go.sum +++ b/go.sum @@ -32,12 +32,11 @@ github.com/gorilla/securecookie v1.1.1/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+ github.com/jeremija/gosubmit v0.2.7 h1:At0OhGCFGPXyjPYAsCchoBUhE099pcBXmsb4iZqROIc= github.com/jeremija/gosubmit v0.2.7/go.mod h1:Ui+HS073lCFREXBbdfrJzMB57OI/bdxTiLtrDHHhFPI= github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI= -github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= -github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= -github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/muhlemmer/gu v0.3.1 h1:7EAqmFrW7n3hETvuAdmFmn4hS8W+z3LgKtrnow+YzNM= github.com/muhlemmer/gu v0.3.1/go.mod h1:YHtHR+gxM+bKEIIs7Hmi9sPT3ZDUvTN/i88wQpZkrdM= +github.com/muhlemmer/httpforwarded v0.1.0 h1:x4DLrzXdliq8mprgUMR0olDvHGkou5BJsK/vWUetyzY= +github.com/muhlemmer/httpforwarded v0.1.0/go.mod h1:yo9czKedo2pdZhoXe+yDkGVbU0TJ0q9oQ90BVoDEtw0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rs/cors v1.10.0 h1:62NOS1h+r8p1mW6FM0FSB0exioXLhd/sh15KpjWBZ+8= @@ -103,7 +102,6 @@ google.golang.org/protobuf v1.31.0 h1:g0LDEJHgrBl9N9r17Ru3sqWhkIx2NB67okBHPwC7hs google.golang.org/protobuf v1.31.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= -gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/square/go-jose.v2 v2.6.0 h1:NGk74WTnPKBNUhNzQX7PYcTLUjoq7mzKk2OKbvwk2iI= gopkg.in/square/go-jose.v2 v2.6.0/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/pkg/op/config.go b/pkg/op/config.go index c40ed39..e5789f7 100644 --- a/pkg/op/config.go +++ b/pkg/op/config.go @@ -2,10 +2,12 @@ package op import ( "errors" + "log" "net/http" "net/url" "strings" + "github.com/muhlemmer/httpforwarded" "golang.org/x/text/language" ) @@ -52,6 +54,21 @@ type Configuration interface { type IssuerFromRequest func(r *http.Request) string func IssuerFromHost(path string) func(bool) (IssuerFromRequest, error) { + return issuerFromForwardedOrHost(path, false) +} + +// IssuerFromForwardedOrHost tries to establish the Issuer based +// on the Forwarded header host field. +// If multiple Forwarded headers are present, the first mention +// of the host field will be used. +// If the Forwarded header is not present, no host field is found, +// or there is a parser error the Request Host will be used as a fallback. +// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded +func IssuerFromForwardedOrHost(path string) func(bool) (IssuerFromRequest, error) { + return issuerFromForwardedOrHost(path, true) +} + +func issuerFromForwardedOrHost(path string, parseForwarded bool) func(bool) (IssuerFromRequest, error) { return func(allowInsecure bool) (IssuerFromRequest, error) { issuerPath, err := url.Parse(path) if err != nil { @@ -61,11 +78,28 @@ func IssuerFromHost(path string) func(bool) (IssuerFromRequest, error) { return nil, err } return func(r *http.Request) string { + if parseForwarded { + if host, ok := hostFromForwarded(r); ok { + return dynamicIssuer(host, path, allowInsecure) + } + } return dynamicIssuer(r.Host, path, allowInsecure) }, nil } } +func hostFromForwarded(r *http.Request) (host string, ok bool) { + fwd, err := httpforwarded.ParseFromRequest(r) + if err != nil { + log.Printf("Err: issuer from forwarded header: %v", err) // TODO change to slog on next branch + return "", false + } + if fwd == nil || len(fwd["host"]) == 0 { + return "", false + } + return fwd["host"][0], true +} + func StaticIssuer(issuer string) func(bool) (IssuerFromRequest, error) { return func(allowInsecure bool) (IssuerFromRequest, error) { if err := ValidateIssuer(issuer, allowInsecure); err != nil { diff --git a/pkg/op/config_test.go b/pkg/op/config_test.go index cfe4e61..dcafc3a 100644 --- a/pkg/op/config_test.go +++ b/pkg/op/config_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestValidateIssuer(t *testing.T) { @@ -234,7 +235,7 @@ func TestIssuerFromHost(t *testing.T) { }, }, { - "custom path unsecure", + "custom path insecure", args{ path: "/custom/", allowInsecure: true, @@ -261,6 +262,109 @@ func TestIssuerFromHost(t *testing.T) { } } +func TestIssuerFromForwardedOrHost(t *testing.T) { + type args struct { + path string + target string + forwarded []string + } + type res struct { + issuer string + } + tests := []struct { + name string + args args + res res + }{ + { + "header parse error", + args{ + path: "/custom/", + target: "https://issuer.com", + forwarded: []string{"~~~"}, + }, + res{ + issuer: "https://issuer.com/custom/", + }, + }, + { + "no forwarded header", + args{ + path: "/custom/", + target: "https://issuer.com", + }, + res{ + issuer: "https://issuer.com/custom/", + }, + }, + // by=;for=;host=;proto= + { + "forwarded header without host", + args{ + path: "/custom/", + target: "https://issuer.com", + forwarded: []string{ + `by=identifier;for=identifier;proto=https`, + }, + }, + res{ + issuer: "https://issuer.com/custom/", + }, + }, + { + "forwarded header with host", + args{ + path: "/custom/", + target: "https://issuer.com", + forwarded: []string{ + `by=identifier;for=identifier;host=first.com;proto=https`, + }, + }, + res{ + issuer: "https://first.com/custom/", + }, + }, + { + "forwarded header with multiple hosts", + args{ + path: "/custom/", + target: "https://issuer.com", + forwarded: []string{ + `by=identifier;for=identifier;host=first.com;proto=https,host=second.com`, + }, + }, + res{ + issuer: "https://first.com/custom/", + }, + }, + { + "multiple forwarded headers hosts", + args{ + path: "/custom/", + target: "https://issuer.com", + forwarded: []string{ + `by=identifier;for=identifier;host=first.com;proto=https,host=second.com`, + `by=identifier;for=identifier;host=third.com;proto=https`, + }, + }, + res{ + issuer: "https://first.com/custom/", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + issuer, err := IssuerFromForwardedOrHost(tt.args.path)(false) + require.NoError(t, err) + req := httptest.NewRequest("", tt.args.target, nil) + if tt.args.forwarded != nil { + req.Header["Forwarded"] = tt.args.forwarded + } + assert.Equal(t, tt.res.issuer, issuer(req)) + }) + } +} + func TestStaticIssuer(t *testing.T) { type args struct { issuer string diff --git a/pkg/op/op.go b/pkg/op/op.go index 484149e..c4be14f 100644 --- a/pkg/op/op.go +++ b/pkg/op/op.go @@ -169,10 +169,17 @@ func NewOpenIDProvider(issuer string, config *Config, storage Storage, opOpts .. return newProvider(config, storage, StaticIssuer(issuer), opOpts...) } +// NewForwardedOpenIDProvider tries to establishes the issuer from the request Host. func NewDynamicOpenIDProvider(path string, config *Config, storage Storage, opOpts ...Option) (*Provider, error) { return newProvider(config, storage, IssuerFromHost(path), opOpts...) } +// NewForwardedOpenIDProvider tries to establish the Issuer from a Forwarded request header, if it is set. +// See [IssuerFromForwardedOrHost] for details. +func NewForwardedOpenIDProvider(path string, config *Config, storage Storage, opOpts ...Option) (*Provider, error) { + return newProvider(config, storage, IssuerFromForwardedOrHost(path), opOpts...) +} + func newProvider(config *Config, storage Storage, issuer func(bool) (IssuerFromRequest, error), opOpts ...Option) (_ *Provider, err error) { o := &Provider{ config: config,