From 601477a52cf7d4eb4b8e4c36b916d6fbeb1a3468 Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Wed, 25 Oct 2023 11:25:01 +0200 Subject: [PATCH] Feature: Allowing relative redirect url though an option (#2183) * Adding relative redirect url option * Updating CHANGELOG.md * tests: adding unit test for getOAuthRedirectURI --------- Co-authored-by: Joel Speed --- CHANGELOG.md | 4 +- docs/docs/configuration/overview.md | 1 + oauthproxy.go | 4 +- oauthproxy_test.go | 87 +++++++++++++++++++++++++++++ pkg/apis/options/options.go | 20 ++++--- 5 files changed, 105 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec311bb7..9c087c2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,10 +7,12 @@ ## Breaking Changes ## Changes since v7.5.1 + - [#2128](https://github.com/oauth2-proxy/oauth2-proxy/pull/2128) Update dependencies (@vllvll) - [#2274](https://github.com/oauth2-proxy/oauth2-proxy/pull/2274) Upgrade golang.org/x/net to v0.17.0 (@pierluigilenoci) - [#2282](https://github.com/oauth2-proxy/oauth2-proxy/pull/2282) Fixed checking Google Groups membership using Google Application Credentials (@kvanzuijlen) - +- [#2183](https://github.com/oauth2-proxy/oauth2-proxy/pull/2183) Allowing relative redirect url though an option +- # V7.5.1 ## Release Highlights diff --git a/docs/docs/configuration/overview.md b/docs/docs/configuration/overview.md index a2a6e75b..a3629492 100644 --- a/docs/docs/configuration/overview.md +++ b/docs/docs/configuration/overview.md @@ -165,6 +165,7 @@ An example [oauth2-proxy.cfg](https://github.com/oauth2-proxy/oauth2-proxy/blob/ | `--real-client-ip-header` | string | Header used to determine the real IP of the client, requires `--reverse-proxy` to be set (one of: X-Forwarded-For, X-Real-IP, or X-ProxyUser-IP) | X-Real-IP | | `--redeem-url` | string | Token redemption endpoint | | | `--redirect-url` | string | the OAuth Redirect URL, e.g. `"https://internalapp.yourcompany.com/oauth2/callback"` | | +| `--relative-redirect-url` | bool | allow relative OAuth Redirect URL.` | | | `--redis-cluster-connection-urls` | string \| list | List of Redis cluster connection URLs (e.g. `redis://HOST[:PORT]`). Used in conjunction with `--redis-use-cluster` | | | `--redis-connection-url` | string | URL of redis server for redis session storage (e.g. `redis://HOST[:PORT]`) | | | `--redis-insecure-skip-tls-verify` | bool | skip TLS verification when connecting to Redis | false | diff --git a/oauthproxy.go b/oauthproxy.go index 10a69b31..c4b0d51b 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -86,6 +86,7 @@ type OAuthProxy struct { allowedRoutes []allowedRoute apiRoutes []apiRoute redirectURL *url.URL // the url to receive requests at + relativeRedirectURL bool whitelistDomains []string provider providers.Provider sessionStore sessionsapi.SessionStore @@ -216,6 +217,7 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr provider: provider, sessionStore: sessionStore, redirectURL: redirectURL, + relativeRedirectURL: opts.RelativeRedirectURL, apiRoutes: apiRoutes, allowedRoutes: allowedRoutes, whitelistDomains: opts.WhitelistDomains, @@ -1018,7 +1020,7 @@ func prepareNoCacheMiddleware(next http.Handler) http.Handler { // This is usually the OAuthProxy callback URL. func (p *OAuthProxy) getOAuthRedirectURI(req *http.Request) string { // if `p.redirectURL` already has a host, return it - if p.redirectURL.Host != "" { + if p.relativeRedirectURL || p.redirectURL.Host != "" { return p.redirectURL.String() } diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 0d8bc91a..85e19f22 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -3257,3 +3257,90 @@ func TestAuthOnlyAllowedEmails(t *testing.T) { }) } } + +func TestGetOAuthRedirectURI(t *testing.T) { + tests := []struct { + name string + setupOpts func(*options.Options) *options.Options + req *http.Request + want string + }{ + { + name: "redirect with https schema", + setupOpts: func(baseOpts *options.Options) *options.Options { + return baseOpts + }, + req: &http.Request{ + Host: "example", + URL: &url.URL{ + Scheme: schemeHTTPS, + }, + }, + want: "https://example/oauth2/callback", + }, + { + name: "redirect with http schema", + setupOpts: func(baseOpts *options.Options) *options.Options { + baseOpts.Cookie.Secure = false + return baseOpts + }, + req: &http.Request{ + Host: "example", + URL: &url.URL{ + Scheme: schemeHTTP, + }, + }, + want: "http://example/oauth2/callback", + }, + { + name: "relative redirect url", + setupOpts: func(baseOpts *options.Options) *options.Options { + baseOpts.RelativeRedirectURL = true + return baseOpts + }, + req: &http.Request{}, + want: "/oauth2/callback", + }, + { + name: "proxy prefix", + setupOpts: func(baseOpts *options.Options) *options.Options { + baseOpts.ProxyPrefix = "/prefix" + return baseOpts + }, + req: &http.Request{ + Host: "example", + URL: &url.URL{ + Scheme: schemeHTTP, + }, + }, + want: "https://example/prefix/callback", + }, + { + name: "proxy prefix with relative redirect", + setupOpts: func(baseOpts *options.Options) *options.Options { + baseOpts.ProxyPrefix = "/prefix" + baseOpts.RelativeRedirectURL = true + return baseOpts + }, + req: &http.Request{ + Host: "example", + URL: &url.URL{ + Scheme: schemeHTTP, + }, + }, + want: "/prefix/callback", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + baseOpts := baseTestOptions() + err := validation.Validate(baseOpts) + assert.NoError(t, err) + + proxy, err := NewOAuthProxy(tt.setupOpts(baseOpts), func(string) bool { return true }) + assert.NoError(t, err) + + assert.Equalf(t, tt.want, proxy.getOAuthRedirectURI(tt.req), "getOAuthRedirectURI(%v)", tt.req) + }) + } +} diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index 0af8df3f..ecf92808 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -18,15 +18,16 @@ type SignatureData struct { // Options holds Configuration Options that can be set by Command Line Flag, // or Config File type Options struct { - ProxyPrefix string `flag:"proxy-prefix" cfg:"proxy_prefix"` - PingPath string `flag:"ping-path" cfg:"ping_path"` - PingUserAgent string `flag:"ping-user-agent" cfg:"ping_user_agent"` - ReadyPath string `flag:"ready-path" cfg:"ready_path"` - ReverseProxy bool `flag:"reverse-proxy" cfg:"reverse_proxy"` - RealClientIPHeader string `flag:"real-client-ip-header" cfg:"real_client_ip_header"` - TrustedIPs []string `flag:"trusted-ip" cfg:"trusted_ips"` - ForceHTTPS bool `flag:"force-https" cfg:"force_https"` - RawRedirectURL string `flag:"redirect-url" cfg:"redirect_url"` + ProxyPrefix string `flag:"proxy-prefix" cfg:"proxy_prefix"` + PingPath string `flag:"ping-path" cfg:"ping_path"` + PingUserAgent string `flag:"ping-user-agent" cfg:"ping_user_agent"` + ReadyPath string `flag:"ready-path" cfg:"ready_path"` + ReverseProxy bool `flag:"reverse-proxy" cfg:"reverse_proxy"` + RealClientIPHeader string `flag:"real-client-ip-header" cfg:"real_client_ip_header"` + TrustedIPs []string `flag:"trusted-ip" cfg:"trusted_ips"` + ForceHTTPS bool `flag:"force-https" cfg:"force_https"` + RawRedirectURL string `flag:"redirect-url" cfg:"redirect_url"` + RelativeRedirectURL bool `flag:"relative-redirect-url" cfg:"relative_redirect_url"` AuthenticatedEmailsFile string `flag:"authenticated-emails-file" cfg:"authenticated_emails_file"` EmailDomains []string `flag:"email-domain" cfg:"email_domains"` @@ -117,6 +118,7 @@ func NewFlagSet() *pflag.FlagSet { flagSet.StringSlice("trusted-ip", []string{}, "list of IPs or CIDR ranges to allow to bypass authentication. WARNING: trusting by IP has inherent security flaws, read the configuration documentation for more information.") flagSet.Bool("force-https", false, "force HTTPS redirect for HTTP requests") flagSet.String("redirect-url", "", "the OAuth Redirect URL. ie: \"https://internalapp.yourcompany.com/oauth2/callback\"") + flagSet.Bool("relative-redirect-url", false, "allow relative OAuth Redirect URL.") flagSet.StringSlice("skip-auth-regex", []string{}, "(DEPRECATED for --skip-auth-route) bypass authentication for requests path's that match (may be given multiple times)") flagSet.StringSlice("skip-auth-route", []string{}, "bypass authentication for requests that match the method & path. Format: method=path_regex OR method!=path_regex. For all methods: path_regex OR !=path_regex") flagSet.StringSlice("api-route", []string{}, "return HTTP 401 instead of redirecting to authentication server if token is not valid. Format: path_regex")