fix: skip redirect url validation when it's the base href (argoproj#1…

…0058) Signed-off-by: CI <[email protected]> nicer way of doing it Signed-off-by: CI <[email protected]>
crenshaw-dev · Jul 27, 2022 · 1f54062 · 1f54062
1 parent 44fea52
commit 1f54062
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 1 deletion.
diff --git a/util/oidc/oidc.go b/util/oidc/oidc.go
@@ -195,7 +195,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
@@ -191,6 +191,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)