feat: endpoint to logout app subdomain URLs (#5428)

Co-authored-by: Bruno Quaresma <bruno@coder.com>
This commit is contained in:
Dean Sheather 2022-12-20 10:45:13 -08:00 committed by GitHub
parent 86257ce7fc
commit 50dfc2082b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 391 additions and 33 deletions

View File

@ -201,7 +201,7 @@ func (api *API) deleteAPIKey(rw http.ResponseWriter, r *http.Request) {
}
// Generates a new ID and secret for an API key.
func generateAPIKeyIDSecret() (id string, secret string, err error) {
func GenerateAPIKeyIDSecret() (id string, secret string, err error) {
// Length of an API Key ID.
id, err = cryptorand.String(10)
if err != nil {
@ -239,7 +239,7 @@ func (api *API) validateAPIKeyLifetime(lifetime time.Duration) error {
}
func (api *API) createAPIKey(ctx context.Context, params createAPIKeyParams) (*http.Cookie, error) {
keyID, keySecret, err := generateAPIKeyIDSecret()
keyID, keySecret, err := GenerateAPIKeyIDSecret()
if err != nil {
return nil, xerrors.Errorf("generate API key: %w", err)
}

View File

@ -4,6 +4,7 @@ import (
"bytes"
"context"
"crypto/sha256"
"crypto/subtle"
"database/sql"
"encoding/base64"
"encoding/json"
@ -36,7 +37,15 @@ const (
// conflict with query parameters that users may use.
//nolint:gosec
subdomainProxyAPIKeyParam = "coder_application_connect_api_key_35e783"
redirectURIQueryParam = "redirect_uri"
// redirectURIQueryParam is the query param for the app URL to be passed
// back to the API auth endpoint on the main access URL.
redirectURIQueryParam = "redirect_uri"
// appLogoutHostname is the hostname to use for the logout redirect. When
// the dashboard logs out, it will redirect to this subdomain of the app
// hostname, and the server will remove the cookie and redirect to the main
// login page.
// It is important that this URL can never match a valid app hostname.
appLogoutHostname = "coder-logout"
)
// 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
return httpapi.ApplicationURL{}, false
}
// Check if the request is part of a logout flow.
if subdomain == appLogoutHostname {
api.handleWorkspaceAppLogout(rw, r)
return httpapi.ApplicationURL{}, false
}
// Parse the application URL from the subdomain.
app, err := httpapi.ParseSubdomainAppURL(subdomain)
if err != nil {
@ -273,6 +288,95 @@ func (api *API) parseWorkspaceApplicationHostname(rw http.ResponseWriter, r *htt
return app, true
}
func (api *API) handleWorkspaceAppLogout(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
// Delete the API key and cookie first before attempting to parse/validate
// the redirect URI.
cookie, err := r.Cookie(httpmw.DevURLSessionTokenCookie)
if err == nil && cookie.Value != "" {
id, secret, err := httpmw.SplitAPIToken(cookie.Value)
// If it's not a valid token then we don't need to delete it from the
// database, but we'll still delete the cookie.
if err == nil {
// To avoid a situation where someone overloads the API with
// different auth formats, and tricks this endpoint into deleting an
// unchecked API key, we validate that the secret matches the secret
// we store in the database.
apiKey, err := api.Database.GetAPIKeyByID(ctx, id)
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Failed to lookup API key.",
Detail: err.Error(),
})
return
}
// This is wrapped in `err == nil` because if the API key doesn't
// exist, we still want to delete the cookie.
if err == nil {
hashedSecret := sha256.Sum256([]byte(secret))
if subtle.ConstantTimeCompare(apiKey.HashedSecret, hashedSecret[:]) != 1 {
httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{
Message: httpmw.SignedOutErrorMessage,
Detail: "API key secret is invalid.",
})
return
}
err = api.Database.DeleteAPIKeyByID(ctx, id)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Failed to delete API key.",
Detail: err.Error(),
})
return
}
}
}
}
if !api.setWorkspaceAppCookie(rw, r, "") {
return
}
// Read the redirect URI from the query string.
redirectURI := r.URL.Query().Get(redirectURIQueryParam)
if redirectURI == "" {
redirectURI = api.AccessURL.String()
} else {
// Validate that the redirect URI is a valid URL and exists on the same
// host as the access URL or an app URL.
parsedRedirectURI, err := url.Parse(redirectURI)
if err != nil {
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
Status: http.StatusBadRequest,
Title: "Invalid redirect URI",
Description: fmt.Sprintf("Could not parse redirect URI %q: %s", redirectURI, err.Error()),
RetryEnabled: false,
DashboardURL: api.AccessURL.String(),
})
return
}
// Check if the redirect URI is on the same host as the access URL or an
// app URL.
ok := httpapi.HostnamesMatch(api.AccessURL.Hostname(), parsedRedirectURI.Hostname())
if !ok && api.AppHostnameRegex != nil {
// We could also check that it's a valid application URL for
// completeness, but this check should be good enough.
_, ok = httpapi.ExecuteHostnamePattern(api.AppHostnameRegex, parsedRedirectURI.Hostname())
}
if !ok {
// 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()
}
http.Redirect(rw, r, redirectURI, http.StatusTemporaryRedirect)
}
// lookupWorkspaceApp looks up the workspace application by slug in the given
// agent and returns it. If the application is not found or there was a server
// 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,
// and strip that query parameter.
if encryptedAPIKey := r.URL.Query().Get(subdomainProxyAPIKeyParam); encryptedAPIKey != "" {
// Exchange the encoded API key for a real one.
_, apiKey, err := decryptAPIKey(r.Context(), api.Database, encryptedAPIKey)
_, token, err := decryptAPIKey(r.Context(), api.Database, encryptedAPIKey)
if err != nil {
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
Status: http.StatusBadRequest,
@ -424,33 +528,7 @@ func (api *API) verifyWorkspaceApplicationSubdomainAuth(rw http.ResponseWriter,
return false
}
hostSplit := strings.SplitN(api.AppHostname, ".", 2)
if len(hostSplit) != 2 {
// This should be impossible as we verify the app hostname on
// startup, but we'll check anyways.
api.Logger.Error(r.Context(), "could not split invalid app hostname", slog.F("hostname", api.AppHostname))
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
Status: http.StatusInternalServerError,
Title: "Internal Server Error",
Description: "The app is configured with an invalid app wildcard hostname. Please contact an administrator.",
RetryEnabled: false,
DashboardURL: api.AccessURL.String(),
})
return false
}
// Set the app cookie for all subdomains of api.AppHostname. This cookie
// is handled properly by the ExtractAPIKey middleware.
cookieHost := "." + hostSplit[1]
http.SetCookie(rw, &http.Cookie{
Name: httpmw.DevURLSessionTokenCookie,
Value: apiKey,
Domain: cookieHost,
Path: "/",
HttpOnly: true,
SameSite: http.SameSiteLaxMode,
Secure: api.SecureAuthCookie,
})
api.setWorkspaceAppCookie(rw, r, token)
// Strip the query parameter.
path := r.URL.Path
@ -484,6 +562,51 @@ func (api *API) verifyWorkspaceApplicationSubdomainAuth(rw http.ResponseWriter,
return false
}
// setWorkspaceAppCookie sets a cookie on the workspace app domain. If the app
// hostname cannot be parsed properly, a static error page is rendered and false
// is returned.
//
// If an empty token is supplied, it will clear the cookie.
func (api *API) setWorkspaceAppCookie(rw http.ResponseWriter, r *http.Request, token string) bool {
hostSplit := strings.SplitN(api.AppHostname, ".", 2)
if len(hostSplit) != 2 {
// This should be impossible as we verify the app hostname on
// startup, but we'll check anyways.
api.Logger.Error(r.Context(), "could not split invalid app hostname", slog.F("hostname", api.AppHostname))
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
Status: http.StatusInternalServerError,
Title: "Internal Server Error",
Description: "The app is configured with an invalid app wildcard hostname. Please contact an administrator.",
RetryEnabled: false,
DashboardURL: api.AccessURL.String(),
})
return false
}
// Set the app cookie for all subdomains of api.AppHostname. This cookie is
// handled properly by the ExtractAPIKey middleware.
//
// We don't set an expiration because the key in the database already has an
// expiration.
maxAge := 0
if token == "" {
maxAge = -1
}
cookieHost := "." + hostSplit[1]
http.SetCookie(rw, &http.Cookie{
Name: httpmw.DevURLSessionTokenCookie,
Value: token,
Domain: cookieHost,
Path: "/",
MaxAge: maxAge,
HttpOnly: true,
SameSite: http.SameSiteLaxMode,
Secure: api.SecureAuthCookie,
})
return true
}
// workspaceApplicationAuth is an endpoint on the main router that handles
// redirects from the subdomain handler.
//

View File

@ -17,7 +17,7 @@ func TestAPIKeyEncryption(t *testing.T) {
t.Parallel()
generateAPIKey := func(t *testing.T, db database.Store) (keyID, keySecret string, hashedSecret []byte, data encryptedAPIKeyPayload) {
keyID, keySecret, err := generateAPIKeyIDSecret()
keyID, keySecret, err := GenerateAPIKeyIDSecret()
require.NoError(t, err)
hashedSecretArray := sha256.Sum256([]byte(keySecret))

View File

@ -21,6 +21,7 @@ import (
"cdr.dev/slog/sloggers/slogtest"
"github.com/coder/coder/agent"
"github.com/coder/coder/coderd"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/coderd/httpapi"
"github.com/coder/coder/coderd/httpmw"
@ -892,6 +893,195 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) {
})
}
func TestAppSubdomainLogout(t *testing.T) {
t.Parallel()
keyID, keySecret, err := coderd.GenerateAPIKeyIDSecret()
require.NoError(t, err)
fakeAPIKey := fmt.Sprintf("%s-%s", keyID, keySecret)
cases := []struct {
name string
// The cookie to send with the request. The regular API key header
// is also sent to bypass any auth checks on this value, and to
// ensure that the logout code is safe when multiple keys are
// passed.
// Empty value means no cookie is sent, "-" means send a valid
// API key, and "bad-secret" means send a valid key ID with a bad
// secret.
cookie string
// You can use "access_url" to use the site access URL as the
// redirect URI, or "app_host" to use a valid app host.
redirectURI string
// If expectedStatus is not an error status, we expect the cookie to
// be deleted if it was set.
expectedStatus int
expectedBodyContains string
// 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
}{
{
name: "OKAccessURL",
cookie: "-",
redirectURI: "access_url",
expectedStatus: http.StatusTemporaryRedirect,
},
{
name: "OKAppHost",
cookie: "-",
redirectURI: "app_host",
expectedStatus: http.StatusTemporaryRedirect,
},
{
name: "OKNoAPIKey",
cookie: "",
redirectURI: "access_url",
// Even if the devurl cookie is missing, we still redirect without
// any complaints.
expectedStatus: http.StatusTemporaryRedirect,
},
{
name: "OKBadAPIKey",
cookie: "test-api-key",
redirectURI: "access_url",
// Even if the devurl cookie is bad, we still delete the cookie and
// redirect without any complaints.
expectedStatus: http.StatusTemporaryRedirect,
},
{
name: "OKUnknownAPIKey",
cookie: fakeAPIKey,
redirectURI: "access_url",
expectedStatus: http.StatusTemporaryRedirect,
},
{
name: "BadAPIKeySecret",
cookie: "bad-secret",
redirectURI: "access_url",
expectedStatus: http.StatusUnauthorized,
expectedBodyContains: "API key secret is invalid",
},
{
name: "InvalidRedirectURI",
cookie: "-",
redirectURI: string([]byte{0x00}),
expectedStatus: http.StatusBadRequest,
expectedBodyContains: "Could not parse redirect URI",
},
{
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",
},
}
for _, c := range cases {
c := c
t.Run(c.name, func(t *testing.T) {
t.Parallel()
client, _, _, _ := setupProxyTest(t)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
// The token should work.
_, err := client.User(ctx, codersdk.Me)
require.NoError(t, err)
appHost, err := client.GetAppHost(ctx)
require.NoError(t, err, "get app host")
if c.cookie == "-" {
c.cookie = client.SessionToken()
} else if c.cookie == "bad-secret" {
keyID, _, err := httpmw.SplitAPIToken(client.SessionToken())
require.NoError(t, err)
c.cookie = fmt.Sprintf("%s-%s", keyID, keySecret)
}
if c.redirectURI == "access_url" {
c.redirectURI = client.URL.String()
} else if c.redirectURI == "app_host" {
c.redirectURI = "http://" + strings.Replace(appHost.Host, "*", "something--something--something--something", 1) + "/"
}
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",
Host: strings.Replace(appHost.Host, "*", "coder-logout", 1),
Path: "/",
}
if c.redirectURI != "" {
q := logoutURL.Query()
q.Set("redirect_uri", c.redirectURI)
logoutURL.RawQuery = q.Encode()
}
req, err := http.NewRequestWithContext(ctx, http.MethodGet, logoutURL.String(), nil)
require.NoError(t, err, "create logout request")
// The header is prioritized over the devurl cookie if both are
// set, so this ensures we can trigger the logout code path with
// bad cookies during tests.
req.Header.Set(codersdk.SessionCustomHeader, client.SessionToken())
if c.cookie != "" {
req.AddCookie(&http.Cookie{
Name: httpmw.DevURLSessionTokenCookie,
Value: c.cookie,
})
}
client.HTTPClient.CheckRedirect = func(req *http.Request, via []*http.Request) error {
return http.ErrUseLastResponse
}
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 != "" {
cookies := resp.Cookies()
require.Len(t, cookies, 1, "logout response cookies")
cookie := cookies[0]
require.Equal(t, httpmw.DevURLSessionTokenCookie, cookie.Name)
require.Equal(t, "", cookie.Value)
require.True(t, cookie.Expires.Before(time.Now()), "cookie should be expired")
// The token shouldn't work anymore if it was the original valid
// session token.
if c.cookie == client.SessionToken() {
_, err = client.User(ctx, codersdk.Me)
require.Error(t, err)
}
}
if c.expectedBodyContains != "" {
body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
require.Contains(t, string(body), c.expectedBodyContains, "logout response body")
}
if c.expectedLocation != "" {
location := resp.Header.Get("Location")
require.Equal(t, c.expectedLocation, location, "logout response location")
}
})
}
}
func TestAppSharing(t *testing.T) {
t.Parallel()

View File

@ -140,6 +140,13 @@ export const authMachine =
hasFirstUser: {
data: boolean
}
signOut: {
data:
| {
redirectUrl: string
}
| undefined
}
},
},
context: {
@ -425,6 +432,10 @@ export const authMachine =
src: "signOut",
id: "signOut",
onDone: [
{
actions: ["unassignMe", "clearAuthError", "redirect"],
cond: "hasRedirectUrl",
},
{
actions: ["unassignMe", "clearAuthError"],
target: "gettingMethods",
@ -469,7 +480,30 @@ 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 { protocol, host } = window.location
const redirect_uri = encodeURIComponent(
`${protocol}//${host}/login`,
)
// The path doesn't matter but we use /api because the dev server
// proxies /api to the backend.
const uri = `${protocol}//${appHost.host.replace(
"*",
"coder-logout",
)}/api/logout?redirect_uri=${redirect_uri}`
return {
redirectUrl: uri,
}
}
},
getMe: API.getUser,
getMethods: API.getAuthMethods,
updateProfile: async (context, event) => {
@ -577,9 +611,20 @@ export const authMachine =
notifySuccessSSHKeyRegenerated: () => {
displaySuccess(Language.successRegenerateSSHKey)
},
redirect: (_, { data }) => {
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- data can be undefined
if (!data) {
throw new Error(
"Redirect only should be called with data.redirectUrl",
)
}
window.location.replace(data.redirectUrl)
},
},
guards: {
isTrue: (_, event) => event.data,
hasRedirectUrl: (_, { data }) => Boolean(data),
},
},
)