chore: fetch workspaces by username with organization permissions (#17707)

Emyrk · web-flow · commit d5360a6da000 · 2025-05-08T14:41:17.000-05:00
Closes #17691 `ExtractOrganizationMembersParam` will allow fetching a user with only organization permissions. If the user belongs to 0 orgs, then the user "does not exist" from an org perspective. But if you are a site-wide admin, then the user does exist.
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -1189,26 +1189,32 @@ func New(options *Options) *API {
 				})
 				r.Route("/{user}", func(r chi.Router) {
 					r.Group(func(r chi.Router) {
-						r.Use(httpmw.ExtractUserParamOptional(options.Database))
+						r.Use(httpmw.ExtractOrganizationMembersParam(options.Database, api.HTTPAuth.Authorize))
 						// Creating workspaces does not require permissions on the user, only the
 						// organization member. This endpoint should match the authz story of
 						// postWorkspacesByOrganization
 						r.Post("/workspaces", api.postUserWorkspaces)
+						r.Route("/workspace/{workspacename}", func(r chi.Router) {
+							r.Get("/", api.workspaceByOwnerAndName)
+							r.Get("/builds/{buildnumber}", api.workspaceBuildByBuildNumber)
+						})
+					})
+
+					r.Group(func(r chi.Router) {
+						r.Use(httpmw.ExtractUserParam(options.Database))
 
 						// Similarly to creating a workspace, evaluating parameters for a
 						// new workspace should also match the authz story of
 						// postWorkspacesByOrganization
+						// TODO: Do not require site wide read user permission. Make this work
+						//   with org member permissions.
 						r.Route("/templateversions/{templateversion}", func(r chi.Router) {
 							r.Use(
 								httpmw.ExtractTemplateVersionParam(options.Database),
 								httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentDynamicParameters),
 							)
 							r.Get("/parameters", api.templateVersionDynamicParameters)
 						})
-					})
-
-					r.Group(func(r chi.Router) {
-						r.Use(httpmw.ExtractUserParam(options.Database))
 
 						r.Post("/convert-login", api.postConvertLoginType)
 						r.Delete("/", api.deleteUser)
@@ -1250,10 +1256,7 @@ func New(options *Options) *API {
 							r.Get("/", api.organizationsByUser)
 							r.Get("/{organizationname}", api.organizationByUserAndName)
 						})
-						r.Route("/workspace/{workspacename}", func(r chi.Router) {
-							r.Get("/", api.workspaceByOwnerAndName)
-							r.Get("/builds/{buildnumber}", api.workspaceBuildByBuildNumber)
-						})
+
 						r.Get("/gitsshkey", api.gitSSHKey)
 						r.Put("/gitsshkey", api.regenerateGitSSHKey)
 						r.Route("/notifications", func(r chi.Router) {
diff --git a/coderd/httpmw/organizationparam.go b/coderd/httpmw/organizationparam.go
@@ -11,12 +11,15 @@ import (
 	"github.com/coder/coder/v2/coderd/database"
 	"github.com/coder/coder/v2/coderd/database/dbauthz"
 	"github.com/coder/coder/v2/coderd/httpapi"
+	"github.com/coder/coder/v2/coderd/rbac"
+	"github.com/coder/coder/v2/coderd/rbac/policy"
 	"github.com/coder/coder/v2/codersdk"
 )
 
 type (
-	organizationParamContextKey       struct{}
-	organizationMemberParamContextKey struct{}
+	organizationParamContextKey        struct{}
+	organizationMemberParamContextKey  struct{}
+	organizationMembersParamContextKey struct{}
 )
 
 // OrganizationParam returns the organization from the ExtractOrganizationParam handler.
@@ -38,6 +41,14 @@ func OrganizationMemberParam(r *http.Request) OrganizationMember {
 	return organizationMember
 }
 
+func OrganizationMembersParam(r *http.Request) OrganizationMembers {
+	organizationMembers, ok := r.Context().Value(organizationMembersParamContextKey{}).(OrganizationMembers)
+	if !ok {
+		panic("developer error: organization members param middleware not provided")
+	}
+	return organizationMembers
+}
+
 // ExtractOrganizationParam grabs an organization from the "organization" URL parameter.
 // This middleware requires the API key middleware higher in the call stack for authentication.
 func ExtractOrganizationParam(db database.Store) func(http.Handler) http.Handler {
@@ -111,35 +122,23 @@ func ExtractOrganizationMemberParam(db database.Store) func(http.Handler) http.H
 	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)
+			_, members, done := ExtractOrganizationMember(ctx, nil, rw, r, db, organization.ID)
+			if done {
 				return
 			}
-			if err != nil {
+
+			if len(members) != 1 {
 				httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
 					Message: "Internal error fetching organization member.",
-					Detail:  err.Error(),
+					// This is a developer error and should never happen.
+					Detail: fmt.Sprintf("Expected exactly one organization member, but got %d.", len(members)),
 				})
 				return
 			}
 
+			organizationMember := members[0]
+
 			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
@@ -151,8 +150,113 @@ func ExtractOrganizationMemberParam(db database.Store) func(http.Handler) http.H
 				// 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,
+				Username:  organizationMember.Username,
+				AvatarURL: organizationMember.AvatarURL,
+			})
+
+			next.ServeHTTP(rw, r.WithContext(ctx))
+		})
+	}
+}
+
+// ExtractOrganizationMember extracts all user memberships from the "user" URL
+// parameter. If orgID is uuid.Nil, then it will return all memberships for the
+// user, otherwise it will only return memberships to the org.
+//
+// If `user` is returned, that means the caller can use the data. This is returned because
+// it is possible to have a user with 0 organizations. So the user != nil, with 0 memberships.
+func ExtractOrganizationMember(ctx context.Context, auth func(r *http.Request, action policy.Action, object rbac.Objecter) bool, rw http.ResponseWriter, r *http.Request, db database.Store, orgID uuid.UUID) (*database.User, []database.OrganizationMembersRow, bool) {
+	// 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 nil, nil, true
+	}
+
+	organizationMembers, err := db.OrganizationMembers(ctx, database.OrganizationMembersParams{
+		OrganizationID: orgID,
+		UserID:         user.ID,
+		IncludeSystem:  false,
+	})
+	if httpapi.Is404Error(err) {
+		httpapi.ResourceNotFound(rw)
+		return nil, nil, true
+	}
+	if err != nil {
+		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
+			Message: "Internal error fetching organization member.",
+			Detail:  err.Error(),
+		})
+		return nil, nil, true
+	}
+
+	// Only return the user data if the caller can read the user object.
+	if auth != nil && auth(r, policy.ActionRead, user) {
+		return &user, organizationMembers, false
+	}
+
+	// If the user cannot be read and 0 memberships exist, throw a 404 to not
+	// leak the user existence.
+	if len(organizationMembers) == 0 {
+		httpapi.ResourceNotFound(rw)
+		return nil, nil, true
+	}
+
+	return nil, organizationMembers, false
+}
+
+type OrganizationMembers struct {
+	// User is `nil` if the caller is not allowed access to the site wide
+	// user object.
+	User *database.User
+	// Memberships can only be length 0 if `user != nil`. If `user == nil`, then
+	// memberships will be at least length 1.
+	Memberships []OrganizationMember
+}
+
+func (om OrganizationMembers) UserID() uuid.UUID {
+	if om.User != nil {
+		return om.User.ID
+	}
+
+	if len(om.Memberships) > 0 {
+		return om.Memberships[0].UserID
+	}
+	return uuid.Nil
+}
+
+// ExtractOrganizationMembersParam grabs all user organization memberships.
+// Only requires the "user" URL parameter.
+//
+// Use this if you want to grab as much information for a user as you can.
+// From an organization context, site wide user information might not available.
+func ExtractOrganizationMembersParam(db database.Store, auth func(r *http.Request, action policy.Action, object rbac.Objecter) bool) 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()
+
+			// Fetch all memberships
+			user, members, done := ExtractOrganizationMember(ctx, auth, rw, r, db, uuid.Nil)
+			if done {
+				return
+			}
+
+			orgMembers := make([]OrganizationMember, 0, len(members))
+			for _, organizationMember := range members {
+				orgMembers = append(orgMembers, OrganizationMember{
+					OrganizationMember: organizationMember.OrganizationMember,
+					Username:           organizationMember.Username,
+					AvatarURL:          organizationMember.AvatarURL,
+				})
+			}
+
+			ctx = context.WithValue(ctx, organizationMembersParamContextKey{}, OrganizationMembers{
+				User:        user,
+				Memberships: orgMembers,
 			})
 			next.ServeHTTP(rw, r.WithContext(ctx))
 		})
diff --git a/coderd/httpmw/organizationparam_test.go b/coderd/httpmw/organizationparam_test.go
@@ -16,6 +16,8 @@ import (
 	"github.com/coder/coder/v2/coderd/database/dbmem"
 	"github.com/coder/coder/v2/coderd/database/dbtime"
 	"github.com/coder/coder/v2/coderd/httpmw"
+	"github.com/coder/coder/v2/coderd/rbac"
+	"github.com/coder/coder/v2/coderd/rbac/policy"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/testutil"
 )
@@ -167,6 +169,10 @@ func TestOrganizationParam(t *testing.T) {
 			httpmw.ExtractOrganizationParam(db),
 			httpmw.ExtractUserParam(db),
 			httpmw.ExtractOrganizationMemberParam(db),
+			httpmw.ExtractOrganizationMembersParam(db, func(r *http.Request, _ policy.Action, _ rbac.Objecter) bool {
+				// Assume the caller cannot read the member
+				return false
+			}),
 		)
 		rtr.Get("/", func(rw http.ResponseWriter, r *http.Request) {
 			org := httpmw.OrganizationParam(r)
@@ -190,6 +196,11 @@ func TestOrganizationParam(t *testing.T) {
 			assert.NotEmpty(t, orgMem.OrganizationMember.UpdatedAt)
 			assert.NotEmpty(t, orgMem.OrganizationMember.UserID)
 			assert.NotEmpty(t, orgMem.OrganizationMember.Roles)
+
+			orgMems := httpmw.OrganizationMembersParam(r)
+			assert.NotZero(t, orgMems)
+			assert.Equal(t, orgMem.UserID, orgMems.Memberships[0].UserID)
+			assert.Nil(t, orgMems.User, "user data should not be available, hard coded false authorize")
 		})
 
 		// Try by ID
diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go
@@ -232,7 +232,7 @@ func (api *API) workspaceBuilds(rw http.ResponseWriter, r *http.Request) {
 // @Router /users/{user}/workspace/{workspacename}/builds/{buildnumber} [get]
 func (api *API) workspaceBuildByBuildNumber(rw http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
-	owner := httpmw.UserParam(r)
+	mems := httpmw.OrganizationMembersParam(r)
 	workspaceName := chi.URLParam(r, "workspacename")
 	buildNumber, err := strconv.ParseInt(chi.URLParam(r, "buildnumber"), 10, 32)
 	if err != nil {
@@ -244,7 +244,7 @@ func (api *API) workspaceBuildByBuildNumber(rw http.ResponseWriter, r *http.Requ
 	}
 
 	workspace, err := api.Database.GetWorkspaceByOwnerIDAndName(ctx, database.GetWorkspaceByOwnerIDAndNameParams{
-		OwnerID: owner.ID,
+		OwnerID: mems.UserID(),
 		Name:    workspaceName,
 	})
 	if httpapi.Is404Error(err) {
diff --git a/coderd/workspaces.go b/coderd/workspaces.go
@@ -253,7 +253,8 @@ func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) {
 // @Router /users/{user}/workspace/{workspacename} [get]
 func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
-	owner := httpmw.UserParam(r)
+
+	mems := httpmw.OrganizationMembersParam(r)
 	workspaceName := chi.URLParam(r, "workspacename")
 	apiKey := httpmw.APIKey(r)
 
@@ -273,12 +274,12 @@ func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
 	}
 
 	workspace, err := api.Database.GetWorkspaceByOwnerIDAndName(ctx, database.GetWorkspaceByOwnerIDAndNameParams{
-		OwnerID: owner.ID,
+		OwnerID: mems.UserID(),
 		Name:    workspaceName,
 	})
 	if includeDeleted && errors.Is(err, sql.ErrNoRows) {
 		workspace, err = api.Database.GetWorkspaceByOwnerIDAndName(ctx, database.GetWorkspaceByOwnerIDAndNameParams{
-			OwnerID: owner.ID,
+			OwnerID: mems.UserID(),
 			Name:    workspaceName,
 			Deleted: includeDeleted,
 		})
@@ -408,6 +409,7 @@ func (api *API) postUserWorkspaces(rw http.ResponseWriter, r *http.Request) {
 		ctx     = r.Context()
 		apiKey  = httpmw.APIKey(r)
 		auditor = api.Auditor.Load()
+		mems    = httpmw.OrganizationMembersParam(r)
 	)
 
 	var req codersdk.CreateWorkspaceRequest
@@ -416,17 +418,16 @@ func (api *API) postUserWorkspaces(rw http.ResponseWriter, r *http.Request) {
 	}
 
 	var owner workspaceOwner
-	// This user fetch is an optimization path for the most common case of creating a
-	// workspace for 'Me'.
-	//
-	// This is also required to allow `owners` to create workspaces for users
-	// that are not in an organization.
-	user, ok := httpmw.UserParamOptional(r)
-	if ok {
+	if mems.User != nil {
+		// This user fetch is an optimization path for the most common case of creating a
+		// workspace for 'Me'.
+		//
+		// This is also required to allow `owners` to create workspaces for users
+		// that are not in an organization.
 		owner = workspaceOwner{
-			ID:        user.ID,
-			Username:  user.Username,
-			AvatarURL: user.AvatarURL,
+			ID:        mems.User.ID,
+			Username:  mems.User.Username,
+			AvatarURL: mems.User.AvatarURL,
 		}
 	} else {
 		// A workspace can still be created if the caller can read the organization
@@ -443,35 +444,21 @@ func (api *API) postUserWorkspaces(rw http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		// We need to fetch the original user as a system user to fetch the
-		// user_id. 'ExtractUserContext' handles all cases like usernames,
-		// 'Me', etc.
-		// nolint:gocritic // The user_id needs to be fetched. This handles all those cases.
-		user, ok := httpmw.ExtractUserContext(dbauthz.AsSystemRestricted(ctx), api.Database, rw, r)
-		if !ok {
-			return
-		}
-
-		organizationMember, err := database.ExpectOne(api.Database.OrganizationMembers(ctx, database.OrganizationMembersParams{
-			OrganizationID: template.OrganizationID,
-			UserID:         user.ID,
-			IncludeSystem:  false,
-		}))
-		if httpapi.Is404Error(err) {
+		// If the caller can find the organization membership in the same org
+		// as the template, then they can continue.
+		orgIndex := slices.IndexFunc(mems.Memberships, func(mem httpmw.OrganizationMember) bool {
+			return mem.OrganizationID == template.OrganizationID
+		})
+		if orgIndex == -1 {
 			httpapi.ResourceNotFound(rw)
 			return
 		}
-		if err != nil {
-			httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
-				Message: "Internal error fetching organization member.",
-				Detail:  err.Error(),
-			})
-			return
-		}
+
+		member := mems.Memberships[orgIndex]
 		owner = workspaceOwner{
-			ID:        organizationMember.OrganizationMember.UserID,
-			Username:  organizationMember.Username,
-			AvatarURL: organizationMember.AvatarURL,
+			ID:        member.UserID,
+			Username:  member.Username,
+			AvatarURL: member.AvatarURL,
 		}
 	}
 
diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go
