feat: remove site wide perms from creating a workspace

coder · Emyrk · Apr 9, 2025 · Apr 8, 2025 · Apr 8, 2025 · Apr 8, 2025
commit bf891a43181e8a84b799cfd4570f739d90ba16f1
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -1148,7 +1148,6 @@ func New(options *Options) *API {
 				})
 				r.Route("/{user}", func(r chi.Router) {
 					r.Group(func(r chi.Router) {
-						r.Use(httpmw.ExtractUserParam(options.Database))
 						// Creating workspaces does not require permissions on the user, only the
 						// organization member. This endpoint should match the authz story of
 						// postWorkspacesByOrganization

diff --git a/coderd/httpmw/organizationparam.go b/coderd/httpmw/organizationparam.go
@@ -110,51 +110,61 @@ type OrganizationMember struct {
 func ExtractOrganizationMemberParam(db database.Store) 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()
-			// We need to resolve the `{user}` URL parameter so that we can get the userID and
-			// username.  We do this as SystemRestricted since the caller might have permission
-			// to access the OrganizationMember object, but *not* the User object.  So, it is
-			// very important that we do not add the User object to the request context or otherwise
-			// leak it to the API handler.
-			// nolint:gocritic
-			user, ok := extractUserContext(dbauthz.AsSystemRestricted(ctx), db, rw, r)
-			if !ok {
-				return
-			}
 			organization := OrganizationParam(r)
-
-			organizationMember, err := database.ExpectOne(db.OrganizationMembers(ctx, database.OrganizationMembersParams{
-				OrganizationID: organization.ID,
-				UserID:         user.ID,
-				IncludeSystem:  false,
-			}))
-			if httpapi.Is404Error(err) {
-				httpapi.ResourceNotFound(rw)
-				return
-			}
-			if err != nil {
-				httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
-					Message: "Internal error fetching organization member.",
-					Detail:  err.Error(),
-				})
+			organizationMember, ok := ExtractOrganizationMemberContext(rw, r, db, organization.ID)
+			if !ok {
 				return
 			}
 
-			ctx = context.WithValue(ctx, organizationMemberParamContextKey{}, OrganizationMember{
-				OrganizationMember: organizationMember.OrganizationMember,
-				// Here we're making two exceptions to the rule about not leaking data about the user
-				// to the API handler, which is to include the username and avatar URL.
-				// If the caller has permission to read the OrganizationMember, then we're explicitly
-				// saying here that they also have permission to see the member's username and avatar.
-				// This is OK!
-				//
-				// API handlers need this information for audit logging and returning the owner's
-				// username in response to creating a workspace. Additionally, the frontend consumes
-				// the Avatar URL and this allows the FE to avoid an extra request.
-				Username:  user.Username,
-				AvatarURL: user.AvatarURL,
-			})
+			ctx := r.Context()
+			ctx = context.WithValue(ctx, organizationMemberParamContextKey{}, organizationMember)
 			next.ServeHTTP(rw, r.WithContext(ctx))
 		})
 	}
 }
+
+func ExtractOrganizationMemberContext(rw http.ResponseWriter, r *http.Request, db database.Store, orgID uuid.UUID) (OrganizationMember, bool) {
+	ctx := r.Context()
+
+	// We need to resolve the `{user}` URL parameter so that we can get the userID and
+	// username.  We do this as SystemRestricted since the caller might have permission
+	// to access the OrganizationMember object, but *not* the User object.  So, it is
+	// very important that we do not add the User object to the request context or otherwise
+	// leak it to the API handler.
+	// nolint:gocritic
+	user, ok := extractUserContext(dbauthz.AsSystemRestricted(ctx), db, rw, r)
+	if !ok {
+		return OrganizationMember{}, false
+	}
+
+	organizationMember, err := database.ExpectOne(db.OrganizationMembers(ctx, database.OrganizationMembersParams{
+		OrganizationID: orgID,
+		UserID:         user.ID,
+		IncludeSystem:  false,
+	}))
+	if httpapi.Is404Error(err) {
+		httpapi.ResourceNotFound(rw)
+		return OrganizationMember{}, false
+	}
+	if err != nil {
+		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
+			Message: "Internal error fetching organization member.",
+			Detail:  err.Error(),
+		})
+		return OrganizationMember{}, false
+	}
+	return OrganizationMember{
+		OrganizationMember: organizationMember.OrganizationMember,
+		// Here we're making two exceptions to the rule about not leaking data about the user
+		// to the API handler, which is to include the username and avatar URL.
+		// If the caller has permission to read the OrganizationMember, then we're explicitly
+		// saying here that they also have permission to see the member's username and avatar.
+		// This is OK!
+		//
+		// API handlers need this information for audit logging and returning the owner's
+		// username in response to creating a workspace. Additionally, the frontend consumes
+		// the Avatar URL and this allows the FE to avoid an extra request.
+		Username:  user.Username,
+		AvatarURL: user.AvatarURL,
+	}, true
+}
diff --git a/coderd/httpmw/userparam.go b/coderd/httpmw/userparam.go
@@ -56,7 +56,7 @@ func extractUserContext(ctx context.Context, db database.Store, rw http.Response
 		httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
 			Message: "\"user\" must be provided.",
 		})
-		return database.User{}, true
+		return database.User{}, false
 	}
 
 	if userQuery == "me" {

diff --git a/coderd/workspaces.go b/coderd/workspaces.go
@@ -406,30 +406,46 @@ func (api *API) postUserWorkspaces(rw http.ResponseWriter, r *http.Request) {
 		ctx     = r.Context()
 		apiKey  = httpmw.APIKey(r)
 		auditor = api.Auditor.Load()
-		user    = httpmw.UserParam(r)
 	)
 
+	var req codersdk.CreateWorkspaceRequest
+	if !httpapi.Read(ctx, rw, r, &req) {
+		return
+	}
+
+	// No middleware exists to fetch the user from. This endpoint needs to fetch
+	// the organization member, which requires the organization. Which can be
+	// sourced from the template.
+	//
+	// TODO: This code gets called twice for each workspace build request.
+	//   This is inefficient and costs at most 2 extra RTTs to the DB.
+	//   This can be optimized. It exists as it is now for code simplicity.
+	template, ok := requestTemplate(ctx, rw, req, api.Database)
+	if !ok {
+		return
+	}
+
+	member, ok := httpmw.ExtractOrganizationMemberContext(rw, r, api.Database, template.OrganizationID)
+	if !ok {
+		return
+	}
+
 	aReq, commitAudit := audit.InitRequest[database.WorkspaceTable](rw, &audit.RequestParams{
 		Audit:   *auditor,
 		Log:     api.Logger,
 		Request: r,
 		Action:  database.AuditActionCreate,
 		AdditionalFields: audit.AdditionalFields{
-			WorkspaceOwner: user.Username,
+			WorkspaceOwner: member.Username,
 		},
 	})
 
 	defer commitAudit()
 
-	var req codersdk.CreateWorkspaceRequest
-	if !httpapi.Read(ctx, rw, r, &req) {
-		return
-	}
-
 	owner := workspaceOwner{
-		ID:        user.ID,
-		Username:  user.Username,
-		AvatarURL: user.AvatarURL,
+		ID:        member.UserID,
+		Username:  member.Username,
+		AvatarURL: member.AvatarURL,
 	}
 	createWorkspace(ctx, aReq, apiKey.UserID, api, owner, req, rw, r)
 }
@@ -450,65 +466,8 @@ func createWorkspace(
 	rw http.ResponseWriter,
 	r *http.Request,
 ) {
-	// If we were given a `TemplateVersionID`, we need to determine the `TemplateID` from it.
-	templateID := req.TemplateID
-	if templateID == uuid.Nil {
-		templateVersion, err := api.Database.GetTemplateVersionByID(ctx, req.TemplateVersionID)
-		if httpapi.Is404Error(err) {
-			httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
-				Message: fmt.Sprintf("Template version %q doesn't exist.", templateID.String()),
-				Validations: []codersdk.ValidationError{{
-					Field:  "template_version_id",
-					Detail: "template not found",
-				}},
-			})
-			return
-		}
-		if err != nil {
-			httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
-				Message: "Internal error fetching template version.",
-				Detail:  err.Error(),
-			})
-			return
-		}
-		if templateVersion.Archived {
-			httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
-				Message: "Archived template versions cannot be used to make a workspace.",
-				Validations: []codersdk.ValidationError{
-					{
-						Field:  "template_version_id",
-						Detail: "template version archived",
-					},
-				},
-			})
-			return
-		}
-
-		templateID = templateVersion.TemplateID.UUID
-	}
-
-	template, err := api.Database.GetTemplateByID(ctx, templateID)
-	if httpapi.Is404Error(err) {
-		httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
-			Message: fmt.Sprintf("Template %q doesn't exist.", templateID.String()),
-			Validations: []codersdk.ValidationError{{
-				Field:  "template_id",
-				Detail: "template not found",
-			}},
-		})
-		return
-	}
-	if err != nil {
-		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
-			Message: "Internal error fetching template.",
-			Detail:  err.Error(),
-		})
-		return
-	}
-	if template.Deleted {
-		httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{
-			Message: fmt.Sprintf("Template %q has been deleted!", template.Name),
-		})
+	template, ok := requestTemplate(ctx, rw, req, api.Database)
+	if !ok {
 		return
 	}
 
@@ -776,6 +735,72 @@ func createWorkspace(
 	httpapi.Write(ctx, rw, http.StatusCreated, w)
 }
 
+func requestTemplate(ctx context.Context, rw http.ResponseWriter, req codersdk.CreateWorkspaceRequest, db database.Store) (database.Template, bool) {
+	// If we were given a `TemplateVersionID`, we need to determine the `TemplateID` from it.
+	templateID := req.TemplateID
+
+	if templateID == uuid.Nil {
+		templateVersion, err := db.GetTemplateVersionByID(ctx, req.TemplateVersionID)
+		if httpapi.Is404Error(err) {
+			httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
+				Message: fmt.Sprintf("Template version %q doesn't exist.", req.TemplateVersionID),
+				Validations: []codersdk.ValidationError{{
+					Field:  "template_version_id",
+					Detail: "template not found",
+				}},
+			})
+			return database.Template{}, false
+		}
+		if err != nil {
+			httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
+				Message: "Internal error fetching template version.",
+				Detail:  err.Error(),
+			})
+			return database.Template{}, false
+		}
+		if templateVersion.Archived {
+			httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
+				Message: "Archived template versions cannot be used to make a workspace.",
+				Validations: []codersdk.ValidationError{
+					{
+						Field:  "template_version_id",
+						Detail: "template version archived",
+					},
+				},
+			})
+			return database.Template{}, false
+		}
+
+		templateID = templateVersion.TemplateID.UUID
+	}
+
+	template, err := db.GetTemplateByID(ctx, templateID)
+	if httpapi.Is404Error(err) {
+		httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
+			Message: fmt.Sprintf("Template %q doesn't exist.", templateID),
+			Validations: []codersdk.ValidationError{{
+				Field:  "template_id",
+				Detail: "template not found",
+			}},
+		})
+		return database.Template{}, false
+	}
+	if err != nil {
+		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
+			Message: "Internal error fetching template.",
+			Detail:  err.Error(),
+		})
+		return database.Template{}, false
+	}
+	if template.Deleted {
+		httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{
+			Message: fmt.Sprintf("Template %q has been deleted!", template.Name),
+		})
+		return database.Template{}, false
+	}
+	return template, true
+}
+
 func (api *API) notifyWorkspaceCreated(
 	ctx context.Context,
 	receiverID uuid.UUID,