From 5f800300a4eabe58697e627a2904d8c824830a15 Mon Sep 17 00:00:00 2001
From: Caleb Doxsey <cdoxsey@pomerium.com>
Date: Wed, 10 Apr 2024 13:39:07 -0600
Subject: [PATCH] core/authenticate: redirect to /.pomerium/signed_out when no
 signout redirect url is defined (#5060)

---
 authenticate/handlers.go      |  5 +++++
 authenticate/handlers_test.go | 30 +++++++++++++++++++++++++-----
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/authenticate/handlers.go b/authenticate/handlers.go
index 61b51f6f0..ba05175cc 100644
--- a/authenticate/handlers.go
+++ b/authenticate/handlers.go
@@ -237,6 +237,11 @@ func (a *Authenticate) signOutRedirect(w http.ResponseWriter, r *http.Request) e
 		log.Warn(r.Context()).Err(err).Msg("authenticate: failed to get sign out url for authenticator")
 	}
 
+	// if the authenticator failed to sign out, and no sign out url is defined, just go to the signed out page
+	if signOutURL == "" {
+		signOutURL = authenticateSignedOutURL
+	}
+
 	httputil.Redirect(w, r, signOutURL, http.StatusFound)
 	return nil
 }
diff --git a/authenticate/handlers_test.go b/authenticate/handlers_test.go
index d4b5f22da..e4ad33733 100644
--- a/authenticate/handlers_test.go
+++ b/authenticate/handlers_test.go
@@ -132,6 +132,7 @@ func TestAuthenticate_SignOut(t *testing.T) {
 		sessionStore sessions.SessionStore
 		wantCode     int
 		wantBody     string
+		wantLocation string
 	}{
 		{
 			"good post",
@@ -145,6 +146,7 @@ func TestAuthenticate_SignOut(t *testing.T) {
 			&mstore.Store{Encrypted: true, Session: &sessions.State{}},
 			http.StatusFound,
 			"",
+			"https://corp.pomerium.io/",
 		},
 		{
 			"signout redirect url",
@@ -158,6 +160,21 @@ func TestAuthenticate_SignOut(t *testing.T) {
 			&mstore.Store{Encrypted: true, Session: &sessions.State{}},
 			http.StatusFound,
 			"",
+			"https://signout-redirect-url.example.com",
+		},
+		{
+			"empty redirect url",
+			http.MethodPost,
+			nil,
+			"",
+			"",
+			"sig",
+			"ts",
+			identity.MockProvider{SignOutError: oidc.ErrSignoutNotImplemented},
+			&mstore.Store{Encrypted: true, Session: &sessions.State{}},
+			http.StatusFound,
+			"",
+			"https://authenticate.pomerium.app/.pomerium/signed_out",
 		},
 		{
 			"failed revoke",
@@ -171,6 +188,7 @@ func TestAuthenticate_SignOut(t *testing.T) {
 			&mstore.Store{Encrypted: true, Session: &sessions.State{}},
 			http.StatusFound,
 			"",
+			"https://corp.pomerium.io/",
 		},
 		{
 			"load session error",
@@ -184,6 +202,7 @@ func TestAuthenticate_SignOut(t *testing.T) {
 			&mstore.Store{Encrypted: true, Session: &sessions.State{}},
 			http.StatusFound,
 			"",
+			"https://corp.pomerium.io/",
 		},
 		{
 			"bad redirect uri",
@@ -197,6 +216,7 @@ func TestAuthenticate_SignOut(t *testing.T) {
 			&mstore.Store{Encrypted: true, Session: &sessions.State{}},
 			http.StatusFound,
 			"",
+			"/corp.pomerium.io/",
 		},
 	}
 	for _, tt := range tests {
@@ -224,7 +244,9 @@ func TestAuthenticate_SignOut(t *testing.T) {
 			params, _ := url.ParseQuery(u.RawQuery)
 			params.Add("sig", tt.sig)
 			params.Add("ts", tt.ts)
-			params.Add(urlutil.QueryRedirectURI, tt.redirectURL)
+			if tt.redirectURL != "" {
+				params.Add(urlutil.QueryRedirectURI, tt.redirectURL)
+			}
 			u.RawQuery = params.Encode()
 			r := httptest.NewRequest(tt.method, u.String(), nil)
 			state, err := tt.sessionStore.LoadSession(r)
@@ -245,10 +267,8 @@ func TestAuthenticate_SignOut(t *testing.T) {
 			if diff := cmp.Diff(body, tt.wantBody); diff != "" {
 				t.Errorf("handler returned wrong body Body: %s", diff)
 			}
-			if tt.signoutRedirectURL != "" {
-				loc := w.Header().Get("Location")
-				assert.Contains(t, loc, tt.signoutRedirectURL)
-			}
+			loc := w.Header().Get("Location")
+			assert.Equal(t, tt.wantLocation, loc)
 		})
 	}
 }