Skip to content

Commit ca8b74e

Browse files
deansheatherdeansheather
committed
chore: ticket provider interface
1 parent 665b84d commit ca8b74e

File tree

6 files changed

+139
-73
lines changed

6 files changed

+139
-73
lines changed

‎coderd/coderd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -763,7 +763,7 @@ type API struct {
763763
metricsCache *metricscache.Cache
764764
workspaceAgentCache *wsconncache.Cache
765765
updateChecker *updatecheck.Checker
766-
WorkspaceAppsProvider *workspaceapps.Provider
766+
WorkspaceAppsProvider *workspaceapps.DBTicketProvider
767767

768768
// Experiments contains the list of experiments currently enabled.
769769
// This is used to gate features that are not yet ready for production.

‎coderd/workspaceapps/auth.go

Lines changed: 71 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -29,26 +29,9 @@ const (
2929
RedirectURIQueryParam = "redirect_uri"
3030
)
3131

32-
// ResolveRequest takes an app request, checks if it's valid and authenticated,
33-
// and returns a ticket with details about the app.
34-
//
35-
// The ticket is written as a signed JWT into a cookie and will be automatically
36-
// used in the next request to the same app to avoid database calls.
37-
//
38-
// Upstream code should avoid any database calls ever.
39-
func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appReq Request) (*Ticket, bool) {
40-
// nolint:gocritic // We need to make a number of database calls. Setting a system context here
41-
// // is simpler than calling dbauthz.AsSystemRestricted on every call.
42-
// // dangerousSystemCtx is only used for database calls. The actual authentication
43-
// // logic is handled in Provider.authorizeWorkspaceApp which directly checks the actor's
44-
// // permissions.
45-
dangerousSystemCtx := dbauthz.AsSystemRestricted(r.Context())
46-
err := appReq.Validate()
47-
if err != nil {
48-
p.writeWorkspaceApp500(rw, r, &appReq, err, "invalid app request")
49-
return nil, false
50-
}
51-
32+
// TODO: remove this temporary shim
33+
func (p *DBTicketProvider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appReq Request) (*Ticket, bool) {
34+
// TODO: this needs to be some sort of normalize function or something
5235
if appReq.WorkspaceAndAgent != "" {
5336
// workspace.agent
5437
workspaceAndAgent := strings.SplitN(appReq.WorkspaceAndAgent, ".", 2)
@@ -66,23 +49,68 @@ func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appRe
6649
}
6750
}
6851

52+
ticket, ok := p.TicketFromRequest(r)
53+
if ok && ticket.MatchesRequest(appReq) {
54+
// The request has a valid ticket and it matches the request.
55+
return ticket, true
56+
}
57+
58+
ticket, ticketStr, ok := p.CreateTicket(r.Context(), rw, r, appReq)
59+
if !ok {
60+
return nil, false
61+
}
62+
63+
// Write the ticket cookie. We always want this to apply to the current
64+
// hostname (even for subdomain apps, without any wildcard shenanigans,
65+
// because the ticket is only valid for a single app).
66+
http.SetCookie(rw, &http.Cookie{
67+
Name: codersdk.DevURLSessionTicketCookie,
68+
Value: ticketStr,
69+
Path: appReq.BasePath,
70+
Expires: ticket.Expiry,
71+
})
72+
73+
return ticket, true
74+
}
75+
76+
func (p *DBTicketProvider) TicketFromRequest(r *http.Request) (*Ticket, bool) {
6977
// Get the existing ticket from the request.
7078
ticketCookie, err := r.Cookie(codersdk.DevURLSessionTicketCookie)
7179
if err == nil {
7280
ticket, err := p.ParseTicket(ticketCookie.Value)
7381
if err == nil {
7482
err := ticket.Request.Validate()
75-
if err == nil && ticket.MatchesRequest(appReq) {
83+
if err == nil {
7684
// The request has a ticket, which is a valid ticket signed by
77-
// us, and matches the app that the user was trying to access.
85+
// us. The caller must check that it matches the request.
7886
return &ticket, true
7987
}
8088
}
8189
}
8290

83-
// There's no ticket or it's invalid, so we need to check auth using the
84-
// session token, validate auth and access to the app, then generate a new
85-
// ticket.
91+
return nil, false
92+
}
93+
94+
// ResolveRequest takes an app request, checks if it's valid and authenticated,
95+
// and returns a ticket with details about the app.
96+
//
97+
// The ticket is written as a signed JWT into a cookie and will be automatically
98+
// used in the next request to the same app to avoid database calls.
99+
//
100+
// Upstream code should avoid any database calls ever.
101+
func (p *DBTicketProvider) CreateTicket(ctx context.Context, rw http.ResponseWriter, r *http.Request, appReq Request) (*Ticket, string, bool) {
102+
// nolint:gocritic // We need to make a number of database calls. Setting a system context here
103+
// // is simpler than calling dbauthz.AsSystemRestricted on every call.
104+
// // dangerousSystemCtx is only used for database calls. The actual authentication
105+
// // logic is handled in Provider.authorizeWorkspaceApp which directly checks the actor's
106+
// // permissions.
107+
dangerousSystemCtx := dbauthz.AsSystemRestricted(ctx)
108+
err := appReq.Validate()
109+
if err != nil {
110+
p.writeWorkspaceApp500(rw, r, &appReq, err, "invalid app request")
111+
return nil, "", false
112+
}
113+
86114
ticket := Ticket{
87115
Request: appReq,
88116
}
@@ -101,17 +129,17 @@ func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appRe
101129
Optional: true,
102130
})
103131
if !ok {
104-
return nil, false
132+
return nil, "", false
105133
}
106134

107135
// Lookup workspace app details from DB.
108136
dbReq, err := appReq.getDatabase(dangerousSystemCtx, p.Database)
109137
if xerrors.Is(err, sql.ErrNoRows) {
110138
p.writeWorkspaceApp404(rw, r, &appReq, err.Error())
111-
return nil, false
139+
return nil, "", false
112140
} else if err != nil {
113141
p.writeWorkspaceApp500(rw, r, &appReq, err, "get app details from database")
114-
return nil, false
142+
return nil, "", false
115143
}
116144
ticket.UserID = dbReq.User.ID
117145
ticket.WorkspaceID = dbReq.Workspace.ID
@@ -124,19 +152,20 @@ func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appRe
124152
// Verify the user has access to the app.
125153
authed, ok := p.verifyAuthz(rw, r, authz, dbReq)
126154
if !ok {
127-
return nil, false
155+
return nil, "", false
128156
}
129157
if !authed {
130158
if apiKey != nil {
131159
// The request has a valid API key but insufficient permissions.
132160
p.writeWorkspaceApp404(rw, r, &appReq, "insufficient permissions")
133-
return nil, false
161+
return nil, "", false
134162
}
135163

136164
// Redirect to login as they don't have permission to access the app
137165
// and they aren't signed in.
138166
switch appReq.AccessMethod {
139167
case AccessMethodPath:
168+
// TODO(@deansheather): this doesn't work on moons
140169
httpmw.RedirectToLogin(rw, r, httpmw.SignedOutErrorMessage)
141170
case AccessMethodSubdomain:
142171
// 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
156185
// Return an error.
157186
httpapi.ResourceNotFound(rw)
158187
}
159-
return nil, false
188+
return nil, "", false
160189
}
161190

162191
// Check that the agent is online.
163192
agentStatus := dbReq.Agent.Status(p.WorkspaceAgentInactiveTimeout)
164193
if agentStatus.Status != database.WorkspaceAgentStatusConnected {
165194
p.writeWorkspaceAppOffline(rw, r, &appReq, fmt.Sprintf("Agent state is %q, not %q", agentStatus.Status, database.WorkspaceAgentStatusConnected))
166-
return nil, false
195+
return nil, "", false
167196
}
168197

169198
// Check that the app is healthy.
170199
if dbReq.AppHealth != "" && dbReq.AppHealth != database.WorkspaceAppHealthDisabled && dbReq.AppHealth != database.WorkspaceAppHealthHealthy {
171200
p.writeWorkspaceAppOffline(rw, r, &appReq, fmt.Sprintf("App health is %q, not %q", dbReq.AppHealth, database.WorkspaceAppHealthHealthy))
172-
return nil, false
201+
return nil, "", false
173202
}
174203

175204
// As a sanity check, ensure the ticket we just made is valid for this
176205
// request.
177206
if !ticket.MatchesRequest(appReq) {
178207
p.writeWorkspaceApp500(rw, r, &appReq, nil, "fresh ticket does not match request")
179-
return nil, false
208+
return nil, "", false
180209
}
181210

182211
// Sign the ticket.
183-
ticketExpiry := time.Now().Add(TicketExpiry)
184-
ticket.Expiry = ticketExpiry.Unix()
212+
ticket.Expiry = time.Now().Add(TicketExpiry)
185213
ticketStr, err := p.GenerateTicket(ticket)
186214
if err != nil {
187215
p.writeWorkspaceApp500(rw, r, &appReq, err, "generate ticket")
188-
return nil, false
216+
return nil, "", false
189217
}
190218

191-
// Write the ticket cookie. We always want this to apply to the current
192-
// hostname (even for subdomain apps, without any wildcard shenanigans,
193-
// because the ticket is only valid for a single app).
194-
http.SetCookie(rw, &http.Cookie{
195-
Name: codersdk.DevURLSessionTicketCookie,
196-
Value: ticketStr,
197-
Path: appReq.BasePath,
198-
Expires: ticketExpiry,
199-
})
200-
201-
return &ticket, true
219+
return &ticket, ticketStr, true
202220
}
203221

204-
func (p *Provider) authorizeRequest(ctx context.Context, roles *httpmw.Authorization, dbReq *databaseRequest) (bool, error) {
222+
func (p *DBTicketProvider) authorizeRequest(ctx context.Context, roles *httpmw.Authorization, dbReq *databaseRequest) (bool, error) {
205223
accessMethod := dbReq.AccessMethod
206224
if accessMethod == "" {
207225
accessMethod = AccessMethodPath
@@ -293,7 +311,7 @@ func (p *Provider) authorizeRequest(ctx context.Context, roles *httpmw.Authoriza
293311
// given app share level in the given workspace. The user's authorization status
294312
// is returned. If a server error occurs, a HTML error page is rendered and
295313
// false is returned so the caller can return early.
296-
func (p *Provider) verifyAuthz(rw http.ResponseWriter, r *http.Request, authz *httpmw.Authorization, dbReq *databaseRequest) (authed bool, ok bool) {
314+
func (p *DBTicketProvider) verifyAuthz(rw http.ResponseWriter, r *http.Request, authz *httpmw.Authorization, dbReq *databaseRequest) (authed bool, ok bool) {
297315
ok, err := p.authorizeRequest(r.Context(), authz, dbReq)
298316
if err != nil {
299317
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
312330

313331
// writeWorkspaceApp404 writes a HTML 404 error page for a workspace app. If
314332
// appReq is not nil, it will be used to log the request details at debug level.
315-
func (p *Provider) writeWorkspaceApp404(rw http.ResponseWriter, r *http.Request, appReq *Request, msg string) {
333+
func (p *DBTicketProvider) writeWorkspaceApp404(rw http.ResponseWriter, r *http.Request, appReq *Request, msg string) {
316334
if appReq != nil {
317335
slog.Helper()
318336
p.Logger.Debug(r.Context(),
@@ -336,7 +354,7 @@ func (p *Provider) writeWorkspaceApp404(rw http.ResponseWriter, r *http.Request,
336354

337355
// writeWorkspaceApp500 writes a HTML 500 error page for a workspace app. If
338356
// appReq is not nil, it's fields will be added to the logged error message.
339-
func (p *Provider) writeWorkspaceApp500(rw http.ResponseWriter, r *http.Request, appReq *Request, err error, msg string) {
357+
func (p *DBTicketProvider) writeWorkspaceApp500(rw http.ResponseWriter, r *http.Request, appReq *Request, err error, msg string) {
340358
slog.Helper()
341359
ctx := r.Context()
342360
if appReq != nil {
@@ -364,7 +382,7 @@ func (p *Provider) writeWorkspaceApp500(rw http.ResponseWriter, r *http.Request,
364382

365383
// writeWorkspaceAppOffline writes a HTML 502 error page for a workspace app. If
366384
// appReq is not nil, it will be used to log the request details at debug level.
367-
func (p *Provider) writeWorkspaceAppOffline(rw http.ResponseWriter, r *http.Request, appReq *Request, msg string) {
385+
func (p *DBTicketProvider) writeWorkspaceAppOffline(rw http.ResponseWriter, r *http.Request, appReq *Request, msg string) {
368386
if appReq != nil {
369387
slog.Helper()
370388
p.Logger.Debug(r.Context(),

‎coderd/workspaceapps/auth_test.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ func Test_ResolveRequest(t *testing.T) {
255255
AppURL: appURL,
256256
}, ticket)
257257
require.NotZero(t, ticket.Expiry)
258-
require.InDelta(t, time.Now().Add(workspaceapps.TicketExpiry).Unix(), ticket.Expiry, time.Minute.Seconds())
258+
require.WithinDuration(t, time.Now().Add(workspaceapps.TicketExpiry), ticket.Expiry, time.Minute)
259259

260260
// Check that the ticket was set in the response and is valid.
261261
require.Len(t, w.Cookies(), 1)
@@ -265,6 +265,9 @@ func Test_ResolveRequest(t *testing.T) {
265265

266266
parsedTicket, err := api.WorkspaceAppsProvider.ParseTicket(cookie.Value)
267267
require.NoError(t, err)
268+
// normalize expiry
269+
require.WithinDuration(t, ticket.Expiry, parsedTicket.Expiry, 2*time.Second)
270+
parsedTicket.Expiry = ticket.Expiry
268271
require.Equal(t, ticket, &parsedTicket)
269272

270273
// Try resolving the request with the ticket only.
@@ -274,6 +277,9 @@ func Test_ResolveRequest(t *testing.T) {
274277

275278
secondTicket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req)
276279
require.True(t, ok)
280+
// normalize expiry
281+
require.WithinDuration(t, ticket.Expiry, secondTicket.Expiry, 2*time.Second)
282+
secondTicket.Expiry = ticket.Expiry
277283
require.Equal(t, ticket, secondTicket)
278284
}
279285
})
@@ -470,7 +476,7 @@ func Test_ResolveRequest(t *testing.T) {
470476
// App name differs
471477
AppSlugOrPort: appNamePublic,
472478
},
473-
Expiry: time.Now().Add(time.Minute).Unix(),
479+
Expiry: time.Now().Add(time.Minute),
474480
UserID: me.ID,
475481
WorkspaceID: workspace.ID,
476482
AgentID: agentID,

‎coderd/workspaceapps/provider.go

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package workspaceapps
22

33
import (
4+
"context"
5+
"net/http"
46
"net/url"
57
"time"
68

@@ -11,10 +13,45 @@ import (
1113
"github.com/coder/coder/codersdk"
1214
)
1315

14-
// Provider provides authentication and authorization for workspace apps.
15-
// TODO(@deansheather): also provide workspace apps as a whole to remove all app
16-
// code from coderd.
17-
type Provider struct {
16+
/*
17+
POST /api/v2/moons/app-auth-ticket
18+
19+
{
20+
"session_token": "xxxx",
21+
"request": { ... }
22+
}
23+
24+
type moonRes struct {
25+
Ticket *Ticket
26+
TicketStr string
27+
}
28+
*/
29+
30+
// TicketProvider provides workspace app tickets.
31+
//
32+
// write a funny comment that says a ridiculous amount of fees will be incurred:
33+
//
34+
// Please keep in mind that all transactions incur a service fee, handling fee,
35+
// order processing fee, delivery fee,
36+
type TicketProvider interface {
37+
// TicketFromRequest returns a ticket from the request. If the request does
38+
// not contain a ticket or the ticket is invalid (expired, invalid
39+
// signature, etc.), it returns false.
40+
TicketFromRequest(r *http.Request) (*Ticket, bool)
41+
// CreateTicket creates a ticket for the given app request. It uses the
42+
// long-lived session token in the HTTP request to authenticate the user.
43+
// The ticket is returned in struct and string form. The string form should
44+
// be written as a cookie.
45+
//
46+
// If the request is invalid or the user is not authorized to access the
47+
// app, false is returned. An error page is written to the response writer
48+
// in this case.
49+
CreateTicket(ctx context.Context, rw http.ResponseWriter, r *http.Request, appReq Request) (*Ticket, string, bool)
50+
}
51+
52+
// DBTicketProvider provides authentication and authorization for workspace apps
53+
// by querying the database if the request is missing a valid ticket.
54+
type DBTicketProvider struct {
1855
Logger slog.Logger
1956

2057
AccessURL *url.URL
@@ -26,7 +63,9 @@ type Provider struct {
2663
TicketSigningKey []byte
2764
}
2865

29-
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 {
66+
var _ TicketProvider = &DBTicketProvider{}
67+
68+
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 {
3069
if len(ticketSigningKey) != 64 {
3170
panic("ticket signing key must be 64 bytes")
3271
}
@@ -35,7 +74,7 @@ func New(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database
3574
workspaceAgentInactiveTimeout = 1 * time.Minute
3675
}
3776

38-
return &Provider{
77+
return &DBTicketProvider{
3978
Logger: log,
4079
AccessURL: accessURL,
4180
Authorizer: authz,

0 commit comments

Comments
 (0)