pr comments

deansheather · deansheather · commit 0f77db359ef4 · 2023-04-20T23:38:30.000Z
diff --git a/coderd/database/dbauthz/system.go b/coderd/database/dbauthz/system.go
@@ -440,7 +440,7 @@ func (q *querier) InsertParameterSchema(ctx context.Context, arg database.Insert
 }
 
 func (q *querier) GetWorkspaceProxyByHostname(ctx context.Context, params database.GetWorkspaceProxyByHostnameParams) (database.WorkspaceProxy, error) {
-	if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil {
+	if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil {
 		return database.WorkspaceProxy{}, err
 	}
 	return q.db.GetWorkspaceProxyByHostname(ctx, params)
diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go
@@ -153,7 +153,10 @@ type ValidWorkspaceAppHostnameOpts struct {
 // ValidWorkspaceAppHostname checks if the given host is a valid workspace app
 // hostname based on the provided options. It returns a scheme to force on
 // success. If the hostname is not valid or doesn't match, an empty string is
-// returned.
+// returned. Any error returned is a 500 error.
+//
+// For hosts that match a wildcard app hostname, the scheme is forced to be the
+// corresponding access URL scheme.
 func (api *API) ValidWorkspaceAppHostname(ctx context.Context, host string, opts ValidWorkspaceAppHostnameOpts) (string, error) {
 	if opts.AllowPrimaryAccessURL && (host == api.AccessURL.Hostname() || host == api.AccessURL.Host) {
 		// Force the redirect URI to have the same scheme as the access URL for
diff --git a/coderd/workspaceapps/apptest/apptest.go b/coderd/workspaceapps/apptest/apptest.go
@@ -111,6 +111,11 @@ func Run(t *testing.T, factory DeploymentFactory) {
 			}
 
 			u := *appDetails.PathAppBaseURL
+			if u.Scheme == "http" {
+				u.Scheme = "ws"
+			} else {
+				u.Scheme = "wss"
+			}
 			u.Path = fmt.Sprintf("/api/v2/workspaceagents/%s/pty", appDetails.Agent.ID.String())
 
 			ctx := testutil.Context(t, testutil.WaitLong)
diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go
@@ -240,6 +240,9 @@ func (api *API) reconnectingPTYSignedToken(rw http.ResponseWriter, r *http.Reque
 	}
 
 	u, err := url.Parse(req.URL)
+	if err == nil && u.Scheme != "ws" && u.Scheme != "wss" {
+		err = xerrors.Errorf("invalid URL scheme %q, expected 'ws' or 'wss'", u.Scheme)
+	}
 	if err != nil {
 		httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
 			Message: "Invalid URL.",
diff --git a/enterprise/coderd/workspaceproxy_test.go b/enterprise/coderd/workspaceproxy_test.go
@@ -275,6 +275,26 @@ func TestReconnectingPTYSignedToken(t *testing.T) {
 		require.Contains(t, sdkErr.Response.Message, "Invalid URL")
 	})
 
+	t.Run("BadURL", func(t *testing.T) {
+		t.Parallel()
+
+		u := *u
+		u.Scheme = "ftp"
+
+		ctx := testutil.Context(t, testutil.WaitLong)
+		res, err := client.IssueReconnectingPTYSignedToken(ctx, codersdk.IssueReconnectingPTYSignedTokenRequest{
+			URL:     u.String(),
+			AgentID: agentID,
+		})
+		require.Error(t, err)
+		require.Empty(t, res)
+		var sdkErr *codersdk.Error
+		require.ErrorAs(t, err, &sdkErr)
+		require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode())
+		require.Contains(t, sdkErr.Response.Message, "Invalid URL")
+		require.Contains(t, sdkErr.Response.Detail, "scheme")
+	})
+
 	t.Run("BadURLPath", func(t *testing.T) {
 		t.Parallel()
 

Original file line number	Diff line number	Diff line change
`@@ -440,7 +440,7 @@ func (q *querier) InsertParameterSchema(ctx context.Context, arg database.Insert`
`440`	`440`	`}`
`441`	`441`
`442`	`442`	`func (q *querier) GetWorkspaceProxyByHostname(ctx context.Context, params database.GetWorkspaceProxyByHostnameParams) (database.WorkspaceProxy, error) {`
`443`		`- if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil {`
	`443`	`+ if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil {`
`444`	`444`	`return database.WorkspaceProxy{}, err`
`445`	`445`	`}`
`446`	`446`	`return q.db.GetWorkspaceProxyByHostname(ctx, params)`
Original file line number	Diff line number	Diff line change
`@@ -240,6 +240,9 @@ func (api API) reconnectingPTYSignedToken(rw http.ResponseWriter, r http.Reque`
`240`	`240`	`}`
`241`	`241`
`242`	`242`	`u, err := url.Parse(req.URL)`
	`243`	`+ if err == nil && u.Scheme != "ws" && u.Scheme != "wss" {`
	`244`	`+ err = xerrors.Errorf("invalid URL scheme %q, expected 'ws' or 'wss'", u.Scheme)`
	`245`	`+ }`
`243`	`246`	`if err != nil {`
`244`	`247`	`httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{`
`245`	`248`	`Message: "Invalid URL.",`