From ca8b74e89feee1e2fa3607b6c36fe20cd10b33b6 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 30 Mar 2023 23:29:22 +0000 Subject: [PATCH 1/4] chore: ticket provider interface --- coderd/coderd.go | 2 +- coderd/workspaceapps/auth.go | 124 ++++++++++++++++------------ coderd/workspaceapps/auth_test.go | 10 ++- coderd/workspaceapps/provider.go | 51 ++++++++++-- coderd/workspaceapps/ticket.go | 12 +-- coderd/workspaceapps/ticket_test.go | 13 +-- 6 files changed, 139 insertions(+), 73 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index c2ab9106260cc..3a617c749eff1 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -763,7 +763,7 @@ type API struct { metricsCache *metricscache.Cache workspaceAgentCache *wsconncache.Cache updateChecker *updatecheck.Checker - WorkspaceAppsProvider *workspaceapps.Provider + WorkspaceAppsProvider *workspaceapps.DBTicketProvider // 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/workspaceapps/auth.go b/coderd/workspaceapps/auth.go index e53c6f6ce66d2..4e1e8aab05749 100644 --- a/coderd/workspaceapps/auth.go +++ b/coderd/workspaceapps/auth.go @@ -29,26 +29,9 @@ const ( 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 - } - +// TODO: remove this temporary shim +func (p *DBTicketProvider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appReq Request) (*Ticket, bool) { + // TODO: this needs to be some sort of normalize function or something if appReq.WorkspaceAndAgent != "" { // workspace.agent workspaceAndAgent := strings.SplitN(appReq.WorkspaceAndAgent, ".", 2) @@ -66,23 +49,68 @@ func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appRe } } + ticket, ok := p.TicketFromRequest(r) + if ok && ticket.MatchesRequest(appReq) { + // The request has a valid ticket and it matches the request. + return ticket, true + } + + ticket, ticketStr, ok := p.CreateTicket(r.Context(), rw, r, appReq) + if !ok { + 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: ticket.Expiry, + }) + + return ticket, true +} + +func (p *DBTicketProvider) TicketFromRequest(r *http.Request) (*Ticket, bool) { // 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) { + if err == nil { // The request has a ticket, which is a valid ticket signed by - // us, and matches the app that the user was trying to access. + // us. The caller must check that it matches the request. 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. + return nil, false +} + +// 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 *DBTicketProvider) CreateTicket(ctx context.Context, rw http.ResponseWriter, r *http.Request, appReq Request) (*Ticket, 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) + err := appReq.Validate() + if err != nil { + p.writeWorkspaceApp500(rw, r, &appReq, err, "invalid app request") + return nil, "", false + } + ticket := Ticket{ Request: appReq, } @@ -101,17 +129,17 @@ func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appRe Optional: true, }) if !ok { - return nil, false + 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 + return nil, "", false } else if err != nil { p.writeWorkspaceApp500(rw, r, &appReq, err, "get app details from database") - return nil, false + return nil, "", false } ticket.UserID = dbReq.User.ID ticket.WorkspaceID = dbReq.Workspace.ID @@ -124,19 +152,20 @@ func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appRe // Verify the user has access to the app. authed, ok := p.verifyAuthz(rw, r, authz, dbReq) if !ok { - return nil, false + 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 + 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 @@ -156,52 +185,41 @@ func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appRe // Return an error. httpapi.ResourceNotFound(rw) } - return nil, false + 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 + 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 + 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 + return nil, "", false } // Sign the ticket. - ticketExpiry := time.Now().Add(TicketExpiry) - ticket.Expiry = ticketExpiry.Unix() + ticket.Expiry = time.Now().Add(TicketExpiry) ticketStr, err := p.GenerateTicket(ticket) if err != nil { p.writeWorkspaceApp500(rw, r, &appReq, err, "generate ticket") - return nil, false + 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 + return &ticket, ticketStr, true } -func (p *Provider) authorizeRequest(ctx context.Context, roles *httpmw.Authorization, dbReq *databaseRequest) (bool, error) { +func (p *DBTicketProvider) authorizeRequest(ctx context.Context, roles *httpmw.Authorization, dbReq *databaseRequest) (bool, error) { accessMethod := dbReq.AccessMethod if accessMethod == "" { accessMethod = AccessMethodPath @@ -293,7 +311,7 @@ func (p *Provider) authorizeRequest(ctx context.Context, roles *httpmw.Authoriza // 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) { +func (p *DBTicketProvider) 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)) @@ -312,7 +330,7 @@ func (p *Provider) verifyAuthz(rw http.ResponseWriter, r *http.Request, authz *h // 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) { +func (p *DBTicketProvider) writeWorkspaceApp404(rw http.ResponseWriter, r *http.Request, appReq *Request, msg string) { if appReq != nil { slog.Helper() p.Logger.Debug(r.Context(), @@ -336,7 +354,7 @@ func (p *Provider) writeWorkspaceApp404(rw http.ResponseWriter, r *http.Request, // 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) { +func (p *DBTicketProvider) writeWorkspaceApp500(rw http.ResponseWriter, r *http.Request, appReq *Request, err error, msg string) { slog.Helper() ctx := r.Context() if appReq != nil { @@ -364,7 +382,7 @@ func (p *Provider) writeWorkspaceApp500(rw http.ResponseWriter, r *http.Request, // 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) { +func (p *DBTicketProvider) writeWorkspaceAppOffline(rw http.ResponseWriter, r *http.Request, appReq *Request, msg string) { if appReq != nil { slog.Helper() p.Logger.Debug(r.Context(), diff --git a/coderd/workspaceapps/auth_test.go b/coderd/workspaceapps/auth_test.go index f1d06ac981938..0ae9961ba8f9a 100644 --- a/coderd/workspaceapps/auth_test.go +++ b/coderd/workspaceapps/auth_test.go @@ -255,7 +255,7 @@ func Test_ResolveRequest(t *testing.T) { AppURL: appURL, }, ticket) require.NotZero(t, ticket.Expiry) - require.InDelta(t, time.Now().Add(workspaceapps.TicketExpiry).Unix(), ticket.Expiry, time.Minute.Seconds()) + require.WithinDuration(t, time.Now().Add(workspaceapps.TicketExpiry), ticket.Expiry, time.Minute) // Check that the ticket was set in the response and is valid. require.Len(t, w.Cookies(), 1) @@ -265,6 +265,9 @@ func Test_ResolveRequest(t *testing.T) { parsedTicket, err := api.WorkspaceAppsProvider.ParseTicket(cookie.Value) require.NoError(t, err) + // normalize expiry + require.WithinDuration(t, ticket.Expiry, parsedTicket.Expiry, 2*time.Second) + parsedTicket.Expiry = ticket.Expiry require.Equal(t, ticket, &parsedTicket) // Try resolving the request with the ticket only. @@ -274,6 +277,9 @@ func Test_ResolveRequest(t *testing.T) { secondTicket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) require.True(t, ok) + // normalize expiry + require.WithinDuration(t, ticket.Expiry, secondTicket.Expiry, 2*time.Second) + secondTicket.Expiry = ticket.Expiry require.Equal(t, ticket, secondTicket) } }) @@ -470,7 +476,7 @@ 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, diff --git a/coderd/workspaceapps/provider.go b/coderd/workspaceapps/provider.go index 23e1fb7e3c736..a17416a08eb7a 100644 --- a/coderd/workspaceapps/provider.go +++ b/coderd/workspaceapps/provider.go @@ -1,6 +1,8 @@ package workspaceapps import ( + "context" + "net/http" "net/url" "time" @@ -11,10 +13,45 @@ import ( "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 { +/* +POST /api/v2/moons/app-auth-ticket + +{ + "session_token": "xxxx", + "request": { ... } +} + +type moonRes struct { + Ticket *Ticket + TicketStr string +} +*/ + +// TicketProvider provides workspace app tickets. +// +// write a funny comment that says a ridiculous amount of fees will be incurred: +// +// Please keep in mind that all transactions incur a service fee, handling fee, +// order processing fee, delivery fee, +type TicketProvider interface { + // TicketFromRequest returns a ticket from the request. If the request does + // not contain a ticket or the ticket is invalid (expired, invalid + // signature, etc.), it returns false. + TicketFromRequest(r *http.Request) (*Ticket, bool) + // CreateTicket creates a ticket for the given app request. It uses the + // long-lived session token in the HTTP request to authenticate the user. + // The ticket 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. + CreateTicket(ctx context.Context, rw http.ResponseWriter, r *http.Request, appReq Request) (*Ticket, string, bool) +} + +// DBTicketProvider provides authentication and authorization for workspace apps +// by querying the database if the request is missing a valid ticket. +type DBTicketProvider struct { Logger slog.Logger AccessURL *url.URL @@ -26,7 +63,9 @@ type Provider struct { TicketSigningKey []byte } -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 { +var _ TicketProvider = &DBTicketProvider{} + +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) *DBTicketProvider { if len(ticketSigningKey) != 64 { panic("ticket signing key must be 64 bytes") } @@ -35,7 +74,7 @@ func New(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database workspaceAgentInactiveTimeout = 1 * time.Minute } - return &Provider{ + return &DBTicketProvider{ Logger: log, AccessURL: accessURL, Authorizer: authz, diff --git a/coderd/workspaceapps/ticket.go b/coderd/workspaceapps/ticket.go index 11fb6465b190a..f97ee83a1548a 100644 --- a/coderd/workspaceapps/ticket.go +++ b/coderd/workspaceapps/ticket.go @@ -21,7 +21,7 @@ type Ticket struct { Request `json:"request"` // Trusted resolved details. - Expiry int64 `json:"expiry"` // set by GenerateTicket if unset + Expiry time.Time `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"` @@ -37,9 +37,9 @@ 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() +func (p *DBTicketProvider) GenerateTicket(payload Ticket) (string, error) { + if payload.Expiry.IsZero() { + payload.Expiry = time.Now().Add(TicketExpiry) } payloadBytes, err := json.Marshal(payload) if err != nil { @@ -69,7 +69,7 @@ func (p *Provider) GenerateTicket(payload Ticket) (string, error) { return serialized, nil } -func (p *Provider) ParseTicket(ticketStr string) (Ticket, error) { +func (p *DBTicketProvider) ParseTicket(ticketStr string) (Ticket, error) { object, err := jose.ParseSigned(ticketStr) if err != nil { return Ticket{}, xerrors.Errorf("parse JWS: %w", err) @@ -91,7 +91,7 @@ func (p *Provider) ParseTicket(ticketStr string) (Ticket, error) { if err != nil { return Ticket{}, xerrors.Errorf("unmarshal payload: %w", err) } - if ticket.Expiry < time.Now().Unix() { + if ticket.Expiry.Before(time.Now()) { return Ticket{}, xerrors.New("ticket expired") } diff --git a/coderd/workspaceapps/ticket_test.go b/coderd/workspaceapps/ticket_test.go index c7fd79cff3104..a5bd01279a7d2 100644 --- a/coderd/workspaceapps/ticket_test.go +++ b/coderd/workspaceapps/ticket_test.go @@ -178,7 +178,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"), @@ -189,10 +189,10 @@ func Test_GenerateTicket(t *testing.T) { ticket, err := provider.ParseTicket(ticketStr) require.NoError(t, err) - require.InDelta(t, time.Now().Unix(), ticket.Expiry, time.Minute.Seconds()) + require.WithinDuration(t, time.Now().Add(time.Minute), ticket.Expiry, 15*time.Second) }) - future := time.Now().Add(time.Hour).Unix() + future := time.Now().Add(time.Hour) cases := []struct { name string ticket workspaceapps.Ticket @@ -248,7 +248,7 @@ 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"), @@ -276,6 +276,9 @@ func Test_GenerateTicket(t *testing.T) { require.ErrorContains(t, err, c.parseErrContains) } else { require.NoError(t, err) + // normalize the expiry + require.WithinDuration(t, c.ticket.Expiry, ticket.Expiry, 10*time.Second) + c.ticket.Expiry = ticket.Expiry require.Equal(t, c.ticket, ticket) } }) @@ -318,7 +321,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"), From 307e572c69c5ebeffea06fbd08e0844824b0ea3f Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Mon, 3 Apr 2023 11:17:16 +0000 Subject: [PATCH 2/4] continue tidying/splitting up workspaceapps code --- coderd/coderd.go | 4 +- coderd/workspaceagents.go | 2 +- coderd/workspaceapps.go | 4 +- coderd/workspaceapps/{auth.go => db.go} | 203 ++++-------------- .../{auth_test.go => db_test.go} | 36 ++-- coderd/workspaceapps/errors.go | 85 ++++++++ coderd/workspaceapps/provider.go | 95 ++++---- coderd/workspaceapps/request.go | 36 +++- coderd/workspaceapps/request_test.go | 55 +---- coderd/workspaceapps/ticket.go | 17 +- coderd/workspaceapps/ticket_test.go | 26 +-- 11 files changed, 247 insertions(+), 316 deletions(-) rename coderd/workspaceapps/{auth.go => db.go} (57%) rename coderd/workspaceapps/{auth_test.go => db_test.go} (91%) create mode 100644 coderd/workspaceapps/errors.go diff --git a/coderd/coderd.go b/coderd/coderd.go index 3a617c749eff1..51b4057a5201a 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -280,7 +280,7 @@ func New(options *Options) *API { Authorizer: options.Authorizer, Logger: options.Logger, }, - WorkspaceAppsProvider: workspaceapps.New( + WorkspaceAppsProvider: workspaceapps.NewDBTicketProvider( options.Logger.Named("workspaceapps"), options.AccessURL, options.Authorizer, @@ -763,7 +763,7 @@ type API struct { metricsCache *metricscache.Cache workspaceAgentCache *wsconncache.Cache updateChecker *updatecheck.Checker - WorkspaceAppsProvider *workspaceapps.DBTicketProvider + WorkspaceAppsProvider workspaceapps.TicketProvider // 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/workspaceagents.go b/coderd/workspaceagents.go index 74e2818a6bbe1..1267bbbbbb66f 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -550,7 +550,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{ + ticket, 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"), diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index cf3dcfb0c1b86..0601e610aa4f9 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{ + ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: basePath, UsernameOrID: chi.URLParam(r, "user"), @@ -247,7 +247,7 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht return } - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, workspaceapps.Request{ + ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodSubdomain, BasePath: "/", UsernameOrID: app.Username, diff --git a/coderd/workspaceapps/auth.go b/coderd/workspaceapps/db.go similarity index 57% rename from coderd/workspaceapps/auth.go rename to coderd/workspaceapps/db.go index 4e1e8aab05749..0e2606444d3cd 100644 --- a/coderd/workspaceapps/auth.go +++ b/coderd/workspaceapps/db.go @@ -5,81 +5,66 @@ import ( "database/sql" "fmt" "net/http" - "strings" + "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" - "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" -) +// DBTicketProvider provides authentication and authorization for workspace apps +// by querying the database if the request is missing a valid ticket. +type DBTicketProvider struct { + Logger slog.Logger + + AccessURL *url.URL + Authorizer rbac.Authorizer + Database database.Store + DeploymentValues *codersdk.DeploymentValues + OAuth2Configs *httpmw.OAuth2Configs + WorkspaceAgentInactiveTimeout time.Duration + TicketSigningKey []byte +} -// TODO: remove this temporary shim -func (p *DBTicketProvider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appReq Request) (*Ticket, bool) { - // TODO: this needs to be some sort of normalize function or something - 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] - } +var _ TicketProvider = &DBTicketProvider{} - // Sanity check. - err := appReq.Validate() - if err != nil { - p.writeWorkspaceApp500(rw, r, &appReq, err, "invalid app request") - return nil, false - } +func NewDBTicketProvider(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database.Store, cfg *codersdk.DeploymentValues, oauth2Cfgs *httpmw.OAuth2Configs, workspaceAgentInactiveTimeout time.Duration, ticketSigningKey []byte) TicketProvider { + if len(ticketSigningKey) != 64 { + panic("ticket signing key must be 64 bytes") } - ticket, ok := p.TicketFromRequest(r) - if ok && ticket.MatchesRequest(appReq) { - // The request has a valid ticket and it matches the request. - return ticket, true + if workspaceAgentInactiveTimeout == 0 { + workspaceAgentInactiveTimeout = 1 * time.Minute } - ticket, ticketStr, ok := p.CreateTicket(r.Context(), rw, r, appReq) - if !ok { - return nil, false + return &DBTicketProvider{ + Logger: log, + AccessURL: accessURL, + Authorizer: authz, + Database: db, + DeploymentValues: cfg, + OAuth2Configs: oauth2Cfgs, + WorkspaceAgentInactiveTimeout: workspaceAgentInactiveTimeout, + TicketSigningKey: ticketSigningKey, } - - // 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: ticket.Expiry, - }) - - return ticket, true } func (p *DBTicketProvider) TicketFromRequest(r *http.Request) (*Ticket, bool) { // Get the existing ticket from the request. ticketCookie, err := r.Cookie(codersdk.DevURLSessionTicketCookie) if err == nil { - ticket, err := p.ParseTicket(ticketCookie.Value) + ticket, err := ParseTicket(p.TicketSigningKey, ticketCookie.Value) if err == nil { - err := ticket.Request.Validate() + req := ticket.Request.Normalize() + err := req.Validate() if err == nil { // The request has a ticket, which is a valid ticket signed by // us. The caller must check that it matches the request. @@ -105,9 +90,11 @@ func (p *DBTicketProvider) CreateTicket(ctx context.Context, rw http.ResponseWri // // 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 { - p.writeWorkspaceApp500(rw, r, &appReq, err, "invalid app request") + WriteWorkspaceApp500(p.Logger, p.AccessURL, rw, r, &appReq, err, "invalid app request") return nil, "", false } @@ -135,10 +122,10 @@ func (p *DBTicketProvider) CreateTicket(ctx context.Context, rw http.ResponseWri // 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()) + WriteWorkspaceApp404(p.Logger, p.AccessURL, rw, r, &appReq, err.Error()) return nil, "", false } else if err != nil { - p.writeWorkspaceApp500(rw, r, &appReq, err, "get app details from database") + WriteWorkspaceApp500(p.Logger, p.AccessURL, rw, r, &appReq, err, "get app details from database") return nil, "", false } ticket.UserID = dbReq.User.ID @@ -150,14 +137,15 @@ func (p *DBTicketProvider) CreateTicket(ctx context.Context, rw http.ResponseWri // is not running. // Verify the user has access to the app. - authed, ok := p.verifyAuthz(rw, r, authz, dbReq) - if !ok { + 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. - p.writeWorkspaceApp404(rw, r, &appReq, "insufficient permissions") + WriteWorkspaceApp404(p.Logger, p.AccessURL, rw, r, &appReq, "insufficient permissions") return nil, "", false } @@ -191,28 +179,28 @@ func (p *DBTicketProvider) CreateTicket(ctx context.Context, rw http.ResponseWri // 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)) + 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 { - p.writeWorkspaceAppOffline(rw, r, &appReq, fmt.Sprintf("App health is %q, not %q", 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 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") + WriteWorkspaceApp500(p.Logger, p.AccessURL, rw, r, &appReq, nil, "fresh ticket does not match request") return nil, "", false } // Sign the ticket. ticket.Expiry = time.Now().Add(TicketExpiry) - ticketStr, err := p.GenerateTicket(ticket) + ticketStr, err := GenerateTicket(p.TicketSigningKey, ticket) if err != nil { - p.writeWorkspaceApp500(rw, r, &appReq, err, "generate ticket") + WriteWorkspaceApp500(p.Logger, p.AccessURL, rw, r, &appReq, err, "generate ticket") return nil, "", false } @@ -306,100 +294,3 @@ func (p *DBTicketProvider) authorizeRequest(ctx context.Context, roles *httpmw.A // 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 *DBTicketProvider) 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 *DBTicketProvider) 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 *DBTicketProvider) 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 *DBTicketProvider) 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/auth_test.go b/coderd/workspaceapps/db_test.go similarity index 91% rename from coderd/workspaceapps/auth_test.go rename to coderd/workspaceapps/db_test.go index 0ae9961ba8f9a..1b4fcd519cbb5 100644 --- a/coderd/workspaceapps/auth_test.go +++ b/coderd/workspaceapps/db_test.go @@ -236,7 +236,7 @@ func Test_ResolveRequest(t *testing.T) { r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) // Try resolving the request without a ticket. - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + ticket, 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) @@ -263,7 +263,7 @@ func Test_ResolveRequest(t *testing.T) { require.Equal(t, codersdk.DevURLSessionTicketCookie, cookie.Name) require.Equal(t, req.BasePath, cookie.Path) - parsedTicket, err := api.WorkspaceAppsProvider.ParseTicket(cookie.Value) + parsedTicket, err := workspaceapps.ParseTicket(api.AppSigningKey, cookie.Value) require.NoError(t, err) // normalize expiry require.WithinDuration(t, ticket.Expiry, parsedTicket.Expiry, 2*time.Second) @@ -275,7 +275,7 @@ func Test_ResolveRequest(t *testing.T) { r = httptest.NewRequest("GET", "/app", nil) r.AddCookie(cookie) - secondTicket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + secondTicket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.True(t, ok) // normalize expiry require.WithinDuration(t, ticket.Expiry, secondTicket.Expiry, 2*time.Second) @@ -304,7 +304,7 @@ 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) + ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) w := rw.Result() _ = w.Body.Close() if app == appNameOwner { @@ -336,7 +336,7 @@ 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) + ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) w := rw.Result() if app != appNamePublic { require.False(t, ok) @@ -367,7 +367,7 @@ func Test_ResolveRequest(t *testing.T) { } rw := httptest.NewRecorder() r := httptest.NewRequest("GET", "/app", nil) - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.False(t, ok) require.Nil(t, ticket) }) @@ -441,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) + ticket, 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) @@ -482,7 +482,7 @@ func Test_ResolveRequest(t *testing.T) { AgentID: agentID, AppURL: appURL, } - badTicketStr, err := api.WorkspaceAppsProvider.GenerateTicket(badTicket) + badTicketStr, err := workspaceapps.GenerateTicket(api.AppSigningKey, badTicket) require.NoError(t, err) req := workspaceapps.Request{ @@ -505,7 +505,7 @@ func Test_ResolveRequest(t *testing.T) { // Even though the ticket is invalid, we should still perform request // resolution. - ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + ticket, 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) @@ -518,7 +518,7 @@ func Test_ResolveRequest(t *testing.T) { 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) + parsedTicket, err := workspaceapps.ParseTicket(api.AppSigningKey, cookies[0].Value) require.NoError(t, err) require.Equal(t, appNameOwner, parsedTicket.AppSlugOrPort) }) @@ -539,7 +539,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) + ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.False(t, ok) require.Nil(t, ticket) }) @@ -560,7 +560,7 @@ 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) + ticket, 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) @@ -579,7 +579,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) + ticket, 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) @@ -606,7 +606,7 @@ 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) + ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.False(t, ok) require.Nil(t, ticket) }) @@ -626,7 +626,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) + ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.False(t, ok) require.Nil(t, ticket) }) @@ -647,7 +647,7 @@ 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) + ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.False(t, ok) require.Nil(t, ticket) @@ -687,7 +687,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) + ticket, 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) @@ -741,7 +741,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) + ticket, 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) 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 a17416a08eb7a..36a405230aa65 100644 --- a/coderd/workspaceapps/provider.go +++ b/coderd/workspaceapps/provider.go @@ -7,32 +7,58 @@ import ( "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" ) -/* -POST /api/v2/moons/app-auth-ticket +const ( + // TODO(@deansheather): configurable expiry + TicketExpiry = time.Minute -{ - "session_token": "xxxx", - "request": { ... } -} + // 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 calls TicketProvider to use an existing ticket in the request +// or issue a new one. If it returns a ticket, it sets the cookie and returns +// it. +func ResolveRequest(log slog.Logger, accessURL *url.URL, p TicketProvider, rw http.ResponseWriter, r *http.Request, appReq Request) (*Ticket, bool) { + appReq = appReq.Normalize() + err := appReq.Validate() + if err != nil { + WriteWorkspaceApp500(log, accessURL, rw, r, &appReq, err, "invalid app request") + return nil, false + } + + ticket, ok := p.TicketFromRequest(r) + if ok && ticket.MatchesRequest(appReq) { + // The request has a valid ticket and it matches the request. + return ticket, true + } -type moonRes struct { - Ticket *Ticket - TicketStr string + ticket, ticketStr, ok := p.CreateTicket(r.Context(), rw, r, appReq) + if !ok { + 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: ticket.Expiry, + }) + + return ticket, true } -*/ // TicketProvider provides workspace app tickets. // -// write a funny comment that says a ridiculous amount of fees will be incurred: -// // Please keep in mind that all transactions incur a service fee, handling fee, -// order processing fee, delivery fee, +// order processing fee, delivery fee, insurance charge, convenience fee, +// inconvenience fee, seat selection fee, and levity levy. :^) type TicketProvider interface { // TicketFromRequest returns a ticket from the request. If the request does // not contain a ticket or the ticket is invalid (expired, invalid @@ -48,40 +74,3 @@ type TicketProvider interface { // in this case. CreateTicket(ctx context.Context, rw http.ResponseWriter, r *http.Request, appReq Request) (*Ticket, string, bool) } - -// DBTicketProvider provides authentication and authorization for workspace apps -// by querying the database if the request is missing a valid ticket. -type DBTicketProvider struct { - Logger slog.Logger - - AccessURL *url.URL - Authorizer rbac.Authorizer - Database database.Store - DeploymentValues *codersdk.DeploymentValues - OAuth2Configs *httpmw.OAuth2Configs - WorkspaceAgentInactiveTimeout time.Duration - TicketSigningKey []byte -} - -var _ TicketProvider = &DBTicketProvider{} - -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) *DBTicketProvider { - if len(ticketSigningKey) != 64 { - panic("ticket signing key must be 64 bytes") - } - - if workspaceAgentInactiveTimeout == 0 { - workspaceAgentInactiveTimeout = 1 * time.Minute - } - - return &DBTicketProvider{ - Logger: log, - AccessURL: accessURL, - Authorizer: authz, - Database: db, - DeploymentValues: cfg, - OAuth2Configs: oauth2Cfgs, - WorkspaceAgentInactiveTimeout: workspaceAgentInactiveTimeout, - TicketSigningKey: ticketSigningKey, - } -} diff --git a/coderd/workspaceapps/request.go b/coderd/workspaceapps/request.go index 4cc309848ad4f..022aad3a96813 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") } @@ -84,16 +107,7 @@ func (r Request) Validate() error { // 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 f97ee83a1548a..dc23f73b76229 100644 --- a/coderd/workspaceapps/ticket.go +++ b/coderd/workspaceapps/ticket.go @@ -14,8 +14,6 @@ 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. Request `json:"request"` @@ -28,6 +26,8 @@ type Ticket struct { AppURL string `json:"app_url"` } +// MatchesRequest returns true if the ticket matches the request. Any ticket +// that does not match the request should be considered invalid. func (t Ticket) MatchesRequest(req Request) bool { return t.AccessMethod == req.AccessMethod && t.BasePath == req.BasePath && @@ -37,7 +37,10 @@ func (t Ticket) MatchesRequest(req Request) bool { t.AppSlugOrPort == req.AppSlugOrPort } -func (p *DBTicketProvider) GenerateTicket(payload Ticket) (string, error) { +// GenerateTicket generates a workspace app ticket with the given key and +// payload. If the ticket doesn't have an expiry, it will be set to the current +// time plus the default expiry. +func GenerateTicket(key []byte, payload Ticket) (string, error) { if payload.Expiry.IsZero() { payload.Expiry = time.Now().Add(TicketExpiry) } @@ -50,7 +53,7 @@ func (p *DBTicketProvider) GenerateTicket(payload Ticket) (string, error) { // future. signer, err := jose.NewSigner(jose.SigningKey{ Algorithm: ticketSigningAlgorithm, - Key: p.TicketSigningKey, + Key: key, }, nil) if err != nil { return "", xerrors.Errorf("create signer: %w", err) @@ -69,7 +72,9 @@ func (p *DBTicketProvider) GenerateTicket(payload Ticket) (string, error) { return serialized, nil } -func (p *DBTicketProvider) ParseTicket(ticketStr string) (Ticket, error) { +// ParseTicket parses a workspace app ticket with the given key and returns the +// payload. If the ticket is invalid, an error is returned. +func ParseTicket(key []byte, ticketStr string) (Ticket, error) { object, err := jose.ParseSigned(ticketStr) if err != nil { return Ticket{}, xerrors.Errorf("parse JWS: %w", err) @@ -81,7 +86,7 @@ func (p *DBTicketProvider) ParseTicket(ticketStr string) (Ticket, error) { 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) + output, err := object.Verify(key) if err != nil { return Ticket{}, xerrors.Errorf("verify JWS: %w", err) } diff --git a/coderd/workspaceapps/ticket_test.go b/coderd/workspaceapps/ticket_test.go index a5bd01279a7d2..9112d04f179c8 100644 --- a/coderd/workspaceapps/ticket_test.go +++ b/coderd/workspaceapps/ticket_test.go @@ -9,8 +9,6 @@ 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" ) @@ -163,12 +161,10 @@ func Test_TicketMatchesRequest(t *testing.T) { func Test_GenerateTicket(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{ + ticketStr, err := workspaceapps.GenerateTicket(coderdtest.AppSigningKey, workspaceapps.Ticket{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/app", @@ -186,7 +182,7 @@ func Test_GenerateTicket(t *testing.T) { }) require.NoError(t, err) - ticket, err := provider.ParseTicket(ticketStr) + ticket, err := workspaceapps.ParseTicket(coderdtest.AppSigningKey, ticketStr) require.NoError(t, err) require.WithinDuration(t, time.Now().Add(time.Minute), ticket.Expiry, 15*time.Second) @@ -264,13 +260,13 @@ 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.GenerateTicket(coderdtest.AppSigningKey, 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) + ticket, err := workspaceapps.ParseTicket(coderdtest.AppSigningKey, str) if c.parseErrContains != "" { require.Error(t, err) require.ErrorContains(t, err, c.parseErrContains) @@ -289,12 +285,10 @@ func Test_GenerateTicket(t *testing.T) { func Test_ParseTicket(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") + ticket, err := workspaceapps.ParseTicket(coderdtest.AppSigningKey, "invalid") require.Error(t, err) require.ErrorContains(t, err, "parse JWS") require.Equal(t, workspaceapps.Ticket{}, ticket) @@ -309,9 +303,7 @@ func Test_ParseTicket(t *testing.T) { 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{ + ticketStr, err := workspaceapps.GenerateTicket(otherKey, workspaceapps.Ticket{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/app", @@ -330,7 +322,7 @@ func Test_ParseTicket(t *testing.T) { require.NoError(t, err) // Verify the ticket is invalid. - ticket, err := provider.ParseTicket(ticketStr) + ticket, err := workspaceapps.ParseTicket(coderdtest.AppSigningKey, ticketStr) require.Error(t, err) require.ErrorContains(t, err, "verify JWS") require.Equal(t, workspaceapps.Ticket{}, ticket) @@ -340,14 +332,14 @@ func Test_ParseTicket(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) + ticket, err := workspaceapps.ParseTicket(coderdtest.AppSigningKey, serialized) require.Error(t, err) require.ErrorContains(t, err, "unmarshal payload") require.Equal(t, workspaceapps.Ticket{}, ticket) From 7c3428cd84593abf6aadc9121f9bdc97a8d167d8 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Mon, 3 Apr 2023 23:51:33 +0000 Subject: [PATCH 3/4] chore: rename app tickets to signed app tokens --- coderd/coderd.go | 10 +- coderd/coderdtest/coderdtest.go | 2 +- coderd/httpapi/cookie.go | 2 +- coderd/workspaceagents.go | 4 +- coderd/workspaceapps.go | 18 ++-- coderd/workspaceapps/db.go | 74 +++++++------- coderd/workspaceapps/db_test.go | 146 ++++++++++++++-------------- coderd/workspaceapps/provider.go | 58 +++++------ coderd/workspaceapps/request.go | 4 +- coderd/workspaceapps/ticket.go | 58 ++++++----- coderd/workspaceapps/ticket_test.go | 79 +++++++-------- coderd/workspaceapps_test.go | 50 +++++----- codersdk/client.go | 5 +- 13 files changed, 251 insertions(+), 259 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 51b4057a5201a..26b7014ccbb5f 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -121,8 +121,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 // APIRateLimit is the minutely throughput rate limit per user or ip. @@ -280,7 +280,7 @@ func New(options *Options) *API { Authorizer: options.Authorizer, Logger: options.Logger, }, - WorkspaceAppsProvider: workspaceapps.NewDBTicketProvider( + WorkspaceAppsProvider: workspaceapps.NewDBTokenProvider( options.Logger.Named("workspaceapps"), options.AccessURL, options.Authorizer, @@ -620,7 +620,7 @@ func New(options *Options) *API { r.Post("/report-lifecycle", api.workspaceAgentReportLifecycle) }) // 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( @@ -763,7 +763,7 @@ type API struct { metricsCache *metricscache.Cache workspaceAgentCache *wsconncache.Cache updateChecker *updatecheck.Checker - WorkspaceAppsProvider workspaceapps.TicketProvider + 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 a6e8cf4a2f85f..deee328d67b7c 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -79,7 +79,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 1267bbbbbb66f..96833a9cab89b 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -550,7 +550,7 @@ func (api *API) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) { api.WebsocketWaitMutex.Unlock() defer api.WebsocketWaitGroup.Done() - ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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"), @@ -594,7 +594,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 0601e610aa4f9..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 := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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 := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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/db.go b/coderd/workspaceapps/db.go index 0e2606444d3cd..84234462e5696 100644 --- a/coderd/workspaceapps/db.go +++ b/coderd/workspaceapps/db.go @@ -20,9 +20,9 @@ import ( "github.com/coder/coder/codersdk" ) -// DBTicketProvider provides authentication and authorization for workspace apps -// by querying the database if the request is missing a valid ticket. -type DBTicketProvider struct { +// 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 *url.URL @@ -31,21 +31,21 @@ type DBTicketProvider struct { DeploymentValues *codersdk.DeploymentValues OAuth2Configs *httpmw.OAuth2Configs WorkspaceAgentInactiveTimeout time.Duration - TicketSigningKey []byte + TokenSigningKey []byte } -var _ TicketProvider = &DBTicketProvider{} +var _ SignedTokenProvider = &DBTokenProvider{} -func NewDBTicketProvider(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database.Store, cfg *codersdk.DeploymentValues, oauth2Cfgs *httpmw.OAuth2Configs, workspaceAgentInactiveTimeout time.Duration, ticketSigningKey []byte) TicketProvider { - if len(ticketSigningKey) != 64 { - panic("ticket signing key must be 64 bytes") +func NewDBTokenProvider(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database.Store, cfg *codersdk.DeploymentValues, oauth2Cfgs *httpmw.OAuth2Configs, workspaceAgentInactiveTimeout time.Duration, tokenSigningKey []byte) SignedTokenProvider { + if len(tokenSigningKey) != 64 { + panic("token signing key must be 64 bytes") } if workspaceAgentInactiveTimeout == 0 { workspaceAgentInactiveTimeout = 1 * time.Minute } - return &DBTicketProvider{ + return &DBTokenProvider{ Logger: log, AccessURL: accessURL, Authorizer: authz, @@ -53,22 +53,23 @@ func NewDBTicketProvider(log slog.Logger, accessURL *url.URL, authz rbac.Authori DeploymentValues: cfg, OAuth2Configs: oauth2Cfgs, WorkspaceAgentInactiveTimeout: workspaceAgentInactiveTimeout, - TicketSigningKey: ticketSigningKey, + TokenSigningKey: tokenSigningKey, } } -func (p *DBTicketProvider) TicketFromRequest(r *http.Request) (*Ticket, bool) { - // Get the existing ticket from the request. - ticketCookie, err := r.Cookie(codersdk.DevURLSessionTicketCookie) +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 { - ticket, err := ParseTicket(p.TicketSigningKey, ticketCookie.Value) + token, err := ParseToken(p.TokenSigningKey, tokenCookie.Value) if err == nil { - req := ticket.Request.Normalize() + req := token.Request.Normalize() err := req.Validate() if err == nil { - // The request has a ticket, which is a valid ticket signed by - // us. The caller must check that it matches the request. - return &ticket, true + // 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 } } } @@ -77,13 +78,8 @@ func (p *DBTicketProvider) TicketFromRequest(r *http.Request) (*Ticket, bool) { } // 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 *DBTicketProvider) CreateTicket(ctx context.Context, rw http.ResponseWriter, r *http.Request, appReq Request) (*Ticket, string, bool) { +// 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 @@ -98,7 +94,7 @@ func (p *DBTicketProvider) CreateTicket(ctx context.Context, rw http.ResponseWri return nil, "", false } - ticket := Ticket{ + token := SignedToken{ Request: appReq, } @@ -128,10 +124,10 @@ func (p *DBTicketProvider) CreateTicket(ctx context.Context, rw http.ResponseWri WriteWorkspaceApp500(p.Logger, p.AccessURL, 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 + 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. @@ -189,25 +185,25 @@ func (p *DBTicketProvider) CreateTicket(ctx context.Context, rw http.ResponseWri return nil, "", false } - // As a sanity check, ensure the ticket we just made is valid for this + // As a sanity check, ensure the token we just made is valid for this // request. - if !ticket.MatchesRequest(appReq) { - WriteWorkspaceApp500(p.Logger, p.AccessURL, rw, r, &appReq, nil, "fresh ticket does not match 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 ticket. - ticket.Expiry = time.Now().Add(TicketExpiry) - ticketStr, err := GenerateTicket(p.TicketSigningKey, ticket) + // 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 ticket") + WriteWorkspaceApp500(p.Logger, p.AccessURL, rw, r, &appReq, err, "generate token") return nil, "", false } - return &ticket, ticketStr, true + return &token, tokenStr, true } -func (p *DBTicketProvider) authorizeRequest(ctx context.Context, roles *httpmw.Authorization, dbReq *databaseRequest) (bool, error) { +func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *httpmw.Authorization, dbReq *databaseRequest) (bool, error) { accessMethod := dbReq.AccessMethod if accessMethod == "" { accessMethod = AccessMethodPath diff --git a/coderd/workspaceapps/db_test.go b/coderd/workspaceapps/db_test.go index 1b4fcd519cbb5..f566c5c838e0b 100644 --- a/coderd/workspaceapps/db_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 := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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,41 +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.WithinDuration(t, time.Now().Add(workspaceapps.TicketExpiry), ticket.Expiry, time.Minute) + }, 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 := workspaceapps.ParseTicket(api.AppSigningKey, cookie.Value) + parsedToken, err := workspaceapps.ParseToken(api.AppSigningKey, cookie.Value) require.NoError(t, err) // normalize expiry - require.WithinDuration(t, ticket.Expiry, parsedTicket.Expiry, 2*time.Second) - parsedTicket.Expiry = ticket.Expiry - require.Equal(t, ticket, &parsedTicket) + 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 := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) + secondToken, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, rw, r, req) require.True(t, ok) // normalize expiry - require.WithinDuration(t, ticket.Expiry, secondTicket.Expiry, 2*time.Second) - secondTicket.Expiry = ticket.Expiry - require.Equal(t, ticket, secondTicket) + require.WithinDuration(t, token.Expiry, secondToken.Expiry, 2*time.Second) + secondToken.Expiry = token.Expiry + require.Equal(t, token, secondToken) } }) } @@ -304,18 +304,18 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, secondUserClient.SessionToken()) - ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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) } }) @@ -336,11 +336,11 @@ func Test_ResolveRequest(t *testing.T) { t.Log("app", app) rw := httptest.NewRecorder() r := httptest.NewRequest("GET", "/app", nil) - ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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 { @@ -350,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) } @@ -367,9 +367,9 @@ func Test_ResolveRequest(t *testing.T) { } rw := httptest.NewRecorder() r := httptest.NewRequest("GET", "/app", nil) - ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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) { @@ -441,7 +441,7 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) - ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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) @@ -450,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", @@ -482,7 +482,7 @@ func Test_ResolveRequest(t *testing.T) { AgentID: agentID, AppURL: appURL, } - badTicketStr, err := workspaceapps.GenerateTicket(api.AppSigningKey, badTicket) + badTokenStr, err := workspaceapps.GenerateToken(api.AppSigningKey, badToken) require.NoError(t, err) req := workspaceapps.Request{ @@ -499,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 := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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 := workspaceapps.ParseTicket(api.AppSigningKey, 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) { @@ -539,9 +539,9 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) - ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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) { @@ -560,10 +560,10 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/", nil) r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) - ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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) { @@ -579,15 +579,15 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) - ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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) { @@ -606,9 +606,9 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, secondUserClient.SessionToken()) - ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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) { @@ -626,9 +626,9 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) - ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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) { @@ -647,9 +647,9 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/some-path", nil) r.Host = "app.com" - ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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() @@ -687,9 +687,9 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) - ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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() @@ -741,9 +741,9 @@ func Test_ResolveRequest(t *testing.T) { r := httptest.NewRequest("GET", "/app", nil) r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) - ticket, ok := workspaceapps.ResolveRequest(api.Logger, api.AccessURL, api.WorkspaceAppsProvider, 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/provider.go b/coderd/workspaceapps/provider.go index 36a405230aa65..7b363ebc909d9 100644 --- a/coderd/workspaceapps/provider.go +++ b/coderd/workspaceapps/provider.go @@ -12,17 +12,17 @@ import ( const ( // TODO(@deansheather): configurable expiry - TicketExpiry = time.Minute + 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" ) -// ResolveRequest calls TicketProvider to use an existing ticket in the request -// or issue a new one. If it returns a ticket, it sets the cookie and returns -// it. -func ResolveRequest(log slog.Logger, accessURL *url.URL, p TicketProvider, rw http.ResponseWriter, r *http.Request, appReq Request) (*Ticket, bool) { +// 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 { @@ -30,47 +30,43 @@ func ResolveRequest(log slog.Logger, accessURL *url.URL, p TicketProvider, rw ht return nil, false } - ticket, ok := p.TicketFromRequest(r) - if ok && ticket.MatchesRequest(appReq) { - // The request has a valid ticket and it matches the request. - return ticket, true + 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 } - ticket, ticketStr, ok := p.CreateTicket(r.Context(), rw, r, appReq) + token, tokenStr, ok := p.CreateToken(r.Context(), rw, r, appReq) if !ok { 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). + // 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.DevURLSessionTicketCookie, - Value: ticketStr, + Name: codersdk.DevURLSignedAppTokenCookie, + Value: tokenStr, Path: appReq.BasePath, - Expires: ticket.Expiry, + Expires: token.Expiry, }) - return ticket, true + return token, true } -// TicketProvider provides workspace app tickets. -// -// Please keep in mind that all transactions incur a service fee, handling fee, -// order processing fee, delivery fee, insurance charge, convenience fee, -// inconvenience fee, seat selection fee, and levity levy. :^) -type TicketProvider interface { - // TicketFromRequest returns a ticket from the request. If the request does - // not contain a ticket or the ticket is invalid (expired, invalid +// 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. - TicketFromRequest(r *http.Request) (*Ticket, bool) - // CreateTicket creates a ticket for the given app request. It uses the - // long-lived session token in the HTTP request to authenticate the user. - // The ticket is returned in struct and string form. The string form should - // be written as a cookie. + 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. - CreateTicket(ctx context.Context, rw http.ResponseWriter, r *http.Request, appReq Request) (*Ticket, string, bool) + 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 022aad3a96813..349d71673d487 100644 --- a/coderd/workspaceapps/request.go +++ b/coderd/workspaceapps/request.go @@ -98,10 +98,10 @@ 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. diff --git a/coderd/workspaceapps/ticket.go b/coderd/workspaceapps/ticket.go index dc23f73b76229..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. -type Ticket struct { +type SignedToken struct { // Request details. Request `json:"request"` // Trusted resolved details. - Expiry time.Time `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"` } -// MatchesRequest returns true if the ticket matches the request. Any ticket -// that does not match the request should be considered invalid. -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,22 +37,20 @@ func (t Ticket) MatchesRequest(req Request) bool { t.AppSlugOrPort == req.AppSlugOrPort } -// GenerateTicket generates a workspace app ticket with the given key and -// payload. If the ticket doesn't have an expiry, it will be set to the current +// 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 GenerateTicket(key []byte, payload Ticket) (string, error) { +func GenerateToken(key []byte, payload SignedToken) (string, error) { if payload.Expiry.IsZero() { - payload.Expiry = time.Now().Add(TicketExpiry) + 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, + Algorithm: tokenSigningAlgorithm, Key: key, }, nil) if err != nil { @@ -72,33 +70,33 @@ func GenerateTicket(key []byte, payload Ticket) (string, error) { return serialized, nil } -// ParseTicket parses a workspace app ticket with the given key and returns the -// payload. If the ticket is invalid, an error is returned. -func ParseTicket(key []byte, 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(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.Before(time.Now()) { - 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 9112d04f179c8..9b5cc6a4e3e85 100644 --- a/coderd/workspaceapps/ticket_test.go +++ b/coderd/workspaceapps/ticket_test.go @@ -13,14 +13,14 @@ import ( "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", @@ -32,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", @@ -49,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, }, @@ -61,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", @@ -76,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", @@ -93,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", @@ -112,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", @@ -133,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", @@ -153,18 +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() t.Run("SetExpiry", func(t *testing.T) { t.Parallel() - ticketStr, err := workspaceapps.GenerateTicket(coderdtest.AppSigningKey, workspaceapps.Ticket{ + tokenStr, err := workspaceapps.GenerateToken(coderdtest.AppSigningKey, workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/app", @@ -182,21 +182,21 @@ func Test_GenerateTicket(t *testing.T) { }) require.NoError(t, err) - ticket, err := workspaceapps.ParseTicket(coderdtest.AppSigningKey, ticketStr) + token, err := workspaceapps.ParseToken(coderdtest.AppSigningKey, tokenStr) require.NoError(t, err) - require.WithinDuration(t, time.Now().Add(time.Minute), ticket.Expiry, 15*time.Second) + require.WithinDuration(t, time.Now().Add(time.Minute), token.Expiry, 15*time.Second) }) 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", @@ -215,7 +215,7 @@ func Test_GenerateTicket(t *testing.T) { }, { name: "OK2", - ticket: workspaceapps.Ticket{ + token: workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodSubdomain, BasePath: "/", @@ -234,7 +234,7 @@ func Test_GenerateTicket(t *testing.T) { }, { name: "Expired", - ticket: workspaceapps.Ticket{ + token: workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodSubdomain, BasePath: "/", @@ -250,7 +250,7 @@ func Test_GenerateTicket(t *testing.T) { AgentID: uuid.MustParse("9ec18681-d2c9-4c9e-9186-f136efb4edbe"), AppURL: "http://127.0.0.1:8080", }, - parseErrContains: "ticket expired", + parseErrContains: "token expired", }, } @@ -260,50 +260,51 @@ func Test_GenerateTicket(t *testing.T) { t.Run(c.name, func(t *testing.T) { t.Parallel() - str, err := workspaceapps.GenerateTicket(coderdtest.AppSigningKey, 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 := workspaceapps.ParseTicket(coderdtest.AppSigningKey, 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) // normalize the expiry - require.WithinDuration(t, c.ticket.Expiry, ticket.Expiry, 10*time.Second) - c.ticket.Expiry = ticket.Expiry - require.Equal(t, c.ticket, ticket) + 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() t.Run("InvalidJWS", func(t *testing.T) { t.Parallel() - ticket, err := workspaceapps.ParseTicket(coderdtest.AppSigningKey, "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) - ticketStr, err := workspaceapps.GenerateTicket(otherKey, workspaceapps.Ticket{ + tokenStr, err := workspaceapps.GenerateToken(otherKey, workspaceapps.SignedToken{ Request: workspaceapps.Request{ AccessMethod: workspaceapps.AccessMethodPath, BasePath: "/app", @@ -321,11 +322,11 @@ func Test_ParseTicket(t *testing.T) { }) require.NoError(t, err) - // Verify the ticket is invalid. - ticket, err := workspaceapps.ParseTicket(coderdtest.AppSigningKey, 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) { @@ -339,9 +340,9 @@ func Test_ParseTicket(t *testing.T) { serialized, err := signedObject.CompactSerialize() require.NoError(t, err) - ticket, err := workspaceapps.ParseTicket(coderdtest.AppSigningKey, 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 24e4529a82664..5a5a650d5edb8 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 312c6ad9c5c72..a65e2bf93dc73 100644 --- a/codersdk/client.go +++ b/codersdk/client.go @@ -42,10 +42,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. From 21af59978fbea6dd9cd59a4f6a4b19dd83f2cfe6 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Mon, 3 Apr 2023 23:52:23 +0000 Subject: [PATCH 4/4] fixup! chore: rename app tickets to signed app tokens --- coderd/workspaceapps/db.go | 1 + 1 file changed, 1 insertion(+) diff --git a/coderd/workspaceapps/db.go b/coderd/workspaceapps/db.go index 84234462e5696..be00a6c6a1f30 100644 --- a/coderd/workspaceapps/db.go +++ b/coderd/workspaceapps/db.go @@ -25,6 +25,7 @@ import ( 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