diff --git a/coderd/coderd.go b/coderd/coderd.go index a97a5a8213f76..6daa8a2db220b 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -123,8 +123,8 @@ 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 denotes the symmetric key to use for signing temporary app + // tokens. The key must be 64 bytes long. AppSigningKey []byte HealthcheckFunc func(ctx context.Context) (*healthcheck.Report, error) HealthcheckTimeout time.Duration @@ -297,7 +297,7 @@ func New(options *Options) *API { Authorizer: options.Authorizer, Logger: options.Logger, }, - WorkspaceAppsProvider: workspaceapps.New( + WorkspaceAppsProvider: workspaceapps.NewDBTokenProvider( options.Logger.Named("workspaceapps"), options.AccessURL, options.Authorizer, @@ -642,7 +642,7 @@ func New(options *Options) *API { r.Post("/metadata/{key}", api.workspaceAgentPostMetadata) }) // No middleware on the PTY endpoint since it uses workspace - // application auth and tickets. + // application auth and signed app tokens. r.Get("/{workspaceagent}/pty", api.workspaceAgentPTY) r.Route("/{workspaceagent}", func(r chi.Router) { r.Use( @@ -788,7 +788,7 @@ type API struct { metricsCache *metricscache.Cache workspaceAgentCache *wsconncache.Cache updateChecker *updatecheck.Checker - WorkspaceAppsProvider *workspaceapps.Provider + WorkspaceAppsProvider workspaceapps.SignedTokenProvider // 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 92541893ffe43..672315727bcd7 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -80,7 +80,7 @@ import ( "github.com/coder/coder/testutil" ) -// AppSigningKey is a 64-byte key used to sign JWTs for workspace app tickets in +// AppSigningKey is a 64-byte key used to sign JWTs for workspace app tokens in // tests. var AppSigningKey = must(hex.DecodeString("64656164626565666465616462656566646561646265656664656164626565666465616462656566646561646265656664656164626565666465616462656566")) diff --git a/coderd/httpapi/cookie.go b/coderd/httpapi/cookie.go index 51cdeec6a7199..289ee2188cf06 100644 --- a/coderd/httpapi/cookie.go +++ b/coderd/httpapi/cookie.go @@ -24,7 +24,7 @@ func StripCoderCookies(header string) string { name == codersdk.OAuth2StateCookie || name == codersdk.OAuth2RedirectCookie || name == codersdk.DevURLSessionTokenCookie || - name == codersdk.DevURLSessionTicketCookie { + name == codersdk.DevURLSignedAppTokenCookie { continue } cookies = append(cookies, part) diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index 3790edc4b45dd..6ce14dad7689e 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -564,7 +564,7 @@ func (api *API) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) { api.WebsocketWaitMutex.Unlock() defer api.WebsocketWaitGroup.Done() - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, workspaceapps.Request{ + appToken, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodTerminal, BasePath: r.URL.Path, AgentNameOrID: chi.URLParam(r, "workspaceagent"), @@ -608,7 +608,7 @@ func (api *API) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) { go httpapi.Heartbeat(ctx, conn) - agentConn, release, err := api.workspaceAgentCache.Acquire(ticket.AgentID) + agentConn, release, err := api.workspaceAgentCache.Acquire(appToken.AgentID) if err != nil { _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("dial workspace agent: %s", err)) return diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index cf3dcfb0c1b86..3ff6268e02b0e 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -124,7 +124,7 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) chiPath = "/" + chiPath } - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, workspaceapps.Request{ + token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: basePath, UsernameOrID: chi.URLParam(r, "user"), @@ -137,7 +137,7 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) return } - api.proxyWorkspaceApplication(rw, r, *ticket, chiPath) + api.proxyWorkspaceApplication(rw, r, *token, chiPath) } // handleSubdomainApplications handles subdomain-based application proxy @@ -247,7 +247,7 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht return } - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, workspaceapps.Request{ + token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodSubdomain, BasePath: "/", UsernameOrID: app.Username, @@ -263,7 +263,7 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht // app. mws := chi.Middlewares(middlewares) mws.Handler(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { - api.proxyWorkspaceApplication(rw, r, *ticket, r.URL.Path) + api.proxyWorkspaceApplication(rw, r, *token, r.URL.Path) })).ServeHTTP(rw, r.WithContext(ctx)) }) } @@ -561,7 +561,7 @@ func (api *API) setWorkspaceAppCookie(rw http.ResponseWriter, r *http.Request, t return true } -func (api *API) proxyWorkspaceApplication(rw http.ResponseWriter, r *http.Request, ticket workspaceapps.Ticket, path string) { +func (api *API) proxyWorkspaceApplication(rw http.ResponseWriter, r *http.Request, appToken workspaceapps.SignedToken, path string) { ctx := r.Context() // Filter IP headers from untrusted origins. @@ -573,12 +573,12 @@ func (api *API) proxyWorkspaceApplication(rw http.ResponseWriter, r *http.Reques return } - appURL, err := url.Parse(ticket.AppURL) + appURL, err := url.Parse(appToken.AppURL) if err != nil { site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ Status: http.StatusBadRequest, Title: "Bad Request", - Description: fmt.Sprintf("Application has an invalid URL %q: %s", ticket.AppURL, err.Error()), + Description: fmt.Sprintf("Application has an invalid URL %q: %s", appToken.AppURL, err.Error()), RetryEnabled: true, DashboardURL: api.AccessURL.String(), }) @@ -592,7 +592,7 @@ func (api *API) proxyWorkspaceApplication(rw http.ResponseWriter, r *http.Reques 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.", ticket.AppURL, port), + Message: fmt.Sprintf("App URL %q has an invalid port %q.", appToken.AppURL, port), Detail: err.Error(), }) return @@ -639,7 +639,7 @@ func (api *API) proxyWorkspaceApplication(rw http.ResponseWriter, r *http.Reques }) } - conn, release, err := api.workspaceAgentCache.Acquire(ticket.AgentID) + conn, release, err := api.workspaceAgentCache.Acquire(appToken.AgentID) if err != nil { site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ Status: http.StatusBadGateway, diff --git a/coderd/workspaceapps/auth.go b/coderd/workspaceapps/auth.go deleted file mode 100644 index e53c6f6ce66d2..0000000000000 --- a/coderd/workspaceapps/auth.go +++ /dev/null @@ -1,387 +0,0 @@ -package workspaceapps - -import ( - "context" - "database/sql" - "fmt" - "net/http" - "strings" - "time" - - "golang.org/x/xerrors" - - "cdr.dev/slog" - "github.com/coder/coder/coderd/database" - "github.com/coder/coder/coderd/database/dbauthz" - "github.com/coder/coder/coderd/httpapi" - "github.com/coder/coder/coderd/httpmw" - "github.com/coder/coder/coderd/rbac" - "github.com/coder/coder/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) { - // nolint:gocritic // We need to make a number of database calls. Setting a system context here - // // is simpler than calling dbauthz.AsSystemRestricted on every call. - // // dangerousSystemCtx is only used for database calls. The actual authentication - // // logic is handled in Provider.authorizeWorkspaceApp which directly checks the actor's - // // permissions. - dangerousSystemCtx := dbauthz.AsSystemRestricted(r.Context()) - 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 { - err := ticket.Request.Validate() - if err == nil && 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{ - Request: appReq, - } - - // 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.DeploymentValues.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 - } - - // Lookup workspace app details from DB. - dbReq, err := appReq.getDatabase(dangerousSystemCtx, p.Database) - if xerrors.Is(err, sql.ErrNoRows) { - p.writeWorkspaceApp404(rw, r, &appReq, err.Error()) - return nil, false - } else if err != nil { - p.writeWorkspaceApp500(rw, r, &appReq, err, "get app details from database") - return nil, false - } - ticket.UserID = dbReq.User.ID - ticket.WorkspaceID = dbReq.Workspace.ID - ticket.AgentID = dbReq.Agent.ID - ticket.AppURL = dbReq.AppURL - - // TODO(@deansheather): return an error if the agent is offline or the app - // is not running. - - // Verify the user has access to the app. - authed, ok := p.verifyAuthz(rw, r, authz, dbReq) - 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. - switch appReq.AccessMethod { - case AccessMethodPath: - httpmw.RedirectToLogin(rw, r, httpmw.SignedOutErrorMessage) - case AccessMethodSubdomain: - // Redirect to the app auth redirect endpoint with a valid redirect - // URI. - 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) - case AccessMethodTerminal: - // Return an error. - httpapi.ResourceNotFound(rw) - } - return nil, false - } - - // Check that the agent is online. - agentStatus := dbReq.Agent.Status(p.WorkspaceAgentInactiveTimeout) - if agentStatus.Status != database.WorkspaceAgentStatusConnected { - p.writeWorkspaceAppOffline(rw, r, &appReq, fmt.Sprintf("Agent state is %q, not %q", agentStatus.Status, database.WorkspaceAgentStatusConnected)) - return nil, false - } - - // Check that the app is healthy. - if dbReq.AppHealth != "" && dbReq.AppHealth != database.WorkspaceAppHealthDisabled && dbReq.AppHealth != database.WorkspaceAppHealthHealthy { - p.writeWorkspaceAppOffline(rw, r, &appReq, fmt.Sprintf("App health is %q, not %q", dbReq.AppHealth, database.WorkspaceAppHealthHealthy)) - 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 -} - -func (p *Provider) authorizeRequest(ctx context.Context, roles *httpmw.Authorization, dbReq *databaseRequest) (bool, error) { - accessMethod := dbReq.AccessMethod - 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. - sharingLevel := dbReq.AppSharingLevel - if isPathApp && !p.DeploymentValues.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 && - dbReq.Workspace.OwnerID.String() != roles.Actor.ID && - !p.DeploymentValues.Dangerous.AllowPathAppSiteOwnerAccess.Value() { - return false, nil - } - - // Figure out which RBAC resource to check. For terminals we use execution - // instead of application connect. - var ( - rbacAction rbac.Action = rbac.ActionCreate - rbacResource rbac.Object = dbReq.Workspace.ApplicationConnectRBAC() - // rbacResourceOwned is for the level "authenticated". We still need to - // make sure the API key has permissions to connect to the actor's own - // workspace. Scopes would prevent this. - rbacResourceOwned rbac.Object = rbac.ResourceWorkspaceApplicationConnect.WithOwner(roles.Actor.ID) - ) - if dbReq.AccessMethod == AccessMethodTerminal { - rbacResource = dbReq.Workspace.ExecutionRBAC() - rbacResourceOwned = rbac.ResourceWorkspaceExecution.WithOwner(roles.Actor.ID) - } - - // 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, rbacAction, rbacResource) - 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: - // Check with the owned resource to ensure the API key has permissions - // to connect to the actor's own workspace. This enforces scopes. - err := p.Authorizer.Authorize(ctx, roles.Actor, rbacAction, rbacResourceOwned) - 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 -} - -// verifyAuthz 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) verifyAuthz(rw http.ResponseWriter, r *http.Request, authz *httpmw.Authorization, dbReq *databaseRequest) (authed bool, ok bool) { - ok, err := p.authorizeRequest(r.Context(), authz, dbReq) - 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(), - }) -} - -// writeWorkspaceAppOffline writes a HTML 502 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) writeWorkspaceAppOffline(rw http.ResponseWriter, r *http.Request, appReq *Request, msg string) { - if appReq != nil { - slog.Helper() - p.Logger.Debug(r.Context(), - "workspace app unavailable: "+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.StatusBadGateway, - Title: "Application Unavailable", - Description: msg, - RetryEnabled: true, - DashboardURL: p.AccessURL.String(), - }) -} diff --git a/coderd/workspaceapps/db.go b/coderd/workspaceapps/db.go new file mode 100644 index 0000000000000..be00a6c6a1f30 --- /dev/null +++ b/coderd/workspaceapps/db.go @@ -0,0 +1,293 @@ +package workspaceapps + +import ( + "context" + "database/sql" + "fmt" + "net/http" + "net/url" + "time" + + "golang.org/x/xerrors" + + "cdr.dev/slog" + + "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/dbauthz" + "github.com/coder/coder/coderd/httpapi" + "github.com/coder/coder/coderd/httpmw" + "github.com/coder/coder/coderd/rbac" + "github.com/coder/coder/codersdk" +) + +// DBTokenProvider provides authentication and authorization for workspace apps +// by querying the database if the request is missing a valid token. +type DBTokenProvider struct { + Logger slog.Logger + + // AccessURL is the main dashboard access URL for error pages. + AccessURL *url.URL + Authorizer rbac.Authorizer + Database database.Store + DeploymentValues *codersdk.DeploymentValues + OAuth2Configs *httpmw.OAuth2Configs + WorkspaceAgentInactiveTimeout time.Duration + TokenSigningKey []byte +} + +var _ SignedTokenProvider = &DBTokenProvider{} + +func NewDBTokenProvider(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database.Store, cfg *codersdk.DeploymentValues, oauth2Cfgs *httpmw.OAuth2Configs, workspaceAgentInactiveTimeout time.Duration, tokenSigningKey []byte) SignedTokenProvider { + if len(tokenSigningKey) != 64 { + panic("token signing key must be 64 bytes") + } + + if workspaceAgentInactiveTimeout == 0 { + workspaceAgentInactiveTimeout = 1 * time.Minute + } + + return &DBTokenProvider{ + Logger: log, + AccessURL: accessURL, + Authorizer: authz, + Database: db, + DeploymentValues: cfg, + OAuth2Configs: oauth2Cfgs, + WorkspaceAgentInactiveTimeout: workspaceAgentInactiveTimeout, + TokenSigningKey: tokenSigningKey, + } +} + +func (p *DBTokenProvider) TokenFromRequest(r *http.Request) (*SignedToken, bool) { + // Get the existing token from the request. + tokenCookie, err := r.Cookie(codersdk.DevURLSignedAppTokenCookie) + if err == nil { + token, err := ParseToken(p.TokenSigningKey, tokenCookie.Value) + if err == nil { + req := token.Request.Normalize() + err := req.Validate() + if err == nil { + // The request has a valid signed app token, which is a valid + // token signed by us. The caller must check that it matches + // the request. + return &token, true + } + } + } + + return nil, false +} + +// ResolveRequest takes an app request, checks if it's valid and authenticated, +// and returns a token with details about the app. +func (p *DBTokenProvider) CreateToken(ctx context.Context, rw http.ResponseWriter, r *http.Request, appReq Request) (*SignedToken, string, bool) { + // nolint:gocritic // We need to make a number of database calls. Setting a system context here + // // is simpler than calling dbauthz.AsSystemRestricted on every call. + // // dangerousSystemCtx is only used for database calls. The actual authentication + // // logic is handled in Provider.authorizeWorkspaceApp which directly checks the actor's + // // permissions. + dangerousSystemCtx := dbauthz.AsSystemRestricted(ctx) + + appReq = appReq.Normalize() + err := appReq.Validate() + if err != nil { + WriteWorkspaceApp500(p.Logger, p.AccessURL, rw, r, &appReq, err, "invalid app request") + return nil, "", false + } + + token := SignedToken{ + Request: appReq, + } + + // 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.DeploymentValues.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 + } + + // Lookup workspace app details from DB. + dbReq, err := appReq.getDatabase(dangerousSystemCtx, p.Database) + if xerrors.Is(err, sql.ErrNoRows) { + WriteWorkspaceApp404(p.Logger, p.AccessURL, rw, r, &appReq, err.Error()) + return nil, "", false + } else if err != nil { + WriteWorkspaceApp500(p.Logger, p.AccessURL, rw, r, &appReq, err, "get app details from database") + return nil, "", false + } + token.UserID = dbReq.User.ID + token.WorkspaceID = dbReq.Workspace.ID + token.AgentID = dbReq.Agent.ID + token.AppURL = dbReq.AppURL + + // TODO(@deansheather): return an error if the agent is offline or the app + // is not running. + + // Verify the user has access to the app. + authed, err := p.authorizeRequest(r.Context(), authz, dbReq) + if err != nil { + WriteWorkspaceApp500(p.Logger, p.AccessURL, rw, r, &appReq, err, "verify authz") + return nil, "", false + } + if !authed { + if apiKey != nil { + // The request has a valid API key but insufficient permissions. + WriteWorkspaceApp404(p.Logger, p.AccessURL, 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. + switch appReq.AccessMethod { + case AccessMethodPath: + // TODO(@deansheather): this doesn't work on moons + httpmw.RedirectToLogin(rw, r, httpmw.SignedOutErrorMessage) + case AccessMethodSubdomain: + // Redirect to the app auth redirect endpoint with a valid redirect + // URI. + 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) + case AccessMethodTerminal: + // Return an error. + httpapi.ResourceNotFound(rw) + } + return nil, "", false + } + + // Check that the agent is online. + agentStatus := dbReq.Agent.Status(p.WorkspaceAgentInactiveTimeout) + if agentStatus.Status != database.WorkspaceAgentStatusConnected { + WriteWorkspaceAppOffline(p.Logger, p.AccessURL, rw, r, &appReq, fmt.Sprintf("Agent state is %q, not %q", agentStatus.Status, database.WorkspaceAgentStatusConnected)) + return nil, "", false + } + + // Check that the app is healthy. + if dbReq.AppHealth != "" && dbReq.AppHealth != database.WorkspaceAppHealthDisabled && dbReq.AppHealth != database.WorkspaceAppHealthHealthy { + WriteWorkspaceAppOffline(p.Logger, p.AccessURL, rw, r, &appReq, fmt.Sprintf("App health is %q, not %q", dbReq.AppHealth, database.WorkspaceAppHealthHealthy)) + return nil, "", false + } + + // As a sanity check, ensure the token we just made is valid for this + // request. + if !token.MatchesRequest(appReq) { + WriteWorkspaceApp500(p.Logger, p.AccessURL, rw, r, &appReq, nil, "fresh token does not match request") + return nil, "", false + } + + // Sign the token. + token.Expiry = time.Now().Add(DefaultTokenExpiry) + tokenStr, err := GenerateToken(p.TokenSigningKey, token) + if err != nil { + WriteWorkspaceApp500(p.Logger, p.AccessURL, rw, r, &appReq, err, "generate token") + return nil, "", false + } + + return &token, tokenStr, true +} + +func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *httpmw.Authorization, dbReq *databaseRequest) (bool, error) { + accessMethod := dbReq.AccessMethod + 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. + sharingLevel := dbReq.AppSharingLevel + if isPathApp && !p.DeploymentValues.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 && + dbReq.Workspace.OwnerID.String() != roles.Actor.ID && + !p.DeploymentValues.Dangerous.AllowPathAppSiteOwnerAccess.Value() { + return false, nil + } + + // Figure out which RBAC resource to check. For terminals we use execution + // instead of application connect. + var ( + rbacAction rbac.Action = rbac.ActionCreate + rbacResource rbac.Object = dbReq.Workspace.ApplicationConnectRBAC() + // rbacResourceOwned is for the level "authenticated". We still need to + // make sure the API key has permissions to connect to the actor's own + // workspace. Scopes would prevent this. + rbacResourceOwned rbac.Object = rbac.ResourceWorkspaceApplicationConnect.WithOwner(roles.Actor.ID) + ) + if dbReq.AccessMethod == AccessMethodTerminal { + rbacResource = dbReq.Workspace.ExecutionRBAC() + rbacResourceOwned = rbac.ResourceWorkspaceExecution.WithOwner(roles.Actor.ID) + } + + // 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, rbacAction, rbacResource) + 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: + // Check with the owned resource to ensure the API key has permissions + // to connect to the actor's own workspace. This enforces scopes. + err := p.Authorizer.Authorize(ctx, roles.Actor, rbacAction, rbacResourceOwned) + 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 +} diff --git a/coderd/workspaceapps/auth_test.go b/coderd/workspaceapps/db_test.go similarity index 80% rename from coderd/workspaceapps/auth_test.go rename to coderd/workspaceapps/db_test.go index f1d06ac981938..f566c5c838e0b 100644 --- a/coderd/workspaceapps/auth_test.go +++ b/coderd/workspaceapps/db_test.go @@ -218,8 +218,8 @@ func Test_ResolveRequest(t *testing.T) { 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. + // Try resolving a request for each app as the owner, without a + // token, then use the token to resolve each app. for _, app := range allApps { req := workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, @@ -235,8 +235,8 @@ func Test_ResolveRequest(t *testing.T) { 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) + // Try resolving the request without a token. + token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) w := rw.Result() if !assert.True(t, ok) { dump, err := httputil.DumpResponse(w, true) @@ -246,35 +246,41 @@ func Test_ResolveRequest(t *testing.T) { } _ = w.Body.Close() - require.Equal(t, &workspaceapps.Ticket{ + require.Equal(t, &workspaceapps.SignedToken{ Request: req, - Expiry: ticket.Expiry, // ignored to avoid flakiness + Expiry: token.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()) + }, token) + require.NotZero(t, token.Expiry) + require.WithinDuration(t, time.Now().Add(workspaceapps.DefaultTokenExpiry), token.Expiry, time.Minute) - // Check that the ticket was set in the response and is valid. + // Check that the token 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, codersdk.DevURLSignedAppTokenCookie, cookie.Name) require.Equal(t, req.BasePath, cookie.Path) - parsedTicket, err := api.WorkspaceAppsProvider.ParseTicket(cookie.Value) + parsedToken, err := workspaceapps.ParseToken(api.AppSigningKey, cookie.Value) require.NoError(t, err) - require.Equal(t, ticket, &parsedTicket) + // normalize expiry + require.WithinDuration(t, token.Expiry, parsedToken.Expiry, 2*time.Second) + parsedToken.Expiry = token.Expiry + require.Equal(t, token, &parsedToken) - // Try resolving the request with the ticket only. + // Try resolving the request with the token only. rw = httptest.NewRecorder() r = httptest.NewRequest("GET", "/app", nil) r.AddCookie(cookie) - secondTicket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + secondToken, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.True(t, ok) - require.Equal(t, ticket, secondTicket) + // normalize expiry + require.WithinDuration(t, token.Expiry, secondToken.Expiry, 2*time.Second) + secondToken.Expiry = token.Expiry + require.Equal(t, token, secondToken) } }) } @@ -298,18 +304,18 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, secondUserClient.SessionToken()) - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) w := rw.Result() _ = w.Body.Close() if app == appNameOwner { require.False(t, ok) - require.Nil(t, ticket) + require.Nil(t, token) require.NotZero(t, w.StatusCode) require.Equal(t, http.StatusNotFound, w.StatusCode) return } require.True(t, ok) - require.NotNil(t, ticket) + require.NotNil(t, token) require.Zero(t, w.StatusCode) } }) @@ -330,11 +336,11 @@ func Test_ResolveRequest(t *testing.T) { t.Log("app", app) rw := httptest.NewRecorder() r := httptest.NewRequest("GET", "/app", nil) - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) w := rw.Result() if app != appNamePublic { require.False(t, ok) - require.Nil(t, ticket) + require.Nil(t, token) require.NotZero(t, rw.Code) require.NotEqual(t, http.StatusOK, rw.Code) } else { @@ -344,7 +350,7 @@ func Test_ResolveRequest(t *testing.T) { t.Log(string(dump)) return } - require.NotNil(t, ticket) + require.NotNil(t, token) if rw.Code != 0 && rw.Code != http.StatusOK { t.Fatalf("expected 200 (or unset) response code, got %d", rw.Code) } @@ -361,9 +367,9 @@ func Test_ResolveRequest(t *testing.T) { } rw := httptest.NewRecorder() r := httptest.NewRequest("GET", "/app", nil) - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.False(t, ok) - require.Nil(t, ticket) + require.Nil(t, token) }) t.Run("SplitWorkspaceAndAgent", func(t *testing.T) { @@ -435,7 +441,7 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) w := rw.Result() if !assert.Equal(t, c.ok, ok) { dump, err := httputil.DumpResponse(w, true) @@ -444,23 +450,23 @@ func Test_ResolveRequest(t *testing.T) { 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) + require.NotNil(t, token) + require.Equal(t, token.WorkspaceNameOrID, c.workspace) + require.Equal(t, token.AgentNameOrID, c.agent) + require.Equal(t, token.WorkspaceID, workspace.ID) + require.Equal(t, token.AgentID, agentID) } else { - require.Nil(t, ticket) + require.Nil(t, token) } _ = w.Body.Close() }) } }) - t.Run("TicketDoesNotMatchRequest", func(t *testing.T) { + t.Run("TokenDoesNotMatchRequest", func(t *testing.T) { t.Parallel() - badTicket := workspaceapps.Ticket{ + badToken := workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/app", @@ -470,13 +476,13 @@ func Test_ResolveRequest(t *testing.T) { // App name differs AppSlugOrPort: appNamePublic, }, - Expiry: time.Now().Add(time.Minute).Unix(), + Expiry: time.Now().Add(time.Minute), UserID: me.ID, WorkspaceID: workspace.ID, AgentID: agentID, AppURL: appURL, } - badTicketStr, err := api.WorkspaceAppsProvider.GenerateTicket(badTicket) + badTokenStr, err := workspaceapps.GenerateToken(api.AppSigningKey, badToken) require.NoError(t, err) req := workspaceapps.Request{ @@ -493,28 +499,28 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) r.AddCookie(&http.Cookie{ - Name: codersdk.DevURLSessionTicketCookie, - Value: badTicketStr, + Name: codersdk.DevURLSignedAppTokenCookie, + Value: badTokenStr, }) - // Even though the ticket is invalid, we should still perform request - // resolution. - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + // Even though the token is invalid, we should still perform request + // resolution without failure since we'll just ignore the bad token. + token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.True(t, ok) - require.NotNil(t, ticket) - require.Equal(t, appNameOwner, ticket.AppSlugOrPort) + require.NotNil(t, token) + require.Equal(t, appNameOwner, token.AppSlugOrPort) // Cookie should be set in response, and it should be a different - // ticket. + // token. 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.Equal(t, cookies[0].Name, codersdk.DevURLSignedAppTokenCookie) + require.NotEqual(t, cookies[0].Value, badTokenStr) + parsedToken, err := workspaceapps.ParseToken(api.AppSigningKey, cookies[0].Value) require.NoError(t, err) - require.Equal(t, appNameOwner, parsedTicket.AppSlugOrPort) + require.Equal(t, appNameOwner, parsedToken.AppSlugOrPort) }) t.Run("PortPathBlocked", func(t *testing.T) { @@ -533,9 +539,9 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.False(t, ok) - require.Nil(t, ticket) + require.Nil(t, token) }) t.Run("PortSubdomain", func(t *testing.T) { @@ -554,10 +560,10 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/", nil) r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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) + require.Equal(t, req.AppSlugOrPort, token.AppSlugOrPort) + require.Equal(t, "http://127.0.0.1:9090", token.AppURL) }) t.Run("Terminal", func(t *testing.T) { @@ -573,15 +579,15 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.True(t, ok) - require.Equal(t, req.AccessMethod, ticket.AccessMethod) - require.Equal(t, req.BasePath, ticket.BasePath) - require.Empty(t, ticket.UsernameOrID) - require.Empty(t, ticket.WorkspaceNameOrID) - require.Equal(t, req.AgentNameOrID, ticket.Request.AgentNameOrID) - require.Empty(t, ticket.AppSlugOrPort) - require.Empty(t, ticket.AppURL) + require.Equal(t, req.AccessMethod, token.AccessMethod) + require.Equal(t, req.BasePath, token.BasePath) + require.Empty(t, token.UsernameOrID) + require.Empty(t, token.WorkspaceNameOrID) + require.Equal(t, req.AgentNameOrID, token.Request.AgentNameOrID) + require.Empty(t, token.AppSlugOrPort) + require.Empty(t, token.AppURL) }) t.Run("InsufficientPermissions", func(t *testing.T) { @@ -600,9 +606,9 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, secondUserClient.SessionToken()) - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.False(t, ok) - require.Nil(t, ticket) + require.Nil(t, token) }) t.Run("UserNotFound", func(t *testing.T) { @@ -620,9 +626,9 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.False(t, ok) - require.Nil(t, ticket) + require.Nil(t, token) }) t.Run("RedirectSubdomainAuth", func(t *testing.T) { @@ -641,9 +647,9 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/some-path", nil) r.Host = "app.com" - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.False(t, ok) - require.Nil(t, ticket) + require.Nil(t, token) w := rw.Result() defer w.Body.Close() @@ -681,9 +687,9 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.False(t, ok, "request succeeded even though agent is not connected") - require.Nil(t, ticket) + require.Nil(t, token) w := rw.Result() defer w.Body.Close() @@ -735,9 +741,9 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + token, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.False(t, ok, "request succeeded even though app is unhealthy") - require.Nil(t, ticket) + require.Nil(t, token) w := rw.Result() defer w.Body.Close() diff --git a/coderd/workspaceapps/errors.go b/coderd/workspaceapps/errors.go new file mode 100644 index 0000000000000..b9724c6f79f69 --- /dev/null +++ b/coderd/workspaceapps/errors.go @@ -0,0 +1,85 @@ +package workspaceapps + +import ( + "net/http" + "net/url" + + "cdr.dev/slog" + "github.com/coder/coder/site" +) + +// 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 WriteWorkspaceApp404(log slog.Logger, accessURL *url.URL, rw http.ResponseWriter, r *http.Request, appReq *Request, msg string) { + if appReq != nil { + slog.Helper() + log.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: 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 WriteWorkspaceApp500(log slog.Logger, accessURL *url.URL, rw http.ResponseWriter, r *http.Request, appReq *Request, err error, msg string) { + ctx := r.Context() + if appReq != nil { + slog.Helper() + ctx = 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), + ) + } + log.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: accessURL.String(), + }) +} + +// WriteWorkspaceAppOffline writes a HTML 502 error page for a workspace app. If +// appReq is not nil, it will be used to log the request details at debug level. +func WriteWorkspaceAppOffline(log slog.Logger, accessURL *url.URL, rw http.ResponseWriter, r *http.Request, appReq *Request, msg string) { + if appReq != nil { + slog.Helper() + log.Debug(r.Context(), + "workspace app unavailable: "+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.StatusBadGateway, + Title: "Application Unavailable", + Description: msg, + RetryEnabled: true, + DashboardURL: accessURL.String(), + }) +} diff --git a/coderd/workspaceapps/provider.go b/coderd/workspaceapps/provider.go index 23e1fb7e3c736..7b363ebc909d9 100644 --- a/coderd/workspaceapps/provider.go +++ b/coderd/workspaceapps/provider.go @@ -1,48 +1,72 @@ package workspaceapps import ( + "context" + "net/http" "net/url" "time" "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 - DeploymentValues *codersdk.DeploymentValues - OAuth2Configs *httpmw.OAuth2Configs - WorkspaceAgentInactiveTimeout time.Duration - TicketSigningKey []byte -} +const ( + // TODO(@deansheather): configurable expiry + DefaultTokenExpiry = 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" +) -func New(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database.Store, cfg *codersdk.DeploymentValues, oauth2Cfgs *httpmw.OAuth2Configs, workspaceAgentInactiveTimeout time.Duration, ticketSigningKey []byte) *Provider { - if len(ticketSigningKey) != 64 { - panic("ticket signing key must be 64 bytes") +// ResolveRequest calls TokenProvider to use an existing signed app token in the +// request or issue a new one. If it returns a newly minted token, it sets the +// cookie for you. +func ResolveRequest(log slog.Logger, accessURL *url.URL, p SignedTokenProvider, rw http.ResponseWriter, r *http.Request, appReq Request) (*SignedToken, bool) { + appReq = appReq.Normalize() + err := appReq.Validate() + if err != nil { + WriteWorkspaceApp500(log, accessURL, rw, r, &appReq, err, "invalid app request") + return nil, false } - if workspaceAgentInactiveTimeout == 0 { - workspaceAgentInactiveTimeout = 1 * time.Minute + token, ok := p.TokenFromRequest(r) + if ok && token.MatchesRequest(appReq) { + // The request has a valid signed app token and it matches the request. + return token, true } - return &Provider{ - Logger: log, - AccessURL: accessURL, - Authorizer: authz, - Database: db, - DeploymentValues: cfg, - OAuth2Configs: oauth2Cfgs, - WorkspaceAgentInactiveTimeout: workspaceAgentInactiveTimeout, - TicketSigningKey: ticketSigningKey, + token, tokenStr, ok := p.CreateToken(r.Context(), rw, r, appReq) + if !ok { + return nil, false } + + // Write the signed app token cookie. We always want this to apply to the + // current hostname (even for subdomain apps, without any wildcard + // shenanigans, because the token is only valid for a single app). + http.SetCookie(rw, &http.Cookie{ + Name: codersdk.DevURLSignedAppTokenCookie, + Value: tokenStr, + Path: appReq.BasePath, + Expires: token.Expiry, + }) + + return token, true +} + +// SignedTokenProvider provides signed workspace app tokens (aka. app tickets). +type SignedTokenProvider interface { + // TokenFromRequest returns a parsed token from the request. If the request + // does not contain a signed app token or is is invalid (expired, invalid + // signature, etc.), it returns false. + TokenFromRequest(r *http.Request) (*SignedToken, bool) + // CreateToken mints a new token for the given app request. It uses the + // long-lived session token in the HTTP request to authenticate and + // authorize the client for the given workspace app. The token is returned + // in struct and string form. The string form should be written as a cookie. + // + // If the request is invalid or the user is not authorized to access the + // app, false is returned. An error page is written to the response writer + // in this case. + CreateToken(ctx context.Context, rw http.ResponseWriter, r *http.Request, appReq Request) (*SignedToken, string, bool) } diff --git a/coderd/workspaceapps/request.go b/coderd/workspaceapps/request.go index 4cc309848ad4f..349d71673d487 100644 --- a/coderd/workspaceapps/request.go +++ b/coderd/workspaceapps/request.go @@ -44,6 +44,25 @@ type Request struct { AppSlugOrPort string `json:"app_slug_or_port"` } +// Normalize replaces WorkspaceAndAgent with WorkspaceNameOrID and +// AgentNameOrID. This must be called before Validate. +func (r Request) Normalize() Request { + req := r + if req.WorkspaceAndAgent != "" { + // workspace.agent + workspaceAndAgent := strings.SplitN(req.WorkspaceAndAgent, ".", 2) + req.WorkspaceAndAgent = "" + req.WorkspaceNameOrID = workspaceAndAgent[0] + if len(workspaceAndAgent) > 1 { + req.AgentNameOrID = workspaceAndAgent[1] + } + } + + return req +} + +// Validate ensures the request is correct and contains the necessary +// parameters. func (r Request) Validate() error { switch r.AccessMethod { case AccessMethodPath, AccessMethodSubdomain, AccessMethodTerminal: @@ -54,8 +73,12 @@ func (r Request) Validate() error { return xerrors.New("base path is required") } + if r.WorkspaceAndAgent != "" { + return xerrors.New("dev error: appReq.Validate() called before appReq.Normalize()") + } + if r.AccessMethod == AccessMethodTerminal { - if r.UsernameOrID != "" || r.WorkspaceAndAgent != "" || r.WorkspaceNameOrID != "" || r.AppSlugOrPort != "" { + if r.UsernameOrID != "" || r.WorkspaceNameOrID != "" || r.AppSlugOrPort != "" { return xerrors.New("dev error: cannot specify any fields other than r.AccessMethod, r.BasePath and r.AgentNameOrID for terminal access method") } @@ -75,25 +98,16 @@ func (r Request) Validate() error { 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. + // different user and potentially reusing a token. // // This is also mitigated by storing the workspace/agent ID in the - // ticket, but we block it here to be double safe. + // token, 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 == "" { + if r.WorkspaceNameOrID == "" { return xerrors.New("workspace name or ID is required") } if r.AppSlugOrPort == "" { diff --git a/coderd/workspaceapps/request_test.go b/coderd/workspaceapps/request_test.go index 56284f2a2b11a..e6fb0279d3ecd 100644 --- a/coderd/workspaceapps/request_test.go +++ b/coderd/workspaceapps/request_test.go @@ -117,7 +117,7 @@ func Test_RequestValidate(t *testing.T) { errContains: `username cannot be "me"`, }, { - name: "InvalidWorkspaceAndAgent/Empty1", + name: "InvalidWorkspaceAndAgent/EmptyWorkspace", req: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/", @@ -125,53 +125,7 @@ func Test_RequestValidate(t *testing.T) { 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", + errContains: "workspace name or ID is required", }, { name: "NoWorkspaceNameOrID", @@ -261,12 +215,13 @@ func Test_RequestValidate(t *testing.T) { c := c t.Run(c.name, func(t *testing.T) { t.Parallel() - err := c.req.Validate() + req := c.req.Normalize() + err := req.Validate() if c.errContains == "" { require.NoError(t, err) } else { require.Error(t, err) - require.Contains(t, err.Error(), c.errContains) + require.ErrorContains(t, err, c.errContains) } }) } diff --git a/coderd/workspaceapps/ticket.go b/coderd/workspaceapps/ticket.go index 11fb6465b190a..bd7d74e4194e5 100644 --- a/coderd/workspaceapps/ticket.go +++ b/coderd/workspaceapps/ticket.go @@ -9,26 +9,26 @@ import ( "gopkg.in/square/go-jose.v2" ) -const ticketSigningAlgorithm = jose.HS512 +const tokenSigningAlgorithm = 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 +// SignedToken is the struct data contained inside a workspace app JWE. It +// contains the details of the workspace app that the token is valid for to // avoid database queries. -// -// The JSON field names are short to reduce the size of the ticket. -type Ticket struct { +type SignedToken struct { // Request details. Request `json:"request"` // Trusted resolved details. - Expiry int64 `json:"expiry"` // set by GenerateTicket if unset + Expiry time.Time `json:"expiry"` // set by GenerateToken if unset UserID uuid.UUID `json:"user_id"` WorkspaceID uuid.UUID `json:"workspace_id"` AgentID uuid.UUID `json:"agent_id"` AppURL string `json:"app_url"` } -func (t Ticket) MatchesRequest(req Request) bool { +// MatchesRequest returns true if the token matches the request. Any token that +// does not match the request should be considered invalid. +func (t SignedToken) MatchesRequest(req Request) bool { return t.AccessMethod == req.AccessMethod && t.BasePath == req.BasePath && t.UsernameOrID == req.UsernameOrID && @@ -37,20 +37,21 @@ func (t Ticket) MatchesRequest(req Request) bool { t.AppSlugOrPort == req.AppSlugOrPort } -func (p *Provider) GenerateTicket(payload Ticket) (string, error) { - if payload.Expiry == 0 { - payload.Expiry = time.Now().Add(TicketExpiry).Unix() +// GenerateToken generates a signed workspace app token with the given key and +// payload. If the payload doesn't have an expiry, it will be set to the current +// time plus the default expiry. +func GenerateToken(key []byte, payload SignedToken) (string, error) { + if payload.Expiry.IsZero() { + payload.Expiry = time.Now().Add(DefaultTokenExpiry) } payloadBytes, err := json.Marshal(payload) if err != nil { return "", xerrors.Errorf("marshal payload to JSON: %w", err) } - // 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, + Algorithm: tokenSigningAlgorithm, + Key: key, }, nil) if err != nil { return "", xerrors.Errorf("create signer: %w", err) @@ -69,31 +70,33 @@ func (p *Provider) GenerateTicket(payload Ticket) (string, error) { return serialized, nil } -func (p *Provider) ParseTicket(ticketStr string) (Ticket, error) { - object, err := jose.ParseSigned(ticketStr) +// ParseToken parses a signed workspace app token with the given key and returns +// the payload. If the token is invalid or expired, an error is returned. +func ParseToken(key []byte, str string) (SignedToken, error) { + object, err := jose.ParseSigned(str) if err != nil { - return Ticket{}, xerrors.Errorf("parse JWS: %w", err) + return SignedToken{}, xerrors.Errorf("parse JWS: %w", err) } if len(object.Signatures) != 1 { - return Ticket{}, xerrors.New("expected 1 signature") + return SignedToken{}, 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) + if object.Signatures[0].Header.Algorithm != string(tokenSigningAlgorithm) { + return SignedToken{}, xerrors.Errorf("expected token signing algorithm to be %q, got %q", tokenSigningAlgorithm, object.Signatures[0].Header.Algorithm) } - output, err := object.Verify(p.TicketSigningKey) + output, err := object.Verify(key) if err != nil { - return Ticket{}, xerrors.Errorf("verify JWS: %w", err) + return SignedToken{}, xerrors.Errorf("verify JWS: %w", err) } - var ticket Ticket - err = json.Unmarshal(output, &ticket) + var tok SignedToken + err = json.Unmarshal(output, &tok) if err != nil { - return Ticket{}, xerrors.Errorf("unmarshal payload: %w", err) + return SignedToken{}, xerrors.Errorf("unmarshal payload: %w", err) } - if ticket.Expiry < time.Now().Unix() { - return Ticket{}, xerrors.New("ticket expired") + if tok.Expiry.Before(time.Now()) { + return SignedToken{}, xerrors.New("signed app token expired") } - return ticket, nil + return tok, nil } diff --git a/coderd/workspaceapps/ticket_test.go b/coderd/workspaceapps/ticket_test.go index c7fd79cff3104..9b5cc6a4e3e85 100644 --- a/coderd/workspaceapps/ticket_test.go +++ b/coderd/workspaceapps/ticket_test.go @@ -9,20 +9,18 @@ import ( "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) { +func Test_TokenMatchesRequest(t *testing.T) { t.Parallel() cases := []struct { - name string - req workspaceapps.Request - ticket workspaceapps.Ticket - want bool + name string + req workspaceapps.Request + token workspaceapps.SignedToken + want bool }{ { name: "OK", @@ -34,7 +32,7 @@ func Test_TicketMatchesRequest(t *testing.T) { AgentNameOrID: "baz", AppSlugOrPort: "qux", }, - ticket: workspaceapps.Ticket{ + token: workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/app", @@ -51,7 +49,7 @@ func Test_TicketMatchesRequest(t *testing.T) { req: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, }, - ticket: workspaceapps.Ticket{ + token: workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodSubdomain, }, @@ -63,7 +61,7 @@ func Test_TicketMatchesRequest(t *testing.T) { req: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, }, - ticket: workspaceapps.Ticket{ + token: workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/app", @@ -78,7 +76,7 @@ func Test_TicketMatchesRequest(t *testing.T) { BasePath: "/app", UsernameOrID: "foo", }, - ticket: workspaceapps.Ticket{ + token: workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/app", @@ -95,7 +93,7 @@ func Test_TicketMatchesRequest(t *testing.T) { UsernameOrID: "foo", WorkspaceNameOrID: "bar", }, - ticket: workspaceapps.Ticket{ + token: workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/app", @@ -114,7 +112,7 @@ func Test_TicketMatchesRequest(t *testing.T) { WorkspaceNameOrID: "bar", AgentNameOrID: "baz", }, - ticket: workspaceapps.Ticket{ + token: workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/app", @@ -135,7 +133,7 @@ func Test_TicketMatchesRequest(t *testing.T) { AgentNameOrID: "baz", AppSlugOrPort: "qux", }, - ticket: workspaceapps.Ticket{ + token: workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/app", @@ -155,20 +153,18 @@ func Test_TicketMatchesRequest(t *testing.T) { t.Run(c.name, func(t *testing.T) { t.Parallel() - require.Equal(t, c.want, c.ticket.MatchesRequest(c.req)) + require.Equal(t, c.want, c.token.MatchesRequest(c.req)) }) } } -func Test_GenerateTicket(t *testing.T) { +func Test_GenerateToken(t *testing.T) { t.Parallel() - provider := workspaceapps.New(slogtest.Make(t, nil), nil, nil, nil, nil, nil, time.Minute, coderdtest.AppSigningKey) - t.Run("SetExpiry", func(t *testing.T) { t.Parallel() - ticketStr, err := provider.GenerateTicket(workspaceapps.Ticket{ + tokenStr, err := workspaceapps.GenerateToken(coderdtest.AppSigningKey, workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/app", @@ -178,7 +174,7 @@ func Test_GenerateTicket(t *testing.T) { AppSlugOrPort: "qux", }, - Expiry: 0, + Expiry: time.Time{}, UserID: uuid.MustParse("b1530ba9-76f3-415e-b597-4ddd7cd466a4"), WorkspaceID: uuid.MustParse("1e6802d3-963e-45ac-9d8c-bf997016ffed"), AgentID: uuid.MustParse("9ec18681-d2c9-4c9e-9186-f136efb4edbe"), @@ -186,21 +182,21 @@ func Test_GenerateTicket(t *testing.T) { }) require.NoError(t, err) - ticket, err := provider.ParseTicket(ticketStr) + token, err := workspaceapps.ParseToken(coderdtest.AppSigningKey, tokenStr) require.NoError(t, err) - require.InDelta(t, time.Now().Unix(), ticket.Expiry, time.Minute.Seconds()) + require.WithinDuration(t, time.Now().Add(time.Minute), token.Expiry, 15*time.Second) }) - future := time.Now().Add(time.Hour).Unix() + future := time.Now().Add(time.Hour) cases := []struct { name string - ticket workspaceapps.Ticket + token workspaceapps.SignedToken parseErrContains string }{ { name: "OK1", - ticket: workspaceapps.Ticket{ + token: workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/app", @@ -219,7 +215,7 @@ func Test_GenerateTicket(t *testing.T) { }, { name: "OK2", - ticket: workspaceapps.Ticket{ + token: workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodSubdomain, BasePath: "/", @@ -238,7 +234,7 @@ func Test_GenerateTicket(t *testing.T) { }, { name: "Expired", - ticket: workspaceapps.Ticket{ + token: workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodSubdomain, BasePath: "/", @@ -248,13 +244,13 @@ func Test_GenerateTicket(t *testing.T) { AppSlugOrPort: "qux", }, - Expiry: time.Now().Add(-time.Hour).Unix(), + Expiry: time.Now().Add(-time.Hour), 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", + parseErrContains: "token expired", }, } @@ -264,51 +260,51 @@ func Test_GenerateTicket(t *testing.T) { t.Run(c.name, func(t *testing.T) { t.Parallel() - str, err := provider.GenerateTicket(c.ticket) + str, err := workspaceapps.GenerateToken(coderdtest.AppSigningKey, c.token) require.NoError(t, err) - // Tickets aren't deterministic as they have a random nonce, so we + // Tokens aren't deterministic as they have a random nonce, so we // can't compare them directly. - ticket, err := provider.ParseTicket(str) + token, err := workspaceapps.ParseToken(coderdtest.AppSigningKey, 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) + // normalize the expiry + require.WithinDuration(t, c.token.Expiry, token.Expiry, 10*time.Second) + c.token.Expiry = token.Expiry + require.Equal(t, c.token, token) } }) } } -// The ParseTicket fn is tested quite thoroughly in the GenerateTicket test. -func Test_ParseTicket(t *testing.T) { +// The ParseToken fn is tested quite thoroughly in the GenerateToken test as +// well. +func Test_ParseToken(t *testing.T) { t.Parallel() - provider := workspaceapps.New(slogtest.Make(t, nil), nil, nil, nil, nil, nil, time.Minute, coderdtest.AppSigningKey) - t.Run("InvalidJWS", func(t *testing.T) { t.Parallel() - ticket, err := provider.ParseTicket("invalid") + token, err := workspaceapps.ParseToken(coderdtest.AppSigningKey, "invalid") require.Error(t, err) require.ErrorContains(t, err, "parse JWS") - require.Equal(t, workspaceapps.Ticket{}, ticket) + require.Equal(t, workspaceapps.SignedToken{}, token) }) t.Run("VerifySignature", func(t *testing.T) { t.Parallel() - // Create a valid ticket using a different key. + // Create a valid token using a different key. otherKey, err := hex.DecodeString("62656566646561646265656664656164626565666465616462656566646561646265656664656164626565666465616462656566646561646265656664656164") require.NoError(t, err) require.NotEqual(t, coderdtest.AppSigningKey, otherKey) require.Len(t, otherKey, 64) - otherProvider := workspaceapps.New(slogtest.Make(t, nil), nil, nil, nil, nil, nil, time.Minute, otherKey) - - ticketStr, err := otherProvider.GenerateTicket(workspaceapps.Ticket{ + tokenStr, err := workspaceapps.GenerateToken(otherKey, workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/app", @@ -318,7 +314,7 @@ func Test_ParseTicket(t *testing.T) { AppSlugOrPort: "qux", }, - Expiry: time.Now().Add(time.Hour).Unix(), + Expiry: time.Now().Add(time.Hour), UserID: uuid.MustParse("b1530ba9-76f3-415e-b597-4ddd7cd466a4"), WorkspaceID: uuid.MustParse("1e6802d3-963e-45ac-9d8c-bf997016ffed"), AgentID: uuid.MustParse("9ec18681-d2c9-4c9e-9186-f136efb4edbe"), @@ -326,27 +322,27 @@ func Test_ParseTicket(t *testing.T) { }) require.NoError(t, err) - // Verify the ticket is invalid. - ticket, err := provider.ParseTicket(ticketStr) + // Verify the token is invalid. + token, err := workspaceapps.ParseToken(coderdtest.AppSigningKey, tokenStr) require.Error(t, err) require.ErrorContains(t, err, "verify JWS") - require.Equal(t, workspaceapps.Ticket{}, ticket) + require.Equal(t, workspaceapps.SignedToken{}, token) }) 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) + signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.HS512, Key: coderdtest.AppSigningKey}, 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) + token, err := workspaceapps.ParseToken(coderdtest.AppSigningKey, serialized) require.Error(t, err) require.ErrorContains(t, err, "unmarshal payload") - require.Equal(t, workspaceapps.Ticket{}, ticket) + require.Equal(t, workspaceapps.SignedToken{}, token) }) } diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index dff7033d89848..3662f4233fc7a 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -410,27 +410,27 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { require.Equal(t, proxyTestAppBody, string(body)) require.Equal(t, http.StatusOK, resp.StatusCode) - var sessionTicketCookie *http.Cookie + var appTokenCookie *http.Cookie for _, c := range resp.Cookies() { - if c.Name == codersdk.DevURLSessionTicketCookie { - sessionTicketCookie = c + if c.Name == codersdk.DevURLSignedAppTokenCookie { + appTokenCookie = c break } } - require.NotNil(t, sessionTicketCookie, "no session ticket in response") - require.Equal(t, sessionTicketCookie.Path, basePath, "incorrect path on session ticket cookie") + require.NotNil(t, appTokenCookie, "no signed app token cookie in response") + require.Equal(t, appTokenCookie.Path, basePath, "incorrect path on app token 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) + // Ensure the session token cookie is valid. + appTokenClient := codersdk.New(client.URL) + appTokenClient.HTTPClient.CheckRedirect = client.HTTPClient.CheckRedirect + appTokenClient.HTTPClient.Transport = client.HTTPClient.Transport + u, err := appTokenClient.URL.Parse(path) require.NoError(t, err) - ticketClient.HTTPClient.Jar, err = cookiejar.New(nil) + appTokenClient.HTTPClient.Jar, err = cookiejar.New(nil) require.NoError(t, err) - ticketClient.HTTPClient.Jar.SetCookies(u, []*http.Cookie{sessionTicketCookie}) + appTokenClient.HTTPClient.Jar.SetCookies(u, []*http.Cookie{appTokenCookie}) - resp, err = requestWithRetries(ctx, t, ticketClient, http.MethodGet, path, nil) + resp, err = requestWithRetries(ctx, t, appTokenClient, http.MethodGet, path, nil) require.NoError(t, err) defer resp.Body.Close() body, err = io.ReadAll(resp.Body) @@ -922,25 +922,25 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { require.Equal(t, proxyTestAppBody, string(body)) require.Equal(t, http.StatusOK, resp.StatusCode) - var sessionTicketCookie *http.Cookie + var appTokenCookie *http.Cookie for _, c := range resp.Cookies() { - if c.Name == codersdk.DevURLSessionTicketCookie { - sessionTicketCookie = c + if c.Name == codersdk.DevURLSignedAppTokenCookie { + appTokenCookie = c break } } - require.NotNil(t, sessionTicketCookie, "no session ticket in response") - require.Equal(t, sessionTicketCookie.Path, "/", "incorrect path on session ticket cookie") + require.NotNil(t, appTokenCookie, "no signed token cookie in response") + require.Equal(t, appTokenCookie.Path, "/", "incorrect path on signed token 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) + // Ensure the session token cookie is valid. + appTokenClient := codersdk.New(client.URL) + appTokenClient.HTTPClient.CheckRedirect = client.HTTPClient.CheckRedirect + appTokenClient.HTTPClient.Transport = client.HTTPClient.Transport + appTokenClient.HTTPClient.Jar, err = cookiejar.New(nil) require.NoError(t, err) - ticketClient.HTTPClient.Jar.SetCookies(u, []*http.Cookie{sessionTicketCookie}) + appTokenClient.HTTPClient.Jar.SetCookies(u, []*http.Cookie{appTokenCookie}) - resp, err = requestWithRetries(ctx, t, ticketClient, http.MethodGet, uStr, nil) + resp, err = requestWithRetries(ctx, t, appTokenClient, http.MethodGet, uStr, nil) require.NoError(t, err) defer resp.Body.Close() body, err = io.ReadAll(resp.Body) diff --git a/codersdk/client.go b/codersdk/client.go index 795896f008b59..841e653856115 100644 --- a/codersdk/client.go +++ b/codersdk/client.go @@ -41,10 +41,11 @@ const ( // token on app domains. //nolint:gosec DevURLSessionTokenCookie = "coder_devurl_session_token" - // DevURLSessionTicketCookie is the name of the cookie that stores a + // DevURLSignedAppTokenCookie 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" + //nolint:gosec + DevURLSignedAppTokenCookie = "coder_devurl_signed_app_token" // BypassRatelimitHeader is the custom header to use to bypass ratelimits. // Only owners can bypass rate limits. This is typically used for scale testing.