From 6d0b9caed549b30de36b62b33deb368b8795e9da Mon Sep 17 00:00:00 2001
From: Michael Crenshaw <michael@crenshaw.dev>
Date: Wed, 27 Jul 2022 15:51:57 -0400
Subject: [PATCH] fix: skip redirect url validation when it's the base href
 (#10058) (#10116)

* fix: skip redirect url validation when it's the base href (#10058)

Signed-off-by: CI <michael@crenshaw.dev>

nicer way of doing it

Signed-off-by: CI <michael@crenshaw.dev>

* fix missin arg

Signed-off-by: CI <michael@crenshaw.dev>
---
 util/oidc/oidc.go      |  2 +-
 util/oidc/oidc_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/util/oidc/oidc.go b/util/oidc/oidc.go
index 030388e0c6365..deeb1e2ee9310 100644
--- a/util/oidc/oidc.go
+++ b/util/oidc/oidc.go
@@ -190,7 +190,7 @@ func (a *ClientApp) verifyAppState(r *http.Request, w http.ResponseWriter, state
 	redirectURL := a.baseHRef
 	parts := strings.SplitN(cookieVal, ":", 2)
 	if len(parts) == 2 && parts[1] != "" {
-		if !isValidRedirectURL(parts[1], []string{a.settings.URL}) {
+		if !isValidRedirectURL(parts[1], []string{a.settings.URL, a.baseHRef}) {
 			sanitizedUrl := parts[1]
 			if len(sanitizedUrl) > 100 {
 				sanitizedUrl = sanitizedUrl[:100]
diff --git a/util/oidc/oidc_test.go b/util/oidc/oidc_test.go
index c673badd06c51..2025f36b12e71 100644
--- a/util/oidc/oidc_test.go
+++ b/util/oidc/oidc_test.go
@@ -192,6 +192,52 @@ requestedScopes: ["oidc"]`, oidcTestServer.URL),
 	})
 }
 
+func Test_Login_Flow(t *testing.T) {
+	// Show that SSO login works when no redirect URL is provided, and we fall back to the configured base href for the
+	// Argo CD instance.
+
+	oidcTestServer := test.GetOIDCTestServer(t)
+	t.Cleanup(oidcTestServer.Close)
+
+	cdSettings := &settings.ArgoCDSettings{
+		URL: "https://argocd.example.com",
+		OIDCConfigRAW: fmt.Sprintf(`
+name: Test
+issuer: %s
+clientID: xxx
+clientSecret: yyy
+requestedScopes: ["oidc"]`, oidcTestServer.URL),
+		OIDCTLSInsecureSkipVerify: true,
+	}
+
+	// The base href (the last argument for NewClientApp) is what HandleLogin will fall back to when no explicit
+	// redirect URL is given.
+	app, err := NewClientApp(cdSettings, "", "/")
+	require.NoError(t, err)
+
+	w := httptest.NewRecorder()
+
+	req := httptest.NewRequest("GET", "https://argocd.example.com/auth/login", nil)
+
+	app.HandleLogin(w, req)
+
+	redirectUrl, err := w.Result().Location()
+	require.NoError(t, err)
+
+	state := redirectUrl.Query()["state"]
+
+	req = httptest.NewRequest("GET", fmt.Sprintf("https://argocd.example.com/auth/callback?state=%s&code=abc", state), nil)
+	for _, cookie := range w.Result().Cookies() {
+		req.AddCookie(cookie)
+	}
+
+	w = httptest.NewRecorder()
+
+	app.HandleCallback(w, req)
+
+	assert.NotContains(t, w.Body.String(), InvalidRedirectURLError.Error())
+}
+
 func TestClientApp_HandleCallback(t *testing.T) {
 	oidcTestServer := test.GetOIDCTestServer(t)
 	t.Cleanup(oidcTestServer.Close)