diff --git a/Makefile b/Makefile index 09a93a38733ff..4de71e6e633da 100644 --- a/Makefile +++ b/Makefile @@ -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 diff --git a/cli/server.go b/cli/server.go index 8875a15480b0c..763e3438b3278 100644 --- a/cli/server.go +++ b/cli/server.go @@ -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 @@ -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, @@ -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", "", diff --git a/coderd/authorize.go b/coderd/authorize.go index 6183092c18e8f..0a6953cb1231e 100644 --- a/coderd/authorize.go +++ b/coderd/authorize.go @@ -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) { @@ -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, ¶ms) { + 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) +} diff --git a/coderd/authorize_test.go b/coderd/authorize_test.go new file mode 100644 index 0000000000000..49c0704f3bf63 --- /dev/null +++ b/coderd/authorize_test.go @@ -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) + }) + } +} diff --git a/coderd/coderd.go b/coderd/coderd.go index a595488687ca5..25ac1afec2f36 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -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 @@ -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, @@ -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), ), @@ -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 from the url httpmw.ExtractWorkspaceAndAgentParam(api.Database), @@ -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) @@ -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) diff --git a/coderd/coderdtest/authorize.go b/coderd/coderdtest/authorize.go index 331c105d13179..8c9441d13ced8 100644 --- a/coderd/coderdtest/authorize.go +++ b/coderd/coderdtest/authorize.go @@ -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 @@ -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}, diff --git a/coderd/coderdtest/authorize_test.go b/coderd/coderdtest/authorize_test.go index c8ef64065a290..4d8daa54c8e27 100644 --- a/coderd/coderdtest/authorize_test.go +++ b/coderd/coderdtest/authorize_test.go @@ -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, }) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index fd31cd55230b9..d1c3e390a3c93 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -65,6 +65,7 @@ import ( ) type Options struct { + AppHostname string AWSCertificates awsidentity.Certificates Authorizer rbac.Authorizer AzureCertificates x509.VerifyOptions @@ -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, diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 81557a9022d00..fa1227c7c7c5a 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -118,6 +118,8 @@ CREATE TABLE api_keys ( scope api_key_scope DEFAULT 'all'::public.api_key_scope NOT NULL ); +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.'; + CREATE TABLE audit_logs ( id uuid NOT NULL, "time" timestamp with time zone NOT NULL, diff --git a/coderd/database/migrations/000051_comment_on_api_key_secret.down.sql b/coderd/database/migrations/000051_comment_on_api_key_secret.down.sql new file mode 100644 index 0000000000000..fd94b6376392f --- /dev/null +++ b/coderd/database/migrations/000051_comment_on_api_key_secret.down.sql @@ -0,0 +1 @@ +-- noop, comments don't need to be removed diff --git a/coderd/database/migrations/000051_comment_on_api_key_secret.up.sql b/coderd/database/migrations/000051_comment_on_api_key_secret.up.sql new file mode 100644 index 0000000000000..c347cdfd9d1f7 --- /dev/null +++ b/coderd/database/migrations/000051_comment_on_api_key_secret.up.sql @@ -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.'; diff --git a/coderd/database/models.go b/coderd/database/models.go index b5d48bf6c0c32..871d6e7af8675 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -333,7 +333,8 @@ func (e *WorkspaceTransition) Scan(src interface{}) error { } type APIKey struct { - ID string `db:"id" json:"id"` + ID string `db:"id" json:"id"` + // 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. HashedSecret []byte `db:"hashed_secret" json:"hashed_secret"` UserID uuid.UUID `db:"user_id" json:"user_id"` LastUsed time.Time `db:"last_used" json:"last_used"` diff --git a/coderd/httpapi/url.go b/coderd/httpapi/url.go index a7780f2b3b2b9..ac7b81fd4269b 100644 --- a/coderd/httpapi/url.go +++ b/coderd/httpapi/url.go @@ -2,6 +2,7 @@ package httpapi import ( "fmt" + "net" "regexp" "strconv" "strings" @@ -21,15 +22,14 @@ var ( // SplitSubdomain splits a subdomain from the rest of the hostname. E.g.: // - "foo.bar.com" becomes "foo", "bar.com" // - "foo.bar.baz.com" becomes "foo", "bar.baz.com" -// -// An error is returned if the string doesn't contain a period. -func SplitSubdomain(hostname string) (subdomain string, rest string, err error) { +// - "foo" becomes "foo", "" +func SplitSubdomain(hostname string) (subdomain string, rest string) { toks := strings.SplitN(hostname, ".", 2) if len(toks) < 2 { - return "", "", xerrors.New("no subdomain") + return toks[0], "" } - return toks[0], toks[1], nil + return toks[0], toks[1] } // ApplicationURL is a parsed application URL hostname. @@ -40,35 +40,31 @@ type ApplicationURL struct { AgentName string WorkspaceName string Username string - // BaseHostname is the rest of the hostname minus the application URL part - // and the first dot. - BaseHostname string } -// String returns the application URL hostname without scheme. +// String returns the application URL hostname without scheme. You will likely +// want to append a period and the base hostname. func (a ApplicationURL) String() string { appNameOrPort := a.AppName if a.Port != 0 { appNameOrPort = strconv.Itoa(int(a.Port)) } - return fmt.Sprintf("%s--%s--%s--%s.%s", appNameOrPort, a.AgentName, a.WorkspaceName, a.Username, a.BaseHostname) + return fmt.Sprintf("%s--%s--%s--%s", appNameOrPort, a.AgentName, a.WorkspaceName, a.Username) } -// ParseSubdomainAppURL parses an ApplicationURL from the given hostname. If +// ParseSubdomainAppURL parses an ApplicationURL from the given subdomain. If // the subdomain is not a valid application URL hostname, returns a non-nil -// error. +// error. If the hostname is not a subdomain of the given base hostname, returns +// a non-nil error. +// +// The base hostname should not include a scheme, leading asterisk or dot. // // Subdomains should be in the form: // // {PORT/APP_NAME}--{AGENT_NAME}--{WORKSPACE_NAME}--{USERNAME} -// (eg. http://8080--main--dev--dean.hi.c8s.io) -func ParseSubdomainAppURL(hostname string) (ApplicationURL, error) { - subdomain, rest, err := SplitSubdomain(hostname) - if err != nil { - return ApplicationURL{}, xerrors.Errorf("split host domain %q: %w", hostname, err) - } - +// (eg. https://8080--main--dev--dean.hi.c8s.io) +func ParseSubdomainAppURL(subdomain string) (ApplicationURL, error) { matches := appURL.FindAllStringSubmatch(subdomain, -1) if len(matches) == 0 { return ApplicationURL{}, xerrors.Errorf("invalid application url format: %q", subdomain) @@ -82,7 +78,6 @@ func ParseSubdomainAppURL(hostname string) (ApplicationURL, error) { AgentName: matchGroup[appURL.SubexpIndex("AgentName")], WorkspaceName: matchGroup[appURL.SubexpIndex("WorkspaceName")], Username: matchGroup[appURL.SubexpIndex("Username")], - BaseHostname: rest, }, nil } @@ -98,3 +93,21 @@ func AppNameOrPort(val string) (string, uint16) { return val, uint16(port) } + +// HostnamesMatch returns true if the hostnames are equal, disregarding +// capitalization, extra leading or trailing periods, and ports. +func HostnamesMatch(a, b string) bool { + a = strings.Trim(a, ".") + b = strings.Trim(b, ".") + + aHost, _, err := net.SplitHostPort(a) + if err != nil { + aHost = a + } + bHost, _, err := net.SplitHostPort(b) + if err != nil { + bHost = b + } + + return strings.EqualFold(aHost, bHost) +} diff --git a/coderd/httpapi/url_test.go b/coderd/httpapi/url_test.go index 567074633e988..91d232ece2301 100644 --- a/coderd/httpapi/url_test.go +++ b/coderd/httpapi/url_test.go @@ -15,49 +15,42 @@ func TestSplitSubdomain(t *testing.T) { Host string ExpectedSubdomain string ExpectedRest string - ExpectedErr string }{ { Name: "Empty", Host: "", ExpectedSubdomain: "", ExpectedRest: "", - ExpectedErr: "no subdomain", }, { Name: "NoSubdomain", Host: "com", - ExpectedSubdomain: "", + ExpectedSubdomain: "com", ExpectedRest: "", - ExpectedErr: "no subdomain", }, { Name: "Domain", Host: "coder.com", ExpectedSubdomain: "coder", ExpectedRest: "com", - ExpectedErr: "", }, { Name: "Subdomain", Host: "subdomain.coder.com", ExpectedSubdomain: "subdomain", ExpectedRest: "coder.com", - ExpectedErr: "", }, { Name: "DoubleSubdomain", Host: "subdomain1.subdomain2.coder.com", ExpectedSubdomain: "subdomain1", ExpectedRest: "subdomain2.coder.com", - ExpectedErr: "", }, { Name: "WithPort", Host: "subdomain.coder.com:8080", ExpectedSubdomain: "subdomain", ExpectedRest: "coder.com:8080", - ExpectedErr: "", }, } @@ -66,13 +59,7 @@ func TestSplitSubdomain(t *testing.T) { t.Run(c.Name, func(t *testing.T) { t.Parallel() - subdomain, rest, err := httpapi.SplitSubdomain(c.Host) - if c.ExpectedErr != "" { - require.Error(t, err) - require.Contains(t, err.Error(), c.ExpectedErr) - } else { - require.NoError(t, err) - } + subdomain, rest := httpapi.SplitSubdomain(c.Host) require.Equal(t, c.ExpectedSubdomain, subdomain) require.Equal(t, c.ExpectedRest, rest) }) @@ -90,7 +77,7 @@ func TestApplicationURLString(t *testing.T) { { Name: "Empty", URL: httpapi.ApplicationURL{}, - Expected: "------.", + Expected: "------", }, { Name: "AppName", @@ -100,9 +87,8 @@ func TestApplicationURLString(t *testing.T) { AgentName: "agent", WorkspaceName: "workspace", Username: "user", - BaseHostname: "coder.com", }, - Expected: "app--agent--workspace--user.coder.com", + Expected: "app--agent--workspace--user", }, { Name: "Port", @@ -112,9 +98,8 @@ func TestApplicationURLString(t *testing.T) { AgentName: "agent", WorkspaceName: "workspace", Username: "user", - BaseHostname: "coder.com", }, - Expected: "8080--agent--workspace--user.coder.com", + Expected: "8080--agent--workspace--user", }, { Name: "Both", @@ -124,10 +109,9 @@ func TestApplicationURLString(t *testing.T) { AgentName: "agent", WorkspaceName: "workspace", Username: "user", - BaseHostname: "coder.com", }, // Prioritizes port over app name. - Expected: "8080--agent--workspace--user.coder.com", + Expected: "8080--agent--workspace--user", }, } @@ -145,93 +129,72 @@ func TestParseSubdomainAppURL(t *testing.T) { t.Parallel() testCases := []struct { Name string - Host string + Subdomain string Expected httpapi.ApplicationURL ExpectedError string }{ - { - Name: "Invalid_Split", - Host: "com", - Expected: httpapi.ApplicationURL{}, - ExpectedError: "no subdomain", - }, { Name: "Invalid_Empty", - Host: "example.com", + Subdomain: "test", Expected: httpapi.ApplicationURL{}, ExpectedError: "invalid application url format", }, { Name: "Invalid_Workspace.Agent--App", - Host: "workspace.agent--app.coder.com", + Subdomain: "workspace.agent--app", Expected: httpapi.ApplicationURL{}, ExpectedError: "invalid application url format", }, { Name: "Invalid_Workspace--App", - Host: "workspace--app.coder.com", + Subdomain: "workspace--app", Expected: httpapi.ApplicationURL{}, ExpectedError: "invalid application url format", }, { Name: "Invalid_App--Workspace--User", - Host: "app--workspace--user.coder.com", + Subdomain: "app--workspace--user", Expected: httpapi.ApplicationURL{}, ExpectedError: "invalid application url format", }, { Name: "Invalid_TooManyComponents", - Host: "1--2--3--4--5.coder.com", + Subdomain: "1--2--3--4--5", Expected: httpapi.ApplicationURL{}, ExpectedError: "invalid application url format", }, // Correct { - Name: "AppName--Agent--Workspace--User", - Host: "app--agent--workspace--user.coder.com", + Name: "AppName--Agent--Workspace--User", + Subdomain: "app--agent--workspace--user", Expected: httpapi.ApplicationURL{ AppName: "app", Port: 0, AgentName: "agent", WorkspaceName: "workspace", Username: "user", - BaseHostname: "coder.com", }, }, { - Name: "Port--Agent--Workspace--User", - Host: "8080--agent--workspace--user.coder.com", + Name: "Port--Agent--Workspace--User", + Subdomain: "8080--agent--workspace--user", Expected: httpapi.ApplicationURL{ AppName: "", Port: 8080, AgentName: "agent", WorkspaceName: "workspace", Username: "user", - BaseHostname: "coder.com", - }, - }, - { - Name: "DeepSubdomain", - Host: "app--agent--workspace--user.dev.dean-was-here.coder.com", - Expected: httpapi.ApplicationURL{ - AppName: "app", - Port: 0, - AgentName: "agent", - WorkspaceName: "workspace", - Username: "user", - BaseHostname: "dev.dean-was-here.coder.com", }, }, { - Name: "HyphenatedNames", - Host: "app-name--agent-name--workspace-name--user-name.coder.com", + Name: "HyphenatedNames", + Subdomain: "app-name--agent-name--workspace-name--user-name", Expected: httpapi.ApplicationURL{ AppName: "app-name", Port: 0, AgentName: "agent-name", WorkspaceName: "workspace-name", Username: "user-name", - BaseHostname: "coder.com", }, }, } @@ -241,7 +204,7 @@ func TestParseSubdomainAppURL(t *testing.T) { t.Run(c.Name, func(t *testing.T) { t.Parallel() - app, err := httpapi.ParseSubdomainAppURL(c.Host) + app, err := httpapi.ParseSubdomainAppURL(c.Subdomain) if c.ExpectedError == "" { require.NoError(t, err) require.Equal(t, c.Expected, app, "expected app") diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go index ddec7d092bde1..395204f1efc33 100644 --- a/coderd/httpmw/apikey.go +++ b/coderd/httpmw/apikey.go @@ -13,25 +13,38 @@ import ( "strings" "time" - "golang.org/x/oauth2" - "github.com/google/uuid" "github.com/tabbed/pqtype" + "golang.org/x/oauth2" + "golang.org/x/xerrors" "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/codersdk" ) +// The special cookie name used for subdomain-based application proxying. +// TODO: this will make dogfooding harder so come up with a more unique +// solution +// +//nolint:gosec +const DevURLSessionTokenCookie = "coder_devurl_session_token" + type apiKeyContextKey struct{} +// APIKeyOptional may return an API key from the ExtractAPIKey handler. +func APIKeyOptional(r *http.Request) (database.APIKey, bool) { + key, ok := r.Context().Value(apiKeyContextKey{}).(database.APIKey) + return key, ok +} + // APIKey returns the API key from the ExtractAPIKey handler. func APIKey(r *http.Request) database.APIKey { - apiKey, ok := r.Context().Value(apiKeyContextKey{}).(database.APIKey) + key, ok := APIKeyOptional(r) if !ok { - panic("developer error: apikey middleware not provided") + panic("developer error: ExtractAPIKey middleware not provided") } - return apiKey + return key } // User roles are the 'subject' field of Authorize() @@ -44,10 +57,17 @@ type Authorization struct { Scope database.APIKeyScope } +// UserAuthorizationOptional may return the roles and scope used for +// authorization. Depends on the ExtractAPIKey handler. +func UserAuthorizationOptional(r *http.Request) (Authorization, bool) { + auth, ok := r.Context().Value(userAuthKey{}).(Authorization) + return auth, ok +} + // UserAuthorization returns the roles and scope used for authorization. Depends // on the ExtractAPIKey handler. func UserAuthorization(r *http.Request) Authorization { - auth, ok := r.Context().Value(userAuthKey{}).(Authorization) + auth, ok := UserAuthorizationOptional(r) if !ok { panic("developer error: ExtractAPIKey middleware not provided") } @@ -66,63 +86,51 @@ const ( internalErrorMessage string = "An internal error occurred. Please try again or contact the system administrator." ) -type loginURLKey struct{} - -func getLoginURL(r *http.Request) (*url.URL, bool) { - val, ok := r.Context().Value(loginURLKey{}).(*url.URL) - return val, ok +type ExtractAPIKeyConfig struct { + DB database.Store + OAuth2Configs *OAuth2Configs + RedirectToLogin bool + + // Optional governs whether the API key is optional. Use this if you want to + // allow unauthenticated requests. + // + // If true and no session token is provided, nothing will be written to the + // request context. Use the APIKeyOptional and UserAuthorizationOptional + // functions to retrieve the API key and authorization instead of the + // regular ones. + // + // If true and the API key is invalid (i.e. deleted, expired), the cookie + // will be deleted and the request will continue. If the request is not a + // cookie-based request, the request will be rejected with a 401. + Optional bool } -// UseLoginURL sets the login URL to use for the request for handlers like -// ExtractAPIKey. -func UseLoginURL(loginURL *url.URL) func(http.Handler) http.Handler { - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { - ctx := context.WithValue(r.Context(), loginURLKey{}, loginURL) - next.ServeHTTP(rw, r.WithContext(ctx)) - }) - } -} - -// ExtractAPIKey requires authentication using a valid API key. -// It handles extending an API key if it comes close to expiry, -// updating the last used time in the database. +// ExtractAPIKey requires authentication using a valid API key. It handles +// extending an API key if it comes close to expiry, updating the last used time +// in the database. // nolint:revive -func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool) func(http.Handler) http.Handler { +func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() // Write wraps writing a response to redirect if the handler - // specified it should. This redirect is used for user-facing - // pages like workspace applications. + // specified it should. This redirect is used for user-facing pages + // like workspace applications. write := func(code int, response codersdk.Response) { - if redirectToLogin { - var ( - u = &url.URL{ - Path: "/login", - } - redirectURL = func() string { - path := r.URL.Path - if r.URL.RawQuery != "" { - path += "?" + r.URL.RawQuery - } - return path - }() - ) - if loginURL, ok := getLoginURL(r); ok { - u = loginURL - // Don't redirect to the current page, as it may be on - // a different domain and we have issues determining the - // scheme to redirect to. - redirectURL = "" + if cfg.RedirectToLogin { + path := r.URL.Path + if r.URL.RawQuery != "" { + path += "?" + r.URL.RawQuery } - q := r.URL.Query() + q := url.Values{} q.Add("message", response.Message) - if redirectURL != "" { - q.Add("redirect", redirectURL) + q.Add("redirect", path) + + u := &url.URL{ + Path: "/login", + RawQuery: q.Encode(), } - u.RawQuery = q.Encode() http.Redirect(rw, r, u.String(), http.StatusTemporaryRedirect) return @@ -131,44 +139,42 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool httpapi.Write(ctx, rw, code, response) } - cookieValue := apiTokenFromRequest(r) - if cookieValue == "" { - write(http.StatusUnauthorized, codersdk.Response{ - Message: signedOutErrorMessage, - Detail: fmt.Sprintf("Cookie %q or query parameter must be provided.", codersdk.SessionTokenKey), - }) - return - } - parts := strings.Split(cookieValue, "-") - // APIKeys are formatted: ID-SECRET - if len(parts) != 2 { - write(http.StatusUnauthorized, codersdk.Response{ - Message: signedOutErrorMessage, - Detail: fmt.Sprintf("Invalid %q cookie API key format.", codersdk.SessionTokenKey), - }) - return + // optionalWrite wraps write, but will pass the request on to the + // next handler if the configuration says the API key is optional. + // + // It should be used when the API key is not provided or is invalid, + // but not when there are other errors. + optionalWrite := func(code int, response codersdk.Response) { + if cfg.Optional { + next.ServeHTTP(rw, r) + return + } + + write(code, response) } - keyID := parts[0] - keySecret := parts[1] - // Ensuring key lengths are valid. - if len(keyID) != 10 { - write(http.StatusUnauthorized, codersdk.Response{ + + token := apiTokenFromRequest(r) + if token == "" { + optionalWrite(http.StatusUnauthorized, codersdk.Response{ Message: signedOutErrorMessage, - Detail: fmt.Sprintf("Invalid %q cookie API key id.", codersdk.SessionTokenKey), + Detail: fmt.Sprintf("Cookie %q or query parameter must be provided.", codersdk.SessionTokenKey), }) return } - if len(keySecret) != 22 { - write(http.StatusUnauthorized, codersdk.Response{ + + keyID, keySecret, err := SplitAPIToken(token) + if err != nil { + optionalWrite(http.StatusUnauthorized, codersdk.Response{ Message: signedOutErrorMessage, - Detail: fmt.Sprintf("Invalid %q cookie API key secret.", codersdk.SessionTokenKey), + Detail: "Invalid API key format: " + err.Error(), }) return } - key, err := db.GetAPIKeyByID(r.Context(), keyID) + + key, err := cfg.DB.GetAPIKeyByID(r.Context(), keyID) if err != nil { if errors.Is(err, sql.ErrNoRows) { - write(http.StatusUnauthorized, codersdk.Response{ + optionalWrite(http.StatusUnauthorized, codersdk.Response{ Message: signedOutErrorMessage, Detail: "API key is invalid.", }) @@ -180,23 +186,25 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool }) return } - hashed := sha256.Sum256([]byte(keySecret)) // Checking to see if the secret is valid. - if subtle.ConstantTimeCompare(key.HashedSecret, hashed[:]) != 1 { - write(http.StatusUnauthorized, codersdk.Response{ + hashedSecret := sha256.Sum256([]byte(keySecret)) + if subtle.ConstantTimeCompare(key.HashedSecret, hashedSecret[:]) != 1 { + optionalWrite(http.StatusUnauthorized, codersdk.Response{ Message: signedOutErrorMessage, Detail: "API key secret is invalid.", }) return } - now := database.Now() - // Tracks if the API key has properties updated! - changed := false - var link database.UserLink + var ( + link database.UserLink + now = database.Now() + // Tracks if the API key has properties updated + changed = false + ) if key.LoginType != database.LoginTypePassword { - link, err = db.GetUserLinkByUserIDLoginType(r.Context(), database.GetUserLinkByUserIDLoginTypeParams{ + link, err = cfg.DB.GetUserLinkByUserIDLoginType(r.Context(), database.GetUserLinkByUserIDLoginTypeParams{ UserID: key.UserID, LoginType: key.LoginType, }) @@ -207,14 +215,14 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool }) return } - // Check if the OAuth token is expired! + // Check if the OAuth token is expired if link.OAuthExpiry.Before(now) && !link.OAuthExpiry.IsZero() { var oauthConfig OAuth2Config switch key.LoginType { case database.LoginTypeGithub: - oauthConfig = oauth.Github + oauthConfig = cfg.OAuth2Configs.Github case database.LoginTypeOIDC: - oauthConfig = oauth.OIDC + oauthConfig = cfg.OAuth2Configs.OIDC default: write(http.StatusInternalServerError, codersdk.Response{ Message: internalErrorMessage, @@ -222,7 +230,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool }) return } - // If it is, let's refresh it from the provided config! + // If it is, let's refresh it from the provided config token, err := oauthConfig.TokenSource(r.Context(), &oauth2.Token{ AccessToken: link.OAuthAccessToken, RefreshToken: link.OAuthRefreshToken, @@ -245,7 +253,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool // Checking if the key is expired. if key.ExpiresAt.Before(now) { - write(http.StatusUnauthorized, codersdk.Response{ + optionalWrite(http.StatusUnauthorized, codersdk.Response{ Message: signedOutErrorMessage, Detail: fmt.Sprintf("API key expired at %q.", key.ExpiresAt.String()), }) @@ -278,7 +286,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool changed = true } if changed { - err := db.UpdateAPIKeyByID(r.Context(), database.UpdateAPIKeyByIDParams{ + err := cfg.DB.UpdateAPIKeyByID(r.Context(), database.UpdateAPIKeyByIDParams{ ID: key.ID, LastUsed: key.LastUsed, ExpiresAt: key.ExpiresAt, @@ -294,7 +302,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool // If the API Key is associated with a user_link (e.g. Github/OIDC) // then we want to update the relevant oauth fields. if link.UserID != uuid.Nil { - link, err = db.UpdateUserLink(r.Context(), database.UpdateUserLinkParams{ + link, err = cfg.DB.UpdateUserLink(r.Context(), database.UpdateUserLinkParams{ UserID: link.UserID, LoginType: link.LoginType, OAuthAccessToken: link.OAuthAccessToken, @@ -314,7 +322,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool // If the key is valid, we also fetch the user roles and status. // The roles are used for RBAC authorize checks, and the status // is to block 'suspended' users from accessing the platform. - roles, err := db.GetAuthorizationUserRoles(r.Context(), key.UserID) + roles, err := cfg.DB.GetAuthorizationUserRoles(r.Context(), key.UserID) if err != nil { write(http.StatusUnauthorized, codersdk.Response{ Message: internalErrorMessage, @@ -346,9 +354,10 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool // apiTokenFromRequest returns the api token from the request. // Find the session token from: // 1: The cookie -// 2: The old cookie -// 3. The coder_session_token query parameter -// 4. The custom auth header +// 1: The devurl cookie +// 3: The old cookie +// 4. The coder_session_token query parameter +// 5. The custom auth header func apiTokenFromRequest(r *http.Request) string { cookie, err := r.Cookie(codersdk.SessionTokenKey) if err == nil && cookie.Value != "" { @@ -373,5 +382,33 @@ func apiTokenFromRequest(r *http.Request) string { return headerValue } + cookie, err = r.Cookie(DevURLSessionTokenCookie) + if err == nil && cookie.Value != "" { + return cookie.Value + } + return "" } + +// SplitAPIToken verifies the format of an API key and returns the split ID and +// secret. +// +// APIKeys are formatted: ${ID}-${SECRET} +func SplitAPIToken(token string) (id string, secret string, err error) { + parts := strings.Split(token, "-") + if len(parts) != 2 { + return "", "", xerrors.Errorf("incorrect amount of API key parts, expected 2 got %d", len(parts)) + } + + // Ensure key lengths are valid. + keyID := parts[0] + keySecret := parts[1] + if len(keyID) != 10 { + return "", "", xerrors.Errorf("invalid API key ID length, expected 10 got %d", len(keyID)) + } + if len(keySecret) != 22 { + return "", "", xerrors.Errorf("invalid API key secret length, expected 22 got %d", len(keySecret)) + } + + return keyID, keySecret, nil +} diff --git a/coderd/httpmw/apikey_test.go b/coderd/httpmw/apikey_test.go index e7844d4549145..2ad9b8b5be922 100644 --- a/coderd/httpmw/apikey_test.go +++ b/coderd/httpmw/apikey_test.go @@ -7,6 +7,7 @@ import ( "net" "net/http" "net/http/httptest" + "sync/atomic" "testing" "time" @@ -46,7 +47,10 @@ func TestAPIKey(t *testing.T) { r = httptest.NewRequest("GET", "/", nil) rw = httptest.NewRecorder() ) - httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(successHandler).ServeHTTP(rw, r) res := rw.Result() defer res.Body.Close() require.Equal(t, http.StatusUnauthorized, res.StatusCode) @@ -59,7 +63,10 @@ func TestAPIKey(t *testing.T) { r = httptest.NewRequest("GET", "/", nil) rw = httptest.NewRecorder() ) - httpmw.ExtractAPIKey(db, nil, true)(successHandler).ServeHTTP(rw, r) + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: true, + })(successHandler).ServeHTTP(rw, r) res := rw.Result() defer res.Body.Close() location, err := res.Location() @@ -77,7 +84,10 @@ func TestAPIKey(t *testing.T) { ) r.Header.Set(codersdk.SessionCustomHeader, "test-wow-hello") - httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(successHandler).ServeHTTP(rw, r) res := rw.Result() defer res.Body.Close() require.Equal(t, http.StatusUnauthorized, res.StatusCode) @@ -92,7 +102,10 @@ func TestAPIKey(t *testing.T) { ) r.Header.Set(codersdk.SessionCustomHeader, "test-wow") - httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(successHandler).ServeHTTP(rw, r) res := rw.Result() defer res.Body.Close() require.Equal(t, http.StatusUnauthorized, res.StatusCode) @@ -107,7 +120,10 @@ func TestAPIKey(t *testing.T) { ) r.Header.Set(codersdk.SessionCustomHeader, "testtestid-wow") - httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(successHandler).ServeHTTP(rw, r) res := rw.Result() defer res.Body.Close() require.Equal(t, http.StatusUnauthorized, res.StatusCode) @@ -123,7 +139,10 @@ func TestAPIKey(t *testing.T) { ) r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret)) - httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(successHandler).ServeHTTP(rw, r) res := rw.Result() defer res.Body.Close() require.Equal(t, http.StatusUnauthorized, res.StatusCode) @@ -149,7 +168,10 @@ func TestAPIKey(t *testing.T) { Scope: database.APIKeyScopeAll, }) require.NoError(t, err) - httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(successHandler).ServeHTTP(rw, r) res := rw.Result() defer res.Body.Close() require.Equal(t, http.StatusUnauthorized, res.StatusCode) @@ -175,7 +197,10 @@ func TestAPIKey(t *testing.T) { Scope: database.APIKeyScopeAll, }) require.NoError(t, err) - httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(successHandler).ServeHTTP(rw, r) res := rw.Result() defer res.Body.Close() require.Equal(t, http.StatusUnauthorized, res.StatusCode) @@ -202,7 +227,10 @@ func TestAPIKey(t *testing.T) { Scope: database.APIKeyScopeAll, }) require.NoError(t, err) - httpmw.ExtractAPIKey(db, nil, false)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { // Checks that it exists on the context! _ = httpmw.APIKey(r) httpapi.Write(r.Context(), rw, http.StatusOK, codersdk.Response{ @@ -244,7 +272,10 @@ func TestAPIKey(t *testing.T) { }) require.NoError(t, err) - httpmw.ExtractAPIKey(db, nil, false)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { // Checks that it exists on the context! apiKey := httpmw.APIKey(r) assert.Equal(t, database.APIKeyScopeApplicationConnect, apiKey.Scope) @@ -282,7 +313,10 @@ func TestAPIKey(t *testing.T) { Scope: database.APIKeyScopeAll, }) require.NoError(t, err) - httpmw.ExtractAPIKey(db, nil, false)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { // Checks that it exists on the context! _ = httpmw.APIKey(r) httpapi.Write(r.Context(), rw, http.StatusOK, codersdk.Response{ @@ -316,7 +350,10 @@ func TestAPIKey(t *testing.T) { Scope: database.APIKeyScopeAll, }) require.NoError(t, err) - httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(successHandler).ServeHTTP(rw, r) res := rw.Result() defer res.Body.Close() require.Equal(t, http.StatusOK, res.StatusCode) @@ -350,7 +387,10 @@ func TestAPIKey(t *testing.T) { Scope: database.APIKeyScopeAll, }) require.NoError(t, err) - httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(successHandler).ServeHTTP(rw, r) res := rw.Result() defer res.Body.Close() require.Equal(t, http.StatusOK, res.StatusCode) @@ -391,7 +431,10 @@ func TestAPIKey(t *testing.T) { }) require.NoError(t, err) - httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(successHandler).ServeHTTP(rw, r) res := rw.Result() defer res.Body.Close() require.Equal(t, http.StatusOK, res.StatusCode) @@ -436,13 +479,17 @@ func TestAPIKey(t *testing.T) { RefreshToken: "moo", Expiry: database.Now().AddDate(0, 0, 1), } - httpmw.ExtractAPIKey(db, &httpmw.OAuth2Configs{ - Github: &oauth2Config{ - tokenSource: oauth2TokenSource(func() (*oauth2.Token, error) { - return token, nil - }), + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + OAuth2Configs: &httpmw.OAuth2Configs{ + Github: &oauth2Config{ + tokenSource: oauth2TokenSource(func() (*oauth2.Token, error) { + return token, nil + }), + }, }, - }, false)(successHandler).ServeHTTP(rw, r) + RedirectToLogin: false, + })(successHandler).ServeHTTP(rw, r) res := rw.Result() defer res.Body.Close() require.Equal(t, http.StatusOK, res.StatusCode) @@ -477,7 +524,10 @@ func TestAPIKey(t *testing.T) { Scope: database.APIKeyScopeAll, }) require.NoError(t, err) - httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r) + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(successHandler).ServeHTTP(rw, r) res := rw.Result() defer res.Body.Close() require.Equal(t, http.StatusOK, res.StatusCode) @@ -487,6 +537,58 @@ func TestAPIKey(t *testing.T) { require.Equal(t, net.ParseIP("1.1.1.1"), gotAPIKey.IPAddress.IPNet.IP) }) + + t.Run("RedirectToLogin", func(t *testing.T) { + t.Parallel() + var ( + db = databasefake.New() + r = httptest.NewRequest("GET", "/", nil) + rw = httptest.NewRecorder() + ) + + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: true, + })(successHandler).ServeHTTP(rw, r) + + res := rw.Result() + defer res.Body.Close() + require.Equal(t, http.StatusTemporaryRedirect, res.StatusCode) + u, err := res.Location() + require.NoError(t, err) + require.Equal(t, "/login", u.Path) + }) + + t.Run("Optional", func(t *testing.T) { + t.Parallel() + var ( + db = databasefake.New() + r = httptest.NewRequest("GET", "/", nil) + rw = httptest.NewRecorder() + + count int64 + handler = http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + atomic.AddInt64(&count, 1) + + apiKey, ok := httpmw.APIKeyOptional(r) + assert.False(t, ok) + assert.Zero(t, apiKey) + + rw.WriteHeader(http.StatusOK) + }) + ) + + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + Optional: true, + })(handler).ServeHTTP(rw, r) + + res := rw.Result() + defer res.Body.Close() + require.Equal(t, http.StatusOK, res.StatusCode) + require.EqualValues(t, 1, atomic.LoadInt64(&count)) + }) } func createUser(ctx context.Context, t *testing.T, db database.Store) database.User { diff --git a/coderd/httpmw/authorize_test.go b/coderd/httpmw/authorize_test.go index e1ac548092f8b..9f2d106895fb6 100644 --- a/coderd/httpmw/authorize_test.go +++ b/coderd/httpmw/authorize_test.go @@ -84,7 +84,11 @@ func TestExtractUserRoles(t *testing.T) { rtr = chi.NewRouter() ) rtr.Use( - httpmw.ExtractAPIKey(db, &httpmw.OAuth2Configs{}, false), + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + OAuth2Configs: &httpmw.OAuth2Configs{}, + RedirectToLogin: false, + }), ) rtr.Get("/", func(_ http.ResponseWriter, r *http.Request) { roles := httpmw.UserAuthorization(r) diff --git a/coderd/httpmw/organizationparam_test.go b/coderd/httpmw/organizationparam_test.go index 58816c3b783fe..faab86228f26f 100644 --- a/coderd/httpmw/organizationparam_test.go +++ b/coderd/httpmw/organizationparam_test.go @@ -67,7 +67,10 @@ func TestOrganizationParam(t *testing.T) { rtr = chi.NewRouter() ) rtr.Use( - httpmw.ExtractAPIKey(db, nil, false), + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + }), httpmw.ExtractOrganizationParam(db), ) rtr.Get("/", nil) @@ -87,7 +90,10 @@ func TestOrganizationParam(t *testing.T) { ) chi.RouteContext(r.Context()).URLParams.Add("organization", uuid.NewString()) rtr.Use( - httpmw.ExtractAPIKey(db, nil, false), + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + }), httpmw.ExtractOrganizationParam(db), ) rtr.Get("/", nil) @@ -107,7 +113,10 @@ func TestOrganizationParam(t *testing.T) { ) chi.RouteContext(r.Context()).URLParams.Add("organization", "not-a-uuid") rtr.Use( - httpmw.ExtractAPIKey(db, nil, false), + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + }), httpmw.ExtractOrganizationParam(db), ) rtr.Get("/", nil) @@ -135,7 +144,10 @@ func TestOrganizationParam(t *testing.T) { chi.RouteContext(r.Context()).URLParams.Add("organization", organization.ID.String()) chi.RouteContext(r.Context()).URLParams.Add("user", u.ID.String()) rtr.Use( - httpmw.ExtractAPIKey(db, nil, false), + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + }), httpmw.ExtractUserParam(db), httpmw.ExtractOrganizationParam(db), httpmw.ExtractOrganizationMemberParam(db), @@ -172,7 +184,10 @@ func TestOrganizationParam(t *testing.T) { chi.RouteContext(r.Context()).URLParams.Add("organization", organization.ID.String()) chi.RouteContext(r.Context()).URLParams.Add("user", user.ID.String()) rtr.Use( - httpmw.ExtractAPIKey(db, nil, false), + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + }), httpmw.ExtractOrganizationParam(db), httpmw.ExtractUserParam(db), httpmw.ExtractOrganizationMemberParam(db), diff --git a/coderd/httpmw/templateparam_test.go b/coderd/httpmw/templateparam_test.go index 7cef1dd7801af..15d175c4a99af 100644 --- a/coderd/httpmw/templateparam_test.go +++ b/coderd/httpmw/templateparam_test.go @@ -132,7 +132,10 @@ func TestTemplateParam(t *testing.T) { db := databasefake.New() rtr := chi.NewRouter() rtr.Use( - httpmw.ExtractAPIKey(db, nil, false), + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + }), httpmw.ExtractTemplateParam(db), httpmw.ExtractOrganizationParam(db), ) diff --git a/coderd/httpmw/templateversionparam_test.go b/coderd/httpmw/templateversionparam_test.go index 1eefc677f542f..b5a5ba2b6f6ca 100644 --- a/coderd/httpmw/templateversionparam_test.go +++ b/coderd/httpmw/templateversionparam_test.go @@ -124,7 +124,10 @@ func TestTemplateVersionParam(t *testing.T) { db := databasefake.New() rtr := chi.NewRouter() rtr.Use( - httpmw.ExtractAPIKey(db, nil, false), + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + }), httpmw.ExtractTemplateVersionParam(db), httpmw.ExtractOrganizationParam(db), ) diff --git a/coderd/httpmw/userparam_test.go b/coderd/httpmw/userparam_test.go index fbb66af173ba8..edd7faf128124 100644 --- a/coderd/httpmw/userparam_test.go +++ b/coderd/httpmw/userparam_test.go @@ -56,7 +56,10 @@ func TestUserParam(t *testing.T) { t.Parallel() db, rw, r := setup(t) - httpmw.ExtractAPIKey(db, nil, false)(http.HandlerFunc(func(rw http.ResponseWriter, returnedRequest *http.Request) { + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(http.HandlerFunc(func(rw http.ResponseWriter, returnedRequest *http.Request) { r = returnedRequest })).ServeHTTP(rw, r) @@ -72,7 +75,10 @@ func TestUserParam(t *testing.T) { t.Parallel() db, rw, r := setup(t) - httpmw.ExtractAPIKey(db, nil, false)(http.HandlerFunc(func(rw http.ResponseWriter, returnedRequest *http.Request) { + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(http.HandlerFunc(func(rw http.ResponseWriter, returnedRequest *http.Request) { r = returnedRequest })).ServeHTTP(rw, r) @@ -91,7 +97,10 @@ func TestUserParam(t *testing.T) { t.Parallel() db, rw, r := setup(t) - httpmw.ExtractAPIKey(db, nil, false)(http.HandlerFunc(func(rw http.ResponseWriter, returnedRequest *http.Request) { + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(http.HandlerFunc(func(rw http.ResponseWriter, returnedRequest *http.Request) { r = returnedRequest })).ServeHTTP(rw, r) diff --git a/coderd/httpmw/workspaceagentparam_test.go b/coderd/httpmw/workspaceagentparam_test.go index ae22582d615b4..9c2af43b0fe1a 100644 --- a/coderd/httpmw/workspaceagentparam_test.go +++ b/coderd/httpmw/workspaceagentparam_test.go @@ -132,7 +132,10 @@ func TestWorkspaceAgentParam(t *testing.T) { db := databasefake.New() rtr := chi.NewRouter() rtr.Use( - httpmw.ExtractAPIKey(db, nil, false), + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + }), httpmw.ExtractWorkspaceAgentParam(db), ) rtr.Get("/", func(rw http.ResponseWriter, r *http.Request) { diff --git a/coderd/httpmw/workspacebuildparam_test.go b/coderd/httpmw/workspacebuildparam_test.go index 97cf08b16122e..851109af0e82c 100644 --- a/coderd/httpmw/workspacebuildparam_test.go +++ b/coderd/httpmw/workspacebuildparam_test.go @@ -107,7 +107,10 @@ func TestWorkspaceBuildParam(t *testing.T) { db := databasefake.New() rtr := chi.NewRouter() rtr.Use( - httpmw.ExtractAPIKey(db, nil, false), + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + }), httpmw.ExtractWorkspaceBuildParam(db), httpmw.ExtractWorkspaceParam(db), ) diff --git a/coderd/httpmw/workspaceparam_test.go b/coderd/httpmw/workspaceparam_test.go index 5c36363f6a1b4..bc040b98bcf0f 100644 --- a/coderd/httpmw/workspaceparam_test.go +++ b/coderd/httpmw/workspaceparam_test.go @@ -100,7 +100,10 @@ func TestWorkspaceParam(t *testing.T) { db := databasefake.New() rtr := chi.NewRouter() rtr.Use( - httpmw.ExtractAPIKey(db, nil, false), + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + }), httpmw.ExtractWorkspaceParam(db), ) rtr.Get("/", func(rw http.ResponseWriter, r *http.Request) { @@ -298,7 +301,10 @@ func TestWorkspaceAgentByNameParam(t *testing.T) { rtr := chi.NewRouter() rtr.Use( - httpmw.ExtractAPIKey(db, nil, true), + httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: true, + }), httpmw.ExtractUserParam(db), httpmw.ExtractWorkspaceAndAgentParam(db), ) diff --git a/coderd/roles.go b/coderd/roles.go index 48e60a596a2bf..4e5ae05d99470 100644 --- a/coderd/roles.go +++ b/coderd/roles.go @@ -1,7 +1,6 @@ package coderd import ( - "fmt" "net/http" "github.com/coder/coder/coderd/httpmw" @@ -39,52 +38,6 @@ func (api *API) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Roles, roles)) } -func (api *API) checkPermissions(rw http.ResponseWriter, r *http.Request) { - ctx := r.Context() - user := httpmw.UserParam(r) - apiKey := httpmw.APIKey(r) - - if !api.Authorize(r, rbac.ActionRead, rbac.ResourceUser) { - httpapi.ResourceNotFound(rw) - return - } - - // use the roles of the user specified, not the person making the request. - roles, err := api.Database.GetAuthorizationUserRoles(ctx, user.ID) - if err != nil { - httpapi.Forbidden(rw) - return - } - - var params codersdk.UserAuthorizationRequest - if !httpapi.Read(ctx, rw, r, ¶ms) { - return - } - - response := make(codersdk.UserAuthorizationResponse) - 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 = roles.ID.String() - } - err := api.Authorizer.ByRoleName(ctx, roles.ID.String(), roles.Roles, apiKey.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) -} - func convertRole(role rbac.Role) codersdk.Role { return codersdk.Role{ DisplayName: role.DisplayName, diff --git a/coderd/roles_test.go b/coderd/roles_test.go index 27b5bc9df6fcf..99496bec963f1 100644 --- a/coderd/roles_test.go +++ b/coderd/roles_test.go @@ -13,95 +13,6 @@ import ( "github.com/coder/coder/testutil" ) -func TestAuthorization(t *testing.T) { - t.Parallel() - - client := coderdtest.New(t, nil) - // Create admin, member, and org admin - admin := coderdtest.CreateFirstUser(t, client) - member := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID) - orgAdmin := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID, rbac.RoleOrgAdmin(admin.OrganizationID)) - - // With admin, member, and org admin - const ( - allUsers = "read-all-users" - readOrgWorkspaces = "read-org-workspaces" - myself = "read-myself" - myWorkspace = "read-my-workspace" - ) - params := map[string]codersdk.UserAuthorization{ - allUsers: { - Object: codersdk.UserAuthorizationObject{ - ResourceType: "users", - }, - Action: "read", - }, - myself: { - Object: codersdk.UserAuthorizationObject{ - ResourceType: "users", - OwnerID: "me", - }, - Action: "read", - }, - myWorkspace: { - Object: codersdk.UserAuthorizationObject{ - ResourceType: "workspaces", - OwnerID: "me", - }, - Action: "read", - }, - readOrgWorkspaces: { - Object: codersdk.UserAuthorizationObject{ - ResourceType: "workspaces", - OrganizationID: admin.OrganizationID.String(), - }, - Action: "read", - }, - } - - testCases := []struct { - Name string - Client *codersdk.Client - Check codersdk.UserAuthorizationResponse - }{ - { - Name: "Admin", - Client: client, - Check: map[string]bool{ - allUsers: true, myself: true, myWorkspace: true, readOrgWorkspaces: true, - }, - }, - { - Name: "Member", - Client: member, - Check: map[string]bool{ - allUsers: false, myself: true, myWorkspace: true, readOrgWorkspaces: false, - }, - }, - { - Name: "OrgAdmin", - Client: orgAdmin, - Check: map[string]bool{ - allUsers: false, myself: true, myWorkspace: true, readOrgWorkspaces: true, - }, - }, - } - - for _, c := range testCases { - c := c - t.Run(c.Name, func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - resp, err := c.Client.CheckPermissions(ctx, codersdk.UserAuthorizationRequest{Checks: params}) - require.NoError(t, err, "check perms") - require.Equal(t, resp, c.Check) - }) - } -} - func TestListRoles(t *testing.T) { t.Parallel() diff --git a/coderd/userauth.go b/coderd/userauth.go index f00a2a8f6b642..8a80e92465ef4 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -162,7 +162,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { return } - api.setAuthCookie(rw, cookie) + http.SetCookie(rw, cookie) redirect := state.Redirect if redirect == "" { @@ -296,7 +296,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { return } - api.setAuthCookie(rw, cookie) + http.SetCookie(rw, cookie) redirect := state.Redirect if redirect == "" { diff --git a/coderd/users.go b/coderd/users.go index 0e75c45cdf341..5441bbafa23fc 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -936,7 +936,7 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { return } - api.setAuthCookie(rw, cookie) + http.SetCookie(rw, cookie) httpapi.Write(ctx, rw, http.StatusCreated, codersdk.LoginWithPasswordResponse{ SessionToken: cookie.Value, @@ -1016,7 +1016,7 @@ func (api *API) postLogout(rw http.ResponseWriter, r *http.Request) { Name: codersdk.SessionTokenKey, Path: "/", } - api.setAuthCookie(rw, cookie) + http.SetCookie(rw, cookie) // Delete the session token from database. apiKey := httpmw.APIKey(r) @@ -1057,6 +1057,7 @@ type createAPIKeyParams struct { // Optional. ExpiresAt time.Time LifetimeSeconds int64 + Scope database.APIKeyScope } func (api *API) createAPIKey(ctx context.Context, params createAPIKeyParams) (*http.Cookie, error) { @@ -1081,6 +1082,12 @@ func (api *API) createAPIKey(ctx context.Context, params createAPIKeyParams) (*h ip = net.IPv4(0, 0, 0, 0) } bitlen := len(ip) * 8 + + scope := database.APIKeyScopeAll + if params.Scope != "" { + scope = params.Scope + } + key, err := api.Database.InsertAPIKey(ctx, database.InsertAPIKeyParams{ ID: keyID, UserID: params.UserID, @@ -1098,7 +1105,7 @@ func (api *API) createAPIKey(ctx context.Context, params createAPIKeyParams) (*h UpdatedAt: database.Now(), HashedSecret: hashed[:], LoginType: params.LoginType, - Scope: database.APIKeyScopeAll, + Scope: scope, }) if err != nil { return nil, xerrors.Errorf("insert API key: %w", err) @@ -1198,15 +1205,6 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create }) } -func (api *API) setAuthCookie(rw http.ResponseWriter, cookie *http.Cookie) { - http.SetCookie(rw, cookie) - - appCookie := api.applicationCookie(cookie) - if appCookie != nil { - http.SetCookie(rw, appCookie) - } -} - func convertUser(user database.User, organizationIDs []uuid.UUID) codersdk.User { convertedUser := codersdk.User{ ID: user.ID, diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index a73dc184b8aa2..9799966a6b562 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -1,15 +1,22 @@ package coderd import ( + "bytes" + "context" + "crypto/sha256" + "encoding/base64" + "encoding/json" "fmt" - "net" "net/http" "net/http/httputil" "net/url" "strings" + "time" "github.com/go-chi/chi/v5" "go.opentelemetry.io/otel/trace" + "golang.org/x/xerrors" + jose "gopkg.in/square/go-jose.v2" "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" @@ -20,6 +27,22 @@ import ( "github.com/coder/coder/site" ) +const ( + // This needs to be a super unique query parameter because we don't want to + // conflict with query parameters that users may use. + // TODO: this will make dogfooding harder so come up with a more unique + // solution + //nolint:gosec + subdomainProxyAPIKeyParam = "coder_application_connect_api_key_35e783" + redirectURIQueryParam = "redirect_uri" +) + +func (api *API) appHost(rw http.ResponseWriter, r *http.Request) { + httpapi.Write(r.Context(), rw, http.StatusOK, codersdk.GetAppHostResponse{ + Host: api.AppHostname, + }) +} + // workspaceAppsProxyPath proxies requests to a workspace application // through a relative URL path. func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) { @@ -50,18 +73,61 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) }, rw, r) } +// handleSubdomainApplications handles subdomain-based application proxy +// requests (aka. DevURLs in Coder V1). +// +// There are a lot of paths here: +// 1. If api.AppHostname is not set then we pass on. +// 2. If we can't read the request hostname then we return a 400. +// 3. If the request hostname matches api.AccessURL then we pass on. +// 5. We split the subdomain into the subdomain and the "rest". If there are no +// periods in the hostname then we pass on. +// 5. We parse the subdomain into a httpapi.ApplicationURL struct. If we +// encounter an error: +// a. If the "rest" does not match api.AppHostname then we pass on; +// b. Otherwise, we return a 400. +// 6. Finally, we verify that the "rest" matches api.AppHostname, else we +// return a 404. +// +// Rationales for each of the above steps: +// 1. We pass on if api.AppHostname is not set to avoid returning any errors if +// `--app-hostname` is not configured. +// 2. Every request should have a valid Host header anyways. +// 3. We pass on if the request hostname matches api.AccessURL so we can +// support having the access URL be at the same level as the application +// base hostname. +// 4. We pass on if there are no periods in the hostname as application URLs +// must be a subdomain of a hostname, which implies there must be at least +// one period. +// 5. a. If the request subdomain is not a valid application URL, and the +// "rest" does not match api.AppHostname, then it is very unlikely that +// the request was intended for this handler. We pass on. +// b. If the request subdomain is not a valid application URL, but the +// "rest" matches api.AppHostname, then we return a 400 because the +// request is probably a typo or something. +// 6. We finally verify that the "rest" matches api.AppHostname for security +// purposes regarding re-authentication and application proxy session +// tokens. func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) http.Handler) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() + // Step 1: Pass on if subdomain-based application proxying is not + // configured. + if api.AppHostname == "" { + next.ServeHTTP(rw, r) + return + } + + // Step 2: Get the request Host. host := httpapi.RequestHost(r) if host == "" { if r.URL.Path == "/derp" { // The /derp endpoint is used by wireguard clients to tunnel // through coderd. For some reason these requests don't set - // a Host header properly sometimes (no idea how), which - // causes this path to get hit. + // a Host header properly sometimes in tests (no idea how), + // which causes this path to get hit. next.ServeHTTP(rw, r) return } @@ -72,15 +138,9 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht return } - app, err := httpapi.ParseSubdomainAppURL(host) - if err != nil { - // Subdomain is not a valid application url. Pass through to the - // rest of the app. - // TODO: @emyrk we should probably catch invalid subdomains. Meaning - // an invalid application should not route to the coderd. - // To do this we would need to know the list of valid access urls - // though? - next.ServeHTTP(rw, r) + // Steps 3-6: Parse application from subdomain. + app, ok := api.parseWorkspaceApplicationHostname(rw, r, next, host) + if !ok { return } @@ -95,6 +155,12 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht workspace := httpmw.WorkspaceParam(r) agent := httpmw.WorkspaceAgentParam(r) + // Verify application auth. This function will redirect or + // return an error page if the user doesn't have permission. + if !api.verifyWorkspaceApplicationAuth(rw, r, workspace, host) { + return + } + api.proxyWorkspaceApplication(proxyApplication{ Workspace: workspace, Agent: agent, @@ -108,6 +174,235 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht } } +func (api *API) parseWorkspaceApplicationHostname(rw http.ResponseWriter, r *http.Request, next http.Handler, host string) (httpapi.ApplicationURL, bool) { + ctx := r.Context() + // Check if the hostname matches the access URL. If it does, the + // user was definitely trying to connect to the dashboard/API. + if httpapi.HostnamesMatch(api.AccessURL.Hostname(), host) { + next.ServeHTTP(rw, r) + return httpapi.ApplicationURL{}, false + } + + // Split the subdomain so we can parse the application details and + // verify it matches the configured app hostname later. + subdomain, rest := httpapi.SplitSubdomain(host) + if rest == "" { + // If there are no periods in the hostname, then it can't be a + // valid application URL. + next.ServeHTTP(rw, r) + return httpapi.ApplicationURL{}, false + } + matchingBaseHostname := httpapi.HostnamesMatch(api.AppHostname, rest) + + // Parse the application URL from the subdomain. + app, err := httpapi.ParseSubdomainAppURL(subdomain) + if err != nil { + // If it isn't a valid app URL and the base domain doesn't match + // the configured app hostname, this request was probably + // destined for the dashboard/API router. + if !matchingBaseHostname { + next.ServeHTTP(rw, r) + return httpapi.ApplicationURL{}, false + } + + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Could not parse subdomain application URL.", + Detail: err.Error(), + }) + return httpapi.ApplicationURL{}, false + } + + // At this point we've verified that the subdomain looks like a + // valid application URL, so the base hostname should match the + // configured app hostname. + if !matchingBaseHostname { + httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ + Message: "The server does not accept application requests on this hostname.", + }) + return httpapi.ApplicationURL{}, false + } + + return app, true +} + +// verifyWorkspaceApplicationAuth checks that the request is authorized to +// access the given application. If the user does not have a app session key, +// they will be redirected to the route below. If the user does have a session +// key but insufficient permissions a static error page will be rendered. +func (api *API) verifyWorkspaceApplicationAuth(rw http.ResponseWriter, r *http.Request, workspace database.Workspace, host string) bool { + ctx := r.Context() + _, ok := httpmw.APIKeyOptional(r) + if ok { + if !api.Authorize(r, rbac.ActionCreate, workspace.ApplicationConnectRBAC()) { + // TODO: This should be a static error page. + httpapi.ResourceNotFound(rw) + return false + } + + // Request should be all good to go! + return true + } + + // If the request has the special query param then we need to set a cookie + // and strip that query parameter. + if encryptedAPIKey := r.URL.Query().Get(subdomainProxyAPIKeyParam); encryptedAPIKey != "" { + // Exchange the encoded API key for a real one. + _, apiKey, err := decryptAPIKey(r.Context(), api.Database, encryptedAPIKey) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Could not decrypt API key. Please remove the query parameter and try again.", + Detail: err.Error(), + }) + return false + } + + // Set the app cookie for all subdomains of api.AppHostname. This cookie + // is handled properly by the ExtractAPIKey middleware. + http.SetCookie(rw, &http.Cookie{ + Name: httpmw.DevURLSessionTokenCookie, + Value: apiKey, + Domain: "." + api.AppHostname, + Path: "/", + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + Secure: api.SecureAuthCookie, + }) + + // Strip the query parameter. + path := r.URL.Path + if path == "" { + path = "/" + } + q := r.URL.Query() + q.Del(subdomainProxyAPIKeyParam) + rawQuery := q.Encode() + if rawQuery != "" { + path += "?" + q.Encode() + } + + http.Redirect(rw, r, path, http.StatusTemporaryRedirect) + return false + } + + // If the user doesn't have a session key, redirect them to the API endpoint + // for application auth. + redirectURI := *r.URL + redirectURI.Scheme = api.AccessURL.Scheme + redirectURI.Host = host + + u := *api.AccessURL + u.Path = "/api/v2/applications/auth-redirect" + q := u.Query() + q.Add(redirectURIQueryParam, redirectURI.String()) + u.RawQuery = q.Encode() + + http.Redirect(rw, r, u.String(), http.StatusTemporaryRedirect) + return false +} + +// workspaceApplicationAuth is an endpoint on the main router that handles +// redirects from the subdomain handler. +func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + if api.AppHostname == "" { + httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ + Message: "The server does not accept subdomain-based application requests.", + }) + return + } + + apiKey := httpmw.APIKey(r) + if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceAPIKey.WithOwner(apiKey.UserID.String())) { + httpapi.ResourceNotFound(rw) + return + } + + // Get the redirect URI from the query parameters and parse it. + redirectURI := r.URL.Query().Get(redirectURIQueryParam) + if redirectURI == "" { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Missing redirect_uri query parameter.", + }) + return + } + u, err := url.Parse(redirectURI) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Invalid redirect_uri query parameter.", + Detail: err.Error(), + }) + return + } + // Force the redirect URI to use the same scheme as the access URL for + // security purposes. + u.Scheme = api.AccessURL.Scheme + + // Ensure that the redirect URI is a subdomain of api.AppHostname and is a + // valid app subdomain. + subdomain, rest := httpapi.SplitSubdomain(u.Hostname()) + if !httpapi.HostnamesMatch(api.AppHostname, rest) { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "The redirect_uri query parameter must be a valid app subdomain.", + }) + return + } + _, err = httpapi.ParseSubdomainAppURL(subdomain) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "The redirect_uri query parameter must be a valid app subdomain.", + Detail: err.Error(), + }) + return + } + + // Create the application_connect-scoped API key with the same lifetime as + // the current session (defaulting to 1 day, capped to 1 week). + exp := apiKey.ExpiresAt + if exp.IsZero() { + exp = database.Now().Add(time.Hour * 24) + } + if time.Until(exp) > time.Hour*24*7 { + exp = database.Now().Add(time.Hour * 24 * 7) + } + lifetime := apiKey.LifetimeSeconds + if lifetime > int64((time.Hour * 24 * 7).Seconds()) { + lifetime = int64((time.Hour * 24 * 7).Seconds()) + } + cookie, err := api.createAPIKey(ctx, createAPIKeyParams{ + UserID: apiKey.UserID, + LoginType: database.LoginTypePassword, + ExpiresAt: exp, + LifetimeSeconds: lifetime, + Scope: database.APIKeyScopeApplicationConnect, + }) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to create API key.", + Detail: err.Error(), + }) + return + } + + // Encrypt the API key. + encryptedAPIKey, err := encryptAPIKey(encryptedAPIKeyPayload{ + APIKey: cookie.Value, + }) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to encrypt API key.", + Detail: err.Error(), + }) + return + } + + // Redirect to the redirect URI with the encrypted API key in the query + // parameters. + q := u.Query() + q.Set(subdomainProxyAPIKeyParam, encryptedAPIKey) + u.RawQuery = q.Encode() + http.Redirect(rw, r, u.String(), http.StatusTemporaryRedirect) +} + // proxyApplication are the required fields to proxy a workspace application. type proxyApplication struct { Workspace database.Workspace @@ -235,22 +530,121 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res proxy.ServeHTTP(rw, r) } -// applicationCookie is a helper function to copy the auth cookie to also -// support subdomains. Until we support creating authentication cookies that can -// only do application authentication, we will just reuse the original token. -// This code should be temporary and be replaced with something that creates -// a unique session_token. +type encryptedAPIKeyPayload struct { + APIKey string `json:"api_key"` + ExpiresAt time.Time `json:"expires_at"` +} + +// encryptAPIKey encrypts an API key with it's own hashed secret. This is used +// for smuggling (application_connect scoped) API keys securely to app +// hostnames. // -// Returns nil if the access URL isn't a hostname. -func (api *API) applicationCookie(authCookie *http.Cookie) *http.Cookie { - if net.ParseIP(api.AccessURL.Hostname()) != nil { - return nil - } - - appCookie := *authCookie - // We only support setting this cookie on the access URL subdomains. This is - // to ensure we don't accidentally leak the auth cookie to subdomains on - // another hostname. - appCookie.Domain = "." + api.AccessURL.Hostname() - return &appCookie +// We encrypt API keys when smuggling them in query parameters to avoid them +// getting accidentally logged in access logs or stored in browser history. +func encryptAPIKey(data encryptedAPIKeyPayload) (string, error) { + if data.APIKey == "" { + return "", xerrors.New("API key is empty") + } + if data.ExpiresAt.IsZero() { + // Very short expiry as these keys are only used once as part of an + // automatic redirection flow. + data.ExpiresAt = database.Now().Add(time.Minute) + } + + payload, err := json.Marshal(data) + if err != nil { + return "", xerrors.Errorf("marshal payload: %w", err) + } + + // We use the hashed key secret as the encryption key. The hashed secret is + // stored in the API keys table. The HashedSecret is NEVER returned from the + // API. + // + // We chose to use the key secret as the private key for encryption instead + // of a shared key for a few reasons: + // 1. A single private key used to encrypt every API key would also be + // stored in the database, which means that the risk factor is similar. + // 2. The secret essentially rotates for each key (for free!), since each + // key has a different secret. This means that if someone acquires an + // old database dump they can't decrypt new API keys. + // 3. These tokens are scoped only for application_connect access. + keyID, keySecret, err := httpmw.SplitAPIToken(data.APIKey) + if err != nil { + return "", xerrors.Errorf("split API key: %w", err) + } + // SHA256 the key secret so it matches the hashed secret in the database. + // The key length doesn't matter to the jose.Encrypter. + privateKey := sha256.Sum256([]byte(keySecret)) + + // JWEs seem to apply a nonce themselves. + encrypter, err := jose.NewEncrypter( + jose.A256GCM, + jose.Recipient{ + Algorithm: jose.A256GCMKW, + KeyID: keyID, + Key: privateKey[:], + }, + &jose.EncrypterOptions{ + Compression: jose.DEFLATE, + }, + ) + if err != nil { + return "", xerrors.Errorf("initializer jose encrypter: %w", err) + } + encryptedObject, err := encrypter.Encrypt(payload) + if err != nil { + return "", xerrors.Errorf("encrypt jwe: %w", err) + } + + encrypted := encryptedObject.FullSerialize() + return base64.RawURLEncoding.EncodeToString([]byte(encrypted)), nil +} + +// decryptAPIKey undoes encryptAPIKey and is used in the subdomain app handler. +func decryptAPIKey(ctx context.Context, db database.Store, encryptedAPIKey string) (database.APIKey, string, error) { + encrypted, err := base64.RawURLEncoding.DecodeString(encryptedAPIKey) + if err != nil { + return database.APIKey{}, "", xerrors.Errorf("base64 decode encrypted API key: %w", err) + } + + object, err := jose.ParseEncrypted(string(encrypted)) + if err != nil { + return database.APIKey{}, "", xerrors.Errorf("parse encrypted API key: %w", err) + } + + // Lookup the API key so we can decrypt it. + keyID := object.Header.KeyID + key, err := db.GetAPIKeyByID(ctx, keyID) + if err != nil { + return database.APIKey{}, "", xerrors.Errorf("get API key by key ID: %w", err) + } + + // Decrypt using the hashed secret. + decrypted, err := object.Decrypt(key.HashedSecret) + if err != nil { + return database.APIKey{}, "", xerrors.Errorf("decrypt API key: %w", err) + } + + // Unmarshal the payload. + var payload encryptedAPIKeyPayload + if err := json.Unmarshal(decrypted, &payload); err != nil { + return database.APIKey{}, "", xerrors.Errorf("unmarshal decrypted payload: %w", err) + } + + // Validate expiry. + if payload.ExpiresAt.Before(database.Now()) { + return database.APIKey{}, "", xerrors.New("encrypted API key expired") + } + + // Validate that the key matches the one we got from the DB. + gotKeyID, gotKeySecret, err := httpmw.SplitAPIToken(payload.APIKey) + if err != nil { + return database.APIKey{}, "", xerrors.Errorf("split API key: %w", err) + } + gotHashedSecret := sha256.Sum256([]byte(gotKeySecret)) + if gotKeyID != key.ID || !bytes.Equal(key.HashedSecret, gotHashedSecret[:]) { + return database.APIKey{}, "", xerrors.New("encrypted API key does not match key in database") + } + + return key, payload.APIKey, nil } diff --git a/coderd/workspaceapps_internal_test.go b/coderd/workspaceapps_internal_test.go new file mode 100644 index 0000000000000..9b98d427e1b26 --- /dev/null +++ b/coderd/workspaceapps_internal_test.go @@ -0,0 +1,100 @@ +package coderd + +import ( + "context" + "crypto/sha256" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/databasefake" + "github.com/coder/coder/testutil" +) + +func TestAPIKeyEncryption(t *testing.T) { + t.Parallel() + + generateAPIKey := func(t *testing.T, db database.Store) (keyID, keySecret string, hashedSecret []byte, data encryptedAPIKeyPayload) { + keyID, keySecret, err := generateAPIKeyIDSecret() + require.NoError(t, err) + + hashedSecretArray := sha256.Sum256([]byte(keySecret)) + data = encryptedAPIKeyPayload{ + APIKey: keyID + "-" + keySecret, + ExpiresAt: database.Now().Add(24 * time.Hour), + } + + return keyID, keySecret, hashedSecretArray[:], data + } + insertAPIKey := func(t *testing.T, db database.Store, keyID string, hashedSecret []byte) { + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + _, err := db.InsertAPIKey(ctx, database.InsertAPIKeyParams{ + ID: keyID, + HashedSecret: hashedSecret, + }) + require.NoError(t, err) + } + + t.Run("OK", func(t *testing.T) { + t.Parallel() + db := databasefake.New() + keyID, _, hashedSecret, data := generateAPIKey(t, db) + insertAPIKey(t, db, keyID, hashedSecret) + + encrypted, err := encryptAPIKey(data) + require.NoError(t, err) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + key, token, err := decryptAPIKey(ctx, db, encrypted) + require.NoError(t, err) + require.Equal(t, keyID, key.ID) + require.Equal(t, hashedSecret[:], key.HashedSecret) + require.Equal(t, data.APIKey, token) + }) + + t.Run("Verifies", func(t *testing.T) { + t.Parallel() + + t.Run("Expiry", func(t *testing.T) { + t.Parallel() + db := databasefake.New() + keyID, _, hashedSecret, data := generateAPIKey(t, db) + insertAPIKey(t, db, keyID, hashedSecret) + + data.ExpiresAt = database.Now().Add(-1 * time.Hour) + encrypted, err := encryptAPIKey(data) + require.NoError(t, err) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + _, _, err = decryptAPIKey(ctx, db, encrypted) + require.Error(t, err) + require.ErrorContains(t, err, "expired") + }) + + t.Run("KeyMatches", func(t *testing.T) { + t.Parallel() + db := databasefake.New() + keyID, _, _, data := generateAPIKey(t, db) + hashedSecret := sha256.Sum256([]byte("wrong")) + insertAPIKey(t, db, keyID, hashedSecret[:]) + + encrypted, err := encryptAPIKey(data) + require.NoError(t, err) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + _, _, err = decryptAPIKey(ctx, db, encrypted) + require.Error(t, err) + require.ErrorContains(t, err, "error in crypto") + }) + }) +} diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index 164ceb2b15de6..e111863f578df 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -2,11 +2,13 @@ package coderd_test import ( "context" + "encoding/json" "fmt" "io" "net" "net/http" "net/url" + "strings" "testing" "time" @@ -31,12 +33,46 @@ const ( proxyTestAppQuery = "query=true" proxyTestAppBody = "hello world" proxyTestFakeAppName = "fake" + + proxyTestSubdomain = "test.coder.com" ) +func TestGetAppHost(t *testing.T) { + t.Parallel() + + cases := []string{"", "test.coder.com"} + for _, c := range cases { + c := c + name := c + if name == "" { + name = "Empty" + } + t.Run(name, func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{ + AppHostname: c, + }) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + // Should not leak to unauthenticated users. + host, err := client.GetAppHost(ctx) + require.Error(t, err) + require.Equal(t, "", host.Host) + + _ = coderdtest.CreateFirstUser(t, client) + host, err = client.GetAppHost(ctx) + require.NoError(t, err) + require.Equal(t, c, host.Host) + }) + } +} + // setupProxyTest creates a workspace with an agent and some apps. It returns a -// codersdk client, the workspace, and the port number the test listener is -// running on. -func setupProxyTest(t *testing.T, workspaceMutators ...func(*codersdk.CreateWorkspaceRequest)) (*codersdk.Client, uuid.UUID, codersdk.Workspace, uint16) { +// codersdk client, the first user, the workspace, and the port number the test +// listener is running on. +func setupProxyTest(t *testing.T, workspaceMutators ...func(*codersdk.CreateWorkspaceRequest)) (*codersdk.Client, codersdk.CreateFirstUserResponse, codersdk.Workspace, uint16) { // #nosec ln, err := net.Listen("tcp", ":0") require.NoError(t, err) @@ -58,6 +94,7 @@ func setupProxyTest(t *testing.T, workspaceMutators ...func(*codersdk.CreateWork require.True(t, ok) client := coderdtest.New(t, &coderdtest.Options{ + AppHostname: proxyTestSubdomain, IncludeProvisionerDaemon: true, AgentStatsRefreshInterval: time.Millisecond * 100, MetricsCacheRefreshInterval: time.Millisecond * 100, @@ -126,12 +163,12 @@ func setupProxyTest(t *testing.T, workspaceMutators ...func(*codersdk.CreateWork } client.HTTPClient.Transport = transport - return client, user.OrganizationID, workspace, uint16(tcpAddr.Port) + return client, user, workspace, uint16(tcpAddr.Port) } func TestWorkspaceAppsProxyPath(t *testing.T) { t.Parallel() - client, orgID, workspace, _ := setupProxyTest(t) + client, firstUser, workspace, _ := setupProxyTest(t) t.Run("LoginWithoutAuth", func(t *testing.T) { t.Parallel() @@ -147,6 +184,7 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { require.NoError(t, err) defer resp.Body.Close() + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) loc, err := resp.Location() require.NoError(t, err) require.True(t, loc.Query().Has("message")) @@ -156,7 +194,7 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { t.Run("NoAccessShould404", func(t *testing.T) { t.Parallel() - userClient := coderdtest.CreateAnotherUser(t, client, orgID, rbac.RoleMember()) + userClient := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID, rbac.RoleMember()) userClient.HTTPClient.CheckRedirect = client.HTTPClient.CheckRedirect userClient.HTTPClient.Transport = client.HTTPClient.Transport @@ -225,9 +263,302 @@ func TestWorkspaceAppsProxyPath(t *testing.T) { }) } +func TestWorkspaceApplicationAuth(t *testing.T) { + t.Parallel() + + // The OK test checks the entire end-to-end flow of authentication. + t.Run("End-to-End", func(t *testing.T) { + t.Parallel() + + client, firstUser, workspace, _ := setupProxyTest(t) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + // Get the current user and API key. + user, err := client.User(ctx, codersdk.Me) + require.NoError(t, err) + currentAPIKey, err := client.GetAPIKey(ctx, firstUser.UserID.String(), strings.Split(client.SessionToken, "-")[0]) + require.NoError(t, err) + + // Try to load the application without authentication. + subdomain := fmt.Sprintf("%s--%s--%s--%s", proxyTestAppName, proxyTestAgentName, workspace.Name, user.Username) + u, err := url.Parse(fmt.Sprintf("http://%s.%s/test", subdomain, proxyTestSubdomain)) + require.NoError(t, err) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil) + require.NoError(t, err) + resp, err := client.HTTPClient.Do(req) + require.NoError(t, err) + resp.Body.Close() + + // Check that the Location is correct. + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + gotLocation, err := resp.Location() + require.NoError(t, err) + require.Equal(t, client.URL.Host, gotLocation.Host) + require.Equal(t, "/api/v2/applications/auth-redirect", gotLocation.Path) + require.Equal(t, u.String(), gotLocation.Query().Get("redirect_uri")) + + // Load the application auth-redirect endpoint. + qp := codersdk.WithQueryParams(map[string]string{ + "redirect_uri": u.String(), + }) + resp, err = client.Request(ctx, http.MethodGet, "/api/v2/applications/auth-redirect", nil, qp) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + gotLocation, err = resp.Location() + require.NoError(t, err) + + // Copy the query parameters and then check equality. + u.RawQuery = gotLocation.RawQuery + require.Equal(t, u, gotLocation) + + // Verify the API key is set. + var encryptedAPIKey string + for k, v := range gotLocation.Query() { + // The query parameter may change dynamically in the future and is + // not exported, so we just use a fuzzy check instead. + if strings.Contains(k, "api_key") { + encryptedAPIKey = v[0] + } + } + require.NotEmpty(t, encryptedAPIKey, "no API key was set in the query parameters") + + // Decrypt the API key by following the request. + t.Log("navigating to: ", gotLocation.String()) + req, err = http.NewRequestWithContext(ctx, "GET", gotLocation.String(), nil) + require.NoError(t, err) + resp, err = client.HTTPClient.Do(req) + require.NoError(t, err) + resp.Body.Close() + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + cookies := resp.Cookies() + require.Len(t, cookies, 1) + apiKey := cookies[0].Value + + // Fetch the API key. + apiKeyInfo, err := client.GetAPIKey(ctx, firstUser.UserID.String(), strings.Split(apiKey, "-")[0]) + require.NoError(t, err) + require.Equal(t, user.ID, apiKeyInfo.UserID) + require.Equal(t, codersdk.LoginTypePassword, apiKeyInfo.LoginType) + require.WithinDuration(t, currentAPIKey.ExpiresAt, apiKeyInfo.ExpiresAt, 5*time.Second) + + // Verify the API key permissions + appClient := codersdk.New(client.URL) + appClient.SessionToken = apiKey + appClient.HTTPClient.CheckRedirect = client.HTTPClient.CheckRedirect + appClient.HTTPClient.Transport = client.HTTPClient.Transport + + var ( + canCreateApplicationConnect = "can-create-application_connect" + canReadUserMe = "can-read-user-me" + ) + authRes, err := appClient.CheckAuthorization(ctx, codersdk.AuthorizationRequest{ + Checks: map[string]codersdk.AuthorizationCheck{ + canCreateApplicationConnect: { + Object: codersdk.AuthorizationObject{ + ResourceType: "application_connect", + OwnerID: "me", + OrganizationID: firstUser.OrganizationID.String(), + ResourceID: uuid.NewString(), + }, + Action: "create", + }, + canReadUserMe: { + Object: codersdk.AuthorizationObject{ + ResourceType: "user", + OwnerID: "me", + ResourceID: firstUser.UserID.String(), + }, + Action: "read", + }, + }, + }) + require.NoError(t, err) + + require.True(t, authRes[canCreateApplicationConnect]) + require.False(t, authRes[canReadUserMe]) + + // Load the application page with the API key set. + gotLocation, err = resp.Location() + require.NoError(t, err) + t.Log("navigating to: ", gotLocation.String()) + req, err = http.NewRequestWithContext(ctx, "GET", gotLocation.String(), nil) + require.NoError(t, err) + req.Header.Set(codersdk.SessionCustomHeader, apiKey) + resp, err = client.HTTPClient.Do(req) + require.NoError(t, err) + resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode) + }) + + t.Run("VerifyRedirectURI", func(t *testing.T) { + t.Parallel() + + client, _, _, _ := setupProxyTest(t) + + cases := []struct { + name string + redirectURI string + status int + messageContains string + }{ + { + name: "NoRedirectURI", + redirectURI: "", + status: http.StatusBadRequest, + messageContains: "Missing redirect_uri query parameter", + }, + { + name: "InvalidURI", + redirectURI: "not a url", + status: http.StatusBadRequest, + messageContains: "Invalid redirect_uri query parameter", + }, + { + name: "NotMatchAppHostname", + redirectURI: "https://app--agent--workspace--user.not-a-match.com", + status: http.StatusBadRequest, + messageContains: "The redirect_uri query parameter must be a valid app subdomain", + }, + { + name: "InvalidAppURL", + redirectURI: "https://not-an-app." + proxyTestSubdomain, + status: http.StatusBadRequest, + messageContains: "The redirect_uri query parameter must be a valid app subdomain", + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + t.Parallel() + qp := map[string]string{} + if c.redirectURI != "" { + qp["redirect_uri"] = c.redirectURI + } + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + resp, err := client.Request(ctx, http.MethodGet, "/api/v2/applications/auth-redirect", nil, codersdk.WithQueryParams(qp)) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusBadRequest, resp.StatusCode) + }) + } + }) +} + +// This test ensures that the subdomain handler does nothing if --app-hostname +// is not set by the admin. +func TestWorkspaceAppsProxySubdomainPassthrough(t *testing.T) { + t.Parallel() + + // No AppHostname set. + client := coderdtest.New(t, &coderdtest.Options{ + AppHostname: "", + }) + firstUser := coderdtest.CreateFirstUser(t, client) + + // Configure the HTTP client to always route all requests to the coder test + // server. + defaultTransport, ok := http.DefaultTransport.(*http.Transport) + require.True(t, ok) + transport := defaultTransport.Clone() + transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { + return (&net.Dialer{}).DialContext(ctx, network, client.URL.Host) + } + client.HTTPClient.Transport = transport + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + uri := fmt.Sprintf("http://app--agent--workspace--username.%s/api/v2/users/me", proxyTestSubdomain) + resp, err := client.Request(ctx, http.MethodGet, uri, nil) + require.NoError(t, err) + defer resp.Body.Close() + + // Should look like a codersdk.User response. + require.Equal(t, http.StatusOK, resp.StatusCode) + var user codersdk.User + err = json.NewDecoder(resp.Body).Decode(&user) + require.NoError(t, err) + require.Equal(t, firstUser.UserID, user.ID) +} + +// This test ensures that the subdomain handler blocks the request if it looks +// like a workspace app request but the configured app hostname differs from the +// request, or the request is not a valid app subdomain but the hostname +// matches. +func TestWorkspaceAppsProxySubdomainBlocked(t *testing.T) { + t.Parallel() + + setup := func(t *testing.T, appHostname string) *codersdk.Client { + client := coderdtest.New(t, &coderdtest.Options{ + AppHostname: appHostname, + }) + _ = coderdtest.CreateFirstUser(t, client) + + // Configure the HTTP client to always route all requests to the coder test + // server. + defaultTransport, ok := http.DefaultTransport.(*http.Transport) + require.True(t, ok) + transport := defaultTransport.Clone() + transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { + return (&net.Dialer{}).DialContext(ctx, network, client.URL.Host) + } + client.HTTPClient.Transport = transport + + return client + } + + t.Run("NotMatchingHostname", func(t *testing.T) { + t.Parallel() + client := setup(t, "test."+proxyTestSubdomain) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + uri := fmt.Sprintf("http://app--agent--workspace--username.%s/api/v2/users/me", proxyTestSubdomain) + resp, err := client.Request(ctx, http.MethodGet, uri, nil) + require.NoError(t, err) + defer resp.Body.Close() + + // Should have an error response. + require.Equal(t, http.StatusNotFound, resp.StatusCode) + var resBody codersdk.Response + err = json.NewDecoder(resp.Body).Decode(&resBody) + require.NoError(t, err) + require.Contains(t, resBody.Message, "does not accept application requests on this hostname") + }) + + t.Run("InvalidSubdomain", func(t *testing.T) { + t.Parallel() + client := setup(t, proxyTestSubdomain) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + uri := fmt.Sprintf("http://not-an-app-subdomain.%s/api/v2/users/me", proxyTestSubdomain) + resp, err := client.Request(ctx, http.MethodGet, uri, nil) + require.NoError(t, err) + defer resp.Body.Close() + + // Should have an error response. + require.Equal(t, http.StatusBadRequest, resp.StatusCode) + var resBody codersdk.Response + err = json.NewDecoder(resp.Body).Decode(&resBody) + require.NoError(t, err) + require.Contains(t, resBody.Message, "Could not parse subdomain application URL") + }) +} + func TestWorkspaceAppsProxySubdomain(t *testing.T) { t.Parallel() - client, orgID, workspace, port := setupProxyTest(t) + client, firstUser, workspace, port := setupProxyTest(t) // proxyURL generates a URL for the proxy subdomain. The default path is a // slash. @@ -254,8 +585,7 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { AgentName: proxyTestAgentName, WorkspaceName: workspace.Name, Username: me.Username, - BaseHostname: "test.coder.com", - }.String() + }.String() + "." + proxyTestSubdomain actualPath := "/" query := "" @@ -274,35 +604,10 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) { }).String() } - t.Run("LoginWithoutAuth", func(t *testing.T) { - t.Parallel() - unauthedClient := codersdk.New(client.URL) - unauthedClient.HTTPClient.CheckRedirect = client.HTTPClient.CheckRedirect - unauthedClient.HTTPClient.Transport = client.HTTPClient.Transport - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - resp, err := unauthedClient.Request(ctx, http.MethodGet, proxyURL(t, proxyTestAppName), nil) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) - - loc, err := resp.Location() - require.NoError(t, err) - require.True(t, loc.Query().Has("message")) - require.False(t, loc.Query().Has("redirect")) - - expectedURL := *client.URL - expectedURL.Path = "/login" - loc.RawQuery = "" - require.Equal(t, &expectedURL, loc) - }) - t.Run("NoAccessShould401", func(t *testing.T) { t.Parallel() - userClient := coderdtest.CreateAnotherUser(t, client, orgID, rbac.RoleMember()) + userClient := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID, rbac.RoleMember()) userClient.HTTPClient.CheckRedirect = client.HTTPClient.CheckRedirect userClient.HTTPClient.Transport = client.HTTPClient.Transport diff --git a/codersdk/authorization.go b/codersdk/authorization.go new file mode 100644 index 0000000000000..edf24c64f4431 --- /dev/null +++ b/codersdk/authorization.go @@ -0,0 +1,70 @@ +package codersdk + +import ( + "context" + "encoding/json" + "net/http" +) + +type AuthorizationResponse map[string]bool + +// AuthorizationRequest is a structure instead of a map because +// go-playground/validate can only validate structs. If you attempt to pass +// a map into 'httpapi.Read', you will get an invalid type error. +type AuthorizationRequest struct { + // Checks is a map keyed with an arbitrary string to a permission check. + // The key can be any string that is helpful to the caller, and allows + // multiple permission checks to be run in a single request. + // The key ensures that each permission check has the same key in the + // response. + Checks map[string]AuthorizationCheck `json:"checks"` +} + +// AuthorizationCheck is used to check if the currently authenticated user (or +// the specified user) can do a given action to a given set of objects. +type AuthorizationCheck struct { + // Object can represent a "set" of objects, such as: + // - All workspaces in an organization + // - All workspaces owned by me + // - All workspaces across the entire product + // When defining an object, use the most specific language when possible to + // produce the smallest set. Meaning to set as many fields on 'Object' as + // you can. Example, if you want to check if you can update all workspaces + // owned by 'me', try to also add an 'OrganizationID' to the settings. + // Omitting the 'OrganizationID' could produce the incorrect value, as + // workspaces have both `user` and `organization` owners. + Object AuthorizationObject `json:"object"` + // Action can be 'create', 'read', 'update', or 'delete' + Action string `json:"action"` +} + +type AuthorizationObject struct { + // ResourceType is the name of the resource. + // './coderd/rbac/object.go' has the list of valid resource types. + ResourceType string `json:"resource_type"` + // OwnerID (optional) is a user_id. It adds the set constraint to all resources owned + // by a given user. + OwnerID string `json:"owner_id,omitempty"` + // OrganizationID (optional) is an organization_id. It adds the set constraint to + // all resources owned by a given organization. + OrganizationID string `json:"organization_id,omitempty"` + // ResourceID (optional) reduces the set to a singular resource. This assigns + // a resource ID to the resource type, eg: a single workspace. + // The rbac library will not fetch the resource from the database, so if you + // are using this option, you should also set the 'OwnerID' and 'OrganizationID' + // if possible. Be as specific as possible using all the fields relevant. + ResourceID string `json:"resource_id,omitempty"` +} + +func (c *Client) CheckAuthorization(ctx context.Context, req AuthorizationRequest) (AuthorizationResponse, error) { + res, err := c.Request(ctx, http.MethodPost, "/api/v2/authcheck", req) + if err != nil { + return nil, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return AuthorizationResponse{}, readBodyAsError(res) + } + var resp AuthorizationResponse + return resp, json.NewDecoder(res.Body).Decode(&resp) +} diff --git a/codersdk/client.go b/codersdk/client.go index fd788d1d9f089..3c0e6a7b0d3ce 100644 --- a/codersdk/client.go +++ b/codersdk/client.go @@ -42,11 +42,21 @@ type Client struct { URL *url.URL } -type requestOption func(*http.Request) +type RequestOption func(*http.Request) + +func WithQueryParams(params map[string]string) RequestOption { + return func(r *http.Request) { + q := r.URL.Query() + for k, v := range params { + q.Add(k, v) + } + r.URL.RawQuery = q.Encode() + } +} // Request performs an HTTP request with the body provided. // The caller is responsible for closing the response body. -func (c *Client) Request(ctx context.Context, method, path string, body interface{}, opts ...requestOption) (*http.Response, error) { +func (c *Client) Request(ctx context.Context, method, path string, body interface{}, opts ...RequestOption) (*http.Response, error) { serverURL, err := c.URL.Parse(path) if err != nil { return nil, xerrors.Errorf("parse url: %w", err) diff --git a/codersdk/pagination.go b/codersdk/pagination.go index c059266dd34c4..7e6cf8d1dd41f 100644 --- a/codersdk/pagination.go +++ b/codersdk/pagination.go @@ -28,7 +28,7 @@ type Pagination struct { // asRequestOption returns a function that can be used in (*Client).Request. // It modifies the request query parameters. -func (p Pagination) asRequestOption() requestOption { +func (p Pagination) asRequestOption() RequestOption { return func(r *http.Request) { q := r.URL.Query() if p.AfterID != uuid.Nil { diff --git a/codersdk/roles.go b/codersdk/roles.go index 34e2800ac5a42..45f27f15bcb8a 100644 --- a/codersdk/roles.go +++ b/codersdk/roles.go @@ -46,16 +46,3 @@ func (c *Client) ListOrganizationRoles(ctx context.Context, org uuid.UUID) ([]As var roles []AssignableRoles return roles, json.NewDecoder(res.Body).Decode(&roles) } - -func (c *Client) CheckPermissions(ctx context.Context, checks UserAuthorizationRequest) (UserAuthorizationResponse, error) { - res, err := c.Request(ctx, http.MethodPost, fmt.Sprintf("/api/v2/users/%s/authorization", Me), checks) - if err != nil { - return nil, err - } - defer res.Body.Close() - if res.StatusCode != http.StatusOK { - return nil, readBodyAsError(res) - } - var roles UserAuthorizationResponse - return roles, json.NewDecoder(res.Body).Decode(&roles) -} diff --git a/codersdk/users.go b/codersdk/users.go index c504b391c3c59..6dae49c7c1a6b 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -54,7 +54,8 @@ type User struct { } type APIKey struct { - ID string `json:"id" validate:"required"` + ID string `json:"id" validate:"required"` + // NOTE: do not ever return the HashedSecret UserID uuid.UUID `json:"user_id" validate:"required"` LastUsed time.Time `json:"last_used" validate:"required"` ExpiresAt time.Time `json:"expires_at" validate:"required"` @@ -102,56 +103,6 @@ type UserRoles struct { OrganizationRoles map[uuid.UUID][]string `json:"organization_roles"` } -type UserAuthorizationResponse map[string]bool - -// UserAuthorizationRequest is a structure instead of a map because -// go-playground/validate can only validate structs. If you attempt to pass -// a map into 'httpapi.Read', you will get an invalid type error. -type UserAuthorizationRequest struct { - // Checks is a map keyed with an arbitrary string to a permission check. - // The key can be any string that is helpful to the caller, and allows - // multiple permission checks to be run in a single request. - // The key ensures that each permission check has the same key in the - // response. - Checks map[string]UserAuthorization `json:"checks"` -} - -// UserAuthorization is used to check if a user can do a given action -// to a given set of objects. -type UserAuthorization struct { - // Object can represent a "set" of objects, such as: - // - All workspaces in an organization - // - All workspaces owned by me - // - All workspaces across the entire product - // When defining an object, use the most specific language when possible to - // produce the smallest set. Meaning to set as many fields on 'Object' as - // you can. Example, if you want to check if you can update all workspaces - // owned by 'me', try to also add an 'OrganizationID' to the settings. - // Omitting the 'OrganizationID' could produce the incorrect value, as - // workspaces have both `user` and `organization` owners. - Object UserAuthorizationObject `json:"object"` - // Action can be 'create', 'read', 'update', or 'delete' - Action string `json:"action"` -} - -type UserAuthorizationObject struct { - // ResourceType is the name of the resource. - // './coderd/rbac/object.go' has the list of valid resource types. - ResourceType string `json:"resource_type"` - // OwnerID (optional) is a user_id. It adds the set constraint to all resources owned - // by a given user. - OwnerID string `json:"owner_id,omitempty"` - // OrganizationID (optional) is an organization_id. It adds the set constraint to - // all resources owned by a given organization. - OrganizationID string `json:"organization_id,omitempty"` - // ResourceID (optional) reduces the set to a singular resource. This assigns - // a resource ID to the resource type, eg: a single workspace. - // The rbac library will not fetch the resource from the database, so if you - // are using this option, you should also set the 'OwnerID' and 'OrganizationID' - // if possible. Be as specific as possible using all the fields relevant. - ResourceID string `json:"resource_id,omitempty"` -} - // LoginWithPasswordRequest enables callers to authenticate with email and password. type LoginWithPasswordRequest struct { Email string `json:"email" validate:"required,email"` diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index 8933908104bbb..a9a2c68e9c5c7 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -51,7 +51,7 @@ type WorkspaceOptions struct { // asRequestOption returns a function that can be used in (*Client).Request. // It modifies the request query parameters. -func (o WorkspaceOptions) asRequestOption() requestOption { +func (o WorkspaceOptions) asRequestOption() RequestOption { return func(r *http.Request) { q := r.URL.Query() if o.IncludeDeleted { @@ -74,7 +74,7 @@ func (c *Client) DeletedWorkspace(ctx context.Context, id uuid.UUID) (Workspace, return c.getWorkspace(ctx, id, o.asRequestOption()) } -func (c *Client) getWorkspace(ctx context.Context, id uuid.UUID, opts ...requestOption) (Workspace, error) { +func (c *Client) getWorkspace(ctx context.Context, id uuid.UUID, opts ...RequestOption) (Workspace, error) { res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/workspaces/%s", id), nil, opts...) if err != nil { return Workspace{}, err @@ -254,7 +254,7 @@ type WorkspaceFilter struct { // asRequestOption returns a function that can be used in (*Client).Request. // It modifies the request query parameters. -func (f WorkspaceFilter) asRequestOption() requestOption { +func (f WorkspaceFilter) asRequestOption() RequestOption { return func(r *http.Request) { var params []string // Make sure all user input is quoted to ensure it's parsed as a single @@ -314,3 +314,28 @@ func (c *Client) WorkspaceByOwnerAndName(ctx context.Context, owner string, name var workspace Workspace return workspace, json.NewDecoder(res.Body).Decode(&workspace) } + +type GetAppHostResponse struct { + Host string `json:"host"` +} + +// GetAppHost returns the site-wide application wildcard hostname without the +// leading "*.", e.g. "apps.coder.com". Apps are accessible at: +// "------.", e.g. +// "my-app--agent--workspace--username.apps.coder.com". +// +// If the app host is not set, the response will contain an empty string. +func (c *Client) GetAppHost(ctx context.Context) (GetAppHostResponse, error) { + res, err := c.Request(ctx, http.MethodGet, "/api/v2/applications/host", nil) + if err != nil { + return GetAppHostResponse{}, err + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return GetAppHostResponse{}, readBodyAsError(res) + } + + var host GetAppHostResponse + return host, json.NewDecoder(res.Body).Decode(&host) +} diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 278b5acd0c52c..eff87221d8f27 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -53,7 +53,12 @@ func New(ctx context.Context, options *Options) (*API, error) { 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, + }) + api.AGPL.APIHandler.Group(func(r chi.Router) { r.Get("/entitlements", api.serveEntitlements) r.Route("/licenses", func(r chi.Router) { diff --git a/enterprise/coderd/coderdenttest/coderdenttest_test.go b/enterprise/coderd/coderdenttest/coderdenttest_test.go index ccea80cf9b968..4d5b391abf601 100644 --- a/enterprise/coderd/coderdenttest/coderdenttest_test.go +++ b/enterprise/coderd/coderdenttest/coderdenttest_test.go @@ -20,6 +20,8 @@ func TestAuthorizeAllEndpoints(t *testing.T) { t.Parallel() client, _, api := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ Options: &coderdtest.Options{ + // Required for any subdomain-based proxy tests to pass. + AppHostname: "test.coder.com", Authorizer: &coderdtest.RecordingAuthorizer{}, IncludeProvisionerDaemon: true, }, diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 5a4cc2a2e9825..c907ecccca10d 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -108,14 +108,10 @@ export const getAuthMethods = async (): Promise => { return response.data } -export const checkUserPermissions = async ( - userId: string, - params: TypesGen.UserAuthorizationRequest, -): Promise => { - const response = await axios.post( - `/api/v2/users/${userId}/authorization`, - params, - ) +export const checkAuthorization = async ( + params: TypesGen.AuthorizationRequest, +): Promise => { + const response = await axios.post(`/api/v2/authcheck`, params) return response.data } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 192b33166ca3b..8031530ef04af 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -103,6 +103,28 @@ export interface AuthMethods { readonly oidc: boolean } +// From codersdk/authorization.go +export interface AuthorizationCheck { + readonly object: AuthorizationObject + readonly action: string +} + +// From codersdk/authorization.go +export interface AuthorizationObject { + readonly resource_type: string + readonly owner_id?: string + readonly organization_id?: string + readonly resource_id?: string +} + +// From codersdk/authorization.go +export interface AuthorizationRequest { + readonly checks: Record +} + +// From codersdk/authorization.go +export type AuthorizationResponse = Record + // From codersdk/workspaceagents.go export interface AzureInstanceIdentityToken { readonly signature: string @@ -242,6 +264,11 @@ export interface GenerateAPIKeyResponse { readonly key: string } +// From codersdk/workspaces.go +export interface GetAppHostResponse { + readonly host: string +} + // From codersdk/gitsshkey.go export interface GitSSHKey { readonly user_id: string @@ -497,28 +524,6 @@ export interface User { readonly avatar_url: string } -// From codersdk/users.go -export interface UserAuthorization { - readonly object: UserAuthorizationObject - readonly action: string -} - -// From codersdk/users.go -export interface UserAuthorizationObject { - readonly resource_type: string - readonly owner_id?: string - readonly organization_id?: string - readonly resource_id?: string -} - -// From codersdk/users.go -export interface UserAuthorizationRequest { - readonly checks: Record -} - -// From codersdk/users.go -export type UserAuthorizationResponse = Record - // From codersdk/users.go export interface UserRoles { readonly roles: string[] diff --git a/site/src/components/Navbar/Navbar.test.tsx b/site/src/components/Navbar/Navbar.test.tsx index 7b2c65d12f4ca..e58ad661ba981 100644 --- a/site/src/components/Navbar/Navbar.test.tsx +++ b/site/src/components/Navbar/Navbar.test.tsx @@ -2,11 +2,7 @@ import { render, screen, waitFor } from "@testing-library/react" import { App } from "app" import { Language } from "components/NavbarView/NavbarView" import { rest } from "msw" -import { - MockEntitlementsWithAuditLog, - MockMemberPermissions, - MockUser, -} from "testHelpers/renderHelpers" +import { MockEntitlementsWithAuditLog, MockMemberPermissions } from "testHelpers/renderHelpers" import { server } from "testHelpers/server" /** @@ -47,7 +43,7 @@ describe("Navbar", () => { it("does not show Audit Log link when not permitted via role", async () => { // set permissions to Member (can't audit) server.use( - rest.post(`/api/v2/users/${MockUser.id}/authorization`, async (req, res, ctx) => { + rest.post("/api/v2/authcheck", async (req, res, ctx) => { return res(ctx.status(200), ctx.json(MockMemberPermissions)) }), ) diff --git a/site/src/pages/TemplatePage/TemplatePage.test.tsx b/site/src/pages/TemplatePage/TemplatePage.test.tsx index 27252bdd7bdfd..99cb2ea71930a 100644 --- a/site/src/pages/TemplatePage/TemplatePage.test.tsx +++ b/site/src/pages/TemplatePage/TemplatePage.test.tsx @@ -7,7 +7,6 @@ import { MockMemberPermissions, MockTemplate, MockTemplateVersion, - MockUser, MockWorkspaceResource, renderWithAuth, } from "../../testHelpers/renderHelpers" @@ -47,7 +46,7 @@ describe("TemplatePage", () => { it("does not allow a member to delete a template", () => { // get member-level permissions server.use( - rest.post(`/api/v2/users/${MockUser.id}/authorization`, async (req, res, ctx) => { + rest.post("/api/v2/authcheck", async (req, res, ctx) => { return res(ctx.status(200), ctx.json(MockMemberPermissions)) }), ) diff --git a/site/src/pages/TemplatesPage/TemplatesPage.test.tsx b/site/src/pages/TemplatesPage/TemplatesPage.test.tsx index d5b2ec4477c7d..a68c3f2244f07 100644 --- a/site/src/pages/TemplatesPage/TemplatesPage.test.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPage.test.tsx @@ -21,7 +21,7 @@ describe("TemplatesPage", () => { rest.get("/api/v2/organizations/:organizationId/templates", (req, res, ctx) => { return res(ctx.status(200), ctx.json([])) }), - rest.post("/api/v2/users/:userId/authorization", async (req, res, ctx) => { + rest.post("/api/v2/authcheck", async (req, res, ctx) => { return res( ctx.status(200), ctx.json({ @@ -51,7 +51,7 @@ describe("TemplatesPage", () => { rest.get("/api/v2/organizations/:organizationId/templates", (req, res, ctx) => { return res(ctx.status(200), ctx.json([])) }), - rest.post("/api/v2/users/:userId/authorization", async (req, res, ctx) => { + rest.post("/api/v2/authcheck", async (req, res, ctx) => { return res( ctx.status(200), ctx.json({ diff --git a/site/src/pages/UsersPage/UsersPage.test.tsx b/site/src/pages/UsersPage/UsersPage.test.tsx index bda2fa27943fe..1c5559fedaca0 100644 --- a/site/src/pages/UsersPage/UsersPage.test.tsx +++ b/site/src/pages/UsersPage/UsersPage.test.tsx @@ -184,7 +184,7 @@ describe("UsersPage", () => { it("does not show 'Create user' button to unauthorized user", async () => { server.use( - rest.post("/api/v2/users/:userId/authorization", async (req, res, ctx) => { + rest.post("/api/v2/authcheck", async (req, res, ctx) => { const permissions = Object.keys(permissionsToCheck) const response = permissions.reduce((obj, permission) => { return { diff --git a/site/src/pages/WorkspacePage/WorkspacePage.tsx b/site/src/pages/WorkspacePage/WorkspacePage.tsx index 6b243aaa500a2..15db6c13694f1 100644 --- a/site/src/pages/WorkspacePage/WorkspacePage.tsx +++ b/site/src/pages/WorkspacePage/WorkspacePage.tsx @@ -16,7 +16,6 @@ import { firstOrItem } from "../../util/array" import { pageTitle } from "../../util/page" import { canExtendDeadline, canReduceDeadline, maxDeadline, minDeadline } from "../../util/schedule" import { getFaviconByStatus } from "../../util/workspace" -import { selectUser } from "../../xServices/auth/authSelectors" import { XServiceContext } from "../../xServices/StateContext" import { workspaceMachine } from "../../xServices/workspace/workspaceXService" import { workspaceScheduleBannerMachine } from "../../xServices/workspaceSchedule/workspaceScheduleBannerXService" @@ -27,18 +26,11 @@ export const WorkspacePage: FC = () => { const { username: usernameQueryParam, workspace: workspaceQueryParam } = useParams() const username = firstOrItem(usernameQueryParam, null) const workspaceName = firstOrItem(workspaceQueryParam, null) - const { t } = useTranslation("workspacePage") - const xServices = useContext(XServiceContext) - const me = useSelector(xServices.authXService, selectUser) const featureVisibility = useSelector(xServices.entitlementsXService, selectFeatureVisibility) - const [workspaceState, workspaceSend] = useMachine(workspaceMachine, { - context: { - userId: me?.id, - }, - }) + const [workspaceState, workspaceSend] = useMachine(workspaceMachine) const { workspace, getWorkspaceError, diff --git a/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx b/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx index b20be6b5ded83..347bf5dbadfba 100644 --- a/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx +++ b/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx @@ -1,15 +1,13 @@ -import { useMachine, useSelector } from "@xstate/react" +import { useMachine } from "@xstate/react" import { scheduleToAutoStart } from "pages/WorkspaceSchedulePage/schedule" import { ttlMsToAutoStop } from "pages/WorkspaceSchedulePage/ttl" -import React, { useContext, useEffect, useState } from "react" +import React, { useEffect, useState } from "react" import { Navigate, useNavigate, useParams } from "react-router-dom" import * as TypesGen from "../../api/typesGenerated" import { ErrorSummary } from "../../components/ErrorSummary/ErrorSummary" import { FullScreenLoader } from "../../components/Loader/FullScreenLoader" import { WorkspaceScheduleForm } from "../../components/WorkspaceScheduleForm/WorkspaceScheduleForm" import { firstOrItem } from "../../util/array" -import { selectUser } from "../../xServices/auth/authSelectors" -import { XServiceContext } from "../../xServices/StateContext" import { workspaceSchedule } from "../../xServices/workspaceSchedule/workspaceScheduleXService" import { formValuesToAutoStartRequest, formValuesToTTLRequest } from "./formToRequest" @@ -24,15 +22,7 @@ export const WorkspaceSchedulePage: React.FC = () => { const navigate = useNavigate() const username = firstOrItem(usernameQueryParam, null) const workspaceName = firstOrItem(workspaceQueryParam, null) - - const xServices = useContext(XServiceContext) - const me = useSelector(xServices.authXService, selectUser) - - const [scheduleState, scheduleSend] = useMachine(workspaceSchedule, { - context: { - userId: me?.id, - }, - }) + const [scheduleState, scheduleSend] = useMachine(workspaceSchedule) const { checkPermissionsError, submitScheduleError, getWorkspaceError, permissions, workspace } = scheduleState.context diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index 3c3b238dec1c6..ef49aa899cb30 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -79,7 +79,7 @@ export const handlers = [ rest.get("/api/v2/users/roles", async (req, res, ctx) => { return res(ctx.status(200), ctx.json(M.MockSiteRoles)) }), - rest.post("/api/v2/users/:userId/authorization", async (req, res, ctx) => { + rest.post("/api/v2/authcheck", async (req, res, ctx) => { const permissions = Object.keys(permissionsToCheck) const response = permissions.reduce((obj, permission) => { return { diff --git a/site/src/xServices/auth/authXService.ts b/site/src/xServices/auth/authXService.ts index 1fa3040e9fb96..784f532ced47c 100644 --- a/site/src/xServices/auth/authXService.ts +++ b/site/src/xServices/auth/authXService.ts @@ -114,7 +114,7 @@ export const authMachine = data: undefined } checkPermissions: { - data: TypesGen.UserAuthorizationResponse + data: TypesGen.AuthorizationResponse } getSSHKey: { data: TypesGen.GitSSHKey @@ -438,12 +438,8 @@ export const authMachine = return API.updateUserPassword(context.me.id, event.data) }, - checkPermissions: async (context) => { - if (!context.me) { - throw new Error("No current user found") - } - - return API.checkUserPermissions(context.me.id, { + checkPermissions: async () => { + return API.checkAuthorization({ checks: permissionsToCheck, }) }, diff --git a/site/src/xServices/workspace/workspaceXService.ts b/site/src/xServices/workspace/workspaceXService.ts index 0db51aa54fcb5..b863b6499eea6 100644 --- a/site/src/xServices/workspace/workspaceXService.ts +++ b/site/src/xServices/workspace/workspaceXService.ts @@ -39,7 +39,6 @@ export interface WorkspaceContext { // permissions permissions?: Permissions checkPermissionsError?: Error | unknown - userId?: string } export type WorkspaceEvent = @@ -117,7 +116,7 @@ export const workspaceMachine = createMachine( data: TypesGen.WorkspaceBuild[] } checkPermissions: { - data: TypesGen.UserAuthorizationResponse + data: TypesGen.AuthorizationResponse } }, }, @@ -604,12 +603,12 @@ export const workspaceMachine = createMachine( } }, checkPermissions: async (context) => { - if (context.workspace && context.userId) { - return await API.checkUserPermissions(context.userId, { + if (context.workspace) { + return await API.checkAuthorization({ checks: permissionsToCheck(context.workspace), }) } else { - throw Error("Cannot check permissions without both workspace and user id") + throw Error("Cannot check permissions workspace id") } }, }, diff --git a/site/src/xServices/workspaceSchedule/workspaceScheduleXService.ts b/site/src/xServices/workspaceSchedule/workspaceScheduleXService.ts index be45a8fdcb075..67fa104736424 100644 --- a/site/src/xServices/workspaceSchedule/workspaceScheduleXService.ts +++ b/site/src/xServices/workspaceSchedule/workspaceScheduleXService.ts @@ -21,8 +21,6 @@ export interface WorkspaceScheduleContext { * machine is partially influenced by workspaceXService. */ workspace?: TypesGen.Workspace - // permissions - userId?: string permissions?: Permissions checkPermissionsError?: Error | unknown submitScheduleError?: Error | unknown @@ -178,8 +176,8 @@ export const workspaceSchedule = createMachine( return await API.getWorkspaceByOwnerAndName(event.username, event.workspaceName) }, checkPermissions: async (context) => { - if (context.workspace && context.userId) { - return await API.checkUserPermissions(context.userId, { + if (context.workspace) { + return await API.checkAuthorization({ checks: permissionsToCheck(context.workspace), }) } else {