Skip to content

Commit dc884eb

Browse files
deansheatherdeansheather
committed
comments
1 parent 82d10d9 commit dc884eb

File tree

7 files changed

+35
-31
lines changed

7 files changed

+35
-31
lines changed

‎coderd/workspaceapps/apptest/apptest.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import (
3333
// Run runs the entire workspace app test suite against deployments minted
3434
// by the provided factory.
3535
func Run(t *testing.T, factory DeploymentFactory) {
36-
setupProxyTest := func(t *testing.T, opts *DeploymentOptions) *AppDetails {
36+
setupProxyTest := func(t *testing.T, opts *DeploymentOptions) *Details {
3737
return setupProxyTestWithFactory(t, factory, opts)
3838
}
3939

@@ -811,7 +811,7 @@ func Run(t *testing.T, factory DeploymentFactory) {
811811
t.Run("AppSharing", func(t *testing.T) {
812812
t.Parallel()
813813

814-
setup := func(t *testing.T, allowPathAppSharing, allowSiteOwnerAccess bool) (appDetails *AppDetails, workspace codersdk.Workspace, agnt codersdk.WorkspaceAgent, user codersdk.User, ownerClient *codersdk.Client, client *codersdk.Client, clientInOtherOrg *codersdk.Client, clientWithNoAuth *codersdk.Client) {
814+
setup := func(t *testing.T, allowPathAppSharing, allowSiteOwnerAccess bool) (appDetails *Details, workspace codersdk.Workspace, agnt codersdk.WorkspaceAgent, user codersdk.User, ownerClient *codersdk.Client, client *codersdk.Client, clientInOtherOrg *codersdk.Client, clientWithNoAuth *codersdk.Client) {
815815
//nolint:gosec
816816
const password = "SomeSecurePassword!"
817817

@@ -910,7 +910,7 @@ func Run(t *testing.T, factory DeploymentFactory) {
910910
return appDetails, workspace, agnt, user, ownerClient, client, clientInOtherOrg, clientWithNoAuth
911911
}
912912

913-
verifyAccess := func(t *testing.T, appDetails *AppDetails, isPathApp bool, username, workspaceName, agentName, appName string, client *codersdk.Client, shouldHaveAccess, shouldRedirectToLogin bool) {
913+
verifyAccess := func(t *testing.T, appDetails *Details, isPathApp bool, username, workspaceName, agentName, appName string, client *codersdk.Client, shouldHaveAccess, shouldRedirectToLogin bool) {
914914
t.Helper()
915915

916916
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)

‎coderd/workspaceapps/apptest/setup.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ type App struct {
8787
Query string
8888
}
8989

90-
// AppDetails are the full test details returned from setupProxyTestWithFactory.
91-
type AppDetails struct {
90+
// Details are the full test details returned from setupProxyTestWithFactory.
91+
type Details struct {
9292
*Deployment
9393

9494
Me codersdk.User
@@ -112,7 +112,7 @@ type AppDetails struct {
112112
// are not followed by default.
113113
//
114114
// The client is authenticated as the first user by default.
115-
func (d *AppDetails) AppClient(t *testing.T) *codersdk.Client {
115+
func (d *Details) AppClient(t *testing.T) *codersdk.Client {
116116
client := codersdk.New(d.PathAppBaseURL)
117117
client.SetSessionToken(d.SDKClient.SessionToken())
118118
forceURLTransport(t, client)
@@ -124,7 +124,7 @@ func (d *AppDetails) AppClient(t *testing.T) *codersdk.Client {
124124
}
125125

126126
// PathAppURL returns the URL for the given path app.
127-
func (d *AppDetails) PathAppURL(app App) *url.URL {
127+
func (d *Details) PathAppURL(app App) *url.URL {
128128
appPath := fmt.Sprintf("/@%s/%s/apps/%s", app.Username, app.WorkspaceName, app.AppSlugOrPort)
129129

130130
u := *d.PathAppBaseURL
@@ -135,7 +135,7 @@ func (d *AppDetails) PathAppURL(app App) *url.URL {
135135
}
136136

137137
// SubdomainAppURL returns the URL for the given subdomain app.
138-
func (d *AppDetails) SubdomainAppURL(app App) *url.URL {
138+
func (d *Details) SubdomainAppURL(app App) *url.URL {
139139
host := fmt.Sprintf("%s--%s--%s--%s", app.AppSlugOrPort, app.AgentName, app.WorkspaceName, app.Username)
140140

141141
u := *d.PathAppBaseURL
@@ -151,7 +151,7 @@ func (d *AppDetails) SubdomainAppURL(app App) *url.URL {
151151
// 3. Create a template version, template and workspace with many apps.
152152
// 4. Start a workspace agent.
153153
// 5. Returns details about the deployment and its apps.
154-
func setupProxyTestWithFactory(t *testing.T, factory DeploymentFactory, opts *DeploymentOptions) *AppDetails {
154+
func setupProxyTestWithFactory(t *testing.T, factory DeploymentFactory, opts *DeploymentOptions) *Details {
155155
if opts == nil {
156156
opts = &DeploymentOptions{}
157157
}
@@ -178,7 +178,7 @@ func setupProxyTestWithFactory(t *testing.T, factory DeploymentFactory, opts *De
178178
require.NoError(t, err)
179179

180180
if opts.noWorkspace {
181-
return &AppDetails{
181+
return &Details{
182182
Deployment: deployment,
183183
Me: me,
184184
}
@@ -189,7 +189,7 @@ func setupProxyTestWithFactory(t *testing.T, factory DeploymentFactory, opts *De
189189
}
190190
workspace, agnt := createWorkspaceWithApps(t, deployment.SDKClient, deployment.FirstUser.OrganizationID, me, opts.port)
191191

192-
return &AppDetails{
192+
return &Details{
193193
Deployment: deployment,
194194
Me: me,
195195
Workspace: &workspace,
@@ -336,8 +336,12 @@ func createWorkspaceWithApps(t *testing.T, client *codersdk.Client, orgID uuid.U
336336
agentClient.SetSessionToken(authToken)
337337

338338
// TODO (@dean): currently, the primary app host is used when generating
339-
// this URL and we don't have any plans to change that until we let
340-
// templates pick which proxy they want to use.
339+
// the port URL we tell the agent to use. We don't have any plans to change
340+
// that until we let templates pick which proxy they want to use in the
341+
// terraform.
342+
//
343+
// This means that all port URLs generated in code-server etc. will be sent
344+
// to the primary.
341345
appHostCtx := testutil.Context(t, testutil.WaitLong)
342346
primaryAppHost, err := client.AppHost(appHostCtx)
343347
require.NoError(t, err)

‎coderd/workspaceapps/db.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,11 @@ func NewDBTokenProvider(log slog.Logger, accessURL *url.URL, authz rbac.Authoriz
5555
}
5656
}
5757

58-
func (p *DBTokenProvider) TokenFromRequest(r *http.Request) (*SignedToken, bool) {
59-
return TokenFromRequest(r, p.SigningKey)
58+
func (p *DBTokenProvider) FromRequest(r *http.Request) (*SignedToken, bool) {
59+
return FromRequest(r, p.SigningKey)
6060
}
6161

62-
func (p *DBTokenProvider) IssueToken(ctx context.Context, rw http.ResponseWriter, r *http.Request, issueReq IssueTokenRequest) (*SignedToken, string, bool) {
62+
func (p *DBTokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *http.Request, issueReq IssueTokenRequest) (*SignedToken, string, bool) {
6363
// nolint:gocritic // We need to make a number of database calls. Setting a system context here
6464
// // is simpler than calling dbauthz.AsSystemRestricted on every call.
6565
// // dangerousSystemCtx is only used for database calls. The actual authentication

‎coderd/workspaceapps/provider.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func ResolveRequest(rw http.ResponseWriter, r *http.Request, opts ResolveRequest
4545
return nil, false
4646
}
4747

48-
token, ok := opts.SignedTokenProvider.TokenFromRequest(r)
48+
token, ok := opts.SignedTokenProvider.FromRequest(r)
4949
if ok && token.MatchesRequest(appReq) {
5050
// The request has a valid signed app token and it matches the request.
5151
return token, true
@@ -60,7 +60,7 @@ func ResolveRequest(rw http.ResponseWriter, r *http.Request, opts ResolveRequest
6060
AppQuery: opts.AppQuery,
6161
}
6262

63-
token, tokenStr, ok := opts.SignedTokenProvider.IssueToken(r.Context(), rw, r, issueReq)
63+
token, tokenStr, ok := opts.SignedTokenProvider.Issue(r.Context(), rw, r, issueReq)
6464
if !ok {
6565
return nil, false
6666
}
@@ -80,17 +80,17 @@ func ResolveRequest(rw http.ResponseWriter, r *http.Request, opts ResolveRequest
8080

8181
// SignedTokenProvider provides signed workspace app tokens (aka. app tickets).
8282
type SignedTokenProvider interface {
83-
// TokenFromRequest returns a parsed token from the request. If the request
84-
// does not contain a signed app token or is is invalid (expired, invalid
83+
// FromRequest returns a parsed token from the request. If the request does
84+
// not contain a signed app token or is is invalid (expired, invalid
8585
// signature, etc.), it returns false.
86-
TokenFromRequest(r *http.Request) (*SignedToken, bool)
87-
// IssueToken mints a new token for the given app request. It uses the
88-
// long-lived session token in the HTTP request to authenticate and
89-
// authorize the client for the given workspace app. The token is returned
90-
// in struct and string form. The string form should be written as a cookie.
86+
FromRequest(r *http.Request) (*SignedToken, bool)
87+
// Issue mints a new token for the given app request. It uses the long-lived
88+
// session token in the HTTP request to authenticate and authorize the
89+
// client for the given workspace app. The token is returned in struct and
90+
// string form. The string form should be written as a cookie.
9191
//
9292
// If the request is invalid or the user is not authorized to access the
9393
// app, false is returned. An error page is written to the response writer
9494
// in this case.
95-
IssueToken(ctx context.Context, rw http.ResponseWriter, r *http.Request, appReq IssueTokenRequest) (*SignedToken, string, bool)
95+
Issue(ctx context.Context, rw http.ResponseWriter, r *http.Request, appReq IssueTokenRequest) (*SignedToken, string, bool)
9696
}

‎coderd/workspaceapps/token.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ func (k SecurityKey) DecryptAPIKey(encryptedAPIKey string) (string, error) {
220220
return payload.APIKey, nil
221221
}
222222

223-
func TokenFromRequest(r *http.Request, key SecurityKey) (*SignedToken, bool) {
223+
func FromRequest(r *http.Request, key SecurityKey) (*SignedToken, bool) {
224224
// Get the existing token from the request.
225225
tokenCookie, err := r.Cookie(codersdk.DevURLSignedAppTokenCookie)
226226
if err == nil {

‎enterprise/coderd/workspaceproxy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ func (api *API) workspaceProxyIssueSignedAppToken(rw http.ResponseWriter, r *htt
195195
userReq.Header.Set(codersdk.SessionTokenHeader, req.SessionToken)
196196

197197
// Exchange the token.
198-
token, tokenStr, ok := api.AGPL.WorkspaceAppsProvider.IssueToken(ctx, rw, userReq, req)
198+
token, tokenStr, ok := api.AGPL.WorkspaceAppsProvider.Issue(ctx, rw, userReq, req)
199199
if !ok {
200200
return
201201
}

‎enterprise/wsproxy/tokenprovider.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@ type ProxyTokenProvider struct {
2323
Logger slog.Logger
2424
}
2525

26-
func (p *ProxyTokenProvider) TokenFromRequest(r *http.Request) (*workspaceapps.SignedToken, bool) {
27-
return workspaceapps.TokenFromRequest(r, p.SecurityKey)
26+
func (p *ProxyTokenProvider) FromRequest(r *http.Request) (*workspaceapps.SignedToken, bool) {
27+
return workspaceapps.FromRequest(r, p.SecurityKey)
2828
}
2929

30-
func (p *ProxyTokenProvider) IssueToken(ctx context.Context, rw http.ResponseWriter, r *http.Request, issueReq workspaceapps.IssueTokenRequest) (*workspaceapps.SignedToken, string, bool) {
30+
func (p *ProxyTokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *http.Request, issueReq workspaceapps.IssueTokenRequest) (*workspaceapps.SignedToken, string, bool) {
3131
appReq := issueReq.AppRequest.Normalize()
3232
err := appReq.Validate()
3333
if err != nil {

0 commit comments

Comments
 (0)