feat: logout app subdomains on logout button

coder · deansheather · Dec 20, 2022 · Dec 15, 2022 · Dec 15, 2022 · Dec 15, 2022
commit 7d6959a8f6a8ee5669ee34c4fbb9f621a9bebee2
diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go
@@ -365,14 +365,10 @@ func (api *API) handleWorkspaceAppLogout(rw http.ResponseWriter, r *http.Request
 			_, ok = httpapi.ExecuteHostnamePattern(api.AppHostnameRegex, parsedRedirectURI.Hostname())
 		}
 		if !ok {
-			site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
-				Status:       http.StatusBadRequest,
-				Title:        "Invalid redirect URI",
-				Description:  fmt.Sprintf("Redirect URI %q is not on the same host as the access URL or an application URL", redirectURI),
-				RetryEnabled: false,
-				DashboardURL: api.AccessURL.String(),
-			})
-			return
+			// The redirect URI they provided is not allowed, but we don't want
+			// to return an error page because it'll interrupt the logout flow,
+			// so we just use the default access URL.
+			parsedRedirectURI = api.AccessURL
 		}
 
 		redirectURI = parsedRedirectURI.String()

diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go
@@ -891,6 +891,7 @@ func TestAppSubdomainLogout(t *testing.T) {
 		// If empty, the expected location is the redirectURI if the
 		// expected status code is http.StatusTemporaryRedirect (using the
 		// access URL if not set).
+		// You can use "access_url" to force the access URL.
 		expectedLocation string
 	}{
 		{
@@ -942,11 +943,15 @@ func TestAppSubdomainLogout(t *testing.T) {
 			expectedBodyContains: "Could not parse redirect URI",
 		},
 		{
-			name:                 "DisallowedRedirectURI",
-			cookie:               "-",
-			redirectURI:          "https://github.com/coder/coder",
-			expectedStatus:       http.StatusBadRequest,
-			expectedBodyContains: "not on the same host as the access URL",
+			name:        "DisallowedRedirectURI",
+			cookie:      "-",
+			redirectURI: "https://github.com/coder/coder",
+			// We don't allow redirecting to a different host, but we don't
+			// show an error page and just redirect to the access URL to avoid
+			// breaking the logout flow if the user is accessing from the wrong
+			// host.
+			expectedStatus:   http.StatusTemporaryRedirect,
+			expectedLocation: "access_url",
 		},
 	}
 
@@ -983,6 +988,9 @@ func TestAppSubdomainLogout(t *testing.T) {
 			if c.expectedLocation == "" && c.expectedStatus == http.StatusTemporaryRedirect {
 				c.expectedLocation = c.redirectURI
 			}
+			if c.expectedLocation == "access_url" {
+				c.expectedLocation = client.URL.String()
+			}
 
 			logoutURL := &url.URL{
 				Scheme: "http",
@@ -1013,6 +1021,7 @@ func TestAppSubdomainLogout(t *testing.T) {
 			}
 			resp, err := client.HTTPClient.Do(req)
 			require.NoError(t, err, "do logout request")
+			defer resp.Body.Close()
 
 			require.Equal(t, c.expectedStatus, resp.StatusCode, "logout response status code")
 			if c.expectedStatus < 400 && c.cookie != "" {

@@ -469,7 +469,21 @@ export const authMachine =
         signIn: async (_, event) => {
           return await API.login(event.email, event.password)
         },
-        signOut: API.logout,
+        signOut: async () => {
+          // Get app hostname so we can see if we need to log out of app URLs.
+          // We need to load this before we log out of the API as this is an
+          // authenticated endpoint.
+          const appHost = await API.getApplicationsHost();
+          await API.logout();
+
+          if (appHost.host) {
+            const redirect_uri = encodeURIComponent(window.location.href);
+            // The path doesn't matter but we use /api because the dev server
+            // proxies /api to the backend.
+            const uri = `${window.location.protocol}//${appHost.host.replace("*", "coder-logout")}/api/logout?redirect_uri=${redirect_uri}`;
+            window.location.replace(uri);
+          }
+        },
         getMe: API.getUser,
         getMethods: API.getAuthMethods,
         updateProfile: async (context, event) => {