Skip to content

feat: secure and cross-domain subdomain-based proxying #4136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 24 commits into from
Sep 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
6d21b37
feat: add --app-hostname flag to coder server
deansheather Sep 19, 2022
b8a7a45
chore: add test for subdomain proxy passthrough
deansheather Sep 19, 2022
3b875b2
fixup! chore: add test for subdomain proxy passthrough
deansheather Sep 19, 2022
39e9b6f
chore: reorganize subdomain app handler
deansheather Sep 19, 2022
3a099ba
chore: add authorization check endpoint
deansheather Sep 20, 2022
25b8182
Merge branch 'main' into dean/app-tokens
deansheather Sep 20, 2022
e864f27
chore: improve proxy auth tests
deansheather Sep 20, 2022
b5d2be3
chore: refactor ExtractAPIKey to accept struct
deansheather Sep 20, 2022
d9b404d
feat: end-to-end workspace application authentication
deansheather Sep 21, 2022
da5f656
Merge branch 'main' into dean/app-tokens
deansheather Sep 21, 2022
312e0d5
fixup! Merge branch 'main' into dean/app-tokens
deansheather Sep 21, 2022
16bbcbe
feat: use a custom cookie name for devurls to avoid clashes
deansheather Sep 21, 2022
a172cd5
feat: /api/v2/applications/host endpoint, PR comments
deansheather Sep 21, 2022
d4986d2
fixup! feat: /api/v2/applications/host endpoint, PR comments
deansheather Sep 21, 2022
9b56f02
fixup! feat: /api/v2/applications/host endpoint, PR comments
deansheather Sep 21, 2022
d9186a8
chore: more pr comments
deansheather Sep 21, 2022
35962fc
Remove checkUserPermissions
kylecarbs Sep 21, 2022
b1436ec
fixup! Remove checkUserPermissions
deansheather Sep 21, 2022
11e985f
Merge branch 'main' into dean/app-tokens
deansheather Sep 21, 2022
496fde3
fixup! Merge branch 'main' into dean/app-tokens
deansheather Sep 21, 2022
3e30a9f
chore: more security stuff
deansheather Sep 21, 2022
11e6061
fixup! chore: more security stuff
deansheather Sep 21, 2022
cf70650
chore: more comments
deansheather Sep 21, 2022
6d66f55
Merge branch 'main' into dean/app-tokens
deansheather Sep 22, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ gen/mark-fresh:
# Runs migrations to output a dump of the database schema after migrations are
# applied.
coderd/database/dump.sql: coderd/database/gen/dump/main.go $(wildcard coderd/database/migrations/*.sql)
go run coderd/database/gen/dump/main.go
go run ./coderd/database/gen/dump/main.go

# Generates Go code for querying the database.
coderd/database/querier.go: coderd/database/sqlc.yaml coderd/database/dump.sql $(wildcard coderd/database/queries/*.sql) coderd/database/gen/enum/main.go
Expand Down
7 changes: 7 additions & 0 deletions cli/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, error))
var (
accessURL string
address string
wildcardAccessURL string
autobuildPollInterval time.Duration
derpServerEnabled bool
derpServerRegionID int
Expand Down Expand Up @@ -347,8 +348,13 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, error))
return xerrors.Errorf("create derp map: %w", err)
}

appHostname := strings.TrimPrefix(wildcardAccessURL, "http://")
appHostname = strings.TrimPrefix(appHostname, "https://")
appHostname = strings.TrimPrefix(appHostname, "*.")

options := &coderd.Options{
AccessURL: accessURLParsed,
AppHostname: appHostname,
Logger: logger.Named("coderd"),
Database: databasefake.New(),
DERPMap: derpMap,
Expand Down Expand Up @@ -755,6 +761,7 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, error))
"External URL to access your deployment. This must be accessible by all provisioned workspaces.")
cliflag.StringVarP(root.Flags(), &address, "address", "a", "CODER_ADDRESS", "127.0.0.1:3000",
"Bind address of the server.")
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".`)
cliflag.StringVarP(root.Flags(), &derpConfigURL, "derp-config-url", "", "CODER_DERP_CONFIG_URL", "",
"URL to fetch a DERP mapping on startup. See: https://tailscale.com/kb/1118/custom-derp-servers/")
cliflag.StringVarP(root.Flags(), &derpConfigPath, "derp-config-path", "", "CODER_DERP_CONFIG_PATH", "",
Expand Down
45 changes: 45 additions & 0 deletions coderd/authorize.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package coderd

import (
"fmt"
"net/http"

"golang.org/x/xerrors"

"cdr.dev/slog"
"github.com/coder/coder/coderd/httpapi"
"github.com/coder/coder/coderd/httpmw"
"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/codersdk"
)

func AuthorizeFilter[O rbac.Objecter](h *HTTPAuthorizer, r *http.Request, action rbac.Action, objects []O) ([]O, error) {
Expand Down Expand Up @@ -81,3 +84,45 @@ func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object r
}
return true
}

// checkAuthorization returns if the current API key can use the given
// permissions, factoring in the current user's roles and the API key scopes.
func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
auth := httpmw.UserAuthorization(r)

var params codersdk.AuthorizationRequest
if !httpapi.Read(ctx, rw, r, &params) {
return
}

api.Logger.Warn(ctx, "check-auth",
slog.F("my_id", httpmw.APIKey(r).UserID),
slog.F("got_id", auth.ID),
slog.F("name", auth.Username),
slog.F("roles", auth.Roles), slog.F("scope", auth.Scope),
)

response := make(codersdk.AuthorizationResponse)
for k, v := range params.Checks {
if v.Object.ResourceType == "" {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: fmt.Sprintf("Object's \"resource_type\" field must be defined for key %q.", k),
})
return
}

if v.Object.OwnerID == "me" {
v.Object.OwnerID = auth.ID.String()
}
err := api.Authorizer.ByRoleName(r.Context(), auth.ID.String(), auth.Roles, auth.Scope.ToRBAC(), rbac.Action(v.Action),
rbac.Object{
Owner: v.Object.OwnerID,
OrgID: v.Object.OrganizationID,
Type: v.Object.ResourceType,
})
response[k] = err == nil
}

httpapi.Write(ctx, rw, http.StatusOK, response)
}
124 changes: 124 additions & 0 deletions coderd/authorize_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
package coderd_test

import (
"context"
"testing"

"github.com/google/uuid"
"github.com/stretchr/testify/require"

"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/codersdk"
"github.com/coder/coder/testutil"
)

func TestCheckPermissions(t *testing.T) {
t.Parallel()

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

adminClient := coderdtest.New(t, nil)
// Create adminClient, member, and org adminClient
adminUser := coderdtest.CreateFirstUser(t, adminClient)
memberClient := coderdtest.CreateAnotherUser(t, adminClient, adminUser.OrganizationID)
memberUser, err := memberClient.User(ctx, codersdk.Me)
require.NoError(t, err)
orgAdminClient := coderdtest.CreateAnotherUser(t, adminClient, adminUser.OrganizationID, rbac.RoleOrgAdmin(adminUser.OrganizationID))
orgAdminUser, err := orgAdminClient.User(ctx, codersdk.Me)
require.NoError(t, err)

// With admin, member, and org admin
const (
readAllUsers = "read-all-users"
readOrgWorkspaces = "read-org-workspaces"
readMyself = "read-myself"
readOwnWorkspaces = "read-own-workspaces"
)
params := map[string]codersdk.AuthorizationCheck{
readAllUsers: {
Object: codersdk.AuthorizationObject{
ResourceType: "users",
},
Action: "read",
},
readMyself: {
Object: codersdk.AuthorizationObject{
ResourceType: "users",
OwnerID: "me",
},
Action: "read",
},
readOwnWorkspaces: {
Object: codersdk.AuthorizationObject{
ResourceType: "workspaces",
OwnerID: "me",
},
Action: "read",
},
readOrgWorkspaces: {
Object: codersdk.AuthorizationObject{
ResourceType: "workspaces",
OrganizationID: adminUser.OrganizationID.String(),
},
Action: "read",
},
}

testCases := []struct {
Name string
Client *codersdk.Client
UserID uuid.UUID
Check codersdk.AuthorizationResponse
}{
{
Name: "Admin",
Client: adminClient,
UserID: adminUser.UserID,
Check: map[string]bool{
readAllUsers: true,
readMyself: true,
readOwnWorkspaces: true,
readOrgWorkspaces: true,
},
},
{
Name: "OrgAdmin",
Client: orgAdminClient,
UserID: orgAdminUser.ID,
Check: map[string]bool{
readAllUsers: false,
readMyself: true,
readOwnWorkspaces: true,
readOrgWorkspaces: true,
},
},
{
Name: "Member",
Client: memberClient,
UserID: memberUser.ID,
Check: map[string]bool{
readAllUsers: false,
readMyself: true,
readOwnWorkspaces: true,
readOrgWorkspaces: false,
},
},
}

for _, c := range testCases {
c := c

t.Run("CheckAuthorization/"+c.Name, func(t *testing.T) {
t.Parallel()

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

resp, err := c.Client.CheckAuthorization(ctx, codersdk.AuthorizationRequest{Checks: params})
require.NoError(t, err, "check perms")
require.Equal(t, c.Check, resp)
})
}
}
67 changes: 48 additions & 19 deletions coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,12 @@ import (
// Options are requires parameters for Coder to start.
type Options struct {
AccessURL *url.URL
Logger slog.Logger
Database database.Store
Pubsub database.Pubsub
// AppHostname should be the wildcard hostname to use for workspace
// applications without the asterisk or leading dot. E.g. "apps.coder.com".
AppHostname string
Logger slog.Logger
Database database.Store
Pubsub database.Pubsub

// CacheDir is used for caching files served by the API.
CacheDir string
Expand Down Expand Up @@ -158,7 +161,20 @@ func New(options *Options) *API {
Github: options.GithubOAuth2Config,
OIDC: options.OIDCConfig,
}
apiKeyMiddleware := httpmw.ExtractAPIKey(options.Database, oauthConfigs, false)

apiKeyMiddleware := httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
DB: options.Database,
OAuth2Configs: oauthConfigs,
RedirectToLogin: false,
Optional: false,
})
// Same as above but it redirects to the login page.
apiKeyMiddlewareRedirect := httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
DB: options.Database,
OAuth2Configs: oauthConfigs,
RedirectToLogin: true,
Optional: false,
})

r.Use(
httpmw.AttachRequestID,
Expand All @@ -170,18 +186,14 @@ func New(options *Options) *API {
api.handleSubdomainApplications(
// Middleware to impose on the served application.
httpmw.RateLimitPerMinute(options.APIRateLimit),
httpmw.UseLoginURL(func() *url.URL {
if options.AccessURL == nil {
return nil
}

u := *options.AccessURL
u.Path = "/login"
return &u
}()),
// This should extract the application specific API key when we
// implement a scoped token.
httpmw.ExtractAPIKey(options.Database, oauthConfigs, true),
httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
DB: options.Database,
OAuth2Configs: oauthConfigs,
// The code handles the the case where the user is not
// authenticated automatically.
RedirectToLogin: false,
Optional: true,
}),
httpmw.ExtractUserParam(api.Database),
httpmw.ExtractWorkspaceAndAgentParam(api.Database),
),
Expand All @@ -199,7 +211,7 @@ func New(options *Options) *API {
r.Use(
tracing.Middleware(api.TracerProvider),
httpmw.RateLimitPerMinute(options.APIRateLimit),
httpmw.ExtractAPIKey(options.Database, oauthConfigs, true),
apiKeyMiddlewareRedirect,
httpmw.ExtractUserParam(api.Database),
// Extracts the <workspace.agent> from the url
httpmw.ExtractWorkspaceAndAgentParam(api.Database),
Expand Down Expand Up @@ -384,8 +396,6 @@ func New(options *Options) *API {
r.Put("/roles", api.putUserRoles)
r.Get("/roles", api.userRoles)

r.Post("/authorization", api.checkPermissions)

r.Route("/keys", func(r chi.Router) {
r.Post("/", api.postAPIKey)
r.Get("/{keyid}", api.apiKey)
Expand Down Expand Up @@ -481,6 +491,25 @@ func New(options *Options) *API {
r.Get("/resources", api.workspaceBuildResources)
r.Get("/state", api.workspaceBuildState)
})
r.Route("/authcheck", func(r chi.Router) {
r.Use(apiKeyMiddleware)
r.Post("/", api.checkAuthorization)
})
r.Route("/applications", func(r chi.Router) {
r.Route("/host", func(r chi.Router) {
// Don't leak the hostname to unauthenticated users.
r.Use(apiKeyMiddleware)
r.Get("/", api.appHost)
})
r.Route("/auth-redirect", func(r chi.Router) {
// We want to redirect to login if they are not authenticated.
r.Use(apiKeyMiddlewareRedirect)

// This is a GET request as it's redirected to by the subdomain app
// handler and the login page.
r.Get("/", api.workspaceApplicationAuth)
})
})
})

r.NotFound(compressHandler(http.HandlerFunc(api.siteHandler.ServeHTTP)).ServeHTTP)
Expand Down
7 changes: 5 additions & 2 deletions coderd/coderdtest/authorize.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
"POST:/api/v2/users/login": {NoAuthorize: true},
"GET:/api/v2/users/authmethods": {NoAuthorize: true},
"POST:/api/v2/csp/reports": {NoAuthorize: true},
// This is a dummy endpoint for compatibility.
"POST:/api/v2/authcheck": {NoAuthorize: true},
"GET:/api/v2/applications/host": {NoAuthorize: true},
// This is a dummy endpoint for compatibility with older CLI versions.
"GET:/api/v2/workspaceagents/{workspaceagent}/dial": {NoAuthorize: true},

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

// These endpoints need payloads to get to the auth part. Payloads will be required
"PUT:/api/v2/users/{user}/roles": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
Expand Down
2 changes: 2 additions & 0 deletions coderd/coderdtest/authorize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
func TestAuthorizeAllEndpoints(t *testing.T) {
t.Parallel()
client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{
// Required for any subdomain-based proxy tests to pass.
AppHostname: "test.coder.com",
Authorizer: &coderdtest.RecordingAuthorizer{},
IncludeProvisionerDaemon: true,
})
Expand Down
2 changes: 2 additions & 0 deletions coderd/coderdtest/coderdtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import (
)

type Options struct {
AppHostname string
AWSCertificates awsidentity.Certificates
Authorizer rbac.Authorizer
AzureCertificates x509.VerifyOptions
Expand Down Expand Up @@ -198,6 +199,7 @@ func NewOptions(t *testing.T, options *Options) (*httptest.Server, context.Cance
// agents are not marked as disconnected during slow tests.
AgentInactiveDisconnectTimeout: testutil.WaitShort,
AccessURL: serverURL,
AppHostname: options.AppHostname,
Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug),
CacheDir: t.TempDir(),
Database: db,
Expand Down
2 changes: 2 additions & 0 deletions coderd/database/dump.sql

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-- noop, comments don't need to be removed
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
COMMENT ON COLUMN api_keys.hashed_secret
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.';
Loading