mirror of
https://github.com/coder/coder.git
synced 2026-09-21 20:51:01 +08:00
perf(coderd): cheaper chatd org membership checks (#24361)
This change reuses the authenticated subject's existing organization membership information during chat creation instead of issuing an `OrganizationMembers` query. The current query is still correct, so this is not required for correctness. However, `workspaceapps` already answers the same question more cheaply from the request's RBAC subject. This extracts that logic into `rbac.Subject.HasOrganizationMembership` and reuses it in both places, removing an extra database lookup from chat creation without changing the authorization behavior. I'm currently debugging a Coder agents scaletest regression where a run on April 2, 2026 with 4800 concurrent chat creations passed, while the same run on April 15, 2026 does not. We could stagger chat creation to reduce the burst, but I'd rather understand why this bottleneck appeared in the first place so we can keep making small hot-path improvements like this one instead of only smoothing over the symptom.
This commit is contained in:
+3
-8
@@ -405,20 +405,15 @@ func (api *API) postChats(rw http.ResponseWriter, r *http.Request) {
|
||||
})
|
||||
return
|
||||
}
|
||||
orgMembers, err := api.Database.OrganizationMembers(ctx, database.OrganizationMembersParams{
|
||||
OrganizationID: req.OrganizationID,
|
||||
UserID: apiKey.UserID,
|
||||
IncludeSystem: false,
|
||||
GithubUserID: 0,
|
||||
})
|
||||
isMember, err := httpmw.UserAuthorization(ctx).HasOrganizationMembership(req.OrganizationID)
|
||||
if err != nil {
|
||||
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
|
||||
Message: "Failed to validate organization membership.",
|
||||
Detail: err.Error(),
|
||||
Detail: xerrors.Errorf("check organization membership: %w", err).Error(),
|
||||
})
|
||||
return
|
||||
}
|
||||
if len(orgMembers) == 0 {
|
||||
if !isMember {
|
||||
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
|
||||
Message: "You are not a member of the specified organization.",
|
||||
})
|
||||
|
||||
@@ -12,6 +12,7 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/ammario/tlru"
|
||||
"github.com/google/uuid"
|
||||
"github.com/open-policy-agent/opa/ast"
|
||||
"github.com/open-policy-agent/opa/v1/rego"
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
@@ -172,6 +173,25 @@ func (s Subject) SafeRoleNames() []RoleIdentifier {
|
||||
return s.Roles.Names()
|
||||
}
|
||||
|
||||
// HasOrganizationMembership reports whether the subject has explicit
|
||||
// membership in organizationID through an org-scoped role. Site-wide roles
|
||||
// alone do not count as organization membership.
|
||||
func (s Subject) HasOrganizationMembership(organizationID uuid.UUID) (bool, error) {
|
||||
roles, err := s.Roles.Expand()
|
||||
if err != nil {
|
||||
return false, xerrors.Errorf("expand user authorization roles: %w", err)
|
||||
}
|
||||
|
||||
organizationIDString := organizationID.String()
|
||||
for _, role := range roles {
|
||||
if _, ok := role.ByOrgID[organizationIDString]; ok {
|
||||
return true, nil
|
||||
}
|
||||
}
|
||||
|
||||
return false, nil
|
||||
}
|
||||
|
||||
type Authorizer interface {
|
||||
// Authorize will authorize the given subject to perform the given action
|
||||
// on the given object. Authorize is pure and deterministic with respect to
|
||||
|
||||
@@ -372,18 +372,16 @@ func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *rbac.Subj
|
||||
return false, warnings, nil
|
||||
}
|
||||
|
||||
// Check if the user is a member of the same organization as the workspace
|
||||
// Check if the user is a member of the same organization as the workspace.
|
||||
workspaceOrgID := dbReq.Workspace.OrganizationID
|
||||
expandedRoles, err := roles.Roles.Expand()
|
||||
isMember, err := roles.HasOrganizationMembership(workspaceOrgID)
|
||||
if err != nil {
|
||||
return false, warnings, xerrors.Errorf("expand roles: %w", err)
|
||||
return false, warnings, xerrors.Errorf("check organization membership: %w", err)
|
||||
}
|
||||
for _, role := range expandedRoles {
|
||||
if _, ok := role.ByOrgID[workspaceOrgID.String()]; ok {
|
||||
return true, []string{}, nil
|
||||
}
|
||||
if isMember {
|
||||
return true, []string{}, nil
|
||||
}
|
||||
// User is not a member of the workspace's organization
|
||||
// User is not a member of the workspace's organization.
|
||||
return false, warnings, nil
|
||||
case database.AppSharingLevelPublic:
|
||||
// We don't really care about scopes and stuff if it's public anyways.
|
||||
|
||||
Reference in New Issue
Block a user