From 08649d4677a50e22b7367ffc79a9b6c72f660512 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Bedn=C3=A1=C5=99?= Date: Fri, 2 Feb 2024 10:12:44 +0100 Subject: [PATCH] feat(oauth): add possibility to specify OAuthLogoutEndpoint for logout from OAuth Identity provider (#6073) --- CHANGELOG.md | 4 +++ oauth2/code_exchange_test.go | 8 +++--- oauth2/mux.go | 12 +++++++-- oauth2/mux_test.go | 48 ++++++++++++++++++++++++++++++++++-- server/server.go | 11 +++++---- 5 files changed, 70 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21a8414adb..e311943ae4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## unreleased +### Features + +1. [#6073](https://github.com/influxdata/chronograf/pull/6073): Possibility to specify OAuth logout endpoint to logout from OAuth Identity provider. + ### Other 1. [#6074](https://github.com/influxdata/chronograf/pull/6074): Upgrade golang to 1.20.13. diff --git a/oauth2/code_exchange_test.go b/oauth2/code_exchange_test.go index 230786bda1..2c5b06141f 100644 --- a/oauth2/code_exchange_test.go +++ b/oauth2/code_exchange_test.go @@ -32,7 +32,7 @@ func Test_CodeExchangeCSRF_AuthCodeURL(t *testing.T) { ProviderURL: "http://localhost:1234", Orgs: "", } - authMux := NewAuthMux(mp, auth, mt, "", clog.New(clog.ParseLevel("debug")), useidtoken, "hello", nil, nil) + authMux := NewAuthMux(mp, auth, mt, "", clog.New(clog.ParseLevel("debug")), useidtoken, "hello", nil, nil, "") // create AuthCodeURL with code exchange without PKCE codeExchange := NewCodeExchange(false, "") @@ -95,7 +95,7 @@ func Test_CodeExchangeCSRF_ExchangeCodeForToken(t *testing.T) { ProviderURL: authServer.URL, Orgs: "", } - authMux := NewAuthMux(mp, auth, auth.Tokens, "", clog.New(clog.ParseLevel("debug")), useidtoken, "hi", nil, nil) + authMux := NewAuthMux(mp, auth, auth.Tokens, "", clog.New(clog.ParseLevel("debug")), useidtoken, "hi", nil, nil, "") // create AuthCodeURL using CodeExchange with PKCE codeExchange := simpleTokenExchange @@ -136,7 +136,7 @@ func Test_CodeExchangePKCE_AuthCodeURL(t *testing.T) { ProviderURL: "http://localhost:1234", Orgs: "", } - authMux := NewAuthMux(mp, auth, mt, "", clog.New(clog.ParseLevel("debug")), useidtoken, "hi", nil, nil) + authMux := NewAuthMux(mp, auth, mt, "", clog.New(clog.ParseLevel("debug")), useidtoken, "hi", nil, nil, "") // create AuthCodeURL using CodeExchange with PKCE codeExchange := NewCodeExchange(true, "secret") @@ -213,7 +213,7 @@ func Test_CodeExchangePKCE_ExchangeCodeForToken(t *testing.T) { ProviderURL: authServer.URL, Orgs: "", } - authMux := NewAuthMux(mp, auth, jwt, "", clog.New(clog.ParseLevel("debug")), useidtoken, "hi", nil, nil) + authMux := NewAuthMux(mp, auth, jwt, "", clog.New(clog.ParseLevel("debug")), useidtoken, "hi", nil, nil, "") // create AuthCodeURL using CodeExchange with PKCE codeExchange := CodeExchangePKCE{Secret: secret} diff --git a/oauth2/mux.go b/oauth2/mux.go index 11c873e3db..881fe4a018 100644 --- a/oauth2/mux.go +++ b/oauth2/mux.go @@ -19,16 +19,24 @@ func NewAuthMux(p Provider, a Authenticator, t Tokenizer, basepath string, l chronograf.Logger, UseIDToken bool, LoginHint string, client *http.Client, codeExchange CodeExchange, -) *AuthMux { + logoutCallback string) *AuthMux { if codeExchange == nil { codeExchange = simpleTokenExchange } + + var afterLogoutURL string + if logoutCallback != "" { + afterLogoutURL = logoutCallback + } else { + afterLogoutURL = path.Join(basepath, "/") + } + mux := &AuthMux{ Provider: p, Auth: a, Tokens: t, SuccessURL: path.Join(basepath, "/landing"), - AfterLogoutURL: path.Join(basepath, "/"), + AfterLogoutURL: afterLogoutURL, FailureURL: path.Join(basepath, "/login"), Now: DefaultNowTime, Logger: l, diff --git a/oauth2/mux_test.go b/oauth2/mux_test.go index 2d25a4248f..b26d8b0f79 100644 --- a/oauth2/mux_test.go +++ b/oauth2/mux_test.go @@ -24,7 +24,7 @@ type mockCallbackResponse struct { // a function, and returning the desired handler. Cleanup is still the // responsibility of the test writer, so the httptest.Server's Close() method // should be deferred. -func setupMuxTest(response interface{}, selector func(*AuthMux) http.Handler) (*http.Client, *httptest.Server, *httptest.Server) { +func setupMuxTest(response interface{}, selector func(*AuthMux) http.Handler, config ...map[string]string) (*http.Client, *httptest.Server, *httptest.Server) { provider := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { rw.Header().Set("content-type", "application/json") rw.WriteHeader(http.StatusOK) @@ -53,7 +53,13 @@ func setupMuxTest(response interface{}, selector func(*AuthMux) http.Handler) (* useidtoken := false - jm := NewAuthMux(mp, auth, mt, "", clog.New(clog.ParseLevel("debug")), useidtoken, "", nil, nil) + logoutCallback := "" + if len(config) > 0 { + if v, ok := config[0]["logoutCallback"]; ok { + logoutCallback = v + } + } + jm := NewAuthMux(mp, auth, mt, "", clog.New(clog.ParseLevel("debug")), useidtoken, "", nil, nil, logoutCallback) ts := httptest.NewServer(selector(jm)) jar, _ := cookiejar.New(nil) hc := http.Client{ @@ -111,6 +117,44 @@ func Test_AuthMux_Logout_DeletesSessionCookie(t *testing.T) { } } +func Test_AuthMux_Logout_RedirectToLogoutCallback(t *testing.T) { + t.Parallel() + + var response interface{} + + hc, ts, prov := setupMuxTest(response, func(j *AuthMux) http.Handler { + return j.Logout() + }, map[string]string{"logoutCallback": "http://custom-url:8123?redirect=http://localhost:8888"}) + defer teardownMuxTest(hc, ts, prov) + + tsURL, _ := url.Parse(ts.URL) + + hc.Jar.SetCookies(tsURL, []*http.Cookie{ + { + Name: DefaultCookieName, + Value: "", + }, + }) + + resp, err := hc.Get(ts.URL) + if err != nil { + t.Fatal("Error communicating with Logout() handler: err:", err) + } + + if resp.StatusCode < 300 || resp.StatusCode >= 400 { + t.Fatal("Expected to be redirected, but received status code", resp.StatusCode) + } + + loc, err := resp.Location() + if err != nil { + t.Fatal("Expected a location to be redirected to, but wasn't present") + } + + if loc.String() != "http://custom-url:8123?redirect=http://localhost:8888" { + t.Fatal("Expected to be redirected to http://custom-url:8123?redirect=http://localhost:8888, but was", loc.String()) + } +} + func Test_AuthMux_Login_RedirectsToCorrectURL(t *testing.T) { t.Parallel() diff --git a/server/server.go b/server/server.go index 7c404c979c..153898f502 100644 --- a/server/server.go +++ b/server/server.go @@ -115,6 +115,7 @@ type Server struct { GenericInsecure bool `long:"generic-insecure" description:"Whether or not to verify auth-url's tls certificates." env:"GENERIC_INSECURE"` GenericRootCA flags.Filename `long:"generic-root-ca" description:"File location of root ca cert for generic oauth tls verification." env:"GENERIC_ROOT_CA"` OAuthNoPKCE bool `long:"oauth-no-pkce" description:"Disables OAuth PKCE." env:"OAUTH_NO_PKCE"` + OAuthLogoutEndpoint string `long:"oauth-logout-endpoint" description:"OAuth endpoint to call for logout from OAuth Identity provider." env:"OAUTH_LOGOUT_ENDPOINT"` Auth0Domain string `long:"auth0-domain" description:"Subdomain of auth0.com used for Auth0 OAuth2 authentication" env:"AUTH0_DOMAIN"` Auth0ClientID string `long:"auth0-client-id" description:"Auth0 Client ID for OAuth2 support" env:"AUTH0_CLIENT_ID"` @@ -343,7 +344,7 @@ func (s *Server) githubOAuth(logger chronograf.Logger, auth oauth2.Authenticator Logger: logger, } jwt := oauth2.NewJWT(s.TokenSecret, s.JwksURL) - ghMux := oauth2.NewAuthMux(&gh, auth, jwt, s.Basepath, logger, s.UseIDToken, s.LoginHint, &s.oauthClient, s.createCodeExchange()) + ghMux := oauth2.NewAuthMux(&gh, auth, jwt, s.Basepath, logger, s.UseIDToken, s.LoginHint, &s.oauthClient, s.createCodeExchange(), s.OAuthLogoutEndpoint) return &gh, ghMux, s.UseGithub } @@ -357,7 +358,7 @@ func (s *Server) googleOAuth(logger chronograf.Logger, auth oauth2.Authenticator Logger: logger, } jwt := oauth2.NewJWT(s.TokenSecret, s.JwksURL) - goMux := oauth2.NewAuthMux(&google, auth, jwt, s.Basepath, logger, s.UseIDToken, s.LoginHint, &s.oauthClient, s.createCodeExchange()) + goMux := oauth2.NewAuthMux(&google, auth, jwt, s.Basepath, logger, s.UseIDToken, s.LoginHint, &s.oauthClient, s.createCodeExchange(), s.OAuthLogoutEndpoint) return &google, goMux, s.UseGoogle } @@ -369,7 +370,7 @@ func (s *Server) herokuOAuth(logger chronograf.Logger, auth oauth2.Authenticator Logger: logger, } jwt := oauth2.NewJWT(s.TokenSecret, s.JwksURL) - hMux := oauth2.NewAuthMux(&heroku, auth, jwt, s.Basepath, logger, s.UseIDToken, s.LoginHint, &s.oauthClient, s.createCodeExchange()) + hMux := oauth2.NewAuthMux(&heroku, auth, jwt, s.Basepath, logger, s.UseIDToken, s.LoginHint, &s.oauthClient, s.createCodeExchange(), s.OAuthLogoutEndpoint) return &heroku, hMux, s.UseHeroku } @@ -388,7 +389,7 @@ func (s *Server) genericOAuth(logger chronograf.Logger, auth oauth2.Authenticato Logger: logger, } jwt := oauth2.NewJWT(s.TokenSecret, s.JwksURL) - genMux := oauth2.NewAuthMux(&gen, auth, jwt, s.Basepath, logger, s.UseIDToken, s.LoginHint, &s.oauthClient, s.createCodeExchange()) + genMux := oauth2.NewAuthMux(&gen, auth, jwt, s.Basepath, logger, s.UseIDToken, s.LoginHint, &s.oauthClient, s.createCodeExchange(), s.OAuthLogoutEndpoint) return &gen, genMux, s.UseGenericOAuth2 } @@ -404,7 +405,7 @@ func (s *Server) auth0OAuth(logger chronograf.Logger, auth oauth2.Authenticator) auth0, err := oauth2.NewAuth0(s.Auth0Domain, s.Auth0ClientID, s.Auth0ClientSecret, redirectURL.String(), s.Auth0Organizations, logger) jwt := oauth2.NewJWT(s.TokenSecret, s.JwksURL) - genMux := oauth2.NewAuthMux(&auth0, auth, jwt, s.Basepath, logger, s.UseIDToken, s.LoginHint, &s.oauthClient, s.createCodeExchange()) + genMux := oauth2.NewAuthMux(&auth0, auth, jwt, s.Basepath, logger, s.UseIDToken, s.LoginHint, &s.oauthClient, s.createCodeExchange(), s.OAuthLogoutEndpoint) if err != nil { logger.Error("Error parsing Auth0 domain: err:", err)