From a6d04469c5bdd45b0b1b5d4f629ce7007c293fcf Mon Sep 17 00:00:00 2001 From: Kshama Jain Date: Thu, 27 May 2021 13:02:06 -0700 Subject: [PATCH] fix: logout redirect URL (#6347) * fix logout redirect url Signed-off-by: kshamajain99 --- server/logout/logout.go | 4 +-- server/logout/logout_test.go | 52 +++++++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/server/logout/logout.go b/server/logout/logout.go index 02337e610a..8446d4b5b1 100644 --- a/server/logout/logout.go +++ b/server/logout/logout.go @@ -67,10 +67,10 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if argoURL == "" { // golang does not provide any easy way to determine scheme of current request // so redirecting ot http which will auto-redirect too https if necessary - argoURL = fmt.Sprintf("http://%s", r.Host) + argoURL = fmt.Sprintf("http://%s", r.Host) + strings.TrimRight(strings.TrimLeft(h.rootPath, "/"), "/") } - logoutRedirectURL := strings.TrimRight(strings.TrimLeft(argoURL, "/"), "/") + strings.TrimRight(strings.TrimLeft(h.rootPath, "/"), "/") + logoutRedirectURL := strings.TrimRight(strings.TrimLeft(argoURL, "/"), "/") cookies := r.Cookies() tokenString, err = httputil.JoinCookies(common.AuthCookieName, cookies) diff --git a/server/logout/logout_test.go b/server/logout/logout_test.go index 5ee23bc2e4..68a2ee7e69 100644 --- a/server/logout/logout_test.go +++ b/server/logout/logout_test.go @@ -25,6 +25,7 @@ import ( var ( validJWTPattern = regexp.MustCompile(`[a-zA-Z0-9-_]+\.[a-zA-Z0-9-_]+\.[a-zA-Z0-9-_]+`) baseURL = "http://localhost:4000" + rootPath = "argocd" baseLogoutURL = "http://localhost:4000/logout" baseLogoutURLwithToken = "http://localhost:4000/logout?id_token_hint={{token}}" baseLogoutURLwithRedirectURL = "http://localhost:4000/logout?post_logout_redirect_uri={{logoutRedirectURL}}" @@ -34,6 +35,7 @@ var ( nonOidcToken = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpYXQiOjE2MDU1NzQyMTIsImlzcyI6ImFyZ29jZCIsIm5iZiI6MTYwNTU3NDIxMiwic3ViIjoiYWRtaW4ifQ.zDJ4piwWnwsHON-oPusHMXWINlnrRDTQykYogT7afeE" expectedNonOIDCLogoutURL = "http://localhost:4000" expectedOIDCLogoutURL = "https://dev-5695098.okta.com/oauth2/v1/logout?id_token_hint=" + oidcToken + "&post_logout_redirect_uri=" + baseURL + expectedOIDCLogoutURLWithRootPath = "https://dev-5695098.okta.com/oauth2/v1/logout?id_token_hint=" + oidcToken + "&post_logout_redirect_uri=" + baseURL + rootPath ) func TestConstructLogoutURL(t *testing.T) { @@ -114,6 +116,38 @@ func TestHandlerConstructLogoutURL(t *testing.T) { }, }, ) + kubeClientWithOIDCConfigButNoURL := fake.NewSimpleClientset( + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.ArgoCDConfigMapName, + Namespace: "default", + Labels: map[string]string{ + "app.kubernetes.io/part-of": "argocd", + }, + }, + Data: map[string]string{ + "oidc.config": "name: Okta \n" + + "issuer: https://dev-5695098.okta.com \n" + + "requestedScopes: [\"openid\", \"profile\", \"email\", \"groups\"] \n" + + "requestedIDTokenClaims: {\"groups\": {\"essential\": true}} \n" + + "logoutURL: https://dev-5695098.okta.com/oauth2/v1/logout?id_token_hint={{token}}&post_logout_redirect_uri={{logoutRedirectURL}}", + "url": "", + }, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.ArgoCDSecretName, + Namespace: "default", + Labels: map[string]string{ + "app.kubernetes.io/part-of": "argocd", + }, + }, + Data: map[string][]byte{ + "admin.password": nil, + "server.secretkey": nil, + }, + }, + ) kubeClientWithOIDCConfigButNoLogoutURL := fake.NewSimpleClientset( &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ @@ -176,10 +210,11 @@ func TestHandlerConstructLogoutURL(t *testing.T) { settingsManagerWithOIDCConfig := settings.NewSettingsManager(context.Background(), kubeClientWithOIDCConfig, "default") settingsManagerWithoutOIDCConfig := settings.NewSettingsManager(context.Background(), kubeClientWithoutOIDCConfig, "default") settingsManagerWithOIDCConfigButNoLogoutURL := settings.NewSettingsManager(context.Background(), kubeClientWithOIDCConfigButNoLogoutURL, "default") + settingsManagerWithOIDCConfigButNoURL := settings.NewSettingsManager(context.Background(), kubeClientWithOIDCConfigButNoURL, "default") sessionManager := session.NewSessionManager(settingsManagerWithOIDCConfig, test.NewFakeProjLister(), "", session.NewUserStateStorage(nil)) - oidcHandler := NewHandler(appclientset.NewSimpleClientset(), settingsManagerWithOIDCConfig, sessionManager, "", "default") + oidcHandler := NewHandler(appclientset.NewSimpleClientset(), settingsManagerWithOIDCConfig, sessionManager, rootPath, "default") oidcHandler.verifyToken = func(tokenString string) (jwt.Claims, string, error) { if !validJWTPattern.MatchString(tokenString) { return nil, "", errors.New("invalid jwt") @@ -201,6 +236,13 @@ func TestHandlerConstructLogoutURL(t *testing.T) { return &jwt.StandardClaims{Issuer: "okta"}, "", nil } + oidcHandlerWithoutBaseURL := NewHandler(appclientset.NewSimpleClientset(), settingsManagerWithOIDCConfigButNoURL, sessionManager, "argocd", "default") + oidcHandlerWithoutBaseURL.verifyToken = func(tokenString string) (jwt.Claims, string, error) { + if !validJWTPattern.MatchString(tokenString) { + return nil, "", errors.New("invalid jwt") + } + return &jwt.StandardClaims{Issuer: "okta"}, "", nil + } oidcTokenHeader := make(map[string][]string) oidcTokenHeader["Cookie"] = []string{"argocd.token=" + oidcToken} nonOidcTokenHeader := make(map[string][]string) @@ -238,6 +280,14 @@ func TestHandlerConstructLogoutURL(t *testing.T) { expectedLogoutURL: expectedOIDCLogoutURL, wantErr: false, }, + { + name: "Case: OIDC logout request with valid token but missing URL", + handler: oidcHandlerWithoutBaseURL, + request: oidcRequest, + responseRecorder: httptest.NewRecorder(), + expectedLogoutURL: expectedOIDCLogoutURLWithRootPath, + wantErr: false, + }, { name: "Case: non-OIDC logout request with valid token", handler: nonoidcHandler,