Skip to content

Commit 6deef06

Browse files
deansheatherkylecarbs
deansheather
and
kylecarbs
authored
feat: secure and cross-domain subdomain-based proxying (#4136)
Co-authored-by: Kyle Carberry <kyle@carberry.com>
1 parent 80b45f1 commit 6deef06

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+1658
-597
lines changed

‎Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ gen/mark-fresh:
410410
# Runs migrations to output a dump of the database schema after migrations are
411411
# applied.
412412
coderd/database/dump.sql: coderd/database/gen/dump/main.go $(wildcard coderd/database/migrations/*.sql)
413-
go run coderd/database/gen/dump/main.go
413+
go run ./coderd/database/gen/dump/main.go
414414

415415
# Generates Go code for querying the database.
416416
coderd/database/querier.go: coderd/database/sqlc.yaml coderd/database/dump.sql $(wildcard coderd/database/queries/*.sql) coderd/database/gen/enum/main.go

‎cli/server.go

+7
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, error))
7272
var (
7373
accessURL string
7474
address string
75+
wildcardAccessURL string
7576
autobuildPollInterval time.Duration
7677
derpServerEnabled bool
7778
derpServerRegionID int
@@ -347,8 +348,13 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, error))
347348
return xerrors.Errorf("create derp map: %w", err)
348349
}
349350

351+
appHostname := strings.TrimPrefix(wildcardAccessURL, "http://")
352+
appHostname = strings.TrimPrefix(appHostname, "https://")
353+
appHostname = strings.TrimPrefix(appHostname, "*.")
354+
350355
options := &coderd.Options{
351356
AccessURL: accessURLParsed,
357+
AppHostname: appHostname,
352358
Logger: logger.Named("coderd"),
353359
Database: databasefake.New(),
354360
DERPMap: derpMap,
@@ -755,6 +761,7 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, error))
755761
"External URL to access your deployment. This must be accessible by all provisioned workspaces.")
756762
cliflag.StringVarP(root.Flags(), &address, "address", "a", "CODER_ADDRESS", "127.0.0.1:3000",
757763
"Bind address of the server.")
764+
cliflag.StringVarP(root.Flags(), &wildcardAccessURL, "wildcard-access-url", "", "CODER_WILDCARD_ACCESS_URL", "", `Specifies the wildcard hostname to use for workspace applications in the form "*.example.com".`)
758765
cliflag.StringVarP(root.Flags(), &derpConfigURL, "derp-config-url", "", "CODER_DERP_CONFIG_URL", "",
759766
"URL to fetch a DERP mapping on startup. See: https://tailscale.com/kb/1118/custom-derp-servers/")
760767
cliflag.StringVarP(root.Flags(), &derpConfigPath, "derp-config-path", "", "CODER_DERP_CONFIG_PATH", "",

‎coderd/authorize.go

+45
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
package coderd
22

33
import (
4+
"fmt"
45
"net/http"
56

67
"golang.org/x/xerrors"
78

89
"cdr.dev/slog"
10+
"github.com/coder/coder/coderd/httpapi"
911
"github.com/coder/coder/coderd/httpmw"
1012
"github.com/coder/coder/coderd/rbac"
13+
"github.com/coder/coder/codersdk"
1114
)
1215

1316
func AuthorizeFilter[O rbac.Objecter](h *HTTPAuthorizer, r *http.Request, action rbac.Action, objects []O) ([]O, error) {
@@ -81,3 +84,45 @@ func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object r
8184
}
8285
return true
8386
}
87+
88+
// checkAuthorization returns if the current API key can use the given
89+
// permissions, factoring in the current user's roles and the API key scopes.
90+
func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
91+
ctx := r.Context()
92+
auth := httpmw.UserAuthorization(r)
93+
94+
var params codersdk.AuthorizationRequest
95+
if !httpapi.Read(ctx, rw, r, &params) {
96+
return
97+
}
98+
99+
api.Logger.Warn(ctx, "check-auth",
100+
slog.F("my_id", httpmw.APIKey(r).UserID),
101+
slog.F("got_id", auth.ID),
102+
slog.F("name", auth.Username),
103+
slog.F("roles", auth.Roles), slog.F("scope", auth.Scope),
104+
)
105+
106+
response := make(codersdk.AuthorizationResponse)
107+
for k, v := range params.Checks {
108+
if v.Object.ResourceType == "" {
109+
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
110+
Message: fmt.Sprintf("Object's \"resource_type\" field must be defined for key %q.", k),
111+
})
112+
return
113+
}
114+
115+
if v.Object.OwnerID == "me" {
116+
v.Object.OwnerID = auth.ID.String()
117+
}
118+
err := api.Authorizer.ByRoleName(r.Context(), auth.ID.String(), auth.Roles, auth.Scope.ToRBAC(), rbac.Action(v.Action),
119+
rbac.Object{
120+
Owner: v.Object.OwnerID,
121+
OrgID: v.Object.OrganizationID,
122+
Type: v.Object.ResourceType,
123+
})
124+
response[k] = err == nil
125+
}
126+
127+
httpapi.Write(ctx, rw, http.StatusOK, response)
128+
}

‎coderd/authorize_test.go

+124
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
package coderd_test
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/google/uuid"
8+
"github.com/stretchr/testify/require"
9+
10+
"github.com/coder/coder/coderd/coderdtest"
11+
"github.com/coder/coder/coderd/rbac"
12+
"github.com/coder/coder/codersdk"
13+
"github.com/coder/coder/testutil"
14+
)
15+
16+
func TestCheckPermissions(t *testing.T) {
17+
t.Parallel()
18+
19+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
20+
t.Cleanup(cancel)
21+
22+
adminClient := coderdtest.New(t, nil)
23+
// Create adminClient, member, and org adminClient
24+
adminUser := coderdtest.CreateFirstUser(t, adminClient)
25+
memberClient := coderdtest.CreateAnotherUser(t, adminClient, adminUser.OrganizationID)
26+
memberUser, err := memberClient.User(ctx, codersdk.Me)
27+
require.NoError(t, err)
28+
orgAdminClient := coderdtest.CreateAnotherUser(t, adminClient, adminUser.OrganizationID, rbac.RoleOrgAdmin(adminUser.OrganizationID))
29+
orgAdminUser, err := orgAdminClient.User(ctx, codersdk.Me)
30+
require.NoError(t, err)
31+
32+
// With admin, member, and org admin
33+
const (
34+
readAllUsers = "read-all-users"
35+
readOrgWorkspaces = "read-org-workspaces"
36+
readMyself = "read-myself"
37+
readOwnWorkspaces = "read-own-workspaces"
38+
)
39+
params := map[string]codersdk.AuthorizationCheck{
40+
readAllUsers: {
41+
Object: codersdk.AuthorizationObject{
42+
ResourceType: "users",
43+
},
44+
Action: "read",
45+
},
46+
readMyself: {
47+
Object: codersdk.AuthorizationObject{
48+
ResourceType: "users",
49+
OwnerID: "me",
50+
},
51+
Action: "read",
52+
},
53+
readOwnWorkspaces: {
54+
Object: codersdk.AuthorizationObject{
55+
ResourceType: "workspaces",
56+
OwnerID: "me",
57+
},
58+
Action: "read",
59+
},
60+
readOrgWorkspaces: {
61+
Object: codersdk.AuthorizationObject{
62+
ResourceType: "workspaces",
63+
OrganizationID: adminUser.OrganizationID.String(),
64+
},
65+
Action: "read",
66+
},
67+
}
68+
69+
testCases := []struct {
70+
Name string
71+
Client *codersdk.Client
72+
UserID uuid.UUID
73+
Check codersdk.AuthorizationResponse
74+
}{
75+
{
76+
Name: "Admin",
77+
Client: adminClient,
78+
UserID: adminUser.UserID,
79+
Check: map[string]bool{
80+
readAllUsers: true,
81+
readMyself: true,
82+
readOwnWorkspaces: true,
83+
readOrgWorkspaces: true,
84+
},
85+
},
86+
{
87+
Name: "OrgAdmin",
88+
Client: orgAdminClient,
89+
UserID: orgAdminUser.ID,
90+
Check: map[string]bool{
91+
readAllUsers: false,
92+
readMyself: true,
93+
readOwnWorkspaces: true,
94+
readOrgWorkspaces: true,
95+
},
96+
},
97+
{
98+
Name: "Member",
99+
Client: memberClient,
100+
UserID: memberUser.ID,
101+
Check: map[string]bool{
102+
readAllUsers: false,
103+
readMyself: true,
104+
readOwnWorkspaces: true,
105+
readOrgWorkspaces: false,
106+
},
107+
},
108+
}
109+
110+
for _, c := range testCases {
111+
c := c
112+
113+
t.Run("CheckAuthorization/"+c.Name, func(t *testing.T) {
114+
t.Parallel()
115+
116+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
117+
t.Cleanup(cancel)
118+
119+
resp, err := c.Client.CheckAuthorization(ctx, codersdk.AuthorizationRequest{Checks: params})
120+
require.NoError(t, err, "check perms")
121+
require.Equal(t, c.Check, resp)
122+
})
123+
}
124+
}

‎coderd/coderd.go

+48-19
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,12 @@ import (
4444
// Options are requires parameters for Coder to start.
4545
type Options struct {
4646
AccessURL *url.URL
47-
Logger slog.Logger
48-
Database database.Store
49-
Pubsub database.Pubsub
47+
// AppHostname should be the wildcard hostname to use for workspace
48+
// applications without the asterisk or leading dot. E.g. "apps.coder.com".
49+
AppHostname string
50+
Logger slog.Logger
51+
Database database.Store
52+
Pubsub database.Pubsub
5053

5154
// CacheDir is used for caching files served by the API.
5255
CacheDir string
@@ -158,7 +161,20 @@ func New(options *Options) *API {
158161
Github: options.GithubOAuth2Config,
159162
OIDC: options.OIDCConfig,
160163
}
161-
apiKeyMiddleware := httpmw.ExtractAPIKey(options.Database, oauthConfigs, false)
164+
165+
apiKeyMiddleware := httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
166+
DB: options.Database,
167+
OAuth2Configs: oauthConfigs,
168+
RedirectToLogin: false,
169+
Optional: false,
170+
})
171+
// Same as above but it redirects to the login page.
172+
apiKeyMiddlewareRedirect := httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
173+
DB: options.Database,
174+
OAuth2Configs: oauthConfigs,
175+
RedirectToLogin: true,
176+
Optional: false,
177+
})
162178

163179
r.Use(
164180
httpmw.AttachRequestID,
@@ -170,18 +186,14 @@ func New(options *Options) *API {
170186
api.handleSubdomainApplications(
171187
// Middleware to impose on the served application.
172188
httpmw.RateLimitPerMinute(options.APIRateLimit),
173-
httpmw.UseLoginURL(func() *url.URL {
174-
if options.AccessURL == nil {
175-
return nil
176-
}
177-
178-
u := *options.AccessURL
179-
u.Path = "/login"
180-
return &u
181-
}()),
182-
// This should extract the application specific API key when we
183-
// implement a scoped token.
184-
httpmw.ExtractAPIKey(options.Database, oauthConfigs, true),
189+
httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
190+
DB: options.Database,
191+
OAuth2Configs: oauthConfigs,
192+
// The code handles the the case where the user is not
193+
// authenticated automatically.
194+
RedirectToLogin: false,
195+
Optional: true,
196+
}),
185197
httpmw.ExtractUserParam(api.Database),
186198
httpmw.ExtractWorkspaceAndAgentParam(api.Database),
187199
),
@@ -199,7 +211,7 @@ func New(options *Options) *API {
199211
r.Use(
200212
tracing.Middleware(api.TracerProvider),
201213
httpmw.RateLimitPerMinute(options.APIRateLimit),
202-
httpmw.ExtractAPIKey(options.Database, oauthConfigs, true),
214+
apiKeyMiddlewareRedirect,
203215
httpmw.ExtractUserParam(api.Database),
204216
// Extracts the <workspace.agent> from the url
205217
httpmw.ExtractWorkspaceAndAgentParam(api.Database),
@@ -384,8 +396,6 @@ func New(options *Options) *API {
384396
r.Put("/roles", api.putUserRoles)
385397
r.Get("/roles", api.userRoles)
386398

387-
r.Post("/authorization", api.checkPermissions)
388-
389399
r.Route("/keys", func(r chi.Router) {
390400
r.Post("/", api.postAPIKey)
391401
r.Get("/{keyid}", api.apiKey)
@@ -481,6 +491,25 @@ func New(options *Options) *API {
481491
r.Get("/resources", api.workspaceBuildResources)
482492
r.Get("/state", api.workspaceBuildState)
483493
})
494+
r.Route("/authcheck", func(r chi.Router) {
495+
r.Use(apiKeyMiddleware)
496+
r.Post("/", api.checkAuthorization)
497+
})
498+
r.Route("/applications", func(r chi.Router) {
499+
r.Route("/host", func(r chi.Router) {
500+
// Don't leak the hostname to unauthenticated users.
501+
r.Use(apiKeyMiddleware)
502+
r.Get("/", api.appHost)
503+
})
504+
r.Route("/auth-redirect", func(r chi.Router) {
505+
// We want to redirect to login if they are not authenticated.
506+
r.Use(apiKeyMiddlewareRedirect)
507+
508+
// This is a GET request as it's redirected to by the subdomain app
509+
// handler and the login page.
510+
r.Get("/", api.workspaceApplicationAuth)
511+
})
512+
})
484513
})
485514

486515
r.NotFound(compressHandler(http.HandlerFunc(api.siteHandler.ServeHTTP)).ServeHTTP)

‎coderd/coderdtest/authorize.go

+5-2
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
4444
"POST:/api/v2/users/login": {NoAuthorize: true},
4545
"GET:/api/v2/users/authmethods": {NoAuthorize: true},
4646
"POST:/api/v2/csp/reports": {NoAuthorize: true},
47-
// This is a dummy endpoint for compatibility.
47+
"POST:/api/v2/authcheck": {NoAuthorize: true},
48+
"GET:/api/v2/applications/host": {NoAuthorize: true},
49+
// This is a dummy endpoint for compatibility with older CLI versions.
4850
"GET:/api/v2/workspaceagents/{workspaceagent}/dial": {NoAuthorize: true},
4951

5052
// Has it's own auth
@@ -238,7 +240,8 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
238240
AssertAction: rbac.ActionRead,
239241
AssertObject: workspaceRBACObj,
240242
},
241-
"GET:/api/v2/users": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceUser},
243+
"GET:/api/v2/users": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceUser},
244+
"GET:/api/v2/applications/auth-redirect": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceAPIKey},
242245

243246
// These endpoints need payloads to get to the auth part. Payloads will be required
244247
"PUT:/api/v2/users/{user}/roles": {StatusCode: http.StatusBadRequest, NoAuthorize: true},

‎coderd/coderdtest/authorize_test.go

+2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010
func TestAuthorizeAllEndpoints(t *testing.T) {
1111
t.Parallel()
1212
client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{
13+
// Required for any subdomain-based proxy tests to pass.
14+
AppHostname: "test.coder.com",
1315
Authorizer: &coderdtest.RecordingAuthorizer{},
1416
IncludeProvisionerDaemon: true,
1517
})

‎coderd/coderdtest/coderdtest.go

+2
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ import (
6565
)
6666

6767
type Options struct {
68+
AppHostname string
6869
AWSCertificates awsidentity.Certificates
6970
Authorizer rbac.Authorizer
7071
AzureCertificates x509.VerifyOptions
@@ -198,6 +199,7 @@ func NewOptions(t *testing.T, options *Options) (*httptest.Server, context.Cance
198199
// agents are not marked as disconnected during slow tests.
199200
AgentInactiveDisconnectTimeout: testutil.WaitShort,
200201
AccessURL: serverURL,
202+
AppHostname: options.AppHostname,
201203
Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug),
202204
CacheDir: t.TempDir(),
203205
Database: db,

‎coderd/database/dump.sql

+2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/migrations/000051_comment_on_api_key_secret.down.sql

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
-- noop, comments don't need to be removed

‎coderd/database/migrations/000051_comment_on_api_key_secret.up.sql

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
COMMENT ON COLUMN api_keys.hashed_secret
2+
IS 'hashed_secret contains a SHA256 hash of the key secret. This is considered a secret and MUST NOT be returned from the API as it is used for API key encryption in app proxying code.';

0 commit comments

Comments
 (0)