From 509bea7fc20995db01be6ab659233d7927d48fed Mon Sep 17 00:00:00 2001 From: Michael Quigley Date: Wed, 18 Oct 2023 11:47:26 -0400 Subject: [PATCH] improved oauth frontend configuration; better separation of concerns (#411) --- CHANGELOG.md | 4 +++ .../self-hosting/oauth/configuring-oauth.md | 26 ++++++++++--------- endpoints/publicProxy/config.go | 16 +++++------- endpoints/publicProxy/github.go | 19 +++----------- endpoints/publicProxy/google.go | 19 +++----------- endpoints/publicProxy/http.go | 16 +++++------- etc/frontend.yml | 19 ++++++++------ 7 files changed, 50 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98ae4031..217ff301 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# v0.4.10 + +CHANGE: The public frontend configuration has been bumped from `v: 2` to `v: 3`. The `redirect_host`, `redirect_port` and `redirect_http_only` parameters have been removed. These three configuration options have been replaced with `bind_address`, `redirect_url` and `cookie_domain`. See the OAuth configuration guide at `docs/guides/self-hosting/oauth/configuring-oauth.md` for more details (https://github.com/openziti/zrok/issues/411) + # v0.4.9 FIX: Remove extraneous share token prepended to OAuth frontend redirect. diff --git a/docs/guides/self-hosting/oauth/configuring-oauth.md b/docs/guides/self-hosting/oauth/configuring-oauth.md index 6eaa1a11..1c567b85 100644 --- a/docs/guides/self-hosting/oauth/configuring-oauth.md +++ b/docs/guides/self-hosting/oauth/configuring-oauth.md @@ -90,23 +90,25 @@ The public frontend configuration includes a new `oauth` section: ```yaml oauth: - redirect_host: oauth.zrok.io - redirect_port: 28080 - redirect_http_only: false - hash_key: "" + bind_address: 0.0.0.0:8181 + redirect_url: https://oauth.zrok.io + cookie_domain: zrok.io + hash_key: "the quick brown fox jumped over the lazy dog" providers: - - name: google - client_id: - client_secret: - - name: github - client_id: - client_secret: + - name: google + client_id: "" + client_secret: "" + - name: github + client_id: "" + client_secret: "" ``` -The `redirect_host` and `redirect_port` value should correspond with the DNS hostname and port configured as your OAuth frontend. +The `bind_address` parameter determines where the OAuth frontend will bind. Should be in `ip:port` format. -The `redirect_http_only` is useful in development environments where your OAuth frontend is not running behind an HTTPS reverse proxy. Should not be enabled in production environments! +The `redirect_url` parameter determines the base URL where OAuth frontend requests will be redirected. + +`cookie_domain` is the domain where authentication cookies should be stored. `hash_key` is a unique string for your installation that is used to secure the authentication payloads for your public frontend. diff --git a/endpoints/publicProxy/config.go b/endpoints/publicProxy/config.go index 416f8032..67206db6 100644 --- a/endpoints/publicProxy/config.go +++ b/endpoints/publicProxy/config.go @@ -2,15 +2,13 @@ package publicProxy import ( "context" - "fmt" "github.com/michaelquigley/cf" "github.com/pkg/errors" "github.com/sirupsen/logrus" zhttp "github.com/zitadel/oidc/v2/pkg/http" - "strings" ) -const V = 2 +const V = 3 type Config struct { V int @@ -21,11 +19,11 @@ type Config struct { } type OauthConfig struct { - RedirectHost string - RedirectPort int - RedirectHttpOnly bool - HashKey string `cf:"+secret"` - Providers []*OauthProviderConfig + BindAddress string + RedirectUrl string + CookieDomain string + HashKey string `cf:"+secret"` + Providers []*OauthProviderConfig } func (oc *OauthConfig) GetProvider(name string) *OauthProviderConfig { @@ -71,6 +69,6 @@ func configureOauthHandlers(ctx context.Context, cfg *Config, tls bool) error { if err := configureGithubOauth(cfg.Oauth, tls); err != nil { return err } - zhttp.StartServer(ctx, fmt.Sprintf("%s:%d", strings.Split(cfg.Address, ":")[0], cfg.Oauth.RedirectPort)) + zhttp.StartServer(ctx, cfg.Oauth.BindAddress) return nil } diff --git a/endpoints/publicProxy/github.go b/endpoints/publicProxy/github.go index ea97bc02..9e9a0713 100644 --- a/endpoints/publicProxy/github.go +++ b/endpoints/publicProxy/github.go @@ -16,7 +16,6 @@ import ( "io" "net/http" "net/url" - "strings" "time" ) @@ -32,12 +31,10 @@ func configureGithubOauth(cfg *OauthConfig, tls bool) error { return nil } clientID := providerCfg.ClientId - callbackPath := "/github/oauth" - redirectUrl := fmt.Sprintf("%s://%s", scheme, cfg.RedirectHost) rpConfig := &oauth2.Config{ ClientID: clientID, ClientSecret: providerCfg.ClientSecret, - RedirectURL: fmt.Sprintf("%v:%v%v", redirectUrl, cfg.RedirectPort, callbackPath), + RedirectURL: fmt.Sprintf("%v/github/oauth", cfg.RedirectUrl), Scopes: []string{"user:email"}, Endpoint: githubOAuth.Endpoint, } @@ -52,15 +49,7 @@ func configureGithubOauth(cfg *OauthConfig, tls bool) error { } key := hash.Sum(nil) - u, err := url.Parse(redirectUrl) - if err != nil { - logrus.Errorf("unable to parse redirect url: %v", err) - return err - } - parts := strings.Split(u.Hostname(), ".") - domain := parts[len(parts)-2] + "." + parts[len(parts)-1] - - cookieHandler := zhttp.NewCookieHandler(key, key, zhttp.WithUnsecure(), zhttp.WithDomain(domain)) + cookieHandler := zhttp.NewCookieHandler(key, key, zhttp.WithUnsecure(), zhttp.WithDomain(cfg.CookieDomain)) options := []rp.Option{ rp.WithCookieHandler(cookieHandler), @@ -177,10 +166,10 @@ func configureGithubOauth(cfg *OauthConfig, tls bool) error { authCheckInterval = i } - SetZrokCookie(w, domain, primaryEmail, tokens.AccessToken, "github", authCheckInterval, key) + SetZrokCookie(w, cfg.CookieDomain, primaryEmail, tokens.AccessToken, "github", authCheckInterval, key) http.Redirect(w, r, fmt.Sprintf("%s://%s", scheme, token.Claims.(*IntermediateJWT).Host), http.StatusFound) } - http.Handle(callbackPath, rp.CodeExchangeHandler(getEmail, relyingParty)) + http.Handle("/github/oauth", rp.CodeExchangeHandler(getEmail, relyingParty)) return nil } diff --git a/endpoints/publicProxy/google.go b/endpoints/publicProxy/google.go index 23133c44..81e66cf2 100644 --- a/endpoints/publicProxy/google.go +++ b/endpoints/publicProxy/google.go @@ -16,7 +16,6 @@ import ( "io" "net/http" "net/url" - "strings" "time" ) @@ -33,12 +32,10 @@ func configureGoogleOauth(cfg *OauthConfig, tls bool) error { } clientID := providerCfg.ClientId - callbackPath := "/google/oauth" - redirectUrl := fmt.Sprintf("%s://%s", scheme, cfg.RedirectHost) rpConfig := &oauth2.Config{ ClientID: clientID, ClientSecret: providerCfg.ClientSecret, - RedirectURL: fmt.Sprintf("%v:%v%v", redirectUrl, cfg.RedirectPort, callbackPath), + RedirectURL: fmt.Sprintf("%v/google/oauth", cfg.RedirectUrl), Scopes: []string{"https://www.googleapis.com/auth/userinfo.email"}, Endpoint: googleOauth.Endpoint, } @@ -53,15 +50,7 @@ func configureGoogleOauth(cfg *OauthConfig, tls bool) error { } key := hash.Sum(nil) - u, err := url.Parse(redirectUrl) - if err != nil { - logrus.Errorf("unable to parse redirect url: %v", err) - return err - } - parts := strings.Split(u.Hostname(), ".") - domain := parts[len(parts)-2] + "." + parts[len(parts)-1] - - cookieHandler := zhttp.NewCookieHandler(key, key, zhttp.WithUnsecure(), zhttp.WithDomain(domain)) + cookieHandler := zhttp.NewCookieHandler(key, key, zhttp.WithUnsecure(), zhttp.WithDomain(cfg.CookieDomain)) options := []rp.Option{ rp.WithCookieHandler(cookieHandler), @@ -157,10 +146,10 @@ func configureGoogleOauth(cfg *OauthConfig, tls bool) error { authCheckInterval = i } - SetZrokCookie(w, domain, rDat.Email, tokens.AccessToken, "google", authCheckInterval, key) + SetZrokCookie(w, cfg.CookieDomain, rDat.Email, tokens.AccessToken, "google", authCheckInterval, key) http.Redirect(w, r, fmt.Sprintf("%s://%s", scheme, token.Claims.(*IntermediateJWT).Host), http.StatusFound) } - http.Handle(callbackPath, rp.CodeExchangeHandler(getEmail, relyingParty)) + http.Handle("/google/oauth", rp.CodeExchangeHandler(getEmail, relyingParty)) return nil } diff --git a/endpoints/publicProxy/http.go b/endpoints/publicProxy/http.go index b3dcd6b7..8fb365d2 100644 --- a/endpoints/publicProxy/http.go +++ b/endpoints/publicProxy/http.go @@ -228,7 +228,7 @@ func authHandler(handler http.Handler, pcfg *Config, key []byte, ctx ziti.Contex cookie, err := r.Cookie("zrok-access") if err != nil { logrus.Errorf("unable to get 'zrok-access' cookie: %v", err) - oauthLoginRequired(w, r, shrToken, pcfg, provider.(string), target, authCheckInterval) + oauthLoginRequired(w, r, pcfg.Oauth, provider.(string), target, authCheckInterval) return } tkn, err := jwt.ParseWithClaims(cookie.Value, &ZrokClaims{}, func(t *jwt.Token) (interface{}, error) { @@ -239,18 +239,18 @@ func authHandler(handler http.Handler, pcfg *Config, key []byte, ctx ziti.Contex }) if err != nil { logrus.Errorf("unable to parse jwt: %v", err) - oauthLoginRequired(w, r, shrToken, pcfg, provider.(string), target, authCheckInterval) + oauthLoginRequired(w, r, pcfg.Oauth, provider.(string), target, authCheckInterval) return } claims := tkn.Claims.(*ZrokClaims) if claims.Provider != provider { logrus.Error("provider mismatch; restarting auth flow") - oauthLoginRequired(w, r, shrToken, pcfg, provider.(string), target, authCheckInterval) + oauthLoginRequired(w, r, pcfg.Oauth, provider.(string), target, authCheckInterval) return } if claims.AuthorizationCheckInterval != authCheckInterval { logrus.Error("authorization check interval mismatch; restarting auth flow") - oauthLoginRequired(w, r, shrToken, pcfg, provider.(string), target, authCheckInterval) + oauthLoginRequired(w, r, pcfg.Oauth, provider.(string), target, authCheckInterval) return } if validDomains, found := oauthCfg.(map[string]interface{})["email_domains"]; found { @@ -347,12 +347,8 @@ func basicAuthRequired(w http.ResponseWriter, realm string) { _, _ = w.Write([]byte("No Authorization\n")) } -func oauthLoginRequired(w http.ResponseWriter, r *http.Request, shrToken string, pcfg *Config, provider, target string, authCheckInterval time.Duration) { - scheme := "https" - if pcfg.Oauth != nil && pcfg.Oauth.RedirectHttpOnly { - scheme = "http" - } - http.Redirect(w, r, fmt.Sprintf("%s://%s:%d/%s/login?targethost=%s&checkInterval=%s", scheme, pcfg.Oauth.RedirectHost, pcfg.Oauth.RedirectPort, provider, url.QueryEscape(target), authCheckInterval.String()), http.StatusFound) +func oauthLoginRequired(w http.ResponseWriter, r *http.Request, cfg *OauthConfig, provider, target string, authCheckInterval time.Duration) { + http.Redirect(w, r, fmt.Sprintf("%s/%s/login?targethost=%s&checkInterval=%s", cfg.RedirectUrl, provider, url.QueryEscape(target), authCheckInterval.String()), http.StatusFound) } func resolveService(hostMatch string, host string) string { diff --git a/etc/frontend.yml b/etc/frontend.yml index cd6e638e..13c78bad 100644 --- a/etc/frontend.yml +++ b/etc/frontend.yml @@ -2,7 +2,7 @@ # configuration, the software will expect this field to be incremented. This protects you against invalid configuration # versions and will refer to you to the documentation when the configuration structure changes. # -v: 2 +v: 3 # Setting the `host_match` setting will cause a `zrok access public` to ignore `Host` headers that do not contain the # configured string. This will allow you to let a load balancer access the frontend by IP address for health check @@ -13,16 +13,19 @@ v: 2 # The OAuth configuration is used when enabling OAuth authentication with your public frontend. # #oauth: -# # `redirect_host` and `redirect_port` should correspond with the DNS hostname and URL representing -# # the OAuth frontend you'll use with your installation. +# # `bind_address` is the of the interface where the OAuth frontend listener should +# # bind # # -# redirect_host: oauth.zrok.io -# redirect_port: 28080 +# bind_address: 127.0.0.1:8181 # -# # `redirect_http_only` will generate an HTTP URI for your OAuth frontend, rather than HTTPS. This -# # should only be set to `true` in development environments. +# # `redirect_url` is the of the URL where OAuth requests should be directed. # # -# redirect_http_only: false +# redirect_url: https://oauth.zrok.io +# +# # `cookie_domain` is the domain where the authentication cookies should be applied. Should likely match +# # the `host_match` specified above. +# # +# cookie_domain: zrok.io # # # `hash_key` is a unique key for your installation that is used to secure authentication payloads # # with OAuth providers.