Repository: zitadel/oidc
Pull request #443 (2023-09-07 09:16:34)

feat: issuer from Forwarded header
Due to the need in zitadel to run OIDC behind a proxy,
we need to be able to overwrite the Host from the request.

To provide that, it is now possible to start a provider that parses the standarized [`Forwarded`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded) header with a fallback to the Request Host field.

```
Forwarded: by=<identifier>;for=<identifier>;host=<host>;proto=<http|https>
```

In order not to break installations that might be already running behind a proxy with the header set, a seperate constructor for the provider is added for the new usecase.

### Definition of Ready

- [x] I am happy with the code
- [x] Short description of the feature/issue is added in the pr description
- [ ] PR is linked to the corresponding user story
- [x] Acceptance criteria are met
- [x] All open todos and follow ups are defined in a new ticket and justified
- [x] Deviations from the acceptance criteria and design are agreed with the PO and documented.
- [x] No debug or dead code
- [x] My code has no repetitions
- [x] Critical parts are tested automatically
- [x] Where possible E2E tests are implemented
- [x] Documentation/examples are up-to-date
- [x] All non-functional requirements are met
- [ ] Functionality of the acceptance criteria is checked manually on the dev system.



codecov[bot]: ## [Codecov](https://app.codecov.io/gh/zitadel/oidc/pull/443?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zitadel) Report
> Merging [#443](https://app.codecov.io/gh/zitadel/oidc/pull/443?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zitadel) (26898c9) into [main](https://app.codecov.io/gh/zitadel/oidc/commit/607a76c15408c667cfd5718284db02d84c93a607?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zitadel) (607a76c) will **increase** coverage by `0.11%`.
> The diff coverage is `90.00%`.

```diff
@@            Coverage Diff             @@
##             main     #443      +/-   ##
==========================================
+ Coverage   53.84%   53.96%   +0.11%     
==========================================
  Files          74       74              
  Lines        5811     5830      +19     
==========================================
+ Hits         3129     3146      +17     
- Misses       2422     2424       +2     
  Partials      260      260              
```


| [Files Changed](https://app.codecov.io/gh/zitadel/oidc/pull/443?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zitadel) | Coverage Δ | |
|---|---|---|
| [pkg/op/op.go](https://app.codecov.io/gh/zitadel/oidc/pull/443?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zitadel#diff-cGtnL29wL29wLmdv) | `64.14% <0.00%> (-0.52%)` | :arrow_down: |
| [pkg/op/config.go](https://app.codecov.io/gh/zitadel/oidc/pull/443?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zitadel#diff-cGtnL29wL2NvbmZpZy5nbw==) | `95.83% <100.00%> (+1.28%)` | :arrow_up: |



Patch:
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/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,


Test patch:
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=<identifier>;for=<identifier>;host=<host>;proto=<http|https>
+		{
+			"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