feat: /api/v2/applications/host endpoint, PR comments

coder · deansheather · Sep 22, 2022 · Sep 19, 2022 · Sep 19, 2022 · Sep 19, 2022
commit a172cd57ea335ee0c3f35c75c18bad04f3052889
diff --git 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/coderd/coderd.go b/coderd/coderd.go
@@ -498,10 +498,15 @@ func New(options *Options) *API {
 				r.Use(apiKeyMiddleware)
 				r.Post("/", api.checkAuthorization)
 			})
-
-			r.Route("/application-auth", func(r chi.Router) {
-				// We do want to redirect to login if they are not
-				// authenticated.
+		})
+		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.getAppHost)
+			})
+			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

diff --git a/coderd/coderdtest/authorize.go b/coderd/coderdtest/authorize.go
@@ -45,6 +45,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
 		"GET:/api/v2/users/authmethods":    {NoAuthorize: true},
 		"POST:/api/v2/csp/reports":         {NoAuthorize: true},
 		"POST:/api/v2/authorization/can-i": {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},
 
@@ -239,8 +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/authorization/application-auth": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceAPIKey},
+		"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/database/dump.sql b/coderd/database/dump.sql
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
@@ -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
@@ -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
diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go
@@ -26,6 +26,8 @@ import (
 // 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{}
@@ -362,11 +364,6 @@ func apiTokenFromRequest(r *http.Request) string {
 		return cookie.Value
 	}
 
-	cookie, err = r.Cookie(DevURLSessionTokenCookie)
-	if err == nil && cookie.Value != "" {
-		return cookie.Value
-	}
-
 	// TODO: @emyrk in October 2022, remove this oldCookie check.
 	//	This is just to support the old cli for 1 release. Then everyone
 	//	must update.
@@ -385,6 +382,11 @@ func apiTokenFromRequest(r *http.Request) string {
 		return headerValue
 	}
 
+	cookie, err = r.Cookie(DevURLSessionTokenCookie)
+	if err == nil && cookie.Value != "" {
+		return cookie.Value
+	}
+
 	return ""
 }
 

diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go
@@ -37,6 +37,12 @@ const (
 	redirectURIQueryParam     = "redirect_uri"
 )
 
+func (api *API) getAppHost(rw http.ResponseWriter, r *http.Request) {
+	httpapi.Write(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) {
@@ -276,7 +282,7 @@ func (api *API) verifyWorkspaceApplicationAuth(rw http.ResponseWriter, r *http.R
 	redirectURI.Host = host
 
 	u := *api.AccessURL
-	u.Path = "/api/v2/authorization/application-auth"
+	u.Path = "/api/v2/applications/auth-redirect"
 	q := u.Query()
 	q.Add(redirectURIQueryParam, redirectURI.String())
 	u.RawQuery = q.Encode()
@@ -516,6 +522,9 @@ type encryptedAPIKeyPayload struct {
 	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.
 func encryptAPIKey(data encryptedAPIKeyPayload) (string, error) {
 	if data.APIKey == "" {
 		return "", xerrors.New("API key is empty")
@@ -532,7 +541,8 @@ func encryptAPIKey(data encryptedAPIKeyPayload) (string, error) {
 	}
 
 	// We use the hashed key secret as the encryption key. The hashed secret is
-	// stored in the API keys table.
+	// 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:
@@ -572,6 +582,7 @@ func encryptAPIKey(data encryptedAPIKeyPayload) (string, error) {
 	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 {

diff --git a/coderd/workspace_apps_internal_test.go → coderd/workspaceapps_internal_test.go b/coderd/workspace_apps_internal_test.go → coderd/workspaceapps_internal_test.go
diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go
@@ -37,6 +37,38 @@ const (
 	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 first user, the workspace, and the port number the test
 // listener is running on.
@@ -264,14 +296,14 @@ func TestWorkspaceApplicationAuth(t *testing.T) {
 		gotLocation, err := resp.Location()
 		require.NoError(t, err)
 		require.Equal(t, client.URL.Host, gotLocation.Host)
-		require.Equal(t, "/api/v2/authorization/application-auth", gotLocation.Path)
+		require.Equal(t, "/api/v2/applications/auth-redirect", gotLocation.Path)
 		require.Equal(t, u.String(), gotLocation.Query().Get("redirect_uri"))
 
-		// Load the application-auth endpoint.
-		qp := codersdk.AddQueryParams(map[string]string{
+		// 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/authorization/application-auth", nil, qp)
+		resp, err = client.Request(ctx, http.MethodGet, "/api/v2/applications/auth-redirect", nil, qp)
 		require.NoError(t, err)
 		defer resp.Body.Close()
 
@@ -411,7 +443,7 @@ func TestWorkspaceApplicationAuth(t *testing.T) {
 				ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
 				defer cancel()
 
-				resp, err := client.Request(ctx, http.MethodGet, "/api/v2/authorization/application-auth", nil, codersdk.AddQueryParams(qp))
+				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)

diff --git a/codersdk/client.go b/codersdk/client.go
@@ -44,7 +44,7 @@ type Client struct {
 
 type RequestOption func(*http.Request)
 
-func AddQueryParams(params map[string]string) RequestOption {
+func WithQueryParams(params map[string]string) RequestOption {
 	return func(r *http.Request) {
 		q := r.URL.Query()
 		for k, v := range params {

diff --git 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"`

diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go
@@ -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:
+// "<app-name>--<agent-name>--<workspace-name>--<username>.<app-host>", 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)
+}
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		COMMENT ON COLUMN api_keys.hashed_secret
		IS 'hashed_secret contains a SHA256 hash of the key secret. This is considered a secret and MUST NOT be returned from the API as it is used for API key encryption in app proxying code.';