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

Closes https://github.com/coder/coder/issues/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.
This commit is contained in:
Steven Masley
2025-05-08 14:41:17 -05:00
committed by GitHub
parent d93a9cfde2
commit d5360a6da0
6 changed files with 185 additions and 74 deletions
+12 -9
View File
@@ -1189,15 +1189,25 @@ 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),
@@ -1205,10 +1215,6 @@ func New(options *Options) *API {
)
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) {
+128 -24
View File
@@ -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))
})
+11
View File
@@ -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
+2 -2
View File
@@ -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) {
+25 -38
View File
@@ -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,
}
}