Skip to content

Commit 50dfc20

Browse files
deansheatherBrunoQuaresma
deansheather
and
BrunoQuaresma
authored
feat: endpoint to logout app subdomain URLs (#5428)
Co-authored-by: Bruno Quaresma <bruno@coder.com>
1 parent 86257ce commit 50dfc20

File tree

5 files changed

+391
-33
lines changed

5 files changed

+391
-33
lines changed

‎coderd/apikey.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ func (api *API) deleteAPIKey(rw http.ResponseWriter, r *http.Request) {
201201
}
202202

203203
// Generates a new ID and secret for an API key.
204-
func generateAPIKeyIDSecret() (id string, secret string, err error) {
204+
func GenerateAPIKeyIDSecret() (id string, secret string, err error) {
205205
// Length of an API Key ID.
206206
id, err = cryptorand.String(10)
207207
if err != nil {
@@ -239,7 +239,7 @@ func (api *API) validateAPIKeyLifetime(lifetime time.Duration) error {
239239
}
240240

241241
func (api *API) createAPIKey(ctx context.Context, params createAPIKeyParams) (*http.Cookie, error) {
242-
keyID, keySecret, err := generateAPIKeyIDSecret()
242+
keyID, keySecret, err := GenerateAPIKeyIDSecret()
243243
if err != nil {
244244
return nil, xerrors.Errorf("generate API key: %w", err)
245245
}

‎coderd/workspaceapps.go

Lines changed: 152 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"context"
66
"crypto/sha256"
7+
"crypto/subtle"
78
"database/sql"
89
"encoding/base64"
910
"encoding/json"
@@ -36,7 +37,15 @@ const (
3637
// conflict with query parameters that users may use.
3738
//nolint:gosec
3839
subdomainProxyAPIKeyParam = "coder_application_connect_api_key_35e783"
39-
redirectURIQueryParam = "redirect_uri"
40+
// redirectURIQueryParam is the query param for the app URL to be passed
41+
// back to the API auth endpoint on the main access URL.
42+
redirectURIQueryParam = "redirect_uri"
43+
// appLogoutHostname is the hostname to use for the logout redirect. When
44+
// the dashboard logs out, it will redirect to this subdomain of the app
45+
// hostname, and the server will remove the cookie and redirect to the main
46+
// login page.
47+
// It is important that this URL can never match a valid app hostname.
48+
appLogoutHostname = "coder-logout"
4049
)
4150

4251
// nonCanonicalHeaders is a map from "canonical" headers to the actual header we
@@ -257,6 +266,12 @@ func (api *API) parseWorkspaceApplicationHostname(rw http.ResponseWriter, r *htt
257266
return httpapi.ApplicationURL{}, false
258267
}
259268

269+
// Check if the request is part of a logout flow.
270+
if subdomain == appLogoutHostname {
271+
api.handleWorkspaceAppLogout(rw, r)
272+
return httpapi.ApplicationURL{}, false
273+
}
274+
260275
// Parse the application URL from the subdomain.
261276
app, err := httpapi.ParseSubdomainAppURL(subdomain)
262277
if err != nil {
@@ -273,6 +288,95 @@ func (api *API) parseWorkspaceApplicationHostname(rw http.ResponseWriter, r *htt
273288
return app, true
274289
}
275290

291+
func (api *API) handleWorkspaceAppLogout(rw http.ResponseWriter, r *http.Request) {
292+
ctx := r.Context()
293+
294+
// Delete the API key and cookie first before attempting to parse/validate
295+
// the redirect URI.
296+
cookie, err := r.Cookie(httpmw.DevURLSessionTokenCookie)
297+
if err == nil && cookie.Value != "" {
298+
id, secret, err := httpmw.SplitAPIToken(cookie.Value)
299+
// If it's not a valid token then we don't need to delete it from the
300+
// database, but we'll still delete the cookie.
301+
if err == nil {
302+
// To avoid a situation where someone overloads the API with
303+
// different auth formats, and tricks this endpoint into deleting an
304+
// unchecked API key, we validate that the secret matches the secret
305+
// we store in the database.
306+
apiKey, err := api.Database.GetAPIKeyByID(ctx, id)
307+
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
308+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
309+
Message: "Failed to lookup API key.",
310+
Detail: err.Error(),
311+
})
312+
return
313+
}
314+
// This is wrapped in `err == nil` because if the API key doesn't
315+
// exist, we still want to delete the cookie.
316+
if err == nil {
317+
hashedSecret := sha256.Sum256([]byte(secret))
318+
if subtle.ConstantTimeCompare(apiKey.HashedSecret, hashedSecret[:]) != 1 {
319+
httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{
320+
Message: httpmw.SignedOutErrorMessage,
321+
Detail: "API key secret is invalid.",
322+
})
323+
return
324+
}
325+
err = api.Database.DeleteAPIKeyByID(ctx, id)
326+
if err != nil {
327+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
328+
Message: "Failed to delete API key.",
329+
Detail: err.Error(),
330+
})
331+
return
332+
}
333+
}
334+
}
335+
}
336+
if !api.setWorkspaceAppCookie(rw, r, "") {
337+
return
338+
}
339+
340+
// Read the redirect URI from the query string.
341+
redirectURI := r.URL.Query().Get(redirectURIQueryParam)
342+
if redirectURI == "" {
343+
redirectURI = api.AccessURL.String()
344+
} else {
345+
// Validate that the redirect URI is a valid URL and exists on the same
346+
// host as the access URL or an app URL.
347+
parsedRedirectURI, err := url.Parse(redirectURI)
348+
if err != nil {
349+
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
350+
Status: http.StatusBadRequest,
351+
Title: "Invalid redirect URI",
352+
Description: fmt.Sprintf("Could not parse redirect URI %q: %s", redirectURI, err.Error()),
353+
RetryEnabled: false,
354+
DashboardURL: api.AccessURL.String(),
355+
})
356+
return
357+
}
358+
359+
// Check if the redirect URI is on the same host as the access URL or an
360+
// app URL.
361+
ok := httpapi.HostnamesMatch(api.AccessURL.Hostname(), parsedRedirectURI.Hostname())
362+
if !ok && api.AppHostnameRegex != nil {
363+
// We could also check that it's a valid application URL for
364+
// completeness, but this check should be good enough.
365+
_, ok = httpapi.ExecuteHostnamePattern(api.AppHostnameRegex, parsedRedirectURI.Hostname())
366+
}
367+
if !ok {
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
372+
}
373+
374+
redirectURI = parsedRedirectURI.String()
375+
}
376+
377+
http.Redirect(rw, r, redirectURI, http.StatusTemporaryRedirect)
378+
}
379+
276380
// lookupWorkspaceApp looks up the workspace application by slug in the given
277381
// agent and returns it. If the application is not found or there was a server
278382
// error while looking it up, an HTML error page is returned and false is
@@ -410,7 +514,7 @@ func (api *API) verifyWorkspaceApplicationSubdomainAuth(rw http.ResponseWriter,
410514
// and strip that query parameter.
411515
if encryptedAPIKey := r.URL.Query().Get(subdomainProxyAPIKeyParam); encryptedAPIKey != "" {
412516
// Exchange the encoded API key for a real one.
413-
_, apiKey, err := decryptAPIKey(r.Context(), api.Database, encryptedAPIKey)
517+
_, token, err := decryptAPIKey(r.Context(), api.Database, encryptedAPIKey)
414518
if err != nil {
415519
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
416520
Status: http.StatusBadRequest,
@@ -424,33 +528,7 @@ func (api *API) verifyWorkspaceApplicationSubdomainAuth(rw http.ResponseWriter,
424528
return false
425529
}
426530

427-
hostSplit := strings.SplitN(api.AppHostname, ".", 2)
428-
if len(hostSplit) != 2 {
429-
// This should be impossible as we verify the app hostname on
430-
// startup, but we'll check anyways.
431-
api.Logger.Error(r.Context(), "could not split invalid app hostname", slog.F("hostname", api.AppHostname))
432-
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
433-
Status: http.StatusInternalServerError,
434-
Title: "Internal Server Error",
435-
Description: "The app is configured with an invalid app wildcard hostname. Please contact an administrator.",
436-
RetryEnabled: false,
437-
DashboardURL: api.AccessURL.String(),
438-
})
439-
return false
440-
}
441-
442-
// Set the app cookie for all subdomains of api.AppHostname. This cookie
443-
// is handled properly by the ExtractAPIKey middleware.
444-
cookieHost := "." + hostSplit[1]
445-
http.SetCookie(rw, &http.Cookie{
446-
Name: httpmw.DevURLSessionTokenCookie,
447-
Value: apiKey,
448-
Domain: cookieHost,
449-
Path: "/",
450-
HttpOnly: true,
451-
SameSite: http.SameSiteLaxMode,
452-
Secure: api.SecureAuthCookie,
453-
})
531+
api.setWorkspaceAppCookie(rw, r, token)
454532

455533
// Strip the query parameter.
456534
path := r.URL.Path
@@ -484,6 +562,51 @@ func (api *API) verifyWorkspaceApplicationSubdomainAuth(rw http.ResponseWriter,
484562
return false
485563
}
486564

565+
// setWorkspaceAppCookie sets a cookie on the workspace app domain. If the app
566+
// hostname cannot be parsed properly, a static error page is rendered and false
567+
// is returned.
568+
//
569+
// If an empty token is supplied, it will clear the cookie.
570+
func (api *API) setWorkspaceAppCookie(rw http.ResponseWriter, r *http.Request, token string) bool {
571+
hostSplit := strings.SplitN(api.AppHostname, ".", 2)
572+
if len(hostSplit) != 2 {
573+
// This should be impossible as we verify the app hostname on
574+
// startup, but we'll check anyways.
575+
api.Logger.Error(r.Context(), "could not split invalid app hostname", slog.F("hostname", api.AppHostname))
576+
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
577+
Status: http.StatusInternalServerError,
578+
Title: "Internal Server Error",
579+
Description: "The app is configured with an invalid app wildcard hostname. Please contact an administrator.",
580+
RetryEnabled: false,
581+
DashboardURL: api.AccessURL.String(),
582+
})
583+
return false
584+
}
585+
586+
// Set the app cookie for all subdomains of api.AppHostname. This cookie is
587+
// handled properly by the ExtractAPIKey middleware.
588+
//
589+
// We don't set an expiration because the key in the database already has an
590+
// expiration.
591+
maxAge := 0
592+
if token == "" {
593+
maxAge = -1
594+
}
595+
cookieHost := "." + hostSplit[1]
596+
http.SetCookie(rw, &http.Cookie{
597+
Name: httpmw.DevURLSessionTokenCookie,
598+
Value: token,
599+
Domain: cookieHost,
600+
Path: "/",
601+
MaxAge: maxAge,
602+
HttpOnly: true,
603+
SameSite: http.SameSiteLaxMode,
604+
Secure: api.SecureAuthCookie,
605+
})
606+
607+
return true
608+
}
609+
487610
// workspaceApplicationAuth is an endpoint on the main router that handles
488611
// redirects from the subdomain handler.
489612
//

‎coderd/workspaceapps_internal_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ func TestAPIKeyEncryption(t *testing.T) {
1717
t.Parallel()
1818

1919
generateAPIKey := func(t *testing.T, db database.Store) (keyID, keySecret string, hashedSecret []byte, data encryptedAPIKeyPayload) {
20-
keyID, keySecret, err := generateAPIKeyIDSecret()
20+
keyID, keySecret, err := GenerateAPIKeyIDSecret()
2121
require.NoError(t, err)
2222

2323
hashedSecretArray := sha256.Sum256([]byte(keySecret))

0 commit comments

Comments
 (0)