diff --git a/cli/server.go b/cli/server.go index 2129b6fca3..c46938e71d 100644 --- a/cli/server.go +++ b/cli/server.go @@ -10,6 +10,7 @@ import ( "crypto/tls" "crypto/x509" "database/sql" + "encoding/hex" "errors" "fmt" "io" @@ -587,19 +588,62 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co defer options.Pubsub.Close() } - deploymentID, err := options.Database.GetDeploymentID(ctx) - if errors.Is(err, sql.ErrNoRows) { - err = nil - } - if err != nil { - return xerrors.Errorf("get deployment id: %w", err) - } - if deploymentID == "" { - deploymentID = uuid.NewString() - err = options.Database.InsertDeploymentID(ctx, deploymentID) + var deploymentID string + err = options.Database.InTx(func(tx database.Store) error { + // This will block until the lock is acquired, and will be + // automatically released when the transaction ends. + err := tx.AcquireLock(ctx, database.LockIDDeploymentSetup) if err != nil { - return xerrors.Errorf("set deployment id: %w", err) + return xerrors.Errorf("acquire lock: %w", err) } + + deploymentID, err = tx.GetDeploymentID(ctx) + if err != nil && !xerrors.Is(err, sql.ErrNoRows) { + return xerrors.Errorf("get deployment id: %w", err) + } + if deploymentID == "" { + deploymentID = uuid.NewString() + err = tx.InsertDeploymentID(ctx, deploymentID) + if err != nil { + return xerrors.Errorf("set deployment id: %w", err) + } + } + + // 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) + 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) + _, 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) + if err != nil { + return xerrors.Errorf("insert freshly generated app signing key to database: %w", err) + } + } + + appSigningKey, err := hex.DecodeString(appSigningKeyStr) + 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)) + } + + options.AppSigningKey = appSigningKey + return nil + }, nil) + if err != nil { + return err } // Disable telemetry if the in-memory database is used unless explicitly defined! diff --git a/coderd/coderd.go b/coderd/coderd.go index 450e1bc074..0535b00baf 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -56,6 +56,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/coderd/wsconncache" "github.com/coder/coder/codersdk" "github.com/coder/coder/provisionerd/proto" @@ -120,6 +121,9 @@ type Options struct { SwaggerEndpoint bool SetUserGroups func(ctx context.Context, tx database.Store, userID uuid.UUID, groupNames []string) error TemplateScheduleStore schedule.TemplateScheduleStore + // AppSigningKey denotes the symmetric key to use for signing app tickets. + // The key must be 64 bytes long. + AppSigningKey []byte // APIRateLimit is the minutely throughput rate limit per user or ip. // Setting a rate limit <0 will disable the rate limiter across the entire @@ -214,6 +218,9 @@ func New(options *Options) *API { if options.TemplateScheduleStore == nil { options.TemplateScheduleStore = schedule.NewAGPLTemplateScheduleStore() } + if len(options.AppSigningKey) != 64 { + panic("coderd: AppSigningKey must be 64 bytes long") + } siteCacheDir := options.CacheDir if siteCacheDir != "" { @@ -236,6 +243,11 @@ func New(options *Options) *API { // static files since it only affects browsers. staticHandler = httpmw.HSTS(staticHandler, options.StrictTransportSecurityCfg) + oauthConfigs := &httpmw.OAuth2Configs{ + Github: options.GithubOAuth2Config, + OIDC: options.OIDCConfig, + } + r := chi.NewRouter() ctx, cancel := context.WithCancel(context.Background()) api := &API{ @@ -250,6 +262,15 @@ func New(options *Options) *API { Authorizer: options.Authorizer, Logger: options.Logger, }, + WorkspaceAppsProvider: workspaceapps.New( + options.Logger.Named("workspaceapps"), + options.AccessURL, + options.Authorizer, + options.Database, + options.DeploymentConfig, + oauthConfigs, + options.AppSigningKey, + ), metricsCache: metricsCache, Auditor: atomic.Pointer[audit.Auditor]{}, TemplateScheduleStore: atomic.Pointer[schedule.TemplateScheduleStore]{}, @@ -266,12 +287,8 @@ func New(options *Options) *API { api.TemplateScheduleStore.Store(&options.TemplateScheduleStore) api.workspaceAgentCache = wsconncache.New(api.dialWorkspaceAgentTailnet, 0) api.TailnetCoordinator.Store(&options.TailnetCoordinator) - oauthConfigs := &httpmw.OAuth2Configs{ - Github: options.GithubOAuth2Config, - OIDC: options.OIDCConfig, - } - apiKeyMiddleware := httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + apiKeyMiddleware := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: options.Database, OAuth2Configs: oauthConfigs, RedirectToLogin: false, @@ -279,7 +296,7 @@ func New(options *Options) *API { Optional: false, }) // Same as above but it redirects to the login page. - apiKeyMiddlewareRedirect := httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + apiKeyMiddlewareRedirect := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: options.Database, OAuth2Configs: oauthConfigs, RedirectToLogin: true, @@ -305,23 +322,9 @@ func New(options *Options) *API { httpmw.Prometheus(options.PrometheusRegistry), // handleSubdomainApplications checks if the first subdomain is a valid // app URL. If it is, it will serve that application. - api.handleSubdomainApplications( - apiRateLimiter, - // Middleware to impose on the served application. - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ - DB: options.Database, - OAuth2Configs: oauthConfigs, - // The code handles the the case where the user is not - // authenticated automatically. - RedirectToLogin: false, - DisableSessionExpiryRefresh: options.DeploymentConfig.DisableSessionExpiryRefresh.Value, - Optional: true, - }), - httpmw.AsAuthzSystem( - httpmw.ExtractUserParam(api.Database, false), - httpmw.ExtractWorkspaceAndAgentParam(api.Database), - ), - ), + // + // Workspace apps do their own auth. + api.handleSubdomainApplications(apiRateLimiter), // Build-Version is helpful for debugging. func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -345,26 +348,8 @@ func New(options *Options) *API { r.Get("/healthz", func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("OK")) }) apps := func(r chi.Router) { - r.Use( - apiRateLimiter, - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ - DB: options.Database, - OAuth2Configs: oauthConfigs, - // Optional is true to allow for public apps. If an - // authorization check fails and the user is not authenticated, - // they will be redirected to the login page by the app handler. - RedirectToLogin: false, - DisableSessionExpiryRefresh: options.DeploymentConfig.DisableSessionExpiryRefresh.Value, - Optional: true, - }), - httpmw.AsAuthzSystem( - // Redirect to the login page if the user tries to open an app with - // "me" as the username and they are not logged in. - httpmw.ExtractUserParam(api.Database, true), - // Extracts the from the url - httpmw.ExtractWorkspaceAndAgentParam(api.Database), - ), - ) + // Workspace apps do their own auth. + r.Use(apiRateLimiter) r.HandleFunc("/*", api.workspaceAppsProxyPath) } // %40 is the encoded character of the @ symbol. VS Code Web does @@ -742,9 +727,10 @@ type API struct { WebsocketWaitGroup sync.WaitGroup derpCloseFunc func() - metricsCache *metricscache.Cache - workspaceAgentCache *wsconncache.Cache - updateChecker *updatecheck.Checker + metricsCache *metricscache.Cache + workspaceAgentCache *wsconncache.Cache + updateChecker *updatecheck.Checker + WorkspaceAppsProvider *workspaceapps.Provider // 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 e0b9a53d4e..26b0149c80 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -11,6 +11,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/base64" + "encoding/hex" "encoding/json" "encoding/pem" "errors" @@ -82,6 +83,10 @@ import ( "github.com/coder/coder/testutil" ) +// AppSigningKey is a 64-byte key used to sign JWTs for workspace app tickets in +// tests. +var AppSigningKey = must(hex.DecodeString("64656164626565666465616462656566646561646265656664656164626565666465616462656566646561646265656664656164626565666465616462656566")) + type Options struct { // AccessURL denotes a custom access URL. By default we use the httptest // server's URL. Setting this may result in unexpected behavior (especially @@ -330,6 +335,7 @@ func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.Can DeploymentConfig: options.DeploymentConfig, UpdateCheckOptions: options.UpdateCheckOptions, SwaggerEndpoint: options.SwaggerEndpoint, + AppSigningKey: AppSigningKey, } } diff --git a/coderd/database/dbauthz/querier.go b/coderd/database/dbauthz/querier.go index 94b36e4306..9e71fd11d0 100644 --- a/coderd/database/dbauthz/querier.go +++ b/coderd/database/dbauthz/querier.go @@ -19,6 +19,14 @@ func (q *querier) Ping(ctx context.Context) (time.Duration, error) { return q.db.Ping(ctx) } +func (q *querier) AcquireLock(ctx context.Context, id int64) error { + return q.db.AcquireLock(ctx, id) +} + +func (q *querier) TryAcquireLock(ctx context.Context, id int64) (bool, error) { + return q.db.TryAcquireLock(ctx, id) +} + // InTx runs the given function in a transaction. func (q *querier) InTx(function func(querier database.Store) error, txOpts *sql.TxOptions) error { return q.db.InTx(func(tx database.Store) error { @@ -317,6 +325,16 @@ func (q *querier) GetLogoURL(ctx context.Context) (string, error) { return q.db.GetLogoURL(ctx) } +func (q *querier) GetAppSigningKey(ctx context.Context) (string, error) { + // No authz checks + return q.db.GetAppSigningKey(ctx) +} + +func (q *querier) InsertAppSigningKey(ctx context.Context, data string) error { + // No authz checks as this is done during startup + return q.db.InsertAppSigningKey(ctx, data) +} + func (q *querier) GetServiceBanner(ctx context.Context) (string, error) { // No authz checks return q.db.GetServiceBanner(ctx) diff --git a/coderd/database/dbfake/databasefake.go b/coderd/database/dbfake/databasefake.go index baeb982e25..4fe32880f1 100644 --- a/coderd/database/dbfake/databasefake.go +++ b/coderd/database/dbfake/databasefake.go @@ -64,6 +64,7 @@ func New() database.Store { workspaceApps: make([]database.WorkspaceApp, 0), workspaces: make([]database.Workspace, 0), licenses: make([]database.License, 0), + locks: map[int64]struct{}{}, }, } } @@ -89,6 +90,11 @@ type fakeQuerier struct { *data } +type fakeTx struct { + *fakeQuerier + locks map[int64]struct{} +} + type data struct { // Legacy tables apiKeys []database.APIKey @@ -124,11 +130,15 @@ type data struct { workspaceResources []database.WorkspaceResource workspaces []database.Workspace + // Locks is a map of lock names. Any keys within the map are currently + // locked. + locks map[int64]struct{} deploymentID string derpMeshKey string lastUpdateCheck []byte serviceBanner []byte logoURL string + appSigningKey string lastLicenseID int32 } @@ -196,11 +206,50 @@ func (*fakeQuerier) Ping(_ context.Context) (time.Duration, error) { return 0, nil } +func (*fakeQuerier) AcquireLock(_ context.Context, _ int64) error { + return xerrors.New("AcquireLock must only be called within a transaction") +} + +func (*fakeQuerier) TryAcquireLock(_ context.Context, _ int64) (bool, error) { + return false, xerrors.New("TryAcquireLock must only be called within a transaction") +} + +func (tx *fakeTx) AcquireLock(_ context.Context, id int64) error { + if _, ok := tx.fakeQuerier.locks[id]; ok { + return xerrors.Errorf("cannot acquire lock %d: already held", id) + } + tx.fakeQuerier.locks[id] = struct{}{} + tx.locks[id] = struct{}{} + return nil +} + +func (tx *fakeTx) TryAcquireLock(_ context.Context, id int64) (bool, error) { + if _, ok := tx.fakeQuerier.locks[id]; ok { + return false, nil + } + tx.fakeQuerier.locks[id] = struct{}{} + tx.locks[id] = struct{}{} + return true, nil +} + +func (tx *fakeTx) releaseLocks() { + for id := range tx.locks { + delete(tx.fakeQuerier.locks, id) + } + tx.locks = map[int64]struct{}{} +} + // InTx doesn't rollback data properly for in-memory yet. func (q *fakeQuerier) InTx(fn func(database.Store) error, _ *sql.TxOptions) error { q.mutex.Lock() defer q.mutex.Unlock() - return fn(&fakeQuerier{mutex: inTxMutex{}, data: q.data}) + tx := &fakeTx{ + fakeQuerier: &fakeQuerier{mutex: inTxMutex{}, data: q.data}, + locks: map[int64]struct{}{}, + } + defer tx.releaseLocks() + + return fn(tx) } func (q *fakeQuerier) AcquireProvisionerJob(_ context.Context, arg database.AcquireProvisionerJobParams) (database.ProvisionerJob, error) { @@ -4004,6 +4053,21 @@ func (q *fakeQuerier) GetLogoURL(_ context.Context) (string, error) { return q.logoURL, nil } +func (q *fakeQuerier) GetAppSigningKey(_ context.Context) (string, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + + return q.appSigningKey, nil +} + +func (q *fakeQuerier) InsertAppSigningKey(_ context.Context, data string) error { + q.mutex.Lock() + defer q.mutex.Unlock() + + q.appSigningKey = data + return nil +} + func (q *fakeQuerier) InsertLicense( _ context.Context, arg database.InsertLicenseParams, ) (database.License, error) { diff --git a/coderd/database/lock.go b/coderd/database/lock.go new file mode 100644 index 0000000000..56675282f9 --- /dev/null +++ b/coderd/database/lock.go @@ -0,0 +1,8 @@ +package database + +// Well-known lock IDs for lock functions in the database. These should not +// change. If locks are deprecated, they should be kept to avoid reusing the +// same ID. +const ( + LockIDDeploymentSetup = iota + 1 +) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 77e83d0ad2..e90e2636a3 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -12,6 +12,13 @@ import ( ) type sqlcQuerier interface { + // Blocks until the lock is acquired. + // + // This must be called from within a transaction. The lock will be automatically + // released when the transaction ends. + // + // Use database.LockID() to generate a unique lock ID from a string. + AcquireLock(ctx context.Context, pgAdvisoryXactLock int64) error // Acquires the lock for a single job that isn't started, completed, // canceled, and that matches an array of provisioner types. // @@ -36,6 +43,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) // GetAuditLogsBefore retrieves `row_limit` number of audit logs before the provided // ID. GetAuditLogsOffset(ctx context.Context, arg GetAuditLogsOffsetParams) ([]GetAuditLogsOffsetRow, error) @@ -139,6 +147,7 @@ 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 @@ -177,6 +186,13 @@ type sqlcQuerier interface { InsertWorkspaceResourceMetadata(ctx context.Context, arg InsertWorkspaceResourceMetadataParams) ([]WorkspaceResourceMetadatum, error) ParameterValue(ctx context.Context, id uuid.UUID) (ParameterValue, error) ParameterValues(ctx context.Context, arg ParameterValuesParams) ([]ParameterValue, error) + // Non blocking lock. Returns true if the lock was acquired, false otherwise. + // + // This must be called from within a transaction. The lock will be automatically + // released when the transaction ends. + // + // Use database.LockID() to generate a unique lock ID from a string. + TryAcquireLock(ctx context.Context, pgTryAdvisoryXactLock int64) (bool, error) UpdateAPIKeyByID(ctx context.Context, arg UpdateAPIKeyByIDParams) error UpdateGitAuthLink(ctx context.Context, arg UpdateGitAuthLinkParams) (GitAuthLink, error) UpdateGitSSHKey(ctx context.Context, arg UpdateGitSSHKeyParams) (GitSSHKey, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index ddf388a2fe..6ec74a10b8 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1437,6 +1437,38 @@ func (q *sqlQuerier) InsertLicense(ctx context.Context, arg InsertLicenseParams) return i, err } +const acquireLock = `-- name: AcquireLock :exec +SELECT pg_advisory_xact_lock($1) +` + +// Blocks until the lock is acquired. +// +// This must be called from within a transaction. The lock will be automatically +// released when the transaction ends. +// +// Use database.LockID() to generate a unique lock ID from a string. +func (q *sqlQuerier) AcquireLock(ctx context.Context, pgAdvisoryXactLock int64) error { + _, err := q.db.ExecContext(ctx, acquireLock, pgAdvisoryXactLock) + return err +} + +const tryAcquireLock = `-- name: TryAcquireLock :one +SELECT pg_try_advisory_xact_lock($1) +` + +// Non blocking lock. Returns true if the lock was acquired, false otherwise. +// +// This must be called from within a transaction. The lock will be automatically +// released when the transaction ends. +// +// Use database.LockID() to generate a unique lock ID from a string. +func (q *sqlQuerier) TryAcquireLock(ctx context.Context, pgTryAdvisoryXactLock int64) (bool, error) { + row := q.db.QueryRowContext(ctx, tryAcquireLock, pgTryAdvisoryXactLock) + var pg_try_advisory_xact_lock bool + err := row.Scan(&pg_try_advisory_xact_lock) + return pg_try_advisory_xact_lock, err +} + const getOrganizationIDsByMemberIDs = `-- name: GetOrganizationIDsByMemberIDs :many SELECT user_id, array_agg(organization_id) :: uuid [ ] AS "organization_IDs" @@ -2909,6 +2941,17 @@ func (q *sqlQuerier) UpdateReplica(ctx context.Context, arg UpdateReplicaParams) return i, err } +const getAppSigningKey = `-- name: GetAppSigningKey :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) + var value string + err := row.Scan(&value) + return value, err +} + const getDERPMeshKey = `-- name: GetDERPMeshKey :one SELECT value FROM site_configs WHERE key = 'derp_mesh_key' ` @@ -2964,6 +3007,15 @@ 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) ` diff --git a/coderd/database/queries/lock.sql b/coderd/database/queries/lock.sql new file mode 100644 index 0000000000..2421c73eda --- /dev/null +++ b/coderd/database/queries/lock.sql @@ -0,0 +1,17 @@ +-- name: AcquireLock :exec +-- Blocks until the lock is acquired. +-- +-- This must be called from within a transaction. The lock will be automatically +-- released when the transaction ends. +-- +-- Use database.LockID() to generate a unique lock ID from a string. +SELECT pg_advisory_xact_lock($1); + +-- name: TryAcquireLock :one +-- Non blocking lock. Returns true if the lock was acquired, false otherwise. +-- +-- This must be called from within a transaction. The lock will be automatically +-- released when the transaction ends. +-- +-- Use database.LockID() to generate a unique lock ID from a string. +SELECT pg_try_advisory_xact_lock($1); diff --git a/coderd/database/queries/siteconfig.sql b/coderd/database/queries/siteconfig.sql index 9b6f47185f..a2b5427f70 100644 --- a/coderd/database/queries/siteconfig.sql +++ b/coderd/database/queries/siteconfig.sql @@ -30,3 +30,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 +SELECT value FROM site_configs WHERE key = 'app_signing_key'; + +-- name: InsertAppSigningKey :exec +INSERT INTO site_configs (key, value) VALUES ('app_signing_key', $1); diff --git a/coderd/httpapi/cookie.go b/coderd/httpapi/cookie.go index 06e5d6ed77..51cdeec6a7 100644 --- a/coderd/httpapi/cookie.go +++ b/coderd/httpapi/cookie.go @@ -22,7 +22,9 @@ func StripCoderCookies(header string) string { name, _, _ := strings.Cut(part, "=") if name == codersdk.SessionTokenCookie || name == codersdk.OAuth2StateCookie || - name == codersdk.OAuth2RedirectCookie { + name == codersdk.OAuth2RedirectCookie || + name == codersdk.DevURLSessionTokenCookie || + name == codersdk.DevURLSessionTicketCookie { continue } cookies = append(cookies, part) diff --git a/coderd/httpapi/url.go b/coderd/httpapi/url.go index ecef3d4f5e..d27abca5de 100644 --- a/coderd/httpapi/url.go +++ b/coderd/httpapi/url.go @@ -4,7 +4,6 @@ import ( "fmt" "net" "regexp" - "strconv" "strings" "golang.org/x/xerrors" @@ -23,9 +22,7 @@ var ( // ApplicationURL is a parsed application URL hostname. type ApplicationURL struct { - // Only one of AppSlug or Port will be set. - AppSlug string - Port uint16 + AppSlugOrPort string AgentName string WorkspaceName string Username string @@ -34,12 +31,7 @@ type ApplicationURL struct { // String returns the application URL hostname without scheme. You will likely // want to append a period and the base hostname. func (a ApplicationURL) String() string { - appSlugOrPort := a.AppSlug - if a.Port != 0 { - appSlugOrPort = strconv.Itoa(int(a.Port)) - } - - return fmt.Sprintf("%s--%s--%s--%s", appSlugOrPort, a.AgentName, a.WorkspaceName, a.Username) + return fmt.Sprintf("%s--%s--%s--%s", a.AppSlugOrPort, a.AgentName, a.WorkspaceName, a.Username) } // ParseSubdomainAppURL parses an ApplicationURL from the given subdomain. If @@ -60,29 +52,14 @@ func ParseSubdomainAppURL(subdomain string) (ApplicationURL, error) { } matchGroup := matches[0] - appSlug, port := AppSlugOrPort(matchGroup[appURL.SubexpIndex("AppSlug")]) return ApplicationURL{ - AppSlug: appSlug, - Port: port, + AppSlugOrPort: matchGroup[appURL.SubexpIndex("AppSlug")], AgentName: matchGroup[appURL.SubexpIndex("AgentName")], WorkspaceName: matchGroup[appURL.SubexpIndex("WorkspaceName")], Username: matchGroup[appURL.SubexpIndex("Username")], }, nil } -// AppSlugOrPort takes a string and returns either the input string or a port -// number. -func AppSlugOrPort(val string) (string, uint16) { - port, err := strconv.ParseUint(val, 10, 16) - if err != nil || port == 0 { - port = 0 - } else { - val = "" - } - - return val, uint16(port) -} - // HostnamesMatch returns true if the hostnames are equal, disregarding // capitalization, extra leading or trailing periods, and ports. func HostnamesMatch(a, b string) bool { diff --git a/coderd/httpapi/url_test.go b/coderd/httpapi/url_test.go index 84cfcac7d3..3beee451f7 100644 --- a/coderd/httpapi/url_test.go +++ b/coderd/httpapi/url_test.go @@ -25,8 +25,7 @@ func TestApplicationURLString(t *testing.T) { { Name: "AppName", URL: httpapi.ApplicationURL{ - AppSlug: "app", - Port: 0, + AppSlugOrPort: "app", AgentName: "agent", WorkspaceName: "workspace", Username: "user", @@ -36,26 +35,13 @@ func TestApplicationURLString(t *testing.T) { { Name: "Port", URL: httpapi.ApplicationURL{ - AppSlug: "", - Port: 8080, + AppSlugOrPort: "8080", AgentName: "agent", WorkspaceName: "workspace", Username: "user", }, Expected: "8080--agent--workspace--user", }, - { - Name: "Both", - URL: httpapi.ApplicationURL{ - AppSlug: "app", - Port: 8080, - AgentName: "agent", - WorkspaceName: "workspace", - Username: "user", - }, - // Prioritizes port over app name. - Expected: "8080--agent--workspace--user", - }, } for _, c := range testCases { @@ -111,8 +97,7 @@ func TestParseSubdomainAppURL(t *testing.T) { Name: "AppName--Agent--Workspace--User", Subdomain: "app--agent--workspace--user", Expected: httpapi.ApplicationURL{ - AppSlug: "app", - Port: 0, + AppSlugOrPort: "app", AgentName: "agent", WorkspaceName: "workspace", Username: "user", @@ -122,8 +107,7 @@ func TestParseSubdomainAppURL(t *testing.T) { Name: "Port--Agent--Workspace--User", Subdomain: "8080--agent--workspace--user", Expected: httpapi.ApplicationURL{ - AppSlug: "", - Port: 8080, + AppSlugOrPort: "8080", AgentName: "agent", WorkspaceName: "workspace", Username: "user", @@ -133,8 +117,7 @@ func TestParseSubdomainAppURL(t *testing.T) { Name: "HyphenatedNames", Subdomain: "app-slug--agent-name--workspace-name--user-name", Expected: httpapi.ApplicationURL{ - AppSlug: "app-slug", - Port: 0, + AppSlugOrPort: "app-slug", AgentName: "agent-name", WorkspaceName: "workspace-name", Username: "user-name", diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go index 535466b264..d2afcf4a88 100644 --- a/coderd/httpmw/apikey.go +++ b/coderd/httpmw/apikey.go @@ -25,13 +25,6 @@ import ( "github.com/coder/coder/codersdk" ) -// The special cookie name used for subdomain-based application proxying. -// TODO: this will make dogfooding harder so come up with a more unique -// solution -// -//nolint:gosec -const DevURLSessionTokenCookie = "coder_devurl_session_token" - type apiKeyContextKey struct{} // APIKeyOptional may return an API key from the ExtractAPIKey handler. @@ -108,268 +101,281 @@ type ExtractAPIKeyConfig struct { Optional bool } -// ExtractAPIKey requires authentication using a valid API key. It handles -// extending an API key if it comes close to expiry, updating the last used time -// in the database. -// nolint:revive -func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler { +// ExtractAPIKeyMW calls ExtractAPIKey with the given config on each request, +// storing the result in the request context. +func ExtractAPIKeyMW(cfg ExtractAPIKeyConfig) 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() - // Write wraps writing a response to redirect if the handler - // specified it should. This redirect is used for user-facing pages - // like workspace applications. - write := func(code int, response codersdk.Response) { - if cfg.RedirectToLogin { - RedirectToLogin(rw, r, response.Message) - return - } - - httpapi.Write(ctx, rw, code, response) - } - - // optionalWrite wraps write, but will pass the request on to the - // next handler if the configuration says the API key is optional. - // - // It should be used when the API key is not provided or is invalid, - // but not when there are other errors. - optionalWrite := func(code int, response codersdk.Response) { - if cfg.Optional { - next.ServeHTTP(rw, r) - return - } - - write(code, response) - } - - token := apiTokenFromRequest(r) - if token == "" { - optionalWrite(http.StatusUnauthorized, codersdk.Response{ - Message: SignedOutErrorMessage, - Detail: fmt.Sprintf("Cookie %q or query parameter must be provided.", codersdk.SessionTokenCookie), - }) + keyPtr, authzPtr, ok := ExtractAPIKey(rw, r, cfg) + if !ok { return } - keyID, keySecret, err := SplitAPIToken(token) - if err != nil { - optionalWrite(http.StatusUnauthorized, codersdk.Response{ - Message: SignedOutErrorMessage, - Detail: "Invalid API key format: " + err.Error(), - }) - return - } - - //nolint:gocritic // System needs to fetch API key to check if it's valid. - key, err := cfg.DB.GetAPIKeyByID(dbauthz.AsSystemRestricted(ctx), keyID) - if err != nil { - if errors.Is(err, sql.ErrNoRows) { - optionalWrite(http.StatusUnauthorized, codersdk.Response{ - Message: SignedOutErrorMessage, - Detail: "API key is invalid.", - }) - return - } - write(http.StatusInternalServerError, codersdk.Response{ - Message: internalErrorMessage, - Detail: fmt.Sprintf("Internal error fetching API key by id. %s", err.Error()), - }) - return - } - - // Checking to see if the secret is valid. - hashedSecret := sha256.Sum256([]byte(keySecret)) - if subtle.ConstantTimeCompare(key.HashedSecret, hashedSecret[:]) != 1 { - optionalWrite(http.StatusUnauthorized, codersdk.Response{ - Message: SignedOutErrorMessage, - Detail: "API key secret is invalid.", - }) - return - } - - var ( - link database.UserLink - now = database.Now() - // Tracks if the API key has properties updated - changed = false - ) - if key.LoginType == database.LoginTypeGithub || key.LoginType == database.LoginTypeOIDC { - //nolint:gocritic // System needs to fetch UserLink to check if it's valid. - link, err = cfg.DB.GetUserLinkByUserIDLoginType(dbauthz.AsSystemRestricted(ctx), database.GetUserLinkByUserIDLoginTypeParams{ - UserID: key.UserID, - LoginType: key.LoginType, - }) - if err != nil { - write(http.StatusInternalServerError, codersdk.Response{ - Message: "A database error occurred", - Detail: fmt.Sprintf("get user link by user ID and login type: %s", err.Error()), - }) - return - } - // Check if the OAuth token is expired - if link.OAuthExpiry.Before(now) && !link.OAuthExpiry.IsZero() && link.OAuthRefreshToken != "" { - var oauthConfig OAuth2Config - switch key.LoginType { - case database.LoginTypeGithub: - oauthConfig = cfg.OAuth2Configs.Github - case database.LoginTypeOIDC: - oauthConfig = cfg.OAuth2Configs.OIDC - default: - write(http.StatusInternalServerError, codersdk.Response{ - Message: internalErrorMessage, - Detail: fmt.Sprintf("Unexpected authentication type %q.", key.LoginType), - }) - return - } - // If it is, let's refresh it from the provided config - token, err := oauthConfig.TokenSource(r.Context(), &oauth2.Token{ - AccessToken: link.OAuthAccessToken, - RefreshToken: link.OAuthRefreshToken, - Expiry: link.OAuthExpiry, - }).Token() - if err != nil { - write(http.StatusUnauthorized, codersdk.Response{ - Message: "Could not refresh expired Oauth token.", - Detail: err.Error(), - }) - return - } - link.OAuthAccessToken = token.AccessToken - link.OAuthRefreshToken = token.RefreshToken - link.OAuthExpiry = token.Expiry - key.ExpiresAt = token.Expiry - changed = true - } - } - - // Checking if the key is expired. - if key.ExpiresAt.Before(now) { - optionalWrite(http.StatusUnauthorized, codersdk.Response{ - Message: SignedOutErrorMessage, - Detail: fmt.Sprintf("API key expired at %q.", key.ExpiresAt.String()), - }) - return - } - - // Only update LastUsed once an hour to prevent database spam. - if now.Sub(key.LastUsed) > time.Hour { - key.LastUsed = now - remoteIP := net.ParseIP(r.RemoteAddr) - if remoteIP == nil { - remoteIP = net.IPv4(0, 0, 0, 0) - } - bitlen := len(remoteIP) * 8 - key.IPAddress = pqtype.Inet{ - IPNet: net.IPNet{ - IP: remoteIP, - Mask: net.CIDRMask(bitlen, bitlen), - }, - Valid: true, - } - changed = true - } - // Only update the ExpiresAt once an hour to prevent database spam. - // We extend the ExpiresAt to reduce re-authentication. - if !cfg.DisableSessionExpiryRefresh { - apiKeyLifetime := time.Duration(key.LifetimeSeconds) * time.Second - if key.ExpiresAt.Sub(now) <= apiKeyLifetime-time.Hour { - key.ExpiresAt = now.Add(apiKeyLifetime) - changed = true - } - } - if changed { - //nolint:gocritic // System needs to update API Key LastUsed - err := cfg.DB.UpdateAPIKeyByID(dbauthz.AsSystemRestricted(ctx), database.UpdateAPIKeyByIDParams{ - ID: key.ID, - LastUsed: key.LastUsed, - ExpiresAt: key.ExpiresAt, - IPAddress: key.IPAddress, - }) - if err != nil { - write(http.StatusInternalServerError, codersdk.Response{ - Message: internalErrorMessage, - Detail: fmt.Sprintf("API key couldn't update: %s.", err.Error()), - }) - return - } - // If the API Key is associated with a user_link (e.g. Github/OIDC) - // then we want to update the relevant oauth fields. - if link.UserID != uuid.Nil { - // nolint:gocritic - link, err = cfg.DB.UpdateUserLink(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLinkParams{ - UserID: link.UserID, - LoginType: link.LoginType, - OAuthAccessToken: link.OAuthAccessToken, - OAuthRefreshToken: link.OAuthRefreshToken, - OAuthExpiry: link.OAuthExpiry, - }) - if err != nil { - write(http.StatusInternalServerError, codersdk.Response{ - Message: internalErrorMessage, - Detail: fmt.Sprintf("update user_link: %s.", err.Error()), - }) - return - } - } - - // We only want to update this occasionally to reduce DB write - // load. We update alongside the UserLink and APIKey since it's - // easier on the DB to colocate writes. - // nolint:gocritic - _, err = cfg.DB.UpdateUserLastSeenAt(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLastSeenAtParams{ - ID: key.UserID, - LastSeenAt: database.Now(), - UpdatedAt: database.Now(), - }) - if err != nil { - write(http.StatusInternalServerError, codersdk.Response{ - Message: internalErrorMessage, - Detail: fmt.Sprintf("update user last_seen_at: %s", err.Error()), - }) - return - } - } - - // If the key is valid, we also fetch the user roles and status. - // The roles are used for RBAC authorize checks, and the status - // is to block 'suspended' users from accessing the platform. - // nolint:gocritic - roles, err := cfg.DB.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), key.UserID) - if err != nil { - write(http.StatusUnauthorized, codersdk.Response{ - Message: internalErrorMessage, - Detail: fmt.Sprintf("Internal error fetching user's roles. %s", err.Error()), - }) - return - } - - if roles.Status != database.UserStatusActive { - write(http.StatusUnauthorized, codersdk.Response{ - Message: fmt.Sprintf("User is not active (status = %q). Contact an admin to reactivate your account.", roles.Status), - }) + if keyPtr == nil || authzPtr == nil { + // Auth was optional and not provided. + next.ServeHTTP(rw, r) return } + key, authz := *keyPtr, *authzPtr // Actor is the user's authorization context. - actor := rbac.Subject{ - ID: key.UserID.String(), - Roles: rbac.RoleNames(roles.Roles), - Groups: roles.Groups, - Scope: rbac.ScopeName(key.Scope), - } + ctx := r.Context() ctx = context.WithValue(ctx, apiKeyContextKey{}, key) - ctx = context.WithValue(ctx, userAuthKey{}, Authorization{ - Username: roles.Username, - Actor: actor, - }) + ctx = context.WithValue(ctx, userAuthKey{}, authz) // Set the auth context for the authzquerier as well. - ctx = dbauthz.As(ctx, actor) + ctx = dbauthz.As(ctx, authz.Actor) next.ServeHTTP(rw, r.WithContext(ctx)) }) } } +// ExtractAPIKey requires authentication using a valid API key. It handles +// extending an API key if it comes close to expiry, updating the last used time +// in the database. +// +// If the configuration specifies that the API key is optional, a nil API key +// and authz object may be returned. False is returned if a response was written +// to the request and the caller should give up. +// nolint:revive +func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyConfig) (*database.APIKey, *Authorization, bool) { + ctx := r.Context() + // Write wraps writing a response to redirect if the handler + // specified it should. This redirect is used for user-facing pages + // like workspace applications. + write := func(code int, response codersdk.Response) (*database.APIKey, *Authorization, bool) { + if cfg.RedirectToLogin { + RedirectToLogin(rw, r, response.Message) + return nil, nil, false + } + + httpapi.Write(ctx, rw, code, response) + return nil, nil, false + } + + // optionalWrite wraps write, but will return nil, true if the API key is + // optional. + // + // It should be used when the API key is not provided or is invalid, + // but not when there are other errors. + optionalWrite := func(code int, response codersdk.Response) (*database.APIKey, *Authorization, bool) { + if cfg.Optional { + return nil, nil, true + } + + write(code, response) + return nil, nil, false + } + + token := apiTokenFromRequest(r) + if token == "" { + return optionalWrite(http.StatusUnauthorized, codersdk.Response{ + Message: SignedOutErrorMessage, + Detail: fmt.Sprintf("Cookie %q or query parameter must be provided.", codersdk.SessionTokenCookie), + }) + } + + keyID, keySecret, err := SplitAPIToken(token) + if err != nil { + return optionalWrite(http.StatusUnauthorized, codersdk.Response{ + Message: SignedOutErrorMessage, + Detail: "Invalid API key format: " + err.Error(), + }) + } + + //nolint:gocritic // System needs to fetch API key to check if it's valid. + key, err := cfg.DB.GetAPIKeyByID(dbauthz.AsSystemRestricted(ctx), keyID) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return optionalWrite(http.StatusUnauthorized, codersdk.Response{ + Message: SignedOutErrorMessage, + Detail: "API key is invalid.", + }) + } + + return write(http.StatusInternalServerError, codersdk.Response{ + Message: internalErrorMessage, + Detail: fmt.Sprintf("Internal error fetching API key by id. %s", err.Error()), + }) + } + + // Checking to see if the secret is valid. + hashedSecret := sha256.Sum256([]byte(keySecret)) + if subtle.ConstantTimeCompare(key.HashedSecret, hashedSecret[:]) != 1 { + return optionalWrite(http.StatusUnauthorized, codersdk.Response{ + Message: SignedOutErrorMessage, + Detail: "API key secret is invalid.", + }) + } + + var ( + link database.UserLink + now = database.Now() + // Tracks if the API key has properties updated + changed = false + ) + if key.LoginType == database.LoginTypeGithub || key.LoginType == database.LoginTypeOIDC { + //nolint:gocritic // System needs to fetch UserLink to check if it's valid. + link, err = cfg.DB.GetUserLinkByUserIDLoginType(dbauthz.AsSystemRestricted(ctx), database.GetUserLinkByUserIDLoginTypeParams{ + UserID: key.UserID, + LoginType: key.LoginType, + }) + if err != nil { + return write(http.StatusInternalServerError, codersdk.Response{ + Message: "A database error occurred", + Detail: fmt.Sprintf("get user link by user ID and login type: %s", err.Error()), + }) + } + // Check if the OAuth token is expired + if link.OAuthExpiry.Before(now) && !link.OAuthExpiry.IsZero() && link.OAuthRefreshToken != "" { + var oauthConfig OAuth2Config + switch key.LoginType { + case database.LoginTypeGithub: + oauthConfig = cfg.OAuth2Configs.Github + case database.LoginTypeOIDC: + oauthConfig = cfg.OAuth2Configs.OIDC + default: + return write(http.StatusInternalServerError, codersdk.Response{ + Message: internalErrorMessage, + Detail: fmt.Sprintf("Unexpected authentication type %q.", key.LoginType), + }) + } + // If it is, let's refresh it from the provided config + token, err := oauthConfig.TokenSource(r.Context(), &oauth2.Token{ + AccessToken: link.OAuthAccessToken, + RefreshToken: link.OAuthRefreshToken, + Expiry: link.OAuthExpiry, + }).Token() + if err != nil { + return write(http.StatusUnauthorized, codersdk.Response{ + Message: "Could not refresh expired Oauth token.", + Detail: err.Error(), + }) + } + link.OAuthAccessToken = token.AccessToken + link.OAuthRefreshToken = token.RefreshToken + link.OAuthExpiry = token.Expiry + key.ExpiresAt = token.Expiry + changed = true + } + } + + // Checking if the key is expired. + if key.ExpiresAt.Before(now) { + return optionalWrite(http.StatusUnauthorized, codersdk.Response{ + Message: SignedOutErrorMessage, + Detail: fmt.Sprintf("API key expired at %q.", key.ExpiresAt.String()), + }) + } + + // Only update LastUsed once an hour to prevent database spam. + if now.Sub(key.LastUsed) > time.Hour { + key.LastUsed = now + remoteIP := net.ParseIP(r.RemoteAddr) + if remoteIP == nil { + remoteIP = net.IPv4(0, 0, 0, 0) + } + bitlen := len(remoteIP) * 8 + key.IPAddress = pqtype.Inet{ + IPNet: net.IPNet{ + IP: remoteIP, + Mask: net.CIDRMask(bitlen, bitlen), + }, + Valid: true, + } + changed = true + } + // Only update the ExpiresAt once an hour to prevent database spam. + // We extend the ExpiresAt to reduce re-authentication. + if !cfg.DisableSessionExpiryRefresh { + apiKeyLifetime := time.Duration(key.LifetimeSeconds) * time.Second + if key.ExpiresAt.Sub(now) <= apiKeyLifetime-time.Hour { + key.ExpiresAt = now.Add(apiKeyLifetime) + changed = true + } + } + if changed { + //nolint:gocritic // System needs to update API Key LastUsed + err := cfg.DB.UpdateAPIKeyByID(dbauthz.AsSystemRestricted(ctx), database.UpdateAPIKeyByIDParams{ + ID: key.ID, + LastUsed: key.LastUsed, + ExpiresAt: key.ExpiresAt, + IPAddress: key.IPAddress, + }) + if err != nil { + return write(http.StatusInternalServerError, codersdk.Response{ + Message: internalErrorMessage, + Detail: fmt.Sprintf("API key couldn't update: %s.", err.Error()), + }) + } + // If the API Key is associated with a user_link (e.g. Github/OIDC) + // then we want to update the relevant oauth fields. + if link.UserID != uuid.Nil { + // nolint:gocritic + link, err = cfg.DB.UpdateUserLink(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLinkParams{ + UserID: link.UserID, + LoginType: link.LoginType, + OAuthAccessToken: link.OAuthAccessToken, + OAuthRefreshToken: link.OAuthRefreshToken, + OAuthExpiry: link.OAuthExpiry, + }) + if err != nil { + return write(http.StatusInternalServerError, codersdk.Response{ + Message: internalErrorMessage, + Detail: fmt.Sprintf("update user_link: %s.", err.Error()), + }) + } + } + + // We only want to update this occasionally to reduce DB write + // load. We update alongside the UserLink and APIKey since it's + // easier on the DB to colocate writes. + // nolint:gocritic + _, err = cfg.DB.UpdateUserLastSeenAt(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLastSeenAtParams{ + ID: key.UserID, + LastSeenAt: database.Now(), + UpdatedAt: database.Now(), + }) + if err != nil { + return write(http.StatusInternalServerError, codersdk.Response{ + Message: internalErrorMessage, + Detail: fmt.Sprintf("update user last_seen_at: %s", err.Error()), + }) + } + } + + // If the key is valid, we also fetch the user roles and status. + // The roles are used for RBAC authorize checks, and the status + // is to block 'suspended' users from accessing the platform. + // nolint:gocritic + roles, err := cfg.DB.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), key.UserID) + if err != nil { + return write(http.StatusUnauthorized, codersdk.Response{ + Message: internalErrorMessage, + Detail: fmt.Sprintf("Internal error fetching user's roles. %s", err.Error()), + }) + } + + if roles.Status != database.UserStatusActive { + return write(http.StatusUnauthorized, codersdk.Response{ + Message: fmt.Sprintf("User is not active (status = %q). Contact an admin to reactivate your account.", roles.Status), + }) + } + + // Actor is the user's authorization context. + authz := Authorization{ + Username: roles.Username, + Actor: rbac.Subject{ + ID: key.UserID.String(), + Roles: rbac.RoleNames(roles.Roles), + Groups: roles.Groups, + Scope: rbac.ScopeName(key.Scope), + }, + } + + return &key, &authz, true +} + // apiTokenFromRequest returns the api token from the request. // Find the session token from: // 1: The cookie @@ -393,7 +399,7 @@ func apiTokenFromRequest(r *http.Request) string { return headerValue } - cookie, err = r.Cookie(DevURLSessionTokenCookie) + cookie, err = r.Cookie(codersdk.DevURLSessionTokenCookie) if err == nil && cookie.Value != "" { return cookie.Value } diff --git a/coderd/httpmw/apikey_test.go b/coderd/httpmw/apikey_test.go index f3573ab348..588acce9d6 100644 --- a/coderd/httpmw/apikey_test.go +++ b/coderd/httpmw/apikey_test.go @@ -47,7 +47,7 @@ func TestAPIKey(t *testing.T) { r = httptest.NewRequest("GET", "/", nil) rw = httptest.NewRecorder() ) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(successHandler).ServeHTTP(rw, r) @@ -63,7 +63,7 @@ func TestAPIKey(t *testing.T) { r = httptest.NewRequest("GET", "/", nil) rw = httptest.NewRecorder() ) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: true, })(successHandler).ServeHTTP(rw, r) @@ -84,7 +84,7 @@ func TestAPIKey(t *testing.T) { ) r.Header.Set(codersdk.SessionTokenHeader, "test-wow-hello") - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(successHandler).ServeHTTP(rw, r) @@ -102,7 +102,7 @@ func TestAPIKey(t *testing.T) { ) r.Header.Set(codersdk.SessionTokenHeader, "test-wow") - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(successHandler).ServeHTTP(rw, r) @@ -120,7 +120,7 @@ func TestAPIKey(t *testing.T) { ) r.Header.Set(codersdk.SessionTokenHeader, "testtestid-wow") - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(successHandler).ServeHTTP(rw, r) @@ -139,7 +139,7 @@ func TestAPIKey(t *testing.T) { ) r.Header.Set(codersdk.SessionTokenHeader, fmt.Sprintf("%s-%s", id, secret)) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(successHandler).ServeHTTP(rw, r) @@ -164,7 +164,7 @@ func TestAPIKey(t *testing.T) { }) ) r.Header.Set(codersdk.SessionTokenHeader, token) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(successHandler).ServeHTTP(rw, r) @@ -188,7 +188,7 @@ func TestAPIKey(t *testing.T) { ) r.Header.Set(codersdk.SessionTokenHeader, token) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(successHandler).ServeHTTP(rw, r) @@ -212,7 +212,7 @@ func TestAPIKey(t *testing.T) { ) r.Header.Set(codersdk.SessionTokenHeader, token) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { @@ -251,7 +251,7 @@ func TestAPIKey(t *testing.T) { Value: token, }) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { @@ -286,7 +286,7 @@ func TestAPIKey(t *testing.T) { q.Add(codersdk.SessionTokenCookie, token) r.URL.RawQuery = q.Encode() - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { @@ -317,7 +317,7 @@ func TestAPIKey(t *testing.T) { ) r.Header.Set(codersdk.SessionTokenHeader, token) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(successHandler).ServeHTTP(rw, r) @@ -348,7 +348,7 @@ func TestAPIKey(t *testing.T) { ) r.Header.Set(codersdk.SessionTokenHeader, token) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(successHandler).ServeHTTP(rw, r) @@ -379,7 +379,7 @@ func TestAPIKey(t *testing.T) { ) r.Header.Set(codersdk.SessionTokenHeader, token) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, DisableSessionExpiryRefresh: true, @@ -416,7 +416,7 @@ func TestAPIKey(t *testing.T) { ) r.Header.Set(codersdk.SessionTokenHeader, token) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(successHandler).ServeHTTP(rw, r) @@ -459,7 +459,7 @@ func TestAPIKey(t *testing.T) { RefreshToken: "moo", Expiry: database.Now().AddDate(0, 0, 1), } - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, OAuth2Configs: &httpmw.OAuth2Configs{ Github: &oauth2Config{ @@ -498,7 +498,7 @@ func TestAPIKey(t *testing.T) { r.RemoteAddr = "1.1.1.1" r.Header.Set(codersdk.SessionTokenHeader, token) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(successHandler).ServeHTTP(rw, r) @@ -520,7 +520,7 @@ func TestAPIKey(t *testing.T) { rw = httptest.NewRecorder() ) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: true, })(successHandler).ServeHTTP(rw, r) @@ -552,7 +552,7 @@ func TestAPIKey(t *testing.T) { }) ) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, Optional: true, @@ -581,7 +581,7 @@ func TestAPIKey(t *testing.T) { ) r.Header.Set(codersdk.SessionTokenHeader, token) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(successHandler).ServeHTTP(rw, r) diff --git a/coderd/httpmw/authorize_test.go b/coderd/httpmw/authorize_test.go index d715c7e313..a0c0baaf19 100644 --- a/coderd/httpmw/authorize_test.go +++ b/coderd/httpmw/authorize_test.go @@ -118,7 +118,7 @@ func TestExtractUserRoles(t *testing.T) { rtr = chi.NewRouter() ) rtr.Use( - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, OAuth2Configs: &httpmw.OAuth2Configs{}, RedirectToLogin: false, diff --git a/coderd/httpmw/organizationparam_test.go b/coderd/httpmw/organizationparam_test.go index 1e73af9898..b566b06758 100644 --- a/coderd/httpmw/organizationparam_test.go +++ b/coderd/httpmw/organizationparam_test.go @@ -43,7 +43,7 @@ func TestOrganizationParam(t *testing.T) { rtr = chi.NewRouter() ) rtr.Use( - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, }), @@ -66,7 +66,7 @@ func TestOrganizationParam(t *testing.T) { ) chi.RouteContext(r.Context()).URLParams.Add("organization", uuid.NewString()) rtr.Use( - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, }), @@ -89,7 +89,7 @@ func TestOrganizationParam(t *testing.T) { ) chi.RouteContext(r.Context()).URLParams.Add("organization", "not-a-uuid") rtr.Use( - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, }), @@ -120,7 +120,7 @@ func TestOrganizationParam(t *testing.T) { chi.RouteContext(r.Context()).URLParams.Add("organization", organization.ID.String()) chi.RouteContext(r.Context()).URLParams.Add("user", u.ID.String()) rtr.Use( - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, }), @@ -151,7 +151,7 @@ func TestOrganizationParam(t *testing.T) { chi.RouteContext(r.Context()).URLParams.Add("organization", organization.ID.String()) chi.RouteContext(r.Context()).URLParams.Add("user", user.ID.String()) rtr.Use( - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, }), diff --git a/coderd/httpmw/ratelimit_test.go b/coderd/httpmw/ratelimit_test.go index cf276fa4bf..a201634edb 100644 --- a/coderd/httpmw/ratelimit_test.go +++ b/coderd/httpmw/ratelimit_test.go @@ -76,7 +76,7 @@ func TestRateLimit(t *testing.T) { _, key := dbgen.APIKey(t, db, database.APIKey{UserID: u.ID}) rtr := chi.NewRouter() - rtr.Use(httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + rtr.Use(httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, Optional: false, })) @@ -122,7 +122,7 @@ func TestRateLimit(t *testing.T) { _, key := dbgen.APIKey(t, db, database.APIKey{UserID: u.ID}) rtr := chi.NewRouter() - rtr.Use(httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + rtr.Use(httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, Optional: false, })) diff --git a/coderd/httpmw/templateparam_test.go b/coderd/httpmw/templateparam_test.go index 899aaa4fab..c630d5570b 100644 --- a/coderd/httpmw/templateparam_test.go +++ b/coderd/httpmw/templateparam_test.go @@ -95,7 +95,7 @@ func TestTemplateParam(t *testing.T) { db := dbfake.New() rtr := chi.NewRouter() rtr.Use( - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, }), diff --git a/coderd/httpmw/templateversionparam_test.go b/coderd/httpmw/templateversionparam_test.go index 5961f7e27c..febaa8f50b 100644 --- a/coderd/httpmw/templateversionparam_test.go +++ b/coderd/httpmw/templateversionparam_test.go @@ -82,7 +82,7 @@ func TestTemplateVersionParam(t *testing.T) { db := dbfake.New() rtr := chi.NewRouter() rtr.Use( - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, }), diff --git a/coderd/httpmw/userparam_test.go b/coderd/httpmw/userparam_test.go index aac7e224a7..4ce4e317f9 100644 --- a/coderd/httpmw/userparam_test.go +++ b/coderd/httpmw/userparam_test.go @@ -37,7 +37,7 @@ func TestUserParam(t *testing.T) { t.Parallel() db, rw, r := setup(t) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(http.HandlerFunc(func(rw http.ResponseWriter, returnedRequest *http.Request) { @@ -56,7 +56,7 @@ func TestUserParam(t *testing.T) { t.Parallel() db, rw, r := setup(t) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(http.HandlerFunc(func(rw http.ResponseWriter, returnedRequest *http.Request) { @@ -78,7 +78,7 @@ func TestUserParam(t *testing.T) { t.Parallel() db, rw, r := setup(t) - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, })(http.HandlerFunc(func(rw http.ResponseWriter, returnedRequest *http.Request) { diff --git a/coderd/httpmw/workspaceagentparam_test.go b/coderd/httpmw/workspaceagentparam_test.go index 62fd580a1e..f6a4c3fe1e 100644 --- a/coderd/httpmw/workspaceagentparam_test.go +++ b/coderd/httpmw/workspaceagentparam_test.go @@ -96,7 +96,7 @@ func TestWorkspaceAgentParam(t *testing.T) { db := dbfake.New() rtr := chi.NewRouter() rtr.Use( - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, }), diff --git a/coderd/httpmw/workspacebuildparam_test.go b/coderd/httpmw/workspacebuildparam_test.go index 00b64fd7a2..ac495537a1 100644 --- a/coderd/httpmw/workspacebuildparam_test.go +++ b/coderd/httpmw/workspacebuildparam_test.go @@ -78,7 +78,7 @@ func TestWorkspaceBuildParam(t *testing.T) { db := dbfake.New() rtr := chi.NewRouter() rtr.Use( - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, }), diff --git a/coderd/httpmw/workspaceparam_test.go b/coderd/httpmw/workspaceparam_test.go index f19b045c28..dd19abe8eb 100644 --- a/coderd/httpmw/workspaceparam_test.go +++ b/coderd/httpmw/workspaceparam_test.go @@ -101,7 +101,7 @@ func TestWorkspaceParam(t *testing.T) { db := dbfake.New() rtr := chi.NewRouter() rtr.Use( - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: false, }), @@ -302,7 +302,7 @@ func TestWorkspaceAgentByNameParam(t *testing.T) { rtr := chi.NewRouter() rtr.Use( - httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: db, RedirectToLogin: true, }), diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 195169378d..d3508df8c7 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -704,6 +704,8 @@ func TestDeleteTemplate(t *testing.T) { func TestTemplateMetrics(t *testing.T) { t.Parallel() + t.Skip("flaky test: https://github.com/coder/coder/issues/6481") + client := coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, AgentStatsRefreshInterval: time.Millisecond * 100, diff --git a/coderd/userauth.go b/coderd/userauth.go index b7ea35fd4f..55f065ae1f 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -197,11 +197,11 @@ func (api *API) postLogout(rw http.ResponseWriter, r *http.Request) { // 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(httpmw.DevURLSessionTokenCookie); appCookie != nil { + if appCookie, _ := r.Cookie(codersdk.DevURLSessionTokenCookie); appCookie != nil { appCookieRemove := &http.Cookie{ // MaxAge < 0 means to delete the cookie now. MaxAge: -1, - Name: httpmw.DevURLSessionTokenCookie, + Name: codersdk.DevURLSessionTokenCookie, Path: "/", Domain: "." + api.AccessURL.Hostname(), } diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index 5ec965db68..92bd2d8ac8 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -17,7 +17,6 @@ import ( "time" "github.com/go-chi/chi/v5" - "github.com/google/uuid" "go.opentelemetry.io/otel/trace" "golang.org/x/xerrors" jose "gopkg.in/square/go-jose.v2" @@ -29,6 +28,7 @@ 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/site" ) @@ -38,9 +38,6 @@ const ( // conflict with query parameters that users may use. //nolint:gosec subdomainProxyAPIKeyParam = "coder_application_connect_api_key_35e783" - // redirectURIQueryParam is the query param for the app URL to be passed - // back to the API auth endpoint on the main access URL. - redirectURIQueryParam = "redirect_uri" // appLogoutHostname is the hostname to use for the logout redirect. When // the dashboard logs out, it will redirect to this subdomain of the app // hostname, and the server will remove the cookie and redirect to the main @@ -66,13 +63,6 @@ var nonCanonicalHeaders = map[string]string{ "Sec-Websocket-Version": "Sec-WebSocket-Version", } -type workspaceAppAccessMethod string - -const ( - workspaceAppAccessMethodPath workspaceAppAccessMethod = "path" - workspaceAppAccessMethodSubdomain workspaceAppAccessMethod = "subdomain" -) - // @Summary Get applications host // @ID get-applications-host // @Security CoderSessionToken @@ -94,9 +84,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) { - workspace := httpmw.WorkspaceParam(r) - agent := httpmw.WorkspaceAgentParam(r) - if api.DeploymentConfig.DisablePathApps.Value { site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ Status: http.StatusUnauthorized, @@ -108,32 +95,24 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) return } - // We do not support port proxying on paths, so lookup the app by slug. - appSlug := chi.URLParam(r, "workspaceapp") - app, ok := api.lookupWorkspaceApp(rw, r, agent.ID, appSlug) - if !ok { - return - } - - appSharingLevel := database.AppSharingLevelOwner - if app.SharingLevel != "" { - appSharingLevel = app.SharingLevel - } - authed, ok := api.fetchWorkspaceApplicationAuth(rw, r, workspaceAppAccessMethodPath, workspace, appSharingLevel) - if !ok { - return - } - if !authed { - _, hasAPIKey := httpmw.APIKeyOptional(r) - if hasAPIKey { - // The request has a valid API key but insufficient permissions. - renderApplicationNotFound(rw, r, api.AccessURL) + // 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.DeploymentConfig.DisableSessionExpiryRefresh.Value, + }) + if !ok { return } - // Redirect to login as they don't have permission to access the app and - // they aren't signed in. - httpmw.RedirectToLogin(rw, r, httpmw.SignedOutErrorMessage) + http.Redirect(rw, r, strings.Replace(r.URL.Path, "@me", "@"+roles.Username, 1), http.StatusTemporaryRedirect) return } @@ -145,14 +124,20 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) chiPath = "/" + chiPath } - api.proxyWorkspaceApplication(proxyApplication{ - AccessMethod: workspaceAppAccessMethodPath, - Workspace: workspace, - Agent: agent, - App: &app, - Port: 0, - Path: chiPath, - }, rw, r) + ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(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, *ticket, chiPath) } // handleSubdomainApplications handles subdomain-based application proxy @@ -226,451 +211,64 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht return } - workspaceAgentKey := fmt.Sprintf("%s.%s", app.WorkspaceName, app.AgentName) - chiCtx := chi.RouteContext(ctx) - chiCtx.URLParams.Add("workspace_and_agent", workspaceAgentKey) - chiCtx.URLParams.Add("user", app.Username) - - // 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) { - workspace := httpmw.WorkspaceParam(r) - agent := httpmw.WorkspaceAgentParam(r) - - var workspaceAppPtr *database.WorkspaceApp - if app.AppSlug != "" { - workspaceApp, ok := api.lookupWorkspaceApp(rw, r, agent.ID, app.AppSlug) - if !ok { - return - } - - workspaceAppPtr = &workspaceApp - } - - // Verify application auth. This function will redirect or - // return an error page if the user doesn't have permission. - sharingLevel := database.AppSharingLevelOwner - if workspaceAppPtr != nil && workspaceAppPtr.SharingLevel != "" { - sharingLevel = workspaceAppPtr.SharingLevel - } - if !api.verifyWorkspaceApplicationSubdomainAuth(rw, r, host, workspace, sharingLevel) { + // 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.proxyWorkspaceApplication(proxyApplication{ - AccessMethod: workspaceAppAccessMethodSubdomain, - Workspace: workspace, - Agent: agent, - App: workspaceAppPtr, - Port: app.Port, - Path: r.URL.Path, - }, rw, r) + 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 + } + + ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(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, *ticket, r.URL.Path) })).ServeHTTP(rw, r.WithContext(ctx)) }) } } -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.handleWorkspaceAppLogout(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) handleWorkspaceAppLogout(rw http.ResponseWriter, r *http.Request) { - ctx := r.Context() - - // Delete the API key and cookie first before attempting to parse/validate - // the redirect URI. - cookie, err := r.Cookie(httpmw.DevURLSessionTokenCookie) - if err == nil && cookie.Value != "" { - id, secret, err := httpmw.SplitAPIToken(cookie.Value) - // If it's not a valid token then we don't need to delete it from the - // database, but we'll still delete the cookie. - if err == nil { - // To avoid a situation where someone overloads the API with - // different auth formats, and tricks this endpoint into deleting an - // unchecked API key, we validate that the secret matches the secret - // we store in the database. - //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(redirectURIQueryParam) - if redirectURI == "" { - redirectURI = api.AccessURL.String() - } else { - // Validate that the redirect URI is a valid URL and exists on the same - // host as the access URL or an app URL. - parsedRedirectURI, err := url.Parse(redirectURI) - if err != nil { - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusBadRequest, - Title: "Invalid redirect URI", - Description: fmt.Sprintf("Could not parse redirect URI %q: %s", redirectURI, err.Error()), - RetryEnabled: false, - DashboardURL: api.AccessURL.String(), - }) - return - } - - // Check if the redirect URI is on the same host as the access URL or an - // app URL. - ok := httpapi.HostnamesMatch(api.AccessURL.Hostname(), parsedRedirectURI.Hostname()) - if !ok && api.AppHostnameRegex != nil { - // We could also check that it's a valid application URL for - // completeness, but this check should be good enough. - _, ok = httpapi.ExecuteHostnamePattern(api.AppHostnameRegex, parsedRedirectURI.Hostname()) - } - if !ok { - // The redirect URI they provided is not allowed, but we don't want - // to return an error page because it'll interrupt the logout flow, - // so we just use the default access URL. - parsedRedirectURI = api.AccessURL - } - - redirectURI = parsedRedirectURI.String() - } - - http.Redirect(rw, r, redirectURI, http.StatusTemporaryRedirect) -} - -// lookupWorkspaceApp looks up the workspace application by slug in the given -// agent and returns it. If the application is not found or there was a server -// error while looking it up, an HTML error page is returned and false is -// returned so the caller can return early. -func (api *API) lookupWorkspaceApp(rw http.ResponseWriter, r *http.Request, agentID uuid.UUID, appSlug string) (database.WorkspaceApp, bool) { - // dbauthz.AsSystemRestricted is allowed here as the app authz is checked later. - // The app authz is determined by the sharing level. - //nolint:gocritic - app, err := api.Database.GetWorkspaceAppByAgentIDAndSlug(dbauthz.AsSystemRestricted(r.Context()), database.GetWorkspaceAppByAgentIDAndSlugParams{ - AgentID: agentID, - Slug: appSlug, - }) - if xerrors.Is(err, sql.ErrNoRows) { - renderApplicationNotFound(rw, r, api.AccessURL) - return database.WorkspaceApp{}, false - } - if err != nil { - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusInternalServerError, - Title: "Internal Server Error", - Description: "Could not fetch workspace application: " + err.Error(), - RetryEnabled: true, - DashboardURL: api.AccessURL.String(), - }) - return database.WorkspaceApp{}, false - } - - return app, true -} - -//nolint:revive -func (api *API) authorizeWorkspaceApp(r *http.Request, accessMethod workspaceAppAccessMethod, sharingLevel database.AppSharingLevel, workspace database.Workspace) (bool, error) { - ctx := r.Context() - - if accessMethod == "" { - accessMethod = workspaceAppAccessMethodPath - } - isPathApp := accessMethod == workspaceAppAccessMethodPath - - // If path-based app sharing is disabled (which is the default), we can - // force the sharing level to be "owner" so that the user can only access - // their own apps. - // - // Site owners are blocked from accessing path-based apps unless the - // Dangerous.AllowPathAppSiteOwnerAccess flag is enabled in the check below. - if isPathApp && !api.DeploymentConfig.Dangerous.AllowPathAppSharing.Value { - sharingLevel = database.AppSharingLevelOwner - } - - // Short circuit if not authenticated. - roles, ok := httpmw.UserAuthorizationOptional(r) - if !ok { - // The user is not authenticated, so they can only access the app if it - // is public. - return sharingLevel == database.AppSharingLevelPublic, nil - } - - // Block anyone from accessing workspaces they don't own in path-based apps - // unless the admin disables this security feature. This blocks site-owners - // from accessing any apps from any user's workspaces. - // - // When the Dangerous.AllowPathAppSharing flag is not enabled, the sharing - // level will be forced to "owner", so this check will always be true for - // workspaces owned by different users. - if isPathApp && - sharingLevel == database.AppSharingLevelOwner && - workspace.OwnerID.String() != roles.Actor.ID && - !api.DeploymentConfig.Dangerous.AllowPathAppSiteOwnerAccess.Value { - - return false, nil - } - - // Do a standard RBAC check. This accounts for share level "owner" and any - // other RBAC rules that may be in place. - // - // Regardless of share level or whether it's enabled or not, the owner of - // the workspace can always access applications (as long as their API key's - // scope allows it). - err := api.Authorizer.Authorize(ctx, roles.Actor, rbac.ActionCreate, workspace.ApplicationConnectRBAC()) - if err == nil { - return true, nil - } - - switch sharingLevel { - case database.AppSharingLevelOwner: - // We essentially already did this above with the regular RBAC check. - // Owners can always access their own apps according to RBAC rules, so - // they have already been returned from this function. - case database.AppSharingLevelAuthenticated: - // The user is authenticated at this point, but we need to make sure - // that they have ApplicationConnect permissions to their own - // workspaces. This ensures that the key's scope has permission to - // connect to workspace apps. - object := rbac.ResourceWorkspaceApplicationConnect.WithOwner(roles.Actor.ID) - err := api.Authorizer.Authorize(ctx, roles.Actor, rbac.ActionCreate, object) - if err == nil { - return true, nil - } - case database.AppSharingLevelPublic: - // We don't really care about scopes and stuff if it's public anyways. - // Someone with a restricted-scope API key could just not submit the - // API key cookie in the request and access the page. - return true, nil - } - - // No checks were successful. - return false, nil -} - -// fetchWorkspaceApplicationAuth authorizes the user using api.AppAuthorizer -// for a given app share level in the given workspace. The user's authorization -// status is returned. If a server error occurs, a HTML error page is rendered -// and false is returned so the caller can return early. -func (api *API) fetchWorkspaceApplicationAuth(rw http.ResponseWriter, r *http.Request, accessMethod workspaceAppAccessMethod, workspace database.Workspace, appSharingLevel database.AppSharingLevel) (authed bool, ok bool) { - ok, err := api.authorizeWorkspaceApp(r, accessMethod, appSharingLevel, workspace) - if err != nil { - api.Logger.Error(r.Context(), "authorize workspace app", slog.Error(err)) - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusInternalServerError, - Title: "Internal Server Error", - Description: "Could not verify authorization. Please try again or contact an administrator.", - RetryEnabled: true, - DashboardURL: api.AccessURL.String(), - }) - return false, false - } - - return ok, true -} - -// checkWorkspaceApplicationAuth authorizes the user using api.AppAuthorizer -// for a given app share level in the given workspace. If the user is not -// authorized or a server error occurs, a discrete HTML error page is rendered -// and false is returned so the caller can return early. -func (api *API) checkWorkspaceApplicationAuth(rw http.ResponseWriter, r *http.Request, accessMethod workspaceAppAccessMethod, workspace database.Workspace, appSharingLevel database.AppSharingLevel) bool { - authed, ok := api.fetchWorkspaceApplicationAuth(rw, r, accessMethod, workspace, appSharingLevel) - if !ok { - return false - } - if !authed { - renderApplicationNotFound(rw, r, api.AccessURL) - return false - } - - return true -} - -// verifyWorkspaceApplicationSubdomainAuth checks that the request is authorized -// to access the given application. If the user does not have a app session key, -// they will be redirected to the route below. If the user does have a session -// key but insufficient permissions a static error page will be rendered. -func (api *API) verifyWorkspaceApplicationSubdomainAuth(rw http.ResponseWriter, r *http.Request, host string, workspace database.Workspace, appSharingLevel database.AppSharingLevel) bool { - authed, ok := api.fetchWorkspaceApplicationAuth(rw, r, workspaceAppAccessMethodSubdomain, workspace, appSharingLevel) - if !ok { - return false - } - if authed { - return true - } - - _, hasAPIKey := httpmw.APIKeyOptional(r) - if hasAPIKey { - // The request has a valid API key but insufficient permissions. - renderApplicationNotFound(rw, r, api.AccessURL) - return false - } - - // 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 false - } - - 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 false - } - - // If the user doesn't have a session key, redirect them to the API endpoint - // for application auth. - redirectURI := *r.URL - redirectURI.Scheme = api.AccessURL.Scheme - redirectURI.Host = host - - u := *api.AccessURL - u.Path = "/api/v2/applications/auth-redirect" - q := u.Query() - q.Add(redirectURIQueryParam, redirectURI.String()) - u.RawQuery = q.Encode() - - http.Redirect(rw, r, u.String(), http.StatusTemporaryRedirect) - return false -} - -// setWorkspaceAppCookie sets a cookie on the workspace app domain. If the app -// hostname cannot be parsed properly, a static error page is rendered and false -// is returned. -// -// If an empty token is supplied, it will clear the cookie. -func (api *API) setWorkspaceAppCookie(rw http.ResponseWriter, r *http.Request, token string) bool { - hostSplit := strings.SplitN(api.AppHostname, ".", 2) - if len(hostSplit) != 2 { - // This should be impossible as we verify the app hostname on - // startup, but we'll check anyways. - api.Logger.Error(r.Context(), "could not split invalid app hostname", slog.F("hostname", api.AppHostname)) - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusInternalServerError, - Title: "Internal Server Error", - Description: "The app is configured with an invalid app wildcard hostname. Please contact an administrator.", - RetryEnabled: false, - DashboardURL: api.AccessURL.String(), - }) - return false - } - - // Set the app cookie for all subdomains of api.AppHostname. This cookie is - // handled properly by the ExtractAPIKey middleware. - // - // We don't set an expiration because the key in the database already has an - // expiration. - maxAge := 0 - if token == "" { - maxAge = -1 - } - cookieHost := "." + hostSplit[1] - http.SetCookie(rw, &http.Cookie{ - Name: httpmw.DevURLSessionTokenCookie, - Value: token, - Domain: cookieHost, - Path: "/", - MaxAge: maxAge, - HttpOnly: true, - SameSite: http.SameSiteLaxMode, - Secure: api.SecureAuthCookie, - }) - - return true -} - // workspaceApplicationAuth is an endpoint on the main router that handles // redirects from the subdomain handler. // @@ -701,7 +299,7 @@ func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request } // Get the redirect URI from the query parameters and parse it. - redirectURI := r.URL.Query().Get(redirectURIQueryParam) + redirectURI := r.URL.Query().Get(workspaceapps.RedirectURIQueryParam) if redirectURI == "" { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Missing redirect_uri query parameter.", @@ -781,35 +379,192 @@ func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request http.Redirect(rw, r, u.String(), http.StatusTemporaryRedirect) } -// proxyApplication are the required fields to proxy a workspace application. -type proxyApplication struct { - AccessMethod workspaceAppAccessMethod - Workspace database.Workspace - Agent database.WorkspaceAgent +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 + } - // Either App or Port must be set, but not both. - App *database.WorkspaceApp - Port uint16 + // 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 + } - // SharingLevel MUST be set to database.AppSharingLevelOwner by default for - // ports. - SharingLevel database.AppSharingLevel - // Path must either be empty or have a leading slash. - Path string + // 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) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.ResponseWriter, r *http.Request) { +func (api *API) handleWorkspaceSubdomainAppLogout(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() - sharingLevel := database.AppSharingLevelOwner - if proxyApp.App != nil && proxyApp.App.SharingLevel != "" { - sharingLevel = proxyApp.App.SharingLevel + // 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.checkWorkspaceApplicationAuth(rw, r, proxyApp.AccessMethod, proxyApp.Workspace, sharingLevel) { + if !api.setWorkspaceAppCookie(rw, r, "") { return } - // Filter IP headers from untrusted origins! + // 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, ticket workspaceapps.Ticket, 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) @@ -818,32 +573,12 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res return } - // If the app does not exist, but the app slug is a port number, then route - // to the port as an "anonymous app". We only support HTTP for port-based - // URLs. - // - // This is only supported for subdomain-based applications. - internalURL := fmt.Sprintf("http://127.0.0.1:%d", proxyApp.Port) - if proxyApp.App != nil { - if !proxyApp.App.Url.Valid { - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusBadRequest, - Title: "Bad Request", - Description: fmt.Sprintf("Application %q does not have a URL set.", proxyApp.App.Slug), - RetryEnabled: true, - DashboardURL: api.AccessURL.String(), - }) - return - } - internalURL = proxyApp.App.Url.String - } - - appURL, err := url.Parse(internalURL) + appURL, err := url.Parse(ticket.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", internalURL, err.Error()), + Description: fmt.Sprintf("Application has an invalid URL %q: %s", ticket.AppURL, err.Error()), RetryEnabled: true, DashboardURL: api.AccessURL.String(), }) @@ -857,7 +592,7 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res 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.", internalURL, port), + Message: fmt.Sprintf("App URL %q has an invalid port %q.", ticket.AppURL, port), Detail: err.Error(), }) return @@ -872,14 +607,14 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res } // Ensure path and query parameter correctness. - if proxyApp.Path == "" { + 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 proxyApp.Path == "/" && r.URL.RawQuery == "" && appURL.RawQuery != "" { + 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 @@ -890,7 +625,7 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res return } - r.URL.Path = proxyApp.Path + r.URL.Path = path appURL.RawQuery = "" proxy := httputil.NewSingleHostReverseProxy(appURL) @@ -904,7 +639,7 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res }) } - conn, release, err := api.workspaceAgentCache.Acquire(r, proxyApp.Agent.ID) + conn, release, err := api.workspaceAgentCache.Acquire(r, ticket.AgentID) if err != nil { site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ Status: http.StatusBadGateway, @@ -1060,15 +795,3 @@ func decryptAPIKey(ctx context.Context, db database.Store, encryptedAPIKey strin return key, payload.APIKey, nil } - -// renderApplicationNotFound should always be used when the app is not found or -// the current user doesn't have permission to access it. -func renderApplicationNotFound(rw http.ResponseWriter, r *http.Request, accessURL *url.URL) { - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusNotFound, - Title: "Application Not Found", - Description: "The application or workspace you are trying to access does not exist or you do not have permission to access it.", - RetryEnabled: false, - DashboardURL: accessURL.String(), - }) -} diff --git a/coderd/workspaceapps/auth.go b/coderd/workspaceapps/auth.go new file mode 100644 index 0000000000..425f9245ba --- /dev/null +++ b/coderd/workspaceapps/auth.go @@ -0,0 +1,494 @@ +package workspaceapps + +import ( + "context" + "database/sql" + "fmt" + "net/http" + "strconv" + "strings" + "time" + + "github.com/google/uuid" + "golang.org/x/xerrors" + + "cdr.dev/slog" + "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/httpapi" + "github.com/coder/coder/coderd/httpmw" + "github.com/coder/coder/coderd/rbac" + "github.com/coder/coder/codersdk" + "github.com/coder/coder/site" +) + +const ( + // TODO(@deansheather): configurable expiry + TicketExpiry = time.Minute + + // RedirectURIQueryParam is the query param for the app URL to be passed + // back to the API auth endpoint on the main access URL. + RedirectURIQueryParam = "redirect_uri" +) + +// ResolveRequest takes an app request, checks if it's valid and authenticated, +// and returns a ticket with details about the app. +// +// The ticket is written as a signed JWT into a cookie and will be automatically +// used in the next request to the same app to avoid database calls. +// +// Upstream code should avoid any database calls ever. +func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appReq Request) (*Ticket, bool) { + err := appReq.Validate() + if err != nil { + p.writeWorkspaceApp500(rw, r, &appReq, err, "invalid app request") + return nil, false + } + + if appReq.WorkspaceAndAgent != "" { + // workspace.agent + workspaceAndAgent := strings.SplitN(appReq.WorkspaceAndAgent, ".", 2) + appReq.WorkspaceAndAgent = "" + appReq.WorkspaceNameOrID = workspaceAndAgent[0] + if len(workspaceAndAgent) > 1 { + appReq.AgentNameOrID = workspaceAndAgent[1] + } + + // Sanity check. + err := appReq.Validate() + if err != nil { + p.writeWorkspaceApp500(rw, r, &appReq, err, "invalid app request") + return nil, false + } + } + + // Get the existing ticket from the request. + ticketCookie, err := r.Cookie(codersdk.DevURLSessionTicketCookie) + if err == nil { + ticket, err := p.ParseTicket(ticketCookie.Value) + if err == nil { + if ticket.MatchesRequest(appReq) { + // The request has a ticket, which is a valid ticket signed by + // us, and matches the app that the user was trying to access. + return &ticket, true + } + } + } + + // There's no ticket or it's invalid, so we need to check auth using the + // session token, validate auth and access to the app, then generate a new + // ticket. + ticket := Ticket{ + AccessMethod: appReq.AccessMethod, + UsernameOrID: appReq.UsernameOrID, + WorkspaceNameOrID: appReq.WorkspaceNameOrID, + AgentNameOrID: appReq.AgentNameOrID, + AppSlugOrPort: appReq.AppSlugOrPort, + } + + // We use the regular API apiKey extraction middleware fn here to avoid any + // differences in behavior between the two. + apiKey, authz, ok := httpmw.ExtractAPIKey(rw, r, httpmw.ExtractAPIKeyConfig{ + DB: p.Database, + OAuth2Configs: p.OAuth2Configs, + RedirectToLogin: false, + DisableSessionExpiryRefresh: p.DeploymentConfig.DisableSessionExpiryRefresh.Value, + // Optional is true to allow for public apps. If an authorization check + // fails and the user is not authenticated, they will be redirected to + // the login page using code below (not the redirect from the + // middleware itself). + Optional: true, + }) + if !ok { + return nil, false + } + + // Get user. + var ( + user database.User + userErr error + ) + if userID, uuidErr := uuid.Parse(appReq.UsernameOrID); uuidErr == nil { + user, userErr = p.Database.GetUserByID(r.Context(), userID) + } else { + user, userErr = p.Database.GetUserByEmailOrUsername(r.Context(), database.GetUserByEmailOrUsernameParams{ + Username: appReq.UsernameOrID, + }) + } + if xerrors.Is(userErr, sql.ErrNoRows) { + p.writeWorkspaceApp404(rw, r, &appReq, fmt.Sprintf("user %q not found", appReq.UsernameOrID)) + return nil, false + } else if userErr != nil { + p.writeWorkspaceApp500(rw, r, &appReq, userErr, "get user") + return nil, false + } + ticket.UserID = user.ID + + // Get workspace. + var ( + workspace database.Workspace + workspaceErr error + ) + if workspaceID, uuidErr := uuid.Parse(appReq.WorkspaceNameOrID); uuidErr == nil { + workspace, workspaceErr = p.Database.GetWorkspaceByID(r.Context(), workspaceID) + } else { + workspace, workspaceErr = p.Database.GetWorkspaceByOwnerIDAndName(r.Context(), database.GetWorkspaceByOwnerIDAndNameParams{ + OwnerID: user.ID, + Name: appReq.WorkspaceNameOrID, + Deleted: false, + }) + } + if xerrors.Is(workspaceErr, sql.ErrNoRows) { + p.writeWorkspaceApp404(rw, r, &appReq, fmt.Sprintf("workspace %q not found", appReq.WorkspaceNameOrID)) + return nil, false + } else if workspaceErr != nil { + p.writeWorkspaceApp500(rw, r, &appReq, workspaceErr, "get workspace") + return nil, false + } + ticket.WorkspaceID = workspace.ID + + // Get agent. + var ( + agent database.WorkspaceAgent + agentErr error + trustAgent = false + ) + if agentID, uuidErr := uuid.Parse(appReq.AgentNameOrID); uuidErr == nil { + agent, agentErr = p.Database.GetWorkspaceAgentByID(r.Context(), agentID) + } else { + build, err := p.Database.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID) + if err != nil { + p.writeWorkspaceApp500(rw, r, &appReq, err, "get latest workspace build") + return nil, false + } + + resources, err := p.Database.GetWorkspaceResourcesByJobID(r.Context(), build.JobID) + if err != nil { + p.writeWorkspaceApp500(rw, r, &appReq, err, "get workspace resources") + return nil, false + } + resourcesIDs := []uuid.UUID{} + for _, resource := range resources { + resourcesIDs = append(resourcesIDs, resource.ID) + } + + agents, err := p.Database.GetWorkspaceAgentsByResourceIDs(r.Context(), resourcesIDs) + if err != nil { + p.writeWorkspaceApp500(rw, r, &appReq, err, "get workspace agents") + return nil, false + } + + if appReq.AgentNameOrID == "" { + if len(agents) != 1 { + p.writeWorkspaceApp404(rw, r, &appReq, "no agent specified, but multiple exist in workspace") + return nil, false + } + + agent = agents[0] + trustAgent = true + } else { + for _, a := range agents { + if a.Name == appReq.AgentNameOrID { + agent = a + trustAgent = true + break + } + } + } + + if agent.ID == uuid.Nil { + agentErr = sql.ErrNoRows + } + } + if xerrors.Is(agentErr, sql.ErrNoRows) { + p.writeWorkspaceApp404(rw, r, &appReq, fmt.Sprintf("agent %q not found", appReq.AgentNameOrID)) + return nil, false + } else if agentErr != nil { + p.writeWorkspaceApp500(rw, r, &appReq, agentErr, "get agent") + return nil, false + } + + // Verify the agent belongs to the workspace. + if !trustAgent { + agentResource, err := p.Database.GetWorkspaceResourceByID(r.Context(), agent.ResourceID) + if err != nil { + p.writeWorkspaceApp500(rw, r, &appReq, err, "get agent resource") + return nil, false + } + build, err := p.Database.GetWorkspaceBuildByJobID(r.Context(), agentResource.JobID) + if err != nil { + p.writeWorkspaceApp500(rw, r, &appReq, err, "get agent workspace build") + return nil, false + } + if build.WorkspaceID != workspace.ID { + p.writeWorkspaceApp404(rw, r, &appReq, "agent does not belong to workspace") + return nil, false + } + } + ticket.AgentID = agent.ID + + // Get app. + appSharingLevel := database.AppSharingLevelOwner + portUint, portUintErr := strconv.ParseUint(appReq.AppSlugOrPort, 10, 16) + if appReq.AccessMethod == AccessMethodSubdomain && portUintErr == nil { + // If the app slug is a port number, then route to the port as an + // "anonymous app". We only support HTTP for port-based URLs. + // + // This is only supported for subdomain-based applications. + ticket.AppURL = fmt.Sprintf("http://127.0.0.1:%d", portUint) + } else { + app, ok := p.lookupWorkspaceApp(rw, r, agent.ID, appReq.AppSlugOrPort) + if !ok { + return nil, false + } + + if !app.Url.Valid { + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusBadRequest, + Title: "Bad Request", + Description: fmt.Sprintf("Application %q does not have a URL set.", app.Slug), + RetryEnabled: true, + DashboardURL: p.AccessURL.String(), + }) + return nil, false + } + + if app.SharingLevel != "" { + appSharingLevel = app.SharingLevel + } + ticket.AppURL = app.Url.String + } + + // Verify the user has access to the app. + authed, ok := p.fetchWorkspaceApplicationAuth(rw, r, authz, appReq.AccessMethod, workspace, appSharingLevel) + if !ok { + return nil, false + } + if !authed { + if apiKey != nil { + // The request has a valid API key but insufficient permissions. + p.writeWorkspaceApp404(rw, r, &appReq, "insufficient permissions") + return nil, false + } + + // Redirect to login as they don't have permission to access the app + // and they aren't signed in. + if appReq.AccessMethod == AccessMethodSubdomain { + redirectURI := *r.URL + redirectURI.Scheme = p.AccessURL.Scheme + redirectURI.Host = httpapi.RequestHost(r) + + u := *p.AccessURL + u.Path = "/api/v2/applications/auth-redirect" + q := u.Query() + q.Add(RedirectURIQueryParam, redirectURI.String()) + u.RawQuery = q.Encode() + + http.Redirect(rw, r, u.String(), http.StatusTemporaryRedirect) + } else { + httpmw.RedirectToLogin(rw, r, httpmw.SignedOutErrorMessage) + } + return nil, false + } + + // As a sanity check, ensure the ticket we just made is valid for this + // request. + if !ticket.MatchesRequest(appReq) { + p.writeWorkspaceApp500(rw, r, &appReq, nil, "fresh ticket does not match request") + return nil, false + } + + // Sign the ticket. + ticketExpiry := time.Now().Add(TicketExpiry) + ticket.Expiry = ticketExpiry.Unix() + ticketStr, err := p.GenerateTicket(ticket) + if err != nil { + p.writeWorkspaceApp500(rw, r, &appReq, err, "generate ticket") + return nil, false + } + + // Write the ticket cookie. We always want this to apply to the current + // hostname (even for subdomain apps, without any wildcard shenanigans, + // because the ticket is only valid for a single app). + http.SetCookie(rw, &http.Cookie{ + Name: codersdk.DevURLSessionTicketCookie, + Value: ticketStr, + Path: appReq.BasePath, + Expires: ticketExpiry, + }) + + return &ticket, true +} + +// lookupWorkspaceApp looks up the workspace application by slug in the given +// agent and returns it. If the application is not found or there was a server +// error while looking it up, an HTML error page is returned and false is +// returned so the caller can return early. +func (p *Provider) lookupWorkspaceApp(rw http.ResponseWriter, r *http.Request, agentID uuid.UUID, appSlug string) (database.WorkspaceApp, bool) { + app, err := p.Database.GetWorkspaceAppByAgentIDAndSlug(r.Context(), database.GetWorkspaceAppByAgentIDAndSlugParams{ + AgentID: agentID, + Slug: appSlug, + }) + if xerrors.Is(err, sql.ErrNoRows) { + p.writeWorkspaceApp404(rw, r, nil, "application not found in agent by slug") + return database.WorkspaceApp{}, false + } + if err != nil { + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusInternalServerError, + Title: "Internal Server Error", + Description: "Could not fetch workspace application: " + err.Error(), + RetryEnabled: true, + DashboardURL: p.AccessURL.String(), + }) + return database.WorkspaceApp{}, false + } + + return app, true +} + +func (p *Provider) authorizeWorkspaceApp(ctx context.Context, roles *httpmw.Authorization, accessMethod AccessMethod, sharingLevel database.AppSharingLevel, workspace database.Workspace) (bool, error) { + if accessMethod == "" { + accessMethod = AccessMethodPath + } + isPathApp := accessMethod == AccessMethodPath + + // If path-based app sharing is disabled (which is the default), we can + // force the sharing level to be "owner" so that the user can only access + // their own apps. + // + // Site owners are blocked from accessing path-based apps unless the + // Dangerous.AllowPathAppSiteOwnerAccess flag is enabled in the check below. + if isPathApp && !p.DeploymentConfig.Dangerous.AllowPathAppSharing.Value { + sharingLevel = database.AppSharingLevelOwner + } + + // Short circuit if not authenticated. + if roles == nil { + // The user is not authenticated, so they can only access the app if it + // is public. + return sharingLevel == database.AppSharingLevelPublic, nil + } + + // Block anyone from accessing workspaces they don't own in path-based apps + // unless the admin disables this security feature. This blocks site-owners + // from accessing any apps from any user's workspaces. + // + // When the Dangerous.AllowPathAppSharing flag is not enabled, the sharing + // level will be forced to "owner", so this check will always be true for + // workspaces owned by different users. + if isPathApp && + sharingLevel == database.AppSharingLevelOwner && + workspace.OwnerID.String() != roles.Actor.ID && + !p.DeploymentConfig.Dangerous.AllowPathAppSiteOwnerAccess.Value { + return false, nil + } + + // Do a standard RBAC check. This accounts for share level "owner" and any + // other RBAC rules that may be in place. + // + // Regardless of share level or whether it's enabled or not, the owner of + // the workspace can always access applications (as long as their API key's + // scope allows it). + err := p.Authorizer.Authorize(ctx, roles.Actor, rbac.ActionCreate, workspace.ApplicationConnectRBAC()) + if err == nil { + return true, nil + } + + switch sharingLevel { + case database.AppSharingLevelOwner: + // We essentially already did this above with the regular RBAC check. + // Owners can always access their own apps according to RBAC rules, so + // they have already been returned from this function. + case database.AppSharingLevelAuthenticated: + // The user is authenticated at this point, but we need to make sure + // that they have ApplicationConnect permissions to their own + // workspaces. This ensures that the key's scope has permission to + // connect to workspace apps. + object := rbac.ResourceWorkspaceApplicationConnect.WithOwner(roles.Actor.ID) + err := p.Authorizer.Authorize(ctx, roles.Actor, rbac.ActionCreate, object) + if err == nil { + return true, nil + } + case database.AppSharingLevelPublic: + // We don't really care about scopes and stuff if it's public anyways. + // Someone with a restricted-scope API key could just not submit the + // API key cookie in the request and access the page. + return true, nil + } + + // No checks were successful. + return false, nil +} + +// fetchWorkspaceApplicationAuth authorizes the user using api.Authorizer for a +// given app share level in the given workspace. The user's authorization status +// is returned. If a server error occurs, a HTML error page is rendered and +// false is returned so the caller can return early. +func (p *Provider) fetchWorkspaceApplicationAuth(rw http.ResponseWriter, r *http.Request, authz *httpmw.Authorization, accessMethod AccessMethod, workspace database.Workspace, appSharingLevel database.AppSharingLevel) (authed bool, ok bool) { + ok, err := p.authorizeWorkspaceApp(r.Context(), authz, accessMethod, appSharingLevel, workspace) + if err != nil { + p.Logger.Error(r.Context(), "authorize workspace app", slog.Error(err)) + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusInternalServerError, + Title: "Internal Server Error", + Description: "Could not verify authorization. Please try again or contact an administrator.", + RetryEnabled: true, + DashboardURL: p.AccessURL.String(), + }) + return false, false + } + + return ok, true +} + +// writeWorkspaceApp404 writes a HTML 404 error page for a workspace app. If +// appReq is not nil, it will be used to log the request details at debug level. +func (p *Provider) writeWorkspaceApp404(rw http.ResponseWriter, r *http.Request, appReq *Request, msg string) { + if appReq != nil { + slog.Helper() + p.Logger.Debug(r.Context(), + "workspace app 404: "+msg, + slog.F("username_or_id", appReq.UsernameOrID), + slog.F("workspace_and_agent", appReq.WorkspaceAndAgent), + slog.F("workspace_name_or_id", appReq.WorkspaceNameOrID), + slog.F("agent_name_or_id", appReq.AgentNameOrID), + slog.F("app_slug_or_port", appReq.AppSlugOrPort), + ) + } + + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusNotFound, + Title: "Application Not Found", + Description: "The application or workspace you are trying to access does not exist or you do not have permission to access it.", + RetryEnabled: false, + DashboardURL: p.AccessURL.String(), + }) +} + +// writeWorkspaceApp500 writes a HTML 500 error page for a workspace app. If +// appReq is not nil, it's fields will be added to the logged error message. +func (p *Provider) writeWorkspaceApp500(rw http.ResponseWriter, r *http.Request, appReq *Request, err error, msg string) { + slog.Helper() + ctx := r.Context() + if appReq != nil { + slog.With(ctx, + slog.F("username_or_id", appReq.UsernameOrID), + slog.F("workspace_and_agent", appReq.WorkspaceAndAgent), + slog.F("workspace_name_or_id", appReq.WorkspaceNameOrID), + slog.F("agent_name_or_id", appReq.AgentNameOrID), + slog.F("app_name_or_port", appReq.AppSlugOrPort), + ) + } + p.Logger.Warn(ctx, + "workspace app auth server error: "+msg, + slog.Error(err), + ) + + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusInternalServerError, + Title: "Internal Server Error", + Description: "An internal server error occurred.", + RetryEnabled: false, + DashboardURL: p.AccessURL.String(), + }) +} diff --git a/coderd/workspaceapps/auth_test.go b/coderd/workspaceapps/auth_test.go new file mode 100644 index 0000000000..41c1657545 --- /dev/null +++ b/coderd/workspaceapps/auth_test.go @@ -0,0 +1,582 @@ +package workspaceapps_test + +import ( + "context" + "fmt" + "net" + "net/http" + "net/http/httptest" + "net/http/httputil" + "net/url" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/agent" + "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/httpmw" + "github.com/coder/coder/coderd/workspaceapps" + "github.com/coder/coder/codersdk" + "github.com/coder/coder/codersdk/agentsdk" + "github.com/coder/coder/provisioner/echo" + "github.com/coder/coder/provisionersdk/proto" + "github.com/coder/coder/testutil" +) + +func Test_ResolveRequest(t *testing.T) { + t.Parallel() + + const ( + agentName = "agent" + appNameOwner = "app-owner" + appNameAuthed = "app-authed" + appNamePublic = "app-public" + appNameInvalidURL = "app-invalid-url" + + // This is not a valid URL we listen on in the test, but it needs to be + // set to a value. + appURL = "http://localhost:8080" + ) + allApps := []string{appNameOwner, appNameAuthed, appNamePublic} + + deploymentConfig := coderdtest.DeploymentConfig(t) + deploymentConfig.DisablePathApps.Value = false + deploymentConfig.Dangerous.AllowPathAppSharing.Value = true + deploymentConfig.Dangerous.AllowPathAppSiteOwnerAccess.Value = true + + client, closer, api := coderdtest.NewWithAPI(t, &coderdtest.Options{ + DeploymentConfig: deploymentConfig, + IncludeProvisionerDaemon: true, + AgentStatsRefreshInterval: time.Millisecond * 100, + MetricsCacheRefreshInterval: time.Millisecond * 100, + RealIPConfig: &httpmw.RealIPConfig{ + TrustedOrigins: []*net.IPNet{{ + IP: net.ParseIP("127.0.0.1"), + Mask: net.CIDRMask(8, 32), + }}, + TrustedHeaders: []string{ + "CF-Connecting-IP", + }, + }, + }) + t.Cleanup(func() { + _ = closer.Close() + }) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium) + defer cancel() + + firstUser := coderdtest.CreateFirstUser(t, client) + me, err := client.User(ctx, codersdk.Me) + require.NoError(t, err) + + secondUserClient, _ := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID) + + agentAuthToken := uuid.NewString() + version := coderdtest.CreateTemplateVersion(t, client, firstUser.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: echo.ProvisionComplete, + ProvisionApply: []*proto.Provision_Response{{ + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{ + Resources: []*proto.Resource{{ + Name: "example", + Type: "aws_instance", + Agents: []*proto.Agent{{ + Id: uuid.NewString(), + Name: agentName, + Auth: &proto.Agent_Token{ + Token: agentAuthToken, + }, + Apps: []*proto.App{ + { + Slug: appNameOwner, + DisplayName: appNameOwner, + SharingLevel: proto.AppSharingLevel_OWNER, + Url: appURL, + }, + { + Slug: appNameAuthed, + DisplayName: appNameAuthed, + SharingLevel: proto.AppSharingLevel_AUTHENTICATED, + Url: appURL, + }, + { + Slug: appNamePublic, + DisplayName: appNamePublic, + SharingLevel: proto.AppSharingLevel_PUBLIC, + Url: appURL, + }, + { + Slug: appNameInvalidURL, + DisplayName: appNameInvalidURL, + SharingLevel: proto.AppSharingLevel_PUBLIC, + Url: "test:path/to/app", + }, + }, + }}, + }}, + }, + }, + }}, + }) + template := coderdtest.CreateTemplate(t, client, firstUser.OrganizationID, version.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, firstUser.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + agentClient := agentsdk.New(client.URL) + agentClient.SetSessionToken(agentAuthToken) + agentCloser := agent.New(agent.Options{ + Client: agentClient, + Logger: slogtest.Make(t, nil).Named("agent"), + }) + t.Cleanup(func() { + _ = agentCloser.Close() + }) + resources := coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID) + + agentID := uuid.Nil + for _, resource := range resources { + for _, agnt := range resource.Agents { + if agnt.Name == agentName { + agentID = agnt.ID + } + } + } + require.NotEqual(t, uuid.Nil, agentID) + + t.Run("OK", func(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + workspaceNameOrID string + agentNameOrID string + }{ + { + name: "Names", + workspaceNameOrID: workspace.Name, + agentNameOrID: agentName, + }, + { + name: "IDs", + workspaceNameOrID: workspace.ID.String(), + agentNameOrID: agentID.String(), + }, + } + + for _, c := range cases { + c := c + + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + // Try resolving a request for each app as the owner, without a ticket, + // then use the ticket to resolve each app. + for _, app := range allApps { + req := workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/app", + UsernameOrID: me.Username, + WorkspaceNameOrID: c.workspaceNameOrID, + AgentNameOrID: c.agentNameOrID, + AppSlugOrPort: app, + } + + t.Log("app", app) + rw := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/app", nil) + r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) + + // Try resolving the request without a ticket. + ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + w := rw.Result() + if !assert.True(t, ok) { + dump, err := httputil.DumpResponse(w, true) + require.NoError(t, err, "error dumping failed response") + t.Log(string(dump)) + return + } + _ = w.Body.Close() + + require.Equal(t, &workspaceapps.Ticket{ + AccessMethod: req.AccessMethod, + UsernameOrID: req.UsernameOrID, + WorkspaceNameOrID: req.WorkspaceNameOrID, + AgentNameOrID: req.AgentNameOrID, + AppSlugOrPort: req.AppSlugOrPort, + Expiry: ticket.Expiry, // ignored to avoid flakiness + UserID: me.ID, + WorkspaceID: workspace.ID, + AgentID: agentID, + AppURL: appURL, + }, ticket) + require.NotZero(t, ticket.Expiry) + require.InDelta(t, time.Now().Add(workspaceapps.TicketExpiry).Unix(), ticket.Expiry, time.Minute.Seconds()) + + // Check that the ticket was set in the response and is valid. + require.Len(t, w.Cookies(), 1) + cookie := w.Cookies()[0] + require.Equal(t, codersdk.DevURLSessionTicketCookie, cookie.Name) + require.Equal(t, req.BasePath, cookie.Path) + + parsedTicket, err := api.WorkspaceAppsProvider.ParseTicket(cookie.Value) + require.NoError(t, err) + require.Equal(t, ticket, &parsedTicket) + + // Try resolving the request with the ticket only. + rw = httptest.NewRecorder() + r = httptest.NewRequest("GET", "/app", nil) + r.AddCookie(cookie) + + secondTicket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + require.True(t, ok) + require.Equal(t, ticket, secondTicket) + } + }) + } + }) + + t.Run("AuthenticatedOtherUser", func(t *testing.T) { + t.Parallel() + + for _, app := range allApps { + req := workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/app", + UsernameOrID: me.Username, + WorkspaceNameOrID: workspace.Name, + AgentNameOrID: agentName, + AppSlugOrPort: app, + } + + t.Log("app", app) + rw := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/app", nil) + r.Header.Set(codersdk.SessionTokenHeader, secondUserClient.SessionToken()) + + ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + w := rw.Result() + _ = w.Body.Close() + if app == appNameOwner { + require.False(t, ok) + require.Nil(t, ticket) + require.NotZero(t, w.StatusCode) + require.Equal(t, http.StatusNotFound, w.StatusCode) + return + } + require.True(t, ok) + require.NotNil(t, ticket) + require.Zero(t, w.StatusCode) + } + }) + + t.Run("Unauthenticated", func(t *testing.T) { + t.Parallel() + + for _, app := range allApps { + req := workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/app", + UsernameOrID: me.Username, + WorkspaceNameOrID: workspace.Name, + AgentNameOrID: agentName, + AppSlugOrPort: app, + } + + t.Log("app", app) + rw := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/app", nil) + ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + w := rw.Result() + if app != appNamePublic { + require.False(t, ok) + require.Nil(t, ticket) + require.NotZero(t, rw.Code) + require.NotEqual(t, http.StatusOK, rw.Code) + } else { + if !assert.True(t, ok) { + dump, err := httputil.DumpResponse(w, true) + require.NoError(t, err, "error dumping failed response") + t.Log(string(dump)) + return + } + require.NotNil(t, ticket) + if rw.Code != 0 && rw.Code != http.StatusOK { + t.Fatalf("expected 200 (or unset) response code, got %d", rw.Code) + } + } + _ = w.Body.Close() + } + }) + + t.Run("Invalid", func(t *testing.T) { + t.Parallel() + + req := workspaceapps.Request{ + AccessMethod: "invalid", + } + rw := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/app", nil) + ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + require.False(t, ok) + require.Nil(t, ticket) + }) + + t.Run("SplitWorkspaceAndAgent", func(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + workspaceAndAgent string + workspace string + agent string + ok bool + }{ + { + name: "WorkspaecOnly", + workspaceAndAgent: workspace.Name, + workspace: workspace.Name, + agent: "", + ok: true, + }, + { + name: "WorkspaceAndAgent", + workspaceAndAgent: fmt.Sprintf("%s.%s", workspace.Name, agentName), + workspace: workspace.Name, + agent: agentName, + ok: true, + }, + { + name: "WorkspaceID", + workspaceAndAgent: workspace.ID.String(), + workspace: workspace.ID.String(), + agent: "", + ok: true, + }, + { + name: "WorkspaceIDAndAgentID", + workspaceAndAgent: fmt.Sprintf("%s.%s", workspace.ID, agentID), + workspace: workspace.ID.String(), + agent: agentID.String(), + ok: true, + }, + { + name: "Invalid1", + workspaceAndAgent: "invalid", + ok: false, + }, + { + name: "Invalid2", + workspaceAndAgent: ".", + ok: false, + }, + { + name: "Slash", + workspaceAndAgent: fmt.Sprintf("%s/%s", workspace.Name, agentName), + ok: false, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + req := workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/app", + UsernameOrID: me.Username, + WorkspaceAndAgent: c.workspaceAndAgent, + AppSlugOrPort: appNamePublic, + } + + rw := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/app", nil) + r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) + + ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + w := rw.Result() + if !assert.Equal(t, c.ok, ok) { + dump, err := httputil.DumpResponse(w, true) + require.NoError(t, err, "error dumping failed response") + t.Log(string(dump)) + return + } + if c.ok { + require.NotNil(t, ticket) + require.Equal(t, ticket.WorkspaceNameOrID, c.workspace) + require.Equal(t, ticket.AgentNameOrID, c.agent) + require.Equal(t, ticket.WorkspaceID, workspace.ID) + require.Equal(t, ticket.AgentID, agentID) + } else { + require.Nil(t, ticket) + } + _ = w.Body.Close() + }) + } + }) + + t.Run("TicketDoesNotMatchRequest", func(t *testing.T) { + t.Parallel() + + badTicket := workspaceapps.Ticket{ + AccessMethod: workspaceapps.AccessMethodPath, + UsernameOrID: me.Username, + WorkspaceNameOrID: workspace.Name, + AgentNameOrID: agentName, + // App name differs + AppSlugOrPort: appNamePublic, + Expiry: time.Now().Add(time.Minute).Unix(), + UserID: me.ID, + WorkspaceID: workspace.ID, + AgentID: agentID, + AppURL: appURL, + } + badTicketStr, err := api.WorkspaceAppsProvider.GenerateTicket(badTicket) + require.NoError(t, err) + + req := workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/app", + UsernameOrID: me.Username, + WorkspaceNameOrID: workspace.Name, + AgentNameOrID: agentName, + // App name differs + AppSlugOrPort: appNameOwner, + } + + rw := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/app", nil) + r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) + r.AddCookie(&http.Cookie{ + Name: codersdk.DevURLSessionTicketCookie, + Value: badTicketStr, + }) + + // Even though the ticket is invalid, we should still perform request + // resolution. + ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + require.True(t, ok) + require.NotNil(t, ticket) + require.Equal(t, appNameOwner, ticket.AppSlugOrPort) + + // Cookie should be set in response, and it should be a different + // ticket. + w := rw.Result() + _ = w.Body.Close() + cookies := w.Cookies() + require.Len(t, cookies, 1) + require.Equal(t, cookies[0].Name, codersdk.DevURLSessionTicketCookie) + require.NotEqual(t, cookies[0].Value, badTicketStr) + parsedTicket, err := api.WorkspaceAppsProvider.ParseTicket(cookies[0].Value) + require.NoError(t, err) + require.Equal(t, appNameOwner, parsedTicket.AppSlugOrPort) + }) + + t.Run("PortPathBlocked", func(t *testing.T) { + t.Parallel() + + req := workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/app", + UsernameOrID: me.Username, + WorkspaceNameOrID: workspace.Name, + AgentNameOrID: agentName, + AppSlugOrPort: "8080", + } + + rw := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/app", nil) + r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) + + ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + require.False(t, ok) + require.Nil(t, ticket) + }) + + t.Run("PortSubdomain", func(t *testing.T) { + t.Parallel() + + req := workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodSubdomain, + BasePath: "/", + UsernameOrID: me.Username, + WorkspaceNameOrID: workspace.Name, + AgentNameOrID: agentName, + AppSlugOrPort: "9090", + } + + rw := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/app", nil) + r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) + + ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + require.True(t, ok) + require.Equal(t, req.AppSlugOrPort, ticket.AppSlugOrPort) + require.Equal(t, "http://127.0.0.1:9090", ticket.AppURL) + }) + + t.Run("InsufficientPermissions", func(t *testing.T) { + t.Parallel() + + req := workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/app", + UsernameOrID: me.Username, + WorkspaceNameOrID: workspace.Name, + AgentNameOrID: agentName, + AppSlugOrPort: appNameOwner, + } + + rw := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/app", nil) + r.Header.Set(codersdk.SessionTokenHeader, secondUserClient.SessionToken()) + + ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + require.False(t, ok) + require.Nil(t, ticket) + }) + + t.Run("RedirectSubdomainAuth", func(t *testing.T) { + t.Parallel() + + req := workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodSubdomain, + BasePath: "/", + UsernameOrID: me.Username, + WorkspaceNameOrID: workspace.Name, + AgentNameOrID: agentName, + AppSlugOrPort: appNameOwner, + } + + rw := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/some-path", nil) + r.Host = "app.com" + + ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + require.False(t, ok) + require.Nil(t, ticket) + + w := rw.Result() + defer w.Body.Close() + require.Equal(t, http.StatusTemporaryRedirect, w.StatusCode) + + loc, err := w.Location() + require.NoError(t, err) + + require.Equal(t, api.AccessURL.Scheme, loc.Scheme) + require.Equal(t, api.AccessURL.Host, loc.Host) + require.Equal(t, "/api/v2/applications/auth-redirect", loc.Path) + + redirectURIStr := loc.Query().Get(workspaceapps.RedirectURIQueryParam) + redirectURI, err := url.Parse(redirectURIStr) + require.NoError(t, err) + + require.Equal(t, "http", redirectURI.Scheme) + require.Equal(t, "app.com", redirectURI.Host) + require.Equal(t, "/some-path", redirectURI.Path) + }) +} diff --git a/coderd/workspaceapps/provider.go b/coderd/workspaceapps/provider.go new file mode 100644 index 0000000000..7e0089015f --- /dev/null +++ b/coderd/workspaceapps/provider.go @@ -0,0 +1,41 @@ +package workspaceapps + +import ( + "net/url" + + "cdr.dev/slog" + "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/httpmw" + "github.com/coder/coder/coderd/rbac" + "github.com/coder/coder/codersdk" +) + +// Provider provides authentication and authorization for workspace apps. +// TODO(@deansheather): also provide workspace apps as a whole to remove all app +// code from coderd. +type Provider struct { + Logger slog.Logger + + AccessURL *url.URL + Authorizer rbac.Authorizer + Database database.Store + DeploymentConfig *codersdk.DeploymentConfig + OAuth2Configs *httpmw.OAuth2Configs + TicketSigningKey []byte +} + +func New(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database.Store, cfg *codersdk.DeploymentConfig, oauth2Cfgs *httpmw.OAuth2Configs, ticketSigningKey []byte) *Provider { + if len(ticketSigningKey) != 64 { + panic("ticket signing key must be 64 bytes") + } + + return &Provider{ + Logger: log, + AccessURL: accessURL, + Authorizer: authz, + Database: db, + DeploymentConfig: cfg, + OAuth2Configs: oauth2Cfgs, + TicketSigningKey: ticketSigningKey, + } +} diff --git a/coderd/workspaceapps/request.go b/coderd/workspaceapps/request.go new file mode 100644 index 0000000000..588a78ed4a --- /dev/null +++ b/coderd/workspaceapps/request.go @@ -0,0 +1,73 @@ +package workspaceapps + +import ( + "strings" + + "golang.org/x/xerrors" + + "github.com/coder/coder/codersdk" +) + +type AccessMethod string + +const ( + AccessMethodPath AccessMethod = "path" + AccessMethodSubdomain AccessMethod = "subdomain" +) + +type Request struct { + AccessMethod AccessMethod + // BasePath of the app. For path apps, this is the path prefix in the router + // for this particular app. For subdomain apps, this should be "/". This is + // used for setting the cookie path. + BasePath string + + UsernameOrID string + // WorkspaceAndAgent xor WorkspaceNameOrID are required. + WorkspaceAndAgent string // "workspace" or "workspace.agent" + WorkspaceNameOrID string + // AgentNameOrID is not required if the workspace has only one agent. + AgentNameOrID string + AppSlugOrPort string +} + +func (r Request) Validate() error { + if r.AccessMethod != AccessMethodPath && r.AccessMethod != AccessMethodSubdomain { + return xerrors.Errorf("invalid access method: %q", r.AccessMethod) + } + if r.BasePath == "" { + return xerrors.New("base path is required") + } + if r.UsernameOrID == "" { + return xerrors.New("username or ID is required") + } + if r.UsernameOrID == codersdk.Me { + // We block "me" for workspace app auth to avoid any security issues + // caused by having an identical workspace name on yourself and a + // different user and potentially reusing a ticket. + // + // This is also mitigated by storing the workspace/agent ID in the + // ticket, but we block it here to be double safe. + // + // Subdomain apps have never been used with "me" from our code, and path + // apps now have a redirect to remove the "me" from the URL. + return xerrors.New(`username cannot be "me" in app requests`) + } + if r.WorkspaceAndAgent != "" { + split := strings.Split(r.WorkspaceAndAgent, ".") + if split[0] == "" || (len(split) == 2 && split[1] == "") || len(split) > 2 { + return xerrors.Errorf("invalid workspace and agent: %q", r.WorkspaceAndAgent) + } + if r.WorkspaceNameOrID != "" || r.AgentNameOrID != "" { + return xerrors.New("dev error: cannot specify both WorkspaceAndAgent and (WorkspaceNameOrID and AgentNameOrID)") + } + } + if r.WorkspaceAndAgent == "" && r.WorkspaceNameOrID == "" { + return xerrors.New("workspace name or ID is required") + } + if r.AppSlugOrPort == "" { + return xerrors.New("app slug or port is required") + } + + return nil +} diff --git a/coderd/workspaceapps/request_test.go b/coderd/workspaceapps/request_test.go new file mode 100644 index 0000000000..f5217bf63b --- /dev/null +++ b/coderd/workspaceapps/request_test.go @@ -0,0 +1,206 @@ +package workspaceapps_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/coderd/workspaceapps" +) + +func Test_RequestValidate(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + req workspaceapps.Request + errContains string + }{ + { + name: "OK1", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/", + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + AgentNameOrID: "baz", + AppSlugOrPort: "qux", + }, + }, + { + name: "OK2", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodSubdomain, + BasePath: "/", + UsernameOrID: "foo", + WorkspaceAndAgent: "bar.baz", + AppSlugOrPort: "qux", + }, + }, + { + name: "OK3", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/", + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + AppSlugOrPort: "baz", + }, + }, + { + name: "NoAccessMethod", + req: workspaceapps.Request{ + AccessMethod: "", + BasePath: "/", + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + AgentNameOrID: "baz", + AppSlugOrPort: "qux", + }, + errContains: "invalid access method", + }, + { + name: "UnknownAccessMethod", + req: workspaceapps.Request{ + AccessMethod: "dean was here", + BasePath: "/", + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + AgentNameOrID: "baz", + AppSlugOrPort: "qux", + }, + errContains: "invalid access method", + }, + { + name: "NoBasePath", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "", + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + AgentNameOrID: "baz", + AppSlugOrPort: "qux", + }, + errContains: "base path is required", + }, + { + name: "NoUsernameOrID", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/", + UsernameOrID: "", + WorkspaceNameOrID: "bar", + AgentNameOrID: "baz", + AppSlugOrPort: "qux", + }, + errContains: "username or ID is required", + }, + { + name: "NoMe", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/", + UsernameOrID: "me", + WorkspaceNameOrID: "bar", + AgentNameOrID: "baz", + AppSlugOrPort: "qux", + }, + errContains: `username cannot be "me"`, + }, + { + name: "InvalidWorkspaceAndAgent/Empty1", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/", + UsernameOrID: "foo", + WorkspaceAndAgent: ".bar", + AppSlugOrPort: "baz", + }, + errContains: "invalid workspace and agent", + }, + { + name: "InvalidWorkspaceAndAgent/Empty2", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/", + UsernameOrID: "foo", + WorkspaceAndAgent: "bar.", + AppSlugOrPort: "baz", + }, + errContains: "invalid workspace and agent", + }, + { + name: "InvalidWorkspaceAndAgent/TwoDots", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/", + UsernameOrID: "foo", + WorkspaceAndAgent: "bar.baz.qux", + AppSlugOrPort: "baz", + }, + errContains: "invalid workspace and agent", + }, + { + name: "AmbiguousWorkspaceAndAgent/1", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/", + UsernameOrID: "foo", + WorkspaceAndAgent: "bar.baz", + WorkspaceNameOrID: "bar", + AppSlugOrPort: "qux", + }, + errContains: "cannot specify both", + }, + { + name: "AmbiguousWorkspaceAndAgent/2", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/", + UsernameOrID: "foo", + WorkspaceAndAgent: "bar.baz", + AgentNameOrID: "baz", + AppSlugOrPort: "qux", + }, + errContains: "cannot specify both", + }, + { + name: "NoWorkspaceNameOrID", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/", + UsernameOrID: "foo", + WorkspaceNameOrID: "", + AgentNameOrID: "baz", + AppSlugOrPort: "qux", + }, + errContains: "workspace name or ID is required", + }, + { + name: "NoAppSlugOrPort", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/", + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + AgentNameOrID: "baz", + AppSlugOrPort: "", + }, + errContains: "app slug or port is required", + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + t.Parallel() + err := c.req.Validate() + if c.errContains == "" { + require.NoError(t, err) + } else { + require.Error(t, err) + require.Contains(t, err.Error(), c.errContains) + } + }) + } +} diff --git a/coderd/workspaceapps/ticket.go b/coderd/workspaceapps/ticket.go new file mode 100644 index 0000000000..34026bc9c9 --- /dev/null +++ b/coderd/workspaceapps/ticket.go @@ -0,0 +1,102 @@ +package workspaceapps + +import ( + "encoding/json" + "time" + + "github.com/google/uuid" + "golang.org/x/xerrors" + "gopkg.in/square/go-jose.v2" +) + +const ticketSigningAlgorithm = jose.HS512 + +// Ticket is the struct data contained inside a workspace app ticket JWE. It +// contains the details of the workspace app that the ticket is valid for to +// avoid database queries. +// +// The JSON field names are short to reduce the size of the ticket. +type Ticket struct { + // Request details. + AccessMethod AccessMethod `json:"access_method"` + UsernameOrID string `json:"username_or_id"` + WorkspaceNameOrID string `json:"workspace_name_or_id"` + AgentNameOrID string `json:"agent_name_or_id"` + AppSlugOrPort string `json:"app_slug_or_port"` + + // Trusted resolved details. + Expiry int64 `json:"expiry"` // set by GenerateTicket 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"` +} + +func (t Ticket) MatchesRequest(req Request) bool { + return t.AccessMethod == req.AccessMethod && + t.UsernameOrID == req.UsernameOrID && + t.WorkspaceNameOrID == req.WorkspaceNameOrID && + t.AgentNameOrID == req.AgentNameOrID && + t.AppSlugOrPort == req.AppSlugOrPort +} + +func (p *Provider) GenerateTicket(payload Ticket) (string, error) { + if payload.Expiry == 0 { + payload.Expiry = time.Now().Add(TicketExpiry).Unix() + } + payloadBytes, err := json.Marshal(payload) + if err != nil { + return "", xerrors.Errorf("marshal payload to JSON: %w", err) + } + + // We use symmetric signing with an RSA key to support satellites in the + // future. + signer, err := jose.NewSigner(jose.SigningKey{ + Algorithm: ticketSigningAlgorithm, + Key: p.TicketSigningKey, + }, 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 +} + +func (p *Provider) ParseTicket(ticketStr string) (Ticket, error) { + object, err := jose.ParseSigned(ticketStr) + if err != nil { + return Ticket{}, xerrors.Errorf("parse JWS: %w", err) + } + if len(object.Signatures) != 1 { + return Ticket{}, xerrors.New("expected 1 signature") + } + if object.Signatures[0].Header.Algorithm != string(ticketSigningAlgorithm) { + return Ticket{}, xerrors.Errorf("expected ticket signing algorithm to be %q, got %q", ticketSigningAlgorithm, object.Signatures[0].Header.Algorithm) + } + + output, err := object.Verify(p.TicketSigningKey) + if err != nil { + return Ticket{}, xerrors.Errorf("verify JWS: %w", err) + } + + var ticket Ticket + err = json.Unmarshal(output, &ticket) + if err != nil { + return Ticket{}, xerrors.Errorf("unmarshal payload: %w", err) + } + if ticket.Expiry < time.Now().Unix() { + return Ticket{}, xerrors.New("ticket expired") + } + + return ticket, nil +} diff --git a/coderd/workspaceapps/ticket_test.go b/coderd/workspaceapps/ticket_test.go new file mode 100644 index 0000000000..f55ace4e15 --- /dev/null +++ b/coderd/workspaceapps/ticket_test.go @@ -0,0 +1,302 @@ +package workspaceapps_test + +import ( + "encoding/hex" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + "gopkg.in/square/go-jose.v2" + + "cdr.dev/slog/sloggers/slogtest" + + "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/workspaceapps" +) + +func Test_TicketMatchesRequest(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + req workspaceapps.Request + ticket workspaceapps.Ticket + want bool + }{ + { + name: "OK", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + AgentNameOrID: "baz", + AppSlugOrPort: "qux", + }, + ticket: workspaceapps.Ticket{ + AccessMethod: workspaceapps.AccessMethodPath, + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + AgentNameOrID: "baz", + AppSlugOrPort: "qux", + }, + want: true, + }, + { + name: "DifferentAccessMethod", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + }, + ticket: workspaceapps.Ticket{ + AccessMethod: workspaceapps.AccessMethodSubdomain, + }, + want: false, + }, + { + name: "DifferentUsernameOrID", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + UsernameOrID: "foo", + }, + ticket: workspaceapps.Ticket{ + AccessMethod: workspaceapps.AccessMethodPath, + UsernameOrID: "bar", + }, + want: false, + }, + { + name: "DifferentWorkspaceNameOrID", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + }, + ticket: workspaceapps.Ticket{ + AccessMethod: workspaceapps.AccessMethodPath, + UsernameOrID: "foo", + WorkspaceNameOrID: "baz", + }, + want: false, + }, + { + name: "DifferentAgentNameOrID", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + AgentNameOrID: "baz", + }, + ticket: workspaceapps.Ticket{ + AccessMethod: workspaceapps.AccessMethodPath, + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + AgentNameOrID: "qux", + }, + want: false, + }, + { + name: "DifferentAppSlugOrPort", + req: workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + AgentNameOrID: "baz", + AppSlugOrPort: "qux", + }, + ticket: workspaceapps.Ticket{ + AccessMethod: workspaceapps.AccessMethodPath, + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + AgentNameOrID: "baz", + AppSlugOrPort: "quux", + }, + want: false, + }, + } + + for _, c := range cases { + c := c + + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + require.Equal(t, c.want, c.ticket.MatchesRequest(c.req)) + }) + } +} + +func Test_GenerateTicket(t *testing.T) { + t.Parallel() + + provider := workspaceapps.New(slogtest.Make(t, nil), nil, nil, nil, nil, nil, coderdtest.AppSigningKey) + + t.Run("SetExpiry", func(t *testing.T) { + t.Parallel() + + ticketStr, err := provider.GenerateTicket(workspaceapps.Ticket{ + AccessMethod: workspaceapps.AccessMethodPath, + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + AgentNameOrID: "baz", + AppSlugOrPort: "qux", + + Expiry: 0, + UserID: uuid.MustParse("b1530ba9-76f3-415e-b597-4ddd7cd466a4"), + WorkspaceID: uuid.MustParse("1e6802d3-963e-45ac-9d8c-bf997016ffed"), + AgentID: uuid.MustParse("9ec18681-d2c9-4c9e-9186-f136efb4edbe"), + AppURL: "http://127.0.0.1:8080", + }) + require.NoError(t, err) + + ticket, err := provider.ParseTicket(ticketStr) + require.NoError(t, err) + + require.InDelta(t, time.Now().Unix(), ticket.Expiry, time.Minute.Seconds()) + }) + + future := time.Now().Add(time.Hour).Unix() + cases := []struct { + name string + ticket workspaceapps.Ticket + parseErrContains string + }{ + { + name: "OK1", + ticket: workspaceapps.Ticket{ + AccessMethod: workspaceapps.AccessMethodPath, + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + AgentNameOrID: "baz", + AppSlugOrPort: "qux", + + Expiry: future, + UserID: uuid.MustParse("b1530ba9-76f3-415e-b597-4ddd7cd466a4"), + WorkspaceID: uuid.MustParse("1e6802d3-963e-45ac-9d8c-bf997016ffed"), + AgentID: uuid.MustParse("9ec18681-d2c9-4c9e-9186-f136efb4edbe"), + AppURL: "http://127.0.0.1:8080", + }, + }, + { + name: "OK2", + ticket: workspaceapps.Ticket{ + AccessMethod: workspaceapps.AccessMethodSubdomain, + UsernameOrID: "oof", + WorkspaceNameOrID: "rab", + AgentNameOrID: "zab", + AppSlugOrPort: "xuq", + + Expiry: future, + UserID: uuid.MustParse("6fa684a3-11aa-49fd-8512-ab527bd9b900"), + WorkspaceID: uuid.MustParse("b2d816cc-505c-441d-afdf-dae01781bc0b"), + AgentID: uuid.MustParse("6c4396e1-af88-4a8a-91a3-13ea54fc29fb"), + AppURL: "http://localhost:9090", + }, + }, + { + name: "Expired", + ticket: workspaceapps.Ticket{ + AccessMethod: workspaceapps.AccessMethodSubdomain, + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + AgentNameOrID: "baz", + AppSlugOrPort: "qux", + + Expiry: time.Now().Add(-time.Hour).Unix(), + UserID: uuid.MustParse("b1530ba9-76f3-415e-b597-4ddd7cd466a4"), + WorkspaceID: uuid.MustParse("1e6802d3-963e-45ac-9d8c-bf997016ffed"), + AgentID: uuid.MustParse("9ec18681-d2c9-4c9e-9186-f136efb4edbe"), + AppURL: "http://127.0.0.1:8080", + }, + parseErrContains: "ticket expired", + }, + } + + for _, c := range cases { + c := c + + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + str, err := provider.GenerateTicket(c.ticket) + require.NoError(t, err) + + // Tickets aren't deterministic as they have a random nonce, so we + // can't compare them directly. + + ticket, err := provider.ParseTicket(str) + if c.parseErrContains != "" { + require.Error(t, err) + require.ErrorContains(t, err, c.parseErrContains) + } else { + require.NoError(t, err) + require.Equal(t, c.ticket, ticket) + } + }) + } +} + +// The ParseTicket fn is tested quite thoroughly in the GenerateTicket test. +func Test_ParseTicket(t *testing.T) { + t.Parallel() + + provider := workspaceapps.New(slogtest.Make(t, nil), nil, nil, nil, nil, nil, coderdtest.AppSigningKey) + + t.Run("InvalidJWS", func(t *testing.T) { + t.Parallel() + + ticket, err := provider.ParseTicket("invalid") + require.Error(t, err) + require.ErrorContains(t, err, "parse JWS") + require.Equal(t, workspaceapps.Ticket{}, ticket) + }) + + t.Run("VerifySignature", func(t *testing.T) { + t.Parallel() + + // Create a valid ticket using a different key. + otherKey, err := hex.DecodeString("62656566646561646265656664656164626565666465616462656566646561646265656664656164626565666465616462656566646561646265656664656164") + require.NoError(t, err) + require.NotEqual(t, coderdtest.AppSigningKey, otherKey) + require.Len(t, otherKey, 64) + + otherProvider := workspaceapps.New(slogtest.Make(t, nil), nil, nil, nil, nil, nil, otherKey) + + ticketStr, err := otherProvider.GenerateTicket(workspaceapps.Ticket{ + AccessMethod: workspaceapps.AccessMethodPath, + UsernameOrID: "foo", + WorkspaceNameOrID: "bar", + AgentNameOrID: "baz", + AppSlugOrPort: "qux", + + Expiry: time.Now().Add(time.Hour).Unix(), + UserID: uuid.MustParse("b1530ba9-76f3-415e-b597-4ddd7cd466a4"), + WorkspaceID: uuid.MustParse("1e6802d3-963e-45ac-9d8c-bf997016ffed"), + AgentID: uuid.MustParse("9ec18681-d2c9-4c9e-9186-f136efb4edbe"), + AppURL: "http://127.0.0.1:8080", + }) + require.NoError(t, err) + + // Verify the ticket is invalid. + ticket, err := provider.ParseTicket(ticketStr) + require.Error(t, err) + require.ErrorContains(t, err, "verify JWS") + require.Equal(t, workspaceapps.Ticket{}, ticket) + }) + + t.Run("InvalidBody", func(t *testing.T) { + t.Parallel() + + // Create a signature for an invalid body. + signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.HS512, Key: provider.TicketSigningKey}, nil) + require.NoError(t, err) + signedObject, err := signer.Sign([]byte("hi")) + require.NoError(t, err) + serialized, err := signedObject.CompactSerialize() + require.NoError(t, err) + + ticket, err := provider.ParseTicket(serialized) + require.Error(t, err) + require.ErrorContains(t, err, "unmarshal payload") + require.Equal(t, workspaceapps.Ticket{}, ticket) + }) +} diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index 6c1b24293d..4051cd8b95 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -8,8 +8,10 @@ import ( "io" "net" "net/http" + "net/http/cookiejar" "net/http/httputil" "net/url" + "strconv" "strings" "testing" "time" @@ -129,9 +131,27 @@ func setupProxyTest(t *testing.T, opts *setupProxyTestOpts) (*codersdk.Client, c opts.AppHost = proxyTestSubdomainRaw } - // #nosec - ln, err := net.Listen("tcp", ":0") - require.NoError(t, err) + // Start a listener on a random port greater than the minimum app port. + var ( + ln net.Listener + tcpAddr *net.TCPAddr + ) + for i := 0; i < 10; i++ { + var err error + // #nosec + ln, err = net.Listen("tcp", ":0") + require.NoError(t, err) + + var ok bool + tcpAddr, ok = ln.Addr().(*net.TCPAddr) + require.True(t, ok) + if tcpAddr.Port < codersdk.WorkspaceAgentMinimumListeningPort { + _ = ln.Close() + time.Sleep(20 * time.Millisecond) + continue + } + } + server := http.Server{ ReadHeaderTimeout: time.Minute, Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -147,8 +167,6 @@ func setupProxyTest(t *testing.T, opts *setupProxyTestOpts) (*codersdk.Client, c _ = ln.Close() }) go server.Serve(ln) - tcpAddr, ok := ln.Addr().(*net.TCPAddr) - require.True(t, ok) deploymentConfig := coderdtest.DeploymentConfig(t) deploymentConfig.DisablePathApps.Value = opts.DisablePathApps @@ -304,7 +322,7 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - resp, err := requestWithRetries(ctx, t, client, http.MethodGet, fmt.Sprintf("/@me/%s/apps/%s", workspace.Name, proxyTestAppNameOwner), nil) + resp, err := requestWithRetries(ctx, t, client, http.MethodGet, fmt.Sprintf("/@%s/%s/apps/%s", coderdtest.FirstUserParams.Username, workspace.Name, proxyTestAppNameOwner), nil) require.NoError(t, err) defer resp.Body.Close() require.Equal(t, http.StatusUnauthorized, resp.StatusCode) @@ -323,7 +341,7 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - resp, err := requestWithRetries(ctx, t, client, http.MethodGet, fmt.Sprintf("/@me/%s/apps/%s", workspace.Name, proxyTestAppNameOwner), nil) + resp, err := requestWithRetries(ctx, t, client, http.MethodGet, fmt.Sprintf("/@%s/%s/apps/%s", coderdtest.FirstUserParams.Username, workspace.Name, proxyTestAppNameOwner), nil) require.NoError(t, err) defer resp.Body.Close() @@ -344,7 +362,7 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - resp, err := requestWithRetries(ctx, t, userClient, http.MethodGet, fmt.Sprintf("/@me/%s/apps/%s", workspace.Name, proxyTestAppNameOwner), nil) + resp, err := requestWithRetries(ctx, t, userClient, http.MethodGet, fmt.Sprintf("/@%s/%s/apps/%s", coderdtest.FirstUserParams.Username, workspace.Name, proxyTestAppNameOwner), nil) require.NoError(t, err) defer resp.Body.Close() require.Equal(t, http.StatusNotFound, resp.StatusCode) @@ -356,7 +374,7 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - resp, err := requestWithRetries(ctx, t, client, http.MethodGet, fmt.Sprintf("/@me/%s/apps/%s", workspace.Name, proxyTestAppNameOwner), nil) + resp, err := requestWithRetries(ctx, t, client, http.MethodGet, fmt.Sprintf("/@%s/%s/apps/%s", coderdtest.FirstUserParams.Username, workspace.Name, proxyTestAppNameOwner), nil) require.NoError(t, err) defer resp.Body.Close() require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) @@ -368,7 +386,7 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - resp, err := requestWithRetries(ctx, t, client, http.MethodGet, fmt.Sprintf("/@me/%s/apps/%s/", workspace.Name, proxyTestAppNameOwner), nil) + resp, err := requestWithRetries(ctx, t, client, http.MethodGet, fmt.Sprintf("/@%s/%s/apps/%s/", coderdtest.FirstUserParams.Username, workspace.Name, proxyTestAppNameOwner), nil) require.NoError(t, err) defer resp.Body.Close() require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) @@ -383,13 +401,78 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - resp, err := requestWithRetries(ctx, t, client, http.MethodGet, fmt.Sprintf("/@me/%s/apps/%s/?%s", workspace.Name, proxyTestAppNameOwner, proxyTestAppQuery), nil) + basePath := fmt.Sprintf("/@%s/%s/apps/%s/", coderdtest.FirstUserParams.Username, workspace.Name, proxyTestAppNameOwner) + path := fmt.Sprintf("%s?%s", basePath, proxyTestAppQuery) + resp, err := requestWithRetries(ctx, t, client, http.MethodGet, path, nil) require.NoError(t, err) defer resp.Body.Close() body, err := io.ReadAll(resp.Body) require.NoError(t, err) require.Equal(t, proxyTestAppBody, string(body)) require.Equal(t, http.StatusOK, resp.StatusCode) + + var sessionTicketCookie *http.Cookie + for _, c := range resp.Cookies() { + if c.Name == codersdk.DevURLSessionTicketCookie { + sessionTicketCookie = c + break + } + } + require.NotNil(t, sessionTicketCookie, "no session ticket in response") + require.Equal(t, sessionTicketCookie.Path, basePath, "incorrect path on session ticket cookie") + + // Ensure the session ticket cookie is valid. + ticketClient := codersdk.New(client.URL) + ticketClient.HTTPClient.CheckRedirect = client.HTTPClient.CheckRedirect + ticketClient.HTTPClient.Transport = client.HTTPClient.Transport + u, err := ticketClient.URL.Parse(path) + require.NoError(t, err) + ticketClient.HTTPClient.Jar, err = cookiejar.New(nil) + require.NoError(t, err) + ticketClient.HTTPClient.Jar.SetCookies(u, []*http.Cookie{sessionTicketCookie}) + + resp, err = requestWithRetries(ctx, t, ticketClient, http.MethodGet, path, nil) + require.NoError(t, err) + defer resp.Body.Close() + body, err = io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, proxyTestAppBody, string(body)) + require.Equal(t, http.StatusOK, resp.StatusCode) + }) + + t.Run("RedirectsMe", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + 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 + + 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() + require.NoError(t, err) + require.Equal(t, "/login", loc.Path) }) t.Run("ForwardsIP", func(t *testing.T) { @@ -398,7 +481,7 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - resp, err := requestWithRetries(ctx, t, client, http.MethodGet, fmt.Sprintf("/@me/%s/apps/%s/?%s", workspace.Name, proxyTestAppNameOwner, proxyTestAppQuery), nil, func(r *http.Request) { + resp, err := requestWithRetries(ctx, t, client, http.MethodGet, fmt.Sprintf("/@%s/%s/apps/%s/?%s", coderdtest.FirstUserParams.Username, workspace.Name, proxyTestAppNameOwner, proxyTestAppQuery), nil, func(r *http.Request) { r.Header.Set("Cf-Connecting-IP", "1.1.1.1") }) require.NoError(t, err) @@ -416,11 +499,23 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - resp, err := client.Request(ctx, http.MethodGet, fmt.Sprintf("/@me/%s/apps/%s/", workspace.Name, proxyTestAppNameFake), nil) + resp, err := client.Request(ctx, http.MethodGet, fmt.Sprintf("/@%s/%s/apps/%s/", coderdtest.FirstUserParams.Username, workspace.Name, proxyTestAppNameFake), nil) require.NoError(t, err) defer resp.Body.Close() require.Equal(t, http.StatusBadGateway, resp.StatusCode) }) + + t.Run("NoProxyPort", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + resp, err := client.Request(ctx, http.MethodGet, fmt.Sprintf("/@%s/%s/apps/%d/", coderdtest.FirstUserParams.Username, workspace.Name, 8080), nil) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusNotFound, resp.StatusCode) + }) } func TestWorkspaceApplicationAuth(t *testing.T) { @@ -706,18 +801,16 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { // proxyURL generates a URL for the proxy subdomain. The default path is a // slash. - proxyURL := func(t *testing.T, client *codersdk.Client, appNameOrPort interface{}, pathAndQuery ...string) string { + proxyURL := func(t *testing.T, client *codersdk.Client, appSlugOrPort interface{}, pathAndQuery ...string) string { t.Helper() - var ( - appName string - port uint16 - ) - if val, ok := appNameOrPort.(string); ok { - appName = val + appSlugOrPortStr := "" + if val, ok := appSlugOrPort.(string); ok { + appSlugOrPortStr = val } else { - port, ok = appNameOrPort.(uint16) + port, ok := appSlugOrPort.(uint16) require.True(t, ok) + appSlugOrPortStr = strconv.Itoa(int(port)) } ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) @@ -736,8 +829,7 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { require.NoError(t, err, "get app host") subdomain := httpapi.ApplicationURL{ - AppSlug: appName, - Port: port, + AppSlugOrPort: appSlugOrPortStr, AgentName: proxyTestAgentName, WorkspaceName: res.Workspaces[0].Name, Username: me.Username, @@ -818,13 +910,42 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - resp, err := requestWithRetries(ctx, t, client, http.MethodGet, proxyURL(t, client, proxyTestAppNameOwner, "/", proxyTestAppQuery), nil) + uStr := proxyURL(t, client, proxyTestAppNameOwner, "/", proxyTestAppQuery) + u, err := url.Parse(uStr) + require.NoError(t, err) + resp, err := requestWithRetries(ctx, t, client, http.MethodGet, uStr, nil) require.NoError(t, err) defer resp.Body.Close() body, err := io.ReadAll(resp.Body) require.NoError(t, err) require.Equal(t, proxyTestAppBody, string(body)) require.Equal(t, http.StatusOK, resp.StatusCode) + + var sessionTicketCookie *http.Cookie + for _, c := range resp.Cookies() { + if c.Name == codersdk.DevURLSessionTicketCookie { + sessionTicketCookie = c + break + } + } + require.NotNil(t, sessionTicketCookie, "no session ticket in response") + require.Equal(t, sessionTicketCookie.Path, "/", "incorrect path on session ticket cookie") + + // Ensure the session ticket cookie is valid. + ticketClient := codersdk.New(client.URL) + ticketClient.HTTPClient.CheckRedirect = client.HTTPClient.CheckRedirect + ticketClient.HTTPClient.Transport = client.HTTPClient.Transport + ticketClient.HTTPClient.Jar, err = cookiejar.New(nil) + require.NoError(t, err) + ticketClient.HTTPClient.Jar.SetCookies(u, []*http.Cookie{sessionTicketCookie}) + + resp, err = requestWithRetries(ctx, t, ticketClient, http.MethodGet, uStr, nil) + require.NoError(t, err) + defer resp.Body.Close() + body, err = io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, proxyTestAppBody, string(body)) + require.Equal(t, http.StatusOK, resp.StatusCode) }) t.Run("ProxiesPort", func(t *testing.T) { @@ -1092,7 +1213,7 @@ func TestAppSubdomainLogout(t *testing.T) { req.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) if c.cookie != "" { req.AddCookie(&http.Cookie{ - Name: httpmw.DevURLSessionTokenCookie, + Name: codersdk.DevURLSessionTokenCookie, Value: c.cookie, }) } @@ -1109,7 +1230,7 @@ func TestAppSubdomainLogout(t *testing.T) { cookies := resp.Cookies() require.Len(t, cookies, 1, "logout response cookies") cookie := cookies[0] - require.Equal(t, httpmw.DevURLSessionTokenCookie, cookie.Name) + require.Equal(t, codersdk.DevURLSessionTokenCookie, cookie.Name) require.Equal(t, "", cookie.Value) require.True(t, cookie.Expires.Before(time.Now()), "cookie should be expired") @@ -1264,7 +1385,7 @@ func TestAppSharing(t *testing.T) { u := fmt.Sprintf("/@%s/%s.%s/apps/%s/?%s", username, workspaceName, agentName, appName, proxyTestAppQuery) if !isPathApp { subdomain := httpapi.ApplicationURL{ - AppSlug: appName, + AppSlugOrPort: appName, AgentName: agentName, WorkspaceName: workspaceName, Username: username, @@ -1512,7 +1633,7 @@ func TestWorkspaceAppsNonCanonicalHeaders(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - u, err := client.URL.Parse(fmt.Sprintf("/@me/%s/apps/%s/?%s", workspace.Name, proxyTestAppNameOwner, proxyTestAppQuery)) + u, err := client.URL.Parse(fmt.Sprintf("/@%s/%s/apps/%s/?%s", coderdtest.FirstUserParams.Username, workspace.Name, proxyTestAppNameOwner, proxyTestAppQuery)) require.NoError(t, err) req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil) diff --git a/codersdk/client.go b/codersdk/client.go index c95fa4f318..f21844ee4c 100644 --- a/codersdk/client.go +++ b/codersdk/client.go @@ -38,6 +38,15 @@ const ( // OAuth2RedirectCookie is the name of the cookie that stores the oauth2 redirect. OAuth2RedirectCookie = "oauth_redirect" + // DevURLSessionTokenCookie is the name of the cookie that stores a devurl + // token on app domains. + //nolint:gosec + DevURLSessionTokenCookie = "coder_devurl_session_token" + // DevURLSessionTicketCookie is the name of the cookie that stores a + // temporary JWT that can be used to authenticate instead of the session + // token. + DevURLSessionTicketCookie = "coder_devurl_session_ticket" + // BypassRatelimitHeader is the custom header to use to bypass ratelimits. // Only owners can bypass rate limits. This is typically used for scale testing. // nolint: gosec diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 8ddfb0bbd9..96e2d048c4 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -63,7 +63,7 @@ func New(ctx context.Context, options *Options) (*API, error) { Github: options.GithubOAuth2Config, OIDC: options.OIDCConfig, } - apiKeyMiddleware := httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + apiKeyMiddleware := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ DB: options.Database, OAuth2Configs: oauthConfigs, RedirectToLogin: false,