Skip to content

Commit 7d6959a

Browse files
committed
feat: logout app subdomains on logout button
1 parent 1a43a35 commit 7d6959a

File tree

3 files changed

+33
-14
lines changed

3 files changed

+33
-14
lines changed

coderd/workspaceapps.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -365,14 +365,10 @@ func (api *API) handleWorkspaceAppLogout(rw http.ResponseWriter, r *http.Request
365365
_, ok = httpapi.ExecuteHostnamePattern(api.AppHostnameRegex, parsedRedirectURI.Hostname())
366366
}
367367
if !ok {
368-
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
369-
Status: http.StatusBadRequest,
370-
Title: "Invalid redirect URI",
371-
Description: fmt.Sprintf("Redirect URI %q is not on the same host as the access URL or an application URL", redirectURI),
372-
RetryEnabled: false,
373-
DashboardURL: api.AccessURL.String(),
374-
})
375-
return
368+
// The redirect URI they provided is not allowed, but we don't want
369+
// to return an error page because it'll interrupt the logout flow,
370+
// so we just use the default access URL.
371+
parsedRedirectURI = api.AccessURL
376372
}
377373

378374
redirectURI = parsedRedirectURI.String()

coderd/workspaceapps_test.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -891,6 +891,7 @@ func TestAppSubdomainLogout(t *testing.T) {
891891
// If empty, the expected location is the redirectURI if the
892892
// expected status code is http.StatusTemporaryRedirect (using the
893893
// access URL if not set).
894+
// You can use "access_url" to force the access URL.
894895
expectedLocation string
895896
}{
896897
{
@@ -942,11 +943,15 @@ func TestAppSubdomainLogout(t *testing.T) {
942943
expectedBodyContains: "Could not parse redirect URI",
943944
},
944945
{
945-
name: "DisallowedRedirectURI",
946-
cookie: "-",
947-
redirectURI: "https://github.com/coder/coder",
948-
expectedStatus: http.StatusBadRequest,
949-
expectedBodyContains: "not on the same host as the access URL",
946+
name: "DisallowedRedirectURI",
947+
cookie: "-",
948+
redirectURI: "https://github.com/coder/coder",
949+
// We don't allow redirecting to a different host, but we don't
950+
// show an error page and just redirect to the access URL to avoid
951+
// breaking the logout flow if the user is accessing from the wrong
952+
// host.
953+
expectedStatus: http.StatusTemporaryRedirect,
954+
expectedLocation: "access_url",
950955
},
951956
}
952957

@@ -983,6 +988,9 @@ func TestAppSubdomainLogout(t *testing.T) {
983988
if c.expectedLocation == "" && c.expectedStatus == http.StatusTemporaryRedirect {
984989
c.expectedLocation = c.redirectURI
985990
}
991+
if c.expectedLocation == "access_url" {
992+
c.expectedLocation = client.URL.String()
993+
}
986994

987995
logoutURL := &url.URL{
988996
Scheme: "http",
@@ -1013,6 +1021,7 @@ func TestAppSubdomainLogout(t *testing.T) {
10131021
}
10141022
resp, err := client.HTTPClient.Do(req)
10151023
require.NoError(t, err, "do logout request")
1024+
defer resp.Body.Close()
10161025

10171026
require.Equal(t, c.expectedStatus, resp.StatusCode, "logout response status code")
10181027
if c.expectedStatus < 400 && c.cookie != "" {

site/src/xServices/auth/authXService.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,21 @@ export const authMachine =
469469
signIn: async (_, event) => {
470470
return await API.login(event.email, event.password)
471471
},
472-
signOut: API.logout,
472+
signOut: async () => {
473+
// Get app hostname so we can see if we need to log out of app URLs.
474+
// We need to load this before we log out of the API as this is an
475+
// authenticated endpoint.
476+
const appHost = await API.getApplicationsHost();
477+
await API.logout();
478+
479+
if (appHost.host) {
480+
const redirect_uri = encodeURIComponent(window.location.href);
481+
// The path doesn't matter but we use /api because the dev server
482+
// proxies /api to the backend.
483+
const uri = `${window.location.protocol}//${appHost.host.replace("*", "coder-logout")}/api/logout?redirect_uri=${redirect_uri}`;
484+
window.location.replace(uri);
485+
}
486+
},
473487
getMe: API.getUser,
474488
getMethods: API.getAuthMethods,
475489
updateProfile: async (context, event) => {

0 commit comments

Comments
 (0)