diff --git a/cli/server.go b/cli/server.go index d7c9586339d5b..c66ec6f625d5b 100644 --- a/cli/server.go +++ b/cli/server.go @@ -78,6 +78,7 @@ import ( "github.com/coder/coder/coderd/tracing" "github.com/coder/coder/coderd/updatecheck" "github.com/coder/coder/coderd/util/slice" + "github.com/coder/coder/coderd/workspaceapps" "github.com/coder/coder/codersdk" "github.com/coder/coder/cryptorand" "github.com/coder/coder/provisioner/echo" @@ -781,37 +782,42 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. } } - // Read the app signing key from the DB. We store it hex - // encoded since the config table uses strings for the value and - // we don't want to deal with automatic encoding issues. - appSigningKeyStr, err := tx.GetAppSigningKey(ctx) + // Read the app signing key from the DB. We store it hex encoded + // since the config table uses strings for the value and we + // don't want to deal with automatic encoding issues. + appSecurityKeyStr, err := tx.GetAppSecurityKey(ctx) if err != nil && !xerrors.Is(err, sql.ErrNoRows) { return xerrors.Errorf("get app signing key: %w", err) } - if appSigningKeyStr == "" { - // Generate 64 byte secure random string. - b := make([]byte, 64) + // If the string in the DB is an invalid hex string or the + // length is not equal to the current key length, generate a new + // one. + // + // If the key is regenerated, old signed tokens and encrypted + // strings will become invalid. New signed app tokens will be + // generated automatically on failure. Any workspace app token + // smuggling operations in progress may fail, although with a + // helpful error. + if decoded, err := hex.DecodeString(appSecurityKeyStr); err != nil || len(decoded) != len(workspaceapps.SecurityKey{}) { + b := make([]byte, len(workspaceapps.SecurityKey{})) _, err := rand.Read(b) if err != nil { return xerrors.Errorf("generate fresh app signing key: %w", err) } - appSigningKeyStr = hex.EncodeToString(b) - err = tx.InsertAppSigningKey(ctx, appSigningKeyStr) + appSecurityKeyStr = hex.EncodeToString(b) + err = tx.UpsertAppSecurityKey(ctx, appSecurityKeyStr) if err != nil { return xerrors.Errorf("insert freshly generated app signing key to database: %w", err) } } - appSigningKey, err := hex.DecodeString(appSigningKeyStr) + appSecurityKey, err := workspaceapps.KeyFromString(appSecurityKeyStr) if err != nil { - return xerrors.Errorf("decode app signing key from database as hex: %w", err) - } - if len(appSigningKey) != 64 { - return xerrors.Errorf("app signing key must be 64 bytes, key in database is %d bytes", len(appSigningKey)) + return xerrors.Errorf("decode app signing key from database: %w", err) } - options.AppSigningKey = appSigningKey + options.AppSecurityKey = appSecurityKey return nil }, nil) if err != nil { diff --git a/cli/server_test.go b/cli/server_test.go index c2c93ebb7a736..93940ac402304 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -668,8 +668,7 @@ func TestServer(t *testing.T) { if c.tlsListener { accessURLParsed, err := url.Parse(c.requestURL) require.NoError(t, err) - client := codersdk.New(accessURLParsed) - client.HTTPClient = &http.Client{ + client := &http.Client{ CheckRedirect: func(req *http.Request, via []*http.Request) error { return http.ErrUseLastResponse }, @@ -682,11 +681,15 @@ func TestServer(t *testing.T) { }, }, } - defer client.HTTPClient.CloseIdleConnections() - _, err = client.HasFirstUser(ctx) - if err != nil { - require.ErrorContains(t, err, "Invalid application URL") - } + defer client.CloseIdleConnections() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, accessURLParsed.String(), nil) + require.NoError(t, err) + resp, err := client.Do(req) + // We don't care much about the response, just that TLS + // worked. + require.NoError(t, err) + defer resp.Body.Close() } }) } diff --git a/coderd/coderd.go b/coderd/coderd.go index 0e4b73c3e852c..5e95360c70c97 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -123,9 +123,9 @@ type Options struct { SwaggerEndpoint bool SetUserGroups func(ctx context.Context, tx database.Store, userID uuid.UUID, groupNames []string) error TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore] - // AppSigningKey denotes the symmetric key to use for signing temporary app - // tokens. The key must be 64 bytes long. - AppSigningKey []byte + // AppSecurityKey is the crypto key used to sign and encrypt tokens related to + // workspace applications. It consists of both a signing and encryption key. + AppSecurityKey workspaceapps.SecurityKey HealthcheckFunc func(ctx context.Context) (*healthcheck.Report, error) HealthcheckTimeout time.Duration HealthcheckRefresh time.Duration @@ -241,9 +241,6 @@ func New(options *Options) *API { v := schedule.NewAGPLTemplateScheduleStore() options.TemplateScheduleStore.Store(&v) } - if len(options.AppSigningKey) != 64 { - panic("coderd: AppSigningKey must be 64 bytes long") - } if options.HealthcheckFunc == nil { options.HealthcheckFunc = func(ctx context.Context) (*healthcheck.Report, error) { return healthcheck.Run(ctx, &healthcheck.ReportOptions{ @@ -309,7 +306,7 @@ func New(options *Options) *API { options.DeploymentValues, oauthConfigs, options.AgentInactiveDisconnectTimeout, - options.AppSigningKey, + options.AppSecurityKey, ), metricsCache: metricsCache, Auditor: atomic.Pointer[audit.Auditor]{}, @@ -334,6 +331,21 @@ func New(options *Options) *API { api.workspaceAgentCache = wsconncache.New(api.dialWorkspaceAgentTailnet, 0) api.TailnetCoordinator.Store(&options.TailnetCoordinator) + api.workspaceAppServer = &workspaceapps.Server{ + Logger: options.Logger.Named("workspaceapps"), + + DashboardURL: api.AccessURL, + AccessURL: api.AccessURL, + Hostname: api.AppHostname, + HostnameRegex: api.AppHostnameRegex, + DeploymentValues: options.DeploymentValues, + RealIPConfig: options.RealIPConfig, + + SignedTokenProvider: api.WorkspaceAppsProvider, + WorkspaceConnCache: api.workspaceAgentCache, + AppSecurityKey: options.AppSecurityKey, + } + apiKeyMiddleware := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: options.Database, OAuth2Configs: oauthConfigs, @@ -366,11 +378,12 @@ func New(options *Options) *API { httpmw.ExtractRealIP(api.RealIPConfig), httpmw.Logger(api.Logger), httpmw.Prometheus(options.PrometheusRegistry), - // handleSubdomainApplications checks if the first subdomain is a valid - // app URL. If it is, it will serve that application. + // SubdomainAppMW checks if the first subdomain is a valid app URL. If + // it is, it will serve that application. // - // Workspace apps do their own auth. - api.handleSubdomainApplications(apiRateLimiter), + // Workspace apps do their own auth and must be BEFORE the auth + // middleware. + api.workspaceAppServer.SubdomainAppMW(apiRateLimiter), // Build-Version is helpful for debugging. func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -393,16 +406,12 @@ func New(options *Options) *API { r.Get("/healthz", func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("OK")) }) - apps := func(r chi.Router) { - // Workspace apps do their own auth. + // Attach workspace apps routes. + r.Group(func(r chi.Router) { r.Use(apiRateLimiter) - r.HandleFunc("/*", api.workspaceAppsProxyPath) - } - // %40 is the encoded character of the @ symbol. VS Code Web does - // not handle character encoding properly, so it's safe to assume - // other applications might not as well. - r.Route("/%40{user}/{workspace_and_agent}/apps/{workspaceapp}", apps) - r.Route("/@{user}/{workspace_and_agent}/apps/{workspaceapp}", apps) + api.workspaceAppServer.Attach(r) + }) + r.Route("/derp", func(r chi.Router) { r.Get("/", derpHandler.ServeHTTP) // This is used when UDP is blocked, and latency must be checked via HTTP(s). @@ -644,9 +653,6 @@ func New(options *Options) *API { r.Post("/report-lifecycle", api.workspaceAgentReportLifecycle) r.Post("/metadata/{key}", api.workspaceAgentPostMetadata) }) - // No middleware on the PTY endpoint since it uses workspace - // application auth and signed app tokens. - r.Get("/{workspaceagent}/pty", api.workspaceAgentPTY) r.Route("/{workspaceagent}", func(r chi.Router) { r.Use( apiKeyMiddleware, @@ -655,11 +661,12 @@ func New(options *Options) *API { ) r.Get("/", api.workspaceAgent) r.Get("/watch-metadata", api.watchWorkspaceAgentMetadata) - r.Get("/pty", api.workspaceAgentPTY) r.Get("/startup-logs", api.workspaceAgentStartupLogs) r.Get("/listening-ports", api.workspaceAgentListeningPorts) r.Get("/connection", api.workspaceAgentConnection) r.Get("/coordinate", api.workspaceAgentClientCoordinate) + + // PTY is part of workspaceAppServer. }) }) r.Route("/workspaces", func(r chi.Router) { @@ -792,6 +799,7 @@ type API struct { workspaceAgentCache *wsconncache.Cache updateChecker *updatecheck.Checker WorkspaceAppsProvider workspaceapps.SignedTokenProvider + workspaceAppServer *workspaceapps.Server // Experiments contains the list of experiments currently enabled. // This is used to gate features that are not yet ready for production. diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index d26ebd1ce0d92..182774e7e8964 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -11,7 +11,6 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/base64" - "encoding/hex" "encoding/json" "encoding/pem" "errors" @@ -69,6 +68,7 @@ import ( "github.com/coder/coder/coderd/telemetry" "github.com/coder/coder/coderd/updatecheck" "github.com/coder/coder/coderd/util/ptr" + "github.com/coder/coder/coderd/workspaceapps" "github.com/coder/coder/codersdk" "github.com/coder/coder/codersdk/agentsdk" "github.com/coder/coder/cryptorand" @@ -81,9 +81,9 @@ import ( "github.com/coder/coder/testutil" ) -// AppSigningKey is a 64-byte key used to sign JWTs for workspace app tokens in -// tests. -var AppSigningKey = must(hex.DecodeString("64656164626565666465616462656566646561646265656664656164626565666465616462656566646561646265656664656164626565666465616462656566")) +// AppSecurityKey is a 96-byte key used to sign JWTs and encrypt JWEs for +// workspace app tokens in tests. +var AppSecurityKey = must(workspaceapps.KeyFromString("6465616e207761732068657265206465616e207761732068657265206465616e207761732068657265206465616e207761732068657265206465616e207761732068657265206465616e207761732068657265206465616e2077617320686572")) type Options struct { // AccessURL denotes a custom access URL. By default we use the httptest @@ -346,7 +346,7 @@ func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.Can DeploymentValues: options.DeploymentValues, UpdateCheckOptions: options.UpdateCheckOptions, SwaggerEndpoint: options.SwaggerEndpoint, - AppSigningKey: AppSigningKey, + AppSecurityKey: AppSecurityKey, SSHConfig: options.ConfigSSH, HealthcheckFunc: options.HealthcheckFunc, HealthcheckTimeout: options.HealthcheckTimeout, diff --git a/coderd/database/dbauthz/querier.go b/coderd/database/dbauthz/querier.go index ccb697795ed01..24df1b31bf681 100644 --- a/coderd/database/dbauthz/querier.go +++ b/coderd/database/dbauthz/querier.go @@ -379,14 +379,14 @@ func (q *querier) GetLogoURL(ctx context.Context) (string, error) { return q.db.GetLogoURL(ctx) } -func (q *querier) GetAppSigningKey(ctx context.Context) (string, error) { +func (q *querier) GetAppSecurityKey(ctx context.Context) (string, error) { // No authz checks - return q.db.GetAppSigningKey(ctx) + return q.db.GetAppSecurityKey(ctx) } -func (q *querier) InsertAppSigningKey(ctx context.Context, data string) error { +func (q *querier) UpsertAppSecurityKey(ctx context.Context, data string) error { // No authz checks as this is done during startup - return q.db.InsertAppSigningKey(ctx, data) + return q.db.UpsertAppSecurityKey(ctx, data) } func (q *querier) GetServiceBanner(ctx context.Context) (string, error) { @@ -994,6 +994,16 @@ func (q *querier) GetTemplateUserRoles(ctx context.Context, id uuid.UUID) ([]dat return q.db.GetTemplateUserRoles(ctx, id) } +func (q *querier) DeleteApplicationConnectAPIKeysByUserID(ctx context.Context, userID uuid.UUID) error { + // TODO: This is not 100% correct because it omits apikey IDs. + err := q.authorizeContext(ctx, rbac.ActionDelete, + rbac.ResourceAPIKey.WithOwner(userID.String())) + if err != nil { + return err + } + return q.db.DeleteApplicationConnectAPIKeysByUserID(ctx, userID) +} + func (q *querier) DeleteAPIKeysByUserID(ctx context.Context, userID uuid.UUID) error { // TODO: This is not 100% correct because it omits apikey IDs. err := q.authorizeContext(ctx, rbac.ActionDelete, diff --git a/coderd/database/dbfake/databasefake.go b/coderd/database/dbfake/databasefake.go index 4633e946ea03d..465cd69ec770e 100644 --- a/coderd/database/dbfake/databasefake.go +++ b/coderd/database/dbfake/databasefake.go @@ -143,7 +143,7 @@ type data struct { lastUpdateCheck []byte serviceBanner []byte logoURL string - appSigningKey string + appSecurityKey string lastLicenseID int32 } @@ -679,6 +679,19 @@ func (q *fakeQuerier) DeleteAPIKeyByID(_ context.Context, id string) error { return sql.ErrNoRows } +func (q *fakeQuerier) DeleteApplicationConnectAPIKeysByUserID(_ context.Context, userID uuid.UUID) error { + q.mutex.Lock() + defer q.mutex.Unlock() + + for i := len(q.apiKeys) - 1; i >= 0; i-- { + if q.apiKeys[i].UserID == userID && q.apiKeys[i].Scope == database.APIKeyScopeApplicationConnect { + q.apiKeys = append(q.apiKeys[:i], q.apiKeys[i+1:]...) + } + } + + return nil +} + func (q *fakeQuerier) DeleteAPIKeysByUserID(_ context.Context, userID uuid.UUID) error { q.mutex.Lock() defer q.mutex.Unlock() @@ -4463,18 +4476,18 @@ func (q *fakeQuerier) GetLogoURL(_ context.Context) (string, error) { return q.logoURL, nil } -func (q *fakeQuerier) GetAppSigningKey(_ context.Context) (string, error) { +func (q *fakeQuerier) GetAppSecurityKey(_ context.Context) (string, error) { q.mutex.RLock() defer q.mutex.RUnlock() - return q.appSigningKey, nil + return q.appSecurityKey, nil } -func (q *fakeQuerier) InsertAppSigningKey(_ context.Context, data string) error { +func (q *fakeQuerier) UpsertAppSecurityKey(_ context.Context, data string) error { q.mutex.Lock() defer q.mutex.Unlock() - q.appSigningKey = data + q.appSecurityKey = data return nil } diff --git a/coderd/database/dbgen/generator.go b/coderd/database/dbgen/generator.go index 3ba5205ab668c..96cd8b004648c 100644 --- a/coderd/database/dbgen/generator.go +++ b/coderd/database/dbgen/generator.go @@ -77,12 +77,23 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database secret, _ := cryptorand.String(22) hashed := sha256.Sum256([]byte(secret)) + ip := seed.IPAddress + if !ip.Valid { + ip = pqtype.Inet{ + IPNet: net.IPNet{ + IP: net.IPv4(127, 0, 0, 1), + Mask: net.IPv4Mask(255, 255, 255, 255), + }, + Valid: true, + } + } + key, err := db.InsertAPIKey(context.Background(), database.InsertAPIKeyParams{ ID: takeFirst(seed.ID, id), // 0 defaults to 86400 at the db layer LifetimeSeconds: takeFirst(seed.LifetimeSeconds, 0), HashedSecret: takeFirstSlice(seed.HashedSecret, hashed[:]), - IPAddress: pqtype.Inet{}, + IPAddress: ip, UserID: takeFirst(seed.UserID, uuid.New()), LastUsed: takeFirst(seed.LastUsed, database.Now()), ExpiresAt: takeFirst(seed.ExpiresAt, database.Now().Add(time.Hour)), diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 3ea7443a46850..5151aead8064c 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -28,6 +28,7 @@ type sqlcQuerier interface { AcquireProvisionerJob(ctx context.Context, arg AcquireProvisionerJobParams) (ProvisionerJob, error) DeleteAPIKeyByID(ctx context.Context, id string) error DeleteAPIKeysByUserID(ctx context.Context, userID uuid.UUID) error + DeleteApplicationConnectAPIKeysByUserID(ctx context.Context, userID uuid.UUID) error DeleteGitSSHKey(ctx context.Context, userID uuid.UUID) error DeleteGroupByID(ctx context.Context, id uuid.UUID) error DeleteGroupMemberFromGroup(ctx context.Context, arg DeleteGroupMemberFromGroupParams) error @@ -46,7 +47,7 @@ type sqlcQuerier interface { GetAPIKeysByUserID(ctx context.Context, arg GetAPIKeysByUserIDParams) ([]APIKey, error) GetAPIKeysLastUsedAfter(ctx context.Context, lastUsed time.Time) ([]APIKey, error) GetActiveUserCount(ctx context.Context) (int64, error) - GetAppSigningKey(ctx context.Context) (string, error) + GetAppSecurityKey(ctx context.Context) (string, error) // GetAuditLogsBefore retrieves `row_limit` number of audit logs before the provided // ID. GetAuditLogsOffset(ctx context.Context, arg GetAuditLogsOffsetParams) ([]GetAuditLogsOffsetRow, error) @@ -161,7 +162,6 @@ type sqlcQuerier interface { // for simplicity since all users is // every member of the org. InsertAllUsersGroup(ctx context.Context, organizationID uuid.UUID) (Group, error) - InsertAppSigningKey(ctx context.Context, value string) error InsertAuditLog(ctx context.Context, arg InsertAuditLogParams) (AuditLog, error) InsertDERPMeshKey(ctx context.Context, value string) error InsertDeploymentID(ctx context.Context, value string) error @@ -248,6 +248,7 @@ type sqlcQuerier interface { UpdateWorkspaceProxyDeleted(ctx context.Context, arg UpdateWorkspaceProxyDeletedParams) error UpdateWorkspaceTTL(ctx context.Context, arg UpdateWorkspaceTTLParams) error UpdateWorkspaceTTLToBeWithinTemplateMax(ctx context.Context, arg UpdateWorkspaceTTLToBeWithinTemplateMaxParams) error + UpsertAppSecurityKey(ctx context.Context, value string) error UpsertLastUpdateCheck(ctx context.Context, value string) error UpsertLogoURL(ctx context.Context, value string) error UpsertServiceBanner(ctx context.Context, value string) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 58cd4c23c17e2..3a8dda0fc4e39 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -17,8 +17,7 @@ import ( ) const deleteAPIKeyByID = `-- name: DeleteAPIKeyByID :exec -DELETE -FROM +DELETE FROM api_keys WHERE id = $1 @@ -41,6 +40,19 @@ func (q *sqlQuerier) DeleteAPIKeysByUserID(ctx context.Context, userID uuid.UUID return err } +const deleteApplicationConnectAPIKeysByUserID = `-- name: DeleteApplicationConnectAPIKeysByUserID :exec +DELETE FROM + api_keys +WHERE + user_id = $1 AND + scope = 'application_connect'::api_key_scope +` + +func (q *sqlQuerier) DeleteApplicationConnectAPIKeysByUserID(ctx context.Context, userID uuid.UUID) error { + _, err := q.db.ExecContext(ctx, deleteApplicationConnectAPIKeysByUserID, userID) + return err +} + const getAPIKeyByID = `-- name: GetAPIKeyByID :one SELECT id, hashed_secret, user_id, last_used, expires_at, created_at, updated_at, login_type, lifetime_seconds, ip_address, scope, token_name @@ -3202,12 +3214,12 @@ func (q *sqlQuerier) UpdateReplica(ctx context.Context, arg UpdateReplicaParams) return i, err } -const getAppSigningKey = `-- name: GetAppSigningKey :one +const getAppSecurityKey = `-- name: GetAppSecurityKey :one SELECT value FROM site_configs WHERE key = 'app_signing_key' ` -func (q *sqlQuerier) GetAppSigningKey(ctx context.Context) (string, error) { - row := q.db.QueryRowContext(ctx, getAppSigningKey) +func (q *sqlQuerier) GetAppSecurityKey(ctx context.Context) (string, error) { + row := q.db.QueryRowContext(ctx, getAppSecurityKey) var value string err := row.Scan(&value) return value, err @@ -3268,15 +3280,6 @@ func (q *sqlQuerier) GetServiceBanner(ctx context.Context) (string, error) { return value, err } -const insertAppSigningKey = `-- name: InsertAppSigningKey :exec -INSERT INTO site_configs (key, value) VALUES ('app_signing_key', $1) -` - -func (q *sqlQuerier) InsertAppSigningKey(ctx context.Context, value string) error { - _, err := q.db.ExecContext(ctx, insertAppSigningKey, value) - return err -} - const insertDERPMeshKey = `-- name: InsertDERPMeshKey :exec INSERT INTO site_configs (key, value) VALUES ('derp_mesh_key', $1) ` @@ -3295,6 +3298,16 @@ func (q *sqlQuerier) InsertDeploymentID(ctx context.Context, value string) error return err } +const upsertAppSecurityKey = `-- name: UpsertAppSecurityKey :exec +INSERT INTO site_configs (key, value) VALUES ('app_signing_key', $1) +ON CONFLICT (key) DO UPDATE set value = $1 WHERE site_configs.key = 'app_signing_key' +` + +func (q *sqlQuerier) UpsertAppSecurityKey(ctx context.Context, value string) error { + _, err := q.db.ExecContext(ctx, upsertAppSecurityKey, value) + return err +} + const upsertLastUpdateCheck = `-- name: UpsertLastUpdateCheck :exec INSERT INTO site_configs (key, value) VALUES ('last_update_check', $1) ON CONFLICT (key) DO UPDATE SET value = $1 WHERE site_configs.key = 'last_update_check' diff --git a/coderd/database/queries/apikeys.sql b/coderd/database/queries/apikeys.sql index 53caa7633521f..4ff77cb469cd5 100644 --- a/coderd/database/queries/apikeys.sql +++ b/coderd/database/queries/apikeys.sql @@ -66,12 +66,18 @@ WHERE id = $1; -- name: DeleteAPIKeyByID :exec -DELETE -FROM +DELETE FROM api_keys WHERE id = $1; +-- name: DeleteApplicationConnectAPIKeysByUserID :exec +DELETE FROM + api_keys +WHERE + user_id = $1 AND + scope = 'application_connect'::api_key_scope; + -- name: DeleteAPIKeysByUserID :exec DELETE FROM api_keys diff --git a/coderd/database/queries/siteconfig.sql b/coderd/database/queries/siteconfig.sql index 9f2e5042abdf3..1fb4a067fa0ad 100644 --- a/coderd/database/queries/siteconfig.sql +++ b/coderd/database/queries/siteconfig.sql @@ -31,8 +31,9 @@ ON CONFLICT (key) DO UPDATE SET value = $1 WHERE site_configs.key = 'logo_url'; -- name: GetLogoURL :one SELECT value FROM site_configs WHERE key = 'logo_url'; --- name: GetAppSigningKey :one +-- name: GetAppSecurityKey :one SELECT value FROM site_configs WHERE key = 'app_signing_key'; --- name: InsertAppSigningKey :exec -INSERT INTO site_configs (key, value) VALUES ('app_signing_key', $1); +-- name: UpsertAppSecurityKey :exec +INSERT INTO site_configs (key, value) VALUES ('app_signing_key', $1) +ON CONFLICT (key) DO UPDATE set value = $1 WHERE site_configs.key = 'app_signing_key'; diff --git a/coderd/userauth.go b/coderd/userauth.go index b3a18b1c52ac2..82c020af1b066 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -195,48 +195,17 @@ func (api *API) postLogout(rw http.ResponseWriter, r *http.Request) { return } - // Deployments should not host app tokens on the same domain as the - // primary deployment. But in the case they are, we should also delete this - // token. - if appCookie, _ := r.Cookie(codersdk.DevURLSessionTokenCookie); appCookie != nil { - appCookieRemove := &http.Cookie{ - // MaxAge < 0 means to delete the cookie now. - MaxAge: -1, - Name: codersdk.DevURLSessionTokenCookie, - Path: "/", - Domain: "." + api.AccessURL.Hostname(), - } - http.SetCookie(rw, appCookieRemove) - - id, _, err := httpmw.SplitAPIToken(appCookie.Value) - if err == nil { - err = api.Database.DeleteAPIKeyByID(ctx, id) - if err != nil { - // Don't block logout, just log any errors. - api.Logger.Warn(r.Context(), "failed to delete devurl token on logout", - slog.Error(err), - slog.F("id", id), - ) - } - } - } - - // This code should be removed after Jan 1 2023. - // This code logs out of the old session cookie before we renamed it - // if it is a valid coder token. Otherwise, this old cookie hangs around - // and we never log out of the user. - oldCookie, err := r.Cookie("session_token") - if err == nil && oldCookie != nil { - _, _, err := httpmw.SplitAPIToken(oldCookie.Value) - if err == nil { - cookie := &http.Cookie{ - // MaxAge < 0 means to delete the cookie now. - MaxAge: -1, - Name: "session_token", - Path: "/", - } - http.SetCookie(rw, cookie) - } + // Invalidate all subdomain app tokens. This saves us from having to + // track which app tokens are associated which this browser session and + // doesn't inconvenience the user as they'll just get redirected if they try + // to access the app again. + err = api.Database.DeleteApplicationConnectAPIKeysByUserID(ctx, apiKey.UserID) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error deleting app tokens.", + Detail: err.Error(), + }) + return } aReq.New = database.APIKey{} diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go index 54c99b933c714..60d19ecb26d48 100644 --- a/coderd/userauth_test.go +++ b/coderd/userauth_test.go @@ -22,6 +22,8 @@ import ( "github.com/coder/coder/coderd/audit" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/dbgen" + "github.com/coder/coder/coderd/database/dbtestutil" "github.com/coder/coder/codersdk" "github.com/coder/coder/testutil" ) @@ -829,6 +831,110 @@ func TestUserOIDC(t *testing.T) { }) } +func TestUserLogout(t *testing.T) { + t.Parallel() + + // Create a custom database so it's easier to make scoped tokens for + // testing. + db, pubSub := dbtestutil.NewDB(t) + + client := coderdtest.New(t, &coderdtest.Options{ + Database: db, + Pubsub: pubSub, + }) + firstUser := coderdtest.CreateFirstUser(t, client) + + ctx := testutil.Context(t, testutil.WaitLong) + + // Create a user with built-in auth. + const ( + email = "dean.was.here@test.coder.com" + username = "dean" + //nolint:gosec + password = "SomeSecurePassword123!" + ) + newUser, err := client.CreateUser(ctx, codersdk.CreateUserRequest{ + Email: email, + Username: username, + Password: password, + OrganizationID: firstUser.OrganizationID, + }) + require.NoError(t, err) + + // Log in with basic auth and keep the the session token (but don't use it). + userClient := codersdk.New(client.URL) + loginRes1, err := userClient.LoginWithPassword(ctx, codersdk.LoginWithPasswordRequest{ + Email: email, + Password: password, + }) + require.NoError(t, err) + + // Log in again but actually set the token this time. + loginRes2, err := userClient.LoginWithPassword(ctx, codersdk.LoginWithPasswordRequest{ + Email: email, + Password: password, + }) + require.NoError(t, err) + userClient.SetSessionToken(loginRes2.SessionToken) + + // Add the user's second session token to the list of API keys that should + // be deleted. + shouldBeDeleted := map[string]string{ + "user login 2 (logging out with this)": loginRes2.SessionToken, + } + + // Add the user's first token, and the admin's session token to the list of + // API keys that should not be deleted. + shouldNotBeDeleted := map[string]string{ + "user login 1 (not logging out of)": loginRes1.SessionToken, + "admin login": client.SessionToken(), + } + + // Create a few application_connect-scoped API keys that should be deleted. + for i := 0; i < 3; i++ { + key, _ := dbgen.APIKey(t, db, database.APIKey{ + UserID: newUser.ID, + Scope: database.APIKeyScopeApplicationConnect, + }) + shouldBeDeleted[fmt.Sprintf("application_connect key owned by logout user %d", i)] = key.ID + } + + // Create a few application_connect-scoped API keys for the admin user that + // should not be deleted. + for i := 0; i < 3; i++ { + key, _ := dbgen.APIKey(t, db, database.APIKey{ + UserID: firstUser.UserID, + Scope: database.APIKeyScopeApplicationConnect, + }) + shouldNotBeDeleted[fmt.Sprintf("application_connect key owned by admin user %d", i)] = key.ID + } + + // Log out of the new user. + err = userClient.Logout(ctx) + require.NoError(t, err) + + // Ensure the new user's session token is no longer valid. + _, err = userClient.User(ctx, codersdk.Me) + require.Error(t, err) + var sdkErr *codersdk.Error + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusUnauthorized, sdkErr.StatusCode()) + + // Check that the deleted keys are gone. + for name, id := range shouldBeDeleted { + id := strings.Split(id, "-")[0] + _, err := db.GetAPIKeyByID(ctx, id) + require.Error(t, err, name) + } + + // Check that the other keys are still there. + for name, id := range shouldNotBeDeleted { + id := strings.Split(id, "-")[0] + _, err := db.GetAPIKeyByID(ctx, id) + require.NoError(t, err, name) + } +} + func oauth2Callback(t *testing.T, client *codersdk.Client) *http.Response { client.HTTPClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { return http.ErrUseLastResponse diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index 6ce14dad7689e..be7a93d511808 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -30,7 +30,6 @@ import ( "tailscale.com/tailcfg" "cdr.dev/slog" - "github.com/coder/coder/agent" "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/database/dbauthz" "github.com/coder/coder/coderd/gitauth" @@ -38,7 +37,6 @@ import ( "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/coderd/tracing" - "github.com/coder/coder/coderd/workspaceapps" "github.com/coder/coder/codersdk" "github.com/coder/coder/codersdk/agentsdk" "github.com/coder/coder/tailnet" @@ -546,83 +544,6 @@ func (api *API) workspaceAgentStartupLogs(rw http.ResponseWriter, r *http.Reques } } -// workspaceAgentPTY spawns a PTY and pipes it over a WebSocket. -// This is used for the web terminal. -// -// @Summary Open PTY to workspace agent -// @ID open-pty-to-workspace-agent -// @Security CoderSessionToken -// @Tags Agents -// @Param workspaceagent path string true "Workspace agent ID" format(uuid) -// @Success 101 -// @Router /workspaceagents/{workspaceagent}/pty [get] -func (api *API) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) { - ctx := r.Context() - - api.WebsocketWaitMutex.Lock() - api.WebsocketWaitGroup.Add(1) - api.WebsocketWaitMutex.Unlock() - defer api.WebsocketWaitGroup.Done() - - appToken, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, workspaceapps.Request{ - AccessMethod: workspaceapps.AccessMethodTerminal, - BasePath: r.URL.Path, - AgentNameOrID: chi.URLParam(r, "workspaceagent"), - }) - if !ok { - return - } - - reconnect, err := uuid.Parse(r.URL.Query().Get("reconnect")) - if err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Query param 'reconnect' must be a valid UUID.", - Validations: []codersdk.ValidationError{ - {Field: "reconnect", Detail: "invalid UUID"}, - }, - }) - return - } - height, err := strconv.ParseUint(r.URL.Query().Get("height"), 10, 16) - if err != nil { - height = 80 - } - width, err := strconv.ParseUint(r.URL.Query().Get("width"), 10, 16) - if err != nil { - width = 80 - } - - conn, err := websocket.Accept(rw, r, &websocket.AcceptOptions{ - CompressionMode: websocket.CompressionDisabled, - }) - if err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Failed to accept websocket.", - Detail: err.Error(), - }) - return - } - - ctx, wsNetConn := websocketNetConn(ctx, conn, websocket.MessageBinary) - defer wsNetConn.Close() // Also closes conn. - - go httpapi.Heartbeat(ctx, conn) - - agentConn, release, err := api.workspaceAgentCache.Acquire(appToken.AgentID) - if err != nil { - _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("dial workspace agent: %s", err)) - return - } - defer release() - ptNetConn, err := agentConn.ReconnectingPTY(ctx, reconnect, uint16(height), uint16(width), r.URL.Query().Get("command")) - if err != nil { - _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("dial: %s", err)) - return - } - defer ptNetConn.Close() - agent.Bicopy(ctx, wsNetConn, ptNetConn) -} - // @Summary Get listening ports for workspace agent // @ID get-listening-ports-for-workspace-agent // @Security CoderSessionToken diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index 3ff6268e02b0e..df6e29f5159d5 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -1,68 +1,19 @@ package coderd import ( - "bytes" - "context" - "crypto/sha256" - "crypto/subtle" - "database/sql" - "encoding/base64" - "encoding/json" "fmt" "net/http" - "net/http/httputil" "net/url" - "strconv" - "strings" "time" - "github.com/go-chi/chi/v5" - "go.opentelemetry.io/otel/trace" - "golang.org/x/xerrors" - jose "gopkg.in/square/go-jose.v2" - - "cdr.dev/slog" "github.com/coder/coder/coderd/database" - "github.com/coder/coder/coderd/database/dbauthz" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/coderd/rbac" - "github.com/coder/coder/coderd/tracing" "github.com/coder/coder/coderd/workspaceapps" "github.com/coder/coder/codersdk" - "github.com/coder/coder/site" -) - -const ( - // This needs to be a super unique query parameter because we don't want to - // conflict with query parameters that users may use. - //nolint:gosec - subdomainProxyAPIKeyParam = "coder_application_connect_api_key_35e783" - // 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 -// should send to the app in the workspace. Some headers (such as the websocket -// upgrade headers from RFC 6455) are not canonical according to the HTTP/1 -// spec. Golang has said that they will not add custom cases for these headers, -// so we need to do it ourselves. -// -// Some apps our customers use are sensitive to the case of these headers. -// -// https://github.com/golang/go/issues/18495 -var nonCanonicalHeaders = map[string]string{ - "Sec-Websocket-Accept": "Sec-WebSocket-Accept", - "Sec-Websocket-Extensions": "Sec-WebSocket-Extensions", - "Sec-Websocket-Key": "Sec-WebSocket-Key", - "Sec-Websocket-Protocol": "Sec-WebSocket-Protocol", - "Sec-Websocket-Version": "Sec-WebSocket-Version", -} - // @Summary Get applications host // @ID get-applications-host // @Security CoderSessionToken @@ -81,194 +32,6 @@ func (api *API) appHost(rw http.ResponseWriter, r *http.Request) { }) } -// workspaceAppsProxyPath proxies requests to a workspace application -// through a relative URL path. -func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) { - if api.DeploymentValues.DisablePathApps.Value() { - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusUnauthorized, - Title: "Unauthorized", - Description: "Path-based applications are disabled on this Coder deployment by the administrator.", - RetryEnabled: false, - DashboardURL: api.AccessURL.String(), - }) - return - } - - // If the username in the request is @me, then redirect to the current - // username. The resolveWorkspaceApp function does not accept @me for - // security purposes. - if chi.URLParam(r, "user") == codersdk.Me { - _, roles, ok := httpmw.ExtractAPIKey(rw, r, httpmw.ExtractAPIKeyConfig{ - DB: api.Database, - OAuth2Configs: &httpmw.OAuth2Configs{ - Github: api.GithubOAuth2Config, - OIDC: api.OIDCConfig, - }, - RedirectToLogin: true, - DisableSessionExpiryRefresh: api.DeploymentValues.DisableSessionExpiryRefresh.Value(), - }) - if !ok { - return - } - - http.Redirect(rw, r, strings.Replace(r.URL.Path, "@me", "@"+roles.Username, 1), http.StatusTemporaryRedirect) - return - } - - // Determine the real path that was hit. The * URL parameter in Chi will not - // include the leading slash if it was present, so we need to add it back. - chiPath := chi.URLParam(r, "*") - basePath := strings.TrimSuffix(r.URL.Path, chiPath) - if strings.HasSuffix(basePath, "/") { - chiPath = "/" + chiPath - } - - token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, workspaceapps.Request{ - AccessMethod: workspaceapps.AccessMethodPath, - BasePath: basePath, - UsernameOrID: chi.URLParam(r, "user"), - WorkspaceAndAgent: chi.URLParam(r, "workspace_and_agent"), - // We don't support port proxying on paths. The ResolveRequest method - // won't allow port proxying on path-based apps if the app is a number. - AppSlugOrPort: chi.URLParam(r, "workspaceapp"), - }) - if !ok { - return - } - - api.proxyWorkspaceApplication(rw, r, *token, chiPath) -} - -// handleSubdomainApplications handles subdomain-based application proxy -// requests (aka. DevURLs in Coder V1). -// -// There are a lot of paths here: -// 1. If api.AppHostname is not set then we pass on. -// 2. If we can't read the request hostname then we return a 400. -// 3. If the request hostname matches api.AccessURL then we pass on. -// 5. We split the subdomain into the subdomain and the "rest". If there are no -// periods in the hostname then we pass on. -// 5. We parse the subdomain into a httpapi.ApplicationURL struct. If we -// encounter an error: -// a. If the "rest" does not match api.AppHostname then we pass on; -// b. Otherwise, we return a 400. -// 6. Finally, we verify that the "rest" matches api.AppHostname, else we -// return a 404. -// -// Rationales for each of the above steps: -// 1. We pass on if api.AppHostname is not set to avoid returning any errors if -// `--app-hostname` is not configured. -// 2. Every request should have a valid Host header anyways. -// 3. We pass on if the request hostname matches api.AccessURL so we can -// support having the access URL be at the same level as the application -// base hostname. -// 4. We pass on if there are no periods in the hostname as application URLs -// must be a subdomain of a hostname, which implies there must be at least -// one period. -// 5. a. If the request subdomain is not a valid application URL, and the -// "rest" does not match api.AppHostname, then it is very unlikely that -// the request was intended for this handler. We pass on. -// b. If the request subdomain is not a valid application URL, but the -// "rest" matches api.AppHostname, then we return a 400 because the -// request is probably a typo or something. -// 6. We finally verify that the "rest" matches api.AppHostname for security -// purposes regarding re-authentication and application proxy session -// tokens. -func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) http.Handler) func(http.Handler) http.Handler { - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { - ctx := r.Context() - - // Step 1: Pass on if subdomain-based application proxying is not - // configured. - if api.AppHostname == "" || api.AppHostnameRegex == nil { - next.ServeHTTP(rw, r) - return - } - - // Step 2: Get the request Host. - host := httpapi.RequestHost(r) - if host == "" { - if r.URL.Path == "/derp" { - // The /derp endpoint is used by wireguard clients to tunnel - // through coderd. For some reason these requests don't set - // a Host header properly sometimes in tests (no idea how), - // which causes this path to get hit. - next.ServeHTTP(rw, r) - return - } - - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Could not determine request Host.", - }) - return - } - - // Steps 3-6: Parse application from subdomain. - app, ok := api.parseWorkspaceApplicationHostname(rw, r, next, host) - if !ok { - return - } - - // If the request has the special query param then we need to set a - // cookie and strip that query parameter. - if encryptedAPIKey := r.URL.Query().Get(subdomainProxyAPIKeyParam); encryptedAPIKey != "" { - // Exchange the encoded API key for a real one. - _, token, err := decryptAPIKey(r.Context(), api.Database, encryptedAPIKey) - if err != nil { - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusBadRequest, - Title: "Bad Request", - Description: "Could not decrypt API key. Please remove the query parameter and try again.", - // Retry is disabled because the user needs to remove - // the query parameter before they try again. - RetryEnabled: false, - DashboardURL: api.AccessURL.String(), - }) - return - } - - api.setWorkspaceAppCookie(rw, r, token) - - // Strip the query parameter. - path := r.URL.Path - if path == "" { - path = "/" - } - q := r.URL.Query() - q.Del(subdomainProxyAPIKeyParam) - rawQuery := q.Encode() - if rawQuery != "" { - path += "?" + q.Encode() - } - - http.Redirect(rw, r, path, http.StatusTemporaryRedirect) - return - } - - token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, workspaceapps.Request{ - AccessMethod: workspaceapps.AccessMethodSubdomain, - BasePath: "/", - UsernameOrID: app.Username, - WorkspaceNameOrID: app.WorkspaceName, - AgentNameOrID: app.AgentName, - AppSlugOrPort: app.AppSlugOrPort, - }) - if !ok { - return - } - - // Use the passed in app middlewares before passing to the proxy - // app. - mws := chi.Middlewares(middlewares) - mws.Handler(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { - api.proxyWorkspaceApplication(rw, r, *token, r.URL.Path) - })).ServeHTTP(rw, r.WithContext(ctx)) - }) - } -} - // workspaceApplicationAuth is an endpoint on the main router that handles // redirects from the subdomain handler. // @@ -318,7 +81,7 @@ func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request // security purposes. u.Scheme = api.AccessURL.Scheme - // Ensure that the redirect URI is a subdomain of api.AppHostname and is a + // Ensure that the redirect URI is a subdomain of api.Hostname and is a // valid app subdomain. subdomain, ok := httpapi.ExecuteHostnamePattern(api.AppHostnameRegex, u.Host) if !ok { @@ -360,7 +123,7 @@ func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request } // Encrypt the API key. - encryptedAPIKey, err := encryptAPIKey(encryptedAPIKeyPayload{ + encryptedAPIKey, err := api.AppSecurityKey.EncryptAPIKey(workspaceapps.EncryptedAPIKeyPayload{ APIKey: cookie.Value, }) if err != nil { @@ -374,424 +137,7 @@ func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request // Redirect to the redirect URI with the encrypted API key in the query // parameters. q := u.Query() - q.Set(subdomainProxyAPIKeyParam, encryptedAPIKey) + q.Set(workspaceapps.SubdomainProxyAPIKeyParam, encryptedAPIKey) u.RawQuery = q.Encode() http.Redirect(rw, r, u.String(), http.StatusTemporaryRedirect) } - -func (api *API) parseWorkspaceApplicationHostname(rw http.ResponseWriter, r *http.Request, next http.Handler, host string) (httpapi.ApplicationURL, bool) { - // Check if the hostname matches the access URL. If it does, the user was - // definitely trying to connect to the dashboard/API. - if httpapi.HostnamesMatch(api.AccessURL.Hostname(), host) { - next.ServeHTTP(rw, r) - return httpapi.ApplicationURL{}, false - } - - // If there are no periods in the hostname, then it can't be a valid - // application URL. - if !strings.Contains(host, ".") { - next.ServeHTTP(rw, r) - return httpapi.ApplicationURL{}, false - } - - // Split the subdomain so we can parse the application details and verify it - // matches the configured app hostname later. - subdomain, ok := httpapi.ExecuteHostnamePattern(api.AppHostnameRegex, host) - if !ok { - // Doesn't match the regex, so it's not a valid application URL. - next.ServeHTTP(rw, r) - return httpapi.ApplicationURL{}, false - } - - // Check if the request is part of a logout flow. - if subdomain == appLogoutHostname { - api.handleWorkspaceSubdomainAppLogout(rw, r) - return httpapi.ApplicationURL{}, false - } - - // Parse the application URL from the subdomain. - app, err := httpapi.ParseSubdomainAppURL(subdomain) - if err != nil { - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusBadRequest, - Title: "Invalid application URL", - Description: fmt.Sprintf("Could not parse subdomain application URL %q: %s", subdomain, err.Error()), - RetryEnabled: false, - DashboardURL: api.AccessURL.String(), - }) - return httpapi.ApplicationURL{}, false - } - - return app, true -} - -func (api *API) handleWorkspaceSubdomainAppLogout(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(codersdk.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. - //nolint:gocritic // needed for workspace app logout - apiKey, err := api.Database.GetAPIKeyByID(dbauthz.AsSystemRestricted(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 - } - //nolint:gocritic // needed for workspace app logout - err = api.Database.DeleteAPIKeyByID(dbauthz.AsSystemRestricted(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(workspaceapps.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) -} - -// 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: codersdk.DevURLSessionTokenCookie, - Value: token, - Domain: cookieHost, - Path: "/", - MaxAge: maxAge, - HttpOnly: true, - SameSite: http.SameSiteLaxMode, - Secure: api.SecureAuthCookie, - }) - - return true -} - -func (api *API) proxyWorkspaceApplication(rw http.ResponseWriter, r *http.Request, appToken workspaceapps.SignedToken, path string) { - ctx := r.Context() - - // Filter IP headers from untrusted origins. - httpmw.FilterUntrustedOriginHeaders(api.RealIPConfig, r) - // Ensure proper IP headers get sent to the forwarded application. - err := httpmw.EnsureXForwardedForHeader(r) - if err != nil { - httpapi.InternalServerError(rw, err) - return - } - - appURL, err := url.Parse(appToken.AppURL) - if err != nil { - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusBadRequest, - Title: "Bad Request", - Description: fmt.Sprintf("Application has an invalid URL %q: %s", appToken.AppURL, err.Error()), - RetryEnabled: true, - DashboardURL: api.AccessURL.String(), - }) - return - } - - // Verify that the port is allowed. See the docs above - // `codersdk.MinimumListeningPort` for more details. - port := appURL.Port() - if port != "" { - portInt, err := strconv.Atoi(port) - if err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: fmt.Sprintf("App URL %q has an invalid port %q.", appToken.AppURL, port), - Detail: err.Error(), - }) - return - } - - if portInt < codersdk.WorkspaceAgentMinimumListeningPort { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: fmt.Sprintf("Application port %d is not permitted. Coder reserves ports less than %d for internal use.", portInt, codersdk.WorkspaceAgentMinimumListeningPort), - }) - return - } - } - - // Ensure path and query parameter correctness. - if path == "" { - // Web applications typically request paths relative to the - // root URL. This allows for routing behind a proxy or subpath. - // See https://github.com/coder/code-server/issues/241 for examples. - http.Redirect(rw, r, r.URL.Path+"/", http.StatusTemporaryRedirect) - return - } - if path == "/" && r.URL.RawQuery == "" && appURL.RawQuery != "" { - // If the application defines a default set of query parameters, - // we should always respect them. The reverse proxy will merge - // query parameters for server-side requests, but sometimes - // client-side applications require the query parameters to render - // properly. With code-server, this is the "folder" param. - r.URL.RawQuery = appURL.RawQuery - http.Redirect(rw, r, r.URL.String(), http.StatusTemporaryRedirect) - return - } - - r.URL.Path = path - appURL.RawQuery = "" - - proxy := httputil.NewSingleHostReverseProxy(appURL) - proxy.ErrorHandler = func(w http.ResponseWriter, r *http.Request, err error) { - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusBadGateway, - Title: "Bad Gateway", - Description: "Failed to proxy request to application: " + err.Error(), - RetryEnabled: true, - DashboardURL: api.AccessURL.String(), - }) - } - - conn, release, err := api.workspaceAgentCache.Acquire(appToken.AgentID) - if err != nil { - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusBadGateway, - Title: "Bad Gateway", - Description: "Could not connect to workspace agent: " + err.Error(), - RetryEnabled: true, - DashboardURL: api.AccessURL.String(), - }) - return - } - defer release() - proxy.Transport = conn.HTTPTransport() - - // This strips the session token from a workspace app request. - cookieHeaders := r.Header.Values("Cookie")[:] - r.Header.Del("Cookie") - for _, cookieHeader := range cookieHeaders { - r.Header.Add("Cookie", httpapi.StripCoderCookies(cookieHeader)) - } - - // Convert canonicalized headers to their non-canonicalized counterparts. - // See the comment on `nonCanonicalHeaders` for more information on why this - // is necessary. - for k, v := range r.Header { - if n, ok := nonCanonicalHeaders[k]; ok { - r.Header.Del(k) - r.Header[n] = v - } - } - - // end span so we don't get long lived trace data - tracing.EndHTTPSpan(r, http.StatusOK, trace.SpanFromContext(ctx)) - - proxy.ServeHTTP(rw, r) -} - -type encryptedAPIKeyPayload struct { - APIKey string `json:"api_key"` - ExpiresAt time.Time `json:"expires_at"` -} - -// encryptAPIKey encrypts an API key with it's own hashed secret. This is used -// for smuggling (application_connect scoped) API keys securely to app -// hostnames. -// -// We encrypt API keys when smuggling them in query parameters to avoid them -// getting accidentally logged in access logs or stored in browser history. -func encryptAPIKey(data encryptedAPIKeyPayload) (string, error) { - if data.APIKey == "" { - return "", xerrors.New("API key is empty") - } - if data.ExpiresAt.IsZero() { - // Very short expiry as these keys are only used once as part of an - // automatic redirection flow. - data.ExpiresAt = database.Now().Add(time.Minute) - } - - payload, err := json.Marshal(data) - if err != nil { - return "", xerrors.Errorf("marshal payload: %w", err) - } - - // We use the hashed key secret as the encryption key. The hashed secret is - // stored in the API keys table. The HashedSecret is NEVER returned from the - // API. - // - // We chose to use the key secret as the private key for encryption instead - // of a shared key for a few reasons: - // 1. A single private key used to encrypt every API key would also be - // stored in the database, which means that the risk factor is similar. - // 2. The secret essentially rotates for each key (for free!), since each - // key has a different secret. This means that if someone acquires an - // old database dump they can't decrypt new API keys. - // 3. These tokens are scoped only for application_connect access. - keyID, keySecret, err := httpmw.SplitAPIToken(data.APIKey) - if err != nil { - return "", xerrors.Errorf("split API key: %w", err) - } - // SHA256 the key secret so it matches the hashed secret in the database. - // The key length doesn't matter to the jose.Encrypter. - privateKey := sha256.Sum256([]byte(keySecret)) - - // JWEs seem to apply a nonce themselves. - encrypter, err := jose.NewEncrypter( - jose.A256GCM, - jose.Recipient{ - Algorithm: jose.A256GCMKW, - KeyID: keyID, - Key: privateKey[:], - }, - &jose.EncrypterOptions{ - Compression: jose.DEFLATE, - }, - ) - if err != nil { - return "", xerrors.Errorf("initializer jose encrypter: %w", err) - } - encryptedObject, err := encrypter.Encrypt(payload) - if err != nil { - return "", xerrors.Errorf("encrypt jwe: %w", err) - } - - encrypted := encryptedObject.FullSerialize() - return base64.RawURLEncoding.EncodeToString([]byte(encrypted)), nil -} - -// decryptAPIKey undoes encryptAPIKey and is used in the subdomain app handler. -func decryptAPIKey(ctx context.Context, db database.Store, encryptedAPIKey string) (database.APIKey, string, error) { - encrypted, err := base64.RawURLEncoding.DecodeString(encryptedAPIKey) - if err != nil { - return database.APIKey{}, "", xerrors.Errorf("base64 decode encrypted API key: %w", err) - } - - object, err := jose.ParseEncrypted(string(encrypted)) - if err != nil { - return database.APIKey{}, "", xerrors.Errorf("parse encrypted API key: %w", err) - } - - // Lookup the API key so we can decrypt it. - keyID := object.Header.KeyID - //nolint:gocritic // needed to check API key - key, err := db.GetAPIKeyByID(dbauthz.AsSystemRestricted(ctx), keyID) - if err != nil { - return database.APIKey{}, "", xerrors.Errorf("get API key by key ID: %w", err) - } - - // Decrypt using the hashed secret. - decrypted, err := object.Decrypt(key.HashedSecret) - if err != nil { - return database.APIKey{}, "", xerrors.Errorf("decrypt API key: %w", err) - } - - // Unmarshal the payload. - var payload encryptedAPIKeyPayload - if err := json.Unmarshal(decrypted, &payload); err != nil { - return database.APIKey{}, "", xerrors.Errorf("unmarshal decrypted payload: %w", err) - } - - // Validate expiry. - if payload.ExpiresAt.Before(database.Now()) { - return database.APIKey{}, "", xerrors.New("encrypted API key expired") - } - - // Validate that the key matches the one we got from the DB. - gotKeyID, gotKeySecret, err := httpmw.SplitAPIToken(payload.APIKey) - if err != nil { - return database.APIKey{}, "", xerrors.Errorf("split API key: %w", err) - } - gotHashedSecret := sha256.Sum256([]byte(gotKeySecret)) - if gotKeyID != key.ID || !bytes.Equal(key.HashedSecret, gotHashedSecret[:]) { - return database.APIKey{}, "", xerrors.New("encrypted API key does not match key in database") - } - - return key, payload.APIKey, nil -} diff --git a/coderd/workspaceapps/db.go b/coderd/workspaceapps/db.go index be00a6c6a1f30..10f9be43afced 100644 --- a/coderd/workspaceapps/db.go +++ b/coderd/workspaceapps/db.go @@ -32,16 +32,12 @@ type DBTokenProvider struct { DeploymentValues *codersdk.DeploymentValues OAuth2Configs *httpmw.OAuth2Configs WorkspaceAgentInactiveTimeout time.Duration - TokenSigningKey []byte + SigningKey SecurityKey } var _ SignedTokenProvider = &DBTokenProvider{} -func NewDBTokenProvider(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database.Store, cfg *codersdk.DeploymentValues, oauth2Cfgs *httpmw.OAuth2Configs, workspaceAgentInactiveTimeout time.Duration, tokenSigningKey []byte) SignedTokenProvider { - if len(tokenSigningKey) != 64 { - panic("token signing key must be 64 bytes") - } - +func NewDBTokenProvider(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database.Store, cfg *codersdk.DeploymentValues, oauth2Cfgs *httpmw.OAuth2Configs, workspaceAgentInactiveTimeout time.Duration, signingKey SecurityKey) SignedTokenProvider { if workspaceAgentInactiveTimeout == 0 { workspaceAgentInactiveTimeout = 1 * time.Minute } @@ -54,7 +50,7 @@ func NewDBTokenProvider(log slog.Logger, accessURL *url.URL, authz rbac.Authoriz DeploymentValues: cfg, OAuth2Configs: oauth2Cfgs, WorkspaceAgentInactiveTimeout: workspaceAgentInactiveTimeout, - TokenSigningKey: tokenSigningKey, + SigningKey: signingKey, } } @@ -62,7 +58,7 @@ func (p *DBTokenProvider) TokenFromRequest(r *http.Request) (*SignedToken, bool) // Get the existing token from the request. tokenCookie, err := r.Cookie(codersdk.DevURLSignedAppTokenCookie) if err == nil { - token, err := ParseToken(p.TokenSigningKey, tokenCookie.Value) + token, err := p.SigningKey.VerifySignedToken(tokenCookie.Value) if err == nil { req := token.Request.Normalize() err := req.Validate() @@ -130,9 +126,6 @@ func (p *DBTokenProvider) CreateToken(ctx context.Context, rw http.ResponseWrite token.AgentID = dbReq.Agent.ID token.AppURL = dbReq.AppURL - // TODO(@deansheather): return an error if the agent is offline or the app - // is not running. - // Verify the user has access to the app. authed, err := p.authorizeRequest(r.Context(), authz, dbReq) if err != nil { @@ -150,7 +143,8 @@ func (p *DBTokenProvider) CreateToken(ctx context.Context, rw http.ResponseWrite // and they aren't signed in. switch appReq.AccessMethod { case AccessMethodPath: - // TODO(@deansheather): this doesn't work on moons + // TODO(@deansheather): this doesn't work on moons so will need to + // be updated to include the access URL as a param httpmw.RedirectToLogin(rw, r, httpmw.SignedOutErrorMessage) case AccessMethodSubdomain: // Redirect to the app auth redirect endpoint with a valid redirect @@ -195,7 +189,7 @@ func (p *DBTokenProvider) CreateToken(ctx context.Context, rw http.ResponseWrite // Sign the token. token.Expiry = time.Now().Add(DefaultTokenExpiry) - tokenStr, err := GenerateToken(p.TokenSigningKey, token) + tokenStr, err := p.SigningKey.SignToken(token) if err != nil { WriteWorkspaceApp500(p.Logger, p.AccessURL, rw, r, &appReq, err, "generate token") return nil, "", false diff --git a/coderd/workspaceapps/db_test.go b/coderd/workspaceapps/db_test.go index f566c5c838e0b..6403eaf8d1633 100644 --- a/coderd/workspaceapps/db_test.go +++ b/coderd/workspaceapps/db_test.go @@ -263,7 +263,7 @@ func Test_ResolveRequest(t *testing.T) { require.Equal(t, codersdk.DevURLSignedAppTokenCookie, cookie.Name) require.Equal(t, req.BasePath, cookie.Path) - parsedToken, err := workspaceapps.ParseToken(api.AppSigningKey, cookie.Value) + parsedToken, err := api.AppSecurityKey.VerifySignedToken(cookie.Value) require.NoError(t, err) // normalize expiry require.WithinDuration(t, token.Expiry, parsedToken.Expiry, 2*time.Second) @@ -482,7 +482,7 @@ func Test_ResolveRequest(t *testing.T) { AgentID: agentID, AppURL: appURL, } - badTokenStr, err := workspaceapps.GenerateToken(api.AppSigningKey, badToken) + badTokenStr, err := api.AppSecurityKey.SignToken(badToken) require.NoError(t, err) req := workspaceapps.Request{ @@ -518,7 +518,7 @@ func Test_ResolveRequest(t *testing.T) { require.Len(t, cookies, 1) require.Equal(t, cookies[0].Name, codersdk.DevURLSignedAppTokenCookie) require.NotEqual(t, cookies[0].Value, badTokenStr) - parsedToken, err := workspaceapps.ParseToken(api.AppSigningKey, cookies[0].Value) + parsedToken, err := api.AppSecurityKey.VerifySignedToken(cookies[0].Value) require.NoError(t, err) require.Equal(t, appNameOwner, parsedToken.AppSlugOrPort) }) diff --git a/coderd/workspaceapps/provider.go b/coderd/workspaceapps/provider.go index 7b363ebc909d9..a8c0ce1ad449d 100644 --- a/coderd/workspaceapps/provider.go +++ b/coderd/workspaceapps/provider.go @@ -19,14 +19,14 @@ const ( RedirectURIQueryParam = "redirect_uri" ) -// ResolveRequest calls TokenProvider to use an existing signed app token in the +// ResolveRequest calls SignedTokenProvider to use an existing signed app token in the // request or issue a new one. If it returns a newly minted token, it sets the // cookie for you. -func ResolveRequest(log slog.Logger, accessURL *url.URL, p SignedTokenProvider, rw http.ResponseWriter, r *http.Request, appReq Request) (*SignedToken, bool) { +func ResolveRequest(log slog.Logger, dashboardURL *url.URL, p SignedTokenProvider, rw http.ResponseWriter, r *http.Request, appReq Request) (*SignedToken, bool) { appReq = appReq.Normalize() err := appReq.Validate() if err != nil { - WriteWorkspaceApp500(log, accessURL, rw, r, &appReq, err, "invalid app request") + WriteWorkspaceApp500(log, dashboardURL, rw, r, &appReq, err, "invalid app request") return nil, false } diff --git a/coderd/workspaceapps/proxy.go b/coderd/workspaceapps/proxy.go new file mode 100644 index 0000000000000..7189d19ff9a3b --- /dev/null +++ b/coderd/workspaceapps/proxy.go @@ -0,0 +1,620 @@ +package workspaceapps + +import ( + "context" + "fmt" + "net" + "net/http" + "net/http/httputil" + "net/url" + "regexp" + "strconv" + "strings" + "sync" + + "github.com/go-chi/chi/v5" + "github.com/google/uuid" + "go.opentelemetry.io/otel/trace" + "nhooyr.io/websocket" + + "cdr.dev/slog" + "github.com/coder/coder/agent" + "github.com/coder/coder/coderd/httpapi" + "github.com/coder/coder/coderd/httpmw" + "github.com/coder/coder/coderd/tracing" + "github.com/coder/coder/coderd/wsconncache" + "github.com/coder/coder/codersdk" + "github.com/coder/coder/site" +) + +const ( + // This needs to be a super unique query parameter because we don't want to + // conflict with query parameters that users may use. + //nolint:gosec + SubdomainProxyAPIKeyParam = "coder_application_connect_api_key_35e783" + // 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. + // + // DEPRECATED: we no longer use this, but we still redirect from it to the + // main login page. + appLogoutHostname = "coder-logout" +) + +// nonCanonicalHeaders is a map from "canonical" headers to the actual header we +// should send to the app in the workspace. Some headers (such as the websocket +// upgrade headers from RFC 6455) are not canonical according to the HTTP/1 +// spec. Golang has said that they will not add custom cases for these headers, +// so we need to do it ourselves. +// +// Some apps our customers use are sensitive to the case of these headers. +// +// https://github.com/golang/go/issues/18495 +var nonCanonicalHeaders = map[string]string{ + "Sec-Websocket-Accept": "Sec-WebSocket-Accept", + "Sec-Websocket-Extensions": "Sec-WebSocket-Extensions", + "Sec-Websocket-Key": "Sec-WebSocket-Key", + "Sec-Websocket-Protocol": "Sec-WebSocket-Protocol", + "Sec-Websocket-Version": "Sec-WebSocket-Version", +} + +// Server serves workspace apps endpoints, including: +// - Path-based apps +// - Subdomain app middleware +// - Workspace reconnecting-pty (aka. web terminal) +type Server struct { + Logger slog.Logger + + // DashboardURL should be a url to the coderd dashboard. This can be the + // same as the AccessURL if the Server is embedded. + DashboardURL *url.URL + AccessURL *url.URL + // Hostname should be the wildcard hostname to use for workspace + // applications INCLUDING the asterisk, (optional) suffix and leading dot. + // It will use the same scheme and port number as the access URL. + // E.g. "*.apps.coder.com" or "*-apps.coder.com". + Hostname string + // HostnameRegex contains the regex version of Hostname as generated by + // httpapi.CompileHostnamePattern(). It MUST be set if Hostname is set. + HostnameRegex *regexp.Regexp + DeploymentValues *codersdk.DeploymentValues + RealIPConfig *httpmw.RealIPConfig + + SignedTokenProvider SignedTokenProvider + WorkspaceConnCache *wsconncache.Cache + AppSecurityKey SecurityKey + + websocketWaitMutex sync.Mutex + websocketWaitGroup sync.WaitGroup +} + +// Close waits for all reconnecting-pty WebSocket connections to drain before +// returning. +func (s *Server) Close() error { + s.websocketWaitMutex.Lock() + s.websocketWaitGroup.Wait() + s.websocketWaitMutex.Unlock() + + // The caller must close the SignedTokenProvider (if necessary) and the + // wsconncache. + + return nil +} + +func (s *Server) Attach(r chi.Router) { + servePathApps := func(r chi.Router) { + r.HandleFunc("/*", s.workspaceAppsProxyPath) + } + + // %40 is the encoded character of the @ symbol. VS Code Web does + // not handle character encoding properly, so it's safe to assume + // other applications might not as well. + r.Route("/%40{user}/{workspace_and_agent}/apps/{workspaceapp}", servePathApps) + r.Route("/@{user}/{workspace_and_agent}/apps/{workspaceapp}", servePathApps) + + r.Get("/api/v2/workspaceagents/{workspaceagent}/pty", s.workspaceAgentPTY) +} + +// workspaceAppsProxyPath proxies requests to a workspace application +// through a relative URL path. +func (s *Server) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) { + if s.DeploymentValues.DisablePathApps.Value() { + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusUnauthorized, + Title: "Unauthorized", + Description: "Path-based applications are disabled on this Coder deployment by the administrator.", + RetryEnabled: false, + DashboardURL: s.DashboardURL.String(), + }) + return + } + + // We don't support @me in path apps since it requires the database to + // lookup the username from token. We used to redirect by doing this lookup. + if chi.URLParam(r, "user") == codersdk.Me { + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusNotFound, + Title: "Application Not Found", + Description: "Applications must be accessed with the full username, not @me.", + RetryEnabled: false, + DashboardURL: s.DashboardURL.String(), + }) + return + } + + // Determine the real path that was hit. The * URL parameter in Chi will not + // include the leading slash if it was present, so we need to add it back. + chiPath := chi.URLParam(r, "*") + basePath := strings.TrimSuffix(r.URL.Path, chiPath) + if strings.HasSuffix(basePath, "/") { + chiPath = "/" + chiPath + } + + // ResolveRequest will only return a new signed token if the actor has the RBAC + // permissions to connect to a workspace. + token, ok := ResolveRequest(s.Logger, s.DashboardURL, s.SignedTokenProvider, rw, r, Request{ + AccessMethod: AccessMethodPath, + BasePath: basePath, + UsernameOrID: chi.URLParam(r, "user"), + WorkspaceAndAgent: chi.URLParam(r, "workspace_and_agent"), + // We don't support port proxying on paths. The ResolveRequest method + // won't allow port proxying on path-based apps if the app is a number. + AppSlugOrPort: chi.URLParam(r, "workspaceapp"), + }) + if !ok { + return + } + + s.proxyWorkspaceApp(rw, r, *token, chiPath) +} + +// SubdomainAppMW handles subdomain-based application proxy requests (aka. +// DevURLs in Coder V1). +// +// There are a lot of paths here: +// 1. If api.Hostname is not set then we pass on. +// 2. If we can't read the request hostname then we return a 400. +// 3. If the request hostname matches api.AccessURL then we pass on. +// 5. We split the subdomain into the subdomain and the "rest". If there are no +// periods in the hostname then we pass on. +// 5. We parse the subdomain into a httpapi.ApplicationURL struct. If we +// encounter an error: +// a. If the "rest" does not match api.Hostname then we pass on; +// b. Otherwise, we return a 400. +// 6. Finally, we verify that the "rest" matches api.Hostname, else we +// return a 404. +// +// Rationales for each of the above steps: +// 1. We pass on if api.Hostname is not set to avoid returning any errors if +// `--app-hostname` is not configured. +// 2. Every request should have a valid Host header anyways. +// 3. We pass on if the request hostname matches api.AccessURL so we can +// support having the access URL be at the same level as the application +// base hostname. +// 4. We pass on if there are no periods in the hostname as application URLs +// must be a subdomain of a hostname, which implies there must be at least +// one period. +// 5. a. If the request subdomain is not a valid application URL, and the +// "rest" does not match api.Hostname, then it is very unlikely that +// the request was intended for this handler. We pass on. +// b. If the request subdomain is not a valid application URL, but the +// "rest" matches api.Hostname, then we return a 400 because the +// request is probably a typo or something. +// 6. We finally verify that the "rest" matches api.Hostname for security +// purposes regarding re-authentication and application proxy session +// tokens. +func (s *Server) SubdomainAppMW(middlewares ...func(http.Handler) http.Handler) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + // Step 1: Pass on if subdomain-based application proxying is not + // configured. + if s.Hostname == "" || s.HostnameRegex == nil { + next.ServeHTTP(rw, r) + return + } + + // Step 2: Get the request Host. + host := httpapi.RequestHost(r) + if host == "" { + if r.URL.Path == "/derp" { + // The /derp endpoint is used by wireguard clients to tunnel + // through coderd. For some reason these requests don't set + // a Host header properly sometimes in tests (no idea how), + // which causes this path to get hit. + next.ServeHTTP(rw, r) + return + } + + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Could not determine request Host.", + }) + return + } + + // Steps 3-6: Parse application from subdomain. + app, ok := s.parseHostname(rw, r, next, host) + if !ok { + return + } + + // If the request has the special query param then we need to set a + // cookie and strip that query parameter. + if encryptedAPIKey := r.URL.Query().Get(SubdomainProxyAPIKeyParam); encryptedAPIKey != "" { + // Exchange the encoded API key for a real one. + token, err := s.AppSecurityKey.DecryptAPIKey(encryptedAPIKey) + if err != nil { + s.Logger.Debug(ctx, "could not decrypt API key", slog.Error(err)) + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusBadRequest, + Title: "Bad Request", + Description: "Could not decrypt API key. Please remove the query parameter and try again.", + // Retry is disabled because the user needs to remove + // the query parameter before they try again. + RetryEnabled: false, + DashboardURL: s.DashboardURL.String(), + }) + return + } + + s.setWorkspaceAppCookie(rw, r, token) + + // Strip the query parameter. + path := r.URL.Path + if path == "" { + path = "/" + } + q := r.URL.Query() + q.Del(SubdomainProxyAPIKeyParam) + rawQuery := q.Encode() + if rawQuery != "" { + path += "?" + q.Encode() + } + + http.Redirect(rw, r, path, http.StatusTemporaryRedirect) + return + } + + token, ok := ResolveRequest(s.Logger, s.DashboardURL, s.SignedTokenProvider, rw, r, Request{ + AccessMethod: AccessMethodSubdomain, + BasePath: "/", + UsernameOrID: app.Username, + WorkspaceNameOrID: app.WorkspaceName, + AgentNameOrID: app.AgentName, + AppSlugOrPort: app.AppSlugOrPort, + }) + if !ok { + return + } + + // Use the passed in app middlewares before passing to the proxy + // app. + mws := chi.Middlewares(middlewares) + mws.Handler(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + s.proxyWorkspaceApp(rw, r, *token, r.URL.Path) + })).ServeHTTP(rw, r.WithContext(ctx)) + }) + } +} + +// parseHostname will return if a given request is attempting to access a +// workspace app via a subdomain. If it is, the hostname of the request is parsed +// into an httpapi.ApplicationURL and true is returned. If the request is not +// accessing a workspace app, then the next handler is called and false is +// returned. +func (s *Server) parseHostname(rw http.ResponseWriter, r *http.Request, next http.Handler, host string) (httpapi.ApplicationURL, bool) { + // Check if the hostname matches either of the access URLs. If it does, the + // user was definitely trying to connect to the dashboard/API or a + // path-based app. + if httpapi.HostnamesMatch(s.DashboardURL.Hostname(), host) || httpapi.HostnamesMatch(s.AccessURL.Hostname(), host) { + next.ServeHTTP(rw, r) + return httpapi.ApplicationURL{}, false + } + + // If there are no periods in the hostname, then it can't be a valid + // application URL. + if !strings.Contains(host, ".") { + next.ServeHTTP(rw, r) + return httpapi.ApplicationURL{}, false + } + + // Split the subdomain so we can parse the application details and verify it + // matches the configured app hostname later. + subdomain, ok := httpapi.ExecuteHostnamePattern(s.HostnameRegex, host) + if !ok { + // Doesn't match the regex, so it's not a valid application URL. + next.ServeHTTP(rw, r) + return httpapi.ApplicationURL{}, false + } + + // Check if the request is part of the deprecated logout flow. If so, we + // just redirect to the main access URL. + if subdomain == appLogoutHostname { + http.Redirect(rw, r, s.AccessURL.String(), http.StatusTemporaryRedirect) + return httpapi.ApplicationURL{}, false + } + + // Parse the application URL from the subdomain. + app, err := httpapi.ParseSubdomainAppURL(subdomain) + if err != nil { + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusBadRequest, + Title: "Invalid Application URL", + Description: fmt.Sprintf("Could not parse subdomain application URL %q: %s", subdomain, err.Error()), + RetryEnabled: false, + DashboardURL: s.DashboardURL.String(), + }) + return httpapi.ApplicationURL{}, false + } + + return app, true +} + +// 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. +func (s *Server) setWorkspaceAppCookie(rw http.ResponseWriter, r *http.Request, token string) bool { + hostSplit := strings.SplitN(s.Hostname, ".", 2) + if len(hostSplit) != 2 { + // This should be impossible as we verify the app hostname on + // startup, but we'll check anyways. + s.Logger.Error(r.Context(), "could not split invalid app hostname", slog.F("hostname", s.Hostname)) + 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: s.DashboardURL.String(), + }) + return false + } + + // Set the app cookie for all subdomains of s.Hostname. We don't set an + // expiration because the key in the database already has an expiration, and + // expired tokens don't affect the user experience (they get auto-redirected + // to re-smuggle the API key). + cookieHost := "." + hostSplit[1] + http.SetCookie(rw, &http.Cookie{ + Name: codersdk.DevURLSessionTokenCookie, + Value: token, + Domain: cookieHost, + Path: "/", + MaxAge: 0, + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + Secure: s.DeploymentValues.SecureAuthCookie.Value(), + }) + + return true +} + +func (s *Server) proxyWorkspaceApp(rw http.ResponseWriter, r *http.Request, appToken SignedToken, path string) { + ctx := r.Context() + + // Filter IP headers from untrusted origins. + httpmw.FilterUntrustedOriginHeaders(s.RealIPConfig, r) + + // Ensure proper IP headers get sent to the forwarded application. + err := httpmw.EnsureXForwardedForHeader(r) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + + appURL, err := url.Parse(appToken.AppURL) + if err != nil { + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusBadRequest, + Title: "Bad Request", + Description: fmt.Sprintf("Application has an invalid URL %q: %s", appToken.AppURL, err.Error()), + RetryEnabled: true, + DashboardURL: s.DashboardURL.String(), + }) + return + } + + // Verify that the port is allowed. See the docs above + // `codersdk.MinimumListeningPort` for more details. + port := appURL.Port() + if port != "" { + portInt, err := strconv.Atoi(port) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf("App URL %q has an invalid port %q.", appToken.AppURL, port), + Detail: err.Error(), + }) + return + } + + if portInt < codersdk.WorkspaceAgentMinimumListeningPort { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf("Application port %d is not permitted. Coder reserves ports less than %d for internal use.", portInt, codersdk.WorkspaceAgentMinimumListeningPort), + }) + return + } + } + + // Ensure path and query parameter correctness. + if path == "" { + // Web applications typically request paths relative to the + // root URL. This allows for routing behind a proxy or subpath. + // See https://github.com/coder/code-server/issues/241 for examples. + http.Redirect(rw, r, r.URL.Path+"/", http.StatusTemporaryRedirect) + return + } + if path == "/" && r.URL.RawQuery == "" && appURL.RawQuery != "" { + // If the application defines a default set of query parameters, + // we should always respect them. The reverse proxy will merge + // query parameters for server-side requests, but sometimes + // client-side applications require the query parameters to render + // properly. With code-server, this is the "folder" param. + r.URL.RawQuery = appURL.RawQuery + http.Redirect(rw, r, r.URL.String(), http.StatusTemporaryRedirect) + return + } + + r.URL.Path = path + appURL.RawQuery = "" + + proxy := httputil.NewSingleHostReverseProxy(appURL) + proxy.ErrorHandler = func(w http.ResponseWriter, r *http.Request, err error) { + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusBadGateway, + Title: "Bad Gateway", + Description: "Failed to proxy request to application: " + err.Error(), + RetryEnabled: true, + DashboardURL: s.DashboardURL.String(), + }) + } + + conn, release, err := s.WorkspaceConnCache.Acquire(appToken.AgentID) + if err != nil { + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusBadGateway, + Title: "Bad Gateway", + Description: "Could not connect to workspace agent: " + err.Error(), + RetryEnabled: true, + DashboardURL: s.DashboardURL.String(), + }) + return + } + defer release() + proxy.Transport = conn.HTTPTransport() + + // This strips the session token from a workspace app request. + cookieHeaders := r.Header.Values("Cookie")[:] + r.Header.Del("Cookie") + for _, cookieHeader := range cookieHeaders { + r.Header.Add("Cookie", httpapi.StripCoderCookies(cookieHeader)) + } + + // Convert canonicalized headers to their non-canonicalized counterparts. + // See the comment on `nonCanonicalHeaders` for more information on why this + // is necessary. + for k, v := range r.Header { + if n, ok := nonCanonicalHeaders[k]; ok { + r.Header.Del(k) + r.Header[n] = v + } + } + + // end span so we don't get long lived trace data + tracing.EndHTTPSpan(r, http.StatusOK, trace.SpanFromContext(ctx)) + + proxy.ServeHTTP(rw, r) +} + +// workspaceAgentPTY spawns a PTY and pipes it over a WebSocket. +// This is used for the web terminal. +// +// @Summary Open PTY to workspace agent +// @ID open-pty-to-workspace-agent +// @Security CoderSessionToken +// @Tags Agents +// @Param workspaceagent path string true "Workspace agent ID" format(uuid) +// @Success 101 +// @Router /workspaceagents/{workspaceagent}/pty [get] +func (s *Server) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + s.websocketWaitMutex.Lock() + s.websocketWaitGroup.Add(1) + s.websocketWaitMutex.Unlock() + defer s.websocketWaitGroup.Done() + + appToken, ok := ResolveRequest(s.Logger, s.AccessURL, s.SignedTokenProvider, rw, r, Request{ + AccessMethod: AccessMethodTerminal, + BasePath: r.URL.Path, + AgentNameOrID: chi.URLParam(r, "workspaceagent"), + }) + if !ok { + return + } + + values := r.URL.Query() + parser := httpapi.NewQueryParamParser() + reconnect := parser.Required("reconnect").UUID(values, uuid.New(), "reconnect") + height := parser.UInt(values, 80, "height") + width := parser.UInt(values, 80, "width") + if len(parser.Errors) > 0 { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Invalid query parameters.", + Validations: parser.Errors, + }) + return + } + + conn, err := websocket.Accept(rw, r, &websocket.AcceptOptions{ + CompressionMode: websocket.CompressionDisabled, + }) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Failed to accept websocket.", + Detail: err.Error(), + }) + return + } + + ctx, wsNetConn := WebsocketNetConn(ctx, conn, websocket.MessageBinary) + defer wsNetConn.Close() // Also closes conn. + + go httpapi.Heartbeat(ctx, conn) + + agentConn, release, err := s.WorkspaceConnCache.Acquire(appToken.AgentID) + if err != nil { + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("dial workspace agent: %s", err)) + return + } + defer release() + ptNetConn, err := agentConn.ReconnectingPTY(ctx, reconnect, uint16(height), uint16(width), r.URL.Query().Get("command")) + if err != nil { + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("dial: %s", err)) + return + } + defer ptNetConn.Close() + agent.Bicopy(ctx, wsNetConn, ptNetConn) +} + +// wsNetConn wraps net.Conn created by websocket.NetConn(). Cancel func +// is called if a read or write error is encountered. +type wsNetConn struct { + cancel context.CancelFunc + net.Conn +} + +func (c *wsNetConn) Read(b []byte) (n int, err error) { + n, err = c.Conn.Read(b) + if err != nil { + c.cancel() + } + return n, err +} + +func (c *wsNetConn) Write(b []byte) (n int, err error) { + n, err = c.Conn.Write(b) + if err != nil { + c.cancel() + } + return n, err +} + +func (c *wsNetConn) Close() error { + defer c.cancel() + return c.Conn.Close() +} + +// WebsocketNetConn wraps websocket.NetConn and returns a context that +// is tied to the parent context and the lifetime of the conn. Any error +// during read or write will cancel the context, but not close the +// conn. Close should be called to release context resources. +func WebsocketNetConn(ctx context.Context, conn *websocket.Conn, msgType websocket.MessageType) (context.Context, net.Conn) { + ctx, cancel := context.WithCancel(ctx) + nc := websocket.NetConn(ctx, conn, msgType) + return ctx, &wsNetConn{ + cancel: cancel, + Conn: nc, + } +} diff --git a/coderd/workspaceapps/proxy_test.go b/coderd/workspaceapps/proxy_test.go new file mode 100644 index 0000000000000..a497ed157041a --- /dev/null +++ b/coderd/workspaceapps/proxy_test.go @@ -0,0 +1,8 @@ +package workspaceapps_test + +// NOTE: for now, app proxying tests are still in their old locations, pending +// being moved to their own package. +// +// See: +// - coderd/workspaceapps_test.go +// - coderd/workspaceagents_test.go (for PTY) diff --git a/coderd/workspaceapps/ticket.go b/coderd/workspaceapps/ticket.go deleted file mode 100644 index bd7d74e4194e5..0000000000000 --- a/coderd/workspaceapps/ticket.go +++ /dev/null @@ -1,102 +0,0 @@ -package workspaceapps - -import ( - "encoding/json" - "time" - - "github.com/google/uuid" - "golang.org/x/xerrors" - "gopkg.in/square/go-jose.v2" -) - -const tokenSigningAlgorithm = jose.HS512 - -// SignedToken is the struct data contained inside a workspace app JWE. It -// contains the details of the workspace app that the token is valid for to -// avoid database queries. -type SignedToken struct { - // Request details. - Request `json:"request"` - - // Trusted resolved details. - Expiry time.Time `json:"expiry"` // set by GenerateToken if unset - UserID uuid.UUID `json:"user_id"` - WorkspaceID uuid.UUID `json:"workspace_id"` - AgentID uuid.UUID `json:"agent_id"` - AppURL string `json:"app_url"` -} - -// MatchesRequest returns true if the token matches the request. Any token that -// does not match the request should be considered invalid. -func (t SignedToken) MatchesRequest(req Request) bool { - return t.AccessMethod == req.AccessMethod && - t.BasePath == req.BasePath && - t.UsernameOrID == req.UsernameOrID && - t.WorkspaceNameOrID == req.WorkspaceNameOrID && - t.AgentNameOrID == req.AgentNameOrID && - t.AppSlugOrPort == req.AppSlugOrPort -} - -// GenerateToken generates a signed workspace app token with the given key and -// payload. If the payload doesn't have an expiry, it will be set to the current -// time plus the default expiry. -func GenerateToken(key []byte, payload SignedToken) (string, error) { - if payload.Expiry.IsZero() { - payload.Expiry = time.Now().Add(DefaultTokenExpiry) - } - payloadBytes, err := json.Marshal(payload) - if err != nil { - return "", xerrors.Errorf("marshal payload to JSON: %w", err) - } - - signer, err := jose.NewSigner(jose.SigningKey{ - Algorithm: tokenSigningAlgorithm, - Key: key, - }, nil) - if err != nil { - return "", xerrors.Errorf("create signer: %w", err) - } - - signedObject, err := signer.Sign(payloadBytes) - if err != nil { - return "", xerrors.Errorf("sign payload: %w", err) - } - - serialized, err := signedObject.CompactSerialize() - if err != nil { - return "", xerrors.Errorf("serialize JWS: %w", err) - } - - return serialized, nil -} - -// ParseToken parses a signed workspace app token with the given key and returns -// the payload. If the token is invalid or expired, an error is returned. -func ParseToken(key []byte, str string) (SignedToken, error) { - object, err := jose.ParseSigned(str) - if err != nil { - return SignedToken{}, xerrors.Errorf("parse JWS: %w", err) - } - if len(object.Signatures) != 1 { - return SignedToken{}, xerrors.New("expected 1 signature") - } - if object.Signatures[0].Header.Algorithm != string(tokenSigningAlgorithm) { - return SignedToken{}, xerrors.Errorf("expected token signing algorithm to be %q, got %q", tokenSigningAlgorithm, object.Signatures[0].Header.Algorithm) - } - - output, err := object.Verify(key) - if err != nil { - return SignedToken{}, xerrors.Errorf("verify JWS: %w", err) - } - - var tok SignedToken - err = json.Unmarshal(output, &tok) - if err != nil { - return SignedToken{}, xerrors.Errorf("unmarshal payload: %w", err) - } - if tok.Expiry.Before(time.Now()) { - return SignedToken{}, xerrors.New("signed app token expired") - } - - return tok, nil -} diff --git a/coderd/workspaceapps/token.go b/coderd/workspaceapps/token.go new file mode 100644 index 0000000000000..58583e2950a7d --- /dev/null +++ b/coderd/workspaceapps/token.go @@ -0,0 +1,219 @@ +package workspaceapps + +import ( + "encoding/base64" + "encoding/hex" + "encoding/json" + "time" + + "github.com/go-jose/go-jose/v3" + "github.com/google/uuid" + "golang.org/x/xerrors" + + "github.com/coder/coder/coderd/database" +) + +const ( + tokenSigningAlgorithm = jose.HS512 + apiKeyEncryptionAlgorithm = jose.A256GCMKW +) + +// SignedToken is the struct data contained inside a workspace app JWE. It +// contains the details of the workspace app that the token is valid for to +// avoid database queries. +type SignedToken struct { + // Request details. + Request `json:"request"` + + // Trusted resolved details. + Expiry time.Time `json:"expiry"` // set by GenerateToken if unset + UserID uuid.UUID `json:"user_id"` + WorkspaceID uuid.UUID `json:"workspace_id"` + AgentID uuid.UUID `json:"agent_id"` + AppURL string `json:"app_url"` +} + +// MatchesRequest returns true if the token matches the request. Any token that +// does not match the request should be considered invalid. +func (t SignedToken) MatchesRequest(req Request) bool { + return t.AccessMethod == req.AccessMethod && + t.BasePath == req.BasePath && + t.UsernameOrID == req.UsernameOrID && + t.WorkspaceNameOrID == req.WorkspaceNameOrID && + t.AgentNameOrID == req.AgentNameOrID && + t.AppSlugOrPort == req.AppSlugOrPort +} + +// SecurityKey is used for signing and encrypting app tokens and API keys. +// +// The first 64 bytes of the key are used for signing tokens with HMAC-SHA256, +// and the last 32 bytes are used for encrypting API keys with AES-256-GCM. +// We use a single key for both operations to avoid having to store and manage +// two keys. +type SecurityKey [96]byte + +func (k SecurityKey) signingKey() []byte { + return k[:64] +} + +func (k SecurityKey) encryptionKey() []byte { + return k[64:] +} + +func KeyFromString(str string) (SecurityKey, error) { + var key SecurityKey + decoded, err := hex.DecodeString(str) + if err != nil { + return key, xerrors.Errorf("decode key: %w", err) + } + if len(decoded) != len(key) { + return key, xerrors.Errorf("expected key to be %d bytes, got %d", len(key), len(decoded)) + } + copy(key[:], decoded) + + return key, nil +} + +// SignToken generates a signed workspace app token with the given payload. If +// the payload doesn't have an expiry, it will be set to the current time plus +// the default expiry. +func (k SecurityKey) SignToken(payload SignedToken) (string, error) { + if payload.Expiry.IsZero() { + payload.Expiry = time.Now().Add(DefaultTokenExpiry) + } + payloadBytes, err := json.Marshal(payload) + if err != nil { + return "", xerrors.Errorf("marshal payload to JSON: %w", err) + } + + signer, err := jose.NewSigner(jose.SigningKey{ + Algorithm: tokenSigningAlgorithm, + Key: k.signingKey(), + }, nil) + if err != nil { + return "", xerrors.Errorf("create signer: %w", err) + } + + signedObject, err := signer.Sign(payloadBytes) + if err != nil { + return "", xerrors.Errorf("sign payload: %w", err) + } + + serialized, err := signedObject.CompactSerialize() + if err != nil { + return "", xerrors.Errorf("serialize JWS: %w", err) + } + + return serialized, nil +} + +// VerifySignedToken parses a signed workspace app token with the given key and +// returns the payload. If the token is invalid or expired, an error is +// returned. +func (k SecurityKey) VerifySignedToken(str string) (SignedToken, error) { + object, err := jose.ParseSigned(str) + if err != nil { + return SignedToken{}, xerrors.Errorf("parse JWS: %w", err) + } + if len(object.Signatures) != 1 { + return SignedToken{}, xerrors.New("expected 1 signature") + } + if object.Signatures[0].Header.Algorithm != string(tokenSigningAlgorithm) { + return SignedToken{}, xerrors.Errorf("expected token signing algorithm to be %q, got %q", tokenSigningAlgorithm, object.Signatures[0].Header.Algorithm) + } + + output, err := object.Verify(k.signingKey()) + if err != nil { + return SignedToken{}, xerrors.Errorf("verify JWS: %w", err) + } + + var tok SignedToken + err = json.Unmarshal(output, &tok) + if err != nil { + return SignedToken{}, xerrors.Errorf("unmarshal payload: %w", err) + } + if tok.Expiry.Before(time.Now()) { + return SignedToken{}, xerrors.New("signed app token expired") + } + + return tok, nil +} + +type EncryptedAPIKeyPayload struct { + APIKey string `json:"api_key"` + ExpiresAt time.Time `json:"expires_at"` +} + +// EncryptAPIKey encrypts an API key for subdomain token smuggling. +func (k SecurityKey) EncryptAPIKey(payload EncryptedAPIKeyPayload) (string, error) { + if payload.APIKey == "" { + return "", xerrors.New("API key is empty") + } + if payload.ExpiresAt.IsZero() { + // Very short expiry as these keys are only used once as part of an + // automatic redirection flow. + payload.ExpiresAt = database.Now().Add(time.Minute) + } + + payloadBytes, err := json.Marshal(payload) + if err != nil { + return "", xerrors.Errorf("marshal payload: %w", err) + } + + // JWEs seem to apply a nonce themselves. + encrypter, err := jose.NewEncrypter( + jose.A256GCM, + jose.Recipient{ + Algorithm: apiKeyEncryptionAlgorithm, + Key: k.encryptionKey(), + }, + &jose.EncrypterOptions{ + Compression: jose.DEFLATE, + }, + ) + if err != nil { + return "", xerrors.Errorf("initializer jose encrypter: %w", err) + } + encryptedObject, err := encrypter.Encrypt(payloadBytes) + if err != nil { + return "", xerrors.Errorf("encrypt jwe: %w", err) + } + + encrypted := encryptedObject.FullSerialize() + return base64.RawURLEncoding.EncodeToString([]byte(encrypted)), nil +} + +// DecryptAPIKey undoes EncryptAPIKey and is used in the subdomain app handler. +func (k SecurityKey) DecryptAPIKey(encryptedAPIKey string) (string, error) { + encrypted, err := base64.RawURLEncoding.DecodeString(encryptedAPIKey) + if err != nil { + return "", xerrors.Errorf("base64 decode encrypted API key: %w", err) + } + + object, err := jose.ParseEncrypted(string(encrypted)) + if err != nil { + return "", xerrors.Errorf("parse encrypted API key: %w", err) + } + if object.Header.Algorithm != string(apiKeyEncryptionAlgorithm) { + return "", xerrors.Errorf("expected API key encryption algorithm to be %q, got %q", apiKeyEncryptionAlgorithm, object.Header.Algorithm) + } + + // Decrypt using the hashed secret. + decrypted, err := object.Decrypt(k.encryptionKey()) + if err != nil { + return "", xerrors.Errorf("decrypt API key: %w", err) + } + + // Unmarshal the payload. + var payload EncryptedAPIKeyPayload + if err := json.Unmarshal(decrypted, &payload); err != nil { + return "", xerrors.Errorf("unmarshal decrypted payload: %w", err) + } + + // Validate expiry. + if payload.ExpiresAt.Before(database.Now()) { + return "", xerrors.New("encrypted API key expired") + } + + return payload.APIKey, nil +} diff --git a/coderd/workspaceapps/ticket_test.go b/coderd/workspaceapps/token_test.go similarity index 76% rename from coderd/workspaceapps/ticket_test.go rename to coderd/workspaceapps/token_test.go index 9b5cc6a4e3e85..8b20180cabaf0 100644 --- a/coderd/workspaceapps/ticket_test.go +++ b/coderd/workspaceapps/token_test.go @@ -1,16 +1,18 @@ package workspaceapps_test import ( - "encoding/hex" + "fmt" "testing" "time" + "github.com/go-jose/go-jose/v3" "github.com/google/uuid" "github.com/stretchr/testify/require" - "gopkg.in/square/go-jose.v2" "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/workspaceapps" + "github.com/coder/coder/cryptorand" ) func Test_TokenMatchesRequest(t *testing.T) { @@ -164,7 +166,7 @@ func Test_GenerateToken(t *testing.T) { t.Run("SetExpiry", func(t *testing.T) { t.Parallel() - tokenStr, err := workspaceapps.GenerateToken(coderdtest.AppSigningKey, workspaceapps.SignedToken{ + tokenStr, err := coderdtest.AppSecurityKey.SignToken(workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/app", @@ -182,7 +184,7 @@ func Test_GenerateToken(t *testing.T) { }) require.NoError(t, err) - token, err := workspaceapps.ParseToken(coderdtest.AppSigningKey, tokenStr) + token, err := coderdtest.AppSecurityKey.VerifySignedToken(tokenStr) require.NoError(t, err) require.WithinDuration(t, time.Now().Add(time.Minute), token.Expiry, 15*time.Second) @@ -260,13 +262,13 @@ func Test_GenerateToken(t *testing.T) { t.Run(c.name, func(t *testing.T) { t.Parallel() - str, err := workspaceapps.GenerateToken(coderdtest.AppSigningKey, c.token) + str, err := coderdtest.AppSecurityKey.SignToken(c.token) require.NoError(t, err) // Tokens aren't deterministic as they have a random nonce, so we // can't compare them directly. - token, err := workspaceapps.ParseToken(coderdtest.AppSigningKey, str) + token, err := coderdtest.AppSecurityKey.VerifySignedToken(str) if c.parseErrContains != "" { require.Error(t, err) require.ErrorContains(t, err, c.parseErrContains) @@ -289,7 +291,7 @@ func Test_ParseToken(t *testing.T) { t.Run("InvalidJWS", func(t *testing.T) { t.Parallel() - token, err := workspaceapps.ParseToken(coderdtest.AppSigningKey, "invalid") + token, err := coderdtest.AppSecurityKey.VerifySignedToken("invalid") require.Error(t, err) require.ErrorContains(t, err, "parse JWS") require.Equal(t, workspaceapps.SignedToken{}, token) @@ -299,12 +301,14 @@ func Test_ParseToken(t *testing.T) { t.Parallel() // Create a valid token using a different key. - otherKey, err := hex.DecodeString("62656566646561646265656664656164626565666465616462656566646561646265656664656164626565666465616462656566646561646265656664656164") - require.NoError(t, err) - require.NotEqual(t, coderdtest.AppSigningKey, otherKey) - require.Len(t, otherKey, 64) - - tokenStr, err := workspaceapps.GenerateToken(otherKey, workspaceapps.SignedToken{ + var otherKey workspaceapps.SecurityKey + copy(otherKey[:], coderdtest.AppSecurityKey[:]) + for i := range otherKey { + otherKey[i] ^= 0xff + } + require.NotEqual(t, coderdtest.AppSecurityKey, otherKey) + + tokenStr, err := otherKey.SignToken(workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/app", @@ -323,7 +327,7 @@ func Test_ParseToken(t *testing.T) { require.NoError(t, err) // Verify the token is invalid. - token, err := workspaceapps.ParseToken(coderdtest.AppSigningKey, tokenStr) + token, err := coderdtest.AppSecurityKey.VerifySignedToken(tokenStr) require.Error(t, err) require.ErrorContains(t, err, "verify JWS") require.Equal(t, workspaceapps.SignedToken{}, token) @@ -333,16 +337,86 @@ func Test_ParseToken(t *testing.T) { t.Parallel() // Create a signature for an invalid body. - signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.HS512, Key: coderdtest.AppSigningKey}, nil) + signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.HS512, Key: coderdtest.AppSecurityKey[:64]}, nil) require.NoError(t, err) signedObject, err := signer.Sign([]byte("hi")) require.NoError(t, err) serialized, err := signedObject.CompactSerialize() require.NoError(t, err) - token, err := workspaceapps.ParseToken(coderdtest.AppSigningKey, serialized) + token, err := coderdtest.AppSecurityKey.VerifySignedToken(serialized) require.Error(t, err) require.ErrorContains(t, err, "unmarshal payload") require.Equal(t, workspaceapps.SignedToken{}, token) }) } + +func TestAPIKeyEncryption(t *testing.T) { + t.Parallel() + + genAPIKey := func(t *testing.T) string { + id, _ := cryptorand.String(10) + secret, _ := cryptorand.String(22) + + return fmt.Sprintf("%s-%s", id, secret) + } + + t.Run("OK", func(t *testing.T) { + t.Parallel() + + key := genAPIKey(t) + encrypted, err := coderdtest.AppSecurityKey.EncryptAPIKey(workspaceapps.EncryptedAPIKeyPayload{ + APIKey: key, + }) + require.NoError(t, err) + + decryptedKey, err := coderdtest.AppSecurityKey.DecryptAPIKey(encrypted) + require.NoError(t, err) + require.Equal(t, key, decryptedKey) + }) + + t.Run("Verifies", func(t *testing.T) { + t.Parallel() + + t.Run("Expiry", func(t *testing.T) { + t.Parallel() + + key := genAPIKey(t) + encrypted, err := coderdtest.AppSecurityKey.EncryptAPIKey(workspaceapps.EncryptedAPIKeyPayload{ + APIKey: key, + ExpiresAt: database.Now().Add(-1 * time.Hour), + }) + require.NoError(t, err) + + decryptedKey, err := coderdtest.AppSecurityKey.DecryptAPIKey(encrypted) + require.Error(t, err) + require.ErrorContains(t, err, "expired") + require.Empty(t, decryptedKey) + }) + + t.Run("EncryptionKey", func(t *testing.T) { + t.Parallel() + + // Create a valid token using a different key. + var otherKey workspaceapps.SecurityKey + copy(otherKey[:], coderdtest.AppSecurityKey[:]) + for i := range otherKey { + otherKey[i] ^= 0xff + } + require.NotEqual(t, coderdtest.AppSecurityKey, otherKey) + + // Encrypt with the other key. + key := genAPIKey(t) + encrypted, err := otherKey.EncryptAPIKey(workspaceapps.EncryptedAPIKeyPayload{ + APIKey: key, + }) + require.NoError(t, err) + + // Decrypt with the original key. + decryptedKey, err := coderdtest.AppSecurityKey.DecryptAPIKey(encrypted) + require.Error(t, err) + require.ErrorContains(t, err, "decrypt API key") + require.Empty(t, decryptedKey) + }) + }) +} diff --git a/coderd/workspaceapps_internal_test.go b/coderd/workspaceapps_internal_test.go deleted file mode 100644 index 4419fa1bf3f8a..0000000000000 --- a/coderd/workspaceapps_internal_test.go +++ /dev/null @@ -1,95 +0,0 @@ -package coderd - -import ( - "context" - "crypto/sha256" - "testing" - "time" - - "github.com/stretchr/testify/require" - - "github.com/coder/coder/coderd/database" - "github.com/coder/coder/coderd/database/dbfake" - "github.com/coder/coder/coderd/database/dbgen" - "github.com/coder/coder/testutil" -) - -func TestAPIKeyEncryption(t *testing.T) { - t.Parallel() - - generateAPIKey := func(t *testing.T, db database.Store) (keyID, keyToken string, hashedSecret []byte, data encryptedAPIKeyPayload) { - key, token := dbgen.APIKey(t, db, database.APIKey{}) - - data = encryptedAPIKeyPayload{ - APIKey: token, - ExpiresAt: database.Now().Add(24 * time.Hour), - } - - return key.ID, token, key.HashedSecret[:], data - } - - t.Run("OK", func(t *testing.T) { - t.Parallel() - db := dbfake.New() - keyID, _, hashedSecret, data := generateAPIKey(t, db) - - encrypted, err := encryptAPIKey(data) - require.NoError(t, err) - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - key, token, err := decryptAPIKey(ctx, db, encrypted) - require.NoError(t, err) - require.Equal(t, keyID, key.ID) - require.Equal(t, hashedSecret[:], key.HashedSecret) - require.Equal(t, data.APIKey, token) - }) - - t.Run("Verifies", func(t *testing.T) { - t.Parallel() - - t.Run("Expiry", func(t *testing.T) { - t.Parallel() - db := dbfake.New() - _, _, _, data := generateAPIKey(t, db) - - data.ExpiresAt = database.Now().Add(-1 * time.Hour) - encrypted, err := encryptAPIKey(data) - require.NoError(t, err) - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - _, _, err = decryptAPIKey(ctx, db, encrypted) - require.Error(t, err) - require.ErrorContains(t, err, "expired") - }) - - t.Run("KeyMatches", func(t *testing.T) { - t.Parallel() - db := dbfake.New() - - hashedSecret := sha256.Sum256([]byte("wrong")) - // Insert a token with a mismatched hashed secret. - _, token := dbgen.APIKey(t, db, database.APIKey{ - HashedSecret: hashedSecret[:], - }) - - data := encryptedAPIKeyPayload{ - APIKey: token, - ExpiresAt: database.Now().Add(24 * time.Hour), - } - - encrypted, err := encryptAPIKey(data) - require.NoError(t, err) - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - _, _, err = decryptAPIKey(ctx, db, encrypted) - require.Error(t, err) - require.ErrorContains(t, err, "error in crypto") - }) - }) -} diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index 3662f4233fc7a..02a1ad3aa1fc9 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -24,7 +24,6 @@ import ( "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/agent" "github.com/coder/coder/cli/clibase" - "github.com/coder/coder/coderd" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" @@ -439,7 +438,7 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { require.Equal(t, http.StatusOK, resp.StatusCode) }) - t.Run("RedirectsMe", func(t *testing.T) { + t.Run("BlocksMe", func(t *testing.T) { t.Parallel() ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) @@ -448,30 +447,11 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { resp, err := requestWithRetries(ctx, t, client, http.MethodGet, fmt.Sprintf("/@me/%s/apps/%s/?%s", workspace.Name, proxyTestAppNameOwner, proxyTestAppQuery), nil) require.NoError(t, err) defer resp.Body.Close() - require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) - loc, err := resp.Location() - require.NoError(t, err) - require.NotContains(t, loc.Path, "@me") - require.Contains(t, loc.Path, "@"+coderdtest.FirstUserParams.Username) - }) - - t.Run("RedirectsMeUnauthenticated", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - unauthenticatedClient := codersdk.New(client.URL) - unauthenticatedClient.HTTPClient.CheckRedirect = client.HTTPClient.CheckRedirect - unauthenticatedClient.HTTPClient.Transport = client.HTTPClient.Transport + require.Equal(t, http.StatusNotFound, resp.StatusCode) - resp, err := requestWithRetries(ctx, t, unauthenticatedClient, http.MethodGet, fmt.Sprintf("/@me/%s/apps/%s/?%s", workspace.Name, proxyTestAppNameOwner, proxyTestAppQuery), nil) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) - loc, err := resp.Location() + body, err := io.ReadAll(resp.Body) require.NoError(t, err) - require.Equal(t, "/login", loc.Path) + require.Contains(t, string(body), "must be accessed with the full username, not @me") }) t.Run("ForwardsIP", func(t *testing.T) { @@ -712,7 +692,7 @@ func TestWorkspaceApplicationAuth(t *testing.T) { func TestWorkspaceAppsProxySubdomainPassthrough(t *testing.T) { t.Parallel() - // No AppHostname set. + // No Hostname set. client := coderdtest.New(t, &coderdtest.Options{ AppHostname: "", }) @@ -1066,195 +1046,6 @@ 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, nil) - - 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.AppHost(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.SessionTokenHeader, client.SessionToken()) - if c.cookie != "" { - req.AddCookie(&http.Cookie{ - Name: codersdk.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, codersdk.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() diff --git a/go.mod b/go.mod index a4b602febc6a9..07732b4cccf38 100644 --- a/go.mod +++ b/go.mod @@ -89,6 +89,7 @@ require ( github.com/go-chi/chi/v5 v5.0.7 github.com/go-chi/httprate v0.7.1 github.com/go-chi/render v1.0.1 + github.com/go-jose/go-jose/v3 v3.0.0 github.com/go-logr/logr v1.2.3 github.com/go-ping/ping v1.1.0 github.com/go-playground/validator/v10 v10.12.0 @@ -166,7 +167,6 @@ require ( google.golang.org/grpc v1.53.0 google.golang.org/protobuf v1.28.2-0.20230118093459-a9481185b34d gopkg.in/natefinch/lumberjack.v2 v2.0.0 - gopkg.in/square/go-jose.v2 v2.6.0 gopkg.in/yaml.v3 v3.0.1 gvisor.dev/gvisor v0.0.0-20221203005347-703fd9b7fbc0 k8s.io/utils v0.0.0-20220728103510-ee6ede2d64ed @@ -346,6 +346,7 @@ require ( golang.zx2c4.com/wireguard/windows v0.5.3 // indirect google.golang.org/appengine v1.6.7 // indirect google.golang.org/genproto v0.0.0-20230223222841-637eb2293923 // indirect + gopkg.in/square/go-jose.v2 v2.6.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect howett.net/plist v1.0.0 // indirect inet.af/peercred v0.0.0-20210906144145-0893ea02156a // indirect diff --git a/go.sum b/go.sum index 9099fb9aa5626..aa4040e03fc34 100644 --- a/go.sum +++ b/go.sum @@ -676,6 +676,8 @@ github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9 github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-ini/ini v1.25.4/go.mod h1:ByCAeIL28uOIIG0E3PJtZPDL8WnHpFKFOtgjp+3Ies8= +github.com/go-jose/go-jose/v3 v3.0.0 h1:s6rrhirfEP/CGIoc6p+PZAeogN2SxKav6Wp7+dyMWVo= +github.com/go-jose/go-jose/v3 v3.0.0/go.mod h1:RNkWWRld676jZEYoV3+XK8L2ZnNSvIsxFMht0mSX+u8= github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-kit/log v0.1.0/go.mod h1:zbhenjAZHb184qTLMA9ZjW7ThYL0H2mk7Q6pNt4vbaY= diff --git a/site/src/xServices/auth/authXService.ts b/site/src/xServices/auth/authXService.ts index 7392aaedaf010..db73f54c39a59 100644 --- a/site/src/xServices/auth/authXService.ts +++ b/site/src/xServices/auth/authXService.ts @@ -141,31 +141,11 @@ const signIn = async ( } const 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() const [authMethods] = await Promise.all([ - API.getAuthMethods(), // Antecipate and load the auth methods + API.getAuthMethods(), // Anticipate and load the auth methods API.logout(), ]) - // Logout the app URLs - 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, - } - } - return { hasFirstUser: true, authMethods, @@ -387,12 +367,8 @@ export const authMachine = clearUpdateProfileError: assign({ updateProfileError: (_) => undefined, }), - redirect: (_, { data }) => { - if (!("redirectUrl" in data)) { - window.location.href = location.origin - } else { - window.location.replace(data.redirectUrl) - } + redirect: (_, _data) => { + window.location.href = location.origin }, }, guards: {