Merge pull request #997 from FStelzer/issue844

Use the raw url path when proxying upstream requests
This commit is contained in:
Joel Speed 2021-09-17 13:45:39 +01:00 committed by GitHub
commit ee7c405bd8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 413 additions and 312 deletions

View File

@ -35,6 +35,7 @@
- [#1317](https://github.com/oauth2-proxy/oauth2-proxy/pull/1317) Fix incorrect `</form>` tag on the sing_in page when *not* using a custom template (@jord1e) - [#1317](https://github.com/oauth2-proxy/oauth2-proxy/pull/1317) Fix incorrect `</form>` tag on the sing_in page when *not* using a custom template (@jord1e)
- [#1330](https://github.com/oauth2-proxy/oauth2-proxy/pull/1330) Allow specifying URL as input for custom sign in logo (@MaikuMori) - [#1330](https://github.com/oauth2-proxy/oauth2-proxy/pull/1330) Allow specifying URL as input for custom sign in logo (@MaikuMori)
- [#1357](https://github.com/oauth2-proxy/oauth2-proxy/pull/1357) Fix unsafe access to session variable (@harzallah) - [#1357](https://github.com/oauth2-proxy/oauth2-proxy/pull/1357) Fix unsafe access to session variable (@harzallah)
- [#997](https://github.com/oauth2-proxy/oauth2-proxy/pull/997) Allow passing the raw url path when proxying upstream requests - e.g. /%2F/ (@FStelzer)
# V7.1.3 # V7.1.3

View File

@ -124,7 +124,7 @@ They may change between releases without notice.
| Field | Type | Description | | Field | Type | Description |
| ----- | ---- | ----------- | | ----- | ---- | ----------- |
| `upstreams` | _[Upstreams](#upstreams)_ | Upstreams is used to configure upstream servers.<br/>Once a user is authenticated, requests to the server will be proxied to<br/>these upstream servers based on the path mappings defined in this list. | | `upstreamConfig` | _[UpstreamConfig](#upstreamconfig)_ | UpstreamConfig is used to configure upstream servers.<br/>Once a user is authenticated, requests to the server will be proxied to<br/>these upstream servers based on the path mappings defined in this list. |
| `injectRequestHeaders` | _[[]Header](#header)_ | InjectRequestHeaders is used to configure headers that should be added<br/>to requests to upstream servers.<br/>Headers may source values from either the authenticated user's session<br/>or from a static secret value. | | `injectRequestHeaders` | _[[]Header](#header)_ | InjectRequestHeaders is used to configure headers that should be added<br/>to requests to upstream servers.<br/>Headers may source values from either the authenticated user's session<br/>or from a static secret value. |
| `injectResponseHeaders` | _[[]Header](#header)_ | InjectResponseHeaders is used to configure headers that should be added<br/>to responses from the proxy.<br/>This is typically used when using the proxy as an external authentication<br/>provider in conjunction with another proxy such as NGINX and its<br/>auth_request module.<br/>Headers may source values from either the authenticated user's session<br/>or from a static secret value. | | `injectResponseHeaders` | _[[]Header](#header)_ | InjectResponseHeaders is used to configure headers that should be added<br/>to responses from the proxy.<br/>This is typically used when using the proxy as an external authentication<br/>provider in conjunction with another proxy such as NGINX and its<br/>auth_request module.<br/>Headers may source values from either the authenticated user's session<br/>or from a static secret value. |
| `server` | _[Server](#server)_ | Server is used to configure the HTTP(S) server for the proxy application.<br/>You may choose to run both HTTP and HTTPS servers simultaneously.<br/>This can be done by setting the BindAddress and the SecureBindAddress simultaneously.<br/>To use the secure server you must configure a TLS certificate and key. | | `server` | _[Server](#server)_ | Server is used to configure the HTTP(S) server for the proxy application.<br/>You may choose to run both HTTP and HTTPS servers simultaneously.<br/>This can be done by setting the BindAddress and the SecureBindAddress simultaneously.<br/>To use the secure server you must configure a TLS certificate and key. |
@ -364,7 +364,7 @@ TLS contains the information for loading a TLS certifcate and key.
### Upstream ### Upstream
(**Appears on:** [Upstreams](#upstreams)) (**Appears on:** [UpstreamConfig](#upstreamconfig))
Upstream represents the configuration for an upstream server. Upstream represents the configuration for an upstream server.
Requests will be proxied to this upstream if the path matches the request path. Requests will be proxied to this upstream if the path matches the request path.
@ -382,11 +382,13 @@ Requests will be proxied to this upstream if the path matches the request path.
| `passHostHeader` | _bool_ | PassHostHeader determines whether the request host header should be proxied<br/>to the upstream server.<br/>Defaults to true. | | `passHostHeader` | _bool_ | PassHostHeader determines whether the request host header should be proxied<br/>to the upstream server.<br/>Defaults to true. |
| `proxyWebSockets` | _bool_ | ProxyWebSockets enables proxying of websockets to upstream servers<br/>Defaults to true. | | `proxyWebSockets` | _bool_ | ProxyWebSockets enables proxying of websockets to upstream servers<br/>Defaults to true. |
### Upstreams ### UpstreamConfig
#### ([[]Upstream](#upstream) alias)
(**Appears on:** [AlphaOptions](#alphaoptions)) (**Appears on:** [AlphaOptions](#alphaoptions))
Upstreams is a collection of definitions for upstream servers. UpstreamConfig is a collection of definitions for upstream servers.
| Field | Type | Description |
| ----- | ---- | ----------- |
| `proxyRawPath` | _bool_ | ProxyRawPath will pass the raw url path to upstream allowing for url's<br/>like: "/%2F/" which would otherwise be redirected to "/" |
| `upstreams` | _[[]Upstream](#upstream)_ | Upstreams represents the configuration for the upstream servers.<br/>Requests will be proxied to this upstream if the path matches the request path. |

View File

@ -2,8 +2,10 @@ package main
import ( import (
"errors" "errors"
"fmt"
"io/ioutil" "io/ioutil"
"os" "os"
"strings"
"time" "time"
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
@ -24,6 +26,8 @@ client_secret="b2F1dGgyLXByb3h5LWNsaWVudC1zZWNyZXQK"
` `
const testAlphaConfig = ` const testAlphaConfig = `
upstreamConfig:
proxyrawpath: false
upstreams: upstreams:
- id: / - id: /
path: / path: /
@ -100,7 +104,8 @@ redirect_url="http://localhost:4180/oauth2/callback"
opts.Cookie.Secure = false opts.Cookie.Secure = false
opts.RawRedirectURL = "http://localhost:4180/oauth2/callback" opts.RawRedirectURL = "http://localhost:4180/oauth2/callback"
opts.UpstreamServers = options.Upstreams{ opts.UpstreamServers = options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: "/", ID: "/",
Path: "/", Path: "/",
@ -109,6 +114,7 @@ redirect_url="http://localhost:4180/oauth2/callback"
PassHostHeader: boolPtr(true), PassHostHeader: boolPtr(true),
ProxyWebSockets: boolPtr(true), ProxyWebSockets: boolPtr(true),
}, },
},
} }
authHeader := options.Header{ authHeader := options.Header{
@ -130,7 +136,7 @@ redirect_url="http://localhost:4180/oauth2/callback"
opts.InjectResponseHeaders = append(opts.InjectResponseHeaders, authHeader) opts.InjectResponseHeaders = append(opts.InjectResponseHeaders, authHeader)
opts.Providers = options.Providers{ opts.Providers = options.Providers{
{ options.Provider{
ID: "google=oauth2-proxy", ID: "google=oauth2-proxy",
Type: "google", Type: "google",
ClientSecret: "b2F1dGgyLXByb3h5LWNsaWVudC1zZWNyZXQK", ClientSecret: "b2F1dGgyLXByb3h5LWNsaWVudC1zZWNyZXQK",
@ -230,7 +236,7 @@ redirect_url="http://localhost:4180/oauth2/callback"
configContent: testCoreConfig, configContent: testCoreConfig,
alphaConfigContent: testAlphaConfig + ":", alphaConfigContent: testAlphaConfig + ":",
expectedOptions: func() *options.Options { return nil }, expectedOptions: func() *options.Options { return nil },
expectedErr: errors.New("failed to load alpha options: error unmarshalling config: error converting YAML to JSON: yaml: line 49: did not find expected key"), expectedErr: fmt.Errorf("failed to load alpha options: error unmarshalling config: error converting YAML to JSON: yaml: line %d: did not find expected key", strings.Count(testAlphaConfig, "\n")),
}), }),
Entry("with alpha configuration and bad core configuration", loadConfigurationTableInput{ Entry("with alpha configuration and bad core configuration", loadConfigurationTableInput{
configContent: testCoreConfig + "unknown_field=\"something\"", configContent: testCoreConfig + "unknown_field=\"something\"",

View File

@ -265,7 +265,9 @@ func (p *OAuthProxy) setupServer(opts *options.Options) error {
} }
func (p *OAuthProxy) buildServeMux(proxyPrefix string) { func (p *OAuthProxy) buildServeMux(proxyPrefix string) {
r := mux.NewRouter() // Use the encoded path here so we can have the option to pass it on in the upstream mux.
// Otherwise something like /%2F/ would be redirected to / here already.
r := mux.NewRouter().UseEncodedPath()
// Everything served by the router must go through the preAuthChain first. // Everything served by the router must go through the preAuthChain first.
r.Use(p.preAuthChain.Then) r.Use(p.preAuthChain.Then)

View File

@ -197,12 +197,14 @@ func TestBasicAuthPassword(t *testing.T) {
basicAuthPassword := "This is a secure password" basicAuthPassword := "This is a secure password"
opts := baseTestOptions() opts := baseTestOptions()
opts.UpstreamServers = options.Upstreams{ opts.UpstreamServers = options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: providerServer.URL, ID: providerServer.URL,
Path: "/", Path: "/",
URI: providerServer.URL, URI: providerServer.URL,
}, },
},
} }
opts.Cookie.Secure = false opts.Cookie.Secure = false
@ -346,15 +348,17 @@ func NewPassAccessTokenTest(opts PassAccessTokenTestOptions) (*PassAccessTokenTe
})) }))
patt.opts = baseTestOptions() patt.opts = baseTestOptions()
patt.opts.UpstreamServers = options.Upstreams{ patt.opts.UpstreamServers = options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: patt.providerServer.URL, ID: patt.providerServer.URL,
Path: "/", Path: "/",
URI: patt.providerServer.URL, URI: patt.providerServer.URL,
}, },
},
} }
if opts.ProxyUpstream.ID != "" { if opts.ProxyUpstream.ID != "" {
patt.opts.UpstreamServers = append(patt.opts.UpstreamServers, opts.ProxyUpstream) patt.opts.UpstreamServers.Upstreams = append(patt.opts.UpstreamServers.Upstreams, opts.ProxyUpstream)
} }
patt.opts.Cookie.Secure = false patt.opts.Cookie.Secure = false
@ -915,6 +919,15 @@ func TestUserInfoEndpointUnauthorizedOnNoCookieSetError(t *testing.T) {
assert.Equal(t, http.StatusUnauthorized, test.rw.Code) assert.Equal(t, http.StatusUnauthorized, test.rw.Code)
} }
func TestEncodedUrlsStayEncoded(t *testing.T) {
encodeTest, err := NewSignInPageTest(false)
if err != nil {
t.Fatal(err)
}
code, _ := encodeTest.GetEndpoint("/%2F/test1/%2F/test2")
assert.Equal(t, 403, code)
}
func NewAuthOnlyEndpointTest(querystring string, modifiers ...OptionsModifier) (*ProcessCookieTest, error) { func NewAuthOnlyEndpointTest(querystring string, modifiers ...OptionsModifier) (*ProcessCookieTest, error) {
pcTest, err := NewProcessCookieTestWithOptionsModifiers(modifiers...) pcTest, err := NewProcessCookieTestWithOptionsModifiers(modifiers...)
if err != nil { if err != nil {
@ -1260,12 +1273,14 @@ func TestAuthSkippedForPreflightRequests(t *testing.T) {
t.Cleanup(upstreamServer.Close) t.Cleanup(upstreamServer.Close)
opts := baseTestOptions() opts := baseTestOptions()
opts.UpstreamServers = options.Upstreams{ opts.UpstreamServers = options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: upstreamServer.URL, ID: upstreamServer.URL,
Path: "/", Path: "/",
URI: upstreamServer.URL, URI: upstreamServer.URL,
}, },
},
} }
opts.SkipAuthPreflight = true opts.SkipAuthPreflight = true
err := validation.Validate(opts) err := validation.Validate(opts)
@ -1335,12 +1350,14 @@ func NewSignatureTest() (*SignatureTest, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} }
opts.UpstreamServers = options.Upstreams{ opts.UpstreamServers = options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: upstreamServer.URL, ID: upstreamServer.URL,
Path: "/", Path: "/",
URI: upstreamServer.URL, URI: upstreamServer.URL,
}, },
},
} }
providerHandler := func(w http.ResponseWriter, r *http.Request) { providerHandler := func(w http.ResponseWriter, r *http.Request) {
@ -1771,12 +1788,14 @@ func Test_noCacheHeaders(t *testing.T) {
t.Cleanup(upstreamServer.Close) t.Cleanup(upstreamServer.Close)
opts := baseTestOptions() opts := baseTestOptions()
opts.UpstreamServers = options.Upstreams{ opts.UpstreamServers = options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: upstreamServer.URL, ID: upstreamServer.URL,
Path: "/", Path: "/",
URI: upstreamServer.URL, URI: upstreamServer.URL,
}, },
},
} }
opts.SkipAuthRegex = []string{".*"} opts.SkipAuthRegex = []string{".*"}
err := validation.Validate(opts) err := validation.Validate(opts)
@ -2041,12 +2060,14 @@ func TestTrustedIPs(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
opts := baseTestOptions() opts := baseTestOptions()
opts.UpstreamServers = options.Upstreams{ opts.UpstreamServers = options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: "static", ID: "static",
Path: "/", Path: "/",
Static: true, Static: true,
}, },
},
} }
opts.TrustedIPs = tt.trustedIPs opts.TrustedIPs = tt.trustedIPs
opts.ReverseProxy = tt.reverseProxy opts.ReverseProxy = tt.reverseProxy
@ -2234,12 +2255,14 @@ func TestAllowedRequest(t *testing.T) {
t.Cleanup(upstreamServer.Close) t.Cleanup(upstreamServer.Close)
opts := baseTestOptions() opts := baseTestOptions()
opts.UpstreamServers = options.Upstreams{ opts.UpstreamServers = options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: upstreamServer.URL, ID: upstreamServer.URL,
Path: "/", Path: "/",
URI: upstreamServer.URL, URI: upstreamServer.URL,
}, },
},
} }
opts.SkipAuthRegex = []string{ opts.SkipAuthRegex = []string{
"^/skip/auth/regex$", "^/skip/auth/regex$",
@ -2349,12 +2372,14 @@ func TestProxyAllowedGroups(t *testing.T) {
test, err := NewProcessCookieTestWithOptionsModifiers(func(opts *options.Options) { test, err := NewProcessCookieTestWithOptionsModifiers(func(opts *options.Options) {
opts.Providers[0].AllowedGroups = tt.allowedGroups opts.Providers[0].AllowedGroups = tt.allowedGroups
opts.UpstreamServers = options.Upstreams{ opts.UpstreamServers = options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: upstreamServer.URL, ID: upstreamServer.URL,
Path: "/", Path: "/",
URI: upstreamServer.URL, URI: upstreamServer.URL,
}, },
},
} }
}) })
if err != nil { if err != nil {

View File

@ -9,10 +9,10 @@ package options
// They may change between releases without notice. // They may change between releases without notice.
// ::: // :::
type AlphaOptions struct { type AlphaOptions struct {
// Upstreams is used to configure upstream servers. // UpstreamConfig is used to configure upstream servers.
// Once a user is authenticated, requests to the server will be proxied to // Once a user is authenticated, requests to the server will be proxied to
// these upstream servers based on the path mappings defined in this list. // these upstream servers based on the path mappings defined in this list.
Upstreams Upstreams `json:"upstreams,omitempty"` UpstreamConfig UpstreamConfig `json:"upstreamConfig,omitempty"`
// InjectRequestHeaders is used to configure headers that should be added // InjectRequestHeaders is used to configure headers that should be added
// to requests to upstream servers. // to requests to upstream servers.
@ -48,19 +48,18 @@ type AlphaOptions struct {
// MergeInto replaces alpha options in the Options struct with the values // MergeInto replaces alpha options in the Options struct with the values
// from the AlphaOptions // from the AlphaOptions
func (a *AlphaOptions) MergeInto(opts *Options) { func (a *AlphaOptions) MergeInto(opts *Options) {
opts.UpstreamServers = a.Upstreams opts.UpstreamServers = a.UpstreamConfig
opts.InjectRequestHeaders = a.InjectRequestHeaders opts.InjectRequestHeaders = a.InjectRequestHeaders
opts.InjectResponseHeaders = a.InjectResponseHeaders opts.InjectResponseHeaders = a.InjectResponseHeaders
opts.Server = a.Server opts.Server = a.Server
opts.MetricsServer = a.MetricsServer opts.MetricsServer = a.MetricsServer
opts.Providers = a.Providers opts.Providers = a.Providers
} }
// ExtractFrom populates the fields in the AlphaOptions with the values from // ExtractFrom populates the fields in the AlphaOptions with the values from
// the Options // the Options
func (a *AlphaOptions) ExtractFrom(opts *Options) { func (a *AlphaOptions) ExtractFrom(opts *Options) {
a.Upstreams = opts.UpstreamServers a.UpstreamConfig = opts.UpstreamServers
a.InjectRequestHeaders = opts.InjectRequestHeaders a.InjectRequestHeaders = opts.InjectRequestHeaders
a.InjectResponseHeaders = opts.InjectResponseHeaders a.InjectResponseHeaders = opts.InjectResponseHeaders
a.Server = opts.Server a.Server = opts.Server

View File

@ -114,13 +114,13 @@ func legacyUpstreamsFlagSet() *pflag.FlagSet {
return flagSet return flagSet
} }
func (l *LegacyUpstreams) convert() (Upstreams, error) { func (l *LegacyUpstreams) convert() (UpstreamConfig, error) {
upstreams := Upstreams{} upstreams := UpstreamConfig{}
for _, upstreamString := range l.Upstreams { for _, upstreamString := range l.Upstreams {
u, err := url.Parse(upstreamString) u, err := url.Parse(upstreamString)
if err != nil { if err != nil {
return nil, fmt.Errorf("could not parse upstream %q: %v", upstreamString, err) return UpstreamConfig{}, fmt.Errorf("could not parse upstream %q: %v", upstreamString, err)
} }
if u.Path == "" { if u.Path == "" {
@ -169,7 +169,7 @@ func (l *LegacyUpstreams) convert() (Upstreams, error) {
upstream.FlushInterval = nil upstream.FlushInterval = nil
} }
upstreams = append(upstreams, upstream) upstreams.Upstreams = append(upstreams.Upstreams, upstream)
} }
return upstreams, nil return upstreams, nil

View File

@ -26,7 +26,8 @@ var _ = Describe("Legacy Options", func() {
truth := true truth := true
staticCode := 204 staticCode := 204
opts.UpstreamServers = Upstreams{ opts.UpstreamServers = UpstreamConfig{
Upstreams: []Upstream{
{ {
ID: "/baz", ID: "/baz",
Path: "/baz", Path: "/baz",
@ -56,6 +57,7 @@ var _ = Describe("Legacy Options", func() {
PassHostHeader: nil, PassHostHeader: nil,
ProxyWebSockets: nil, ProxyWebSockets: nil,
}, },
},
} }
opts.InjectRequestHeaders = []Header{ opts.InjectRequestHeaders = []Header{
@ -124,7 +126,7 @@ var _ = Describe("Legacy Options", func() {
Context("Legacy Upstreams", func() { Context("Legacy Upstreams", func() {
type convertUpstreamsTableInput struct { type convertUpstreamsTableInput struct {
upstreamStrings []string upstreamStrings []string
expectedUpstreams Upstreams expectedUpstreams []Upstream
errMsg string errMsg string
} }
@ -219,51 +221,51 @@ var _ = Describe("Legacy Options", func() {
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())
} }
Expect(upstreams).To(ConsistOf(in.expectedUpstreams)) Expect(upstreams.Upstreams).To(ConsistOf(in.expectedUpstreams))
}, },
Entry("with no upstreams", &convertUpstreamsTableInput{ Entry("with no upstreams", &convertUpstreamsTableInput{
upstreamStrings: []string{}, upstreamStrings: []string{},
expectedUpstreams: Upstreams{}, expectedUpstreams: []Upstream{},
errMsg: "", errMsg: "",
}), }),
Entry("with a valid HTTP upstream", &convertUpstreamsTableInput{ Entry("with a valid HTTP upstream", &convertUpstreamsTableInput{
upstreamStrings: []string{validHTTP}, upstreamStrings: []string{validHTTP},
expectedUpstreams: Upstreams{validHTTPUpstream}, expectedUpstreams: []Upstream{validHTTPUpstream},
errMsg: "", errMsg: "",
}), }),
Entry("with a HTTP upstream with an empty path", &convertUpstreamsTableInput{ Entry("with a HTTP upstream with an empty path", &convertUpstreamsTableInput{
upstreamStrings: []string{emptyPathHTTP}, upstreamStrings: []string{emptyPathHTTP},
expectedUpstreams: Upstreams{emptyPathHTTPUpstream}, expectedUpstreams: []Upstream{emptyPathHTTPUpstream},
errMsg: "", errMsg: "",
}), }),
Entry("with a valid File upstream with a fragment", &convertUpstreamsTableInput{ Entry("with a valid File upstream with a fragment", &convertUpstreamsTableInput{
upstreamStrings: []string{validFileWithFragment}, upstreamStrings: []string{validFileWithFragment},
expectedUpstreams: Upstreams{validFileWithFragmentUpstream}, expectedUpstreams: []Upstream{validFileWithFragmentUpstream},
errMsg: "", errMsg: "",
}), }),
Entry("with a valid static upstream", &convertUpstreamsTableInput{ Entry("with a valid static upstream", &convertUpstreamsTableInput{
upstreamStrings: []string{validStatic}, upstreamStrings: []string{validStatic},
expectedUpstreams: Upstreams{validStaticUpstream}, expectedUpstreams: []Upstream{validStaticUpstream},
errMsg: "", errMsg: "",
}), }),
Entry("with an invalid static upstream, code is 200", &convertUpstreamsTableInput{ Entry("with an invalid static upstream, code is 200", &convertUpstreamsTableInput{
upstreamStrings: []string{invalidStatic}, upstreamStrings: []string{invalidStatic},
expectedUpstreams: Upstreams{invalidStaticUpstream}, expectedUpstreams: []Upstream{invalidStaticUpstream},
errMsg: "", errMsg: "",
}), }),
Entry("with an invalid HTTP upstream", &convertUpstreamsTableInput{ Entry("with an invalid HTTP upstream", &convertUpstreamsTableInput{
upstreamStrings: []string{invalidHTTP}, upstreamStrings: []string{invalidHTTP},
expectedUpstreams: Upstreams{}, expectedUpstreams: []Upstream{},
errMsg: invalidHTTPErrMsg, errMsg: invalidHTTPErrMsg,
}), }),
Entry("with an invalid HTTP upstream and other upstreams", &convertUpstreamsTableInput{ Entry("with an invalid HTTP upstream and other upstreams", &convertUpstreamsTableInput{
upstreamStrings: []string{validHTTP, invalidHTTP}, upstreamStrings: []string{validHTTP, invalidHTTP},
expectedUpstreams: Upstreams{}, expectedUpstreams: []Upstream{},
errMsg: invalidHTTPErrMsg, errMsg: invalidHTTPErrMsg,
}), }),
Entry("with multiple valid upstreams", &convertUpstreamsTableInput{ Entry("with multiple valid upstreams", &convertUpstreamsTableInput{
upstreamStrings: []string{validHTTP, validFileWithFragment, validStatic}, upstreamStrings: []string{validHTTP, validFileWithFragment, validStatic},
expectedUpstreams: Upstreams{validHTTPUpstream, validFileWithFragmentUpstream, validStaticUpstream}, expectedUpstreams: []Upstream{validHTTPUpstream, validFileWithFragmentUpstream, validStaticUpstream},
errMsg: "", errMsg: "",
}), }),
) )

View File

@ -469,6 +469,7 @@ sub:
It("should load a full example AlphaOptions", func() { It("should load a full example AlphaOptions", func() {
config := []byte(` config := []byte(`
upstreamConfig:
upstreams: upstreams:
- id: httpbin - id: httpbin
path: / path: /
@ -502,6 +503,7 @@ injectResponseHeaders:
flushInterval := Duration(500 * time.Millisecond) flushInterval := Duration(500 * time.Millisecond)
Expect(into).To(Equal(&AlphaOptions{ Expect(into).To(Equal(&AlphaOptions{
UpstreamConfig: UpstreamConfig{
Upstreams: []Upstream{ Upstreams: []Upstream{
{ {
ID: "httpbin", ID: "httpbin",
@ -510,6 +512,7 @@ injectResponseHeaders:
FlushInterval: &flushInterval, FlushInterval: &flushInterval,
}, },
}, },
},
InjectRequestHeaders: []Header{ InjectRequestHeaders: []Header{
{ {
Name: "X-Forwarded-User", Name: "X-Forwarded-User",

View File

@ -41,7 +41,7 @@ type Options struct {
// Not used in the legacy config, name not allowed to match an external key (upstreams) // Not used in the legacy config, name not allowed to match an external key (upstreams)
// TODO(JoelSpeed): Rename when legacy config is removed // TODO(JoelSpeed): Rename when legacy config is removed
UpstreamServers Upstreams `cfg:",internal"` UpstreamServers UpstreamConfig `cfg:",internal"`
InjectRequestHeaders []Header `cfg:",internal"` InjectRequestHeaders []Header `cfg:",internal"`
InjectResponseHeaders []Header `cfg:",internal"` InjectResponseHeaders []Header `cfg:",internal"`

View File

@ -7,8 +7,16 @@ const (
DefaultUpstreamFlushInterval = 1 * time.Second DefaultUpstreamFlushInterval = 1 * time.Second
) )
// Upstreams is a collection of definitions for upstream servers. // UpstreamConfig is a collection of definitions for upstream servers.
type Upstreams []Upstream type UpstreamConfig struct {
// ProxyRawPath will pass the raw url path to upstream allowing for url's
// like: "/%2F/" which would otherwise be redirected to "/"
ProxyRawPath bool `json:"proxyRawPath,omitempty"`
// Upstreams represents the configuration for the upstream servers.
// Requests will be proxied to this upstream if the path matches the request path.
Upstreams []Upstream `json:"upstreams,omitempty"`
}
// Upstream represents the configuration for an upstream server. // Upstream represents the configuration for an upstream server.
// Requests will be proxied to this upstream if the path matches the request path. // Requests will be proxied to this upstream if the path matches the request path.

View File

@ -22,12 +22,16 @@ type ProxyErrorHandler func(http.ResponseWriter, *http.Request, error)
// NewProxy creates a new multiUpstreamProxy that can serve requests directed to // NewProxy creates a new multiUpstreamProxy that can serve requests directed to
// multiple upstreams. // multiple upstreams.
func NewProxy(upstreams options.Upstreams, sigData *options.SignatureData, writer pagewriter.Writer) (http.Handler, error) { func NewProxy(upstreams options.UpstreamConfig, sigData *options.SignatureData, writer pagewriter.Writer) (http.Handler, error) {
m := &multiUpstreamProxy{ m := &multiUpstreamProxy{
serveMux: mux.NewRouter(), serveMux: mux.NewRouter(),
} }
for _, upstream := range sortByPathLongest(upstreams) { if upstreams.ProxyRawPath {
m.serveMux.UseEncodedPath()
}
for _, upstream := range sortByPathLongest(upstreams.Upstreams) {
if upstream.Static { if upstream.Static {
if err := m.registerStaticResponseHandler(upstream, writer); err != nil { if err := m.registerStaticResponseHandler(upstream, writer); err != nil {
return nil, fmt.Errorf("could not register static upstream %q: %v", upstream.ID, err) return nil, fmt.Errorf("could not register static upstream %q: %v", upstream.ID, err)
@ -153,7 +157,7 @@ func registerTrailingSlashHandler(serveMux *mux.Router) {
// precedence (note this is the input to the rewrite logic). // precedence (note this is the input to the rewrite logic).
// This does not account for when a rewrite would actually make the path shorter. // This does not account for when a rewrite would actually make the path shorter.
// This should maintain the sorting behaviour of the standard go serve mux. // This should maintain the sorting behaviour of the standard go serve mux.
func sortByPathLongest(in options.Upstreams) options.Upstreams { func sortByPathLongest(in []options.Upstream) []options.Upstream {
sort.Slice(in, func(i, j int) bool { sort.Slice(in, func(i, j int) bool {
iRW := in[i].RewriteTarget iRW := in[i].RewriteTarget
jRW := in[j].RewriteTarget jRW := in[j].RewriteTarget

View File

@ -16,10 +16,16 @@ import (
) )
var _ = Describe("Proxy Suite", func() { var _ = Describe("Proxy Suite", func() {
var upstreamServer http.Handler type proxyTableInput struct {
target string
response testHTTPResponse
upstream string
upstreams options.UpstreamConfig
}
Context("multiUpstreamProxy", func() { Context("multiUpstreamProxy", func() {
BeforeEach(func() { DescribeTable("Proxy ServeHTTP",
func(in *proxyTableInput) {
sigData := &options.SignatureData{Hash: crypto.SHA256, Key: "secret"} sigData := &options.SignatureData{Hash: crypto.SHA256, Key: "secret"}
writer := &pagewriter.WriterFuncs{ writer := &pagewriter.WriterFuncs{
@ -32,7 +38,10 @@ var _ = Describe("Proxy Suite", func() {
ok := http.StatusOK ok := http.StatusOK
accepted := http.StatusAccepted accepted := http.StatusAccepted
upstreams := options.Upstreams{ // Allows for specifying settings and even individual upstreams for specific tests and uses the default upstreams/configs otherwise
upstreams := in.upstreams
if len(in.upstreams.Upstreams) == 0 {
upstreams.Upstreams = []options.Upstream{
{ {
ID: "http-backend", ID: "http-backend",
Path: "/http/", Path: "/http/",
@ -90,20 +99,11 @@ var _ = Describe("Proxy Suite", func() {
URI: serverAddr, URI: serverAddr,
}, },
} }
var err error
upstreamServer, err = NewProxy(upstreams, sigData, writer)
Expect(err).ToNot(HaveOccurred())
})
type proxyTableInput struct {
target string
response testHTTPResponse
upstream string
} }
DescribeTable("Proxy ServeHTTP", upstreamServer, err := NewProxy(upstreams, sigData, writer)
func(in *proxyTableInput) { Expect(err).ToNot(HaveOccurred())
req := middlewareapi.AddRequestScope( req := middlewareapi.AddRequestScope(
httptest.NewRequest("", in.target, nil), httptest.NewRequest("", in.target, nil),
&middlewareapi.RequestScope{}, &middlewareapi.RequestScope{},
@ -131,10 +131,12 @@ var _ = Describe("Proxy Suite", func() {
} }
// Compare the reflected request to the upstream // Compare the reflected request to the upstream
if body != nil {
request := testHTTPRequest{} request := testHTTPRequest{}
Expect(json.Unmarshal(body, &request)).To(Succeed()) Expect(json.Unmarshal(body, &request)).To(Succeed())
testSanitizeRequestHeader(request.Header) testSanitizeRequestHeader(request.Header)
Expect(request).To(Equal(in.response.request)) Expect(request).To(Equal(in.response.request))
}
}, },
Entry("with a request to the HTTP service", &proxyTableInput{ Entry("with a request to the HTTP service", &proxyTableInput{
target: "http://example.localhost/http/1234", target: "http://example.localhost/http/1234",
@ -310,33 +312,58 @@ var _ = Describe("Proxy Suite", func() {
}, },
upstream: "double-match-rewrite", upstream: "double-match-rewrite",
}), }),
Entry("containing an escaped '/' without ProxyRawPath", &proxyTableInput{
target: "http://example.localhost/%2F/test1/%2F/test2",
response: testHTTPResponse{
code: 301,
header: map[string][]string{
"Location": {
"http://example.localhost/test1/test2",
},
},
},
upstream: "",
}),
Entry("containing an escaped '/' with ProxyRawPath", &proxyTableInput{
upstreams: options.UpstreamConfig{ProxyRawPath: true},
target: "http://example.localhost/%2F/test1/%2F/test2",
response: testHTTPResponse{
code: 404,
header: map[string][]string{
"X-Content-Type-Options": {"nosniff"},
contentType: {textPlainUTF8},
},
raw: "404 page not found\n",
},
upstream: "",
}),
) )
}) })
Context("sortByPathLongest", func() { Context("sortByPathLongest", func() {
type sortByPathLongestTableInput struct { type sortByPathLongestTableInput struct {
input options.Upstreams input []options.Upstream
expectedOutput options.Upstreams expectedOutput []options.Upstream
} }
var httpPath = options.Upstream{ httpPath := options.Upstream{
Path: "/http/", Path: "/http/",
} }
var httpSubPath = options.Upstream{ httpSubPath := options.Upstream{
Path: "/http/subpath/", Path: "/http/subpath/",
} }
var longerPath = options.Upstream{ longerPath := options.Upstream{
Path: "/longer-than-http", Path: "/longer-than-http",
} }
var shortPathWithRewrite = options.Upstream{ shortPathWithRewrite := options.Upstream{
Path: "^/h/(.*)", Path: "^/h/(.*)",
RewriteTarget: "/$1", RewriteTarget: "/$1",
} }
var shortSubPathWithRewrite = options.Upstream{ shortSubPathWithRewrite := options.Upstream{
Path: "^/h/bar/(.*)", Path: "^/h/bar/(.*)",
RewriteTarget: "/$1", RewriteTarget: "/$1",
} }
@ -346,40 +373,40 @@ var _ = Describe("Proxy Suite", func() {
Expect(sortByPathLongest(in.input)).To(Equal(in.expectedOutput)) Expect(sortByPathLongest(in.input)).To(Equal(in.expectedOutput))
}, },
Entry("with a mix of paths registered", sortByPathLongestTableInput{ Entry("with a mix of paths registered", sortByPathLongestTableInput{
input: options.Upstreams{httpPath, httpSubPath, shortSubPathWithRewrite, longerPath, shortPathWithRewrite}, input: []options.Upstream{httpPath, httpSubPath, shortSubPathWithRewrite, longerPath, shortPathWithRewrite},
expectedOutput: options.Upstreams{shortSubPathWithRewrite, shortPathWithRewrite, longerPath, httpSubPath, httpPath}, expectedOutput: []options.Upstream{shortSubPathWithRewrite, shortPathWithRewrite, longerPath, httpSubPath, httpPath},
}), }),
Entry("when a subpath is registered (in order)", sortByPathLongestTableInput{ Entry("when a subpath is registered (in order)", sortByPathLongestTableInput{
input: options.Upstreams{httpSubPath, httpPath}, input: []options.Upstream{httpSubPath, httpPath},
expectedOutput: options.Upstreams{httpSubPath, httpPath}, expectedOutput: []options.Upstream{httpSubPath, httpPath},
}), }),
Entry("when a subpath is registered (out of order)", sortByPathLongestTableInput{ Entry("when a subpath is registered (out of order)", sortByPathLongestTableInput{
input: options.Upstreams{httpPath, httpSubPath}, input: []options.Upstream{httpPath, httpSubPath},
expectedOutput: options.Upstreams{httpSubPath, httpPath}, expectedOutput: []options.Upstream{httpSubPath, httpPath},
}), }),
Entry("when longer paths are registered (in order)", sortByPathLongestTableInput{ Entry("when longer paths are registered (in order)", sortByPathLongestTableInput{
input: options.Upstreams{longerPath, httpPath}, input: []options.Upstream{longerPath, httpPath},
expectedOutput: options.Upstreams{longerPath, httpPath}, expectedOutput: []options.Upstream{longerPath, httpPath},
}), }),
Entry("when longer paths are registered (out of order)", sortByPathLongestTableInput{ Entry("when longer paths are registered (out of order)", sortByPathLongestTableInput{
input: options.Upstreams{httpPath, longerPath}, input: []options.Upstream{httpPath, longerPath},
expectedOutput: options.Upstreams{longerPath, httpPath}, expectedOutput: []options.Upstream{longerPath, httpPath},
}), }),
Entry("when a rewrite target is registered (in order)", sortByPathLongestTableInput{ Entry("when a rewrite target is registered (in order)", sortByPathLongestTableInput{
input: options.Upstreams{shortPathWithRewrite, longerPath}, input: []options.Upstream{shortPathWithRewrite, longerPath},
expectedOutput: options.Upstreams{shortPathWithRewrite, longerPath}, expectedOutput: []options.Upstream{shortPathWithRewrite, longerPath},
}), }),
Entry("when a rewrite target is registered (out of order)", sortByPathLongestTableInput{ Entry("when a rewrite target is registered (out of order)", sortByPathLongestTableInput{
input: options.Upstreams{longerPath, shortPathWithRewrite}, input: []options.Upstream{longerPath, shortPathWithRewrite},
expectedOutput: options.Upstreams{shortPathWithRewrite, longerPath}, expectedOutput: []options.Upstream{shortPathWithRewrite, longerPath},
}), }),
Entry("with multiple rewrite targets registered (in order)", sortByPathLongestTableInput{ Entry("with multiple rewrite targets registered (in order)", sortByPathLongestTableInput{
input: options.Upstreams{shortSubPathWithRewrite, shortPathWithRewrite}, input: []options.Upstream{shortSubPathWithRewrite, shortPathWithRewrite},
expectedOutput: options.Upstreams{shortSubPathWithRewrite, shortPathWithRewrite}, expectedOutput: []options.Upstream{shortSubPathWithRewrite, shortPathWithRewrite},
}), }),
Entry("with multiple rewrite targets registered (out of order)", sortByPathLongestTableInput{ Entry("with multiple rewrite targets registered (out of order)", sortByPathLongestTableInput{
input: options.Upstreams{shortPathWithRewrite, shortSubPathWithRewrite}, input: []options.Upstream{shortPathWithRewrite, shortSubPathWithRewrite},
expectedOutput: options.Upstreams{shortSubPathWithRewrite, shortPathWithRewrite}, expectedOutput: []options.Upstream{shortSubPathWithRewrite, shortPathWithRewrite},
}), }),
) )
}) })

View File

@ -22,7 +22,7 @@ const (
func testOptions() *options.Options { func testOptions() *options.Options {
o := options.NewOptions() o := options.NewOptions()
o.UpstreamServers = append(o.UpstreamServers, options.Upstream{ o.UpstreamServers.Upstreams = append(o.UpstreamServers.Upstreams, options.Upstream{
ID: "upstream", ID: "upstream",
Path: "/", Path: "/",
URI: "http://127.0.0.1:8080/", URI: "http://127.0.0.1:8080/",

View File

@ -7,12 +7,12 @@ import (
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
) )
func validateUpstreams(upstreams options.Upstreams) []string { func validateUpstreams(upstreams options.UpstreamConfig) []string {
msgs := []string{} msgs := []string{}
ids := make(map[string]struct{}) ids := make(map[string]struct{})
paths := make(map[string]struct{}) paths := make(map[string]struct{})
for _, upstream := range upstreams { for _, upstream := range upstreams.Upstreams {
msgs = append(msgs, validateUpstream(upstream, ids, paths)...) msgs = append(msgs, validateUpstream(upstream, ids, paths)...)
} }

View File

@ -11,7 +11,7 @@ import (
var _ = Describe("Upstreams", func() { var _ = Describe("Upstreams", func() {
type validateUpstreamTableInput struct { type validateUpstreamTableInput struct {
upstreams options.Upstreams upstreams options.UpstreamConfig
errStrings []string errStrings []string
} }
@ -54,79 +54,94 @@ var _ = Describe("Upstreams", func() {
Expect(validateUpstreams(o.upstreams)).To(ConsistOf(o.errStrings)) Expect(validateUpstreams(o.upstreams)).To(ConsistOf(o.errStrings))
}, },
Entry("with no upstreams", &validateUpstreamTableInput{ Entry("with no upstreams", &validateUpstreamTableInput{
upstreams: options.Upstreams{}, upstreams: options.UpstreamConfig{},
errStrings: []string{}, errStrings: []string{},
}), }),
Entry("with valid upstreams", &validateUpstreamTableInput{ Entry("with valid upstreams", &validateUpstreamTableInput{
upstreams: options.Upstreams{ upstreams: options.UpstreamConfig{
Upstreams: []options.Upstream{
validHTTPUpstream, validHTTPUpstream,
validStaticUpstream, validStaticUpstream,
validFileUpstream, validFileUpstream,
}, },
},
errStrings: []string{}, errStrings: []string{},
}), }),
Entry("with an empty ID", &validateUpstreamTableInput{ Entry("with an empty ID", &validateUpstreamTableInput{
upstreams: options.Upstreams{ upstreams: options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: "", ID: "",
Path: "/foo", Path: "/foo",
URI: "http://localhost:8080", URI: "http://localhost:8080",
}, },
}, },
},
errStrings: []string{emptyIDMsg}, errStrings: []string{emptyIDMsg},
}), }),
Entry("with an empty Path", &validateUpstreamTableInput{ Entry("with an empty Path", &validateUpstreamTableInput{
upstreams: options.Upstreams{ upstreams: options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: "foo", ID: "foo",
Path: "", Path: "",
URI: "http://localhost:8080", URI: "http://localhost:8080",
}, },
}, },
},
errStrings: []string{emptyPathMsg}, errStrings: []string{emptyPathMsg},
}), }),
Entry("with an empty Path", &validateUpstreamTableInput{ Entry("with an empty Path", &validateUpstreamTableInput{
upstreams: options.Upstreams{ upstreams: options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: "foo", ID: "foo",
Path: "", Path: "",
URI: "http://localhost:8080", URI: "http://localhost:8080",
}, },
}, },
},
errStrings: []string{emptyPathMsg}, errStrings: []string{emptyPathMsg},
}), }),
Entry("with an empty URI", &validateUpstreamTableInput{ Entry("with an empty URI", &validateUpstreamTableInput{
upstreams: options.Upstreams{ upstreams: options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: "foo", ID: "foo",
Path: "/foo", Path: "/foo",
URI: "", URI: "",
}, },
}, },
},
errStrings: []string{emptyURIMsg}, errStrings: []string{emptyURIMsg},
}), }),
Entry("with an invalid URI", &validateUpstreamTableInput{ Entry("with an invalid URI", &validateUpstreamTableInput{
upstreams: options.Upstreams{ upstreams: options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: "foo", ID: "foo",
Path: "/foo", Path: "/foo",
URI: ":", URI: ":",
}, },
}, },
},
errStrings: []string{invalidURIMsg}, errStrings: []string{invalidURIMsg},
}), }),
Entry("with an invalid URI scheme", &validateUpstreamTableInput{ Entry("with an invalid URI scheme", &validateUpstreamTableInput{
upstreams: options.Upstreams{ upstreams: options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: "foo", ID: "foo",
Path: "/foo", Path: "/foo",
URI: "ftp://foo", URI: "ftp://foo",
}, },
}, },
},
errStrings: []string{invalidURISchemeMsg}, errStrings: []string{invalidURISchemeMsg},
}), }),
Entry("with a static upstream and invalid optons", &validateUpstreamTableInput{ Entry("with a static upstream and invalid optons", &validateUpstreamTableInput{
upstreams: options.Upstreams{ upstreams: options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: "foo", ID: "foo",
Path: "/foo", Path: "/foo",
@ -138,6 +153,7 @@ var _ = Describe("Upstreams", func() {
InsecureSkipTLSVerify: true, InsecureSkipTLSVerify: true,
}, },
}, },
},
errStrings: []string{ errStrings: []string{
staticWithURIMsg, staticWithURIMsg,
staticWithInsecureMsg, staticWithInsecureMsg,
@ -147,7 +163,8 @@ var _ = Describe("Upstreams", func() {
}, },
}), }),
Entry("with duplicate IDs", &validateUpstreamTableInput{ Entry("with duplicate IDs", &validateUpstreamTableInput{
upstreams: options.Upstreams{ upstreams: options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: "foo", ID: "foo",
Path: "/foo1", Path: "/foo1",
@ -159,10 +176,12 @@ var _ = Describe("Upstreams", func() {
URI: "http://foo", URI: "http://foo",
}, },
}, },
},
errStrings: []string{multipleIDsMsg}, errStrings: []string{multipleIDsMsg},
}), }),
Entry("with duplicate Paths", &validateUpstreamTableInput{ Entry("with duplicate Paths", &validateUpstreamTableInput{
upstreams: options.Upstreams{ upstreams: options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: "foo1", ID: "foo1",
Path: "/foo", Path: "/foo",
@ -174,16 +193,19 @@ var _ = Describe("Upstreams", func() {
URI: "http://foo", URI: "http://foo",
}, },
}, },
},
errStrings: []string{multiplePathsMsg}, errStrings: []string{multiplePathsMsg},
}), }),
Entry("when a static code is supplied without static", &validateUpstreamTableInput{ Entry("when a static code is supplied without static", &validateUpstreamTableInput{
upstreams: options.Upstreams{ upstreams: options.UpstreamConfig{
Upstreams: []options.Upstream{
{ {
ID: "foo", ID: "foo",
Path: "/foo", Path: "/foo",
StaticCode: &staticCode200, StaticCode: &staticCode200,
}, },
}, },
},
errStrings: []string{emptyURIMsg, staticCodeMsg}, errStrings: []string{emptyURIMsg, staticCodeMsg},
}), }),
) )